On 8/20/25 4:34 PM, Dima Chumak via dev wrote: > After introducing multi-table routing, all local routing entries now > resides in a dedicated table CLS_LOCAL. There is no need to keep a > per-entry flag to distinguish local from non-local entry any longer, so > drop it.
I guess, worth mentioning that this is the case because we're not considering any routes that are not unicast or local, i.e. we're not considering broadcast routes that would also normally reside in the local table. > > Signed-off-by: Dima Chumak <[email protected]> > --- > lib/netdev-dummy.c | 12 ++++++------ > lib/ovs-router.c | 44 +++++++++++++++++++------------------------- > lib/ovs-router.h | 6 ++---- > lib/route-table.c | 3 +-- > 4 files changed, 28 insertions(+), 37 deletions(-) > > diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c > index 5e75012cd57a..b16085bad8e3 100644 > --- a/lib/netdev-dummy.c > +++ b/lib/netdev-dummy.c > @@ -2223,11 +2223,11 @@ netdev_dummy_ip4addr(struct unixctl_conn *conn, int > argc OVS_UNUSED, > > in6_addr_set_mapped_ipv4(&ip6, ip.s_addr); > /* Insert local route entry for the new address. */ > - ovs_router_force_insert(CLS_LOCAL, 0, &ip6, 32 + 96, true, > - argv[1], &in6addr_any, &ip6); > + ovs_router_force_insert(CLS_LOCAL, 0, &ip6, 32 + 96, argv[1], > + &in6addr_any, &ip6); > /* Insert network route entry for the new address. */ > - ovs_router_force_insert(CLS_MAIN, 0, &ip6, plen + 96, false, > - argv[1], &in6addr_any, &ip6); > + ovs_router_force_insert(CLS_MAIN, 0, &ip6, plen + 96, argv[1], > + &in6addr_any, &ip6); > > unixctl_command_reply(conn, "OK"); > } else { > @@ -2260,10 +2260,10 @@ netdev_dummy_ip6addr(struct unixctl_conn *conn, int > argc OVS_UNUSED, > netdev_dummy_add_in6(netdev, &ip6, &mask); > > /* Insert local route entry for the new address. */ > - ovs_router_force_insert(CLS_LOCAL, 0, &ip6, 128, true, argv[1], > + ovs_router_force_insert(CLS_LOCAL, 0, &ip6, 128, argv[1], > &in6addr_any, &ip6); > /* Insert network route entry for the new address. */ > - ovs_router_force_insert(CLS_MAIN, 0, &ip6, plen, false, argv[1], > + ovs_router_force_insert(CLS_MAIN, 0, &ip6, plen, argv[1], > &in6addr_any, &ip6); > > unixctl_command_reply(conn, "OK"); > diff --git a/lib/ovs-router.c b/lib/ovs-router.c > index 91f331847c99..d25e6426afa5 100644 > --- a/lib/ovs-router.c > +++ b/lib/ovs-router.c > @@ -84,7 +84,6 @@ struct ovs_router_entry { > struct in6_addr src_addr; > uint8_t plen; > uint8_t priority; > - bool local; > uint32_t mark; > }; > > @@ -127,7 +126,7 @@ cls_flush(struct classifier *cls, bool flush_all) > > classifier_defer(cls); > CLS_FOR_EACH (rt, cr, cls) { > - if (flush_all || rt->priority == rt->plen || rt->local) { > + if (flush_all || rt->priority == rt->plen) { > rt_entry_delete__(&rt->cr, cls); > } > } > @@ -197,12 +196,7 @@ ovs_router_lookup(uint32_t mark, const struct in6_addr > *ip6_dst, > > cr_src = classifier_lookup(cls_local, OVS_VERSION_MAX, &flow_src, > NULL, NULL); > - if (cr_src) { > - struct ovs_router_entry *p_src = ovs_router_entry_cast(cr_src); > - if (!p_src->local) { > - return false; > - } > - } else { > + if (!cr_src) { > return false; > } > } > @@ -363,7 +357,7 @@ out: > > static int > ovs_router_insert__(uint32_t table, uint32_t mark, uint8_t priority, > - bool local, const struct in6_addr *ip6_dst, > + const struct in6_addr *ip6_dst, > uint8_t plen, const char output_netdev[], > const struct in6_addr *gw, > const struct in6_addr *ip6_src) > @@ -387,7 +381,6 @@ ovs_router_insert__(uint32_t table, uint32_t mark, > uint8_t priority, > p->mark = mark; > p->nw_addr = match.flow.ipv6_dst; > p->plen = plen; > - p->local = local; > p->priority = priority; > > if (ipv6_addr_is_set(ip6_src)) { > @@ -432,13 +425,13 @@ ovs_router_insert__(uint32_t table, uint32_t mark, > uint8_t priority, > > void > ovs_router_insert(uint32_t table, uint32_t mark, const struct in6_addr > *ip_dst, > - uint8_t plen, bool local, const char output_netdev[], > + uint8_t plen, const char output_netdev[], > const struct in6_addr *gw, const struct in6_addr *prefsrc) > { > if (use_system_routing_table) { > - uint8_t priority = local ? plen + 64 : plen; > - ovs_router_insert__(table, mark, priority, local, ip_dst, plen, > - output_netdev, gw, prefsrc); > + uint8_t priority = table == CLS_LOCAL ? plen + 64 : plen; The +64 trick was used to give local routes higher priority. Do we still need that, since the local table is always confidered first? With that do we have any other cases where priority != plen, except for user-added routes? We have quite a few places where we check for user-added routes to, e.g., avoid flushing them or while printing, these checks may be simplified down to just comparing the plen and priority and not looking at the table. Otherwise, we will have a problem if users add routes to the local table. One problem here though is that routes comming from the netdev-dummy are not added as user-routes, but they are also now not marked as local, so they can be flushed, which should not happen. One solution here may be to add them as user routes instead. It may also be easier if we introduce a 'user' flag instead and stop checking priorities and table numbers. Removing the +32 priority for user-added routes would be a behavior change, so I'm not sure if this should be done, but also we'd need to make sure that users can't remove cached routes if we go this way. > + ovs_router_insert__(table, mark, priority, ip_dst, plen, > output_netdev, > + gw, prefsrc); > } > } > > @@ -447,14 +440,14 @@ ovs_router_insert(uint32_t table, uint32_t mark, const > struct in6_addr *ip_dst, > void > ovs_router_force_insert(uint32_t table, uint32_t mark, > const struct in6_addr *ip_dst, > - uint8_t plen, bool local, const char output_netdev[], > + uint8_t plen, const char output_netdev[], > const struct in6_addr *gw, > const struct in6_addr *prefsrc) > { > - uint8_t priority = local ? plen + 64 : plen; > + uint8_t priority = table == CLS_LOCAL ? plen + 64 : plen; Ditto. > > - ovs_router_insert__(table, mark, priority, local, ip_dst, plen, > - output_netdev, gw, prefsrc); > + ovs_router_insert__(table, mark, priority, ip_dst, plen, output_netdev, > gw, > + prefsrc); > } > > static void > @@ -585,8 +578,8 @@ ovs_router_add(struct unixctl_conn *conn, int argc, > in6_addr_set_mapped_ipv4(&src6, src); > } > > - err = ovs_router_insert__(CLS_MAIN, mark, plen + 32, false, &ip6, plen, > - argv[2], &gw6, &src6); > + err = ovs_router_insert__(CLS_MAIN, mark, plen + 32, &ip6, plen, argv[2], > + &gw6, &src6); > if (err) { > unixctl_command_reply_error(conn, "Error while inserting route."); > } else { > @@ -637,7 +630,7 @@ ovs_router_show_json(struct json *json_routes, const > struct classifier *cls, > } > > CLS_FOR_EACH (rt, cr, cls) { > - bool user = rt->priority != rt->plen && !rt->local; > + bool user = rt->priority != rt->plen && table != CLS_LOCAL; Here, if the user adds a route to the local table, the route will not be reported as user route. Previously, such a route would not have the local flag set. > uint8_t plen = rt->plen; > struct json *json, *nh; > > @@ -650,7 +643,8 @@ ovs_router_show_json(struct json *json_routes, const > struct classifier *cls, > > json_object_put(json, "table", json_integer_create(table)); > json_object_put(json, "user", json_boolean_create(user)); > - json_object_put(json, "local", json_boolean_create(rt->local)); > + json_object_put(json, "local", > + json_boolean_create(table == CLS_LOCAL)); > json_object_put(json, "priority", json_integer_create(rt->priority)); > json_object_put(json, "prefix", json_integer_create(plen)); > json_object_put_string(nh, "dev", rt->output_netdev); > @@ -708,7 +702,7 @@ ovs_router_show_text(struct ds *ds, const struct > classifier *cls, > > CLS_FOR_EACH (rt, cr, cls) { > uint8_t plen; > - if (rt->priority == rt->plen || rt->local) { > + if (rt->priority == rt->plen || table == CLS_LOCAL) { Ditto. > ds_put_format(ds, "Cached: "); > } else { > ds_put_format(ds, "User: "); > @@ -730,7 +724,7 @@ ovs_router_show_text(struct ds *ds, const struct > classifier *cls, > } > ds_put_format(ds, " SRC "); > ipv6_format_mapped(&rt->src_addr, ds); > - if (rt->local) { > + if (table == CLS_LOCAL) { It's possible to have non-local routes int the local table. The table itself doesn't really impose any restrictions on types of routes in there and one can add any routes into the local table in the kernel. So, I'm not sure if we can realy get rid of the 'local' flag in the structure. It is not required for the real route lookups as we're just traversing tables one by one, but when we're printing the routing tables, makring non-local routes as local may be confusing. > ds_put_format(ds, " local"); > } > if (!is_standard_table(table) && !show_header) { > @@ -860,7 +854,7 @@ ovs_router_flush(bool flush_all) > > ovs_mutex_lock(&mutex); > CMAP_FOR_EACH (node, cmap_node, &clsmap) { > - cls_flush(&node->cls, flush_all); > + cls_flush(&node->cls, flush_all || node->table == CLS_LOCAL); This will flush user-added routes in the local table. > if (!node->cls.n_rules) { > cmap_remove(&clsmap, &node->cmap_node, hash_int(node->table, 0)); > ovsrcu_postpone(clsmap_node_destroy_cb, node); > diff --git a/lib/ovs-router.h b/lib/ovs-router.h > index c397d01bd66d..6fe3bfa7cafa 100644 > --- a/lib/ovs-router.h > +++ b/lib/ovs-router.h > @@ -40,13 +40,11 @@ void ovs_router_init(void); > bool ovs_router_is_empty(uint32_t table); > bool ovs_router_is_referenced(uint32_t table); > void ovs_router_insert(uint32_t table, uint32_t mark, > - const struct in6_addr *ip_dst, > - uint8_t plen, bool local, > + const struct in6_addr *ip_dst, uint8_t plen, > const char output_netdev[], const struct in6_addr *gw, > const struct in6_addr *prefsrc); > void ovs_router_force_insert(uint32_t table, uint32_t mark, > - const struct in6_addr *ip_dst, > - uint8_t plen, bool local, > + const struct in6_addr *ip_dst, uint8_t plen, > const char output_netdev[], > const struct in6_addr *gw, > const struct in6_addr *prefsrc); > diff --git a/lib/route-table.c b/lib/route-table.c > index 432c646e254c..a38f2aadc7aa 100644 > --- a/lib/route-table.c > +++ b/lib/route-table.c > @@ -760,8 +760,7 @@ route_table_handle_msg(const struct route_table_msg > *change, > ovs_router_insert(table, rd->rta_mark, &rd->rta_dst, > IN6_IS_ADDR_V4MAPPED(&rd->rta_dst) > ? rd->rtm_dst_len + 96 : rd->rtm_dst_len, > - rd->rtn_local, rdnh->ifname, &rdnh->addr, > - &rd->rta_prefsrc); > + rdnh->ifname, &rdnh->addr, &rd->rta_prefsrc); > } > } > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
