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-ndisc.c b/src/network/networkd-ndisc.c index 030b4a4339..f9fb7e207b 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); @@ -444,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; @@ -1480,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; @@ -1640,7 +1719,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); 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-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..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); @@ -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 c5c2693ac4..912c6e54a9 100644 --- a/src/network/networkd-route.h +++ b/src/network/networkd-route.h @@ -96,9 +96,12 @@ 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); 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); } diff --git a/test/test-network/conf/25-veth-bridge.network b/test/test-network/conf/25-veth-bridge.network index b2f163412a..a8be06d12a 100644 --- a/test/test-network/conf/25-veth-bridge.network +++ b/test/test-network/conf/25-veth-bridge.network @@ -1,10 +1,10 @@ # SPDX-License-Identifier: LGPL-2.1-or-later [Match] Name=client-p +Name=router-p Name=router-high-p Name=router-low-p [Network] Bridge=bridge99 IPv6AcceptRA=no -IPv6SendRA=yes 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-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-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/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 77c30300c7..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', @@ -5314,9 +5345,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 +5365,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',