[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Z/mA27oWj2eSvTTF@shell.armlinux.org.uk>
Date: Fri, 11 Apr 2025 21:51:39 +0100
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Vladimir Oltean <olteanv@...il.com>
Cc: netdev@...r.kernel.org
Subject: Re: [BUG] 6.14: WARNING: CPU: 0 PID: 478 at net/bridge/br_vlan.c:433
nbp_vlan_flush+0xc0/0xc4
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@....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!
Powered by blists - more mailing lists