Re: [B.A.T.M.A.N.] [PATCH 2/4] batman-adv: Speed up dat by snooping received ip traffic

[Simon Wunderlich <sw@simonwunderlich.de>]

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

On Friday 12 February 2016 14:52:34 Andreas Pape wrote:
> From cc88159dcf18f4b8310414d2d71635fad76bf5bb Mon Sep 17 00:00:00 2001
> From: Andreas Pape <apape@phoenixcontact.com>
> Date: Fri, 12 Feb 2016 11:03:10 +0100
> Subject: [PATCH 2/4] batman-adv: Speed up dat by snooping received ip
> traffic
> 
> This patch shall speed up dat by snooping all incoming ip traffic instead
> of only relying on ARP handling. This shall especially increase the
> probability
> that a gateway into a backbone network already has a fitting dat entry to
> answer
> incoming arp requests directly coming from the backbone network.
> 
> Signed-off-by: Andreas Pape <apape@phoenixcontact.com>
> ---
>  net/batman-adv/distributed-arp-table.c |   18 ++++++++++++++++++
>  net/batman-adv/distributed-arp-table.h |    8 +++++++-
>  net/batman-adv/soft-interface.c        |   21 ++++++++++++++++++++-
>  3 files changed, 45 insertions(+), 2 deletions(-)
> 
> diff --git a/net/batman-adv/distributed-arp-table.c
> b/net/batman-adv/distributed-arp-table.c
> index 93893bf..4e64e6c 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..a2ab16b 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,11 @@ static inline void batadv_dat_inc_counter(struct
> batadv_priv *bat_priv,
>  {
>  }
> 
> +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..41d7987 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,28 @@ 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.
> +                * Question: does this break the fundamental idea of
> dat????
> +                */

That is a really good question, although it doesn't belong in the code ;)

@Antonio, CC'ing you since this is more a design question/proposal and you may 
have thought about this yet.

Basically, doing this change means that we will put a lot of IP addresses in 
our cache which are not in our local network - typically, all Internet IP 
addresses along with the gateway backbone. Also these addresses will never be 
requested by ARP and are therefore practically just littering our cache. They 
are purged after 5 minutes so the impact may be reasonable, but still ...

Maybe there is a way to limit the entries to local networks? Also (and in 
general), should we have an upper limit how many entries we store in DAT? 
After applying this patch, doing a subnet ping scan can deplete the RAM in 
small routers I'm afraid. :)

(even now, that would be possible with fake ARP replies I guess)

Also, why don't you check the ip destination as well while at it?

Cheers,
      Simon





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