* [PATCH net] net: dsa: motorcomm: Do not inherit ports from dynamic FDB entries
@ 2026-11-09 15:34 David Yang
2026-11-11 15:38 ` netdev-bot+sashiko
0 siblings, 1 reply; 4+ messages in thread
From: David Yang @ 2026-11-09 15:34 UTC (permalink / raw)
To: netdev
Cc: David Yang, Andrew Lunn, Vladimir Oltean, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, linux-kernel
When adding an fdb/mdb entry that the hardware has already learned as
dynamic, the learned ports were silently converted into the static
entry, which DSA loses track of; the entry is silently expanded and can
never be deleted. Replace the port mask instead of merging when the
existing entry is not static.
Found by code inspection.
Fixes: 186623f4aa72 ("net: dsa: yt921x: Add support for Motorcomm YT921x")
Signed-off-by: David Yang <mmyangfl@gmail.com>
---
drivers/net/dsa/motorcomm/chip.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/net/dsa/motorcomm/chip.c b/drivers/net/dsa/motorcomm/chip.c
index a7a9a7439304..6efd74e7eac6 100644
--- a/drivers/net/dsa/motorcomm/chip.c
+++ b/drivers/net/dsa/motorcomm/chip.c
@@ -805,7 +805,10 @@ yt921x_fdb_join(struct yt921x_priv *priv, const unsigned char *addr, u16 vid,
ctrl1 = val1 & ~YT921X_FDB_IO1_STATUS_M;
ctrl1 |= YT921X_FDB_IO1_STATUS_STATIC;
- ctrl2 = val2 | YT921X_FDB_IO2_EGR_PORTS(ports_mask);
+ ctrl2 = val2;
+ if (ctrl1 != val1)
+ ctrl2 &= ~YT921X_FDB_IO2_EGR_PORTS_M;
+ ctrl2 |= YT921X_FDB_IO2_EGR_PORTS(ports_mask);
if (ctrl1 == val1 && ctrl2 == val2)
return 0;
--
2.53.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH net] net: dsa: motorcomm: Do not inherit ports from dynamic FDB entries 2026-11-09 15:34 [PATCH net] net: dsa: motorcomm: Do not inherit ports from dynamic FDB entries David Yang @ 2026-11-11 15:38 ` netdev-bot+sashiko 2026-11-11 16:44 ` David Yang 0 siblings, 1 reply; 4+ messages in thread From: netdev-bot+sashiko @ 2026-11-11 15:38 UTC (permalink / raw) To: mmyangfl Cc: netdev, andrew, olteanv, davem, edumazet, kuba, pabeni, linux-kernel Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider. Critical: 0 · High: 1 · Medium: 0 · Low: 0 If you believe none of the issues are legitimate, please reply to each issue of Medium severity or higher and explain why the AI is wrong. If any of them are legitimate and you plan to respin, please let patchwork know by including "pw-bot: cr" as a separate line at the end of your reply (one such reply per series is enough). - [High] This patch replaces the egress port mask in yt921x_fdb_join() (drivers/net/dsa/motorcomm/chip.c) only when the existing hardware… -- net: dsa: motorcomm: Do not inherit ports from dynamic FDB entries This patch changes yt921x_fdb_join() so that when the existing hardware FDB entry is not static, its egress port mask is replaced instead of merged. The goal is to avoid converting hardware-learned ports into a static entry that DSA does not track and can never delete. > diff --git a/drivers/net/dsa/motorcomm/chip.c b/drivers/net/dsa/motorcomm/chip.c > index 33e5fc127244d..150c51f1aa358 100644 > --- a/drivers/net/dsa/motorcomm/chip.c > +++ b/drivers/net/dsa/motorcomm/chip.c > @@ -799,7 +799,10 @@ yt921x_fdb_join(struct yt921x_priv *priv, const unsigned char *addr, u16 vid, > > ctrl1 = val1 & ~YT921X_FDB_IO1_STATUS_M; > ctrl1 |= YT921X_FDB_IO1_STATUS_STATIC; > - ctrl2 = val2 | YT921X_FDB_IO2_EGR_PORTS(ports_mask); > + ctrl2 = val2; > + if (ctrl1 != val1) > + ctrl2 &= ~YT921X_FDB_IO2_EGR_PORTS_M; > + ctrl2 |= YT921X_FDB_IO2_EGR_PORTS(ports_mask); [Severity: High] If the existing entry is already static, this path still ORs the new port into the mask, and it does so for unicast addresses too. Can a static unicast entry still be "silently expanded and can never be deleted" when it moves between ports? One sequence that seems to reach this: bridge fdb add <mac> dev swp0 master static bridge fdb replace <mac> dev swp1 master static In net/bridge/br_fdb.c:fdb_add_entry(), fdb_update_dst() sees that the port changed and returns true. The bridge then calls only fdb_notify(br, fdb, RTM_NEWNEIGH, true). As a result, br_switchdev_fdb_notify() sends SWITCHDEV_FDB_ADD_TO_DEVICE for swp1 and sends no delete for swp0. For user ports, dsa_port_do_fdb_add() in net/dsa/switch.c calls ds->ops->port_fdb_add() directly. That ends up in yt921x_fdb_join(priv, mac, vid, BIT(swp1)). The hardware entry is already STATIC there, so ctrl1 == val1. The new clear of YT921X_FDB_IO2_EGR_PORTS_M is skipped, and EGR_PORTS becomes swp0 | swp1. Unicast frames to <mac> would then go out both ports, while the bridge thinks the address is only on swp1. A later delete on swp1 goes through yt921x_fdb_leave(), which clears only the departing port's bit: ctrl2 = val2 & ~YT921X_FDB_IO2_EGR_PORTS(ports_mask); if (ctrl2 == val2) return 0; if (!(ctrl2 & YT921X_FDB_IO2_EGR_PORTS_M)) { That leaves a static entry pointing at swp0 in hardware. The driver only flushes with flush_static=false, so nothing would remove that entry later. A host FDB entry on the CPU port, installed with assisted_learning_on_cpu_port, looks like it can reach the same path when a replace moves the address to a user port. For comparison, mv88e6xxx_port_db_load_purge() replaces the port vector for static unicast and merges only otherwise: if (state == MV88E6XXX_G1_ATU_DATA_STATE_UC_STATIC) entry.portvec = BIT(port); else entry.portvec |= BIT(port); Hardware-learned entries can only be unicast, because source MACs are never multicast. Would it work to decide based on the address instead, for example by replacing the mask when !is_multicast_ether_addr(addr) and merging only for MDB entries? That would cover both the dynamic case this patch targets and the static port-move case above. > if (ctrl1 == val1 && ctrl2 == val2) > return 0; > -- Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261109153407.1761032-1-mmyangfl%40gmail.com ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net] net: dsa: motorcomm: Do not inherit ports from dynamic FDB entries 2026-11-11 15:38 ` netdev-bot+sashiko @ 2026-11-11 16:44 ` David Yang 2026-11-11 23:19 ` Jakub Kicinski 0 siblings, 1 reply; 4+ messages in thread From: David Yang @ 2026-11-11 16:44 UTC (permalink / raw) To: netdev-bot+sashiko Cc: netdev, andrew, olteanv, davem, edumazet, kuba, pabeni, linux-kernel On Wed, Nov 11, 2026 at 11:38 PM <netdev-bot+sashiko@kernel.org> wrote: > > Thank you for your contribution! Sashiko AI review found 1 potential > issue(s) to consider. > > Critical: 0 · High: 1 · Medium: 0 · Low: 0 > > If you believe none of the issues are legitimate, please reply to each > issue of Medium severity or higher and explain why the AI is wrong. > If any of them are legitimate and you plan to respin, please let patchwork > know by including "pw-bot: cr" as a separate line at the end of your reply > (one such reply per series is enough). > > - [High] This patch replaces the egress port mask in yt921x_fdb_join() > (drivers/net/dsa/motorcomm/chip.c) only when the existing hardware… Looks like a pre-existing issue. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net] net: dsa: motorcomm: Do not inherit ports from dynamic FDB entries 2026-11-11 16:44 ` David Yang @ 2026-11-11 23:19 ` Jakub Kicinski 0 siblings, 0 replies; 4+ messages in thread From: Jakub Kicinski @ 2026-11-11 23:19 UTC (permalink / raw) To: David Yang Cc: netdev-bot+sashiko, netdev, andrew, olteanv, davem, edumazet, pabeni, linux-kernel On Thu, 12 Nov 2026 00:44:53 +0800 David Yang wrote: > On Wed, Nov 11, 2026 at 11:38 PM <netdev-bot+sashiko@kernel.org> wrote: > > > > Thank you for your contribution! Sashiko AI review found 1 potential > > issue(s) to consider. > > > > Critical: 0 · High: 1 · Medium: 0 · Low: 0 > > > > If you believe none of the issues are legitimate, please reply to each > > issue of Medium severity or higher and explain why the AI is wrong. > > If any of them are legitimate and you plan to respin, please let patchwork > > know by including "pw-bot: cr" as a separate line at the end of your reply > > (one such reply per series is enough). > > > > - [High] This patch replaces the egress port mask in yt921x_fdb_join() > > (drivers/net/dsa/motorcomm/chip.c) only when the existing hardware… > > Looks like a pre-existing issue. I think AI is saying that the fix will be in the same place, tho, so we may be better off fixing them both in the same series. Please try, if AI keeps finding more and more issues we can cut it off, but not at v1. ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-11-11 23:19 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-11-09 15:34 [PATCH net] net: dsa: motorcomm: Do not inherit ports from dynamic FDB entries David Yang 2026-11-11 15:38 ` netdev-bot+sashiko 2026-11-11 16:44 ` David Yang 2026-11-11 23:19 ` Jakub Kicinski
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox