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?
> 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?
> 2) trafgen -o lo --cpus 1 -n 1 '{ eth(da=11:22:33:44:55:66, da[0]=dinc()),
> tcp() }'
>
> M lo 54 1482259682s.24357875ns #2
> [ Eth MAC (00:00:00:00:00:00 => 11:22:33:44:55:66)
>
> 3) trafgen -o lo --cpus 1 -n 3 '{ eth(da=11:22:33:44:55:66, da[0]=dinc()),
> tcp() }'
>
> M lo 54 1482259851s.161018621ns #3
> [ Eth MAC (00:00:00:00:00:00 => 11:22:33:44:55:66)
>
> P lo 54 1482259851s.161032201ns #4
> [ Eth MAC (00:00:00:00:00:00 => 12:22:33:44:55:66)
>
> M lo 54 1482259851s.161033977ns #5
> [ Eth MAC (00:00:00:00:00:00 => 13:22:33:44:55:66)
>
> 4) trafgen -o lo --cpus 1 -n 3 '{ ipv4(da=1.2.3.4, da[0]=dinc()), tcp() }'
>
> < lo 54 1482265434s.453794790ns #1
> [ IPv4 Addr (127.0.0.1 => 1.2.3.4)
>
> < lo 54 1482265434s.453811528ns #2
> [ IPv4 Addr (127.0.0.1 => 2.2.3.4)
>
> < lo 54 1482265434s.453815331ns #3
> [ IPv4 Addr (127.0.0.1 => 3.2.3.4)
>
> 5) trafgen -o lo --cpus 1 -n 3 '{ ipv4(da=192.168.1.1, da[1]=dinc()), tcp() }'
>
> < lo 54 1482265603s.917104425ns #4
> [ IPv4 Addr (127.0.0.1 => 192.168.1.1)
>
> < lo 54 1482265603s.917122777ns #5
> [ IPv4 Addr (127.0.0.1 => 192.169.1.1)
>
> < lo 54 1482265603s.917127151ns #6
> [ IPv4 Addr (127.0.0.1 => 192.170.1.1)
> >
> > 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!
--
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.
