Re: [B.A.T.M.A.N.] [PATCH v5 3/6] batman-adv: prevent duplication of ARP replies when DAT is used

[Simon Wunderlich <sw@simonwunderlich.de>]

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

On Friday 10 June 2016 13:11:01 Andreas Pape wrote:
> If none of the backbone gateways in a bla setup has already knowledge of
> the mac address searched for in an incoming ARP request from the backbone
> an address resolution via the DHT of DAT is started. The gateway can send
> several ARP requests to different DHT nodes and therefore can get several
> replies. This patch assures that not all of the possible ARP replies are
> returned to the backbone by checking the local DAT cache of the gateway.
> If there is an entry in the local cache the gateway has already learned
> the requested address and there is no need to forward the additional reply
> to the backbone.
> Furthermore it is checked if this gateway has claimed the source of the ARP
> reply and only forwards it to the backbone if it has claimed the source or
> if there is no claim at all.

Acked-by: Simon Wunderlich <sw@simonwunderlich.de>

but one small style suggestion below:

> 
> Signed-off-by: Andreas Pape <apape@phoenixcontact.com>
> ---
>  net/batman-adv/distributed-arp-table.c |   32
> ++++++++++++++++++++++++++++++++ 1 files changed, 32 insertions(+), 0
> deletions(-)
> 
> diff --git a/net/batman-adv/distributed-arp-table.c
> b/net/batman-adv/distributed-arp-table.c index 998a4b8..e7b054a 100644
> --- a/net/batman-adv/distributed-arp-table.c
> +++ b/net/batman-adv/distributed-arp-table.c
> @@ -1244,6 +1244,7 @@ bool batadv_dat_snoop_incoming_arp_reply(struct
> batadv_priv *bat_priv, __be32 ip_src, ip_dst;
>  	u8 *hw_src, *hw_dst;
>  	bool dropped = false;
> +	struct batadv_dat_entry *dat_entry = NULL;
>  	unsigned short vid;
> 
>  	if (!atomic_read(&bat_priv->distributed_arp_table))
> @@ -1263,12 +1264,41 @@ bool batadv_dat_snoop_incoming_arp_reply(struct
> batadv_priv *bat_priv, hw_dst = batadv_arp_hw_dst(skb, hdr_size);
>  	ip_dst = batadv_arp_ip_dst(skb, hdr_size);
> 
> +	/* If ip_dst is already in cache and has the right mac address,
> +	 * drop this frame if this ARP reply is destined for us because it's
> +	 * most probably an ARP reply generated by another node of the DHT.
> +	 * We have most probably received already a reply earlier. Delivering
> +	 * this frame would lead to doubled receive of an ARP reply.
> +	 */
> +	dat_entry = batadv_dat_entry_hash_find(bat_priv, ip_src, vid);
> +	if ((dat_entry) && (batadv_compare_eth(hw_src, dat_entry->mac_addr))) {

you can drop the braces for dat_entry and batadv_compare_eth here.

Cheers,
     Simon

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