openwrt-mirror/openwrt
1
0 Fork 0
mirror of https://github.com/openwrt/openwrt.git synced 2026-10-13 17:38:48 +04:00
Code Issues Packages Projects Releases Wiki Activity

realtek: dsa: count the routes using an L2 next hop

Browse Source 
Routes through one gateway share a single L2 entry: the second route to
be installed finds the first one already there and takes it as it is.
Nothing recorded that, so the first route to be torn down took the next
hop bit off the entry the others were still forwarding through, and when
the driver had created that entry itself it was invalidated outright and
the row handed back to the switch.

Count them. Taking a next hop claims a reference on its entry, dropping
one releases it, and the entry is only given back when the last is gone
and the bridge is not holding it either.

The reference is keyed on the flag that already says whether the next
hop is in hardware, because the caller runs this again for every route
behind a gateway on every neighbour event, and an unconditional count
would climb for as long as the neighbour keeps changing state. A next
hop that moves to another row - a gateway whose MAC changed - releases
the row it left, after asking that row whether it is still the one the
reference was taken on, because whoever claims a dropped row next counts
itself from zero. When that was the last reference the row is
given back there and then, by the same step a removal uses, because
nothing would ever come looking for it again.

An entry that was invalid when it was claimed has its count zeroed
first: nothing clears the map when the switch ages a row out by itself,
so what is in the cell may belong to a route that is long gone.

The count pins at its ceiling instead of wrapping. Seven bits hold 127
where the vendor SDK gives the same counter, nh_used_cnt, sixteen; an
entry that reached the ceiling is never freed by the driver again, which
leaves a row in use but never hands a live one to somebody else.

Both checks stay in front of the count, the way the removal already
tested the entry before touching it. An entry that is still the one that
was installed but no longer a next hop is left alone, and its reference
is not dropped. Inside the driver only the release clears that bit, and
it runs once the count has reached zero; the switch can also take the
row away on its own, and a row re-learnt under that address afterwards
carries a count nobody will spend - the same staleness the paragraph
above bounds, not a new one.

The search, the count and the write now happen under priv->reg_mutex,
which is what the bridge FDB handlers already hold across the same
read-modify-write, so nothing can come between them. That mutex is also
ctrl->lock, so an L3 caller must not hold it across these two; none
does. The add still runs inside an RCU read-side section it should not,
which it did before this patch as well - the table accessors it already
called sleep.

Functionally tested on a Hasivo S1100W-8XGT-SE (RTL9303), kernel
6.18.52, with CONFIG_NET_DSA_RTL83XX_RTL930X_L3_OFFLOAD=y and
ip_forward=0, so only the hardware can forward. Two routes through one
gateway share one L2 entry; removing either of them leaves it valid and
still a next hop, and the other route keeps forwarding. Removing the
second one takes the next hop bit off it. The row a moved next hop
leaves behind was not exercised: a second MAC for the gateway cannot be
arranged from the test image, so that path is by inspection. That kernel
carries no atomic-sleep or lock debugging, so a clean log says nothing
about the RCU section above.

Not exercised on RTL838x or RTL931x: neither config carries a setup op,
which is what both notifiers gate on, so the routing side never reaches
them. RTL839x does reach it and is untested - no hardware here.

Compile-tested on realtek/rtl930x and realtek/rtl839x.

