[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Z_mAFvJ9w4kn0v_G@shell.armlinux.org.uk>
Date: Fri, 11 Apr 2025 21:48:22 +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: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?
--
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