On 2015-06-30 at 11:41:04 +0200, Daniel Borkmann <[email protected]> wrote:
> On 06/29/2015 02:58 AM, Vadim Kochan wrote:
> >Hi,
> >
> >This is the 1-st "try" version of how I see the protocol generation API for 
> >the
> >trafgen util as mz replacement (at least for better performance).
> >
> >I am sending this just because to get your feedback about conceptual idea,
> >and as soon as I got some basic working version I decided to share the
> >code just to get know if I am moving in the right direction.
> >
> >Added high-level command line protocol packet building intreface,
> >which allows to specify protocol parameters to build the header and
> >payload.
> >
> >Each protocol is represented by proto_gen struct which is responsible
> >only for providing field info (size, data) by name to trafgen's
> >low level packet generation layer.
> >
> >All packet generation routine is performed by the generic code in
> >trafgen.c which parses the command line, obtains proto name, param=value
> >list and calls the specific protocol handler to get protocol field info
> >by name, so the TX routine remains the same.
> >
> >The command line syntax looks like:
> >
> >     trafgen/trafgen --dev lo eth da = AA:BB:CC:DD:EE:FF 
> > sa=11:22:33:44:55:66, arp op=rep tip=192.168.1.1 -n 1
> >
> >so the first is proto name and after there are param value pairs which
> >are separated by space, in case if there are multiple protocols
> >specified - their should be separated by "," after last param value of
> >the previous protocol.
> >
> >I think the picture will be more clear after adding IP protocol with checksum
> >handling.
> 
> First of all, thanks for working on this, Vadim! I like seeing something like
> this integrated after we've resolved all outstanding issues. I'll certainly
> make trafgen also easier to use.

I can only second that. Very nice to see work in this direction, much
appreciated! Thanks Vadim.

> Before digging into the very details, I have a couple of high-level comments/
> thoughts. All the manual string parsing you are doing, isn't it easier to just
> extend the flex/bison files with the related protocol information?
> 
> I.e. I was thinking of 1) make the current configuration syntax also available
> for the direct command line interface, and after that 2) extend the flex/bison
> parser with L2, L3, etc information in a similar syntax as you did above (e.g.
> multiple packets could also here be defined with separator { ... }, if no 
> separator
> is provided, we assume a single packet). This would give the flexibility of 
> having
> a mz-like cmdline syntax and at the same time one could also use it in the
> config file. Do you see any major obstacles with that?
> 
> Regarding the default values, f.e. if we've specified only L3 information 
> (e.g.
> IPv4), but no L2 information, we should look up src/dst mac based on the 
> output
> interface resp. the neighbor cache. We should be careful with broadcasts, i.e.
> if no other information is available for determining a dst, only then we 
> should
> use broadcast (f.e. if only L2 was specified w/o a dst mac, etc); in all other
> cases we should try hard to resolve all needed information from the kernel.
> 
> Anything I've missed, Tobias? :)

I've only had time for a rough review so far. Essentially my review
comments also boil down to the remarks Daniel made above :)

- Extending the current configuration syntax/grammar to allow for
  additional protocol information.
- No manual string parsing (as in patch 08/10), but use the parser
  generated from the extended flex/bison grammar.
- I generally dislike the idea of giving default values to non-specified
  protocol fields (e.g. using broadcast as default eth dst field). This
  holds potential for a lot of unexpected behavior. IMO we should - as
  Daniel suggested - try to get the information base on output interface
  etc. or even more extreme treat this as an error (at least for
  mandatory fields).

Detailled comments will follow as replies to the individual patches or
to your replies.

Again, thanks a lot for taking the time to work on this.

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 [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to