From f65d4e6c4ea6d06aee995d46d7c9c39452859da8 Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Mon, 13 Jul 2026 19:23:21 +0100 Subject: [PATCH] network: compare all multipath route nexthops The multipath route comparator looks up every nexthop in the first route and hence compares each entry with itself. Compare both ordered nexthop sets instead, including weights, as the hash function does. Follow-up for 47420573a778e83f2bddd536cf185cd5829e8ba9 --- src/network/networkd-route-nexthop.c | 24 +++++++++++----- src/network/test-networkd-conf.c | 42 ++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 7 deletions(-) diff --git a/src/network/networkd-route-nexthop.c b/src/network/networkd-route-nexthop.c index bebb401b33..642cc508f0 100644 --- a/src/network/networkd-route-nexthop.c +++ b/src/network/networkd-route-nexthop.c @@ -185,9 +185,16 @@ void route_nexthops_hash_func(const Route *route, struct siphash *state) { return; default: { + uint64_t xor_hash = 0; RouteNextHop *nh; - ORDERED_SET_FOREACH(nh, route->nexthops) - route_nexthop_hash_func(nh, state); + ORDERED_SET_FOREACH(nh, route->nexthops) { + struct siphash nh_state = *ASSERT_PTR(state); + + route_nexthop_hash_func(nh, &nh_state); + xor_hash ^= siphash24_finalize(&nh_state); + } + + siphash24_compress_typesafe(xor_hash, state); }} } @@ -211,12 +218,15 @@ int route_nexthops_compare_func(const Route *a, const Route *b) { return route_nexthop_compare_func_full(&a->nexthop, &b->nexthop, /* with_weight= */ false); default: { + r = CMP(ordered_set_size(a->nexthops), ordered_set_size(b->nexthops)); + if (r != 0) + return r; + RouteNextHop *nh; - ORDERED_SET_FOREACH(nh, a->nexthops) { - r = CMP(nh, (RouteNextHop*) ordered_set_get(a->nexthops, nh)); - if (r != 0) - return r; - } + ORDERED_SET_FOREACH(nh, a->nexthops) + if (!ordered_set_get(b->nexthops, nh)) + return 1; + return 0; }} } diff --git a/src/network/test-networkd-conf.c b/src/network/test-networkd-conf.c index 00791cea91..31b89fda2f 100644 --- a/src/network/test-networkd-conf.c +++ b/src/network/test-networkd-conf.c @@ -6,6 +6,7 @@ #include "networkd-address.h" #include "networkd-manager.h" #include "networkd-network.h" +#include "networkd-route.h" #include "networkd-route-nexthop.h" #include "ordered-set.h" #include "set.h" @@ -307,6 +308,47 @@ static void test_config_parse_multipath_route_verify( ASSERT_EQ(nh->weight, expected_weight); } +TEST(route_nexthops_compare) { + _cleanup_ordered_set_free_ OrderedSet *a_nexthops = NULL, *b_nexthops = NULL; + Route a = { .family = AF_INET }, b = { .family = AF_INET }; + _cleanup_set_free_ Set *routes = NULL; + + ASSERT_OK_EQ(parse_mpr("10.0.0.1@1 10", &a_nexthops), 1); + ASSERT_OK_EQ(parse_mpr("10.0.0.2@2 20", &a_nexthops), 1); + ASSERT_OK_EQ(parse_mpr("10.0.0.1@1 10", &b_nexthops), 1); + ASSERT_OK_EQ(parse_mpr("10.0.0.2@2 20", &b_nexthops), 1); + + a.nexthops = a_nexthops; + b.nexthops = b_nexthops; + ASSERT_EQ(route_nexthops_compare_func(&a, &b), 0); + + b_nexthops = ordered_set_free(b_nexthops); + ASSERT_OK_EQ(parse_mpr("10.0.0.1@1 10", &b_nexthops), 1); + ASSERT_OK_EQ(parse_mpr("10.0.0.3@2 20", &b_nexthops), 1); + b.nexthops = b_nexthops; + ASSERT_NE(route_nexthops_compare_func(&a, &b), 0); + + b_nexthops = ordered_set_free(b_nexthops); + ASSERT_OK_EQ(parse_mpr("10.0.0.1@1 10", &b_nexthops), 1); + ASSERT_OK_EQ(parse_mpr("10.0.0.2@2 30", &b_nexthops), 1); + b.nexthops = b_nexthops; + ASSERT_NE(route_nexthops_compare_func(&a, &b), 0); + + b_nexthops = ordered_set_free(b_nexthops); + ASSERT_OK_EQ(parse_mpr("10.0.0.2@2 20", &b_nexthops), 1); + ASSERT_OK_EQ(parse_mpr("10.0.0.1@1 10", &b_nexthops), 1); + b.nexthops = b_nexthops; + ASSERT_EQ(route_nexthops_compare_func(&a, &b), 0); + + ASSERT_NOT_NULL(routes = set_new(&route_hash_ops)); + ASSERT_OK_POSITIVE(set_put(routes, &a)); + ASSERT_PTR_EQ(set_get(routes, &b), &a); + ASSERT_OK_ZERO(set_put(routes, &b)); + + ASSERT_OK_EQ(parse_mpr("10.0.0.3@3 30", &b_nexthops), 1); + ASSERT_NE(route_nexthops_compare_func(&a, &b), 0); +} + TEST(config_parse_multipath_route) { _cleanup_ordered_set_free_ OrderedSet *nexthops = NULL;