From a86763c70b29a3a6b2fc543fef9ed5502c9c8991 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 9 Feb 2024 14:50:46 +0900 Subject: [PATCH 1/8] network/ndisc: also drop SLAAC addresses and routes on reconfiguring Especially, this is important for routes, as link_drop_managed_routes() removes only static routes, rather than non-foreign routes. --- src/network/networkd-ndisc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/network/networkd-ndisc.c b/src/network/networkd-ndisc.c index 030b4a4339..5f93f45b3c 100644 --- a/src/network/networkd-ndisc.c +++ b/src/network/networkd-ndisc.c @@ -1640,7 +1640,8 @@ int ndisc_stop(Link *link) { void ndisc_flush(Link *link) { assert(link); - /* Remove all RDNSS, DNSSL, and Captive Portal entries, without exception. */ + /* Remove all addresses, routes, RDNSS, DNSSL, and Captive Portal entries, without exception. */ + (void) ndisc_drop_outdated(link, /* timestamp_usec = */ USEC_INFINITY); link->ndisc_rdnss = set_free(link->ndisc_rdnss); link->ndisc_dnssl = set_free(link->ndisc_dnssl); From e09b758876219de8222df798114d1d63b0323b58 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 9 Feb 2024 14:33:07 +0900 Subject: [PATCH 2/8] network: rename link_drop_managed_xyz() -> link_drop_static_xyz() As networkd does not configure nexthops, neighbors, or routing policy rules dynamically, this only changes link_drop_managed_addresses(). Previously it also removed dynamic addresses, but with this commit, now it only removes static addresses. Note, link_drop_managed_routes() has removed only static routes, so this commit only changes its name. Note. as commented in the code, the dynamic addresses will be removed on lease expiration or stopping client, and link_drop_managed_xyz() are called on reconfiguration or so, and at that time, all dynamic addressing clients are stopped or restarted, so this should not effective behavior change. --- src/network/networkd-address.c | 7 ++++--- src/network/networkd-address.h | 2 +- src/network/networkd-link.c | 11 +++++------ src/network/networkd-neighbor.c | 4 ++-- src/network/networkd-neighbor.h | 2 +- src/network/networkd-nexthop.c | 2 +- src/network/networkd-nexthop.h | 2 +- src/network/networkd-route.h | 2 +- src/network/networkd-routing-policy-rule.c | 2 +- src/network/networkd-routing-policy-rule.h | 2 +- 10 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/network/networkd-address.c b/src/network/networkd-address.c index 2aa8c88bb2..e301cf153d 100644 --- a/src/network/networkd-address.c +++ b/src/network/networkd-address.c @@ -1383,15 +1383,16 @@ int link_drop_foreign_addresses(Link *link) { return r; } -int link_drop_managed_addresses(Link *link) { +int link_drop_static_addresses(Link *link) { Address *address; int r = 0; assert(link); SET_FOREACH(address, link->addresses) { - /* Do not touch addresses managed by kernel or other tools. */ - if (address->source == NETWORK_CONFIG_SOURCE_FOREIGN) + /* Remove only static addresses here. Dynamic addresses will be removed e.g. on lease + * expiration or stopping the DHCP client. */ + if (address->source != NETWORK_CONFIG_SOURCE_STATIC) continue; /* Ignore addresses not assigned yet or already removing. */ diff --git a/src/network/networkd-address.h b/src/network/networkd-address.h index 350d2f6b38..3efbe6f48e 100644 --- a/src/network/networkd-address.h +++ b/src/network/networkd-address.h @@ -103,7 +103,7 @@ bool link_check_addresses_ready(Link *link, NetworkConfigSource source); DEFINE_SECTION_CLEANUP_FUNCTIONS(Address, address_unref); -int link_drop_managed_addresses(Link *link); +int link_drop_static_addresses(Link *link); int link_drop_foreign_addresses(Link *link); int link_drop_ipv6ll_addresses(Link *link); void link_foreignize_addresses(Link *link); diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index ee442e3a4b..566d251b15 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -1115,12 +1115,11 @@ static int link_drop_managed_config(Link *link) { assert(link); assert(link->manager); - r = link_drop_managed_routes(link); - - RET_GATHER(r, link_drop_managed_nexthops(link)); - RET_GATHER(r, link_drop_managed_addresses(link)); - RET_GATHER(r, link_drop_managed_neighbors(link)); - RET_GATHER(r, link_drop_managed_routing_policy_rules(link)); + r = link_drop_static_routes(link); + RET_GATHER(r, link_drop_static_nexthops(link)); + RET_GATHER(r, link_drop_static_addresses(link)); + RET_GATHER(r, link_drop_static_neighbors(link)); + RET_GATHER(r, link_drop_static_routing_policy_rules(link)); return r; } diff --git a/src/network/networkd-neighbor.c b/src/network/networkd-neighbor.c index 4347a1d282..6b81950f96 100644 --- a/src/network/networkd-neighbor.c +++ b/src/network/networkd-neighbor.c @@ -502,7 +502,7 @@ int link_drop_foreign_neighbors(Link *link) { return r; } -int link_drop_managed_neighbors(Link *link) { +int link_drop_static_neighbors(Link *link) { Neighbor *neighbor; int r = 0; @@ -510,7 +510,7 @@ int link_drop_managed_neighbors(Link *link) { SET_FOREACH(neighbor, link->neighbors) { /* Do not touch nexthops managed by kernel or other tools. */ - if (neighbor->source == NETWORK_CONFIG_SOURCE_FOREIGN) + if (neighbor->source != NETWORK_CONFIG_SOURCE_STATIC) continue; /* Ignore neighbors not assigned yet or already removing. */ diff --git a/src/network/networkd-neighbor.h b/src/network/networkd-neighbor.h index 93b3927f02..7917930bcd 100644 --- a/src/network/networkd-neighbor.h +++ b/src/network/networkd-neighbor.h @@ -36,7 +36,7 @@ int neighbor_remove(Neighbor *neighbor, Link *link); int network_drop_invalid_neighbors(Network *network); -int link_drop_managed_neighbors(Link *link); +int link_drop_static_neighbors(Link *link); int link_drop_foreign_neighbors(Link *link); void link_foreignize_neighbors(Link *link); diff --git a/src/network/networkd-nexthop.c b/src/network/networkd-nexthop.c index 56abd3908a..b07feede2b 100644 --- a/src/network/networkd-nexthop.c +++ b/src/network/networkd-nexthop.c @@ -869,7 +869,7 @@ static void link_mark_nexthops(Link *link, bool foreign) { continue; /* When 'foreign' is true, mark only foreign nexthops, and vice versa. */ - if (foreign != (nexthop->source == NETWORK_CONFIG_SOURCE_FOREIGN)) + if (nexthop->source != (foreign ? NETWORK_CONFIG_SOURCE_FOREIGN : NETWORK_CONFIG_SOURCE_STATIC)) continue; /* Ignore nexthops not assigned yet or already removed. */ diff --git a/src/network/networkd-nexthop.h b/src/network/networkd-nexthop.h index 9ce36c6cd9..fbe330ad71 100644 --- a/src/network/networkd-nexthop.h +++ b/src/network/networkd-nexthop.h @@ -57,7 +57,7 @@ int link_drop_nexthops(Link *link, bool foreign); static inline int link_drop_foreign_nexthops(Link *link) { return link_drop_nexthops(link, /* foreign = */ true); } -static inline int link_drop_managed_nexthops(Link *link) { +static inline int link_drop_static_nexthops(Link *link) { return link_drop_nexthops(link, /* foreign = */ false); } void link_foreignize_nexthops(Link *link); diff --git a/src/network/networkd-route.h b/src/network/networkd-route.h index c5c2693ac4..a5019db65c 100644 --- a/src/network/networkd-route.h +++ b/src/network/networkd-route.h @@ -98,7 +98,7 @@ int route_remove_and_cancel(Route *route, Manager *manager); int route_get(Manager *manager, const Route *route, Route **ret); int link_drop_routes(Link *link, bool foreign); -static inline int link_drop_managed_routes(Link *link) { +static inline int link_drop_static_routes(Link *link) { return link_drop_routes(link, false); } static inline int link_drop_foreign_routes(Link *link) { diff --git a/src/network/networkd-routing-policy-rule.c b/src/network/networkd-routing-policy-rule.c index e8e154dd60..bb757eed71 100644 --- a/src/network/networkd-routing-policy-rule.c +++ b/src/network/networkd-routing-policy-rule.c @@ -654,7 +654,7 @@ static void manager_mark_routing_policy_rules(Manager *m, bool foreign, const Li continue; /* When 'foreign' is true, mark only foreign rules, and vice versa. */ - if (foreign != (rule->source == NETWORK_CONFIG_SOURCE_FOREIGN)) + if (rule->source != (foreign ? NETWORK_CONFIG_SOURCE_FOREIGN : NETWORK_CONFIG_SOURCE_STATIC)) continue; /* Ignore rules not assigned yet or already removing. */ diff --git a/src/network/networkd-routing-policy-rule.h b/src/network/networkd-routing-policy-rule.h index 85016bbc8a..42a575c493 100644 --- a/src/network/networkd-routing-policy-rule.h +++ b/src/network/networkd-routing-policy-rule.h @@ -66,7 +66,7 @@ int manager_drop_routing_policy_rules_internal(Manager *m, bool foreign, const L static inline int manager_drop_foreign_routing_policy_rules(Manager *m) { return manager_drop_routing_policy_rules_internal(m, true, NULL); } -static inline int link_drop_managed_routing_policy_rules(Link *link) { +static inline int link_drop_static_routing_policy_rules(Link *link) { assert(link); return manager_drop_routing_policy_rules_internal(link->manager, false, link); } From 7027cdbd79c26d5a8890759a37d70165c8aeb214 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 2 Feb 2024 12:18:33 +0900 Subject: [PATCH 3/8] network/route: remove existing route if some property conflict with requested ones Fixes #28853. --- src/network/networkd-route-metric.c | 22 ++++++++++++++++ src/network/networkd-route-metric.h | 1 + src/network/networkd-route.c | 40 +++++++++++++++++++++++++++++ src/network/networkd-route.h | 2 ++ 4 files changed, 65 insertions(+) diff --git a/src/network/networkd-route-metric.c b/src/network/networkd-route-metric.c index 9c417a4fc4..1bae893934 100644 --- a/src/network/networkd-route-metric.c +++ b/src/network/networkd-route-metric.c @@ -63,6 +63,28 @@ int route_metric_compare_func(const RouteMetric *a, const RouteMetric *b) { return strcmp_ptr(a->tcp_congestion_control_algo, b->tcp_congestion_control_algo); } +bool route_metric_can_update(const RouteMetric *a, const RouteMetric *b, bool expiration_by_kernel) { + assert(a); + assert(b); + + /* If the kernel has expiration timer for the route, then only MTU can be updated. */ + + if (!expiration_by_kernel) + return route_metric_compare_func(a, b) == 0; + + if (a->n_metrics != b->n_metrics) + return false; + + for (size_t i = 1; i < a->n_metrics; i++) { + if (i != RTAX_MTU) + continue; + if (a->metrics[i] != b->metrics[i]) + return false; + } + + return streq_ptr(a->tcp_congestion_control_algo, b->tcp_congestion_control_algo); +} + int route_metric_set_full(RouteMetric *metric, uint16_t attr, uint32_t value, bool force) { assert(metric); diff --git a/src/network/networkd-route-metric.h b/src/network/networkd-route-metric.h index 1a3aaf2ae5..212f9079cb 100644 --- a/src/network/networkd-route-metric.h +++ b/src/network/networkd-route-metric.h @@ -26,6 +26,7 @@ int route_metric_copy(const RouteMetric *src, RouteMetric *dest); void route_metric_hash_func(const RouteMetric *metric, struct siphash *state); int route_metric_compare_func(const RouteMetric *a, const RouteMetric *b); +bool route_metric_can_update(const RouteMetric *a, const RouteMetric *b, bool expiration_by_kernel); int route_metric_set_full(RouteMetric *metric, uint16_t attr, uint32_t value, bool force); static inline int route_metric_set(RouteMetric *metric, uint16_t attr, uint32_t value) { diff --git a/src/network/networkd-route.c b/src/network/networkd-route.c index b0bc732054..743b4f5520 100644 --- a/src/network/networkd-route.c +++ b/src/network/networkd-route.c @@ -1305,6 +1305,43 @@ static bool route_by_kernel(const Route *route) { return false; } +bool route_can_update(const Route *existing, const Route *requesting) { + assert(existing); + assert(requesting); + + if (route_compare_func(existing, requesting) != 0) + return false; + + switch (existing->family) { + case AF_INET: + if (existing->nexthop.weight != requesting->nexthop.weight) + return false; + return true; + + case AF_INET6: + if (existing->protocol != requesting->protocol) + return false; + if (existing->type != requesting->type) + return false; + if (existing->flags != requesting->flags) + return false; + if (!in6_addr_equal(&existing->prefsrc.in6, &requesting->prefsrc.in6)) + return false; + if (existing->pref != requesting->pref) + return false; + if (existing->expiration_managed_by_kernel && requesting->lifetime_usec != USEC_INFINITY) + return false; /* We cannot disable expiration timer in the kernel. */ + if (!route_metric_can_update(&existing->metric, &requesting->metric, existing->expiration_managed_by_kernel)) + return false; + if (existing->nexthop.weight != requesting->nexthop.weight) + return false; + return true; + + default: + assert_not_reached(); + } +} + static int link_unmark_route(Link *link, const Route *route, const RouteNextHop *nh) { _cleanup_(route_unrefp) Route *tmp = NULL; Route *existing; @@ -1324,6 +1361,9 @@ static int link_unmark_route(Link *link, const Route *route, const RouteNextHop if (route_get(link->manager, tmp, &existing) < 0) return 0; + if (!route_can_update(existing, tmp)) + return 0; + route_unmark(existing); return 1; } diff --git a/src/network/networkd-route.h b/src/network/networkd-route.h index a5019db65c..d292716b1d 100644 --- a/src/network/networkd-route.h +++ b/src/network/networkd-route.h @@ -97,6 +97,8 @@ int route_remove_and_cancel(Route *route, Manager *manager); int route_get(Manager *manager, const Route *route, Route **ret); +bool route_can_update(const Route *existing, const Route *requesting); + int link_drop_routes(Link *link, bool foreign); static inline int link_drop_static_routes(Link *link) { return link_drop_routes(link, false); From 972f1d17ab461a51142a142609dd3ec50bae8440 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 2 Feb 2024 12:30:32 +0900 Subject: [PATCH 4/8] network/ndisc: remove conflicting routes on configuring routes based on newly received RA The linux kernel does not update several parameters, e.g. RTA_PREF. Hence, when we configure routes based on a RA, we need to remove existing conflicting routes. Fixes #28426 and #28439. --- src/network/networkd-ndisc.c | 42 +++++++++++++++++++++++++++++++++++- src/network/networkd-route.c | 2 +- src/network/networkd-route.h | 1 + 3 files changed, 43 insertions(+), 2 deletions(-) diff --git a/src/network/networkd-ndisc.c b/src/network/networkd-ndisc.c index 5f93f45b3c..e813f8c98b 100644 --- a/src/network/networkd-ndisc.c +++ b/src/network/networkd-ndisc.c @@ -205,7 +205,6 @@ static int ndisc_request_route(Route *route, Link *link, sd_ndisc_router *rt) { route->provider.in6 = router; if (!route->table_set) route->table = link_get_ipv6_accept_ra_route_table(link); - ndisc_set_route_priority(link, route); if (!route->protocol_set) route->protocol = RTPROT_RA; r = route_metric_set(&route->metric, RTAX_MTU, mtu); @@ -222,6 +221,47 @@ static int ndisc_request_route(Route *route, Link *link, sd_ndisc_router *rt) { if (r < 0) return r; + 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); + + /* 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. */ + 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."); + 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); + 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."); + r = route_remove_and_cancel(existing, link->manager); + if (r < 0) + return r; + } + } + } + + /* The preference (and priority) may be changed in the above loop. Restore it. */ + route->pref = pref_original; + ndisc_set_route_priority(link, route); + is_new = route_get(link->manager, route, NULL) < 0; r = link_request_route(link, route, &link->ndisc_messages, ndisc_route_handler); diff --git a/src/network/networkd-route.c b/src/network/networkd-route.c index 743b4f5520..ee12f1664a 100644 --- a/src/network/networkd-route.c +++ b/src/network/networkd-route.c @@ -352,7 +352,7 @@ static int route_get_link(Manager *manager, const Route *route, Link **ret) { return route_nexthop_get_link(manager, &route->nexthop, ret); } -static int route_get_request(Manager *manager, const Route *route, Request **ret) { +int route_get_request(Manager *manager, const Route *route, Request **ret) { Request *req; assert(manager); diff --git a/src/network/networkd-route.h b/src/network/networkd-route.h index d292716b1d..912c6e54a9 100644 --- a/src/network/networkd-route.h +++ b/src/network/networkd-route.h @@ -96,6 +96,7 @@ int route_remove(Route *route, Manager *manager); int route_remove_and_cancel(Route *route, Manager *manager); int route_get(Manager *manager, const Route *route, Route **ret); +int route_get_request(Manager *manager, const Route *route, Request **ret); bool route_can_update(const Route *existing, const Route *requesting); From e3cc2bd9c6609d22c5e7cd3efba3c8d41216734e Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 8 Feb 2024 19:01:27 +0900 Subject: [PATCH 5/8] test-network: drop unnecessary IPv6SendRA=yes setting --- test/test-network/conf/25-veth-bridge.network | 1 - 1 file changed, 1 deletion(-) diff --git a/test/test-network/conf/25-veth-bridge.network b/test/test-network/conf/25-veth-bridge.network index b2f163412a..8059eb332e 100644 --- a/test/test-network/conf/25-veth-bridge.network +++ b/test/test-network/conf/25-veth-bridge.network @@ -7,4 +7,3 @@ Name=router-low-p [Network] Bridge=bridge99 IPv6AcceptRA=no -IPv6SendRA=yes From 9fbab82bef7bbb14e0898239d7c13d3b5896fa1c Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 8 Feb 2024 18:24:33 +0900 Subject: [PATCH 6/8] test-network: add test case for removing conflicting routes For issue #28439. --- .../conf/25-veth-router-high2.network | 18 +++++++++++++ .../conf/25-veth-router-low2.network | 18 +++++++++++++ test/test-network/systemd-networkd-tests.py | 26 +++++++++++++++++++ 3 files changed, 62 insertions(+) create mode 100644 test/test-network/conf/25-veth-router-high2.network create mode 100644 test/test-network/conf/25-veth-router-low2.network diff --git a/test/test-network/conf/25-veth-router-high2.network b/test/test-network/conf/25-veth-router-high2.network new file mode 100644 index 0000000000..47e8cd715c --- /dev/null +++ b/test/test-network/conf/25-veth-router-high2.network @@ -0,0 +1,18 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +[Match] +Name=router-low + +[Network] +IPv6AcceptRA=no +IPv6SendRA=yes + +[IPv6SendRA] +# changed from low to high +RouterPreference=high +EmitDNS=no +EmitDomains=no + +[IPv6Prefix] +Prefix=2002:da8:1:98::/64 +PreferredLifetimeSec=1000s +ValidLifetimeSec=2100s diff --git a/test/test-network/conf/25-veth-router-low2.network b/test/test-network/conf/25-veth-router-low2.network new file mode 100644 index 0000000000..f318938ca7 --- /dev/null +++ b/test/test-network/conf/25-veth-router-low2.network @@ -0,0 +1,18 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +[Match] +Name=router-high + +[Network] +IPv6AcceptRA=no +IPv6SendRA=yes + +[IPv6SendRA] +# changed from high to low +RouterPreference=low +EmitDNS=no +EmitDomains=no + +[IPv6Prefix] +Prefix=2002:da8:1:99::/64 +PreferredLifetimeSec=1000s +ValidLifetimeSec=2100s diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index 77c30300c7..134dad3bb9 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -5314,9 +5314,11 @@ class NetworkdRATests(unittest.TestCase, Utilities): output = check_output('ip -6 route show dev client default via fe80::1034:56ff:fe78:9a99') print(output) + self.assertIn('metric 512', output) self.assertIn('pref high', output) output = check_output('ip -6 route show dev client default via fe80::1034:56ff:fe78:9a98') print(output) + self.assertIn('metric 2048', output) self.assertIn('pref low', output) with open(os.path.join(network_unit_dir, '25-veth-client.network'), mode='a', encoding='utf-8') as f: @@ -5332,11 +5334,35 @@ class NetworkdRATests(unittest.TestCase, Utilities): output = check_output('ip -6 route show dev client default via fe80::1034:56ff:fe78:9a99') print(output) + self.assertIn('metric 100', output) + self.assertNotIn('metric 512', output) self.assertIn('pref high', output) output = check_output('ip -6 route show dev client default via fe80::1034:56ff:fe78:9a98') print(output) + self.assertIn('metric 300', output) + self.assertNotIn('metric 2048', output) self.assertIn('pref low', output) + # swap the preference (for issue #28439) + remove_network_unit('25-veth-router-high.network', '25-veth-router-low.network') + copy_network_unit('25-veth-router-high2.network', '25-veth-router-low2.network') + networkctl_reload() + self.wait_route('client', 'default via fe80::1034:56ff:fe78:9a99 proto ra metric 300', ipv='-6', timeout_sec=10) + self.wait_route('client', 'default via fe80::1034:56ff:fe78:9a98 proto ra metric 100', ipv='-6', timeout_sec=10) + + output = check_output('ip -6 route show dev client default via fe80::1034:56ff:fe78:9a99') + print(output) + self.assertIn('metric 300', output) + self.assertNotIn('metric 100', output) + self.assertIn('pref low', output) + self.assertNotIn('pref high', output) + output = check_output('ip -6 route show dev client default via fe80::1034:56ff:fe78:9a98') + print(output) + self.assertIn('metric 100', output) + self.assertNotIn('metric 300', output) + self.assertIn('pref high', output) + self.assertNotIn('pref low', output) + @unittest.skipUnless(radvd_check_config('captive-portal.conf'), "Installed radvd doesn't support captive portals") def test_captive_portal(self): copy_network_unit('25-veth-client.netdev', From b15ed2be2ffdb8d884c60582179f49b08759881e Mon Sep 17 00:00:00 2001 From: Matt Muggeridge Date: Thu, 8 Feb 2024 06:21:58 +0900 Subject: [PATCH 7/8] network/ndisc: also set sysctl hop_limit value Otherwise, the hop limit set to the routes may not be used in some situations. Fixes #28437. Co-authored-by: Yu Watanabe --- src/network/networkd-ndisc.c | 39 ++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/src/network/networkd-ndisc.c b/src/network/networkd-ndisc.c index e813f8c98b..f9fb7e207b 100644 --- a/src/network/networkd-ndisc.c +++ b/src/network/networkd-ndisc.c @@ -484,6 +484,41 @@ static int ndisc_router_process_retransmission_time(Link *link, sd_ndisc_router return 0; } +static int ndisc_router_process_hop_limit(Link *link, sd_ndisc_router *rt) { + uint8_t hop_limit; + int r; + + assert(link); + assert(link->network); + assert(rt); + + if (!link->network->ipv6_accept_ra_use_hop_limit) + return 0; + + r = sd_ndisc_router_get_hop_limit(rt, &hop_limit); + if (r < 0) + return log_link_warning_errno(link, r, "Failed to get hop limit from RA: %m"); + + /* 0 is the unspecified value and must not be set (see RFC4861, 6.3.4): + * + * A Router Advertisement field (e.g., Cur Hop Limit, Reachable Time, and Retrans Timer) may contain + * a value denoting that it is unspecified. In such cases, the parameter should be ignored and the + * host should continue using whatever value it is already using. In particular, a host MUST NOT + * interpret the unspecified value as meaning change back to the default value that was in use before + * the first Router Advertisement was received. + * + * If the received Cur Hop Limit value is non-zero, the host SHOULD set + * its CurHopLimit variable to the received value.*/ + if (hop_limit <= 0) + return 0; + + r = sysctl_write_ip_property_uint32(AF_INET6, link->ifname, "hop_limit", (uint32_t) hop_limit); + if (r < 0) + log_link_warning_errno(link, r, "Failed to apply hop_limit (%u), ignoring: %m", hop_limit); + + return 0; +} + static int ndisc_router_process_autonomous_prefix(Link *link, sd_ndisc_router *rt) { usec_t lifetime_valid_usec, lifetime_preferred_usec; _cleanup_set_free_ Set *addresses = NULL; @@ -1520,6 +1555,10 @@ static int ndisc_router_handler(Link *link, sd_ndisc_router *rt) { if (r < 0) return r; + r = ndisc_router_process_hop_limit(link, rt); + if (r < 0) + return r; + r = ndisc_router_process_options(link, rt); if (r < 0) return r; From 29fbbb138920d82b5213880cc042bfdaedf2da6b Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 8 Feb 2024 18:49:44 +0900 Subject: [PATCH 8/8] test-network: add test for advertised hop limit For issue #28437. --- test/test-network/conf/25-veth-bridge.network | 1 + .../conf/25-veth-router-hop-limit.network | 18 +++++++++++ test/test-network/conf/25-veth-router.netdev | 9 ++++++ test/test-network/systemd-networkd-tests.py | 31 +++++++++++++++++++ 4 files changed, 59 insertions(+) create mode 100644 test/test-network/conf/25-veth-router-hop-limit.network create mode 100644 test/test-network/conf/25-veth-router.netdev diff --git a/test/test-network/conf/25-veth-bridge.network b/test/test-network/conf/25-veth-bridge.network index 8059eb332e..a8be06d12a 100644 --- a/test/test-network/conf/25-veth-bridge.network +++ b/test/test-network/conf/25-veth-bridge.network @@ -1,6 +1,7 @@ # SPDX-License-Identifier: LGPL-2.1-or-later [Match] Name=client-p +Name=router-p Name=router-high-p Name=router-low-p diff --git a/test/test-network/conf/25-veth-router-hop-limit.network b/test/test-network/conf/25-veth-router-hop-limit.network new file mode 100644 index 0000000000..643e362c67 --- /dev/null +++ b/test/test-network/conf/25-veth-router-hop-limit.network @@ -0,0 +1,18 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +[Match] +Name=router + +[Network] +IPv6AcceptRA=no +IPv6SendRA=yes + +[IPv6SendRA] +RouterPreference=high +EmitDNS=no +EmitDomains=no +HopLimit=42 + +[IPv6Prefix] +Prefix=2002:da8:1:99::/64 +PreferredLifetimeSec=1000s +ValidLifetimeSec=2100s diff --git a/test/test-network/conf/25-veth-router.netdev b/test/test-network/conf/25-veth-router.netdev new file mode 100644 index 0000000000..d462313af8 --- /dev/null +++ b/test/test-network/conf/25-veth-router.netdev @@ -0,0 +1,9 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +[NetDev] +Name=router +Kind=veth +MACAddress=12:34:56:78:9a:99 + +[Peer] +Name=router-p +MACAddress=12:34:56:78:9b:99 diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index 134dad3bb9..3cfddf59d2 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -5288,6 +5288,37 @@ class NetworkdRATests(unittest.TestCase, Utilities): self.assertIn('2002:da8:1:0:b47e:7975:fc7a:7d6e', output) self.assertIn('2002:da8:2:0:f689:561a:8eda:7443', output) + def check_router_hop_limit(self, hop_limit): + self.wait_route('client', rf'default via fe80::1034:56ff:fe78:9a99 proto ra .* hoplimit {hop_limit}', ipv='-6', timeout_sec=10) + + output = check_output('ip -6 route show dev client default via fe80::1034:56ff:fe78:9a99') + print(output) + self.assertIn(f'hoplimit {hop_limit}', output) + + self.check_ipv6_sysctl_attr('client', 'hop_limit', f'{hop_limit}') + + def test_router_hop_limit(self): + copy_network_unit('25-veth-client.netdev', + '25-veth-router.netdev', + '26-bridge.netdev', + '25-veth-bridge.network', + '25-veth-client.network', + '25-veth-router-hop-limit.network', + '25-bridge99.network') + start_networkd() + self.wait_online('client-p:enslaved', + 'router:degraded', 'router-p:enslaved', + 'bridge99:routable') + + self.check_router_hop_limit(42) + + with open(os.path.join(network_unit_dir, '25-veth-router-hop-limit.network'), mode='a', encoding='utf-8') as f: + f.write('\n[IPv6SendRA]\nHopLimit=43\n') + + networkctl_reload() + + self.check_router_hop_limit(43) + def test_router_preference(self): copy_network_unit('25-veth-client.netdev', '25-veth-router-high.netdev',