From 7af3e8cd0034a6eb2e614b052c94c6d0b4556e98 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 23 Jun 2024 14:36:08 +0900 Subject: [PATCH 1/3] network/ndisc: do not override conflicting static routes We have already ignored conflicting address configurations requested by NDisc protocol. See ndisc_request_address(). Let's follow the same rule for routes. That is, if there are conflicting static routes configured or requested, do not override them by NDisc. Also, swap the order of checking existing route and existing request. Fixes a regression caused by 972f1d17ab461a51142a142609dd3ec50bae8440. Prompted by #33346. --- src/network/networkd-ndisc.c | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/src/network/networkd-ndisc.c b/src/network/networkd-ndisc.c index 7cafe1f6a3e..0c43965ea60 100644 --- a/src/network/networkd-ndisc.c +++ b/src/network/networkd-ndisc.c @@ -222,20 +222,33 @@ static int ndisc_request_route(Route *route, Link *link) { /* Note, here do not call route_remove_and_cancel() with 'route' directly, otherwise * existing route(s) may be removed needlessly. */ - if (route_get(link->manager, route, &existing) >= 0) { - /* Found an existing route that may conflict with this route. */ + /* First, check if a conflicting route is already requested. If there is an existing route, + * and also an existing pending request, then the source may be updated by the request. So, + * we first need to check the source of the requested route. */ + if (route_get_request(link->manager, route, &req) >= 0) { + existing = ASSERT_PTR(req->userdata); if (!route_can_update(existing, route)) { - log_link_debug(link, "Found an existing route that conflicts with new route based on a received RA, removing."); + if (existing->source == NETWORK_CONFIG_SOURCE_STATIC) { + log_link_debug(link, "Found a pending route request that conflicts with new request based on a received RA, ignoring request."); + return 0; + } + + log_link_debug(link, "Found a pending route request that conflicts with new request based on a received RA, cancelling."); r = route_remove_and_cancel(existing, link->manager); if (r < 0) return r; } } - if (route_get_request(link->manager, route, &req) >= 0) { - existing = ASSERT_PTR(req->userdata); + /* Then, check if a conflicting route exists. */ + if (route_get(link->manager, route, &existing) >= 0) { if (!route_can_update(existing, route)) { - log_link_debug(link, "Found a pending route request that conflicts with new request based on a received RA, cancelling."); + if (existing->source == NETWORK_CONFIG_SOURCE_STATIC) { + log_link_debug(link, "Found an existing route that conflicts with new route based on a received RA, ignoring request."); + return 0; + } + + log_link_debug(link, "Found an existing route that conflicts with new route based on a received RA, removing."); r = route_remove_and_cancel(existing, link->manager); if (r < 0) return r; From fd436c8d67e75eebd0ef9499f699524e4cbe2a92 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 24 Jun 2024 16:20:27 +0900 Subject: [PATCH 2/3] network/ndisc: do not remove static routes when received RA with zero lifetime Similar to the previous commit, but for preventing from removing static routes on receiving RA with zero lifetime. Fixes a regresson caused by 479d3e1994a2e4ff7070dc2a0cb1615af7120b0c. Fixes #33346. --- src/network/networkd-ndisc.c | 44 ++++++++++++++++++++++++++++-------- 1 file changed, 35 insertions(+), 9 deletions(-) diff --git a/src/network/networkd-ndisc.c b/src/network/networkd-ndisc.c index 0c43965ea60..50d284b26ac 100644 --- a/src/network/networkd-ndisc.c +++ b/src/network/networkd-ndisc.c @@ -304,18 +304,44 @@ static int ndisc_remove_route(Route *route, Link *link) { if (r < 0) return r; - if (route->pref_set) { - ndisc_set_route_priority(link, route); - return route_remove_and_cancel(route, link->manager); - } - - uint8_t pref; + uint8_t pref, pref_original = route->pref; FOREACH_ARGUMENT(pref, SD_NDISC_PREFERENCE_LOW, SD_NDISC_PREFERENCE_MEDIUM, SD_NDISC_PREFERENCE_HIGH) { + Route *existing; + Request *req; + + /* If the preference is specified by the user config (that is, for semi-static routes), + * rather than RA, then only search conflicting routes that have the same preference. */ + if (route->pref_set && pref != pref_original) + continue; + route->pref = pref; ndisc_set_route_priority(link, route); - r = route_remove_and_cancel(route, link->manager); - if (r < 0) - return r; + + /* Unfortunately, we cannot directly pass 'route' to route_remove_and_cancel() here, as the + * same or similar route may be configured or requested statically. */ + + /* First, check if the route is already requested. If there is an existing route, and also an + * existing pending request, then the source may be updated by the request. So, we first need + * to check the source of the requested route. */ + if (route_get_request(link->manager, route, &req) >= 0) { + existing = ASSERT_PTR(req->userdata); + if (existing->source == NETWORK_CONFIG_SOURCE_STATIC) + continue; + + r = route_remove_and_cancel(existing, link->manager); + if (r < 0) + return r; + } + + /* Then, check if the route exists. */ + if (route_get(link->manager, route, &existing) >= 0) { + if (existing->source == NETWORK_CONFIG_SOURCE_STATIC) + continue; + + r = route_remove_and_cancel(existing, link->manager); + if (r < 0) + return r; + } } return 0; From 62fb079a3b4a6553d87d06a3004d21b4e322a4e5 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 23 Jun 2024 14:37:01 +0900 Subject: [PATCH 3/3] test-network: check if static routes not overridden by NDisc routes --- .../25-ipv6-prefix-veth-static-route.network | 14 +++++++++++ test/test-network/systemd-networkd-tests.py | 24 +++++++++++++++++++ 2 files changed, 38 insertions(+) create mode 100644 test/test-network/conf/25-ipv6-prefix-veth-static-route.network diff --git a/test/test-network/conf/25-ipv6-prefix-veth-static-route.network b/test/test-network/conf/25-ipv6-prefix-veth-static-route.network new file mode 100644 index 00000000000..a2ea7bff2c0 --- /dev/null +++ b/test/test-network/conf/25-ipv6-prefix-veth-static-route.network @@ -0,0 +1,14 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +[Match] +Name=veth99 + +[Network] +IPv6AcceptRA=true + +[Route] +Gateway=fe80::1034:56ff:fe78:9abd +GatewayOnLink=no +Metric=256 + +[IPv6AcceptRA] +RouteMetric=256 diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index 0355c7aca1b..7c336ba9e29 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -5824,6 +5824,30 @@ class NetworkdRATests(unittest.TestCase, Utilities): self.assertIn('pref high', output) self.assertNotIn('pref low', output) + def test_ndisc_vs_static_route(self): + copy_network_unit('25-veth.netdev', '25-ipv6-prefix.network', '25-ipv6-prefix-veth-static-route.network') + start_networkd() + self.wait_online('veth99:routable', 'veth-peer:degraded') + + output = check_output('ip -6 route show dev veth99 table all') + print(output) + + # If a conflicting static route is already configured, do not override the static route. + output = check_output('ip -6 route show dev veth99 default via fe80::1034:56ff:fe78:9abd') + print(output) + self.assertIn('default proto static metric 256 pref medium', output) + self.assertNotIn('proto ra', output) + + if not os.path.exists(test_ndisc_send): + self.skipTest(f"{test_ndisc_send} does not exist.") + + # Also check if the static route is protected from RA with zero lifetime + check_output(f'{test_ndisc_send} --interface veth-peer --type router-advertisement --lifetime 0') + time.sleep(2) + output = check_output('ip -6 route show dev veth99 default via fe80::1034:56ff:fe78:9abd') + print(output) + self.assertIn('default proto static metric 256 pref medium', output) + # radvd supports captive portal since v2.20. # https://github.com/radvd-project/radvd/commit/791179a7f730decbddb2290ef0e34aa85d71b1bc @unittest.skipUnless(radvd_check_config('captive-portal.conf'), "Installed radvd doesn't support captive portals")