On 2/25/26 6:05 PM, Matteo Perin via dev wrote:
> Even with --disable-system-route set, non-standard system routing rules
> were still being cached at startup via route_table_reset()
> calling ovs_router_rule_add() from rule_handle_msg() in route-table.c.
>
> The use_system_routing_table flag was only checked in ovs_router_insert()
> and ovs_router_lookup_fallback(), but not in ovs_router_rule_add(),
> allowing non-standard system rules to pollute the routing cache.
>
> Fix this by splitting ovs_router_rule_add() into an internal static
> ovs_router_rule_add__() function and a public ovs_router_rule_add()
> wrapper that checks the use_system_routing_table flag before adding
> rules. Internal callers (init_standard_rules, ovs_router_rule_add_cmd)
> use the internal version directly, while external callers like
> route-table.c go through the public API which respects the flag.
>
> Fixes: dc14e92bcc25 ("route-table: Introduce multi-table route lookup.")
> Signed-off-by: Matteo Perin <[email protected]>
> ---
> lib/ovs-router.c | 48 ++++++++++++++++++++++++++++++++----------------
> 1 file changed, 32 insertions(+), 16 deletions(-)
This patch breaks the CI, we should not do that. Every patch in the
set must pass CI individually, otherwise it's hard to bisect when
looking for a caouse of other issues, if the tests are suddenly failing
on an unrelated commit.
>
> diff --git a/lib/ovs-router.c b/lib/ovs-router.c
> index 496690b46..9542da6c2 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);
>
> 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");
> @@ -1221,17 +1225,17 @@ static void
> init_standard_rules(void)
nit: I'd add the thread safety annotation here as well, just for completeness.
> {
> /* 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);
nit: Indentation of the every second line is off by one.
> }
>
> static void
> @@ -1307,10 +1311,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)
nit: Indentation.
> OVS_REQUIRES(mutex)
> {
> struct router_rule *rule = xzalloc(sizeof *rule);
> @@ -1327,6 +1331,18 @@ 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)
> +{
> + if (use_system_routing_table) {
> + 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
