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

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

[Andreas Pape]

Speeding up dat address lookup is achieved by snooping all incoming ip
traffic. This especially increases the propability in bla setups that
a gateway into a common backbone network already has a fitting dat entry
to answer incoming ARP requests directly coming from the backbone
network thus further reducing ARP traffic in the mesh.

Signed-off-by: Andreas Pape <apape@phoenixcontact.com>
---
 net/batman-adv/distributed-arp-table.c |   18 ++++++++++++++++++
 net/batman-adv/distributed-arp-table.h |    9 ++++++++-
 net/batman-adv/soft-interface.c        |   22 +++++++++++++++++++++-
 3 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/net/batman-adv/distributed-arp-table.c b/net/batman-adv/distributed-arp-table.c
index a9152e7..0f899b9 100644
--- a/net/batman-adv/distributed-arp-table.c
+++ b/net/batman-adv/distributed-arp-table.c
@@ -362,6 +362,24 @@ out:
 		batadv_dat_entry_put(dat_entry);
 }

+/**
+ * batadv_dat_entry_check - check and update a dat entry
+ * @bat_priv: the bat priv with all the soft interface information
+ * @ip: ipv4 to add/edit
+ * @mac_addr: mac address to assign to the given ipv4
+ * @vid: VLAN identifier
+ *
+ * checks additionally, if dat is enabled. can be called from other modules.
+ */
+void batadv_dat_entry_check(struct batadv_priv *bat_priv, __be32 ip,
+			    u8 *mac_addr, unsigned short vid)
+{
+	if (!atomic_read(&bat_priv->distributed_arp_table))
+		return;
+
+	batadv_dat_entry_add(bat_priv, ip, mac_addr, vid);
+}
+
 #ifdef CONFIG_BATMAN_ADV_DEBUG

 /**
diff --git a/net/batman-adv/distributed-arp-table.h b/net/batman-adv/distributed-arp-table.h
index 813ecea..e1b4848 100644
--- a/net/batman-adv/distributed-arp-table.h
+++ b/net/batman-adv/distributed-arp-table.h
@@ -80,7 +80,8 @@ batadv_dat_init_own_addr(struct batadv_priv *bat_priv,
 int batadv_dat_init(struct batadv_priv *bat_priv);
 void batadv_dat_free(struct batadv_priv *bat_priv);
 int batadv_dat_cache_seq_print_text(struct seq_file *seq, void *offset);
-
+void batadv_dat_entry_check(struct batadv_priv *bat_priv, __be32 ip,
+			    u8 *mac_addr, unsigned short vid);
 /**
  * batadv_dat_inc_counter - increment the correct DAT packet counter
  * @bat_priv: the bat priv with all the soft interface information
@@ -173,6 +174,12 @@ static inline void batadv_dat_inc_counter(struct batadv_priv *bat_priv,
 {
 }

+static inline
+void batadv_dat_entry_check(struct batadv_priv *bat_priv, __be32 ip,
+			    u8 *mac_addr, unsigned short vid)
+{
+}
+
 #endif /* CONFIG_BATMAN_ADV_DAT */

 #endif /* _NET_BATMAN_ADV_DISTRIBUTED_ARP_TABLE_H_ */
diff --git a/net/batman-adv/soft-interface.c b/net/batman-adv/soft-interface.c
index 0710379..f031781 100644
--- a/net/batman-adv/soft-interface.c
+++ b/net/batman-adv/soft-interface.c
@@ -28,6 +28,7 @@
 #include <linux/fs.h>
 #include <linux/if_ether.h>
 #include <linux/if_vlan.h>
+#include <linux/ip.h>
 #include <linux/jiffies.h>
 #include <linux/kernel.h>
 #include <linux/kref.h>