Assisted-by: Claude:claude-opus-5
Signed-off-by: Gennaro Cimmino <gcimmino@rayonra.net>
Link: https://github.com/openwrt/openwrt/pull/25231
Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>
This commit is contained in:
Gennaro Cimmino authored and Markus Stockhausen committed 2026-09-18 07:36:02 +02:00
1 parent 9b60c14f08
commit f286d9af81
1 file changed
+95 -19
Download Patch File Download Diff File Expand all files Collapse all files
target/linux/realtek/files-6.18/drivers/net/dsa/rtl83xx/common.c
+95 -19
View File
@@ -634,6 +634,85 @@ void rtldsa_packet_cntr_free(struct rtl838x_switch_priv *priv, int idx)
}
}
/* The count pins at its ceiling rather than wrapping: an entry that reached it
* is never freed by the driver again, which leaks a row but never hands a live
* one to somebody else.
*/
static void rtldsa_l2_uc_get(struct rtl838x_switch_priv *priv, int idx)
{
struct rtldsa_l2_uc *m = rtldsa_l2_uc_lookup(priv, idx);
if (!m)
return;
if (m->l3_refcount == RTLDSA_L2_L3_REFCOUNT_MAX) {
dev_warn_once(priv->dev, "L2 entry %d has too many routes to count\n", idx);
return;
}
m->l3_refcount++;
}
static void rtldsa_l2_uc_put(struct rtl838x_switch_priv *priv, int idx)
{
struct rtldsa_l2_uc *m = rtldsa_l2_uc_lookup(priv, idx);
if (!m || !m->l3_refcount || m->l3_refcount == RTLDSA_L2_L3_REFCOUNT_MAX)
return;
m->l3_refcount--;
}
/* Give the row back once nothing forwards through it any more. @e has to be a
* fresh read of the row at nh->l2_id.
*/
static void rtldsa_l2_uc_release_row(struct rtl838x_switch_priv *priv,
struct otto_l3_nexthop *nh,
struct rtl838x_l2_entry *e)
{
struct rtldsa_l2_uc *m = rtldsa_l2_uc_lookup(priv, nh->l2_id);
/* Another route is still forwarding through this entry: it has to stay
* exactly as it is, next hop and route id included.
*/
if (m && m->l3_refcount)
return;
/* 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
* goes back is the relay VID, which the row still carries either way.
*/
e->vid = e->rvid;
priv->r->write_l2_entry_using_hash(nh->l2_id >> 2, nh->l2_id & 0x3, e);
}
/* Release a reference taken on a remembered index. The switch drops rows on
* its own, by ageing and by the per-port flush the bridge asks for, and
* whoever claims one next counts itself from zero: that count is not ours to
* spend. Search on the seed the reference was taken on, because the caller has
* already overwritten the address.
*/
static void rtldsa_l2_uc_put_row(struct rtl838x_switch_priv *priv,
struct otto_l3_nexthop *nh)
{
struct rtl838x_l2_entry e = {};
if (rtldsa_find_l2_hash_entry(priv, nh->l2_seed, true, &e) != nh->l2_id)
return;
if (!e.next_hop)
return;
rtldsa_l2_uc_put(priv, nh->l2_id);
rtldsa_l2_uc_release_row(priv, nh, &e);
}
/* Add an L2 nexthop entry for the L3 routing system / PIE forwarding in the SoC
* Use VID and MAC in rtl838x_l2_entry to identify either a free slot in the L2 hash table
* or mark an existing entry as a nexthop by setting it's nexthop bit
@@ -649,6 +728,11 @@ int rtldsa_l2_nexthop_add(struct rtl838x_switch_priv *priv, struct otto_l3_nexth
pr_debug("%s searching for %08llx vid %d, seed: %016llx\n",
__func__, nh->mac, nh->rvid, seed);
/* The search, the count and the write are one step: anything else may
* claim the entry we settled on in between.
*/
guard(mutex)(&priv->reg_mutex);
idx = rtldsa_find_l2_hash_entry(priv, seed, false, &e);
if (idx < 0) {
pr_err("%s: No more L2 forwarding entries available\n", __func__);
@@ -656,14 +740,19 @@ 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)
rtldsa_l2_uc_put_row(priv, nh);
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.
/* An entry nobody had claimed carries whatever its last owner
* left behind, including a count for a route long gone.
*/
if (m && !e.valid)
*m = (struct rtldsa_l2_uc){};
rtldsa_l2_uc_get(priv, idx);
}
nh->l2_id = idx;
@@ -700,11 +789,12 @@ 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;
guard(mutex)(&priv->reg_mutex);
dev_dbg(priv->dev, "next hop %d sits at key %d, index %d\n", nh->l2_id, key, i);
/* Search on the seed the installer claimed the row on, because the
@@ -712,8 +802,6 @@ int rtldsa_l2_nexthop_del(struct rtl838x_switch_priv *priv, struct otto_l3_nexth
* recorded index answers both questions at once: the row still holds
* the address that was installed, and it is still the same row. A
* negative index means the address has left the bucket altogether.
* Nothing counts the routes sharing a next hop yet, so a sibling taken
* down first can get here.
*/
idx = rtldsa_find_l2_hash_entry(priv, nh->l2_seed, true, &e);
if (idx != nh->l2_id) {
@@ -728,20 +816,8 @@ int rtldsa_l2_nexthop_del(struct rtl838x_switch_priv *priv, struct otto_l3_nexth
return -ESTALE;
}
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
* goes back is the relay VID, which the row still carries either way.
*/
e.vid = e.rvid;
priv->r->write_l2_entry_using_hash(key, i, &e);
rtldsa_l2_uc_put(priv, nh->l2_id);
rtldsa_l2_uc_release_row(priv, nh, &e);
return 0;
}