[netsniff-ng] Re: [PATCH v2 11/16] trafgen: parser: Add syntax to generate ARP header fields

[Tobias Klauser]

On 2016-01-26 at 09:47:32 +0100, Vadim Kochan  wrote:
> On Tue, Jan 26, 2016 at 10:25 AM, Tobias Klauser  wrote:
> > On 2016-01-26 at 00:11:53 +0100, Vadim Kochan  wrote:
> >> Add syntax to generate ARP header fields:
> >>
> >> { arp(op=req, sip=1.1.1.1, smac=11:22:33:44:55:66) }
> >> { arp() }
> >>
> >> Signed-off-by: Vadim Kochan 
> >> ---
> >>
> >>  %%
> >>
> >> @@ -107,7 +109,16 @@ mac  
> >> ({mac_hex}:{mac_hex}:{mac_hex}:{mac_hex}:{mac_hex}:{mac_hex})
> >>  "saddr"|"sa" { return K_SADDR; }
> >>  "prot"[o]?   { return K_PROT; }
> >>
> >
> > Shouldn't we allow to specify htype, ptype, hlen and plen as well (as a
> > user might want to set non-conforming values)?
> >
> Well meanwhile it was easier to implement for me the Ethernet-IPv4
> form of ARP (which is more generic and
> used by masezahn too if I am not mistaken), and it looks a little
> tricky to allow full-flexible ARP header crafting.
> I'd like to add such ability on later work, if I will not dig into
> mac80211 headers ... :-)

Ok, I see :-) But htpye/ptype should still be possible without having
dynamic header length, no?

> >> +"sha"|"smac" { return K_SHA; }
> >> +"spa"|"sip"  { return K_SPA; }
> >> +"tha"|"tmac" { return K_THA; }
> >> +"tpa"|"tip"  { return K_TPA; }
> >> +"req"{ return K_REQ; }
> >
> > Please add "request" as well.
> >
> Sure.
> 
> >>   ;
> >>
> >> +arp_proto
> >> + : arp '(' arp_param_list ')' { }
> >> + ;
> >> +
> >> +arp_param_list
> >> + : { }
> >> + | arp_field { }
> >> + | arp_field delimiter arp_param_list { }
> >> + ;
> >> +
> >> +arp_field
> >> + : K_OPER  skip_white '=' skip_white K_REQ
> >> + { proto_field_set_be16(hdr, ARP_OPER, ARPOP_REQUEST); }
> >> + | K_OPER  skip_white '=' skip_white K_RESP
> >> + { proto_field_set_be16(hdr, ARP_OPER, ARPOP_REPLY); }
> >
> > Would be nice to allow numeric values here as well (again, to be able to
> > specify values not conforming to the standard).
> >
> 
> Right I will change it to the form:
> 
> arp(request, ...)
> arp(reply, ...)
> arp(op=)

Looks good.

> Not sure if the following is also will be needed considering above's forms:
> 
> arp(op=request)

I'd keep it in for consistency reasons.

-- 
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 11/16] trafgen: parser: Add syntax to generate ARP header fields

[Vadim Kochan]

On Tue, Jan 26, 2016 at 10:25 AM, Tobias Klauser  wrote:
> On 2016-01-26 at 00:11:53 +0100, Vadim Kochan  wrote:
>> Add syntax to generate ARP header fields:
>>
>> { arp(op=req, sip=1.1.1.1, smac=11:22:33:44:55:66) }
>> { arp() }
>>
>> Signed-off-by: Vadim Kochan 
>> ---
>>
>>  %%
>>
>> @@ -107,7 +109,16 @@ mac  
>> ({mac_hex}:{mac_hex}:{mac_hex}:{mac_hex}:{mac_hex}:{mac_hex})
>>  "saddr"|"sa" { return K_SADDR; }
>>  "prot"[o]?   { return K_PROT; }
>>
>
> Shouldn't we allow to specify htype, ptype, hlen and plen as well (as a
> user might want to set non-conforming values)?
>
Well meanwhile it was easier to implement for me the Ethernet-IPv4
form of ARP (which is more generic and
used by masezahn too if I am not mistaken), and it looks a little
tricky to allow full-flexible ARP header crafting.
I'd like to add such ability on later work, if I will not dig into
mac80211 headers ... :-)

>> +"sha"|"smac" { return K_SHA; }
>> +"spa"|"sip"  { return K_SPA; }
>> +"tha"|"tmac" { return K_THA; }
>> +"tpa"|"tip"  { return K_TPA; }
>> +"req"{ return K_REQ; }
>
> Please add "request" as well.
>
Sure.

>>   ;
>>
>> +arp_proto
>> + : arp '(' arp_param_list ')' { }
>> + ;
>> +
>> +arp_param_list
>> + : { }
>> + | arp_field { }
>> + | arp_field delimiter arp_param_list { }
>> + ;
>> +
>> +arp_field
>> + : K_OPER  skip_white '=' skip_white K_REQ
>> + { proto_field_set_be16(hdr, ARP_OPER, ARPOP_REQUEST); }
>> + | K_OPER  skip_white '=' skip_white K_RESP
>> + { proto_field_set_be16(hdr, ARP_OPER, ARPOP_REPLY); }
>
> Would be nice to allow numeric values here as well (again, to be able to
> specify values not conforming to the standard).
>

Right I will change it to the form:

arp(request, ...)
arp(reply, ...)
arp(op=)

Not sure if the following is also will be needed considering above's forms:

arp(op=request)
>> --
>> 2.6.3
>>

-- 
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.