From e92f40c6b0b92dbde9f08f24ceb17f50249cf2c1 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 27 Jan 2025 05:15:48 +0900 Subject: [PATCH 1/6] network/route: adjust configuration source based on Gateway= setting If Gateway=_dhcp4/_ra, the route will be anyway configured with NETWORK_CONFIG_SOURCE_DHCP4/_NDISC. See dhcp4_request_route() and ndisc_route_prepare(). This is mostly for avoiding link_drop_routes(), which drops unnecessary static and/or foreign routes, unexpectedly filtering an existing route with the route specified with Gateway=_dhcp4/_ra. --- src/network/networkd-dhcp4.c | 6 ++---- src/network/networkd-ndisc.c | 10 ++++------ src/network/networkd-route-nexthop.c | 21 +++++++++++++++++---- src/network/networkd-route.c | 5 ++++- 4 files changed, 27 insertions(+), 15 deletions(-) diff --git a/src/network/networkd-dhcp4.c b/src/network/networkd-dhcp4.c index 30abef49a2..0dfb1737e8 100644 --- a/src/network/networkd-dhcp4.c +++ b/src/network/networkd-dhcp4.c @@ -643,13 +643,11 @@ static int dhcp4_request_semi_static_routes(Link *link) { _cleanup_(route_unrefp) Route *route = NULL; struct in_addr gw; - if (!rt->gateway_from_dhcp_or_ra) - continue; - - if (rt->nexthop.family != AF_INET) + if (rt->source != NETWORK_CONFIG_SOURCE_DHCP4) continue; assert(rt->family == AF_INET); + assert(rt->nexthop.family == AF_INET); r = dhcp4_find_gateway_for_destination(link, &rt->dst.in, rt->dst_prefixlen, &gw); if (r == -EHOSTUNREACH) { diff --git a/src/network/networkd-ndisc.c b/src/network/networkd-ndisc.c index 42bed0d642..9c579db445 100644 --- a/src/network/networkd-ndisc.c +++ b/src/network/networkd-ndisc.c @@ -1083,11 +1083,10 @@ static int ndisc_router_drop_default(Link *link, sd_ndisc_router *rt) { HASHMAP_FOREACH(route_gw, link->network->routes_by_section) { _cleanup_(route_unrefp) Route *tmp = NULL; - if (!route_gw->gateway_from_dhcp_or_ra) + if (route_gw->source != NETWORK_CONFIG_SOURCE_NDISC) continue; - if (route_gw->nexthop.family != AF_INET6) - continue; + assert(route_gw->nexthop.family == AF_INET6); r = route_dup(route_gw, NULL, &tmp); if (r < 0) @@ -1158,11 +1157,10 @@ static int ndisc_router_process_default(Link *link, sd_ndisc_router *rt) { HASHMAP_FOREACH(route_gw, link->network->routes_by_section) { _cleanup_(route_unrefp) Route *route = NULL; - if (!route_gw->gateway_from_dhcp_or_ra) + if (route_gw->source != NETWORK_CONFIG_SOURCE_NDISC) continue; - if (route_gw->nexthop.family != AF_INET6) - continue; + assert(route_gw->nexthop.family == AF_INET6); r = route_dup(route_gw, NULL, &route); if (r < 0) diff --git a/src/network/networkd-route-nexthop.c b/src/network/networkd-route-nexthop.c index bf0271807b..9482b25bb8 100644 --- a/src/network/networkd-route-nexthop.c +++ b/src/network/networkd-route-nexthop.c @@ -844,11 +844,24 @@ int route_section_verify_nexthops(Route *route) { return log_route_section(route, "Invalid route family."); } - if (route->nexthop.family == AF_INET && !FLAGS_SET(route->network->dhcp, ADDRESS_FAMILY_IPV4)) - return log_route_section(route, "Gateway=\"_dhcp4\" is specified but DHCPv4 client is disabled."); + switch (route->nexthop.family) { + case AF_INET: + if (!FLAGS_SET(route->network->dhcp, ADDRESS_FAMILY_IPV4)) + return log_route_section(route, "Gateway=\"_dhcp4\" is specified but DHCPv4 client is disabled."); - if (route->nexthop.family == AF_INET6 && route->network->ndisc == 0) - return log_route_section(route, "Gateway=\"_ipv6ra\" is specified but IPv6AcceptRA= is disabled."); + route->source = NETWORK_CONFIG_SOURCE_DHCP4; + break; + + case AF_INET6: + if (route->network->ndisc == 0) + return log_route_section(route, "Gateway=\"_ipv6ra\" is specified but IPv6AcceptRA= is disabled."); + + route->source = NETWORK_CONFIG_SOURCE_NDISC; + break; + + default: + assert_not_reached(); + } } /* When only Gateway= is specified, assume the route family based on the Gateway address. */ diff --git a/src/network/networkd-route.c b/src/network/networkd-route.c index f6386e0426..ff93c1d27d 100644 --- a/src/network/networkd-route.c +++ b/src/network/networkd-route.c @@ -1065,7 +1065,7 @@ int link_request_static_routes(Link *link, bool only_ipv4) { link->static_routes_configured = false; HASHMAP_FOREACH(route, link->network->routes_by_section) { - if (route->gateway_from_dhcp_or_ra) + if (route->source != NETWORK_CONFIG_SOURCE_STATIC) continue; if (only_ipv4 && route->family != AF_INET) @@ -1519,6 +1519,9 @@ int link_drop_routes(Link *link, bool only_static) { continue; HASHMAP_FOREACH(route, other->network->routes_by_section) { + if (route->source != NETWORK_CONFIG_SOURCE_STATIC) + continue; + if (route->family == AF_INET || ordered_set_isempty(route->nexthops)) { r = link_unmark_route(other, route, NULL); if (r < 0) From c9ee0d2eebab8fc4f692bf37484690af83bf1a20 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 27 Jan 2025 06:16:24 +0900 Subject: [PATCH 2/6] network/dhcp4: make dhcp4_request_route_to_gateway() take Route object No functional change, preparation for later commits. --- src/network/networkd-dhcp4.c | 39 ++++++++++++++++++++---------------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/src/network/networkd-dhcp4.c b/src/network/networkd-dhcp4.c index 0dfb1737e8..b42d2ffce0 100644 --- a/src/network/networkd-dhcp4.c +++ b/src/network/networkd-dhcp4.c @@ -438,14 +438,19 @@ static int dhcp4_request_prefix_route(Link *link) { return dhcp4_request_route(route, link); } -static int dhcp4_request_route_to_gateway(Link *link, const struct in_addr *gw) { +static int dhcp4_request_route_to_gateway(Link *link, const Route *rt) { _cleanup_(route_unrefp) Route *route = NULL; struct in_addr address; int r; assert(link); assert(link->dhcp_lease); - assert(gw); + assert(rt); + + if (in_addr_is_set(rt->nexthop.family, &rt->nexthop.gw) <= 0) + return 0; + + assert(rt->nexthop.family == AF_INET); r = sd_dhcp_lease_get_address(link->dhcp_lease, &address); if (r < 0) @@ -455,7 +460,7 @@ static int dhcp4_request_route_to_gateway(Link *link, const struct in_addr *gw) if (r < 0) return r; - route->dst.in = *gw; + route->dst.in = rt->nexthop.gw.in; route->dst_prefixlen = 32; route->prefsrc.in = address; route->scope = RT_SCOPE_LINK; @@ -526,14 +531,14 @@ static int dhcp4_request_route_auto( route->prefsrc.in = address; } else { - r = dhcp4_request_route_to_gateway(link, gw); - if (r < 0) - return r; - route->scope = RT_SCOPE_UNIVERSE; route->nexthop.family = AF_INET; route->nexthop.gw.in = *gw; route->prefsrc.in = address; + + r = dhcp4_request_route_to_gateway(link, route); + if (r < 0) + return r; } return dhcp4_request_route(route, link); @@ -613,12 +618,6 @@ static int dhcp4_request_default_gateway(Link *link) { if (r < 0) return r; - /* The dhcp netmask may mask out the gateway. First, add an explicit route for the gateway host - * so that we can route no matter the netmask or existing kernel route tables. */ - r = dhcp4_request_route_to_gateway(link, &router); - if (r < 0) - return r; - r = route_new(&route); if (r < 0) return r; @@ -628,6 +627,12 @@ static int dhcp4_request_default_gateway(Link *link) { route->nexthop.gw.in = router; route->prefsrc.in = address; + /* The dhcp netmask may mask out the gateway. First, add an explicit route for the gateway host + * so that we can route no matter the netmask or existing kernel route tables. */ + r = dhcp4_request_route_to_gateway(link, route); + if (r < 0) + return r; + return dhcp4_request_route(route, link); } @@ -664,16 +669,16 @@ static int dhcp4_request_semi_static_routes(Link *link) { continue; } - r = dhcp4_request_route_to_gateway(link, &gw); - if (r < 0) - return r; - r = route_dup(rt, NULL, &route); if (r < 0) return r; route->nexthop.gw.in = gw; + r = dhcp4_request_route_to_gateway(link, route); + if (r < 0) + return r; + r = dhcp4_request_route(route, link); if (r < 0) return r; From c7adb64e12574912cc0f0e096091941c348dacce Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 27 Jan 2025 06:58:06 +0900 Subject: [PATCH 3/6] network/dhcp4: rename link_prefixroute() -> prefixroute_by_kernel() No functional change, just refactoring. --- src/network/networkd-dhcp4.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/network/networkd-dhcp4.c b/src/network/networkd-dhcp4.c index b42d2ffce0..8790b61ee2 100644 --- a/src/network/networkd-dhcp4.c +++ b/src/network/networkd-dhcp4.c @@ -405,7 +405,7 @@ static int dhcp4_request_route(Route *route, Link *link) { return link_request_route(link, route, &link->dhcp4_messages, dhcp4_route_handler); } -static bool link_prefixroute(Link *link) { +static bool prefixroute_by_kernel(Link *link) { return !link->network->dhcp_route_table_set || link->network->dhcp_route_table == RT_TABLE_MAIN; } @@ -417,8 +417,8 @@ static int dhcp4_request_prefix_route(Link *link) { assert(link); assert(link->dhcp_lease); - if (link_prefixroute(link)) - /* When true, the route will be created by kernel. See dhcp4_update_address(). */ + if (prefixroute_by_kernel(link)) + /* The prefix route in the main table will be created by the kernel. See dhcp4_update_address(). */ return 0; r = route_new(&route); @@ -968,7 +968,7 @@ static int dhcp4_request_address(Link *link, bool announce) { r = sd_dhcp_lease_get_broadcast(link->dhcp_lease, &addr->broadcast); if (r < 0 && r != -ENODATA) return log_link_warning_errno(link, r, "DHCP: failed to get broadcast address: %m"); - SET_FLAG(addr->flags, IFA_F_NOPREFIXROUTE, !link_prefixroute(link)); + SET_FLAG(addr->flags, IFA_F_NOPREFIXROUTE, !prefixroute_by_kernel(link)); addr->route_metric = link->network->dhcp_route_metric; addr->duplicate_address_detection = link->network->dhcp_send_decline ? ADDRESS_FAMILY_IPV4 : ADDRESS_FAMILY_NO; From 863293dbd3116d5e1e157f7a8563d46565a285eb Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 27 Jan 2025 05:17:44 +0900 Subject: [PATCH 4/6] network/dhcp4: create prefix route and route to gateway in the specified table with Gateway=_dhcp4 Previously, the following setting ==== [Route] Gateway=_dhcp4 Table=100 ==== only configured the route in the specified table. But it was mostly useless. This makes prefix route and route to the gateway are also configured in the specified table. Before: ==== $ ip route show table 100 default via 192.168.0.1 dev eth0 proto dhcp metric 1024 ==== After: ==== $ ip route show table 100 default via 192.168.0.1 dev eth0 proto dhcp metric 1024 192.168.0.0/24 dev eth0 proto dhcp src 192.168.0.100 metric 1024 192.168.0.1 dev eth0 proto dhcp scope link src 192.168.0.100 metric 1024 ==== --- src/network/networkd-dhcp4.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/network/networkd-dhcp4.c b/src/network/networkd-dhcp4.c index 8790b61ee2..949f08cf53 100644 --- a/src/network/networkd-dhcp4.c +++ b/src/network/networkd-dhcp4.c @@ -410,14 +410,14 @@ static bool prefixroute_by_kernel(Link *link) { link->network->dhcp_route_table == RT_TABLE_MAIN; } -static int dhcp4_request_prefix_route(Link *link) { +static int dhcp4_request_prefix_route(Link *link, Route *rt) { _cleanup_(route_unrefp) Route *route = NULL; int r; assert(link); assert(link->dhcp_lease); - if (prefixroute_by_kernel(link)) + if (prefixroute_by_kernel(link) && (!rt || !rt->table_set || rt->table == RT_TABLE_MAIN)) /* The prefix route in the main table will be created by the kernel. See dhcp4_update_address(). */ return 0; @@ -426,6 +426,10 @@ static int dhcp4_request_prefix_route(Link *link) { return r; route->scope = RT_SCOPE_LINK; + if (rt) { + route->table_set = rt->table_set; + route->table = rt->table; + } r = sd_dhcp_lease_get_prefix(link->dhcp_lease, &route->dst.in, &route->dst_prefixlen); if (r < 0) @@ -464,6 +468,8 @@ static int dhcp4_request_route_to_gateway(Link *link, const Route *rt) { route->dst_prefixlen = 32; route->prefsrc.in = address; route->scope = RT_SCOPE_LINK; + route->table = rt->table; + route->table_set = rt->table_set; return dhcp4_request_route(route, link); } @@ -675,6 +681,10 @@ static int dhcp4_request_semi_static_routes(Link *link) { route->nexthop.gw.in = gw; + r = dhcp4_request_prefix_route(link, route); + if (r < 0) + return r; + r = dhcp4_request_route_to_gateway(link, route); if (r < 0) return r; @@ -778,7 +788,7 @@ static int dhcp4_request_routes(Link *link) { assert(link); assert(link->dhcp_lease); - r = dhcp4_request_prefix_route(link); + r = dhcp4_request_prefix_route(link, /* rt = */ NULL); if (r < 0) return log_link_error_errno(link, r, "DHCP error: Could not request prefix route: %m"); From 7befabafad9992744a186858b3a9c43aba9578a6 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 27 Jan 2025 07:07:55 +0900 Subject: [PATCH 5/6] network/dhcp4: Gateway=_dhcp4 also assign DHCP address as preferred source With the following, now preferred source address is set to the DHCP address. ==== [Route] Gatewa=_dhcp4 Table=100 ==== Before: ==== $ ip route show table 100 default default via 192.168.0.1 dev eth0 proto dhcp metric 1024 ==== After: ==== $ ip route show table 100 default default via 192.168.0.1 dev eth0 proto dhcp src 192.168.0.100 metric 1024 ==== To avoid the assignment, this also introduces PreferredSource=no. --- man/systemd.network.xml | 14 +++++++++----- src/network/networkd-dhcp4.c | 6 ++++++ src/network/networkd-route.c | 15 ++++++++++++++- src/network/networkd-route.h | 1 + 4 files changed, 30 insertions(+), 6 deletions(-) diff --git a/man/systemd.network.xml b/man/systemd.network.xml index 126accaca9..186c0250b2 100644 --- a/man/systemd.network.xml +++ b/man/systemd.network.xml @@ -2010,8 +2010,10 @@ NFTSet=prefix:netdev:filter:eth_ipv4_prefix Gateway= Takes the gateway address or the special values _dhcp4 and - _ipv6ra. If _dhcp4 or _ipv6ra is - set, then the gateway address provided by DHCPv4 or IPv6 RA is used. + _ipv6ra. If _dhcp4 or _ipv6ra is set, then + the gateway address provided by DHCPv4 or IPv6 RA is used. When_dhcp4, the + acquired DHCPv4 address will be used as the preferred source address of the route, unless it is + explicitly configured in PreferredSource=. @@ -2117,10 +2119,12 @@ NFTSet=prefix:netdev:filter:eth_ipv4_prefix PreferredSource= - The preferred source address of the route. The address must be in the format described - in + The preferred source address of the route. Takes no or an address + in the format described in inet_pton3. - + If Gateway=_dhcp4 is specified, defaults to the acquired DHCPv4 address. + Otherwise, defaults to unset. The value no may be useful to configure a route + with Gateway=_dhcp4 without setting preferred source route address. diff --git a/src/network/networkd-dhcp4.c b/src/network/networkd-dhcp4.c index 949f08cf53..926e769258 100644 --- a/src/network/networkd-dhcp4.c +++ b/src/network/networkd-dhcp4.c @@ -681,6 +681,12 @@ static int dhcp4_request_semi_static_routes(Link *link) { route->nexthop.gw.in = gw; + if (!route->prefsrc_set) { + r = sd_dhcp_lease_get_address(link->dhcp_lease, &route->prefsrc.in); + if (r < 0) + return r; + } + r = dhcp4_request_prefix_route(link, route); if (r < 0) return r; diff --git a/src/network/networkd-route.c b/src/network/networkd-route.c index ff93c1d27d..e41b4c31be 100644 --- a/src/network/networkd-route.c +++ b/src/network/networkd-route.c @@ -1663,7 +1663,19 @@ static int config_parse_preferred_src( Route *route = ASSERT_PTR(userdata); int r; - assert(rvalue); + if (isempty(rvalue)) { + route->prefsrc_set = false; + route->prefsrc = IN_ADDR_NULL; + return 1; + } + + r = parse_boolean(rvalue); + if (r == 0) { + /* Accepts only no. That prohibits prefsrc set by DHCP lease. */ + route->prefsrc_set = true; + route->prefsrc = IN_ADDR_NULL; + return 1; + } if (route->family == AF_UNSPEC) r = in_addr_from_string_auto(rvalue, &route->family, &route->prefsrc); @@ -1672,6 +1684,7 @@ static int config_parse_preferred_src( if (r < 0) return log_syntax_parse_error(unit, filename, line, r, lvalue, rvalue); + route->prefsrc_set = true; return 1; } diff --git a/src/network/networkd-route.h b/src/network/networkd-route.h index b9d2dbf9a6..46bbe2899e 100644 --- a/src/network/networkd-route.h +++ b/src/network/networkd-route.h @@ -71,6 +71,7 @@ struct Route { bool expiration_managed_by_kernel:1; /* RTA_CACHEINFO has nonzero rta_expires */ /* Only used by conf persers and route_section_verify(). */ + bool prefsrc_set:1; bool scope_set:1; bool table_set:1; bool priority_set:1; From 2ea15435fdd16e3ba48662ffa011dd20795eab97 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 27 Jan 2025 05:54:30 +0900 Subject: [PATCH 6/6] test-network: add test case for Gateway=_dhcp4 with Table= --- .../conf/25-dhcp-client-ipv4-only.network | 8 +++++-- test/test-network/systemd-networkd-tests.py | 23 +++++++++++++++++-- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/test/test-network/conf/25-dhcp-client-ipv4-only.network b/test/test-network/conf/25-dhcp-client-ipv4-only.network index 7d79ee3485..30a5fcc22c 100644 --- a/test/test-network/conf/25-dhcp-client-ipv4-only.network +++ b/test/test-network/conf/25-dhcp-client-ipv4-only.network @@ -43,5 +43,9 @@ Destination=192.168.7.0/24 [Route] Gateway=_dhcp4 -Destination=10.0.0.0/8 -Table=211 +Destination=192.0.2.0/24 + +[Route] +Gateway=_dhcp4 +Destination=198.51.100.0/24 +Table=212 diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index a151e00a2f..65b8b9e3c9 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -7121,7 +7121,14 @@ class NetworkdDHCPClientTests(unittest.TestCase, Utilities): self.assertRegex(output, f'192.168.5.1 proto dhcp scope link src {address1} metric 24') self.assertRegex(output, f'192.168.5.6 proto dhcp scope link src {address1} metric 24') self.assertRegex(output, f'192.168.5.7 proto dhcp scope link src {address1} metric 24') - self.assertIn('10.0.0.0/8 via 192.168.5.1 proto dhcp', output) + self.assertRegex(output, f'192.0.2.0/24 via 192.168.5.1 proto dhcp src {address1}') + + print('## ip route show table 212 dev veth99') + output = check_output('ip route show table 212 dev veth99') + print(output) + self.assertRegex(output, f'192.168.5.0/24 proto dhcp scope link src {address1} metric 24') + self.assertRegex(output, f'192.168.5.1 proto dhcp scope link src {address1} metric 24') + self.assertRegex(output, f'198.51.100.0/24 via 192.168.5.1 proto dhcp src {address1}') print('## link state file') output = read_link_state_file('veth99') @@ -7221,7 +7228,14 @@ class NetworkdDHCPClientTests(unittest.TestCase, Utilities): self.assertNotIn('192.168.5.6', output) self.assertRegex(output, f'192.168.5.7 proto dhcp scope link src {address2} metric 24') self.assertRegex(output, f'192.168.5.8 proto dhcp scope link src {address2} metric 24') - self.assertIn('10.0.0.0/8 via 192.168.5.1 proto dhcp', output) + self.assertRegex(output, f'192.0.2.0/24 via 192.168.5.1 proto dhcp src {address2}') + + print('## ip route show table 212 dev veth99') + output = check_output('ip route show table 212 dev veth99') + print(output) + self.assertRegex(output, f'192.168.5.0/24 proto dhcp scope link src {address2} metric 24') + self.assertRegex(output, f'192.168.5.1 proto dhcp scope link src {address2} metric 24') + self.assertRegex(output, f'198.51.100.0/24 via 192.168.5.1 proto dhcp src {address2}') print('## link state file') output = read_link_state_file('veth99') @@ -7292,6 +7306,11 @@ class NetworkdDHCPClientTests(unittest.TestCase, Utilities): print(output) self.assertNotIn(f'{address2}', output) + print('## ip route show table 212 dev veth99') + output = check_output('ip route show table 212 dev veth99') + print(output) + self.assertNotIn(f'{address2}', output) + self.teardown_nftset('addr4', 'network4', 'ifindex') def test_dhcp_client_ipv4_dbus_status(self):