bride: IPv6 multicast snooping enhancements

bride: IPv6 multicast snooping enhancements

[Linus Lüssing]

Hi,

Here are two, small feature changes I would like to submit to increase
the usefulness of the multicast snooping of the bridge code.

The first patch is an unaltered one I had submitted before, but since it
got no feedback I'm resubmitting it here for net-next. With the recently
added patch to disable snooping if there is no querier (b00589af + 248ba8ec05
+ 8d50af4fb), it should be a safe choice now (without these, patch 1/2 would
have introduced another potential for lost IPv6 multicast packets).

Both conceptually and also with some testing and fuzzing, I couldn't spot
any more causes for potential packet loss. And since the multicast snooping
code has now been tried by various people, I think it should be a safe
choice to apply the multicast snooping not only for IPv6 multicast packets
with a scope greater than link-local, but also for packets of exactly this
scope. The IPv6 standard mandates MLD reports for link-local multicast, too,
so we can safely snoop them as well (in contrast to IPv4 link-local).

Cheers, Linus

[PATCH net-next 1/2] bridge: prevent flooding IPv6 packets that do not have a listener

[Linus Lüssing]

Currently if there is no listener for a certain group then IPv6 packets
for that group are flooded on all ports, even though there might be no
host and router interested in it on a port.

With this commit they are only forwarded to ports with a multicast
router.

