Re: [PATCH net] net: dsa: fix VLAN 0 filter imbalance when toggling filtering

[Florian Fainelli]

On 4/24/2025 12:25 PM, Vladimir Oltean wrote:
> On Tue, Apr 22, 2025 at 08:49:13PM +0200, Jonas Gorski wrote:
> > When a net device has NETIF_F_HW_VLAN_CTAG_FILTER set, the 8021q code
> > will add VLAN 0 when enabling the device, and remove it on disabling it
> > again.
> >
> > But since we are changing NETIF_F_HW_VLAN_CTAG_FILTER during runtime in
> > dsa_user_manage_vlan_filtering(), user ports that are already enabled
> > may end up with no VLAN 0 configured, or VLAN 0 left configured.
> >
> > E.g.the following sequence would leave sw1p1 without VLAN 0 configured:
> >
> > $ ip link add br0 type bridge vlan_filtering 1
> > $ ip link set br0 up
> > $ ip link set sw1p1 up (filtering is 0, so no HW filter added)
> > $ ip link set sw1p1 master br0 (filtering gets set to 1, but already up)
> >
> > while the following sequence would work:
> >
> > $ ip link add br0 type bridge vlan_filtering 1
> > $ ip link set br0 up
> > $ ip link set sw1p1 master br0 (filtering gets set to 1)
> > $ ip link set sw1p1 up (filtering is 1, HW filter is added)
> >
> > Likewise, the following sequence would leave sw1p2 with a VLAN 0 filter
> > enabled on a vlan_filtering_is_global dsa switch:
> >
> > $ ip link add br0 type bridge vlan_filtering 1
> > $ ip link set br0 up
> > $ ip link set sw1p1 master br0 (filtering set to 1 for all devices)
> > $ ip link set sw1p2 up (filtering is 1, so VLAN 0 filter is added)
> > $ ip link set sw1p1 nomaster (filtering is reset to 0 again)
> > $ ip link set sw1p2 down (VLAN 0 filter is left configured)
> >
> > This even causes untagged traffic to break on b53 after undoing the
> > bridge (though this is partially caused by b53's own doing).
> >
> > Fix this by emulating 8021q's vlan_device_event() behavior when changing
> > the NETIF_F_HW_VLAN_CTAG_FILTER flag, including the printk, so that the
> > absence of it doesn't become a red herring.
> >
> > While vlan_vid_add() has a return value, vlan_device_event() does not
> > check its return value, so let us do the same.
> >
> > Fixes: 06cfb2df7eb0 ("net: dsa: don't advertise 'rx-vlan-filter' when not needed")
> > Signed-off-by: Jonas Gorski <jonas.gorski@xxxxxxxxx>
> > ---
>
> Why does the b53 driver depend on VID 0? CONFIG_VLAN_8021Q can be
> disabled or be an unloaded module, how does it work in that case?

This is explained in this commit:

https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=64a81b24487f0d2fba0f033029eec2abc7d82cee

however the case of starting up with CONFIG_VLAN_8021Q and then loading 
the 8021q module was not thought about, arguably I am not sure what sort 
of notification or event we can hook onto in order to react properly to 
that module being loaded. Do you know?
--
Florian