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@xxxxxxxxxxxxxxxx> 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:
pgpDDEdpjwDM9.pgp
Description: PGP signature
