On Wed, Dec 21, 2016 at 10:17:17AM +0100, Tobias Klauser wrote: > On 2016-12-20 at 22:10:54 +0100, Vadim Kochan <vadi...@gmail.com> wrote: > > On Tue, Dec 20, 2016 at 12:33:47PM +0100, Tobias Klauser wrote: > > > On 2016-12-18 at 10:52:49 +0100, Vadim Kochan <vadi...@gmail.com> wrote: > > > > Extend proto field expression to: > > > > > > > > proto_field[{index}:{len}] = {func} > > > > > > I like the idea behind this very much, but I'm not particularly happy > > > with the syntax of this. > > > > > > First, I find it a bit confusing that the length is specified and not > > > the end index (as at least I would assume it i.e. like array indexing in > > > Python works). > > > > Well, I do not thing we need to rely on Python's syntax, actually > > I used such syntax from pcap filter (man pcap-filter): > > No, I don't think we need Python's syntax either. It's just what I'm > currently more used to - so it was rather a statement of personal > preference. Annd I didn't really know about the pcap filter synatax. In > any case, I'm also fine with how you currently implemented it. > > ICould you please mention the fact that it is inspired by pcap > filter syntax in the man page? > > > ... > > > > To access data inside the packet, use the following syntax: > > proto [ expr : size ] Proto is one of ether, fddi, tr, wlan, ppp, > > slip, link, ip, arp, rarp, tcp, udp, icmp, ip6 or radio, and indicates > > the protocol layer for the index operation. (ether, fddi, wlan, tr, > > ppp, slip and link all refer to the link layer. radio refers to the > > "radio header" added to some 802.11 captures.) Note that tcp, udp and > > other upper-layer protocol types only apply to IPv4, not IPv6 (this will > > be fixed in the future). The byte offset, relative to the indicated > > protocol layer, is given by expr. Size is optional and indicates the > > number of bytes in the field of interest; it can be either one, two, or > > four, and defaults to one. The length operator, indicated by the > > keyword len, gives the length of the packet. > > > > ... > > > > > > Second, the need to mention a field twice with > > > something like: > > > > > > eth(saddr=aa:bb:cc:dd:ee:ff, saddr[0]=dinc()) > > > > > > isn't very intuitive to understand IMO. And moreover, from the existing > > > syntax I'd assume some kind of function call semantics (like in C) with > > > the fields being parameters to the function. But indexing a function > > > parameter isn't really what I'm used to (but then again this is just > > > personal taste). > > > > Hm, to reduce using field assignment twice I think it is possible to > > extend proto dynamic functions with default value as parameter then > > it make possible to have a mix of index + function parameter like: > > > > eth(sa[0]=dinc(11:22:33:44:55:66)) > > > > with min & max drnd parameters: > > > > eth(sa[0]=dinc(100, 200, 11:22:33:44:55:66)) > > I think these are more confusing than the current solution. > > > If you like it then can we 1st introduce current solution and after > > just extend this to the above's syntax ? > > Yes, I guess we can introduce your currently proposed solution and then > extend upon it if need be. > > > > And third, it's not immediately intuitive to which item the index > > > refers, i.e. is the MAC address ab:bb:cc:dd:ee:ff afterwards or > > > aa:bb:cc:dd:ee:00? > > > > Indexing starts from 0 byte in the network order, please see a below > > examples of > > trafgen + netsniff-ng outputs: > > Ok. Could you please mention this fact (that indices start from byte 0 > in network order) in the man page? >
Hm, actually I did it in the man patch, but may be too shortly ... > > 1) trafgen -o lo --cpus 1 -n 1 '{ eth(da[0]=dinc(), da=11:22:33:44:55:66), > > tcp() }' > > > > P lo 54 1482259546s.934331688ns #1 > > [ Eth MAC (00:00:00:00:00:00 => 00:22:33:44:55:66) > > > > Hm, here you can see that if to specify 1st proto function and after a > > field value then looks like the value will be overwritten by function, not > > sure if it is a bug or feature ... > > Might be a bit confusing. But I'd say we can currently live with that. > Maybe it's worth to mention this fact in the man page too, i.e. that > functions are always applied after setting field values? > > > > ... > > > But then I currently don't immediately see a better way to implement > > > this behavior and I also see why you'd chosen the indexing to work with > > > length rather than end index. > > > > > > One option might be to let the user specify multi-byte/-word fields as a > > > C-array-like initializer list and support function calls therein, i.e. > > > the example above could be written as: > > > > > > eth(saddr={ 0xaa, 0xbb, 0xcc, 0xdd, 0xee, drnd() }) > > > > Hm, looks interesting, in such way it will possible to specify few > > functions at 1 shot, need to think how it would be possible to specify > > the value size. > > > > > > > > which would be a bit more in line with the traditional trafgen syntax. > > > > > > Let me think about this a bit more... > > > > I think we can have 1 or 3 different solutions: > > > > 1) Current > > > > 2) Current + function param default value > > > > 3) Array-like > > I'll go ahead and apply your series except for the man page patch. Could > you please send an update for this one incorporating the changes > mentioned above? > > From there on we could then think about implementing option 3 in > addition... > > Thanks a lot! Sure I will collect your comments and use them for man page update. Regards, Vadim Kochan -- You received this message because you are subscribed to the Google Groups "netsniff-ng" group. To unsubscribe from this group and stop receiving emails from it, send an email to netsniff-ng+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.