From b5395bdc8d93ead173f7ebbb5104c917dfe0dbdb Mon Sep 17 00:00:00 2001 From: Gennaro Cimmino Date: Thu, 8 Oct 2026 19:48:07 +0200 Subject: [PATCH] realtek: l3: hold a DMAC entry per gateway MAC On RTL930x, the L2 entry of a next hop names the DMAC entry from which routed frames take their destination MAC (the field that replaces the VID), and the driver wrote that DMAC entry per route, at the route's id. Routes through one gateway share its L2 entry, which keeps naming the id of the route that claimed it first. When that route goes, another one takes its id and writes its own gateway MAC there, and the routes still going through the shared entry leave with that MAC. Hold the DMAC entry per gateway MAC instead, refcounted by the routes through it as the Realtek GPL SDK does (dal_longan_l3.c, _dal_longan_l3_dmacEntry_alloc()), and have the L2 entry name it. An entry that is already a next hop is pointed at it again on every claim, as the SDK writes it on every add, so an entry that stayed a next hop while its DMAC entry was given up does not keep naming it. A route holds at most one DMAC entry, and the table has one for every route id; entry 0 stays unused, as before. An entry whose write fails is not taken, and the route is left as it was. RTL838x and RTL839x still put the route id in that field, as before, and allocate nothing. Tested on a Hasivo S1100W-8XGT-SE (RTL9303) with a kprobe on otto_table_write_bytes() logging the DMAC writes. 10.99.80.0/24 (A) and 10.99.81.0/24 (B) went through one gateway; A was deleted and 10.99.82.0/24 (C) added through another. On main, C took A's id and wrote its own gateway MAC into the DMAC entry that the shared L2 entry named, and echo requests routed through B fell from 8/8 to 0/8 at their gateway. With this change that gateway's MAC was written once, to entry 1, which the L2 entry named; C's gateway got entry 2, and B still delivered 8/8. Assisted-by: Claude:claude-opus-5-5 Signed-off-by: Gennaro Cimmino Link: https://github.com/openwrt/openwrt/pull/25736 Signed-off-by: Markus Stockhausen --- .../files-6.18/drivers/net/dsa/rtl83xx/l2.c | 21 +++- .../files-6.18/drivers/net/dsa/rtl83xx/l3.c | 95 +++++++++++++++++-- .../files-6.18/drivers/net/dsa/rtl83xx/l3.h | 8 +- 3 files changed, 107 insertions(+), 17 deletions(-) diff --git a/target/linux/realtek/files-6.18/drivers/net/dsa/rtl83xx/l2.c b/target/linux/realtek/files-6.18/drivers/net/dsa/rtl83xx/l2.c index eb228b04a1b..23d984cafbe 100644 --- a/target/linux/realtek/files-6.18/drivers/net/dsa/rtl83xx/l2.c +++ b/target/linux/realtek/files-6.18/drivers/net/dsa/rtl83xx/l2.c @@ -106,8 +106,9 @@ static void otto_l2_uc_release_row(struct rtl838x_switch_priv *priv, 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. + /* A DMAC entry or 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; @@ -199,9 +200,19 @@ int otto_l2_nexthop_add(struct rtl838x_switch_priv *priv, struct otto_l3_nexthop nh->port = e.port; nh->rvid = e.rvid; nh->dev_id = e.stack_dev; - /* If the entry is already a valid next hop entry, don't change it */ - if (e.next_hop) + /* If the entry is already a valid next hop entry, don't change it, + * but for the DMAC entry it names where the L3 tables hold one + * per gateway MAC, the families a route traps on: every route + * through the entry holds that one, and the entry may still name + * one given up while it stayed a next hop. + */ + if (e.next_hop) { + if (!require_existing || e.nh_route_id == nh->dmac_id) + return 0; + e.nh_route_id = nh->dmac_id; + priv->r->write_l2_entry_using_hash(idx >> 2, idx & 0x3, &e); return 0; + } } else { memset(&e, 0, sizeof(e)); e.type = L2_UNICAST; @@ -212,7 +223,7 @@ int otto_l2_nexthop_add(struct rtl838x_switch_priv *priv, struct otto_l3_nexthop u64_to_ether_addr(nh->mac, &e.mac[0]); } e.next_hop = true; - e.nh_route_id = nh->id; /* NH route ID takes place of VID */ + e.nh_route_id = nh->dmac_id; /* DMAC entry or route ID takes place of VID */ e.nh_vlan_target = false; priv->r->write_l2_entry_using_hash(idx >> 2, idx & 0x3, &e); diff --git a/target/linux/realtek/files-6.18/drivers/net/dsa/rtl83xx/l3.c b/target/linux/realtek/files-6.18/drivers/net/dsa/rtl83xx/l3.c index cdb01f753e9..f8e0a30975b 100644 --- a/target/linux/realtek/files-6.18/drivers/net/dsa/rtl83xx/l3.c +++ b/target/linux/realtek/files-6.18/drivers/net/dsa/rtl83xx/l3.c @@ -254,12 +254,12 @@ static u64 otto_l3_930x_get_egress_mac(struct otto_l3_ctrl *ctrl, u32 idx) /* Set the Destination-MAC of a route or the Source MAC of an L3 egress interface * in the SoC's L3_EGR_INTF_MAC table. Indexes 0-2047 are DMACs, 2048+ are SMACs */ -static void otto_l3_930x_set_egress_mac(struct otto_l3_ctrl *ctrl, u32 idx, u64 mac) +static int otto_l3_930x_set_egress_mac(struct otto_l3_ctrl *ctrl, u32 idx, u64 mac) { u32 data[2] = { mac >> 32, mac }; dev_dbg(ctrl->dev, "setting index %d to %016llx\n", idx, mac); - otto_table_write(RTL9300_TBL_L3_EGR_INTF_MAC, idx, &data); + return otto_table_write(RTL9300_TBL_L3_EGR_INTF_MAC, idx, &data); } /* Decode a host route entry fetched into @data. Returns false unless it is a @@ -1549,6 +1549,60 @@ static void otto_l3_route_rewrite(struct otto_l3_ctrl *ctrl, struct otto_l3_rout } } +/* Routes through one gateway share its L2 entry, and that names a single + * DMAC entry, whichever route claimed it first. So the entry is held per MAC + * by every route through it, the way the SDK holds it + * (_dal_longan_l3_dmacEntry_alloc()), not per route: a route that goes leaves + * it to the others. A route holds one at most, and there are no more routes + * than entries. Entry 0 stays unused, as it did while route 0, which is never + * allocated, named it. + */ +static int otto_l3_dmac_get(struct otto_l3_ctrl *ctrl, u64 mac) +{ + int i, unused = -1, err; + + mutex_lock(ctrl->lock); + + for (i = 1; i < MAX_DMACS; i++) { + if (ctrl->dmac_refs[i] && ctrl->dmacs[i] == mac) + break; + if (unused < 0 && !ctrl->dmac_refs[i]) + unused = i; + } + + if (i == MAX_DMACS) { + i = unused; + if (i < 0) { + i = -ENOSPC; + goto out; + } + + /* An entry whose write failed stays free */ + err = ctrl->cfg->set_egress_mac(ctrl, i, mac); + if (err) { + i = err; + goto out; + } + ctrl->dmacs[i] = mac; + } + + ctrl->dmac_refs[i]++; +out: + mutex_unlock(ctrl->lock); + + return i; +} + +static void otto_l3_dmac_put(struct otto_l3_ctrl *ctrl, int idx) +{ + if (!ctrl->cfg->set_egress_mac || idx < 0) + return; + + mutex_lock(ctrl->lock); + ctrl->dmac_refs[idx]--; + mutex_unlock(ctrl->lock); +} + static void otto_l3_route_update_hw(struct otto_l3_ctrl *ctrl, struct otto_l3_route *r, u64 mac) { @@ -1556,8 +1610,10 @@ static void otto_l3_route_update_hw(struct otto_l3_ctrl *ctrl, struct otto_l3_ro bool require_existing = ctrl->cfg->use_l3_tables; char dst[INET6_ADDRSTRLEN + sizeof("/128")]; bool trapped = r->attr.action == ROUTE_ACT_TRAP2CPU; + int old_dmac = r->nh.dmac_id; bool first = !r->nh.mac; bool no_port, trap; + int dmac = r->id; dev_dbg(ctrl->dev, "setting up fwding: gw %pI6c, mac %016llx\n", &r->gw_ip, mac); @@ -1578,18 +1634,26 @@ static void otto_l3_route_update_hw(struct otto_l3_ctrl *ctrl, struct otto_l3_ro otto_l3_route_rewrite(ctrl, r); } + if (ctrl->cfg->set_egress_mac) { + dmac = otto_l3_dmac_get(ctrl, mac); + if (dmac < 0) { + dev_err(ctrl->dev, "no DMAC entry for %016llx: %d\n", mac, dmac); + return; + } + } + r->nh.mac = r->nh.gw = mac; r->nh.port = priv->r->port_ignore; r->nh.id = r->id; - - /* Do we need to explicitly add a DMAC entry with the route's nh index? */ - if (ctrl->cfg->set_egress_mac) - ctrl->cfg->set_egress_mac(ctrl, r->id, mac); + r->nh.dmac_id = dmac; /* Update ROUTING table: map gateway-mac and switch-mac id to route id */ if (!otto_l2_nexthop_add(priv, &r->nh, require_existing)) r->nh.l2_installed = true; + /* The route names its gateway's entry now, through its L2 entry */ + otto_l3_dmac_put(ctrl, old_dmac); + /* A next hop with no port delivers the frame twice, so where the * route entry can trap, let the CPU route it alone until an update * brings one, the way otto_l3_fib_add_v4() treats a host route @@ -1983,11 +2047,13 @@ static void otto_l3_route_teardown(struct otto_l3_ctrl *ctrl, struct otto_l3_rou /* Once the rows are not where the list says, after a failed move or a * lookup that timed out, the row of a prefix route may still be in - * hardware and route through its egress interface: it keeps that for - * good. + * hardware and route through its egress interface and its gateway's + * DMAC entry: it keeps them for good. */ - if (r->is_host_route || !ctrl->prefix_rows_stale) + if (r->is_host_route || !ctrl->prefix_rows_stale) { otto_l3_route_put_intf(ctrl, r); + otto_l3_dmac_put(ctrl, r->nh.dmac_id); + } otto_l3_route_free(ctrl, r); } @@ -2019,7 +2085,7 @@ static struct otto_l3_route *otto_l3_route_alloc(struct otto_l3_ctrl *ctrl, } /* We require a unique route ID irrespective of whether it is a prefix or host - * route (on RTL93xx) as we use this ID to associate a DMAC and next-hop entry + * route (on RTL93xx) as we use this ID to associate a next-hop entry */ r->id = host ? idx + MAX_ROUTES : idx; r->row = -1; /* no row until placed; a host route never has one */ @@ -2027,6 +2093,7 @@ static struct otto_l3_route *otto_l3_route_alloc(struct otto_l3_ctrl *ctrl, r->pr.id = -1; /* We still need to allocate a rule in HW */ r->pr.packet_cntr = -1; r->nh.if_id = -1; + r->nh.dmac_id = -1; r->is_host_route = host; INIT_LIST_HEAD(&r->srcs); @@ -3945,6 +4012,14 @@ int otto_l3_probe(struct device *dev, struct rtl838x_switch_priv *priv) return dev_err_probe(dev, -EINVAL, "No compatible configuration found\n"); ctrl->cfg = match->data; + if (ctrl->cfg->set_egress_mac) { + ctrl->dmacs = devm_kcalloc(dev, MAX_DMACS, sizeof(*ctrl->dmacs), GFP_KERNEL); + ctrl->dmac_refs = devm_kcalloc(dev, MAX_DMACS, sizeof(*ctrl->dmac_refs), + GFP_KERNEL); + if (!ctrl->dmacs || !ctrl->dmac_refs) + return -ENOMEM; + } + if (ctrl->cfg->setup) { err = ctrl->cfg->setup(ctrl); if (err) diff --git a/target/linux/realtek/files-6.18/drivers/net/dsa/rtl83xx/l3.h b/target/linux/realtek/files-6.18/drivers/net/dsa/rtl83xx/l3.h index da4d8a9d036..bd58c0f4157 100644 --- a/target/linux/realtek/files-6.18/drivers/net/dsa/rtl83xx/l3.h +++ b/target/linux/realtek/files-6.18/drivers/net/dsa/rtl83xx/l3.h @@ -13,6 +13,7 @@ struct fib6_info; #define MAX_SMACS 64 +#define MAX_DMACS 2048 #define MAX_ROUTER_MACS 64 #define MAX_HOST_ROUTES 1536 #define MAX_ROUTES 512 @@ -66,7 +67,7 @@ struct otto_l3_route_attr { }; struct otto_l3_nexthop { - u16 id; /* ID: L3_NEXT_HOP table-index or route-index set in L2_NEXT_HOP */ + u16 id; /* ID: L3_NEXT_HOP table-index or route-index */ u32 dev_id; u16 port; u16 rvid; /* Relay VID/FID for the L2 table entry */ @@ -76,6 +77,7 @@ struct otto_l3_nexthop { u64 l2_seed; /* Seed the entry at l2_id was claimed on */ u64 gw; /* The gateway MAC address packets are forwarded to */ int if_id; /* Interface (into L3_EGR_INTF_IDX) */ + int dmac_id; /* Gateway MAC entry the L2 entry names, or route id */ bool l2_installed; /* Entry written to the L2 table */ }; @@ -112,7 +114,7 @@ struct otto_l3_config { void (*get_egress_intf)(struct otto_l3_ctrl *ctrl, int idx, struct otto_l3_intf *intf); void (*set_egress_intf)(struct otto_l3_ctrl *ctrl, int idx, struct otto_l3_intf *intf); u64 (*get_egress_mac)(struct otto_l3_ctrl *ctrl, u32 idx); - void (*set_egress_mac)(struct otto_l3_ctrl *ctrl, u32 idx, u64 mac); + int (*set_egress_mac)(struct otto_l3_ctrl *ctrl, u32 idx, u64 mac); void (*host_route_write)(struct otto_l3_ctrl *ctrl, int idx, struct otto_l3_route *rt); void (*get_router_mac)(struct otto_l3_ctrl *ctrl, u32 idx, struct otto_l3_router_mac *m); void (*set_router_mac)(struct otto_l3_ctrl *ctrl, u32 idx, struct otto_l3_router_mac *m); @@ -142,6 +144,8 @@ struct otto_l3_ctrl { unsigned long host_route_use_bm[MAX_HOST_ROUTES / 32]; struct otto_l3_intf interfaces[MAX_SMACS]; unsigned int intf_refs[MAX_SMACS]; /* routes holding each */ + u64 *dmacs; /* gateway MAC of each DMAC entry */ + unsigned int *dmac_refs; /* routes holding each */ DECLARE_BITMAP(router_mac_bm, MAX_ROUTER_MACS); /* entries written here */ bool prefix_rows_stale; /* a move failed, the rows are not where we say */ bool v4_fwd_off; /* policy rules keep IPv4 forwarding in software */