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

From: Tobias Waldekranz <[email-masked]>
Date: Mon, 08 Mar 2021 09:06:33 +0100

On Sun, Mar 07, 2021 at 12:21, Vladimir Oltean <[email-masked]> wrote:
> From: Vladimir Oltean <[email-masked]>
>
> 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 <[email-masked]>
> Signed-off-by: Vladimir Oltean <[email-masked]>
> ---
> This is the logical v2 of Tobias' patches from here:
> https://patchwork.kernel.org/project/netdevbpf/cover/[email-masked]/

The issue related to the combo of software lagged ports in a VLAN
filtering bridge is a separate one, so I think this is fine the way it
is. I will address that issue in a separate patch.

Reviewed-by: Tobias Waldekranz <[email-masked]>
Tested-by: Tobias Waldekranz <[email-masked]>