Linux Ethernet Bridging - Feb 2021 - [Bridge] [PATCH v2 net-next 04/11] net: bridge: offload initial and final port flags through switchdev

On Tue, Feb 09, 2021 at 08:51:00PM +0200, Ido Schimmel
wrote:
> On Tue, Feb 09, 2021 at 05:19:29PM +0200, Vladimir Oltean wrote:
> > So switchdev drivers operating in standalone mode should disable
address
> > learning. As a matter of practicality, we can reduce code duplication
in
> > drivers by having the bridge notify through switchdev of the initial
and
> > final brport flags. Then, drivers can simply start up hardcoded for no
> > address learning (similar to how they already start up hardcoded for
no
> > forwarding), then they only need to listen for
> > SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS and their job is basically done,
no
> > need for special cases when the port joins or leaves the bridge etc.
> 
> How are you handling the case where a port leaves a LAG that is linked
> to a bridge? In this case the port becomes a standalone port, but will
> not get this notification.
Apparently the answer to that question is "I delete the code that makes
this use case work", how smart of me. Thanks.

Unless you have any idea how I could move the logic into the bridge, I
guess I'm stuck with DSA and all the other switchdev drivers having this
forest of corner cases to deal with. At least I can add a comment so I'm
not tempted to delete it next time.

On Tue, Feb 09, 2021 at 10:20:45PM +0200, Vladimir Oltean
wrote:
> On Tue, Feb 09, 2021 at 08:51:00PM +0200, Ido Schimmel wrote:
> > On Tue, Feb 09, 2021 at 05:19:29PM +0200, Vladimir Oltean wrote:
> > > So switchdev drivers operating in standalone mode should disable
address
> > > learning. As a matter of practicality, we can reduce code
duplication in
> > > drivers by having the bridge notify through switchdev of the
initial and
> > > final brport flags. Then, drivers can simply start up hardcoded
for no
> > > address learning (similar to how they already start up hardcoded
for no
> > > forwarding), then they only need to listen for
> > > SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS and their job is basically
done, no
> > > need for special cases when the port joins or leaves the bridge
etc.
> > 
> > How are you handling the case where a port leaves a LAG that is linked
> > to a bridge? In this case the port becomes a standalone port, but will
> > not get this notification.
> 
> Apparently the answer to that question is "I delete the code that
makes
> this use case work", how smart of me. Thanks.
Not sure how you expect to interpret this.
> 
> Unless you have any idea how I could move the logic into the bridge, I
> guess I'm stuck with DSA and all the other switchdev drivers having
this
> forest of corner cases to deal with. At least I can add a comment so
I'm
> not tempted to delete it next time.
There are too many moving pieces with stacked devices. It is not only
LAG/bridge. In L3 you have VRFs, SVIs, macvlans etc. It might be better
to gracefully / explicitly not handle a case rather than pretending to
handle it correctly with complex / buggy code.

For example, you should refuse to be enslaved to a LAG that already has
upper devices such as a bridge. You are probably not handling this
correctly / at all. This is easy. Just a call to
netdev_has_any_upper_dev().

The reverse, during unlinking, would be to refuse unlinking if the upper
has uppers of its own. netdev_upper_dev_unlink() needs to learn to
return an error and callers such as team/bond need to learn to handle
it, but it seems patchable.