@@ -390,6 +391,7 @@ void batadv_interface_rx(struct net_device *soft_iface,
 	__be16 ethertype = htons(ETH_P_BATMAN);
 	struct vlan_ethhdr *vhdr;
 	struct ethhdr *ethhdr;
+	struct iphdr *iphdr;
 	unsigned short vid;
 	bool is_bcast;

@@ -412,11 +414,29 @@ void batadv_interface_rx(struct net_device *soft_iface,
 	ethhdr = eth_hdr(skb);

 	switch (ntohs(ethhdr->h_proto)) {
+	case ETH_P_IP:
+		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);
+		break;
 	case ETH_P_8021Q:
 		vhdr = (struct vlan_ethhdr *)skb->data;

-		if (vhdr->h_vlan_encapsulated_proto != ethertype)
+		if (vhdr->h_vlan_encapsulated_proto != ethertype) {
+			/* snoop incoming traffic for dat update also for vlan
+			 * tagged frames.
+			 */
+			if (vhdr->h_vlan_encapsulated_proto == ETH_P_IP) {
+				iphdr = (struct iphdr *)(vhdr +
+						sizeof(struct vlan_ethhdr));
+				batadv_dat_entry_check(bat_priv, iphdr->saddr,
+						       vhdr->h_source, vid);
+			}
 			break;
+		}

 		/* fall through */
 	case ETH_P_BATMAN:
--
1.7.0.4



..................................................................
PHOENIX CONTACT ELECTRONICS GmbH

Sitz der Gesellschaft / registered office of the company: 31812 Bad Pyrmont
USt-Id-Nr.: DE811742156
Amtsgericht Hannover HRB 100528 / district court Hannover HRB 100528
Geschäftsführer / Executive Board: Roland Bent, Dr. Martin Heubeck
___________________________________________________________________
Diese E-Mail enthält vertrauliche und/oder rechtlich geschützte Informationen. Wenn Sie nicht der richtige Adressat sind oder diese E-Mail irrtümlich erhalten haben, informieren Sie bitte sofort den Absender und vernichten Sie diese Mail. Das unerlaubte Kopieren, jegliche anderweitige Verwendung sowie die unbefugte Weitergabe dieser Mail ist nicht gestattet.
----------------------------------------------------------------------------------------------------
This e-mail may contain confidential and/or privileged information. If you are not the intended recipient (or have received this e-mail in error) please notify the sender immediately and destroy this e-mail. Any unauthorized copying, disclosure, distribution or other use of the material or parts thereof is strictly forbidden.
___________________________________________________________________

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

[Sven Eckelmann]

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

On Friday 26 February 2016 14:18:17 Andreas Pape wrote:
> Speeding up dat address lookup is achieved by snooping all incoming ip
> traffic. This especially increases the propability in bla setups that
> a gateway into a common backbone network already has a fitting dat entry
> to answer incoming ARP requests directly coming from the backbone
> network thus further reducing ARP traffic in the mesh.
> 
> Signed-off-by: Andreas Pape <apape@phoenixcontact.com>
> ---
[...]
>  	switch (ntohs(ethhdr->h_proto)) {
> +	case ETH_P_IP:
> +		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);
> +		break;
>  	case ETH_P_8021Q:
>  		vhdr = (struct vlan_ethhdr *)skb->data;
> 
> -		if (vhdr->h_vlan_encapsulated_proto != ethertype)
> +		if (vhdr->h_vlan_encapsulated_proto != ethertype) {
> +			/* snoop incoming traffic for dat update also for vlan
> +			 * tagged frames.
> +			 */
> +			if (vhdr->h_vlan_encapsulated_proto == ETH_P_IP) {
> +				iphdr = (struct iphdr *)(vhdr +
> +						sizeof(struct vlan_ethhdr));
> +				batadv_dat_entry_check(bat_priv, iphdr->saddr,
> +						       vhdr->h_source, vid);
> +			}

Where is the code to check that there is enough data for the iphdr 
(pskb_may_pull)? Same question goes to Marek for his initial change in 
48628bb9419f ("batman-adv: softif bridge loop avoidance") that introduced the 
vhdr->vlan_ethhdr check

Kind regards,
	Sven

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

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

[Sven Eckelmann]

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

On Friday 26 February 2016 14:18:17 Andreas Pape wrote:
> +				iphdr = (struct iphdr *)(vhdr +
> +						sizeof(struct vlan_ethhdr));

Forgot this one: sizeof(*vhdr) == 18. So you basically calculated

    (u8 *)vhdr + 18 * 18

But I would guess that you actually wanted to calculate

    (u8 *)vhdr + 18

Kind regards,
	Sven

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

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

[Sven Eckelmann]

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

On Friday 26 February 2016 14:18:17 Andreas Pape wrote:
> +                       if (vhdr->h_vlan_encapsulated_proto == ETH_P_IP) {

Either you htons the ETH_P_IP or you have to ntohs the
vhdr->h_vlan_encapsulated_proto. But one of them has to converted to the byte 
order of the other one.

Kind regards,
	Sven

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

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

[Andreas Pape]

Hello Sven,


Sven Eckelmann <sven@narfation.org> schrieb am 26.02.2016 17:00:49:

> Von: Sven Eckelmann <sven@narfation.org>
> An: b.a.t.m.a.n@lists.open-mesh.org
> Kopie: Andreas Pape <apape@phoenixcontact.com>, Marek Lindner
> <lindner_marek@yahoo.de>
> Datum: 26.02.2016 17:00
> Betreff: Re: [B.A.T.M.A.N.] [PATCHv2 2/7] batman-adv: speed up dat
> by snooping received ip traffic
>
> On Friday 26 February 2016 14:18:17 Andreas Pape wrote:
> > Speeding up dat address lookup is achieved by snooping all incoming ip
> > traffic. This especially increases the propability in bla setups that
> > a gateway into a common backbone network already has a fitting dat
entry
> > to answer incoming ARP requests directly coming from the backbone
> > network thus further reducing ARP traffic in the mesh.
> >
> > Signed-off-by: Andreas Pape <apape@phoenixcontact.com>
> > ---
> [...]
> >     switch (ntohs(ethhdr->h_proto)) {
> > +   case ETH_P_IP:
> > +      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);
> > +      break;
> >     case ETH_P_8021Q:
> >        vhdr = (struct vlan_ethhdr *)skb->data;
> >
> > -      if (vhdr->h_vlan_encapsulated_proto != ethertype)
> > +      if (vhdr->h_vlan_encapsulated_proto != ethertype) {
> > +         /* snoop incoming traffic for dat update also for vlan
> > +          * tagged frames.
> > +          */
> > +         if (vhdr->h_vlan_encapsulated_proto == ETH_P_IP) {
> > +            iphdr = (struct iphdr *)(vhdr +
> > +                  sizeof(struct vlan_ethhdr));
> > +            batadv_dat_entry_check(bat_priv, iphdr->saddr,
> > +                         vhdr->h_source, vid);
> > +         }
>
> Where is the code to check that there is enough data for the iphdr
> (pskb_may_pull)? Same question goes to Marek for his initial change in
> 48628bb9419f ("batman-adv: softif bridge loop avoidance") that
introduced the
> vhdr->vlan_ethhdr check
>

I will leave this to the result of the discussion in [1] which you started
and will not handle this in the next version of my patchset. Correct?

Kind regards,
Andreas

[1]
https://lists.open-mesh.org/pipermail/b.a.t.m.a.n/2016-February/014506.html


..................................................................
PHOENIX CONTACT ELECTRONICS GmbH

Sitz der Gesellschaft / registered office of the company: 31812 Bad Pyrmont
USt-Id-Nr.: DE811742156
Amtsgericht Hannover HRB 100528 / district court Hannover HRB 100528
Geschäftsführer / Executive Board: Roland Bent, Dr. Martin Heubeck
___________________________________________________________________
Diese E-Mail enthält vertrauliche und/oder rechtlich geschützte Informationen. Wenn Sie nicht der richtige Adressat sind oder diese E-Mail irrtümlich erhalten haben, informieren Sie bitte sofort den Absender und vernichten Sie diese Mail. Das unerlaubte Kopieren, jegliche anderweitige Verwendung sowie die unbefugte Weitergabe dieser Mail ist nicht gestattet.
----------------------------------------------------------------------------------------------------
This e-mail may contain confidential and/or privileged information. If you are not the intended recipient (or have received this e-mail in error) please notify the sender immediately and destroy this e-mail. Any unauthorized copying, disclosure, distribution or other use of the material or parts thereof is strictly forbidden.
___________________________________________________________________

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

[Sven Eckelmann]

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

On Tuesday 01 March 2016 16:52:47 Andreas Pape wrote:
[...]
> > Where is the code to check that there is enough data for the iphdr
> > (pskb_may_pull)? Same question goes to Marek for his initial change in
> > 48628bb9419f ("batman-adv: softif bridge loop avoidance") that
> introduced the
> > vhdr->vlan_ethhdr check
> >
> 
> I will leave this to the result of the discussion in [1] which you started
> and will not handle this in the next version of my patchset. Correct?

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.

Btw. you can skip this check when you just use skb_header_pointer to access
the IPv4 header. It will return NULL when the data cannot be accessed. It
will also take care of copying data out of the fragments in your temporary 
"buffer" when the bytes are not in the main buffer of the skb. You just have 
to prepare a large enough buffer on the stack and only use the returned 
pointer.

Kind regards,
	Sven

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

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

[Andreas Pape]

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);
                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);
                        }
                        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)?

> Btw. you can skip this check when you just use skb_header_pointer to
access
> the IPv4 header. It will return NULL when the data cannot be accessed.
It
> will also take care of copying data out of the fragments in your
temporary
> "buffer" when the bytes are not in the main buffer of the skb. You just
have
> to prepare a large enough buffer on the stack and only use the returned
> pointer.
>
Kind regards,
Andreas



..................................................................
PHOENIX CONTACT ELECTRONICS GmbH

Sitz der Gesellschaft / registered office of the company: 31812 Bad Pyrmont
USt-Id-Nr.: DE811742156
Amtsgericht Hannover HRB 100528 / district court Hannover HRB 100528
Geschäftsführer / Executive Board: Roland Bent, Dr. Martin Heubeck
___________________________________________________________________
Diese E-Mail enthält vertrauliche und/oder rechtlich geschützte Informationen. Wenn Sie nicht der richtige Adressat sind oder diese E-Mail irrtümlich erhalten haben, informieren Sie bitte sofort den Absender und vernichten Sie diese Mail. Das unerlaubte Kopieren, jegliche anderweitige Verwendung sowie die unbefugte Weitergabe dieser Mail ist nicht gestattet.
----------------------------------------------------------------------------------------------------
This e-mail may contain confidential and/or privileged information. If you are not the intended recipient (or have received this e-mail in error) please notify the sender immediately and destroy this e-mail. Any unauthorized copying, disclosure, distribution or other use of the material or parts thereof is strictly forbidden.
___________________________________________________________________

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]

