Re: [B.A.T.M.A.N.] Antwort: Re: Antwort: Re: [PATCHv2 2/7] batman-adv: speed up dat by snooping received ip traffic

[Sven Eckelmann <sven@narfation.org>]

[-- Attachment #1: Type: text/plain, Size: 2796 bytes --]

On Wednesday 02 March 2016 08:17:12 Andreas Pape wrote:
> Hello Sven,
> 
> Sven Eckelmann <sven@narfation.org> schrieb am 01.03.2016 17:02:57:
> 
> > You definitely have to check the size for the IP header. The vlan
> ethernet
> > header check is a different playground and mostly unrelated to your
> problem.
> >
> 
> If I understood you correctly I am supposed to implement something like
> this:
> 
>         switch (ntohs(ethhdr->h_proto)) {
>         case ETH_P_IP:
>                 if (unlikely(!pskb_may_pull(skb, sizeof(struct iphdr))))
>                         goto dropped;
>                 iphdr = (struct iphdr *)(skb->data + ETH_HLEN);
>                 /* snoop incoming traffic for dat update using the source
> mac
>                  * and source ip to speed up dat.
>                  */
>                 batadv_dat_entry_check(bat_priv, iphdr->saddr,
>                                        ethhdr->h_source, vid);

I doubt that you should drop the data. You new feature is just a nice
enhancement but should not decide whether packet is valid or not. This is why
I told you about skb_header_pointer. Maybe it would also be better to move
your stuff in an extra function and avoid to add extra stuff in the softif
loop check. Maybe I will even move the softif loop check in an extra function
which would be named in a way that doesn't sound like you should add your
stuff there :)

>                 break;
>         case ETH_P_8021Q:
>                 if (unlikely(!pskb_may_pull(skb, sizeof(struct
> vlan_ethhdr))))
>                         goto dropped;
>                 vhdr = (struct vlan_ethhdr *)(skb->data + ETH_HLEN);
> 
>                 if (vhdr->h_vlan_encapsulated_proto != ethertype) {
>                         /* snoop incoming traffic for dat update also for
> vlan
>                          * tagged frames.
>                          */
>                         if (ntohs(vhdr->h_vlan_encapsulated_proto) ==
>                                         ETH_P_IP) {
>                                 iphdr = (struct iphdr *)(vhdr + 1);
>                                 batadv_dat_entry_check(bat_priv,
> iphdr->saddr,
>                                                        vhdr->h_source,
> vid);

Dont forget to check the size here too.

>                         }
>                         break;
>                 }
> Correctly understood?
> 
> Looking through the code of this function batadv_interface_rx leads me to
> another
> question: Am I right that skb->data is pointing to the beginning of the
> mac
> layer 2 header of the packet at that point in time as it isn't advanced to
> the beginning of the layer 3 header by calling eth_type_trans yet(which is
> called
> a few lines later)?

I think this should be correct.

Kind regards,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 819 bytes --]