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
Linus Lüssing
2013-Sep-04 00:13 UTC
[PATCH net-next 1/2] bridge: prevent flooding IPv6 packets that do not have a listener
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
Linus Lüssing
2013-Sep-04 00:13 UTC
[Bridge] [PATCH net-next 1/2] bridge: prevent flooding IPv6 packets that do not have a listener
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 at 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
Linus Lüssing
2013-Sep-04 00:13 UTC
[PATCH net-next 2/2] bridge: apply multicast snooping to IPv6 link-local, too
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
Linus Lüssing
2013-Sep-04 00:13 UTC
[Bridge] [PATCH net-next 2/2] bridge: apply multicast snooping to IPv6 link-local, too
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 at 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
From: Linus L?ssing <linus.luessing at 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!
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 at vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > > >