[v7,1/5] batman-adv: prevent multiple ARP replies sent by gateways if dat enabled
Commit Message
From: Andreas Pape <APape@phoenixcontact.com>
If dat is enabled it must be made sure that only the backbone gw which has
claimed the remote destination for the ARP request answers the ARP request
directly if the MAC address is known due to the local dat table. This
prevents multiple ARP replies in a common backbone if more than one
gateway already knows the remote mac searched for in the ARP request.
Signed-off-by: Andreas Pape <apape@phoenixcontact.com>
Acked-by: Simon Wunderlich <sw@simonwunderlich.de>
[sven@narfation.org: fix conflicts with current version]
Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
net/batman-adv/bridge_loop_avoidance.c | 49 ++++++++++++++++++++++++++++++++++
net/batman-adv/bridge_loop_avoidance.h | 11 ++++++++
net/batman-adv/distributed-arp-table.c | 15 +++++++++++
3 files changed, 75 insertions(+)
Comments
Hi,
this should actually should have ended in the cover letter:
The v7 version is just a rebased version of v6 [1] because Marek noticed that
these patches don't apply anymore.
Kind regards,
Sven
[1] https://lists.open-mesh.org/pipermail/b.a.t.m.a.n/2016-July/015858.html
Hello Sven,
thanks for updating the patchset. I am currently a little bit "cut off",
as the current batman-adv version does not compile anymore without a lot
of effort under the old kernel version I use. Therefore I started the work
of updating the kernel version on my devices. I think I will be able to
run the current batman version on my devices soon.
Am I expected to do anything else concerning the patchset? Sorry for my
lack of experience about how such an open source project works but as you
might have noticed this is the first time that I tried to contribute some
stuff.
Best regards,
Andreas
-----"B.A.T.M.A.N" <b.a.t.m.a.n-bounces@lists.open-mesh.org> schrieb: -----
An: b.a.t.m.a.n@lists.open-mesh.org
Von: Sven Eckelmann
Gesendet von: "B.A.T.M.A.N"
Datum: 05.09.2016 13:24
Betreff: [B.A.T.M.A.N.] [PATCH v7 0/5] batman-adv: prevent multiple ARP replies sent by gateways if dat enabled
Hi,
this should actually should have ended in the cover letter:
The v7 version is just a rebased version of v6 [1] because Marek noticed that
these patches don't apply anymore.
Kind regards,
Sven
[1] https://lists.open-mesh.org/pipermail/b.a.t.m.a.n/2016-July/015858.html
[Anhang 'signature.asc' entfernt von Andreas Pape/Phoenix Contact]
..................................................................
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: Ulrich Leidecker, Christoph Leifer
On Donnerstag, 8. September 2016 07:11:52 CEST Andreas Pape wrote:
[...]
> Am I expected to do anything else concerning the patchset?
No, everything fine at the moment. We are just waiting for Marek to have more
free time to check + apply the patches. :)
He was starting to merge them at the beginning of the week but complained that
there are conflicts. I've just jumped in and fixed them for you because I
hoped that they will be in the master branch by now. But it looks like this
didn't work out. So lets see what Marek will say.
Kind regards,
Sven
Hi,
On Montag, 5. September 2016 13:20:25 CET Sven Eckelmann wrote:
> From: Andreas Pape <APape@phoenixcontact.com>
>
> If dat is enabled it must be made sure that only the backbone gw which has
> claimed the remote destination for the ARP request answers the ARP request
> directly if the MAC address is known due to the local dat table. This
> prevents multiple ARP replies in a common backbone if more than one
> gateway already knows the remote mac searched for in the ARP request.
[....]
Just in case someone it wondering why this wasn't applied yet. Antonio didn't
like the patches and thus the patches were assigned to him in Patchwork [1].
It is currently unknown what problems he meant, but he created a branch under
his namespace [2]. This branch only contains a re-factorization of the first
patch (+ batadv_bla_check_claim/batadv_bla_is_my_claim behavior changes when
batadv_primary_if_get_selected or batadv_claim_hash_find return NULL). We
currently don't know whether this all or if he had more problems with these
patches.
It is therefore currently unknown how to continue from here. Antonio, please
provide a statement what we should do with the patches.
We don't want to add new patches before the v2017.0 release but maybe this is
a candidate for v2017.1.
Kind regards,
Sven
[1] https://patchwork.open-mesh.org/project/b.a.t.m.a.n./list/?delegate=7
[2] https://git.open-mesh.org/batman-adv.git/shortlog/refs/heads/ordex/bla_dat
Hi,
On Mon, Feb 06, 2017 at 10:56:25AM +0100, Sven Eckelmann wrote:
> It is therefore currently unknown how to continue from here. Antonio, please
> provide a statement what we should do with the patches.
There were two issues here:
1) the code was not really following the kernel style and I started my branch to
"smoothen" it a little bit and possibly re-propose it to the mailing list;
2) in one of the patches I found a logical issue that needed some rework.
Unfortunately, I can't recall exactly where the issue was. I'd need to dig
again into the code. I can have a look later this week and let you know what I
find.
Cheers,
Hi,
On Mon, Feb 06, 2017 at 07:48:23PM +0800, Antonio Quartulli wrote:
> Hi,
>
> On Mon, Feb 06, 2017 at 10:56:25AM +0100, Sven Eckelmann wrote:
> > It is therefore currently unknown how to continue from here. Antonio, please
> > provide a statement what we should do with the patches.
>
> There were two issues here:
> 1) the code was not really following the kernel style and I started my branch to
> "smoothen" it a little bit and possibly re-propose it to the mailing list;
>
I pushed my ordex/bla_dat branch after rebasing it on top of current master.
Patch 1 and 2 are ok for me.
The doubts I had were about the rest, which is mostly about rearrangement of
BLA.
@Simon, I think here I'd need you to give a deep look to understand what could
possibly go wrong in the cases described. These changes may violate some of the
scenarios fixed by BLA..
Could you please have a look?
Cheers,
@@ -2450,3 +2450,52 @@ out:
return ret;
}
+
+#ifdef CONFIG_BATMAN_ADV_DAT
+/**
+ * batadv_bla_check_claim - check if address is claimed
+ *
+ * @bat_priv: the bat priv with all the soft interface information
+ * @addr: mac address of which the claim status is checked
+ * @vid: the VLAN ID
+ *
+ * addr is checked if this address is claimed by the local device itself.
+ *
+ * Return: true if bla is disabled or the mac is claimed by the device,
+ * false if the device addr is already claimed by another gateway
+ */
+bool batadv_bla_check_claim(struct batadv_priv *bat_priv,
+ u8 *addr, unsigned short vid)
+{
+ struct batadv_bla_claim search_claim;
+ struct batadv_bla_claim *claim = NULL;
+ struct batadv_hard_iface *primary_if = NULL;
+ bool ret = true;
+
+ if (!atomic_read(&bat_priv->bridge_loop_avoidance))
+ return ret;
+
+ primary_if = batadv_primary_if_get_selected(bat_priv);
+ if (!primary_if)
+ return ret;
+
+ /* First look if the mac address is claimed */
+ ether_addr_copy(search_claim.addr, addr);
+ search_claim.vid = vid;
+
+ claim = batadv_claim_hash_find(bat_priv, &search_claim);
+
+ /* If there is a claim and we are not owner of the claim,
+ * return false.
+ */
+ if (claim) {
+ if (!batadv_compare_eth(claim->backbone_gw->orig,
+ primary_if->net_dev->dev_addr))
+ ret = false;
+ batadv_claim_put(claim);
+ }
+
+ batadv_hardif_put(primary_if);
+ return ret;
+}
+#endif
@@ -51,6 +51,10 @@ void batadv_bla_status_update(struct net_device *net_dev);
int batadv_bla_init(struct batadv_priv *bat_priv);
void batadv_bla_free(struct batadv_priv *bat_priv);
int batadv_bla_claim_dump(struct sk_buff *msg, struct netlink_callback *cb);
+#ifdef CONFIG_BATMAN_ADV_DAT
+bool batadv_bla_check_claim(struct batadv_priv *bat_priv, u8 *addr,
+ unsigned short vid);
+#endif
#define BATADV_BLA_CRC_INIT 0
#else /* ifdef CONFIG_BATMAN_ADV_BLA */
@@ -127,6 +131,13 @@ static inline int batadv_bla_backbone_dump(struct sk_buff *msg,
return -EOPNOTSUPP;
}
+static inline
+bool batadv_bla_check_claim(struct batadv_priv *bat_priv, u8 *addr,
+ unsigned short vid)
+{
+ return true;
+}
+
#endif /* ifdef CONFIG_BATMAN_ADV_BLA */
#endif /* ifndef _NET_BATMAN_ADV_BLA_H_ */
@@ -43,6 +43,7 @@
#include <linux/workqueue.h>
#include <net/arp.h>
+#include "bridge_loop_avoidance.h"
#include "hard-interface.h"
#include "hash.h"
#include "log.h"
@@ -1007,6 +1008,20 @@ bool batadv_dat_snoop_outgoing_arp_request(struct batadv_priv *bat_priv,
goto out;
}
+ /* If BLA is enabled, only send ARP replies if we have claimed
+ * the destination for the ARP request or if no one else of
+ * the backbone gws belonging to our backbone has claimed the
+ * destination.
+ */
+ if (!batadv_bla_check_claim(bat_priv,
+ dat_entry->mac_addr, vid)) {
+ batadv_dbg(BATADV_DBG_DAT, bat_priv,
+ "Device %pM claimed by another backbone gw. Don't send ARP reply!",
+ dat_entry->mac_addr);
+ ret = true;
+ goto out;
+ }
+
skb_new = arp_create(ARPOP_REPLY, ETH_P_ARP, ip_src,
bat_priv->soft_iface, ip_dst, hw_src,
dat_entry->mac_addr, hw_src);