[-- 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 --]

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

[Andreas Pape]

Sven Eckelmann <sven@narfation.org> schrieb am 02.03.2016 11:23:21:


> >         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 :)

Ooops.. Of course you are right about not dropping the packet here!
Further more
I will follow your suggestion to move this into a separate function to not
mix two different things.

Best regards,
Andreas


..................................................................
PHOENIX CONTACT ELECTRONICS GmbH

Sitz der Gesellschaft / registered office of the company: 31812 Bad Pyrmont
USt-Id-Nr.: DE811742156
Amtsgericht Hannover HRB 100528 / district court Hannover HRB 100528
Geschäftsführer / Executive Board: Roland Bent, Dr. Martin Heubeck
___________________________________________________________________
Diese E-Mail enthält vertrauliche und/oder rechtlich geschützte Informationen. Wenn Sie nicht der richtige Adressat sind oder diese E-Mail irrtümlich erhalten haben, informieren Sie bitte sofort den Absender und vernichten Sie diese Mail. Das unerlaubte Kopieren, jegliche anderweitige Verwendung sowie die unbefugte Weitergabe dieser Mail ist nicht gestattet.
----------------------------------------------------------------------------------------------------
This e-mail may contain confidential and/or privileged information. If you are not the intended recipient (or have received this e-mail in error) please notify the sender immediately and destroy this e-mail. Any unauthorized copying, disclosure, distribution or other use of the material or parts thereof is strictly forbidden.
___________________________________________________________________