On Monday, September 14, 2020 9:53:47 PM CEST Linus Lüssing wrote:
Scenario:
- Multicast frame send from BLA backbone gateways (multiple nodes with their bat0 bridged together, with BLA enabled) sharing the same LAN to nodes in the mesh
Issue:
- Nodes receive the frame multiple times on bat0 from the mesh, once from each foreign BLA backbone gateway which shares the same LAN with another
For multicast frames via batman-adv broadcast packets coming from the same BLA backbone but from different backbone gateways duplicates are currently detected via a CRC history of previously received packets.
However this CRC so far was not performed for multicast frames received via batman-adv unicast packets. Fixing this by appyling the same check for such packets, too.
Room for improvements in the future: Ideally we would introduce the possibility to not only claim a client, but a complete originator, too. This would allow us to only send a multicast-in-unicast packet from a BLA backbone gateway claiming the node and by that avoid potential redundant transmissions in the first place.
Fixes: e5cf86d30a9b ("batman-adv: add broadcast duplicate check") Signed-off-by: Linus Lüssing linus.luessing@c0d3.blue
net/batman-adv/bridge_loop_avoidance.c | 107 +++++++++++++++++++++---- 1 file changed, 92 insertions(+), 15 deletions(-)
diff --git a/net/batman-adv/bridge_loop_avoidance.c b/net/batman-adv/bridge_loop_avoidance.c index 3d2a66f2..e4f66dd8 100644 --- a/net/batman-adv/bridge_loop_avoidance.c +++ b/net/batman-adv/bridge_loop_avoidance.c @@ -1580,13 +1580,16 @@ int batadv_bla_init(struct batadv_priv *bat_priv) }
/**
- batadv_bla_check_bcast_duplist() - Check if a frame is in the broadcast
dup. + * batadv_bla_check_duplist() - Check if a frame is in the broadcast dup. * @bat_priv: the bat priv with all the soft interface information - * @skb: contains the bcast_packet to be checked
- @skb: contains the multicast packet to be checked
- @payload_ptr: pointer to position inside the head buffer of the skb
- marking the start of the data to be CRC'ed
- @orig: originator mac address, NULL if unknown
- check if it is on our broadcast list. Another gateway might
- have sent the same packet because it is connected to the same backbone,
- so we have to remove this duplicate.
- Check if it is on our broadcast list. Another gateway might have sent
the + * same packet because it is connected to the same backbone, so we have to + * remove this duplicate.
- This is performed by checking the CRC, which will tell us
- with a good chance that it is the same packet. If it is furthermore
@@ -1595,23 +1598,23 @@ int batadv_bla_init(struct batadv_priv *bat_priv)
- Return: true if a packet is in the duplicate list, false otherwise.
*/ -bool batadv_bla_check_bcast_duplist(struct batadv_priv *bat_priv,
struct sk_buff *skb)+static bool batadv_bla_check_duplist(struct batadv_priv *bat_priv,
struct sk_buff *skb, u8 *payload_ptr,const u8 *orig){
- struct batadv_bcast_duplist_entry *entry;
- bool ret = false; int i, curr; __be32 crc;
struct batadv_bcast_packet *bcast_packet;
struct batadv_bcast_duplist_entry *entry;
bool ret = false;
bcast_packet = (struct batadv_bcast_packet *)skb->data;
/* calculate the crc ... */
crc = batadv_skb_crc32(skb, (u8 *)(bcast_packet + 1));
crc = batadv_skb_crc32(skb, payload_ptr);
spin_lock_bh(&bat_priv->bla.bcast_duplist_lock);
for (i = 0; i < BATADV_DUPLIST_SIZE; i++) {
bool is_from_same_orig = false;curr = (bat_priv->bla.bcast_duplist_curr + i); curr %= BATADV_DUPLIST_SIZE; entry = &bat_priv->bla.bcast_duplist[curr];
@@ -1626,7 +1629,24 @@ bool batadv_bla_check_bcast_duplist(struct batadv_priv *bat_priv, if (entry->crc != crc) continue;
if (batadv_compare_eth(entry->orig, bcast_packet->orig))
/* are the originators both known and not anonymous? */if (orig && !is_zero_ether_addr(orig) &&!is_zero_ether_addr(entry->orig)) {/* if known, check if the new frame came from* the same originator*/if (batadv_compare_eth(entry->orig, orig))is_from_same_orig = true;}/* we are safe to take identical frames from the* same orig, if known, as multiplications in* the mesh are detected via the (orig, seqno) pair;* so we can be a bit more liberal here and allow* identical frames from the same orig which the source* host might have sent multiple times on purpose*/if (is_from_same_orig) continue;
I think this "is_from_same_orig" variable is redundant, it's not used again so you could continue directly above.
The rest looks good.
Cheers, Simon
/* this entry seems to match: same crc, not too old,@@ -1643,7 +1663,14 @@ bool batadv_bla_check_bcast_duplist(struct batadv_priv *bat_priv, entry = &bat_priv->bla.bcast_duplist[curr]; entry->crc = crc; entry->entrytime = jiffies;
- ether_addr_copy(entry->orig, bcast_packet->orig);
- /* known originator */
- if (orig)
ether_addr_copy(entry->orig, orig);- /* anonymous originator */
- else
eth_zero_addr(entry->orig);- bat_priv->bla.bcast_duplist_curr = curr;
out: @@ -1652,6 +1679,48 @@ bool batadv_bla_check_bcast_duplist(struct batadv_priv *bat_priv, return ret; }
+/**
- batadv_bla_check_ucast_duplist() - Check if a frame is in the broadcast
dup. + * @bat_priv: the bat priv with all the soft interface information +
- @skb: contains the multicast packet to be checked, decapsulated from a +
- unicast_packet
- Check if it is on our broadcast list. Another gateway might have sent
the + * same packet because it is connected to the same backbone, so we have to + * remove this duplicate.
- Return: true if a packet is in the duplicate list, false otherwise.
- */
+static bool batadv_bla_check_ucast_duplist(struct batadv_priv *bat_priv,
struct sk_buff *skb)+{
- return batadv_bla_check_duplist(bat_priv, skb, (u8 *)skb->data, NULL);
+}
+/**
- batadv_bla_check_bcast_duplist() - Check if a frame is in the broadcast
dup. + * @bat_priv: the bat priv with all the soft interface information +
- @skb: contains the bcast_packet to be checked
- Check if it is on our broadcast list. Another gateway might have sent
the + * same packet because it is connected to the same backbone, so we have to + * remove this duplicate.
- Return: true if a packet is in the duplicate list, false otherwise.
- */
+bool batadv_bla_check_bcast_duplist(struct batadv_priv *bat_priv,
struct sk_buff *skb)+{
- struct batadv_bcast_packet *bcast_packet;
- u8 *payload_ptr;
- bcast_packet = (struct batadv_bcast_packet *)skb->data;
- payload_ptr = (u8 *)(bcast_packet + 1);
- return batadv_bla_check_duplist(bat_priv, skb, payload_ptr,
bcast_packet->orig);+}
/**
- batadv_bla_is_backbone_gw_orig() - Check if the originator is a gateway
for * the VLAN identified by vid. @@ -1866,6 +1935,14 @@ bool batadv_bla_rx(struct batadv_priv *bat_priv, struct sk_buff *skb, packet_type == BATADV_UNICAST) goto handled;
- /* potential duplicates from foreign BLA backbone gateways via
* multicast-in-unicast packets*/- if (is_multicast_ether_addr(ethhdr->h_dest) &&
packet_type == BATADV_UNICAST &&batadv_bla_check_ucast_duplist(bat_priv, skb))goto handled;- ether_addr_copy(search_claim.addr, ethhdr->h_source); search_claim.vid = vid; claim = batadv_claim_hash_find(bat_priv, &search_claim);