[maint,v2,1/4] batman-adv: mcast/TT: fix wrongly dropped or rerouted packets

Message ID 20200904182803.8428-2-linus.luessing@c0d3.blue (mailing list archive)
State Accepted, archived
Delegated to: Simon Wunderlich
Headers
Series batman-adv: mcast: TT/BLA fixes |

Commit Message

Linus Lüssing Sept. 4, 2020, 6:28 p.m. UTC
  The unicast packet rerouting code makes several assumptions. For
instance it assumes that there is always exactly one destination in the
TT. This breaks for multicast frames in a unicast packets in several ways:

For one thing if there is actually no TT entry and the destination node
was selected due to the multicast tvlv flags it announced. Then an
intermediate node will wrongly drop the packet.

For another thing if there is a TT entry but the TTVN of this entry is
newer than the originally addressed destination node: Then the
intermediate node will wrongly redirect the packet, leading to
duplicated multicast packets at a multicast listener and missing
packets at other multicast listeners or multicast routers.

Fixing this by not applying the unicast packet rerouting to batman-adv
unicast packets with a multicast payload. We are not able to detect a
roaming multicast listener at the moment and will just continue to send
the multicast frame to both the new and old destination for a while in
case of such a roaming multicast listener.

Fixes: cea194d90b11 ("batman-adv: improved client announcement mechanism")
Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
---
 net/batman-adv/routing.c | 4 ++++
 1 file changed, 4 insertions(+)
  

Comments

Simon Wunderlich Sept. 9, 2020, 11:50 a.m. UTC | #1
On Friday, September 4, 2020 8:28:00 PM CEST Linus Lüssing wrote:
> The unicast packet rerouting code makes several assumptions. For
> instance it assumes that there is always exactly one destination in the
> TT. This breaks for multicast frames in a unicast packets in several ways:
> 
> For one thing if there is actually no TT entry and the destination node
> was selected due to the multicast tvlv flags it announced. Then an
> intermediate node will wrongly drop the packet.
> 
> For another thing if there is a TT entry but the TTVN of this entry is
> newer than the originally addressed destination node: Then the
> intermediate node will wrongly redirect the packet, leading to
> duplicated multicast packets at a multicast listener and missing
> packets at other multicast listeners or multicast routers.
> 
> Fixing this by not applying the unicast packet rerouting to batman-adv
> unicast packets with a multicast payload. We are not able to detect a
> roaming multicast listener at the moment and will just continue to send
> the multicast frame to both the new and old destination for a while in
> case of such a roaming multicast listener.
> 
> Fixes: cea194d90b11 ("batman-adv: improved client announcement mechanism")
> Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>

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

Patch

diff --git a/net/batman-adv/routing.c b/net/batman-adv/routing.c
index 27cdf5e4..9e5c71e4 100644
--- a/net/batman-adv/routing.c
+++ b/net/batman-adv/routing.c
@@ -826,6 +826,10 @@  static bool batadv_check_unicast_ttvn(struct batadv_priv *bat_priv,
 	vid = batadv_get_vid(skb, hdr_len);
 	ethhdr = (struct ethhdr *)(skb->data + hdr_len);
 
+	/* do not reroute multicast frames in a unicast header */
+	if (is_multicast_ether_addr(ethhdr->h_dest))
+		return true;
+
 	/* check if the destination client was served by this node and it is now
 	 * roaming. In this case, it means that the node has got a ROAM_ADV
 	 * message and that it knows the new destination in the mesh to re-route