Re: [Linux-ATM-General] [RFC][PATCH 0/3] atm: br2682: Improve encapsulation checks and headroom allocation

[Pascal H. <pa...@pl...>]

chas williams - CONTRACTOR a écrit :
> 
> this looks fine at a glance.  i do wonder if:
> 
> *((u16 *) (skb->data + 8)) == 0
> 
> works on all platforms.  cpu's other than x86 sometimes have issues
> with arbitrary alignments of data.   it should probalby be a memcmp()
> but that should probably be fixed in a seperate patch.
[...]
> my only suggestion is that besides getting minheadroom correct, is
> that you should set hard_headerlen correctly as well when the interface
> is setup.  in br2684_setup_routed() you should also do something
> like netdev->hard_header_len = 2;
> 
> sometimes this should be 0 but you dont know the encapsulation until
> the vcc is registered so at this point you are best just picking the
> worst case.
> 
> also, for the bridged case we know that the hard_header_len is going to 
> atmost ETH_HLEN + 10 since we will also always prepend something.
> 
> this should keep the need to skb_realloc_headroom() to a minimum.

Patch 1 now updated to v2 and new patch 3 added. Patch 1 v2 unchanged.

[PATCH 1/3 v2] atm: br2684: Improve encapsulation checks

 - Check that the LLC header matches the expected payload type.
 - Check that the PAD field is 0x00-00 also in LLC bridged mode.
 - Check that the frame length in bridged mode is at least ETH_HLEN (14)
   without FCS or ETH_ZLEN + ETH_FCS_LEN (64) with FCS.
 - Trim the FCS bytes only if the PID is ethernet with FCS.
 - Check the data length before reading the IP header version field in
   VC-MUX routed mode.

[Patch 2/3 v2] atm: br2684: Make headroom and hard_header_len depend on
the payload type

 Routed payload requires less headroom than bridged payload.
 So do not reallocate headroom if not needed.
 Also, add worst case AAL5 overhead to netdev->hard_header_len.

[Patch 3/3 v1] atm: br2684: Avoid alignement issues

 Use memcmp() instead of cast to u16 when checking the PAD field.