From 45e76094abd69f7ed36408282777a636bfd26598 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 25 Jul 2023 02:36:25 +0900 Subject: [PATCH 01/12] in-addr-util: introduce in_addr_prefix_covers_full() and friends --- src/basic/in-addr-util.c | 25 +++++++++++++++++-------- src/basic/in-addr-util.h | 15 ++++++++++++--- 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/src/basic/in-addr-util.c b/src/basic/in-addr-util.c index 30d90cce0d7..ee4ea67ac63 100644 --- a/src/basic/in-addr-util.c +++ b/src/basic/in-addr-util.c @@ -727,10 +727,11 @@ int in_addr_mask(int family, union in_addr_union *addr, unsigned char prefixlen) } } -int in4_addr_prefix_covers( +int in4_addr_prefix_covers_full( const struct in_addr *prefix, unsigned char prefixlen, - const struct in_addr *address) { + const struct in_addr *address, + unsigned char address_prefixlen) { struct in_addr masked_prefix, masked_address; int r; @@ -738,6 +739,9 @@ int in4_addr_prefix_covers( assert(prefix); assert(address); + if (prefixlen > address_prefixlen) + return false; + masked_prefix = *prefix; r = in4_addr_mask(&masked_prefix, prefixlen); if (r < 0) @@ -751,10 +755,11 @@ int in4_addr_prefix_covers( return in4_addr_equal(&masked_prefix, &masked_address); } -int in6_addr_prefix_covers( +int in6_addr_prefix_covers_full( const struct in6_addr *prefix, unsigned char prefixlen, - const struct in6_addr *address) { + const struct in6_addr *address, + unsigned char address_prefixlen) { struct in6_addr masked_prefix, masked_address; int r; @@ -762,6 +767,9 @@ int in6_addr_prefix_covers( assert(prefix); assert(address); + if (prefixlen > address_prefixlen) + return false; + masked_prefix = *prefix; r = in6_addr_mask(&masked_prefix, prefixlen); if (r < 0) @@ -775,20 +783,21 @@ int in6_addr_prefix_covers( return in6_addr_equal(&masked_prefix, &masked_address); } -int in_addr_prefix_covers( +int in_addr_prefix_covers_full( int family, const union in_addr_union *prefix, unsigned char prefixlen, - const union in_addr_union *address) { + const union in_addr_union *address, + unsigned char address_prefixlen) { assert(prefix); assert(address); switch (family) { case AF_INET: - return in4_addr_prefix_covers(&prefix->in, prefixlen, &address->in); + return in4_addr_prefix_covers_full(&prefix->in, prefixlen, &address->in, address_prefixlen); case AF_INET6: - return in6_addr_prefix_covers(&prefix->in6, prefixlen, &address->in6); + return in6_addr_prefix_covers_full(&prefix->in6, prefixlen, &address->in6, address_prefixlen); default: return -EAFNOSUPPORT; } diff --git a/src/basic/in-addr-util.h b/src/basic/in-addr-util.h index 200b9eb69df..95e73162b13 100644 --- a/src/basic/in-addr-util.h +++ b/src/basic/in-addr-util.h @@ -144,9 +144,18 @@ int in4_addr_default_subnet_mask(const struct in_addr *addr, struct in_addr *mas int in4_addr_mask(struct in_addr *addr, unsigned char prefixlen); int in6_addr_mask(struct in6_addr *addr, unsigned char prefixlen); int in_addr_mask(int family, union in_addr_union *addr, unsigned char prefixlen); -int in4_addr_prefix_covers(const struct in_addr *prefix, unsigned char prefixlen, const struct in_addr *address); -int in6_addr_prefix_covers(const struct in6_addr *prefix, unsigned char prefixlen, const struct in6_addr *address); -int in_addr_prefix_covers(int family, const union in_addr_union *prefix, unsigned char prefixlen, const union in_addr_union *address); +int in4_addr_prefix_covers_full(const struct in_addr *prefix, unsigned char prefixlen, const struct in_addr *address, unsigned char address_prefixlen); +int in6_addr_prefix_covers_full(const struct in6_addr *prefix, unsigned char prefixlen, const struct in6_addr *address, unsigned char address_prefixlen); +int in_addr_prefix_covers_full(int family, const union in_addr_union *prefix, unsigned char prefixlen, const union in_addr_union *address, unsigned char address_prefixlen); +static inline int in4_addr_prefix_covers(const struct in_addr *prefix, unsigned char prefixlen, const struct in_addr *address) { + return in4_addr_prefix_covers_full(prefix, prefixlen, address, 32); +} +static inline int in6_addr_prefix_covers(const struct in6_addr *prefix, unsigned char prefixlen, const struct in6_addr *address) { + return in6_addr_prefix_covers_full(prefix, prefixlen, address, 128); +} +static inline int in_addr_prefix_covers(int family, const union in_addr_union *prefix, unsigned char prefixlen, const union in_addr_union *address) { + return in_addr_prefix_covers_full(family, prefix, prefixlen, address, family == AF_INET ? 32 : family == AF_INET6 ? 128 : 0); +} int in_addr_parse_prefixlen(int family, const char *p, unsigned char *ret); int in_addr_prefix_from_string(const char *p, int family, union in_addr_union *ret_prefix, unsigned char *ret_prefixlen); From 33acdc511c52c662ba8a42d851d1d8dde0f714f7 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 25 Jul 2023 01:57:10 +0900 Subject: [PATCH 02/12] sd-dhcp: introduce sd_dhcp_lease_get_prefix() --- src/libsystemd-network/sd-dhcp-lease.c | 28 ++++++++++++++++++++++++++ src/systemd/sd-dhcp-lease.h | 1 + 2 files changed, 29 insertions(+) diff --git a/src/libsystemd-network/sd-dhcp-lease.c b/src/libsystemd-network/sd-dhcp-lease.c index 03fd80064b7..5e8869a3b71 100644 --- a/src/libsystemd-network/sd-dhcp-lease.c +++ b/src/libsystemd-network/sd-dhcp-lease.c @@ -201,6 +201,34 @@ int sd_dhcp_lease_get_netmask(sd_dhcp_lease *lease, struct in_addr *addr) { return 0; } +int sd_dhcp_lease_get_prefix(sd_dhcp_lease *lease, struct in_addr *ret_prefix, uint8_t *ret_prefixlen) { + struct in_addr address, netmask; + uint8_t prefixlen; + int r; + + assert_return(lease, -EINVAL); + + r = sd_dhcp_lease_get_address(lease, &address); + if (r < 0) + return r; + + r = sd_dhcp_lease_get_netmask(lease, &netmask); + if (r < 0) + return r; + + prefixlen = in4_addr_netmask_to_prefixlen(&netmask); + + r = in4_addr_mask(&address, prefixlen); + if (r < 0) + return r; + + if (ret_prefix) + *ret_prefix = address; + if (ret_prefixlen) + *ret_prefixlen = prefixlen; + return 0; +} + int sd_dhcp_lease_get_server_identifier(sd_dhcp_lease *lease, struct in_addr *addr) { assert_return(lease, -EINVAL); assert_return(addr, -EINVAL); diff --git a/src/systemd/sd-dhcp-lease.h b/src/systemd/sd-dhcp-lease.h index a0e09e52aca..8a270595722 100644 --- a/src/systemd/sd-dhcp-lease.h +++ b/src/systemd/sd-dhcp-lease.h @@ -52,6 +52,7 @@ int sd_dhcp_lease_get_t1(sd_dhcp_lease *lease, uint32_t *t1); int sd_dhcp_lease_get_t2(sd_dhcp_lease *lease, uint32_t *t2); int sd_dhcp_lease_get_broadcast(sd_dhcp_lease *lease, struct in_addr *addr); int sd_dhcp_lease_get_netmask(sd_dhcp_lease *lease, struct in_addr *addr); +int sd_dhcp_lease_get_prefix(sd_dhcp_lease *lease, struct in_addr *ret_prefix, uint8_t *ret_prefixlen); int sd_dhcp_lease_get_router(sd_dhcp_lease *lease, const struct in_addr **addr); int sd_dhcp_lease_get_next_server(sd_dhcp_lease *lease, struct in_addr *addr); int sd_dhcp_lease_get_server_identifier(sd_dhcp_lease *lease, struct in_addr *addr); From 2faf2e2a42b7dc2b942fe45aa4643399eb2a00c7 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 25 Jul 2023 02:52:07 +0900 Subject: [PATCH 03/12] network/dhcp4: use sd_dhcp_lease_get_prefix() --- src/network/networkd-dhcp4.c | 28 +++++++++++----------------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/src/network/networkd-dhcp4.c b/src/network/networkd-dhcp4.c index 321e87bc7eb..b907f344fd2 100644 --- a/src/network/networkd-dhcp4.c +++ b/src/network/networkd-dhcp4.c @@ -220,7 +220,6 @@ static bool link_prefixroute(Link *link) { static int dhcp4_request_prefix_route(Link *link) { _cleanup_(route_freep) Route *route = NULL; - struct in_addr address, netmask; int r; assert(link); @@ -230,23 +229,20 @@ static int dhcp4_request_prefix_route(Link *link) { /* When true, the route will be created by kernel. See dhcp4_update_address(). */ return 0; - r = sd_dhcp_lease_get_address(link->dhcp_lease, &address); - if (r < 0) - return r; - - r = sd_dhcp_lease_get_netmask(link->dhcp_lease, &netmask); - if (r < 0) - return r; - r = route_new(&route); if (r < 0) return r; - route->dst.in.s_addr = address.s_addr & netmask.s_addr; - route->dst_prefixlen = in4_addr_netmask_to_prefixlen(&netmask); - route->prefsrc.in = address; route->scope = RT_SCOPE_LINK; + r = sd_dhcp_lease_get_prefix(link->dhcp_lease, &route->dst.in, &route->dst_prefixlen); + if (r < 0) + return r; + + r = sd_dhcp_lease_get_address(link->dhcp_lease, &route->prefsrc.in); + if (r < 0) + return r; + return dhcp4_request_route(TAKE_PTR(route), link); } @@ -800,8 +796,8 @@ static int dhcp4_address_handler(sd_netlink *rtnl, sd_netlink_message *m, Reques static int dhcp4_request_address(Link *link, bool announce) { _cleanup_(address_freep) Address *addr = NULL; - struct in_addr address, netmask, server; - unsigned prefixlen; + struct in_addr address, server; + uint8_t prefixlen; Address *existing; usec_t lifetime_usec; int r; @@ -815,7 +811,7 @@ static int dhcp4_request_address(Link *link, bool announce) { if (r < 0) return log_link_warning_errno(link, r, "DHCP error: no address: %m"); - r = sd_dhcp_lease_get_netmask(link->dhcp_lease, &netmask); + r = sd_dhcp_lease_get_prefix(link->dhcp_lease, NULL, &prefixlen); if (r < 0) return log_link_warning_errno(link, r, "DHCP error: no netmask: %m"); @@ -836,8 +832,6 @@ static int dhcp4_request_address(Link *link, bool announce) { } else lifetime_usec = USEC_INFINITY; - prefixlen = in4_addr_netmask_to_prefixlen(&netmask); - if (announce) { const struct in_addr *router; From a49cf370ac97ad6f5439968842f345c87bdc32a2 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 25 Jul 2023 03:26:31 +0900 Subject: [PATCH 04/12] network/dhcp4: honor received broadcast address --- src/network/networkd-dhcp4.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/network/networkd-dhcp4.c b/src/network/networkd-dhcp4.c index b907f344fd2..2622dddadb0 100644 --- a/src/network/networkd-dhcp4.c +++ b/src/network/networkd-dhcp4.c @@ -872,6 +872,9 @@ static int dhcp4_request_address(Link *link, bool announce) { addr->lifetime_preferred_usec = lifetime_usec; addr->lifetime_valid_usec = lifetime_usec; addr->prefixlen = prefixlen; + 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"); address_set_broadcast(addr, link); SET_FLAG(addr->flags, IFA_F_NOPREFIXROUTE, !link_prefixroute(link)); addr->route_metric = link->network->dhcp_route_metric; From 1953bcd14ac754668c34d25cf8f4c7ed4d131baf Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 24 Jul 2023 20:09:25 +0900 Subject: [PATCH 05/12] network/dhcp4: use FOREACH_ARRAY() macro --- src/network/networkd-dhcp4.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/network/networkd-dhcp4.c b/src/network/networkd-dhcp4.c index 2622dddadb0..3388a9dfb59 100644 --- a/src/network/networkd-dhcp4.c +++ b/src/network/networkd-dhcp4.c @@ -362,7 +362,7 @@ static int dhcp4_request_route_auto( static int dhcp4_request_static_routes(Link *link, struct in_addr *ret_default_gw) { _cleanup_free_ sd_dhcp_route **static_routes = NULL, **classless_routes = NULL; - size_t n_static_routes, n_classless_routes, n; + size_t n_static_routes, n_classless_routes, n_routes; struct in_addr default_gw = {}; sd_dhcp_route **routes; int r; @@ -402,25 +402,25 @@ static int dhcp4_request_static_routes(Link *link, struct in_addr *ret_default_g /* Even if UseRoutes=no, try to find default gateway to make semi-static routes and * routes to DNS or NTP servers can be configured in later steps. */ - for (size_t i = 0; i < n_classless_routes; i++) { + FOREACH_ARRAY(e, classless_routes, n_classless_routes) { struct in_addr dst; uint8_t prefixlen; - r = sd_dhcp_route_get_destination(classless_routes[i], &dst); + r = sd_dhcp_route_get_destination(*e, &dst); if (r < 0) return r; if (in4_addr_is_set(&dst)) continue; - r = sd_dhcp_route_get_destination_prefix_length(classless_routes[i], &prefixlen); + r = sd_dhcp_route_get_destination_prefix_length(*e, &prefixlen); if (r < 0) return r; if (prefixlen != 0) continue; - r = sd_dhcp_route_get_gateway(classless_routes[i], ret_default_gw); + r = sd_dhcp_route_get_gateway(*e, ret_default_gw); if (r < 0) return r; @@ -433,15 +433,15 @@ static int dhcp4_request_static_routes(Link *link, struct in_addr *ret_default_g } if (n_classless_routes > 0) { - n = n_classless_routes; + n_routes = n_classless_routes; routes = classless_routes; } else if (n_static_routes > 0){ - n = n_static_routes; + n_routes = n_static_routes; routes = static_routes; } else assert_not_reached(); - for (size_t i = 0; i < n; i++) { + FOREACH_ARRAY(e, routes, n_routes) { _cleanup_(route_freep) Route *route = NULL; struct in_addr gw; @@ -451,15 +451,15 @@ static int dhcp4_request_static_routes(Link *link, struct in_addr *ret_default_g route->gw_family = AF_INET; - r = sd_dhcp_route_get_gateway(routes[i], &gw); + r = sd_dhcp_route_get_gateway(*e, &gw); if (r < 0) return r; - r = sd_dhcp_route_get_destination(routes[i], &route->dst.in); + r = sd_dhcp_route_get_destination(*e, &route->dst.in); if (r < 0) return r; - r = sd_dhcp_route_get_destination_prefix_length(routes[i], &route->dst_prefixlen); + r = sd_dhcp_route_get_destination_prefix_length(*e, &route->dst_prefixlen); if (r < 0) return r; @@ -596,17 +596,17 @@ static int dhcp4_request_routes_to_servers( assert(servers || n_servers == 0); assert(gw); - for (size_t i = 0; i < n_servers; i++) { + FOREACH_ARRAY(dst, servers, n_servers) { _cleanup_(route_freep) Route *route = NULL; - if (in4_addr_is_null(&servers[i])) + if (in4_addr_is_null(dst)) continue; r = route_new(&route); if (r < 0) return r; - route->dst.in = servers[i]; + route->dst.in = *dst; route->dst_prefixlen = 32; r = dhcp4_request_route_auto(TAKE_PTR(route), link, gw, /* force_use_gw = */ false); From 5e6dad11b7462318cd8ac9626e53ef314c9c8f04 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 24 Jul 2023 20:12:54 +0900 Subject: [PATCH 06/12] network/dhcp4: drop unnecessary assignment It will be set in dhcp4_request_route_auto(). --- src/network/networkd-dhcp4.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/network/networkd-dhcp4.c b/src/network/networkd-dhcp4.c index 3388a9dfb59..3df447776fa 100644 --- a/src/network/networkd-dhcp4.c +++ b/src/network/networkd-dhcp4.c @@ -449,8 +449,6 @@ static int dhcp4_request_static_routes(Link *link, struct in_addr *ret_default_g if (r < 0) return r; - route->gw_family = AF_INET; - r = sd_dhcp_route_get_gateway(*e, &gw); if (r < 0) return r; From d52c5a72fefefdeb754264edde557801ce045e12 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 24 Jul 2023 19:45:58 +0900 Subject: [PATCH 07/12] network/dhcp4: introduce dhcp4_get_classless_static_or_static_routes() helper No functional changes, just refactoring and preparation for later commits. --- src/network/networkd-dhcp4.c | 85 ++++++++++++++++++++---------------- 1 file changed, 47 insertions(+), 38 deletions(-) diff --git a/src/network/networkd-dhcp4.c b/src/network/networkd-dhcp4.c index 3df447776fa..2f221fc0de4 100644 --- a/src/network/networkd-dhcp4.c +++ b/src/network/networkd-dhcp4.c @@ -52,6 +52,39 @@ void network_adjust_dhcp4(Network *network) { network->dhcp_client_identifier = network->dhcp_anonymize ? DHCP_CLIENT_ID_MAC : DHCP_CLIENT_ID_DUID; } +static int dhcp4_get_classless_static_or_static_routes(Link *link, sd_dhcp_route ***ret_routes, size_t *ret_num) { + _cleanup_free_ sd_dhcp_route **routes = NULL; + int r; + + assert(link); + assert(link->dhcp_lease); + + /* If the DHCP server returns both a Classless Static Routes option and a Static Routes option, + * the DHCP client MUST ignore the Static Routes option. */ + + r = sd_dhcp_lease_get_classless_routes(link->dhcp_lease, &routes); + if (r >= 0) { + assert(r > 0); + if (ret_routes) + *ret_routes = TAKE_PTR(routes); + if (ret_num) + *ret_num = r; + return 1; /* classless */ + } else if (r != -ENODATA) + return r; + + r = sd_dhcp_lease_get_static_routes(link->dhcp_lease, &routes); + if (r < 0) + return r; + + assert(r > 0); + if (ret_routes) + *ret_routes = TAKE_PTR(routes); + if (ret_num) + *ret_num = r; + return 0; /* static */ +} + static int dhcp4_remove_address_and_routes(Link *link, bool only_marked) { Address *address; Route *route; @@ -361,48 +394,33 @@ static int dhcp4_request_route_auto( } static int dhcp4_request_static_routes(Link *link, struct in_addr *ret_default_gw) { - _cleanup_free_ sd_dhcp_route **static_routes = NULL, **classless_routes = NULL; - size_t n_static_routes, n_classless_routes, n_routes; + _cleanup_free_ sd_dhcp_route **routes = NULL; + size_t n_routes; + bool is_classless; struct in_addr default_gw = {}; - sd_dhcp_route **routes; int r; assert(link); assert(link->dhcp_lease); assert(ret_default_gw); - r = sd_dhcp_lease_get_static_routes(link->dhcp_lease, &static_routes); + r = dhcp4_get_classless_static_or_static_routes(link, &routes, &n_routes); if (r == -ENODATA) - n_static_routes = 0; - else if (r < 0) - return r; - else - n_static_routes = r; - - r = sd_dhcp_lease_get_classless_routes(link->dhcp_lease, &classless_routes); - if (r == -ENODATA) - n_classless_routes = 0; - else if (r < 0) - return r; - else - n_classless_routes = r; - - if (n_classless_routes == 0 && n_static_routes == 0) { - log_link_debug(link, "DHCP: No static routes received from DHCP server."); return 0; - } + if (r < 0) + return r; - /* if the DHCP server returns both a Classless Static Routes option and a Static Routes option, - * the DHCP client MUST ignore the Static Routes option. */ - if (n_classless_routes > 0 && n_static_routes > 0) - log_link_debug(link, "Classless static routes received from DHCP server: ignoring static-route option"); + is_classless = r > 0; if (!link->network->dhcp_use_routes) { + if (!is_classless) + return 0; + /* Even if UseRoutes=no, try to find default gateway to make semi-static routes and * routes to DNS or NTP servers can be configured in later steps. */ - FOREACH_ARRAY(e, classless_routes, n_classless_routes) { + FOREACH_ARRAY(e, routes, n_routes) { struct in_addr dst; uint8_t prefixlen; @@ -432,15 +450,6 @@ static int dhcp4_request_static_routes(Link *link, struct in_addr *ret_default_g return 0; } - if (n_classless_routes > 0) { - n_routes = n_classless_routes; - routes = classless_routes; - } else if (n_static_routes > 0){ - n_routes = n_static_routes; - routes = static_routes; - } else - assert_not_reached(); - FOREACH_ARRAY(e, routes, n_routes) { _cleanup_(route_freep) Route *route = NULL; struct in_addr gw; @@ -464,20 +473,20 @@ static int dhcp4_request_static_routes(Link *link, struct in_addr *ret_default_g /* When classless static routes are provided, then router option will be ignored. To * use the default gateway later in other routes, e.g., routes to dns servers, here we * need to find the default gateway in the classless static routes. */ - if (n_classless_routes > 0 && + if (is_classless && in4_addr_is_null(&route->dst.in) && route->dst_prefixlen == 0 && in4_addr_is_null(&default_gw)) default_gw = gw; /* Do not ignore the gateway given by the classless route option even if the destination is * in the same network. See issue #28280. */ - r = dhcp4_request_route_auto(TAKE_PTR(route), link, &gw, /* force_use_gw = */ n_classless_routes > 0); + r = dhcp4_request_route_auto(TAKE_PTR(route), link, &gw, /* force_use_gw = */ is_classless); if (r < 0) return r; } *ret_default_gw = default_gw; - return n_classless_routes > 0; + return is_classless; } static int dhcp4_request_gateway(Link *link, struct in_addr *gw) { From 15aa7a5c3d141f618d7c6db48f16ecab6b53bd1a Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 24 Jul 2023 19:47:06 +0900 Subject: [PATCH 08/12] network/dhcp4: introduce dhcp4_get_router() helper function Previously, we use the first router address, and if it is null, we ignore the address. Now, we use the first non-null address. That is, if the first router address is null, but the second is not, then we use the second one. That should not cause functional change in most cases, except for the case when a DHCP server provides such spurious reply. This is mostly for refactoring and preparation for later commits. --- src/network/networkd-dhcp4.c | 46 ++++++++++++++++++++++++++---------- 1 file changed, 33 insertions(+), 13 deletions(-) diff --git a/src/network/networkd-dhcp4.c b/src/network/networkd-dhcp4.c index 2f221fc0de4..3880960722a 100644 --- a/src/network/networkd-dhcp4.c +++ b/src/network/networkd-dhcp4.c @@ -52,6 +52,31 @@ void network_adjust_dhcp4(Network *network) { network->dhcp_client_identifier = network->dhcp_anonymize ? DHCP_CLIENT_ID_MAC : DHCP_CLIENT_ID_DUID; } +static int dhcp4_get_router(Link *link, struct in_addr *ret) { + const struct in_addr *routers; + int r; + + assert(link); + assert(link->dhcp_lease); + assert(ret); + + r = sd_dhcp_lease_get_router(link->dhcp_lease, &routers); + if (r < 0) + return r; + + /* The router option may provide multiple routers, We only use the first non-null address. */ + + FOREACH_ARRAY(router, routers, r) { + if (in4_addr_is_null(router)) + continue; + + *ret = *router; + return 0; + } + + return -ENODATA; +} + static int dhcp4_get_classless_static_or_static_routes(Link *link, sd_dhcp_route ***ret_routes, size_t *ret_num) { _cleanup_free_ sd_dhcp_route **routes = NULL; int r; @@ -491,8 +516,7 @@ static int dhcp4_request_static_routes(Link *link, struct in_addr *ret_default_g static int dhcp4_request_gateway(Link *link, struct in_addr *gw) { _cleanup_(route_freep) Route *route = NULL; - const struct in_addr *router; - struct in_addr address; + struct in_addr address, router; int r; assert(link); @@ -503,17 +527,13 @@ static int dhcp4_request_gateway(Link *link, struct in_addr *gw) { if (r < 0) return r; - r = sd_dhcp_lease_get_router(link->dhcp_lease, &router); - if (IN_SET(r, 0, -ENODATA)) { - log_link_debug(link, "DHCP: No gateway received from DHCP server."); + r = dhcp4_get_router(link, &router); + if (r == -ENODATA) { + log_link_debug(link, "DHCP: No valid router address received from DHCP server."); return 0; } if (r < 0) return r; - if (in4_addr_is_null(&router[0])) { - log_link_debug(link, "DHCP: Received gateway address is null."); - return 0; - } if (!link->network->dhcp_use_gateway) { /* When no classless static route is provided, even if UseGateway=no, use the gateway @@ -521,13 +541,13 @@ static int dhcp4_request_gateway(Link *link, struct in_addr *gw) { * neither UseRoutes= nor UseGateway= is disabled, use the default gateway in classless * static routes if provided (in that case, in4_addr_is_null(gw) below is true). */ if (in4_addr_is_null(gw)) - *gw = router[0]; + *gw = router; return 0; } /* 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[0]); + r = dhcp4_request_route_to_gateway(link, &router); if (r < 0) return r; @@ -537,7 +557,7 @@ static int dhcp4_request_gateway(Link *link, struct in_addr *gw) { /* Next, add a default gateway. */ route->gw_family = AF_INET; - route->gw.in = router[0]; + route->gw.in = router; route->prefsrc.in = address; r = dhcp4_request_route(TAKE_PTR(route), link); @@ -546,7 +566,7 @@ static int dhcp4_request_gateway(Link *link, struct in_addr *gw) { /* When no classless static route is provided, or UseRoutes=no, then use the router address to * configure semi-static routes and routes to DNS or NTP servers in later steps. */ - *gw = router[0]; + *gw = router; return 0; } From 0bc8b6e265e182830558a114a5b6bce349283d3a Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 25 Jul 2023 03:12:17 +0900 Subject: [PATCH 09/12] network/dhcp4: introduce dhcp4_prefix_covers() helper function No functional change, just refactoring. --- src/network/networkd-dhcp4.c | 34 +++++++++++++++++++++++++++------- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/src/network/networkd-dhcp4.c b/src/network/networkd-dhcp4.c index 3880960722a..8a76da63c89 100644 --- a/src/network/networkd-dhcp4.c +++ b/src/network/networkd-dhcp4.c @@ -52,6 +52,30 @@ void network_adjust_dhcp4(Network *network) { network->dhcp_client_identifier = network->dhcp_anonymize ? DHCP_CLIENT_ID_MAC : DHCP_CLIENT_ID_DUID; } +static int dhcp4_prefix_covers( + Link *link, + const struct in_addr *in_prefix, + uint8_t in_prefixlen) { + + struct in_addr prefix; + uint8_t prefixlen; + int r; + + assert(link); + assert(link->dhcp_lease); + assert(in_prefix); + + /* Return true if the input address or address range is in the assigned network. + * E.g. if the DHCP server provides 192.168.0.100/24, then this returns true for the address or + * address range in 192.168.0.0/24, and returns false otherwise. */ + + r = sd_dhcp_lease_get_prefix(link->dhcp_lease, &prefix, &prefixlen); + if (r < 0) + return r; + + return in4_addr_prefix_covers_full(&prefix, prefixlen, in_prefix, in_prefixlen); +} + static int dhcp4_get_router(Link *link, struct in_addr *ret) { const struct in_addr *routers; int r; @@ -336,7 +360,7 @@ static int dhcp4_request_route_auto( bool force_use_gw) { _cleanup_(route_freep) Route *route = in; - struct in_addr address, netmask, prefix; + struct in_addr address, prefix; uint8_t prefixlen; int r; @@ -349,13 +373,10 @@ static int dhcp4_request_route_auto( if (r < 0) return r; - r = sd_dhcp_lease_get_netmask(link->dhcp_lease, &netmask); + r = sd_dhcp_lease_get_prefix(link->dhcp_lease, &prefix, &prefixlen); if (r < 0) return r; - prefix.s_addr = address.s_addr & netmask.s_addr; - prefixlen = in4_addr_netmask_to_prefixlen(&netmask); - if (in4_addr_is_localhost(&route->dst.in)) { if (in4_addr_is_set(gw)) log_link_debug(link, "DHCP: requested route destination "IPV4_ADDRESS_FMT_STR"/%u is localhost, " @@ -379,8 +400,7 @@ static int dhcp4_request_route_auto( route->prefsrc.in = address; } else if (!force_use_gw && - route->dst_prefixlen >= prefixlen && - (route->dst.in.s_addr & netmask.s_addr) == prefix.s_addr) { + dhcp4_prefix_covers(link, &route->dst.in, route->dst_prefixlen) > 0) { if (in4_addr_is_set(gw)) log_link_debug(link, "DHCP: requested route destination "IPV4_ADDRESS_FMT_STR"/%u is in the assigned network " IPV4_ADDRESS_FMT_STR"/%u, ignoring gateway address "IPV4_ADDRESS_FMT_STR, From 0ce86f5eeb0921b44a9782260a8c88aafb15ffde Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 24 Jul 2023 20:40:04 +0900 Subject: [PATCH 10/12] network/dhcp4: always find suitable gateway for destination address And if not found, refuse to configure the route. If a DHCP server provides classless static or static routes, then we should use the gateway for accessing a node in the range specified in the route. E.g. if a DHCP server provides the default gateway is 192.168.0.1, and classless static route for 8.0.0.0/8 with gateway 192.168.0.2, then we should access 8.8.8.8 through 192.168.0.2 rather than 192.168.0.1, but should use 192.168.0.1 for 9.9.9.9. Fixes #28358. --- src/network/networkd-dhcp4.c | 152 +++++++++++++++++--- test/test-network/systemd-networkd-tests.py | 10 +- 2 files changed, 139 insertions(+), 23 deletions(-) diff --git a/src/network/networkd-dhcp4.c b/src/network/networkd-dhcp4.c index 8a76da63c89..ca93c6773f0 100644 --- a/src/network/networkd-dhcp4.c +++ b/src/network/networkd-dhcp4.c @@ -134,6 +134,104 @@ static int dhcp4_get_classless_static_or_static_routes(Link *link, sd_dhcp_route return 0; /* static */ } +static int dhcp4_find_gateway_for_destination( + Link *link, + const struct in_addr *destination, + uint8_t prefixlength, + bool allow_null, + struct in_addr *ret) { + + _cleanup_free_ sd_dhcp_route **routes = NULL; + size_t n_routes = 0; + bool is_classless, reachable; + uint8_t max_prefixlen = UINT8_MAX; + struct in_addr gw; + int r; + + assert(link); + assert(link->dhcp_lease); + assert(destination); + assert(ret); + + /* This tries to find the most suitable gateway for an address or address range. + * E.g. if the server provides the default gateway 192.168.0.1 and a classless static route for + * 8.0.0.0/8 with gateway 192.168.0.2, then this returns 192.168.0.2 for 8.8.8.8/32, and 192.168.0.1 + * for 9.9.9.9/32. If 'allow_null' flag is set, and the input address or address range is in the + * assigned network, then the default gateway will be ignored and the null address will be returned + * unless a matching non-default gateway found. */ + + r = dhcp4_prefix_covers(link, destination, prefixlength); + if (r < 0) + return r; + reachable = r > 0; + + r = dhcp4_get_classless_static_or_static_routes(link, &routes, &n_routes); + if (r < 0 && r != -ENODATA) + return r; + is_classless = r > 0; + + /* First, find most suitable gateway. */ + FOREACH_ARRAY(e, routes, n_routes) { + struct in_addr dst; + uint8_t len; + + r = sd_dhcp_route_get_destination(*e, &dst); + if (r < 0) + return r; + + r = sd_dhcp_route_get_destination_prefix_length(*e, &len); + if (r < 0) + return r; + + r = in4_addr_prefix_covers_full(&dst, len, destination, prefixlength); + if (r < 0) + return r; + if (r == 0) + continue; + + if (max_prefixlen != UINT8_MAX && max_prefixlen > len) + continue; + + r = sd_dhcp_route_get_gateway(*e, &gw); + if (r < 0) + return r; + + max_prefixlen = len; + } + + /* Found a suitable gateway in classless static routes or static routes. */ + if (max_prefixlen != UINT8_MAX) { + if (max_prefixlen == 0 && reachable && allow_null) + /* Do not return the default gateway, if the destination is in the assigned network. */ + *ret = (struct in_addr) {}; + else + *ret = gw; + return 0; + } + + /* When the destination is in the assigned network, return the null address if allowed. */ + if (reachable && allow_null) { + *ret = (struct in_addr) {}; + return 0; + } + + /* According to RFC 3442: If the DHCP server returns both a Classless Static Routes option and + * a Router option, the DHCP client MUST ignore the Router option. */ + if (!is_classless) { + r = dhcp4_get_router(link, ret); + if (r >= 0) + return 0; + if (r != -ENODATA) + return r; + } + + if (!reachable) + return -EHOSTUNREACH; /* Not in the same network, cannot reach the destination. */ + + assert(!allow_null); + return -ENODATA; /* No matching gateway found. */ +} + static int dhcp4_remove_address_and_routes(Link *link, bool only_marked) { Address *address; Route *route; @@ -590,20 +688,17 @@ static int dhcp4_request_gateway(Link *link, struct in_addr *gw) { return 0; } -static int dhcp4_request_semi_static_routes(Link *link, const struct in_addr *gw) { +static int dhcp4_request_semi_static_routes(Link *link) { Route *rt; int r; assert(link); assert(link->dhcp_lease); assert(link->network); - assert(gw); - - if (in4_addr_is_null(gw)) - return 0; HASHMAP_FOREACH(rt, link->network->routes_by_section) { _cleanup_(route_freep) Route *route = NULL; + struct in_addr gw; if (!rt->gateway_from_dhcp_or_ra) continue; @@ -611,7 +706,18 @@ static int dhcp4_request_semi_static_routes(Link *link, const struct in_addr *gw if (rt->gw_family != AF_INET) continue; - r = dhcp4_request_route_to_gateway(link, gw); + assert(rt->family == AF_INET); + + r = dhcp4_find_gateway_for_destination(link, &rt->dst.in, rt->dst_prefixlen, /* allow_null = */ false, &gw); + if (IN_SET(r, -EHOSTUNREACH, -ENODATA)) { + log_link_debug_errno(link, r, "DHCP: Cannot find suitable gateway for destination %s of semi-static route, ignoring: %m", + IN4_ADDR_PREFIX_TO_STRING(&rt->dst.in, rt->dst_prefixlen)); + continue; + } + if (r < 0) + return r; + + r = dhcp4_request_route_to_gateway(link, &gw); if (r < 0) return r; @@ -619,7 +725,7 @@ static int dhcp4_request_semi_static_routes(Link *link, const struct in_addr *gw if (r < 0) return r; - route->gw.in = *gw; + route->gw.in = gw; r = dhcp4_request_route(TAKE_PTR(route), link); if (r < 0) @@ -632,8 +738,7 @@ static int dhcp4_request_semi_static_routes(Link *link, const struct in_addr *gw static int dhcp4_request_routes_to_servers( Link *link, const struct in_addr *servers, - size_t n_servers, - const struct in_addr *gw) { + size_t n_servers) { int r; @@ -641,14 +746,23 @@ static int dhcp4_request_routes_to_servers( assert(link->dhcp_lease); assert(link->network); assert(servers || n_servers == 0); - assert(gw); FOREACH_ARRAY(dst, servers, n_servers) { _cleanup_(route_freep) Route *route = NULL; + struct in_addr gw; if (in4_addr_is_null(dst)) continue; + r = dhcp4_find_gateway_for_destination(link, dst, 32, /* allow_null = */ true, &gw); + if (r == -EHOSTUNREACH) { + log_link_debug_errno(link, r, "DHCP: Cannot find suitable gateway for destination %s, ignoring: %m", + IN4_ADDR_PREFIX_TO_STRING(dst, 32)); + continue; + } + if (r < 0) + return r; + r = route_new(&route); if (r < 0) return r; @@ -656,7 +770,7 @@ static int dhcp4_request_routes_to_servers( route->dst.in = *dst; route->dst_prefixlen = 32; - r = dhcp4_request_route_auto(TAKE_PTR(route), link, gw, /* force_use_gw = */ false); + r = dhcp4_request_route_auto(TAKE_PTR(route), link, &gw, /* force_use_gw = */ false); if (r < 0) return r; } @@ -664,14 +778,13 @@ static int dhcp4_request_routes_to_servers( return 0; } -static int dhcp4_request_routes_to_dns(Link *link, const struct in_addr *gw) { +static int dhcp4_request_routes_to_dns(Link *link) { const struct in_addr *dns; int r; assert(link); assert(link->dhcp_lease); assert(link->network); - assert(gw); if (!link->network->dhcp_use_dns || !link->network->dhcp_routes_to_dns) @@ -683,17 +796,16 @@ static int dhcp4_request_routes_to_dns(Link *link, const struct in_addr *gw) { if (r < 0) return r; - return dhcp4_request_routes_to_servers(link, dns, r, gw); + return dhcp4_request_routes_to_servers(link, dns, r); } -static int dhcp4_request_routes_to_ntp(Link *link, const struct in_addr *gw) { +static int dhcp4_request_routes_to_ntp(Link *link) { const struct in_addr *ntp; int r; assert(link); assert(link->dhcp_lease); assert(link->network); - assert(gw); if (!link->network->dhcp_use_ntp || !link->network->dhcp_routes_to_ntp) @@ -705,7 +817,7 @@ static int dhcp4_request_routes_to_ntp(Link *link, const struct in_addr *gw) { if (r < 0) return r; - return dhcp4_request_routes_to_servers(link, ntp, r, gw); + return dhcp4_request_routes_to_servers(link, ntp, r); } static int dhcp4_request_routes(Link *link) { @@ -730,15 +842,15 @@ static int dhcp4_request_routes(Link *link) { return log_link_error_errno(link, r, "DHCP error: Could not request gateway: %m"); } - r = dhcp4_request_semi_static_routes(link, &gw); + r = dhcp4_request_semi_static_routes(link); if (r < 0) return log_link_error_errno(link, r, "DHCP error: Could not request routes with Gateway=_dhcp4 setting: %m"); - r = dhcp4_request_routes_to_dns(link, &gw); + r = dhcp4_request_routes_to_dns(link); if (r < 0) return log_link_error_errno(link, r, "DHCP error: Could not request routes to DNS servers: %m"); - r = dhcp4_request_routes_to_ntp(link, &gw); + r = dhcp4_request_routes_to_ntp(link); if (r < 0) return log_link_error_errno(link, r, "DHCP error: Could not request routes to NTP servers: %m"); diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index 01336bc1751..f2e2279124c 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -5042,9 +5042,13 @@ class NetworkdDHCPClientTests(unittest.TestCase, Utilities): if dns_and_ntp_routes: self.assertRegex(output, r'192.168.5.10 proto dhcp scope link src 192.168.5.[0-9]* metric 1024') self.assertRegex(output, r'192.168.5.11 proto dhcp scope link src 192.168.5.[0-9]* metric 1024') - if classless and use_routes: - self.assertRegex(output, r'8.8.8.8 via 192.168.5.4 proto dhcp src 192.168.5.[0-9]* metric 1024') - self.assertRegex(output, r'9.9.9.9 via 192.168.5.4 proto dhcp src 192.168.5.[0-9]* metric 1024') + if use_routes: + if classless: + self.assertRegex(output, r'8.8.8.8 via 192.168.5.5 proto dhcp src 192.168.5.[0-9]* metric 1024') + self.assertRegex(output, r'9.9.9.9 via 192.168.5.4 proto dhcp src 192.168.5.[0-9]* metric 1024') + else: + self.assertRegex(output, r'8.8.8.8 via 192.168.5.3 proto dhcp src 192.168.5.[0-9]* metric 1024') + self.assertRegex(output, r'9.9.9.9 via 192.168.5.1 proto dhcp src 192.168.5.[0-9]* metric 1024') else: self.assertRegex(output, r'8.8.8.8 via 192.168.5.1 proto dhcp src 192.168.5.[0-9]* metric 1024') self.assertRegex(output, r'9.9.9.9 via 192.168.5.1 proto dhcp src 192.168.5.[0-9]* metric 1024') From 4001653ddb2067978af3c0aff59e41019559cd57 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 24 Jul 2023 23:34:18 +0900 Subject: [PATCH 11/12] network/dhcp4: always honor specified gateway address Follow-up for 77451f654a89d822cd288883edfac315949d1cb6. Now, gateway for routes to DNS or NTP servers should be correctly picked, hence it is not necessary to adjust the gateway address in dhcp4_request_route_auto() again. Also, similar for classless static routes, let's always honor gateway address specified in (non-classless) static routes. --- src/network/networkd-dhcp4.c | 50 +++++++++------------ test/test-network/systemd-networkd-tests.py | 8 ++-- 2 files changed, 25 insertions(+), 33 deletions(-) diff --git a/src/network/networkd-dhcp4.c b/src/network/networkd-dhcp4.c index ca93c6773f0..dedc0a00151 100644 --- a/src/network/networkd-dhcp4.c +++ b/src/network/networkd-dhcp4.c @@ -454,12 +454,10 @@ static int dhcp4_request_route_to_gateway(Link *link, const struct in_addr *gw) static int dhcp4_request_route_auto( Route *in, Link *link, - const struct in_addr *gw, - bool force_use_gw) { + const struct in_addr *gw) { _cleanup_(route_freep) Route *route = in; - struct in_addr address, prefix; - uint8_t prefixlen; + struct in_addr address; int r; assert(route); @@ -471,10 +469,6 @@ static int dhcp4_request_route_auto( if (r < 0) return r; - r = sd_dhcp_lease_get_prefix(link->dhcp_lease, &prefix, &prefixlen); - if (r < 0) - return r; - if (in4_addr_is_localhost(&route->dst.in)) { if (in4_addr_is_set(gw)) log_link_debug(link, "DHCP: requested route destination "IPV4_ADDRESS_FMT_STR"/%u is localhost, " @@ -497,25 +491,23 @@ static int dhcp4_request_route_auto( route->gw = IN_ADDR_NULL; route->prefsrc.in = address; - } else if (!force_use_gw && - dhcp4_prefix_covers(link, &route->dst.in, route->dst_prefixlen) > 0) { - if (in4_addr_is_set(gw)) - log_link_debug(link, "DHCP: requested route destination "IPV4_ADDRESS_FMT_STR"/%u is in the assigned network " - IPV4_ADDRESS_FMT_STR"/%u, ignoring gateway address "IPV4_ADDRESS_FMT_STR, - IPV4_ADDRESS_FMT_VAL(route->dst.in), route->dst_prefixlen, - IPV4_ADDRESS_FMT_VAL(prefix), prefixlen, - IPV4_ADDRESS_FMT_VAL(*gw)); - - route->scope = RT_SCOPE_LINK; - route->gw_family = AF_UNSPEC; - route->gw = IN_ADDR_NULL; - route->prefsrc.in = address; - } else if (in4_addr_is_null(gw)) { - log_link_debug(link, "DHCP: requested route destination "IPV4_ADDRESS_FMT_STR"/%u is not in the assigned network " - IPV4_ADDRESS_FMT_STR"/%u, but no gateway is specified, using 'link' scope.", - IPV4_ADDRESS_FMT_VAL(route->dst.in), route->dst_prefixlen, - IPV4_ADDRESS_FMT_VAL(prefix), prefixlen); + r = dhcp4_prefix_covers(link, &route->dst.in, route->dst_prefixlen); + if (r < 0) + return r; + if (r == 0 && DEBUG_LOGGING) { + struct in_addr prefix; + uint8_t prefixlen; + + r = sd_dhcp_lease_get_prefix(link->dhcp_lease, &prefix, &prefixlen); + if (r < 0) + return r; + + log_link_debug(link, "DHCP: requested route destination "IPV4_ADDRESS_FMT_STR"/%u is not in the assigned network " + IPV4_ADDRESS_FMT_STR"/%u, but no gateway is specified, using 'link' scope.", + IPV4_ADDRESS_FMT_VAL(route->dst.in), route->dst_prefixlen, + IPV4_ADDRESS_FMT_VAL(prefix), prefixlen); + } route->scope = RT_SCOPE_LINK; route->gw_family = AF_UNSPEC; @@ -621,9 +613,7 @@ static int dhcp4_request_static_routes(Link *link, struct in_addr *ret_default_g in4_addr_is_null(&default_gw)) default_gw = gw; - /* Do not ignore the gateway given by the classless route option even if the destination is - * in the same network. See issue #28280. */ - r = dhcp4_request_route_auto(TAKE_PTR(route), link, &gw, /* force_use_gw = */ is_classless); + r = dhcp4_request_route_auto(TAKE_PTR(route), link, &gw); if (r < 0) return r; } @@ -770,7 +760,7 @@ static int dhcp4_request_routes_to_servers( route->dst.in = *dst; route->dst_prefixlen = 32; - r = dhcp4_request_route_auto(TAKE_PTR(route), link, &gw, /* force_use_gw = */ false); + r = dhcp4_request_route_auto(TAKE_PTR(route), link, &gw); if (r < 0) return r; } diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index f2e2279124c..766abab819b 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -4993,7 +4993,7 @@ class NetworkdDHCPClientTests(unittest.TestCase, Utilities): additional_options = [ '--dhcp-option=option:dns-server,192.168.5.10,8.8.8.8', '--dhcp-option=option:ntp-server,192.168.5.11,9.9.9.9', - '--dhcp-option=option:static-route,192.168.5.100,192.168.5.2,8.8.8.8,192.168.5.3' + '--dhcp-option=option:static-route,192.168.6.100,192.168.5.2,8.8.8.8,192.168.5.3' ] if classless: additional_options += [ @@ -5014,16 +5014,18 @@ class NetworkdDHCPClientTests(unittest.TestCase, Utilities): self.assertRegex(output, r'192.168.5.4 proto dhcp scope link src 192.168.5.[0-9]* metric 1024') self.assertRegex(output, r'192.168.5.5 proto dhcp scope link src 192.168.5.[0-9]* metric 1024') else: - self.assertRegex(output, r'192.168.5.0/24 proto dhcp scope link src 192.168.5.[0-9]* metric 1024') + self.assertRegex(output, r'192.168.6.0/24 via 192.168.5.2 proto dhcp src 192.168.5.[0-9]* metric 1024') self.assertRegex(output, r'8.0.0.0/8 via 192.168.5.3 proto dhcp src 192.168.5.[0-9]* metric 1024') + self.assertRegex(output, r'192.168.5.2 proto dhcp scope link src 192.168.5.[0-9]* metric 1024') self.assertRegex(output, r'192.168.5.3 proto dhcp scope link src 192.168.5.[0-9]* metric 1024') else: self.assertNotRegex(output, r'default via 192.168.5.4 proto dhcp src 192.168.5.[0-9]* metric 1024') self.assertNotRegex(output, r'8.0.0.0/8 via 192.168.5.5 proto dhcp src 192.168.5.[0-9]* metric 1024') self.assertNotRegex(output, r'192.168.5.4 proto dhcp scope link src 192.168.5.[0-9]* metric 1024') self.assertNotRegex(output, r'192.168.5.5 proto dhcp scope link src 192.168.5.[0-9]* metric 1024') - self.assertNotRegex(output, r'192.168.5.0/24 proto dhcp scope link src 192.168.5.[0-9]* metric 1024') + self.assertNotRegex(output, r'192.168.6.0/24 via 192.168.5.2 proto dhcp src 192.168.5.[0-9]* metric 1024') self.assertNotRegex(output, r'8.0.0.0/8 via 192.168.5.3 proto dhcp src 192.168.5.[0-9]* metric 1024') + self.assertNotRegex(output, r'192.168.5.2 proto dhcp scope link src 192.168.5.[0-9]* metric 1024') self.assertNotRegex(output, r'192.168.5.3 proto dhcp scope link src 192.168.5.[0-9]* metric 1024') # Check UseGateway= From a56347db7dde1ae42df3cdd92f15873422c56512 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 24 Jul 2023 19:51:21 +0900 Subject: [PATCH 12/12] network/dhcp4: drop unused logic of finding default gateway --- src/network/networkd-dhcp4.c | 104 +++++++---------------------------- 1 file changed, 21 insertions(+), 83 deletions(-) diff --git a/src/network/networkd-dhcp4.c b/src/network/networkd-dhcp4.c index dedc0a00151..5614330384c 100644 --- a/src/network/networkd-dhcp4.c +++ b/src/network/networkd-dhcp4.c @@ -528,16 +528,16 @@ static int dhcp4_request_route_auto( return dhcp4_request_route(TAKE_PTR(route), link); } -static int dhcp4_request_static_routes(Link *link, struct in_addr *ret_default_gw) { +static int dhcp4_request_classless_static_or_static_routes(Link *link) { _cleanup_free_ sd_dhcp_route **routes = NULL; size_t n_routes; - bool is_classless; - struct in_addr default_gw = {}; int r; assert(link); assert(link->dhcp_lease); - assert(ret_default_gw); + + if (!link->network->dhcp_use_routes) + return 0; r = dhcp4_get_classless_static_or_static_routes(link, &routes, &n_routes); if (r == -ENODATA) @@ -545,46 +545,6 @@ static int dhcp4_request_static_routes(Link *link, struct in_addr *ret_default_g if (r < 0) return r; - is_classless = r > 0; - - if (!link->network->dhcp_use_routes) { - - if (!is_classless) - return 0; - - /* Even if UseRoutes=no, try to find default gateway to make semi-static routes and - * routes to DNS or NTP servers can be configured in later steps. */ - - FOREACH_ARRAY(e, routes, n_routes) { - struct in_addr dst; - uint8_t prefixlen; - - r = sd_dhcp_route_get_destination(*e, &dst); - if (r < 0) - return r; - - if (in4_addr_is_set(&dst)) - continue; - - r = sd_dhcp_route_get_destination_prefix_length(*e, &prefixlen); - if (r < 0) - return r; - - if (prefixlen != 0) - continue; - - r = sd_dhcp_route_get_gateway(*e, ret_default_gw); - if (r < 0) - return r; - - break; - } - - /* Do not return 1 here, to ensure the router option can override the default gateway - * that was found. */ - return 0; - } - FOREACH_ARRAY(e, routes, n_routes) { _cleanup_(route_freep) Route *route = NULL; struct in_addr gw; @@ -605,31 +565,30 @@ static int dhcp4_request_static_routes(Link *link, struct in_addr *ret_default_g if (r < 0) return r; - /* When classless static routes are provided, then router option will be ignored. To - * use the default gateway later in other routes, e.g., routes to dns servers, here we - * need to find the default gateway in the classless static routes. */ - if (is_classless && - in4_addr_is_null(&route->dst.in) && route->dst_prefixlen == 0 && - in4_addr_is_null(&default_gw)) - default_gw = gw; - r = dhcp4_request_route_auto(TAKE_PTR(route), link, &gw); if (r < 0) return r; } - *ret_default_gw = default_gw; - return is_classless; + return 0; } -static int dhcp4_request_gateway(Link *link, struct in_addr *gw) { +static int dhcp4_request_default_gateway(Link *link) { _cleanup_(route_freep) Route *route = NULL; struct in_addr address, router; int r; assert(link); assert(link->dhcp_lease); - assert(gw); + + if (!link->network->dhcp_use_gateway) + return 0; + + /* According to RFC 3442: If the DHCP server returns both a Classless Static Routes option and + * a Router option, the DHCP client MUST ignore the Router option. */ + if (link->network->dhcp_use_routes && + dhcp4_get_classless_static_or_static_routes(link, NULL, NULL) > 0) + return 0; r = sd_dhcp_lease_get_address(link->dhcp_lease, &address); if (r < 0) @@ -643,16 +602,6 @@ static int dhcp4_request_gateway(Link *link, struct in_addr *gw) { if (r < 0) return r; - if (!link->network->dhcp_use_gateway) { - /* When no classless static route is provided, even if UseGateway=no, use the gateway - * address to configure semi-static routes or routes to DNS or NTP servers. Note, if - * neither UseRoutes= nor UseGateway= is disabled, use the default gateway in classless - * static routes if provided (in that case, in4_addr_is_null(gw) below is true). */ - if (in4_addr_is_null(gw)) - *gw = router; - return 0; - } - /* 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); @@ -668,14 +617,7 @@ static int dhcp4_request_gateway(Link *link, struct in_addr *gw) { route->gw.in = router; route->prefsrc.in = address; - r = dhcp4_request_route(TAKE_PTR(route), link); - if (r < 0) - return r; - - /* When no classless static route is provided, or UseRoutes=no, then use the router address to - * configure semi-static routes and routes to DNS or NTP servers in later steps. */ - *gw = router; - return 0; + return dhcp4_request_route(TAKE_PTR(route), link); } static int dhcp4_request_semi_static_routes(Link *link) { @@ -811,7 +753,6 @@ static int dhcp4_request_routes_to_ntp(Link *link) { } static int dhcp4_request_routes(Link *link) { - struct in_addr gw = {}; int r; assert(link); @@ -821,16 +762,13 @@ static int dhcp4_request_routes(Link *link) { if (r < 0) return log_link_error_errno(link, r, "DHCP error: Could not request prefix route: %m"); - r = dhcp4_request_static_routes(link, &gw); + r = dhcp4_request_default_gateway(link); + if (r < 0) + return log_link_error_errno(link, r, "DHCP error: Could not request default gateway: %m"); + + r = dhcp4_request_classless_static_or_static_routes(link); if (r < 0) return log_link_error_errno(link, r, "DHCP error: Could not request static routes: %m"); - if (r == 0) { - /* According to RFC 3442: If the DHCP server returns both a Classless Static Routes option and - * a Router option, the DHCP client MUST ignore the Router option. */ - r = dhcp4_request_gateway(link, &gw); - if (r < 0) - return log_link_error_errno(link, r, "DHCP error: Could not request gateway: %m"); - } r = dhcp4_request_semi_static_routes(link); if (r < 0)