From c8f47df6fcd2fa6f44eb381d1eddac48c7c88690 Mon Sep 17 00:00:00 2001 From: Gennaro Cimmino Date: Thu, 17 Sep 2026 19:36:07 +0200 Subject: [PATCH] realtek: dsa: mark L2 entries the bridge is using An L2 unicast entry can be wanted by the bridge and by a route at the same time, and neither knows about the other. A static FDB entry now records itself in the map, so taking a next hop down no longer takes the address away from the bridge that also asked for it. Such an entry stays where it is, as the next hop it already was, with no port of its own and not ageing - which is how the driver writes a next hop it created itself. The other direction is wired here and starts working with the last patch. The FDB delete asks the map before it invalidates an entry, but nothing counts the routes behind a gateway yet, so the branch that keeps such an entry is not reached until they are counted. Keeping it on the route's behalf rests on two things, not one: the map has to say a route wants it, and the entry itself has to still say it is a next hop. The map alone can be stale, because the switch drops rows by ageing and by the per-port flush a bridge asks for and nothing tells the driver; a stale count on its own would turn an ordinary FDB delete into a row that is valid, static, reachable through no port and never ageing. An entry nobody had claimed is zeroed before the bridge or a route takes it, for the same reason: the cell may still hold what its last owner left there, and a stale mark on a row a route has since taken would stop the removal from handing it back. CAM entries are left alone. The CAM is a table of its own with its own numbering, and this driver never places a next hop there, so nothing can be sharing one - the unconditional removal there is already right. No effect on RTL838x or RTL931x, although the handlers run there: nothing marks a next hop on those families, because neither config carries a setup op, which is what both notifiers gate on, so the entry is given back exactly as before. Not exercised on hardware: nothing in the test image puts an FDB entry and a route on one address. Compile-tested on realtek/rtl930x and realtek/rtl839x. Assisted-by: Claude:claude-opus-5 Signed-off-by: Gennaro Cimmino Link: https://github.com/openwrt/openwrt/pull/25231 Signed-off-by: Markus Stockhausen --- .../drivers/net/dsa/rtl83xx/common.c | 18 +++++++++++- .../files-6.18/drivers/net/dsa/rtl83xx/dsa.c | 29 ++++++++++++++++++- 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/target/linux/realtek/files-6.18/drivers/net/dsa/rtl83xx/common.c b/target/linux/realtek/files-6.18/drivers/net/dsa/rtl83xx/common.c index fec56e68559..5d5e282dc2a 100644 --- a/target/linux/realtek/files-6.18/drivers/net/dsa/rtl83xx/common.c +++ b/target/linux/realtek/files-6.18/drivers/net/dsa/rtl83xx/common.c @@ -656,6 +656,16 @@ int rtldsa_l2_nexthop_add(struct rtl838x_switch_priv *priv, struct otto_l3_nexth } /* Found an existing (e->valid is true) or empty entry, make it a nexthop entry */ + if (!nh->l2_installed || nh->l2_id != idx) { + struct rtldsa_l2_uc *m = rtldsa_l2_uc_lookup(priv, idx); + + /* An entry nobody had claimed carries whatever its last + * owner left behind. + */ + if (m && !e.valid) + *m = (struct rtldsa_l2_uc){}; + } + nh->l2_id = idx; nh->l2_seed = seed; if (e.valid) { @@ -690,6 +700,7 @@ int rtldsa_l2_nexthop_add(struct rtl838x_switch_priv *priv, struct otto_l3_nexth int rtldsa_l2_nexthop_del(struct rtl838x_switch_priv *priv, struct otto_l3_nexthop *nh) { struct rtl838x_l2_entry e = {}; + struct rtldsa_l2_uc *m; u32 key = nh->l2_id >> 2; int i = nh->l2_id & 0x3; int idx; @@ -717,7 +728,12 @@ int rtldsa_l2_nexthop_del(struct rtl838x_switch_priv *priv, struct otto_l3_nexth return -ESTALE; } - if (e.is_static) + m = rtldsa_l2_uc_lookup(priv, nh->l2_id); + + /* The bridge put this address here as well, so the entry stays; it + * just stops being a next hop. + */ + if (e.is_static && (!m || !m->fdb_ref)) e.valid = false; e.next_hop = false; /* A route id takes that field on the families that keep one, so what diff --git a/target/linux/realtek/files-6.18/drivers/net/dsa/rtl83xx/dsa.c b/target/linux/realtek/files-6.18/drivers/net/dsa/rtl83xx/dsa.c index c6f9d668eea..8020bbed768 100644 --- a/target/linux/realtek/files-6.18/drivers/net/dsa/rtl83xx/dsa.c +++ b/target/linux/realtek/files-6.18/drivers/net/dsa/rtl83xx/dsa.c @@ -1666,6 +1666,17 @@ static int rtldsa_port_fdb_add(struct dsa_switch *ds, int port, /* Found an existing or empty entry */ if (idx >= 0) { + struct rtldsa_l2_uc *m = rtldsa_l2_uc_lookup(priv, idx); + + if (m) { + /* A slot nobody had claimed carries whatever its last + * owner left behind. + */ + if (!e.valid) + *m = (struct rtldsa_l2_uc){}; + m->fdb_ref = true; + } + rtldsa_setup_l2_uc_entry(&e, port, vid, mac); priv->r->write_l2_entry_using_hash(idx >> 2, idx & 0x3, &e); goto out; @@ -1709,8 +1720,24 @@ static int rtldsa_port_fdb_del(struct dsa_switch *ds, int port, idx = rtldsa_find_l2_hash_entry(priv, seed, true, &e); if (idx >= 0) { + struct rtldsa_l2_uc *m = rtldsa_l2_uc_lookup(priv, idx); + pr_debug("Found entry index %d, key %d and bucket %d\n", idx, idx >> 2, idx & 3); - e.valid = false; + + if (m) + m->fdb_ref = false; + + /* A route still forwarding through this address keeps it, as + * the next hop it already is, reachable through no port of its + * own. + */ + if (m && m->l3_refcount && e.next_hop) { + e.port = priv->r->port_ignore; + e.age = 0; + } else { + e.valid = false; + } + priv->r->write_l2_entry_using_hash(idx >> 2, idx & 0x3, &e); goto out; }