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:48:22PM +0100, Russell King (Oracle) wrote:
> 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?

Or maybe test mv88e6xxx_has_stu() before calling mv88e6xxx_mst_put() ?

If mv88e6xxx_has_stu() is not sufficient, then mv88e6xxx_vlan_msti_set()
is another site where mv88e6xxx_vtu_get() is used followed by use of
vlan.sid.

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