| Message ID | 1435003931-6418-1-git-send-email-nikolay@cumulusnetworks.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
> On Jun 22, 2015, at 11:12 PM, Nikolay Aleksandrov <nikolay@cumulusnetworks.com> wrote: > > Currently when a port goes in blocking state the multicast is not > disabled. Fix it by disabling a port if its state has transitioned to > blocking, this has effect for both user- and kernel-space stp. > > Reported-by: Herbert Xu <herbert@gondor.apana.org.au> > Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> > --- > note: this is on top of patch: > "bridge: multicast: start querier timer when running user-space stp" > > net/bridge/br_stp.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/net/bridge/br_stp.c b/net/bridge/br_stp.c > index e7ab74b405a1..1a73c5595f52 100644 > --- a/net/bridge/br_stp.c > +++ b/net/bridge/br_stp.c > @@ -463,6 +463,8 @@ void br_port_state_selection(struct net_bridge *br) > > if (p->state != BR_STATE_BLOCKING) > br_multicast_enable_port(p); > + else > + br_multicast_disable_port(p); > if (p->state == BR_STATE_FORWARDING) > ++liveports; > } > -- > 2.4.3 > Actually I don’t think this is the correct way to go about this because when the port goes in blocking state and br_multicast_disable_port() is called then all groups are deleted which includes the user-added ones.-- To unsubscribe from this list: send the line "unsubscribe netdev" in
> On Jun 22, 2015, at 11:25 PM, Nikolay Aleksandrov <nikolay@cumulusnetworks.com> wrote: > > >> On Jun 22, 2015, at 11:12 PM, Nikolay Aleksandrov <nikolay@cumulusnetworks.com> wrote: >> >> Currently when a port goes in blocking state the multicast is not >> disabled. Fix it by disabling a port if its state has transitioned to >> blocking, this has effect for both user- and kernel-space stp. >> >> Reported-by: Herbert Xu <herbert@gondor.apana.org.au> >> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> >> --- >> note: this is on top of patch: >> "bridge: multicast: start querier timer when running user-space stp" >> >> net/bridge/br_stp.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/net/bridge/br_stp.c b/net/bridge/br_stp.c >> index e7ab74b405a1..1a73c5595f52 100644 >> --- a/net/bridge/br_stp.c >> +++ b/net/bridge/br_stp.c >> @@ -463,6 +463,8 @@ void br_port_state_selection(struct net_bridge *br) >> >> if (p->state != BR_STATE_BLOCKING) >> br_multicast_enable_port(p); >> + else >> + br_multicast_disable_port(p); >> if (p->state == BR_STATE_FORWARDING) >> ++liveports; >> } >> -- >> 2.4.3 >> > > Actually I don’t think this is the correct way to go about this because when the > port goes in blocking state and br_multicast_disable_port() is called then all groups are deleted > which includes the user-added ones.-- > To unsubscribe from this list: send the line "unsubscribe netdev” in One more thing - I don’t think we need any additional changes because there’s a check in br_multicast_port_query_expired(): if (port->state == BR_STATE_DISABLED || port->state == BR_STATE_BLOCKING) goto out; So it looks like the port should be fine (i.e. not sending) when it goes in blocking state. -- To unsubscribe from this list: send the line "unsubscribe netdev" in
On Mon, Jun 22, 2015 at 11:38:36PM +0300, Nikolay Aleksandrov wrote: > > One more thing - I don’t think we need any additional changes because there’s a check in > br_multicast_port_query_expired(): > if (port->state == BR_STATE_DISABLED || > port->state == BR_STATE_BLOCKING) > goto out; > > So it looks like the port should be fine (i.e. not sending) when it goes in blocking state. Thanks for looking into this! Perhaps we could at least add a comment to state that the disable_port isn't needed because the timers will expire and kill themselves automatically? I think adding it to the place where you were going to place the disable_port call would be the most obvious. Thanks,
> On Jun 23, 2015, at 2:52 AM, Herbert Xu <herbert@gondor.apana.org.au> wrote: > > On Mon, Jun 22, 2015 at 11:38:36PM +0300, Nikolay Aleksandrov wrote: >> >> One more thing - I don’t think we need any additional changes because there’s a check in >> br_multicast_port_query_expired(): >> if (port->state == BR_STATE_DISABLED || >> port->state == BR_STATE_BLOCKING) >> goto out; >> >> So it looks like the port should be fine (i.e. not sending) when it goes in blocking state. > > Thanks for looking into this! Perhaps we could at least add a > comment to state that the disable_port isn't needed because the > timers will expire and kill themselves automatically? I think > adding it to the place where you were going to place the disable_port > call would be the most obvious. > > Thanks, > -- > Email: Herbert Xu <herbert@gondor.apana.org.au> > Home Page: http://gondor.apana.org.au/~herbert/ > PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt Right, sounds good. I’ll post a patch with the comment in a bit. Thank you, Nik-- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/bridge/br_stp.c b/net/bridge/br_stp.c index e7ab74b405a1..1a73c5595f52 100644 --- a/net/bridge/br_stp.c +++ b/net/bridge/br_stp.c @@ -463,6 +463,8 @@ void br_port_state_selection(struct net_bridge *br) if (p->state != BR_STATE_BLOCKING) br_multicast_enable_port(p); + else + br_multicast_disable_port(p); if (p->state == BR_STATE_FORWARDING) ++liveports; }
Currently when a port goes in blocking state the multicast is not disabled. Fix it by disabling a port if its state has transitioned to blocking, this has effect for both user- and kernel-space stp. Reported-by: Herbert Xu <herbert@gondor.apana.org.au> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> --- note: this is on top of patch: "bridge: multicast: start querier timer when running user-space stp" net/bridge/br_stp.c | 2 ++ 1 file changed, 2 insertions(+)