SymbX/systemd
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

network/ndisc: fix removal of unnecessary routes (#35128)

Browse Source 
Follow-up for 972f1d17ab.

This fixes the logic of removing unnecessary routes configured by the
previously received RAs. Previously, we wrongly handled existing routes
could be updated, and unexpected routes would be kept.
This commit is contained in:
Luca Boccassi
2024-11-13 14:06:21 +00:00
committed by GitHub
parent b4dc8b6415 d7b323c2dd
commit 4efc556211
4 changed files with 119 additions and 76 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/network/networkd-ndisc.c
+63 -30
View File
@@ -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)
src/network/networkd-route.c
+12
View File
@@ -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;
src/network/networkd-route.h
+1
View File
@@ -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);
test/test-network/systemd-networkd-tests.py
+43 -46
View File
@@ -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)