On 2/18/26 6:48 PM, Matteo Perin via dev wrote: > The --disable-system-route option was not fully preventing system routing > rules from being cached at startup. When route_table_reset() was called, > it would query all kernel routing rules via RTM_GETRULE and cache them > with user=false, regardless of the use_system_routing_table flag. > > This also caused some unit tests to fail if non-standard system routing > rules were present in the system and would appear in the cache. > > An internal ovs_router_rule_add__() function that unconditionally > adds routing rules was added, following the same pattern used > by ovs_router_insert(). > > ovs_router_rule_add() was modified to filter system rules based on > configuration and table type: > - Always allows user-configured rules (user=true) > - Always allows standard table rules (local/main/default) even when > system routing is disabled > - Only allows non-standard system rules when use_system_routing_table > is true > > Updated internal callers (init_standard_rules() and > ovs_router_rule_add_cmd()) to use ovs_router_rule_add__() directly, > ensuring standard rules and user-configured rules bypass the filter. >
This needs a 'Fixes' tag pointing to the commit that introduced the issue. > Signed-off-by: Matteo Perin <[email protected]> > --- > lib/ovs-router.c | 53 +++++++++++++++++++++++++++++++++--------------- > 1 file changed, 37 insertions(+), 16 deletions(-) > > diff --git a/lib/ovs-router.c b/lib/ovs-router.c > index 496690b46..89e5d80b7 100644 > --- a/lib/ovs-router.c > +++ b/lib/ovs-router.c > @@ -91,6 +91,10 @@ struct ovs_router_entry { > }; > > static void rt_entry_delete__(const struct cls_rule *, struct classifier *); > +static void ovs_router_rule_add__(uint32_t prio, bool invert, bool user, > + uint8_t src_len, const struct in6_addr > *from, > + uint32_t lookup_table, bool ipv4) > + OVS_REQUIRES(mutex); > > static struct classifier * > cls_find(uint32_t table) > @@ -1035,7 +1039,7 @@ ovs_router_rule_add_cmd(struct unixctl_conn *conn, int > argc OVS_UNUSED, > prev_prio = rule->prio; > } > } > - ovs_router_rule_add(prio, invert, true, src_len, &from, table, ipv4); > + ovs_router_rule_add__(prio, invert, true, src_len, &from, table, ipv4); > ovs_mutex_unlock(&mutex); > > unixctl_command_reply(conn, "OK"); > @@ -1219,19 +1223,20 @@ ovs_router_flush(bool flush_all) > > static void > init_standard_rules(void) > + OVS_REQUIRES(mutex) > { > /* Add default rules using same priorities as Linux kernel does. */ > - ovs_router_rule_add(0, false, false, 0, > - &in6addr_v4mapped_any, CLS_LOCAL, true); > - ovs_router_rule_add(0x7FFE, false, false, 0, > - &in6addr_v4mapped_any, CLS_MAIN, true); > - ovs_router_rule_add(0x7FFF, false, false, 0, > - &in6addr_v4mapped_any, CLS_DEFAULT, true); > - > - ovs_router_rule_add(0, false, false, 0, > - &in6addr_any, CLS_LOCAL, false); > - ovs_router_rule_add(0x7FFE, false, false, 0, > - &in6addr_any, CLS_MAIN, false); > + ovs_router_rule_add__(0, false, false, 0, > + &in6addr_v4mapped_any, CLS_LOCAL, true); > + ovs_router_rule_add__(0x7FFE, false, false, 0, > + &in6addr_v4mapped_any, CLS_MAIN, true); > + ovs_router_rule_add__(0x7FFF, false, false, 0, > + &in6addr_v4mapped_any, CLS_DEFAULT, true); > + > + ovs_router_rule_add__(0, false, false, 0, > + &in6addr_any, CLS_LOCAL, false); > + ovs_router_rule_add__(0x7FFE, false, false, 0, > + &in6addr_any, CLS_MAIN, false); > } > > static void > @@ -1307,10 +1312,10 @@ rule_pvec_prio(uint32_t prio) > } > } > > -void > -ovs_router_rule_add(uint32_t prio, bool invert, bool user, uint8_t src_len, > - const struct in6_addr *from, uint32_t lookup_table, > - bool ipv4) > +static void > +ovs_router_rule_add__(uint32_t prio, bool invert, bool user, uint8_t src_len, > + const struct in6_addr *from, uint32_t lookup_table, > + bool ipv4) > OVS_REQUIRES(mutex) > { > struct router_rule *rule = xzalloc(sizeof *rule); > @@ -1327,6 +1332,22 @@ ovs_router_rule_add(uint32_t prio, bool invert, bool > user, uint8_t src_len, > pvector_publish(&rules); > } > > +void > +ovs_router_rule_add(uint32_t prio, bool invert, bool user, uint8_t src_len, > + const struct in6_addr *from, uint32_t lookup_table, > + bool ipv4) > + OVS_REQUIRES(mutex) Hmm, I don't think the caller of this function takes this lock, doesn't it? We should probably take it in this fucntion around the ovs_router_rule_add__() call. May need to be a separate patch though. > +{ > + /* Always add user-configured rules. > + * For system (non-user) rules, only add if disable-system-route is false > + * or it is a standard table rule (local, main, default). > + */ > + if (user || use_system_routing_table || is_standard_table(lookup_table)) > { This function is never called with user==true, why do we need this argument here at all? Also, why the exception for the standard tables? Is it because the flush removes them? I'd say the flush removing them is the problem then and we should add them back right after the flush in case it's not the flush_all. E.g. by calling the init function inside the ovs_router_rules_flush(). > + ovs_router_rule_add__(prio, invert, user, src_len, from, > lookup_table, > + ipv4); > + } > +} > + > int > ovs_router_rule_del(uint32_t prio, bool invert, uint8_t src_len, > const struct in6_addr *from, uint32_t lookup_table, _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
