Re: [Bridge] [PATCH 3/3] bridge: fix bridge root block on designated port

["Luis R. Rodriguez" <mcgrof@suse.com>]

[-- Attachment #1: Type: text/plain, Size: 1596 bytes --]

On Thu, Mar 13, 2014 at 03:16:23PM -0700, Stephen Hemminger wrote:
> On Wed, 12 Mar 2014 20:15:27 -0700
> "Luis R. Rodriguez" <mcgrof@do-not-panic.com> wrote:
> 
> > --- a/net/bridge/br_private.h
> > +++ b/net/bridge/br_private.h
> > @@ -150,6 +150,7 @@ struct net_bridge_port
> >  	u8				priority;
> >  	u8				state;
> >  	u16				port_no;
> > +	bool				root_block_enabled;
> >  	unsigned char			topology_change_ack;
> 
> It seems a bit confusing to have both a ROOT_BLOCK flag in the
> data structure and and additional root_block_enabled flag.
> If nothing else it is a waste of space.

Indeed, however there is a use for it. Consider the case where we loop
over each port and check to see if its root blocked and need to tickle it
or the bridge. In the case that root port block was enabled before and
someone is lifting it the flag would be removed and therefore not on
but it was root blocked though and we need a way to keep track of that.

The flag then is a toggle for userspace, while the bool tells us about
the current state.

> Looks like you are changing the meaning slightly. 

Let me know in what way. I can't see it.

> is possible to have BR_ROOT_BLOCK set but !root_block_enabled? 

Yeah in the case a new request to set it to root block then
BR_ROOT_BLOCK would be set but root_block_enabled would not be set.

> and what about the inverse?

BR_ROOT_BLOCK would not be set when userspace wants to disable root
port block and root_block_enabled would be enabled in this case if
it used to be enabled. So yes, both are possible.

  Luis

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]