From: Luca Boccassi <luca.boccassi@gmail.com>
Date: Mon, 13 Jul 2026 18:23:21 +0000 (+0100)
Subject: network: compare all multipath route nexthops
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=f65d4e6c4ea6d06aee995d46d7c9c39452859da8;p=thirdparty%2Fsystemd.git

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
---

diff --git a/src/network/networkd-route-nexthop.c b/src/network/networkd-route-nexthop.c
index bebb401b339..642cc508f05 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 00791cea913..31b89fda2fa 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;