diff --git a/src/network/networkd-ndisc.c b/src/network/networkd-ndisc.c index c1d2373f65..3871618e5f 100644 --- a/src/network/networkd-ndisc.c +++ b/src/network/networkd-ndisc.c @@ -196,11 +196,6 @@ static int ndisc_request_route(Route *route, Link *link) { assert(link->manager); assert(link->network); - route->source = NETWORK_CONFIG_SOURCE_NDISC; - - if (!route->table_set) - route->table = link_get_ndisc_route_table(link); - r = route_metric_set(&route->metric, RTAX_QUICKACK, link->network->ndisc_quickack); if (r < 0) return r; @@ -229,6 +224,9 @@ static int ndisc_request_route(Route *route, Link *link) { * 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) { + route->pref = pref_original; + ndisc_set_route_priority(link, route); + existing = ASSERT_PTR(req->userdata); if (!route_can_update(existing, route)) { if (existing->source == NETWORK_CONFIG_SOURCE_STATIC) { @@ -243,8 +241,14 @@ static int ndisc_request_route(Route *route, Link *link) { } } + route->pref = pref; + ndisc_set_route_priority(link, route); + /* Then, check if a conflicting route exists. */ if (route_get(link->manager, route, &existing) >= 0) { + route->pref = pref_original; + ndisc_set_route_priority(link, route); + if (!route_can_update(existing, route)) { 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."); @@ -274,6 +278,29 @@ static int ndisc_request_route(Route *route, Link *link) { return 0; } +static void ndisc_route_prepare(Route *route, Link *link) { + assert(route); + assert(link); + + route->source = NETWORK_CONFIG_SOURCE_NDISC; + + if (!route->table_set) + route->table = link_get_ndisc_route_table(link); +} + +static int ndisc_router_route_prepare(Route *route, Link *link, sd_ndisc_router *rt) { + assert(route); + assert(link); + assert(rt); + + ndisc_route_prepare(route, link); + + if (!route->protocol_set) + route->protocol = RTPROT_RA; + + return sd_ndisc_router_get_sender_address(rt, &route->provider.in6); +} + static int ndisc_request_router_route(Route *route, Link *link, sd_ndisc_router *rt) { int r; @@ -281,28 +308,20 @@ static int ndisc_request_router_route(Route *route, Link *link, sd_ndisc_router assert(link); assert(rt); - r = sd_ndisc_router_get_sender_address(rt, &route->provider.in6); + r = ndisc_router_route_prepare(route, link, rt); if (r < 0) return r; - if (!route->protocol_set) - route->protocol = RTPROT_RA; - return ndisc_request_route(route, link); } static int ndisc_remove_route(Route *route, Link *link) { - int r; + int r, ret = 0; assert(route); assert(link); assert(link->manager); - ndisc_set_route_priority(link, route); - - if (!route->table_set) - route->table = link_get_ndisc_route_table(link); - r = route_adjust_nexthops(route, link); if (r < 0) return r; @@ -331,9 +350,7 @@ static int ndisc_remove_route(Route *route, Link *link) { if (existing->source == NETWORK_CONFIG_SOURCE_STATIC) continue; - r = route_remove_and_cancel(existing, link->manager); - if (r < 0) - return r; + RET_GATHER(ret, route_remove_and_cancel(existing, link->manager)); } /* Then, check if the route exists. */ @@ -341,13 +358,25 @@ static int ndisc_remove_route(Route *route, Link *link) { if (existing->source == NETWORK_CONFIG_SOURCE_STATIC) continue; - r = route_remove_and_cancel(existing, link->manager); - if (r < 0) - return r; + RET_GATHER(ret, route_remove_and_cancel(existing, link->manager)); } } - return 0; + return ret; +} + +static int ndisc_remove_router_route(Route *route, Link *link, sd_ndisc_router *rt) { + int r; + + assert(route); + assert(link); + assert(rt); + + r = ndisc_router_route_prepare(route, link, rt); + if (r < 0) + return r; + + return ndisc_remove_route(route, link); } static int ndisc_address_handler(sd_netlink *rtnl, sd_netlink_message *m, Request *req, Link *link, Address *address) { @@ -480,6 +509,8 @@ static int ndisc_remove_redirect_route(Link *link, sd_ndisc_redirect *rd) { if (r < 0) return r; + ndisc_route_prepare(route, link); + return ndisc_remove_route(route, link); } @@ -680,6 +711,8 @@ static int ndisc_redirect_handler(Link *link, sd_ndisc_redirect *rd) { if (r < 0) return r; + ndisc_route_prepare(route, link); + return ndisc_request_route(route, link); } @@ -755,7 +788,7 @@ static int ndisc_router_drop_default(Link *link, sd_ndisc_router *rt) { route->nexthop.family = AF_INET6; route->nexthop.gw.in6 = gateway; - r = ndisc_remove_route(route, link); + r = ndisc_remove_router_route(route, link, rt); if (r < 0) return log_link_warning_errno(link, r, "Failed to remove the default gateway configured by RA: %m"); @@ -775,7 +808,7 @@ static int ndisc_router_drop_default(Link *link, sd_ndisc_router *rt) { tmp->nexthop.gw.in6 = gateway; - r = ndisc_remove_route(tmp, link); + r = ndisc_remove_router_route(tmp, link, rt); if (r < 0) return log_link_warning_errno(link, r, "Could not remove semi-static gateway: %m"); } @@ -1280,7 +1313,7 @@ static int ndisc_router_process_onlink_prefix(Link *link, sd_ndisc_router *rt) { * received advertisement, reset its invalidation timer to the Valid Lifetime value in the Prefix * Information option. If the new Lifetime value is zero, timeout the prefix immediately. */ if (lifetime_usec == 0) { - r = ndisc_remove_route(route, link); + r = ndisc_remove_router_route(route, link, rt); if (r < 0) return log_link_warning_errno(link, r, "Failed to remove prefix route: %m"); } else { @@ -1418,7 +1451,7 @@ static int ndisc_router_process_route(Link *link, sd_ndisc_router *rt) { if (r < 0) return log_link_warning_errno(link, r, "Could not request additional route: %m"); } else { - r = ndisc_remove_route(route, link); + r = ndisc_remove_router_route(route, link, rt); if (r < 0) return log_link_warning_errno(link, r, "Could not remove additional route with zero lifetime: %m"); } @@ -2062,7 +2095,7 @@ static int ndisc_drop_outdated(Link *link, const struct in6_addr *router, usec_t if (route->source != NETWORK_CONFIG_SOURCE_NDISC) continue; - if (route->nexthop.ifindex != link->ifindex) + if (!route_is_bound_to_link(route, link)) continue; if (route->protocol == RTPROT_REDIRECT) @@ -2198,7 +2231,7 @@ static int ndisc_setup_expire(Link *link) { if (route->source != NETWORK_CONFIG_SOURCE_NDISC) continue; - if (route->nexthop.ifindex != link->ifindex) + if (!route_is_bound_to_link(route, link)) continue; if (!route_exists(route)) @@ -2436,7 +2469,7 @@ static int ndisc_neighbor_handle_router_message(Link *link, sd_ndisc_neighbor *n if (route->source != NETWORK_CONFIG_SOURCE_NDISC) continue; - if (route->nexthop.ifindex != link->ifindex) + if (!route_is_bound_to_link(route, link)) continue; if (!in6_addr_equal(&route->provider.in6, &original_address)) @@ -2729,7 +2762,7 @@ int link_drop_ndisc_config(Link *link, Network *network) { if (route->source != NETWORK_CONFIG_SOURCE_NDISC) continue; - if (route->nexthop.ifindex != link->ifindex) + if (!route_is_bound_to_link(route, link)) continue; if (route->protocol == RTPROT_REDIRECT) diff --git a/src/network/networkd-route.c b/src/network/networkd-route.c index 5104223396..b85d2a037a 100644 --- a/src/network/networkd-route.c +++ b/src/network/networkd-route.c @@ -352,6 +352,18 @@ static int route_get_link(Manager *manager, const Route *route, Link **ret) { return route_nexthop_get_link(manager, &route->nexthop, ret); } +bool route_is_bound_to_link(const Route *route, Link *link) { + assert(route); + assert(link); + assert(link->manager); + + Link *route_link; + if (route_get_link(link->manager, route, &route_link) < 0) + return false; + + return route_link->ifindex == link->ifindex; +} + int route_get_request(Manager *manager, const Route *route, Request **ret) { Request *req; diff --git a/src/network/networkd-route.h b/src/network/networkd-route.h index f391d95714..b9d2dbf9a6 100644 --- a/src/network/networkd-route.h +++ b/src/network/networkd-route.h @@ -100,6 +100,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); +bool route_is_bound_to_link(const Route *route, Link *link); int route_get_request(Manager *manager, const Route *route, Request **ret); bool route_can_update(const Route *existing, const Route *requesting); diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index 3944996868..5ae57dd951 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -6312,14 +6312,11 @@ class NetworkdRATests(unittest.TestCase, Utilities): self.wait_route('client', 'default via fe80::1034:56ff:fe78:9a99 proto ra metric 512', ipv='-6', timeout_sec=10) self.wait_route('client', 'default via fe80::1034:56ff:fe78:9a98 proto ra metric 2048', ipv='-6', timeout_sec=10) - output = check_output('ip -6 route show dev client default via fe80::1034:56ff:fe78:9a99') + print('### ip -6 route show dev client default') + output = check_output('ip -6 route show dev client default') 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) + self.assertRegex(output, r'default via fe80::1034:56ff:fe78:9a99 proto ra metric 512 expires [0-9]*sec pref high') + self.assertRegex(output, r'default via fe80::1034:56ff:fe78:9a98 proto ra metric 2048 expires [0-9]*sec pref low') with open(os.path.join(network_unit_dir, '25-veth-client.network'), mode='a', encoding='utf-8') as f: f.write('\n[Link]\nMACAddress=12:34:56:78:9a:01\n[IPv6AcceptRA]\nRouteMetric=100:200:300\n') @@ -6332,16 +6329,13 @@ class NetworkdRATests(unittest.TestCase, Utilities): self.wait_route('client', 'default via fe80::1034:56ff:fe78:9a99 proto ra metric 100', ipv='-6', timeout_sec=10) self.wait_route('client', 'default via fe80::1034:56ff:fe78:9a98 proto ra metric 300', ipv='-6', timeout_sec=10) - output = check_output('ip -6 route show dev client default via fe80::1034:56ff:fe78:9a99') + print('### ip -6 route show dev client default') + output = check_output('ip -6 route show dev client default') print(output) - self.assertIn('metric 100', output) + self.assertRegex(output, r'default via fe80::1034:56ff:fe78:9a99 proto ra metric 100 expires [0-9]*sec pref high') + self.assertRegex(output, r'default via fe80::1034:56ff:fe78:9a98 proto ra metric 300 expires [0-9]*sec pref low') 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') @@ -6350,42 +6344,39 @@ class NetworkdRATests(unittest.TestCase, Utilities): 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('### ip -6 route show dev client default') + output = check_output('ip -6 route show dev client default') 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) + self.assertRegex(output, r'default via fe80::1034:56ff:fe78:9a99 proto ra metric 300 expires [0-9]*sec pref low') + self.assertRegex(output, r'default via fe80::1034:56ff:fe78:9a98 proto ra metric 100 expires [0-9]*sec pref high') + self.assertNotRegex(output, 'default via fe80::1034:56ff:fe78:9a99 proto ra metric 100') + self.assertNotRegex(output, 'default via fe80::1034:56ff:fe78:9a98 proto ra metric 300') + self.assertNotIn('metric 512', output) + self.assertNotIn('metric 2048', 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('### ip -6 route show dev veth99 default') + output = check_output('ip -6 route show dev veth99 default') print(output) - self.assertIn('default proto static metric 256 pref medium', output) + self.assertIn('via fe80::1034:56ff:fe78:9abd 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') + with open(os.path.join(network_unit_dir, '25-ipv6-prefix.network'), mode='a', encoding='utf-8') as f: + f.write('\n[Network]\nIPv6SendRA=no\n') + networkctl_reload() # This makes veth-peer being reconfigured, and send RA with zero lifetime + self.wait_route_dropped('veth99', 'default via fe80::1034:56ff:fe78:9abd proto ra metric 256', ipv='-6', timeout_sec=10) + + print('### ip -6 route show dev veth99 default') + output = check_output('ip -6 route show dev veth99 default') print(output) - self.assertIn('default proto static metric 256 pref medium', output) + self.assertIn('via fe80::1034:56ff:fe78:9abd proto static metric 256 pref medium', output) + self.assertNotIn('proto ra', output) # radvd supports captive portal since v2.20. # https://github.com/radvd-project/radvd/commit/791179a7f730decbddb2290ef0e34aa85d71b1bc @@ -8275,22 +8266,25 @@ class NetworkdIPv6PrefixTests(unittest.TestCase, Utilities): start_networkd() self.wait_online('veth99:routable', 'veth-peer:routable', 'dummy98:routable') - output = check_output('ip address show dev veth-peer') + print('### ip -6 address show dev veth-peer') + output = check_output('ip -6 address show dev veth-peer') print(output) self.assertIn('inet6 2001:db8:0:1:', output) self.assertNotIn('inet6 2001:db8:0:2:', output) self.assertNotIn('inet6 2001:db8:0:3:', output) + print('### ip -6 route show dev veth-peer') output = check_output('ip -6 route show dev veth-peer') print(output) self.assertIn('2001:db8:0:1::/64 proto ra', output) self.assertNotIn('2001:db8:0:2::/64 proto ra', output) self.assertNotIn('2001:db8:0:3::/64 proto ra', output) - self.assertIn('2001:db0:fff::/64 via ', output) - self.assertNotIn('2001:db1:fff::/64 via ', output) - self.assertNotIn('2001:db2:fff::/64 via ', output) + self.assertRegex(output, '2001:db0:fff::/64 via fe80::1034:56ff:fe78:9abc') + self.assertNotIn('2001:db1:fff::/64', output) + self.assertNotIn('2001:db2:fff::/64', output) - output = check_output('ip address show dev veth99') + print('### ip -6 address show dev veth99') + output = check_output('ip -6 address show dev veth99') print(output) self.assertNotIn('inet6 2001:db8:0:1:', output) self.assertIn('inet6 2001:db8:0:2:1a:2b:3c:4d', output) @@ -8326,19 +8320,22 @@ class NetworkdIPv6PrefixTests(unittest.TestCase, Utilities): start_networkd() self.wait_online('veth99:routable', 'veth-peer:routable', 'dummy98:routable') - output = check_output('ip address show dev veth-peer') + print('### ip -6 address show dev veth-peer') + output = check_output('ip -6 address show dev veth-peer') print(output) self.assertIn('inet6 2001:db8:0:1:', output) self.assertNotIn('inet6 2001:db8:0:2:', output) + print('### ip -6 route show dev veth-peer') output = check_output('ip -6 route show dev veth-peer') print(output) self.assertIn('2001:db8:0:1::/64 proto ra', output) self.assertNotIn('2001:db8:0:2::/64 proto ra', output) - self.assertIn('2001:db0:fff::/64 via ', output) - self.assertNotIn('2001:db1:fff::/64 via ', output) + self.assertRegex(output, '2001:db0:fff::/64 via fe80::1034:56ff:fe78:9abc') + self.assertNotIn('2001:db1:fff::/64', output) - output = check_output('ip address show dev veth99') + print('### ip -6 address show dev veth99') + output = check_output('ip -6 address show dev veth99') print(output) self.assertNotIn('inet6 2001:db8:0:1:', output) self.assertIn('inet6 2001:db8:0:2:', output)