Re: [PATCH 4.19 01/24] bridge: do not add port to router list when receives query with source 0.0.0.0
Am 20.02.2019 um 15:46 schrieb Hangbin Liu: Could you please include more details about the setup that's broken ? Note that we were warned[1] of potential breakage from this change Sorry I missed Linus's reply after Ying Xu replied. I will read it and disscuss with Ying Xu. after it went in and regardless of the suggestion from the RFC we'll probably have to revert this patch. Ying Xu as author of the patch, any thoughts ? No, we are also waiting for more details from Sebastian. the test scenario is the following. one of my testers of the dd-wrt firmware is using a iptv stream from the france isp "orange". he just bridged the iptv vlan interface which has no ip (the isp doesnt provide a ip configuration here) to a bridge and played with igmp snooping to avoid flooding of all ports within the same bridge. he found out that this setting simply does not work with the current kernel, but does if this patch has been removed. we used kernel 4.4 in the test scenario. all latest revision. i was able to reproduce the same problem on kernel 4.9 and 4.14 on a different device using a german isp based iptv stream. same effect. after reverting the problematic code it worked again. Sebastian Thanks Hangbin Also adding Linus Lüssing to the CC as he was the one who warned against it. Note that the warning was sent as a reply to my breakage fix, but it was intended for the original patch. Thanks, Nik [1] https://www.mail-archive.com/netdev@vger.kernel.org/msg272944.html
Re: [PATCH 4.19 01/24] bridge: do not add port to router list when receives query with source 0.0.0.0
On Wed, Feb 20, 2019 at 01:48:47PM +0100, Sebastian Gottschall wrote: > *reminder* > > Am 18.02.2019 um 11:18 schrieb Sebastian Gottschall: > > > > Am 17.02.2019 um 17:48 schrieb Greg Kroah-Hartman: > > > On Sun, Feb 17, 2019 at 03:29:22PM +0100, Sebastian Gottschall wrote: > > > > according to user reports this patch will cause a serious > > > > regression. igmp > > > > snooping is not working anymore with this patch > > > > > > > > Am 02.11.2018 um 19:34 schrieb Greg Kroah-Hartman: > > > > > 4.19-stable review patch. If anyone has any objections, > > > > > please let me know. > > > > > > > > > > -- > > > > > > > > > > From: Hangbin Liu > > > > > > > > > > [ Upstream commit 5a2de63fd1a59c30c02526d427bc014b98adf508 ] > > > > > > > > > > Based on RFC 4541, 2.1.1. IGMP Forwarding Rules > > > > > > > > > > The switch supporting IGMP snooping must maintain a list of > > > > > multicast routers and the ports on which they are attached. This > > > > > list can be constructed in any combination of the following ways: > > > > > > > > > > a) This list should be built by the snooping switch sending > > > > > Multicast Router Solicitation messages as described in IGMP > > > > > Multicast Router Discovery [MRDISC]. It may also snoop > > > > > Multicast Router Advertisement messages sent by and to other > > > > > nodes. > > > > > > > > > > b) The arrival port for IGMP Queries (sent by multicast routers) > > > > > where the source address is not 0.0.0.0. > > > > > > > > > > We should not add the port to router list when receives > > > > > query with source > > > > > 0.0.0.0. > > > > > > > > > > Reported-by: Ying Xu > > > > > Signed-off-by: Hangbin Liu > > > > > Acked-by: Nikolay Aleksandrov > > > > > Acked-by: Roopa Prabhu > > > > > Signed-off-by: David S. Miller > > > > > Signed-off-by: Greg Kroah-Hartman > > > > > --- > > > > > net/bridge/br_multicast.c | 10 +- > > > > > 1 file changed, 9 insertions(+), 1 deletion(-) > > > > > > > > > > --- a/net/bridge/br_multicast.c > > > > > +++ b/net/bridge/br_multicast.c > > > > > @@ -1420,7 +1420,15 @@ static void br_multicast_query_received( > > > > > return; > > > > > br_multicast_update_query_timer(br, query, max_delay); > > > > > - br_multicast_mark_router(br, port); > > > > > + > > > > > + /* Based on RFC4541, section 2.1.1 IGMP Forwarding Rules, > > > > > + * the arrival port for IGMP Queries where the source address > > > > > + * is 0.0.0.0 should not be added to router port list. > > > > > + */ > > > > > + if ((saddr->proto == htons(ETH_P_IP) && saddr->u.ip4) || > > > > > + (saddr->proto == htons(ETH_P_IPV6) && > > > > > + !ipv6_addr_any(>u.ip6))) > > > > > + br_multicast_mark_router(br, port); > > > > > } > > > > > static void br_ip4_multicast_query(struct net_bridge *br, > > > Is this also a problem in 4.20? This patch went into 4.20-rc1, so it > > > has been around for a while with no reported issues that I can find. > > > Any pointers to the reports? > > > > i need to check this. i found this patch in 4.9, 4.14 and 4.4 > > the rest was picked up from the mailinglist. according to the git > > sources of 4.20 and 5.0 the same code is in there as well > > > > i just got the report from users today and was able to reproduce it with > > iptv streams. just by disabling the code it was working again. Reminder of what? If this is still an issue in 4.20, and 5.0-rc, there's nothing I can do. Please work with the networking developers to resolve the issue and get the fix merged into Linus's tree and then I will be glad to take the fix into the stable trees. If this is _not_ an issue with 4.20 and 5.0-rc, I need to know that as well. thanks, greg k-h
Re: [PATCH 4.19 01/24] bridge: do not add port to router list when receives query with source 0.0.0.0
On Wed, Feb 20, 2019 at 03:09:21PM +0200, Nikolay Aleksandrov wrote: > On 20/02/2019 14:48, Sebastian Gottschall wrote: > > *reminder* > > > [snip] > > } > > static void br_ip4_multicast_query(struct net_bridge *br, > >>> Is this also a problem in 4.20? This patch went into 4.20-rc1, so it > >>> has been around for a while with no reported issues that I can find. > >>> Any pointers to the reports? > >> > >> i need to check this. i found this patch in 4.9, 4.14 and 4.4 > >> the rest was picked up from the mailinglist. according to the git sources > >> of 4.20 and 5.0 the same code is in there as well > >> > >> i just got the report from users today and was able to reproduce it with > >> iptv streams. just by disabling the code it was working again. > >> > >> Sebastian > >>> > >>> thanks, > >>> > >>> greg k-h > >>> > >> > > Could you please include more details about the setup that's broken ? > Note that we were warned[1] of potential breakage from this change Sorry I missed Linus's reply after Ying Xu replied. I will read it and disscuss with Ying Xu. > after it went in and regardless of the suggestion from the RFC we'll > probably have to revert this patch. > > Ying Xu as author of the patch, any thoughts ? No, we are also waiting for more details from Sebastian. Thanks Hangbin > > Also adding Linus Lüssing to the CC as he was the one who warned against it. > Note that the warning was sent as a reply to my breakage fix, but it was > intended > for the original patch. > > Thanks, > Nik > > [1] https://www.mail-archive.com/netdev@vger.kernel.org/msg272944.html >
Re: [PATCH 4.19 01/24] bridge: do not add port to router list when receives query with source 0.0.0.0
On 20/02/2019 15:09, Nikolay Aleksandrov wrote: > On 20/02/2019 14:48, Sebastian Gottschall wrote: >> *reminder* >> > [snip] >> } >> static void br_ip4_multicast_query(struct net_bridge *br, Is this also a problem in 4.20? This patch went into 4.20-rc1, so it has been around for a while with no reported issues that I can find. Any pointers to the reports? >>> >>> i need to check this. i found this patch in 4.9, 4.14 and 4.4 >>> the rest was picked up from the mailinglist. according to the git sources >>> of 4.20 and 5.0 the same code is in there as well >>> >>> i just got the report from users today and was able to reproduce it with >>> iptv streams. just by disabling the code it was working again. >>> >>> Sebastian thanks, greg k-h >>> > > Could you please include more details about the setup that's broken ? > Note that we were warned[1] of potential breakage from this change > after it went in and regardless of the suggestion from the RFC we'll > probably have to revert this patch. > > Ying Xu as author of the patch, any thoughts ? > My bad, it's Hangbin Liu's patch. It was reported by Ying Xu. > Also adding Linus Lüssing to the CC as he was the one who warned against it. > Note that the warning was sent as a reply to my breakage fix, but it was > intended > for the original patch. > > Thanks, > Nik > > [1] https://www.mail-archive.com/netdev@vger.kernel.org/msg272944.html >
Re: [PATCH 4.19 01/24] bridge: do not add port to router list when receives query with source 0.0.0.0
On 20/02/2019 14:48, Sebastian Gottschall wrote: > *reminder* > [snip] > } > static void br_ip4_multicast_query(struct net_bridge *br, >>> Is this also a problem in 4.20? This patch went into 4.20-rc1, so it >>> has been around for a while with no reported issues that I can find. >>> Any pointers to the reports? >> >> i need to check this. i found this patch in 4.9, 4.14 and 4.4 >> the rest was picked up from the mailinglist. according to the git sources of >> 4.20 and 5.0 the same code is in there as well >> >> i just got the report from users today and was able to reproduce it with >> iptv streams. just by disabling the code it was working again. >> >> Sebastian >>> >>> thanks, >>> >>> greg k-h >>> >> Could you please include more details about the setup that's broken ? Note that we were warned[1] of potential breakage from this change after it went in and regardless of the suggestion from the RFC we'll probably have to revert this patch. Ying Xu as author of the patch, any thoughts ? Also adding Linus Lüssing to the CC as he was the one who warned against it. Note that the warning was sent as a reply to my breakage fix, but it was intended for the original patch. Thanks, Nik [1] https://www.mail-archive.com/netdev@vger.kernel.org/msg272944.html
Re: [PATCH 4.19 01/24] bridge: do not add port to router list when receives query with source 0.0.0.0
*reminder* Am 18.02.2019 um 11:18 schrieb Sebastian Gottschall: Am 17.02.2019 um 17:48 schrieb Greg Kroah-Hartman: On Sun, Feb 17, 2019 at 03:29:22PM +0100, Sebastian Gottschall wrote: according to user reports this patch will cause a serious regression. igmp snooping is not working anymore with this patch Am 02.11.2018 um 19:34 schrieb Greg Kroah-Hartman: 4.19-stable review patch. If anyone has any objections, please let me know. -- From: Hangbin Liu [ Upstream commit 5a2de63fd1a59c30c02526d427bc014b98adf508 ] Based on RFC 4541, 2.1.1. IGMP Forwarding Rules The switch supporting IGMP snooping must maintain a list of multicast routers and the ports on which they are attached. This list can be constructed in any combination of the following ways: a) This list should be built by the snooping switch sending Multicast Router Solicitation messages as described in IGMP Multicast Router Discovery [MRDISC]. It may also snoop Multicast Router Advertisement messages sent by and to other nodes. b) The arrival port for IGMP Queries (sent by multicast routers) where the source address is not 0.0.0.0. We should not add the port to router list when receives query with source 0.0.0.0. Reported-by: Ying Xu Signed-off-by: Hangbin Liu Acked-by: Nikolay Aleksandrov Acked-by: Roopa Prabhu Signed-off-by: David S. Miller Signed-off-by: Greg Kroah-Hartman --- net/bridge/br_multicast.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) --- a/net/bridge/br_multicast.c +++ b/net/bridge/br_multicast.c @@ -1420,7 +1420,15 @@ static void br_multicast_query_received( return; br_multicast_update_query_timer(br, query, max_delay); - br_multicast_mark_router(br, port); + + /* Based on RFC4541, section 2.1.1 IGMP Forwarding Rules, + * the arrival port for IGMP Queries where the source address + * is 0.0.0.0 should not be added to router port list. + */ + if ((saddr->proto == htons(ETH_P_IP) && saddr->u.ip4) || + (saddr->proto == htons(ETH_P_IPV6) && + !ipv6_addr_any(>u.ip6))) + br_multicast_mark_router(br, port); } static void br_ip4_multicast_query(struct net_bridge *br, Is this also a problem in 4.20? This patch went into 4.20-rc1, so it has been around for a while with no reported issues that I can find. Any pointers to the reports? i need to check this. i found this patch in 4.9, 4.14 and 4.4 the rest was picked up from the mailinglist. according to the git sources of 4.20 and 5.0 the same code is in there as well i just got the report from users today and was able to reproduce it with iptv streams. just by disabling the code it was working again. Sebastian thanks, greg k-h
Re: [PATCH 4.19 01/24] bridge: do not add port to router list when receives query with source 0.0.0.0
Am 17.02.2019 um 17:48 schrieb Greg Kroah-Hartman: On Sun, Feb 17, 2019 at 03:29:22PM +0100, Sebastian Gottschall wrote: according to user reports this patch will cause a serious regression. igmp snooping is not working anymore with this patch Am 02.11.2018 um 19:34 schrieb Greg Kroah-Hartman: 4.19-stable review patch. If anyone has any objections, please let me know. -- From: Hangbin Liu [ Upstream commit 5a2de63fd1a59c30c02526d427bc014b98adf508 ] Based on RFC 4541, 2.1.1. IGMP Forwarding Rules The switch supporting IGMP snooping must maintain a list of multicast routers and the ports on which they are attached. This list can be constructed in any combination of the following ways: a) This list should be built by the snooping switch sending Multicast Router Solicitation messages as described in IGMP Multicast Router Discovery [MRDISC]. It may also snoop Multicast Router Advertisement messages sent by and to other nodes. b) The arrival port for IGMP Queries (sent by multicast routers) where the source address is not 0.0.0.0. We should not add the port to router list when receives query with source 0.0.0.0. Reported-by: Ying Xu Signed-off-by: Hangbin Liu Acked-by: Nikolay Aleksandrov Acked-by: Roopa Prabhu Signed-off-by: David S. Miller Signed-off-by: Greg Kroah-Hartman --- net/bridge/br_multicast.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) --- a/net/bridge/br_multicast.c +++ b/net/bridge/br_multicast.c @@ -1420,7 +1420,15 @@ static void br_multicast_query_received( return; br_multicast_update_query_timer(br, query, max_delay); - br_multicast_mark_router(br, port); + + /* Based on RFC4541, section 2.1.1 IGMP Forwarding Rules, + * the arrival port for IGMP Queries where the source address + * is 0.0.0.0 should not be added to router port list. + */ + if ((saddr->proto == htons(ETH_P_IP) && saddr->u.ip4) || + (saddr->proto == htons(ETH_P_IPV6) && + !ipv6_addr_any(>u.ip6))) + br_multicast_mark_router(br, port); } static void br_ip4_multicast_query(struct net_bridge *br, Is this also a problem in 4.20? This patch went into 4.20-rc1, so it has been around for a while with no reported issues that I can find. Any pointers to the reports? i need to check this. i found this patch in 4.9, 4.14 and 4.4 the rest was picked up from the mailinglist. according to the git sources of 4.20 and 5.0 the same code is in there as well i just got the report from users today and was able to reproduce it with iptv streams. just by disabling the code it was working again. Sebastian thanks, greg k-h
Re: [PATCH 4.19 01/24] bridge: do not add port to router list when receives query with source 0.0.0.0
On Sun, Feb 17, 2019 at 03:29:22PM +0100, Sebastian Gottschall wrote: > according to user reports this patch will cause a serious regression. igmp > snooping is not working anymore with this patch > > Am 02.11.2018 um 19:34 schrieb Greg Kroah-Hartman: > > 4.19-stable review patch. If anyone has any objections, please let me know. > > > > -- > > > > From: Hangbin Liu > > > > [ Upstream commit 5a2de63fd1a59c30c02526d427bc014b98adf508 ] > > > > Based on RFC 4541, 2.1.1. IGMP Forwarding Rules > > > >The switch supporting IGMP snooping must maintain a list of > >multicast routers and the ports on which they are attached. This > >list can be constructed in any combination of the following ways: > > > >a) This list should be built by the snooping switch sending > > Multicast Router Solicitation messages as described in IGMP > > Multicast Router Discovery [MRDISC]. It may also snoop > > Multicast Router Advertisement messages sent by and to other > > nodes. > > > >b) The arrival port for IGMP Queries (sent by multicast routers) > > where the source address is not 0.0.0.0. > > > > We should not add the port to router list when receives query with source > > 0.0.0.0. > > > > Reported-by: Ying Xu > > Signed-off-by: Hangbin Liu > > Acked-by: Nikolay Aleksandrov > > Acked-by: Roopa Prabhu > > Signed-off-by: David S. Miller > > Signed-off-by: Greg Kroah-Hartman > > --- > > net/bridge/br_multicast.c | 10 +- > > 1 file changed, 9 insertions(+), 1 deletion(-) > > > > --- a/net/bridge/br_multicast.c > > +++ b/net/bridge/br_multicast.c > > @@ -1420,7 +1420,15 @@ static void br_multicast_query_received( > > return; > > br_multicast_update_query_timer(br, query, max_delay); > > - br_multicast_mark_router(br, port); > > + > > + /* Based on RFC4541, section 2.1.1 IGMP Forwarding Rules, > > +* the arrival port for IGMP Queries where the source address > > +* is 0.0.0.0 should not be added to router port list. > > +*/ > > + if ((saddr->proto == htons(ETH_P_IP) && saddr->u.ip4) || > > + (saddr->proto == htons(ETH_P_IPV6) && > > +!ipv6_addr_any(>u.ip6))) > > + br_multicast_mark_router(br, port); > > } > > static void br_ip4_multicast_query(struct net_bridge *br, Is this also a problem in 4.20? This patch went into 4.20-rc1, so it has been around for a while with no reported issues that I can find. Any pointers to the reports? thanks, greg k-h
Re: [PATCH 4.19 01/24] bridge: do not add port to router list when receives query with source 0.0.0.0
according to user reports this patch will cause a serious regression. igmp snooping is not working anymore with this patch Am 02.11.2018 um 19:34 schrieb Greg Kroah-Hartman: 4.19-stable review patch. If anyone has any objections, please let me know. -- From: Hangbin Liu [ Upstream commit 5a2de63fd1a59c30c02526d427bc014b98adf508 ] Based on RFC 4541, 2.1.1. IGMP Forwarding Rules The switch supporting IGMP snooping must maintain a list of multicast routers and the ports on which they are attached. This list can be constructed in any combination of the following ways: a) This list should be built by the snooping switch sending Multicast Router Solicitation messages as described in IGMP Multicast Router Discovery [MRDISC]. It may also snoop Multicast Router Advertisement messages sent by and to other nodes. b) The arrival port for IGMP Queries (sent by multicast routers) where the source address is not 0.0.0.0. We should not add the port to router list when receives query with source 0.0.0.0. Reported-by: Ying Xu Signed-off-by: Hangbin Liu Acked-by: Nikolay Aleksandrov Acked-by: Roopa Prabhu Signed-off-by: David S. Miller Signed-off-by: Greg Kroah-Hartman --- net/bridge/br_multicast.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) --- a/net/bridge/br_multicast.c +++ b/net/bridge/br_multicast.c @@ -1420,7 +1420,15 @@ static void br_multicast_query_received( return; br_multicast_update_query_timer(br, query, max_delay); - br_multicast_mark_router(br, port); + + /* Based on RFC4541, section 2.1.1 IGMP Forwarding Rules, +* the arrival port for IGMP Queries where the source address +* is 0.0.0.0 should not be added to router port list. +*/ + if ((saddr->proto == htons(ETH_P_IP) && saddr->u.ip4) || + (saddr->proto == htons(ETH_P_IPV6) && +!ipv6_addr_any(>u.ip6))) + br_multicast_mark_router(br, port); } static void br_ip4_multicast_query(struct net_bridge *br,
