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

[Florian Fainelli <f.fainelli@gmail.com>]

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@gmail.com>
>> ---
> 
> 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