From b541e7bf8f0b20f3ce224546d3e68e3fa8b97974 Mon Sep 17 00:00:00 2001 From: Gennaro Cimmino Date: Mon, 5 Oct 2026 20:25:24 +0200 Subject: [PATCH] realtek: l3: drop the route hash table The rhltable keyed on the gateway had two lookups. In otto_l3_nexthop_update() it only let a gateway that no route has leave early; for one that some route has, the function walked routes_list right after. otto_l3_fib_del_v4() looked the route up by gateway, though otto_l3_route_find() had already found it by table and destination a few lines earlier. Both run on the single-threaded queue that already walks the list without a lock, and the tables are small, so the list serves both. The table goes, with its init, destroy, insert and remove, and the insert failure path; the comments and log lines that spoke of it are reworded. The handling stays the same: a neighbour update still returns -ENOENT for a gateway that no route has, and a delete still tears the route of that destination down only if it is via that gateway. Of the two errors for a route not found on delete, by gateway and by destination, the second one remains. Tested on a Hasivo S1100W-8XGT-SE (RTL9303), before and after: a route via a host forwards, traps once the host's neighbour entry is deleted and forwards again once it is resolved, and its delete leaves no row. The bench rounds on trap rows and on source-specific IPv6 routes pass 46 of 46 and 20 of 20, as before. Assisted-by: Claude:claude-opus-5-5 Signed-off-by: Gennaro Cimmino Link: https://github.com/openwrt/openwrt/pull/25643 Signed-off-by: Markus Stockhausen --- .../files-6.18/drivers/net/dsa/rtl83xx/l3.c | 101 +++++------------- .../files-6.18/drivers/net/dsa/rtl83xx/l3.h | 4 +- 2 files changed, 25 insertions(+), 80 deletions(-) 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 6022b6d5b87..97612bb94bd 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 @@ -66,7 +66,6 @@ #include #include #include -#include #include #include #include @@ -114,12 +113,6 @@ #define MAX_ROUTER_MACS 64 #define L3_EGRESS_DMACS 2048 -static const struct rhashtable_params otto_l3_route_ht_params = { - .key_len = sizeof(struct in6_addr), - .key_offset = offsetof(struct otto_l3_route, gw_ip), - .head_offset = offsetof(struct otto_l3_route, linkage), -}; - struct otto_l3_net_event_work { struct work_struct work; struct otto_l3_ctrl *ctrl; @@ -1440,29 +1433,18 @@ static int otto_l3_nexthop_update(struct otto_l3_ctrl *ctrl, u8 type, int ifinde const struct in6_addr *gw, u64 mac, bool valid) { struct otto_l3_route *r; - bool known; - - /* The lookup runs in the section the rhashtable asks for and ends - * there: on a kernel without preemptible RCU that section is - * preempt_disable(), and every table op below it sleeps on a mutex. - * Only whether the gateway is known leaves that section, so no - * rhashtable pointer outlives it. The routes it would have walked are - * the ones on the driver's own list with that gateway, and the work - * queue that runs this is single threaded, which is what lets the rest - * of the driver walk that list with no lock. - */ - rcu_read_lock(); - known = rhltable_lookup(&ctrl->routes, gw, otto_l3_route_ht_params); - rcu_read_unlock(); - if (!known) - return -ENOENT; + bool known = false; list_for_each_entry(r, &ctrl->routes_list, list) { + if (!ipv6_addr_equal(&r->gw_ip, gw)) + continue; + + known = true; + /* An IPv4 gateway written v4-mapped is a valid IPv6 one, so - * the key alone does not say whose route this is. + * the address alone does not say whose route this is. */ - if (r->attr.type != type || r->gw_ifindex != ifindex || - !ipv6_addr_equal(&r->gw_ip, gw)) + if (r->attr.type != type || r->gw_ifindex != ifindex) continue; if (valid) @@ -1471,6 +1453,9 @@ static int otto_l3_nexthop_update(struct otto_l3_ctrl *ctrl, u8 type, int ifinde otto_l3_route_trap_hw(ctrl, r); } + if (!known) + return -ENOENT; + /* An address of one family can be written as an address of the other, * so a neighbour of the wrong family reaches this far without having * placed anything, and has nothing to report. @@ -1478,10 +1463,10 @@ static int otto_l3_nexthop_update(struct otto_l3_ctrl *ctrl, u8 type, int ifinde if (type != ROUTE_TYPE_IP6UC) return 0; - /* Reporting the offload allocates and can send a netlink message, so it - * waits until the lookup above is over. It takes no lock of its own, - * so unlike the FIB work this path needs no rtnl. The FIB notifier is - * registered on init_net, which is where every route here comes from. + /* Reporting the offload allocates and can send a netlink message. It + * takes no lock of its own, so unlike the FIB work this path needs no + * rtnl. The FIB notifier is registered on init_net, which is where + * every route here comes from. */ list_for_each_entry(r, &ctrl->routes_list, list) { bool trap; @@ -1575,9 +1560,6 @@ static bool otto_l3_route_is_at(struct otto_l3_ctrl *ctrl, int id, struct otto_l return false; } -/* The routes are hashed on the gateway, which is the field a replace - * changes, so a lookup by destination needs a list of its own. - */ static struct otto_l3_route *otto_l3_route_find(struct otto_l3_ctrl *ctrl, u32 tb_id, u8 type, u32 dst_ip, const struct in6_addr *dst_ip6, int prefix_len) @@ -1616,9 +1598,6 @@ static void otto_l3_route_free(struct otto_l3_ctrl *ctrl, struct otto_l3_route * { struct otto_l3_route_src *s, *tmp; - if (rhltable_remove(&ctrl->routes, &r->linkage, otto_l3_route_ht_params)) - dev_warn(ctrl->dev, "Could not remove route\n"); - list_for_each_entry_safe(s, tmp, &r->srcs, list) kfree(s); @@ -1729,7 +1708,7 @@ static struct otto_l3_route *otto_l3_route_alloc(struct otto_l3_ctrl *ctrl, unsigned long *use_bm = host ? ctrl->host_route_use_bm : ctrl->route_use_bm; int size = host ? MAX_HOST_ROUTES : MAX_ROUTES; struct otto_l3_route *r; - int idx, err; + int idx; mutex_lock(ctrl->lock); @@ -1759,14 +1738,6 @@ static struct otto_l3_route *otto_l3_route_alloc(struct otto_l3_ctrl *ctrl, r->is_host_route = host; INIT_LIST_HEAD(&r->srcs); - err = rhltable_insert(&ctrl->routes, &r->linkage, otto_l3_route_ht_params); - if (err) { - dev_err(ctrl->dev, "Could not insert new rule\n"); - mutex_unlock(ctrl->lock); - kfree(r); - return NULL; - } - list_add_tail(&r->list, &ctrl->routes_list); set_bit(idx, use_bm); @@ -1983,9 +1954,9 @@ static int otto_l3_fib_add_v4(struct otto_l3_ctrl *ctrl, struct fib_entry_notifi ROUTE_HOST : ROUTE_PREFIX); if (route) - dev_info(ctrl->dev, "route hashtable extended for gw %pI4\n", &nh->fib_nh_gw4); + dev_info(ctrl->dev, "route allocated for gw %pI4\n", &nh->fib_nh_gw4); else { - dev_err(ctrl->dev, "could not extend route hashtable for gw %pI4\n", + dev_err(ctrl->dev, "could not allocate a route for gw %pI4\n", &nh->fib_nh_gw4); return -ENOSPC; } @@ -2062,9 +2033,7 @@ out_free_rt: static int otto_l3_fib_del_v4(struct otto_l3_ctrl *ctrl, struct fib_entry_notifier_info *info) { - struct rhlist_head *tmp, *list; struct otto_l3_route *route; - bool found = false; struct in6_addr gw; struct fib_nh *nh; @@ -2085,7 +2054,7 @@ static int otto_l3_fib_del_v4(struct otto_l3_ctrl *ctrl, struct fib_entry_notifi if (otto_l3_fib_check_v4(ctrl, info, FIB_EVENT_ENTRY_DEL)) return 0; - /* The entry that traps for a route left out is keyed on no gateway, + /* The entry that traps for a route left out carries no gateway, * whatever gateway the route itself names */ route = otto_l3_route_find(ctrl, info->tb_id, ROUTE_TYPE_IP4UC, info->dst, NULL, @@ -2098,29 +2067,13 @@ static int otto_l3_fib_del_v4(struct otto_l3_ctrl *ctrl, struct fib_entry_notifi ipv6_addr_set_v4mapped(nh->fib_nh_gw4, &gw); - rcu_read_lock(); - list = rhltable_lookup(&ctrl->routes, &gw, otto_l3_route_ht_params); - if (!list) { - rcu_read_unlock(); - dev_err(ctrl->dev, "no such gateway: %pI4\n", &nh->fib_nh_gw4); - return -ENOENT; - } - rhl_for_each_entry_rcu(route, tmp, list, linkage) { - if (route->attr.type == ROUTE_TYPE_IP4UC && - route->dst_ip == info->dst && route->prefix_len == info->dst_len) { - dev_info(ctrl->dev, "found a route with id %d, nh-id %d\n", - route->id, route->nh.id); - found = true; - break; - } - } - rcu_read_unlock(); - - if (!found) { + if (!route || !ipv6_addr_equal(&route->gw_ip, &gw)) { dev_err(ctrl->dev, "no route %pI4/%d via %pI4\n", &info->dst, info->dst_len, &nh->fib_nh_gw4); return -ENOENT; } + dev_info(ctrl->dev, "found a route with id %d, nh-id %d\n", + route->id, route->nh.id); otto_l3_route_teardown(ctrl, route); @@ -2477,7 +2430,7 @@ static int otto_l3_fib_add_v6(struct otto_l3_ctrl *ctrl, struct fib6_entry_notif route = otto_l3_route_alloc(ctrl, gw, ROUTE_PREFIX); if (!route) { - dev_err(ctrl->dev, "could not extend route hashtable for gw %pI6c\n", gw); + dev_err(ctrl->dev, "could not allocate a route for gw %pI6c\n", gw); goto out_failed; } @@ -3670,14 +3623,12 @@ void otto_l3_remove(struct rtl838x_switch_priv *priv) flush_workqueue(priv->wq); /* Nothing takes a route out now, and a FIB entry one still names would - * be kept alive by it. Destroying the hash table also waits for the - * resize work it runs on its own. + * be kept alive by it. */ rtnl_lock(); list_for_each_entry_safe(r, tmp, &ctrl->routes_list, list) otto_l3_route_free(ctrl, r); rtnl_unlock(); - rhltable_destroy(&ctrl->routes); } struct otto_l3_flush_work { @@ -3781,11 +3732,7 @@ int otto_l3_probe(struct device *dev, struct rtl838x_switch_priv *priv) return dev_err_probe(dev, err, "device specific L3 setup failed\n"); } - /* Initialize hash table for L3 routing */ INIT_LIST_HEAD(&ctrl->routes_list); - err = rhltable_init(&ctrl->routes, &otto_l3_route_ht_params); - if (err) - return dev_err_probe(dev, err, "could not set up the route hash table\n"); /* Before the notifiers, so no destination is dropped in the window * where the tables are live and the routes have not arrived yet. 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 a562252a2f7..ed3570834c5 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 @@ -92,8 +92,7 @@ struct otto_l3_route { unsigned int members; /* FIB entries a trap row stands for */ struct list_head srcs; /* source-specific routes a trap row stands for */ bool srcs_incomplete; /* a source could not be tracked */ - struct rhlist_head linkage; - struct list_head list; /* all routes, for lookups by destination */ + struct list_head list; /* all routes, for every lookup */ u32 tb_id; /* routing table the route came from */ u16 switch_mac_id; /* Index into switch's own MACs, RTL839X only */ struct otto_l3_nexthop nh; @@ -135,7 +134,6 @@ struct otto_l3_ctrl { struct delayed_work resync_work; unsigned int resync_delay; bool resync_wanted; - struct rhltable routes; struct list_head routes_list; unsigned long route_use_bm[MAX_ROUTES / 32]; unsigned long host_route_use_bm[MAX_HOST_ROUTES / 32];