Re: [PATCH v2 net] net: dsa: fix switchdev objects on bridge master mistakenly being applied on ports

[Tobias Waldekranz <tobias@waldekranz.com>]

On Sun, Mar 07, 2021 at 12:21, Vladimir Oltean <olteanv@gmail.com> wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
>
> Tobias reports that after the blamed patch, VLAN objects being added to
> a bridge device are being added to all slave ports instead (swp2, swp3).
>
> ip link add br0 type bridge vlan_filtering 1
> ip link set swp2 master br0
> ip link set swp3 master br0
> bridge vlan add dev br0 vid 100 self
>
> This is because the fix was too broad: we made dsa_port_offloads_netdev
> say "yes, I offload the br0 bridge" for all slave ports, but we didn't
> add the checks whether the switchdev object was in fact meant for the
> physical port or for the bridge itself. So we are reacting on events in
> a way in which we shouldn't.
>
> The reason why the fix was too broad is because the question itself,
> "does this DSA port offload this netdev", was too broad in the first
> place. The solution is to disambiguate the question and separate it into
> two different functions, one to be called for each switchdev attribute /
> object that has an orig_dev == net_bridge (dsa_port_offloads_bridge),
> and the other for orig_dev == net_bridge_port (*_offloads_bridge_port).
>
> In the case of VLAN objects on the bridge interface, this solves the
> problem because we know that VLAN objects are per bridge port and not
> per bridge. And when orig_dev is equal to the net_bridge, we offload it
> as a bridge, but not as a bridge port; that's how we are able to skip
> reacting on those events. Note that this is compatible with future plans
> to have explicit offloading of VLAN objects on the bridge interface as a
> bridge port (in DSA, this signifies that we should add that VLAN towards
> the CPU port).
>
> Fixes: 99b8202b179f ("net: dsa: fix SWITCHDEV_ATTR_ID_BRIDGE_VLAN_FILTERING getting ignored")
> Reported-by: Tobias Waldekranz <tobias@waldekranz.com>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---

Please wait before applying.

I need to do some more testing later (possibly tomorrow). But I am
pretty sure that this patch does not work with the (admittedly somewhat
exotic) combination of:

- Non-offloaded LAG
- Bridge with VLAN filtering enabled.

When adding the LAG to the bridge, I get an error because mv88e6xxx
tries to add VLAN 1 to the ports (which it should not do as the LAG is
not offloaded).