* [PATCH net-next] mcast: ensure L-L IPv6 packets are accepted by bridge @ 2019-08-13 14:18 Patrick Ruddy 2019-08-13 19:53 ` Ido Schimmel 0 siblings, 1 reply; 8+ messages in thread From: Patrick Ruddy @ 2019-08-13 14:18 UTC (permalink / raw) To: netdev At present only all-nodes IPv6 multicast packets are accepted by a bridge interface that is not in multicast router mode. Since other protocols can be running in the absense of multicast forwarding e.g. OSPFv3 IPv6 ND. Change the test to allow all of the FFx2::/16 range to be accepted when not in multicast router mode. This aligns the code with IPv4 link-local reception and RFC4291 Signed-off-by: Patrick Ruddy <pruddy@vyatta.att-mail.com> --- include/net/addrconf.h | 15 +++++++++++++++ net/bridge/br_multicast.c | 2 +- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/include/net/addrconf.h b/include/net/addrconf.h index becdad576859..05b42867e969 100644 --- a/include/net/addrconf.h +++ b/include/net/addrconf.h @@ -434,6 +434,21 @@ static inline void addrconf_addr_solict_mult(const struct in6_addr *addr, htonl(0xFF000000) | addr->s6_addr32[3]); } +/* + * link local multicast address range ffx2::/16 rfc4291 + */ +static inline bool ipv6_addr_is_ll_mcast(const struct in6_addr *addr) +{ +#if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) && BITS_PER_LONG == 64 + __be64 *p = (__be64 *)addr; + return ((p[0] & cpu_to_be64(0xff0f000000000000UL)) + ^ cpu_to_be64(0xff02000000000000UL)) == 0UL; +#else + return ((addr->s6_addr32[0] & htonl(0xff0f0000)) ^ + htonl(0xff020000)) == 0; +#endif +} + static inline bool ipv6_addr_is_ll_all_nodes(const struct in6_addr *addr) { #if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) && BITS_PER_LONG == 64 diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c index 9b379e110129..ed3957381fa2 100644 --- a/net/bridge/br_multicast.c +++ b/net/bridge/br_multicast.c @@ -1664,7 +1664,7 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br, err = ipv6_mc_check_mld(skb); if (err == -ENOMSG) { - if (!ipv6_addr_is_ll_all_nodes(&ipv6_hdr(skb)->daddr)) + if (!ipv6_addr_is_ll_mcast(&ipv6_hdr(skb)->daddr)) BR_INPUT_SKB_CB(skb)->mrouters_only = 1; if (ipv6_addr_is_all_snoopers(&ipv6_hdr(skb)->daddr)) { -- 2.20.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net-next] mcast: ensure L-L IPv6 packets are accepted by bridge 2019-08-13 14:18 [PATCH net-next] mcast: ensure L-L IPv6 packets are accepted by bridge Patrick Ruddy @ 2019-08-13 19:53 ` Ido Schimmel 2019-08-13 23:55 ` Nikolay Aleksandrov 0 siblings, 1 reply; 8+ messages in thread From: Ido Schimmel @ 2019-08-13 19:53 UTC (permalink / raw) To: Patrick Ruddy; +Cc: netdev, roopa, nikolay, linus.luessing + Bridge maintainers, Linus On Tue, Aug 13, 2019 at 03:18:04PM +0100, Patrick Ruddy wrote: > At present only all-nodes IPv6 multicast packets are accepted by > a bridge interface that is not in multicast router mode. Since > other protocols can be running in the absense of multicast > forwarding e.g. OSPFv3 IPv6 ND. Change the test to allow > all of the FFx2::/16 range to be accepted when not in multicast > router mode. This aligns the code with IPv4 link-local reception > and RFC4291 Can you please quote the relevant part from RFC 4291? > > Signed-off-by: Patrick Ruddy <pruddy@vyatta.att-mail.com> > --- > include/net/addrconf.h | 15 +++++++++++++++ > net/bridge/br_multicast.c | 2 +- > 2 files changed, 16 insertions(+), 1 deletion(-) > > diff --git a/include/net/addrconf.h b/include/net/addrconf.h > index becdad576859..05b42867e969 100644 > --- a/include/net/addrconf.h > +++ b/include/net/addrconf.h > @@ -434,6 +434,21 @@ static inline void addrconf_addr_solict_mult(const struct in6_addr *addr, > htonl(0xFF000000) | addr->s6_addr32[3]); > } > > +/* > + * link local multicast address range ffx2::/16 rfc4291 > + */ > +static inline bool ipv6_addr_is_ll_mcast(const struct in6_addr *addr) > +{ > +#if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) && BITS_PER_LONG == 64 > + __be64 *p = (__be64 *)addr; > + return ((p[0] & cpu_to_be64(0xff0f000000000000UL)) > + ^ cpu_to_be64(0xff02000000000000UL)) == 0UL; > +#else > + return ((addr->s6_addr32[0] & htonl(0xff0f0000)) ^ > + htonl(0xff020000)) == 0; > +#endif > +} > + > static inline bool ipv6_addr_is_ll_all_nodes(const struct in6_addr *addr) > { > #if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) && BITS_PER_LONG == 64 > diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c > index 9b379e110129..ed3957381fa2 100644 > --- a/net/bridge/br_multicast.c > +++ b/net/bridge/br_multicast.c > @@ -1664,7 +1664,7 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br, > err = ipv6_mc_check_mld(skb); > > if (err == -ENOMSG) { > - if (!ipv6_addr_is_ll_all_nodes(&ipv6_hdr(skb)->daddr)) > + if (!ipv6_addr_is_ll_mcast(&ipv6_hdr(skb)->daddr)) > BR_INPUT_SKB_CB(skb)->mrouters_only = 1; IIUC, you want IPv6 link-local packets to be locally received, but this also changes how these packets are flooded. RFC 4541 says that packets addressed to the all hosts address are a special case and should be forwarded to all ports: "In IPv6, the data forwarding rules are more straight forward because MLD is mandated for addresses with scope 2 (link-scope) or greater. The only exception is the address FF02::1 which is the all hosts link-scope address for which MLD messages are never sent. Packets with the all hosts link-scope address should be forwarded on all ports." Maybe you want something like: diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c index 09b1dd8cd853..9f312a73f61c 100644 --- a/net/bridge/br_input.c +++ b/net/bridge/br_input.c @@ -132,7 +132,8 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb if ((mdst || BR_INPUT_SKB_CB_MROUTERS_ONLY(skb)) && br_multicast_querier_exists(br, eth_hdr(skb))) { if ((mdst && mdst->host_joined) || - br_multicast_is_router(br)) { + br_multicast_is_router(br) || + BR_INPUT_SKB_CB_LOCAL_RECEIVE(skb)) { local_rcv = true; br->dev->stats.multicast++; } diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c index 9b379e110129..f03cecf6174e 100644 --- a/net/bridge/br_multicast.c +++ b/net/bridge/br_multicast.c @@ -1667,6 +1667,9 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br, if (!ipv6_addr_is_ll_all_nodes(&ipv6_hdr(skb)->daddr)) BR_INPUT_SKB_CB(skb)->mrouters_only = 1; + if (ipv6_addr_is_ll_mcast(&ipv6_hdr(skb)->daddr)) + BR_INPUT_SKB_CB(skb)->local_receive = 1; + if (ipv6_addr_is_all_snoopers(&ipv6_hdr(skb)->daddr)) { err = br_ip6_multicast_mrd_rcv(br, port, skb); diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h index b7a4942ff1b3..d76394ca4059 100644 --- a/net/bridge/br_private.h +++ b/net/bridge/br_private.h @@ -426,6 +426,7 @@ struct br_input_skb_cb { #ifdef CONFIG_BRIDGE_IGMP_SNOOPING u8 igmp; u8 mrouters_only:1; + u8 local_receive:1; #endif u8 proxyarp_replied:1; u8 src_port_isolated:1; @@ -445,8 +446,10 @@ struct br_input_skb_cb { #ifdef CONFIG_BRIDGE_IGMP_SNOOPING # define BR_INPUT_SKB_CB_MROUTERS_ONLY(__skb) (BR_INPUT_SKB_CB(__skb)->mrouters_only) +# define BR_INPUT_SKB_CB_LOCAL_RECEIVE(__skb) (BR_INPUT_SKB_CB(__skb)->local_receive) #else # define BR_INPUT_SKB_CB_MROUTERS_ONLY(__skb) (0) +# define BR_INPUT_SKB_CB_LOCAL_RECEIVE(__skb) (0) #endif #define br_printk(level, br, format, args...) \ ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net-next] mcast: ensure L-L IPv6 packets are accepted by bridge 2019-08-13 19:53 ` Ido Schimmel @ 2019-08-13 23:55 ` Nikolay Aleksandrov 2019-08-14 16:40 ` Patrick Ruddy 0 siblings, 1 reply; 8+ messages in thread From: Nikolay Aleksandrov @ 2019-08-13 23:55 UTC (permalink / raw) To: Ido Schimmel, Patrick Ruddy; +Cc: netdev, roopa, linus.luessing On 8/13/19 10:53 PM, Ido Schimmel wrote: > + Bridge maintainers, Linus > Good catch Ido, thanks! First I'd say the subject needs to reflect that this is a bridge change better, please rearrange it like so - bridge: mcast: ... More below, > On Tue, Aug 13, 2019 at 03:18:04PM +0100, Patrick Ruddy wrote: >> At present only all-nodes IPv6 multicast packets are accepted by >> a bridge interface that is not in multicast router mode. Since >> other protocols can be running in the absense of multicast >> forwarding e.g. OSPFv3 IPv6 ND. Change the test to allow >> all of the FFx2::/16 range to be accepted when not in multicast >> router mode. This aligns the code with IPv4 link-local reception >> and RFC4291 > > Can you please quote the relevant part from RFC 4291? > >> >> Signed-off-by: Patrick Ruddy <pruddy@vyatta.att-mail.com> >> --- >> include/net/addrconf.h | 15 +++++++++++++++ >> net/bridge/br_multicast.c | 2 +- >> 2 files changed, 16 insertions(+), 1 deletion(-) >> >> diff --git a/include/net/addrconf.h b/include/net/addrconf.h >> index becdad576859..05b42867e969 100644 >> --- a/include/net/addrconf.h >> +++ b/include/net/addrconf.h >> @@ -434,6 +434,21 @@ static inline void addrconf_addr_solict_mult(const struct in6_addr *addr, >> htonl(0xFF000000) | addr->s6_addr32[3]); >> } >> >> +/* >> + * link local multicast address range ffx2::/16 rfc4291 >> + */ >> +static inline bool ipv6_addr_is_ll_mcast(const struct in6_addr *addr) >> +{ >> +#if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) && BITS_PER_LONG == 64 >> + __be64 *p = (__be64 *)addr; >> + return ((p[0] & cpu_to_be64(0xff0f000000000000UL)) >> + ^ cpu_to_be64(0xff02000000000000UL)) == 0UL; >> +#else >> + return ((addr->s6_addr32[0] & htonl(0xff0f0000)) ^ >> + htonl(0xff020000)) == 0; >> +#endif >> +} >> + >> static inline bool ipv6_addr_is_ll_all_nodes(const struct in6_addr *addr) >> { >> #if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) && BITS_PER_LONG == 64 >> diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c >> index 9b379e110129..ed3957381fa2 100644 >> --- a/net/bridge/br_multicast.c >> +++ b/net/bridge/br_multicast.c >> @@ -1664,7 +1664,7 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br, >> err = ipv6_mc_check_mld(skb); >> >> if (err == -ENOMSG) { >> - if (!ipv6_addr_is_ll_all_nodes(&ipv6_hdr(skb)->daddr)) >> + if (!ipv6_addr_is_ll_mcast(&ipv6_hdr(skb)->daddr)) >> BR_INPUT_SKB_CB(skb)->mrouters_only = 1; > > IIUC, you want IPv6 link-local packets to be locally received, but this > also changes how these packets are flooded. RFC 4541 says that packets Indeed, we'll start flooding them all, not just the all hosts address. If that is at all required it'll definitely have to be optional. > addressed to the all hosts address are a special case and should be > forwarded to all ports: > > "In IPv6, the data forwarding rules are more straight forward because MLD is > mandated for addresses with scope 2 (link-scope) or greater. The only exception > is the address FF02::1 which is the all hosts link-scope address for which MLD > messages are never sent. Packets with the all hosts link-scope address should > be forwarded on all ports." > I wonder what is the problem for the host to join such group on behalf of the bridge ? Then you'll receive the traffic at least locally and the RFC says it itself - MLD is mandated for the other link-local addresses. It's very late here and maybe I'm missing something.. :) > Maybe you want something like: > I think we can do without the new field, either pass local_rcv into br_multicast_rcv() or set it based on return value. The extra test will have to remain unfortunately, but we can reduce the tests by one if carefully done. > diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c > index 09b1dd8cd853..9f312a73f61c 100644 > --- a/net/bridge/br_input.c > +++ b/net/bridge/br_input.c > @@ -132,7 +132,8 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb > if ((mdst || BR_INPUT_SKB_CB_MROUTERS_ONLY(skb)) && > br_multicast_querier_exists(br, eth_hdr(skb))) { > if ((mdst && mdst->host_joined) || > - br_multicast_is_router(br)) { > + br_multicast_is_router(br) || > + BR_INPUT_SKB_CB_LOCAL_RECEIVE(skb)) { > local_rcv = true; > br->dev->stats.multicast++; > } > diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c > index 9b379e110129..f03cecf6174e 100644 > --- a/net/bridge/br_multicast.c > +++ b/net/bridge/br_multicast.c > @@ -1667,6 +1667,9 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br, > if (!ipv6_addr_is_ll_all_nodes(&ipv6_hdr(skb)->daddr)) > BR_INPUT_SKB_CB(skb)->mrouters_only = 1; > > + if (ipv6_addr_is_ll_mcast(&ipv6_hdr(skb)->daddr)) > + BR_INPUT_SKB_CB(skb)->local_receive = 1; > + > if (ipv6_addr_is_all_snoopers(&ipv6_hdr(skb)->daddr)) { > err = br_ip6_multicast_mrd_rcv(br, port, skb); > > diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h > index b7a4942ff1b3..d76394ca4059 100644 > --- a/net/bridge/br_private.h > +++ b/net/bridge/br_private.h > @@ -426,6 +426,7 @@ struct br_input_skb_cb { > #ifdef CONFIG_BRIDGE_IGMP_SNOOPING > u8 igmp; > u8 mrouters_only:1; > + u8 local_receive:1; > #endif > u8 proxyarp_replied:1; > u8 src_port_isolated:1; > @@ -445,8 +446,10 @@ struct br_input_skb_cb { > > #ifdef CONFIG_BRIDGE_IGMP_SNOOPING > # define BR_INPUT_SKB_CB_MROUTERS_ONLY(__skb) (BR_INPUT_SKB_CB(__skb)->mrouters_only) > +# define BR_INPUT_SKB_CB_LOCAL_RECEIVE(__skb) (BR_INPUT_SKB_CB(__skb)->local_receive) > #else > # define BR_INPUT_SKB_CB_MROUTERS_ONLY(__skb) (0) > +# define BR_INPUT_SKB_CB_LOCAL_RECEIVE(__skb) (0) > #endif > > #define br_printk(level, br, format, args...) \ > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next] mcast: ensure L-L IPv6 packets are accepted by bridge 2019-08-13 23:55 ` Nikolay Aleksandrov @ 2019-08-14 16:40 ` Patrick Ruddy 2019-08-14 16:58 ` Nikolay Aleksandrov 2019-08-14 20:11 ` Linus Lüssing 0 siblings, 2 replies; 8+ messages in thread From: Patrick Ruddy @ 2019-08-14 16:40 UTC (permalink / raw) To: Nikolay Aleksandrov, Ido Schimmel; +Cc: netdev, roopa, linus.luessing Thanks both for the quick replies, answers inline... On Wed, 2019-08-14 at 02:55 +0300, Nikolay Aleksandrov wrote: > On 8/13/19 10:53 PM, Ido Schimmel wrote: > > + Bridge maintainers, Linus > > > > Good catch Ido, thanks! > First I'd say the subject needs to reflect that this is a bridge change > better, please rearrange it like so - bridge: mcast: ... > More below, > > > On Tue, Aug 13, 2019 at 03:18:04PM +0100, Patrick Ruddy wrote: > > > At present only all-nodes IPv6 multicast packets are accepted by > > > a bridge interface that is not in multicast router mode. Since > > > other protocols can be running in the absense of multicast > > > forwarding e.g. OSPFv3 IPv6 ND. Change the test to allow > > > all of the FFx2::/16 range to be accepted when not in multicast > > > router mode. This aligns the code with IPv4 link-local reception > > > and RFC4291 > > > > Can you please quote the relevant part from RFC 4291? > > > > > Signed-off-by: Patrick Ruddy <pruddy@vyatta.att-mail.com> > > > --- > > > include/net/addrconf.h | 15 +++++++++++++++ > > > net/bridge/br_multicast.c | 2 +- > > > 2 files changed, 16 insertions(+), 1 deletion(-) > > > > > > diff --git a/include/net/addrconf.h b/include/net/addrconf.h > > > index becdad576859..05b42867e969 100644 > > > --- a/include/net/addrconf.h > > > +++ b/include/net/addrconf.h > > > @@ -434,6 +434,21 @@ static inline void addrconf_addr_solict_mult(const struct in6_addr *addr, > > > htonl(0xFF000000) | addr->s6_addr32[3]); > > > } > > > > > > +/* > > > + * link local multicast address range ffx2::/16 rfc4291 > > > + */ > > > +static inline bool ipv6_addr_is_ll_mcast(const struct in6_addr *addr) > > > +{ > > > +#if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) && BITS_PER_LONG == 64 > > > + __be64 *p = (__be64 *)addr; > > > + return ((p[0] & cpu_to_be64(0xff0f000000000000UL)) > > > + ^ cpu_to_be64(0xff02000000000000UL)) == 0UL; > > > +#else > > > + return ((addr->s6_addr32[0] & htonl(0xff0f0000)) ^ > > > + htonl(0xff020000)) == 0; > > > +#endif > > > +} > > > + > > > static inline bool ipv6_addr_is_ll_all_nodes(const struct in6_addr *addr) > > > { > > > #if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) && BITS_PER_LONG == 64 > > > diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c > > > index 9b379e110129..ed3957381fa2 100644 > > > --- a/net/bridge/br_multicast.c > > > +++ b/net/bridge/br_multicast.c > > > @@ -1664,7 +1664,7 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br, > > > err = ipv6_mc_check_mld(skb); > > > > > > if (err == -ENOMSG) { > > > - if (!ipv6_addr_is_ll_all_nodes(&ipv6_hdr(skb)->daddr)) > > > + if (!ipv6_addr_is_ll_mcast(&ipv6_hdr(skb)->daddr)) > > > BR_INPUT_SKB_CB(skb)->mrouters_only = 1; > > > > IIUC, you want IPv6 link-local packets to be locally received, but this > > also changes how these packets are flooded. RFC 4541 says that packets > > Indeed, we'll start flooding them all, not just the all hosts address. > If that is at all required it'll definitely have to be optional. > > > addressed to the all hosts address are a special case and should be > > forwarded to all ports: > > > > "In IPv6, the data forwarding rules are more straight forward because MLD is > > mandated for addresses with scope 2 (link-scope) or greater. The only exception > > is the address FF02::1 which is the all hosts link-scope address for which MLD > > messages are never sent. Packets with the all hosts link-scope address should > > be forwarded on all ports." > > > > I wonder what is the problem for the host to join such group on behalf of the bridge ? > Then you'll receive the traffic at least locally and the RFC says it itself - MLD is mandated > for the other link-local addresses. > It's very late here and maybe I'm missing something.. :) > The group is being joined by MLD at the L3 level but the packets are not being passed up to the l3 interface becasue there is a MLD querier on the network snippet from /proc/net/igmp6 ... 40 sw1 ff0200000000000000000001ff008700 1 00000004 0 40 sw1 ff020000000000000000000000000002 1 00000004 0 40 sw1 ff020000000000000000000000000001 1 0000000C 0 40 sw1 ff010000000000000000000000000001 1 00000008 0 41 lo1 ff020000000000000000000000000001 1 0000000C 0 41 lo1 ff010000000000000000000000000001 1 00000008 0 42 sw1.1 ff020000000000000000000000000006 1 00000004 0 42 sw1.1 ff020000000000000000000000000005 1 00000004 0 42 sw1.1 ff0200000000000000000001ff000000 2 00000004 0 42 sw1.1 ff0200000000000000000001ff008700 1 00000004 0 42 sw1.1 ff0200000000000000000001ff000099 1 00000004 0 42 sw1.1 ff020000000000000000000000000002 1 00000004 0 42 sw1.1 ff020000000000000000000000000001 1 0000000C 0 42 sw1.1 ff010000000000000000000000000001 1 00000008 0 ... the bridge is sw1 and the l3 intervace is sw1.1 Ido is correct about the flooding - I will update the patch with the comments and reissue. Thanks again -pr > > > Maybe you want something like: > > > > I think we can do without the new field, either pass local_rcv into br_multicast_rcv() or > set it based on return value. The extra test will have to remain unfortunately, but we > can reduce the tests by one if carefully done. > > > diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c > > index 09b1dd8cd853..9f312a73f61c 100644 > > --- a/net/bridge/br_input.c > > +++ b/net/bridge/br_input.c > > @@ -132,7 +132,8 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb > > if ((mdst || BR_INPUT_SKB_CB_MROUTERS_ONLY(skb)) && > > br_multicast_querier_exists(br, eth_hdr(skb))) { > > if ((mdst && mdst->host_joined) || > > - br_multicast_is_router(br)) { > > + br_multicast_is_router(br) || > > + BR_INPUT_SKB_CB_LOCAL_RECEIVE(skb)) { > > local_rcv = true; > > br->dev->stats.multicast++; > > } > > diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c > > index 9b379e110129..f03cecf6174e 100644 > > --- a/net/bridge/br_multicast.c > > +++ b/net/bridge/br_multicast.c > > @@ -1667,6 +1667,9 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br, > > if (!ipv6_addr_is_ll_all_nodes(&ipv6_hdr(skb)->daddr)) > > BR_INPUT_SKB_CB(skb)->mrouters_only = 1; > > > > + if (ipv6_addr_is_ll_mcast(&ipv6_hdr(skb)->daddr)) > > + BR_INPUT_SKB_CB(skb)->local_receive = 1; > > + > > if (ipv6_addr_is_all_snoopers(&ipv6_hdr(skb)->daddr)) { > > err = br_ip6_multicast_mrd_rcv(br, port, skb); > > > > diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h > > index b7a4942ff1b3..d76394ca4059 100644 > > --- a/net/bridge/br_private.h > > +++ b/net/bridge/br_private.h > > @@ -426,6 +426,7 @@ struct br_input_skb_cb { > > #ifdef CONFIG_BRIDGE_IGMP_SNOOPING > > u8 igmp; > > u8 mrouters_only:1; > > + u8 local_receive:1; > > #endif > > u8 proxyarp_replied:1; > > u8 src_port_isolated:1; > > @@ -445,8 +446,10 @@ struct br_input_skb_cb { > > > > #ifdef CONFIG_BRIDGE_IGMP_SNOOPING > > # define BR_INPUT_SKB_CB_MROUTERS_ONLY(__skb) (BR_INPUT_SKB_CB(__skb)->mrouters_only) > > +# define BR_INPUT_SKB_CB_LOCAL_RECEIVE(__skb) (BR_INPUT_SKB_CB(__skb)->local_receive) > > #else > > # define BR_INPUT_SKB_CB_MROUTERS_ONLY(__skb) (0) > > +# define BR_INPUT_SKB_CB_LOCAL_RECEIVE(__skb) (0) > > #endif > > > > #define br_printk(level, br, format, args...) \ > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next] mcast: ensure L-L IPv6 packets are accepted by bridge 2019-08-14 16:40 ` Patrick Ruddy @ 2019-08-14 16:58 ` Nikolay Aleksandrov 2019-08-14 20:11 ` Linus Lüssing 1 sibling, 0 replies; 8+ messages in thread From: Nikolay Aleksandrov @ 2019-08-14 16:58 UTC (permalink / raw) To: pruddy, Ido Schimmel; +Cc: netdev, roopa, linus.luessing On 8/14/19 7:40 PM, Patrick Ruddy wrote: > Thanks both for the quick replies, answers inline... > > On Wed, 2019-08-14 at 02:55 +0300, Nikolay Aleksandrov wrote: >> On 8/13/19 10:53 PM, Ido Schimmel wrote: >>> + Bridge maintainers, Linus >>> >> >> Good catch Ido, thanks! >> First I'd say the subject needs to reflect that this is a bridge change >> better, please rearrange it like so - bridge: mcast: ... >> More below, >> >>> On Tue, Aug 13, 2019 at 03:18:04PM +0100, Patrick Ruddy wrote: >>>> At present only all-nodes IPv6 multicast packets are accepted by >>>> a bridge interface that is not in multicast router mode. Since >>>> other protocols can be running in the absense of multicast >>>> forwarding e.g. OSPFv3 IPv6 ND. Change the test to allow >>>> all of the FFx2::/16 range to be accepted when not in multicast >>>> router mode. This aligns the code with IPv4 link-local reception >>>> and RFC4291 >>> >>> Can you please quote the relevant part from RFC 4291? >>> >>>> Signed-off-by: Patrick Ruddy <pruddy@vyatta.att-mail.com> >>>> --- >>>> include/net/addrconf.h | 15 +++++++++++++++ >>>> net/bridge/br_multicast.c | 2 +- >>>> 2 files changed, 16 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/include/net/addrconf.h b/include/net/addrconf.h >>>> index becdad576859..05b42867e969 100644 >>>> --- a/include/net/addrconf.h >>>> +++ b/include/net/addrconf.h >>>> @@ -434,6 +434,21 @@ static inline void addrconf_addr_solict_mult(const struct in6_addr *addr, >>>> htonl(0xFF000000) | addr->s6_addr32[3]); >>>> } >>>> >>>> +/* >>>> + * link local multicast address range ffx2::/16 rfc4291 >>>> + */ >>>> +static inline bool ipv6_addr_is_ll_mcast(const struct in6_addr *addr) >>>> +{ >>>> +#if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) && BITS_PER_LONG == 64 >>>> + __be64 *p = (__be64 *)addr; >>>> + return ((p[0] & cpu_to_be64(0xff0f000000000000UL)) >>>> + ^ cpu_to_be64(0xff02000000000000UL)) == 0UL; >>>> +#else >>>> + return ((addr->s6_addr32[0] & htonl(0xff0f0000)) ^ >>>> + htonl(0xff020000)) == 0; >>>> +#endif >>>> +} >>>> + >>>> static inline bool ipv6_addr_is_ll_all_nodes(const struct in6_addr *addr) >>>> { >>>> #if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) && BITS_PER_LONG == 64 >>>> diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c >>>> index 9b379e110129..ed3957381fa2 100644 >>>> --- a/net/bridge/br_multicast.c >>>> +++ b/net/bridge/br_multicast.c >>>> @@ -1664,7 +1664,7 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br, >>>> err = ipv6_mc_check_mld(skb); >>>> >>>> if (err == -ENOMSG) { >>>> - if (!ipv6_addr_is_ll_all_nodes(&ipv6_hdr(skb)->daddr)) >>>> + if (!ipv6_addr_is_ll_mcast(&ipv6_hdr(skb)->daddr)) >>>> BR_INPUT_SKB_CB(skb)->mrouters_only = 1; >>> >>> IIUC, you want IPv6 link-local packets to be locally received, but this >>> also changes how these packets are flooded. RFC 4541 says that packets >> >> Indeed, we'll start flooding them all, not just the all hosts address. >> If that is at all required it'll definitely have to be optional. >> >>> addressed to the all hosts address are a special case and should be >>> forwarded to all ports: >>> >>> "In IPv6, the data forwarding rules are more straight forward because MLD is >>> mandated for addresses with scope 2 (link-scope) or greater. The only exception >>> is the address FF02::1 which is the all hosts link-scope address for which MLD >>> messages are never sent. Packets with the all hosts link-scope address should >>> be forwarded on all ports." >>> >> >> I wonder what is the problem for the host to join such group on behalf of the bridge ? >> Then you'll receive the traffic at least locally and the RFC says it itself - MLD is mandated >> for the other link-local addresses. >> It's very late here and maybe I'm missing something.. :) >> > The group is being joined by MLD at the L3 level but the packets are > not being passed up to the l3 interface becasue there is a MLD querier > on the network > That shouldn't matter if the host has joined the group, there is a specific check for that. If the host has joined the group and we have an mdst then we'll hit this code: mdst = br_mdb_get(br, skb, vid); if ((mdst || BR_INPUT_SKB_CB_MROUTERS_ONLY(skb)) && br_multicast_querier_exists(br, eth_hdr(skb))) { if ((mdst && mdst->host_joined) || br_multicast_is_router(br)) { local_rcv = true; br->dev->stats.multicast++; } mcast_hit = true; } else { local_rcv become true and the packet is passed up, so what is the problem ? Have you missed to refresh the group and it has expired in the bridge perhaps ? > snippet from /proc/net/igmp6 > ... > 40 sw1 ff0200000000000000000001ff008700 1 00000004 0 > 40 sw1 ff020000000000000000000000000002 1 00000004 0 > 40 sw1 ff020000000000000000000000000001 1 0000000C 0 > 40 sw1 ff010000000000000000000000000001 1 00000008 0 > 41 lo1 ff020000000000000000000000000001 1 0000000C 0 > 41 lo1 ff010000000000000000000000000001 1 00000008 0 > 42 sw1.1 ff020000000000000000000000000006 1 00000004 0 > 42 sw1.1 ff020000000000000000000000000005 1 00000004 0 > 42 sw1.1 ff0200000000000000000001ff000000 2 00000004 0 > 42 sw1.1 ff0200000000000000000001ff008700 1 00000004 0 > 42 sw1.1 ff0200000000000000000001ff000099 1 00000004 0 > 42 sw1.1 ff020000000000000000000000000002 1 00000004 0 > 42 sw1.1 ff020000000000000000000000000001 1 0000000C 0 > 42 sw1.1 ff010000000000000000000000000001 1 00000008 0 > ... > > the bridge is sw1 and the l3 intervace is sw1.1 > > Ido is correct about the flooding - I will update the patch with the > comments and reissue. > > Thanks again > > -pr >> >>> Maybe you want something like: >>> >> >> I think we can do without the new field, either pass local_rcv into br_multicast_rcv() or >> set it based on return value. The extra test will have to remain unfortunately, but we >> can reduce the tests by one if carefully done. >> >>> diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c >>> index 09b1dd8cd853..9f312a73f61c 100644 >>> --- a/net/bridge/br_input.c >>> +++ b/net/bridge/br_input.c >>> @@ -132,7 +132,8 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb >>> if ((mdst || BR_INPUT_SKB_CB_MROUTERS_ONLY(skb)) && >>> br_multicast_querier_exists(br, eth_hdr(skb))) { >>> if ((mdst && mdst->host_joined) || >>> - br_multicast_is_router(br)) { >>> + br_multicast_is_router(br) || >>> + BR_INPUT_SKB_CB_LOCAL_RECEIVE(skb)) { >>> local_rcv = true; >>> br->dev->stats.multicast++; >>> } >>> diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c >>> index 9b379e110129..f03cecf6174e 100644 >>> --- a/net/bridge/br_multicast.c >>> +++ b/net/bridge/br_multicast.c >>> @@ -1667,6 +1667,9 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br, >>> if (!ipv6_addr_is_ll_all_nodes(&ipv6_hdr(skb)->daddr)) >>> BR_INPUT_SKB_CB(skb)->mrouters_only = 1; >>> >>> + if (ipv6_addr_is_ll_mcast(&ipv6_hdr(skb)->daddr)) >>> + BR_INPUT_SKB_CB(skb)->local_receive = 1; >>> + >>> if (ipv6_addr_is_all_snoopers(&ipv6_hdr(skb)->daddr)) { >>> err = br_ip6_multicast_mrd_rcv(br, port, skb); >>> >>> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h >>> index b7a4942ff1b3..d76394ca4059 100644 >>> --- a/net/bridge/br_private.h >>> +++ b/net/bridge/br_private.h >>> @@ -426,6 +426,7 @@ struct br_input_skb_cb { >>> #ifdef CONFIG_BRIDGE_IGMP_SNOOPING >>> u8 igmp; >>> u8 mrouters_only:1; >>> + u8 local_receive:1; >>> #endif >>> u8 proxyarp_replied:1; >>> u8 src_port_isolated:1; >>> @@ -445,8 +446,10 @@ struct br_input_skb_cb { >>> >>> #ifdef CONFIG_BRIDGE_IGMP_SNOOPING >>> # define BR_INPUT_SKB_CB_MROUTERS_ONLY(__skb) (BR_INPUT_SKB_CB(__skb)->mrouters_only) >>> +# define BR_INPUT_SKB_CB_LOCAL_RECEIVE(__skb) (BR_INPUT_SKB_CB(__skb)->local_receive) >>> #else >>> # define BR_INPUT_SKB_CB_MROUTERS_ONLY(__skb) (0) >>> +# define BR_INPUT_SKB_CB_LOCAL_RECEIVE(__skb) (0) >>> #endif >>> >>> #define br_printk(level, br, format, args...) \ >>> > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next] mcast: ensure L-L IPv6 packets are accepted by bridge 2019-08-14 16:40 ` Patrick Ruddy 2019-08-14 16:58 ` Nikolay Aleksandrov @ 2019-08-14 20:11 ` Linus Lüssing 2019-08-14 20:34 ` Nikolay Aleksandrov 1 sibling, 1 reply; 8+ messages in thread From: Linus Lüssing @ 2019-08-14 20:11 UTC (permalink / raw) To: Patrick Ruddy; +Cc: bridge, Nikolay Aleksandrov, Ido Schimmel, netdev, roopa On Wed, Aug 14, 2019 at 05:40:58PM +0100, Patrick Ruddy wrote: > The group is being joined by MLD at the L3 level but the packets are > not being passed up to the l3 interface becasue there is a MLD querier > on the network > > snippet from /proc/net/igmp6 > ... > 40 sw1 ff0200000000000000000001ff008700 1 00000004 0 > 40 sw1 ff020000000000000000000000000002 1 00000004 0 > 40 sw1 ff020000000000000000000000000001 1 0000000C 0 > 40 sw1 ff010000000000000000000000000001 1 00000008 0 > 41 lo1 ff020000000000000000000000000001 1 0000000C 0 > 41 lo1 ff010000000000000000000000000001 1 00000008 0 > 42 sw1.1 ff020000000000000000000000000006 1 00000004 0 > 42 sw1.1 ff020000000000000000000000000005 1 00000004 0 > 42 sw1.1 ff0200000000000000000001ff000000 2 00000004 0 > 42 sw1.1 ff0200000000000000000001ff008700 1 00000004 0 > 42 sw1.1 ff0200000000000000000001ff000099 1 00000004 0 > 42 sw1.1 ff020000000000000000000000000002 1 00000004 0 > 42 sw1.1 ff020000000000000000000000000001 1 0000000C 0 > 42 sw1.1 ff010000000000000000000000000001 1 00000008 0 > ... > > the bridge is sw1 and the l3 intervace is sw1.1 What kind of interface is sw1.1 exactly? Is it a VLAN or a VRF interface? Something else? Could you also post the output of bridge mdb show? Regards, Linus PS: Also please include the bridge mailinglist in the future. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next] mcast: ensure L-L IPv6 packets are accepted by bridge 2019-08-14 20:11 ` Linus Lüssing @ 2019-08-14 20:34 ` Nikolay Aleksandrov 2019-08-15 13:36 ` Patrick Ruddy 0 siblings, 1 reply; 8+ messages in thread From: Nikolay Aleksandrov @ 2019-08-14 20:34 UTC (permalink / raw) To: Linus Lüssing, Patrick Ruddy; +Cc: bridge, Ido Schimmel, netdev, roopa On 8/14/19 11:11 PM, Linus Lüssing wrote: > On Wed, Aug 14, 2019 at 05:40:58PM +0100, Patrick Ruddy wrote: >> The group is being joined by MLD at the L3 level but the packets are >> not being passed up to the l3 interface becasue there is a MLD querier >> on the network >> >> snippet from /proc/net/igmp6 >> ... >> 40 sw1 ff0200000000000000000001ff008700 1 00000004 0 >> 40 sw1 ff020000000000000000000000000002 1 00000004 0 >> 40 sw1 ff020000000000000000000000000001 1 0000000C 0 >> 40 sw1 ff010000000000000000000000000001 1 00000008 0 >> 41 lo1 ff020000000000000000000000000001 1 0000000C 0 >> 41 lo1 ff010000000000000000000000000001 1 00000008 0 >> 42 sw1.1 ff020000000000000000000000000006 1 00000004 0 >> 42 sw1.1 ff020000000000000000000000000005 1 00000004 0 >> 42 sw1.1 ff0200000000000000000001ff000000 2 00000004 0 >> 42 sw1.1 ff0200000000000000000001ff008700 1 00000004 0 >> 42 sw1.1 ff0200000000000000000001ff000099 1 00000004 0 >> 42 sw1.1 ff020000000000000000000000000002 1 00000004 0 >> 42 sw1.1 ff020000000000000000000000000001 1 0000000C 0 >> 42 sw1.1 ff010000000000000000000000000001 1 00000008 0 >> ... >> >> the bridge is sw1 and the l3 intervace is sw1.1 > > What kind of interface is sw1.1 exactly? Is it a VLAN or a VRF > interface? Something else? > +1 > Could you also post the output of bridge mdb show? > > Regards, Linus > > > PS: Also please include the bridge mailinglist in the future. > Note that if you'd like to debug a host joined group currently bridge mdb show will not dump it and if the group is host-joined only it can even be empty. You can use my latest set (not applied yet): http://patchwork.ozlabs.org/project/netdev/list/?series=125169 Alternatively you could monitor the mdb events, it will show up there even today without any changes (bridge monitor mdb) and you can check if it's getting deleted. Cheers, Nik ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next] mcast: ensure L-L IPv6 packets are accepted by bridge 2019-08-14 20:34 ` Nikolay Aleksandrov @ 2019-08-15 13:36 ` Patrick Ruddy 0 siblings, 0 replies; 8+ messages in thread From: Patrick Ruddy @ 2019-08-15 13:36 UTC (permalink / raw) To: Nikolay Aleksandrov, Linus Lüssing Cc: bridge, Ido Schimmel, netdev, roopa On Wed, 2019-08-14 at 23:34 +0300, Nikolay Aleksandrov wrote: > On 8/14/19 11:11 PM, Linus Lüssing wrote: > > On Wed, Aug 14, 2019 at 05:40:58PM +0100, Patrick Ruddy wrote: > > > The group is being joined by MLD at the L3 level but the packets are > > > not being passed up to the l3 interface becasue there is a MLD querier > > > on the network > > > > > > snippet from /proc/net/igmp6 > > > ... > > > 40 sw1 ff0200000000000000000001ff008700 1 00000004 0 > > > 40 sw1 ff020000000000000000000000000002 1 00000004 0 > > > 40 sw1 ff020000000000000000000000000001 1 0000000C 0 > > > 40 sw1 ff010000000000000000000000000001 1 00000008 0 > > > 41 lo1 ff020000000000000000000000000001 1 0000000C 0 > > > 41 lo1 ff010000000000000000000000000001 1 00000008 0 > > > 42 sw1.1 ff020000000000000000000000000006 1 00000004 0 > > > 42 sw1.1 ff020000000000000000000000000005 1 00000004 0 > > > 42 sw1.1 ff0200000000000000000001ff000000 2 00000004 0 > > > 42 sw1.1 ff0200000000000000000001ff008700 1 00000004 0 > > > 42 sw1.1 ff0200000000000000000001ff000099 1 00000004 0 > > > 42 sw1.1 ff020000000000000000000000000002 1 00000004 0 > > > 42 sw1.1 ff020000000000000000000000000001 1 0000000C 0 > > > 42 sw1.1 ff010000000000000000000000000001 1 00000008 0 > > > ... > > > > > > the bridge is sw1 and the l3 intervace is sw1.1 > > > > What kind of interface is sw1.1 exactly? Is it a VLAN or a VRF > > interface? Something else? > > > +1 > > > Could you also post the output of bridge mdb show? > > > > Regards, Linus > > > > > > PS: Also please include the bridge mailinglist in the future. > > > > Note that if you'd like to debug a host joined group currently bridge mdb show > will not dump it and if the group is host-joined only it > can even be empty. You can use my latest set (not applied yet): > https://urldefense.proofpoint.com/v2/url?u=http-3A__patchwork.ozlabs.org_project_netdev_list_-3Fseries-3D125169&d=DwIDaQ&c=LFYZ-o9_HUMeMTSQicvjIg&r=au3D9TlUU6OvFpWOU9cuIHeNeV2fw-AOF1ZqCRqsILc&m=KsdarH0MAMMoKZ4PuvHrEC57uEluTGK-XSL4uUxu9MY&s=jyoK6VVmFh1KpKZirrtUYwq9nLy8fz-GigFFLjaLsoE&e= > > Alternatively you could monitor the mdb events, it will show up there even > today without any changes (bridge monitor mdb) and you can check if it's > getting deleted. > > Cheers, > Nik The sw1.1 interface is a .1q vlan The output of "bridge monitor mdb" is empty I can see the incoming query and the outging report on tshark: 29002 72654.887739 fe80::4041:1ff:fe00:101 → ff02::1 ICMPv6 94 Multicast Listener Query 29003 72655.502035 fe80::eac5:7aff:fe00:8700 → ff02::16 ICMPv6 194 Multicast Listener Report Message v2 debugging shows that bridge code sees the incoming query but not the outgoing report. Thanks for all the pointers - I will pursue what is happening. -pr ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-08-15 13:37 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-08-13 14:18 [PATCH net-next] mcast: ensure L-L IPv6 packets are accepted by bridge Patrick Ruddy 2019-08-13 19:53 ` Ido Schimmel 2019-08-13 23:55 ` Nikolay Aleksandrov 2019-08-14 16:40 ` Patrick Ruddy 2019-08-14 16:58 ` Nikolay Aleksandrov 2019-08-14 20:11 ` Linus Lüssing 2019-08-14 20:34 ` Nikolay Aleksandrov 2019-08-15 13:36 ` Patrick Ruddy
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).