Re: [BUG] 6.14: WARNING: CPU: 0 PID: 478 at net/bridge/br_vlan.c:433 nbp_vlan_flush+0xc0/0xc4

["Russell King (Oracle)" <linux@armlinux.org.uk>]

On Fri, Apr 11, 2025 at 09:49:02PM +0300, Vladimir Oltean wrote:
> From 508d912b5f6b56c3f588b1bf28d3caed9e30db1b Mon Sep 17 00:00:00 2001
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> Date: Fri, 11 Apr 2025 21:38:52 +0300
> Subject: [PATCH] net: dsa: mv88e6xxx: fix -ENOENT while deleting user port
>  VLANs
> 
> Russell King reports that on the ZII dev rev B, deleting a bridge VLAN
> from a user port fails with -ENOENT:
> https://lore.kernel.org/netdev/Z_lQXNP0s5-IiJzd@shell.armlinux.org.uk/
> 
> This comes from mv88e6xxx_port_vlan_leave() -> mv88e6xxx_mst_put(),
> which tries to find an MST entry in &chip->msts associated with the SID,
> but fails and returns -ENOENT as such.
> 
> But we know that this chip does not support MST at all, so that is not
> surprising. The question is why does the guard in mv88e6xxx_mst_put()
> not exit early:
> 
> 	if (!sid)
> 		return 0;
> 
> And the answer seems to be simple: the sid comes from vlan.sid which
> supposedly was previously populated by mv88e6xxx_vtu_loadpurge().
> But some chip->info->ops->vtu_loadpurge() implementations do not look at
> vlan.sid at all, for example see mv88e6185_g1_vtu_loadpurge().

This paragraph isn't accurate. It's actually:

mv88e6xxx_port_vlan_leave()
{
	struct mv88e6xxx_vtu_entry vlan;

	err = mv88e6xxx_vtu_get(chip, vid, &vlan);

and _this_ leaves vlan.sid uninitialised when mv88e6xxx_vtu_get()
ends up calling mv88e6185_g1_vtu_getnext().

I posioned to vlan (using 0xde) and then hexdump'd it after this call,
and got:

[   50.748068] mv88e6085 mdio_mux-0.4:00: p9 dsa_port_do_vlan_del vid 1
[   50.754802] e0b61b08: 01 00 02 00 de 01 de 03 03 03 03 03 03 03 03 03
[   50.761343] e0b61b18: 00 de de 00 00 00 00 00 00 00 00 00 00 de de de
[   50.767855] mv88e6085 mdio_mux-0.4:00: p9 vid 1 valid 0 (0-10)
[   50.773943] mv88e6085 mdio_mux-0.4:00: p9 !user err=-2

Note byte 4, which is the sid, is the poison value.

So, should mv88e6xxx_vtu_get(), being the first caller of the iterator,
clear vlan entirely before calling chip->info->ops->vtu_getnext()
rather than just initialising a few fields? Or should
mv88e6185_g1_vtu_getnext() ensure that entry->sid is set to zero?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!