On Sunday 28 February 2016 17:02:27 Antonio Quartulli wrote:
On Sun, Feb 28, 2016 at 07:36:40AM +0100, Sven Eckelmann wrote:
On Sunday 28 February 2016 08:49:02 Marek Lindner wrote:
On Friday, February 26, 2016 17:56:13 Sven Eckelmann wrote:
In its current form that check is useless because batadv_recv_unicast_packet() already calls batadv_check_unicast_packet() which does the same pskb_may_pull(skb, hdr_size). Am I overlooking something ?
Looks like it also only checks the size of the batman-adv header and the content of the outer (not the encapsulated) ethernet header.
I think you are both right here :-) in batadv_check_unicast_packet() we perform a may_pull with the same hdr_size, therefore it here once again is not useful. It is not really related to this patch, but I think that the removal of the useless pskb_may_pull() in interface_rx() could be done here since you are cleaning up all the pulls in this function.
Ok, I misunderstood his comment. This one is not necessary when each path to this function uses batadv_check_unicast_packet or does "if (!pskb_may_pull(skb, hdr_size))" directly. The only callers are batadv_recv_bcast_packet (does "if (unlikely(!pskb_may_pull(skb, hdr_size)))") and batadv_recv_unicast_packet (calls batadv_check_unicast_packet). I would say that an extra patch for that can be introduced later to remove it. But it should not be part of this patch because this fix should not contain cleanup stuff ("batman-adv header size check") which is not related to the encapsulated ethernet (vlan) header.
Kind regards, Sven