Just like commit bd4265fe36 ("bridge: Only flood unregistered groups
to routers") did for IPv4, let's do the same for IPv6 with the same
reasoning.

Signed-off-by: Linus Lüssing <linus.luessing@web.de>
---
 net/bridge/br_multicast.c |   10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index bbcb435..662ba7b 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -1547,8 +1547,14 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
 	 *  - MLD has always Router Alert hop-by-hop option
 	 *  - But we do not support jumbrograms.
 	 */
-	if (ip6h->version != 6 ||
-	    ip6h->nexthdr != IPPROTO_HOPOPTS ||
+	if (ip6h->version != 6)
+		return 0;
+
+	/* Prevent flooding this packet if there is no listener present */
+	if (ipv6_is_transient_multicast(&ip6h->daddr))
+		BR_INPUT_SKB_CB(skb)->mrouters_only = 1;
+
+	if (ip6h->nexthdr != IPPROTO_HOPOPTS ||
 	    ip6h->payload_len == 0)
 		return 0;
 
-- 
1.7.10.4

[PATCH net-next 2/2] bridge: apply multicast snooping to IPv6 link-local, too

[Linus Lüssing]

The multicast snooping code should have matured enough to be safely
applicable to IPv6 link-local multicast addresses (excluding the
link-local all nodes address, ff02::1), too.

Signed-off-by: Linus Lüssing <linus.luessing@web.de>
---
 net/bridge/br_mdb.c       |    3 ++-
 net/bridge/br_multicast.c |    7 ++++---
 net/bridge/br_private.h   |   10 ----------
 3 files changed, 6 insertions(+), 14 deletions(-)

diff --git a/net/bridge/br_mdb.c b/net/bridge/br_mdb.c
index a7c6cd0..85a09bb 100644
--- a/net/bridge/br_mdb.c
+++ b/net/bridge/br_mdb.c
@@ -9,6 +9,7 @@
 #include <net/netlink.h>
 #if IS_ENABLED(CONFIG_IPV6)
 #include <net/ipv6.h>
+#include <net/addrconf.h>
 #endif
 
 #include "br_private.h"
@@ -254,7 +255,7 @@ static bool is_valid_mdb_entry(struct br_mdb_entry *entry)
 			return false;
 #if IS_ENABLED(CONFIG_IPV6)
 	} else if (entry->addr.proto == htons(ETH_P_IPV6)) {
-		if (!ipv6_is_transient_multicast(&entry->addr.u.ip6))
+		if (ipv6_addr_is_ll_all_nodes(&entry->addr.u.ip6))
 			return false;
 #endif
 	} else
diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index 662ba7b..3b0ed99 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -29,6 +29,7 @@
 #include <net/ipv6.h>
 #include <net/mld.h>
 #include <net/ip6_checksum.h>
+#include <net/addrconf.h>
 #endif
 
 #include "br_private.h"
@@ -724,7 +725,7 @@ static int br_ip6_multicast_add_group(struct net_bridge *br,
 {
 	struct br_ip br_group;
 
-	if (!ipv6_is_transient_multicast(group))
+	if (ipv6_addr_is_ll_all_nodes(group))
 		return 0;
 
 	br_group.u.ip6 = *group;
@@ -1410,7 +1411,7 @@ static void br_ip6_multicast_leave_group(struct net_bridge *br,
 						  &br->ip6_query;
 
 
-	if (!ipv6_is_transient_multicast(group))
+	if (ipv6_addr_is_ll_all_nodes(group))
 		return;
 
 	br_group.u.ip6 = *group;
@@ -1551,7 +1552,7 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
 		return 0;
 
 	/* Prevent flooding this packet if there is no listener present */
-	if (ipv6_is_transient_multicast(&ip6h->daddr))
+	if (!ipv6_addr_is_ll_all_nodes(&ip6h->daddr))
 		BR_INPUT_SKB_CB(skb)->mrouters_only = 1;
 
 	if (ip6h->nexthdr != IPPROTO_HOPOPTS ||
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index f225fb6..598cb0b 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -494,16 +494,6 @@ extern void br_mdb_notify(struct net_device *dev, struct net_bridge_port *port,
 #define mlock_dereference(X, br) \
 	rcu_dereference_protected(X, lockdep_is_held(&br->multicast_lock))
 
-#if IS_ENABLED(CONFIG_IPV6)
-#include <net/addrconf.h>
-static inline int ipv6_is_transient_multicast(const struct in6_addr *addr)
-{
-	if (ipv6_addr_is_multicast(addr) && IPV6_ADDR_MC_FLAG_TRANSIENT(addr))
-		return 1;
-	return 0;
-}
-#endif
-
 static inline bool br_multicast_is_router(struct net_bridge *br)
 {
 	return br->multicast_router == 2 ||
-- 
1.7.10.4

Re: bride: IPv6 multicast snooping enhancements

[David Miller]

From: Linus Lüssing <linus.luessing@web.de>
Date: Wed,  4 Sep 2013 02:13:37 +0200

> Here are two, small feature changes I would like to submit to increase
> the usefulness of the multicast snooping of the bridge code.
> 
> The first patch is an unaltered one I had submitted before, but since it
> got no feedback I'm resubmitting it here for net-next. With the recently
> added patch to disable snooping if there is no querier (b00589af + 248ba8ec05
> + 8d50af4fb), it should be a safe choice now (without these, patch 1/2 would
> have introduced another potential for lost IPv6 multicast packets).
> 
> Both conceptually and also with some testing and fuzzing, I couldn't spot
> any more causes for potential packet loss. And since the multicast snooping
> code has now been tried by various people, I think it should be a safe
> choice to apply the multicast snooping not only for IPv6 multicast packets
> with a scope greater than link-local, but also for packets of exactly this
> scope. The IPv6 standard mandates MLD reports for link-local multicast, too,
> so we can safely snoop them as well (in contrast to IPv4 link-local).

Both patches applied, thanks!

Re: bride: IPv6 multicast snooping enhancements

[Vasily Averin]

This patch prevent forwarding of ICMPv6 in bridges,
so containers/VMs with virtual eth adapters connected in local bridge cannot ping each other via ipv6 (but can do it via ipv4)

Could you please clarify, is it expected behavior?
Do we need to enable multicast routing or multicast_snooping on all local ports on such bridges to enable just ICMPv6?
I believe ICMPv6 is an exception and should not be filtered by multicast spoofing.

Thank you,
	Vasily Averin

On 04.09.2013 04:13, Linus Lüssing wrote:
> Hi,
> 
> Here are two, small feature changes I would like to submit to increase
> the usefulness of the multicast snooping of the bridge code.
> 
> The first patch is an unaltered one I had submitted before, but since it
> got no feedback I'm resubmitting it here for net-next. With the recently
> added patch to disable snooping if there is no querier (b00589af + 248ba8ec05
> + 8d50af4fb), it should be a safe choice now (without these, patch 1/2 would
> have introduced another potential for lost IPv6 multicast packets).
> 
> Both conceptually and also with some testing and fuzzing, I couldn't spot
> any more causes for potential packet loss. And since the multicast snooping
> code has now been tried by various people, I think it should be a safe
> choice to apply the multicast snooping not only for IPv6 multicast packets
> with a scope greater than link-local, but also for packets of exactly this
> scope. The IPv6 standard mandates MLD reports for link-local multicast, too,
> so we can safely snoop them as well (in contrast to IPv4 link-local).
> 
> Cheers, Linus
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 
> 
> 

Re: bride: IPv6 multicast snooping enhancements

[Linus Lüssing]

Hi Vasily,

On Tue, Feb 10, 2015 at 11:44:29AM +0300, Vasily Averin wrote:
> This patch prevent forwarding of ICMPv6 in bridges,
> so containers/VMs with virtual eth adapters connected in local bridge cannot ping each other via ipv6 (but can do it via ipv4)

If a host wants to receive packets, then it needs to signalize
that via MLD. If your host does not do that, then it is expected
to not receive ICMPv6 echo requests to multicast addresses. An
exception is ff02::1, that should always work.

> 
> Could you please clarify, is it expected behavior?
> Do we need to enable multicast routing or multicast_snooping on all local ports on such bridges to enable just ICMPv6?

Nope, you shouldn't. You'd need multicast listeners. You shouldn't
need to play with the bridge settings to fix protocols.

> I believe ICMPv6 is an exception and should not be filtered by multicast spoofing.

Signaling multicast joins is mandatory by the IPv6 standard. If
your protocol/application does not do that, then it seems to me
that the application might be broken.


By the way, which kernel version(s) are you using?

Cheers, Linus

Re: bride: IPv6 multicast snooping enhancements

[Vasily Averin]

On 10.02.2015 14:44, Linus Lüssing wrote:
> Hi Vasily,
> 
> On Tue, Feb 10, 2015 at 11:44:29AM +0300, Vasily Averin wrote:
>> This patch prevent forwarding of ICMPv6 in bridges,
>> so containers/VMs with virtual eth adapters connected in local bridge cannot ping each other via ipv6 (but can do it via ipv4)
> 
> If a host wants to receive packets, then it needs to signalize
> that via MLD. If your host does not do that, then it is expected
> to not receive ICMPv6 echo requests to multicast addresses. An
> exception is ff02::1, that should always work.

Thank you for explanation, seems now I understand finally how it should work.

I'm trying to fix ICMPv6 processing broken in OpenVZ after rebase to last RHEL6u6 kernel.
After some unclear manipulation bridge begins to forward icmp6 NS (fe02::1) into wrong port,
and at present I do not found the reason of this failure.

Thank you,
	Vasily Averin

Re: bride: IPv6 multicast snooping enhancements

[Linus Lüssing]

On Tue, Feb 10, 2015 at 04:59:09PM +0300, Vasily Averin wrote:
> I'm trying to fix ICMPv6 processing broken in OpenVZ after rebase to last RHEL6u6 kernel.
> After some unclear manipulation bridge begins to forward icmp6 NS (fe02::1) into wrong port,
> and at present I do not found the reason of this failure.

fe02::1 seems uncommon for ICMPv6 NS messages. Would you mind
making some dumps for ~10 minutes with tcpdump on all bridge ports
and the bridge interface itself with a filter "icmp6" and uploading the
result somewhere?

Also provide a dump from "bridge mdb show dev $bridge" please (if
possible - not sure whether that's available on the ancient 2.6.32
kernel as used on RHEL6u6).

Cheers, Linus

Re: bride: IPv6 multicast snooping enhancements

[Vasily Averin]

On 12.02.2015 14:41, Linus Lüssing wrote:
> On Tue, Feb 10, 2015 at 04:59:09PM +0300, Vasily Averin wrote:
>> I'm trying to fix ICMPv6 processing broken in OpenVZ after rebase to last RHEL6u6 kernel.
>> After some unclear manipulation bridge begins to forward icmp6 NS (fe02::1) into wrong port,
>> and at present I do not found the reason of this failure.
> 
> fe02::1 seems uncommon for ICMPv6 NS messages. Would you mind
> making some dumps for ~10 minutes with tcpdump on all bridge ports
> and the bridge interface itself with a filter "icmp6" and uploading the
> result somewhere?
> 
> Also provide a dump from "bridge mdb show dev $bridge" please (if
> possible - not sure whether that's available on the ancient 2.6.32
> kernel as used on RHEL6u6).

Thank you, "bridge mdb show" helps me to find an external router.

Therefore bridge forwarded all locally generated multicasts to external port.
Then forwarded messages was lost or was filtered or 
just incorrectly processed, but in any case they never returned back.

So as far as I understand bridge itself works correctly, 
it isn't responsible for external troubles.

Thank you,
	Vasily Averin