[netsniff-ng] Re: [PATCH v2 6/7] trafgen: parser: Allow to set function at field offset
On 2016-12-21 at 10:13:03 +0100, Vadim Kochan wrote: > On Wed, Dec 21, 2016 at 10:17:17AM +0100, Tobias Klauser wrote: > > On 2016-12-20 at 22:10:54 +0100, Vadim Kochan wrote: > > > On Tue, Dec 20, 2016 at 12:33:47PM +0100, Tobias Klauser wrote: [...] > > > > 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 ... Oops, sorry. I obviously didn't read the patch careful enough. Maybe you could add one of the examples to the man page as well to make it immediately obvious what the result will be? [...] > > > > 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. Great, thanks. Tobias -- 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.
[netsniff-ng] Re: [PATCH v2 6/7] trafgen: parser: Allow to set function at field offset
On Wed, Dec 21, 2016 at 10:17:17AM +0100, Tobias Klauser wrote:
> On 2016-12-20 at 22:10:54 +0100, Vadim Kochan 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 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.
> >
>
[netsniff-ng] Re: [PATCH v2 6/7] trafgen: parser: Allow to set function at field offset
On 2016-12-20 at 22:10:54 +0100, Vadim Kochan 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 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 =
[netsniff-ng] Re: [PATCH v2 6/7] trafgen: parser: Allow to set function at field offset
On Tue, Dec 20, 2016 at 12:33:47PM +0100, Tobias Klauser wrote:
> On 2016-12-18 at 10:52:49 +0100, Vadim Kochan 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):
...
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))
If you like it then can we 1st introduce current solution and after
just extend this to the above's syntax ?
>
> 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:
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 ...
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 trafg
[netsniff-ng] Re: [PATCH v2 6/7] trafgen: parser: Allow to set function at field offset
On 2016-12-18 at 10:52:49 +0100, Vadim Kochan 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).
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).
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?
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() })
which would be a bit more in line with the traditional trafgen syntax.
Let me think about this a bit more...
--
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.
