From 573b02f5c15c49a402be3559f141f4158bd0fc5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 7 Oct 2020 14:13:39 +0200 Subject: [PATCH 01/10] man: adjust description of MaxAttempts The code was changed in 715cedfbf03a2eb1d4dca5d1b2b876e52a3b652d to allow more than six attempts and the old description stopped making sense. --- man/systemd.network.xml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/man/systemd.network.xml b/man/systemd.network.xml index 70e1608c4a..1f389a05f3 100644 --- a/man/systemd.network.xml +++ b/man/systemd.network.xml @@ -1664,9 +1664,9 @@ IPv6Token=prefixstable:2002:da8:1:: MaxAttempts= Specifies how many times the DHCPv4 client configuration should be attempted. Takes a - number or infinity. Defaults to infinity. - Note that the time between retries is increased exponentially, so the network will not be - overloaded even if this number is high. + number or infinity. Defaults to infinity. Note that the + time between retries is increased exponentially, up to approximately one per minute, so the + network will not be overloaded even if this number is high. From e4dc0845bc26b3a3abec58932bdefe346d879d0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 7 Oct 2020 14:14:09 +0200 Subject: [PATCH 02/10] sd-dhcp-client: minor simplification --- src/libsystemd-network/sd-dhcp-client.c | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/src/libsystemd-network/sd-dhcp-client.c b/src/libsystemd-network/sd-dhcp-client.c index 3ce45b2b0d..76f762cf96 100644 --- a/src/libsystemd-network/sd-dhcp-client.c +++ b/src/libsystemd-network/sd-dhcp-client.c @@ -1187,7 +1187,7 @@ static int client_timeout_resend( sd_dhcp_client *client = userdata; DHCP_CLIENT_DONT_DESTROY(client); - usec_t next_timeout = 0; + usec_t next_timeout; uint64_t time_now; uint32_t time_left; int r; @@ -1203,17 +1203,14 @@ static int client_timeout_resend( switch (client->state) { case DHCP_STATE_RENEWING: - time_left = (client->lease->t2 - client->lease->t1) / 2; if (time_left < 60) time_left = 60; next_timeout = time_now + time_left * USEC_PER_SEC; - break; case DHCP_STATE_REBINDING: - time_left = (client->lease->lifetime - client->lease->t2) / 2; if (time_left < 60) time_left = 60; @@ -1230,24 +1227,20 @@ static int client_timeout_resend( r = client_start(client); if (r < 0) goto error; - else { - log_dhcp_client(client, "REBOOTED"); - return 0; - } + + log_dhcp_client(client, "REBOOTED"); + return 0; case DHCP_STATE_INIT: case DHCP_STATE_INIT_REBOOT: case DHCP_STATE_SELECTING: case DHCP_STATE_REQUESTING: case DHCP_STATE_BOUND: - - if (client->attempt < client->max_attempts) - client->attempt++; - else + if (client->attempt >= client->max_attempts) goto error; + client->attempt++; next_timeout = time_now + ((UINT64_C(1) << MIN(client->attempt, (uint64_t) 6)) - 1) * USEC_PER_SEC; - break; case DHCP_STATE_STOPPED: @@ -1295,12 +1288,10 @@ static int client_timeout_resend( client->state = DHCP_STATE_REBOOTING; client->request_sent = time_now; - break; case DHCP_STATE_REBOOTING: case DHCP_STATE_BOUND: - break; case DHCP_STATE_STOPPED: From 3ca1fab70a0db80dd0460478ead42c6db284ee7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 8 Oct 2020 16:59:26 +0200 Subject: [PATCH 03/10] networkd: merge ll addressing fallback modes into normal "boolean" values They are not really boolean, because we have both ipv4 and ipv6, but for each protocol we have either unset, no, and yes. From https://github.com/systemd/systemd/issues/13316#issuecomment-582906817: LinkLocalAddressing must be a boolean option, at least for ipv4: - LinkLocalAddressing=no => no LL at all. - LinkLocalAddressing=yes + Static Address => invalid configuration, warn and interpret as LinkLocalAddressing=no, no LL at all. (we check that during parsing and reject) - LinkLocalAddressing=yes + DHCP => LL process should be subordinated to the DHCP one, an LL address must be acquired at start or after a short N unsuccessful DHCP attemps, and must not stop DHCP to keeping trying. When a DHCP address is acquired, drop the LL address. If the DHCP address is lost, re-adquire a new LL address. (next patch will move in this direction) - LinkLocalAddressing=fallback has no reason to exist, because LL address must always be allocated as a fallback option when using DHCP. Having both DHCP and LL address at the same time is an RFC violation, so LinkLocalAdressing=yes correctly implemented is already the "fallback" behavior. The fallback option must be deprecated and if present in older configs must be interpreted as LinkLocalAddressing=yes. (removed) - And for IPv6, the LinkLocalAddress option has any sense at all? IPv6-LL address aren't required to be always set for every IPv6 enabled interface (in this case, coexisting with static or dynamic address if any)? Shouldn't be always =yes? (good question) This effectively reverts 29e81083bd2fcb2dbf83f67ef358c7d25adf7e9d. There is no special "fallback" mode now, so the check doesn't make sense anymore. --- man/systemd.network.xml | 18 +++++++----------- src/basic/string-table.h | 5 ++--- src/network/networkd-dhcp4.c | 3 +-- src/network/networkd-ipv4ll.c | 2 +- src/network/networkd-link.c | 13 ++++++------- src/network/networkd-link.h | 2 +- src/network/networkd-network.c | 7 ------- src/network/networkd-util.c | 19 +++++++++---------- src/network/networkd-util.h | 3 --- ...ient-with-ipv4ll-with-dhcp-server.network} | 2 +- ...t-with-ipv4ll-without-dhcp-server.network} | 2 +- test/test-network/systemd-networkd-tests.py | 12 ++++++------ 12 files changed, 35 insertions(+), 53 deletions(-) rename test/test-network/conf/{dhcp-client-with-ipv4ll-fallback-with-dhcp-server.network => dhcp-client-with-ipv4ll-with-dhcp-server.network} (66%) rename test/test-network/conf/{dhcp-client-with-ipv4ll-fallback-without-dhcp-server.network => dhcp-client-with-ipv4ll-without-dhcp-server.network} (73%) diff --git a/man/systemd.network.xml b/man/systemd.network.xml index 1f389a05f3..3b029dca63 100644 --- a/man/systemd.network.xml +++ b/man/systemd.network.xml @@ -364,17 +364,13 @@ LinkLocalAddressing= - Enables link-local address autoconfiguration. Accepts yes, - no, ipv4, ipv6, - fallback, or ipv4-fallback. If - fallback or ipv4-fallback is specified, then an IPv4 - link-local address is configured only when DHCPv4 fails. If fallback, - an IPv6 link-local address is always configured, and if ipv4-fallback, - the address is not configured. Note that, the fallback mechanism works only when DHCPv4 - client is enabled, that is, it requires DHCP=yes or - DHCP=ipv4. If Bridge= is set, defaults to - no, and if not, defaults to ipv6. - + Enables link-local address autoconfiguration. Accepts , + , , and . An IPv6 link-local address + is configured when or . An IPv4 link-local address is + configured when or . + + Defaults to when Bridge=yes is set, and + otherwise. diff --git a/src/basic/string-table.h b/src/basic/string-table.h index b6b3611ace..4911a455c5 100644 --- a/src/basic/string-table.h +++ b/src/basic/string-table.h @@ -28,13 +28,12 @@ ssize_t string_table_lookup(const char * const *table, size_t len, const char *k #define _DEFINE_STRING_TABLE_LOOKUP_FROM_STRING_WITH_BOOLEAN(name,type,yes,scope) \ scope type name##_from_string(const char *s) { \ - int b; \ if (!s) \ return -1; \ - b = parse_boolean(s); \ + int b = parse_boolean(s); \ if (b == 0) \ return (type) 0; \ - else if (b > 0) \ + if (b > 0) \ return yes; \ return (type) string_table_lookup(name##_table, ELEMENTSOF(name##_table), s); \ } diff --git a/src/network/networkd-dhcp4.c b/src/network/networkd-dhcp4.c index a521822a50..8a2292b1a6 100644 --- a/src/network/networkd-dhcp4.c +++ b/src/network/networkd-dhcp4.c @@ -1048,8 +1048,7 @@ static int dhcp4_handler(sd_dhcp_client *client, int event, void *userdata) { switch (event) { case SD_DHCP_CLIENT_EVENT_STOP: - - if (link_ipv4ll_enabled(link, ADDRESS_FAMILY_FALLBACK_IPV4)) { + if (link_ipv4ll_enabled(link)) { assert(link->ipv4ll); log_link_debug(link, "DHCP client is stopped. Acquiring IPv4 link-local address"); diff --git a/src/network/networkd-ipv4ll.c b/src/network/networkd-ipv4ll.c index 598af25de6..7e1c1ac16e 100644 --- a/src/network/networkd-ipv4ll.c +++ b/src/network/networkd-ipv4ll.c @@ -148,7 +148,7 @@ int ipv4ll_configure(Link *link) { assert(link); - if (!link_ipv4ll_enabled(link, ADDRESS_FAMILY_IPV4 | ADDRESS_FAMILY_FALLBACK_IPV4)) + if (!link_ipv4ll_enabled(link)) return 0; if (!link->ipv4ll) { diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index 4feda54c70..baaf0b8812 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -55,9 +55,8 @@ #include "util.h" #include "vrf.h" -bool link_ipv4ll_enabled(Link *link, AddressFamily mask) { +bool link_ipv4ll_enabled(Link *link) { assert(link); - assert((mask & ~(ADDRESS_FAMILY_IPV4 | ADDRESS_FAMILY_FALLBACK_IPV4)) == 0); if (link->flags & IFF_LOOPBACK) return false; @@ -80,7 +79,7 @@ bool link_ipv4ll_enabled(Link *link, AddressFamily mask) { if (link->network->bond) return false; - return link->network->link_local & mask; + return link->network->link_local & ADDRESS_FAMILY_IPV4; } bool link_ipv6ll_enabled(Link *link) { @@ -795,7 +794,7 @@ void link_check_ready(Link *link) { bool has_ndisc_address = false; NDiscAddress *n; - if (link_ipv4ll_enabled(link, ADDRESS_FAMILY_IPV4) && !link->ipv4ll_address_configured) { + if (link_ipv4ll_enabled(link) && !link->ipv4ll_address_configured) { log_link_debug(link, "%s(): IPv4LL is not configured.", __func__); return; } @@ -814,7 +813,7 @@ void link_check_ready(Link *link) { if ((link_dhcp4_enabled(link) || link_dhcp6_enabled(link)) && !link->dhcp_address && set_isempty(link->dhcp6_addresses) && !has_ndisc_address && - !(link_ipv4ll_enabled(link, ADDRESS_FAMILY_FALLBACK_IPV4) && link->ipv4ll_address_configured)) { + !(link_ipv4ll_enabled(link) && link->ipv4ll_address_configured)) { log_link_debug(link, "%s(): DHCP4 or DHCP6 is enabled but no dynamic address is assigned yet.", __func__); return; } @@ -824,7 +823,7 @@ void link_check_ready(Link *link) { !(link->dhcp6_address_configured && link->dhcp6_route_configured) && !(link->dhcp6_pd_address_configured && link->dhcp6_pd_route_configured) && !(link->ndisc_addresses_configured && link->ndisc_routes_configured) && - !(link_ipv4ll_enabled(link, ADDRESS_FAMILY_FALLBACK_IPV4) && link->ipv4ll_address_configured)) { + !(link_ipv4ll_enabled(link) && link->ipv4ll_address_configured)) { /* When DHCP or RA is enabled, at least one protocol must provide an address, or * an IPv4ll fallback address must be configured. */ log_link_debug(link, "%s(): dynamic addresses or routes are not configured.", __func__); @@ -1230,7 +1229,7 @@ static int link_acquire_ipv4_conf(Link *link) { assert(link->manager); assert(link->manager->event); - if (link_ipv4ll_enabled(link, ADDRESS_FAMILY_IPV4)) { + if (link_ipv4ll_enabled(link)) { assert(link->ipv4ll); log_link_debug(link, "Acquiring IPv4 link-local address"); diff --git a/src/network/networkd-link.h b/src/network/networkd-link.h index cd541920cb..666ba4acf2 100644 --- a/src/network/networkd-link.h +++ b/src/network/networkd-link.h @@ -231,7 +231,7 @@ int link_ipv6ll_gained(Link *link, const struct in6_addr *address); int link_set_mtu(Link *link, uint32_t mtu); -bool link_ipv4ll_enabled(Link *link, AddressFamily mask); +bool link_ipv4ll_enabled(Link *link); int link_stop_engines(Link *link, bool may_keep_dhcp); diff --git a/src/network/networkd-network.c b/src/network/networkd-network.c index 195bf3aaae..7ad8d087f4 100644 --- a/src/network/networkd-network.c +++ b/src/network/networkd-network.c @@ -210,13 +210,6 @@ int network_verify(Network *network) { if (network->link_local < 0) network->link_local = network->bridge ? ADDRESS_FAMILY_NO : ADDRESS_FAMILY_IPV6; - if (FLAGS_SET(network->link_local, ADDRESS_FAMILY_FALLBACK_IPV4) && - !FLAGS_SET(network->dhcp, ADDRESS_FAMILY_IPV4)) { - log_warning("%s: fallback assignment of IPv4 link local address is enabled but DHCPv4 is disabled. " - "Disabling the fallback assignment.", network->filename); - SET_FLAG(network->link_local, ADDRESS_FAMILY_FALLBACK_IPV4, false); - } - /* IPMasquerade=yes implies IPForward=yes */ if (network->ip_masquerade) network->ip_forward |= ADDRESS_FAMILY_IPV4; diff --git a/src/network/networkd-util.c b/src/network/networkd-util.c index 8ddcbb2fce..83b38b2b05 100644 --- a/src/network/networkd-util.c +++ b/src/network/networkd-util.c @@ -15,15 +15,6 @@ static const char* const address_family_table[_ADDRESS_FAMILY_MAX] = { [ADDRESS_FAMILY_IPV6] = "ipv6", }; -static const char* const link_local_address_family_table[_ADDRESS_FAMILY_MAX] = { - [ADDRESS_FAMILY_NO] = "no", - [ADDRESS_FAMILY_YES] = "yes", - [ADDRESS_FAMILY_IPV4] = "ipv4", - [ADDRESS_FAMILY_IPV6] = "ipv6", - [ADDRESS_FAMILY_FALLBACK] = "fallback", - [ADDRESS_FAMILY_FALLBACK_IPV4] = "ipv4-fallback", -}; - static const char* const routing_policy_rule_address_family_table[_ADDRESS_FAMILY_MAX] = { [ADDRESS_FAMILY_YES] = "both", [ADDRESS_FAMILY_IPV4] = "ipv4", @@ -47,7 +38,15 @@ static const char* const dhcp_lease_server_type_table[_SD_DHCP_LEASE_SERVER_TYPE }; DEFINE_STRING_TABLE_LOOKUP_WITH_BOOLEAN(address_family, AddressFamily, ADDRESS_FAMILY_YES); -DEFINE_STRING_TABLE_LOOKUP_WITH_BOOLEAN(link_local_address_family, AddressFamily, ADDRESS_FAMILY_YES); + +AddressFamily link_local_address_family_from_string(const char *s) { + if (streq_ptr(s, "fallback")) /* compat name */ + return ADDRESS_FAMILY_YES; + if (streq_ptr(s, "fallback-ipv4")) /* compat name */ + return ADDRESS_FAMILY_IPV4; + return address_family_from_string(s); +} + DEFINE_STRING_TABLE_LOOKUP(routing_policy_rule_address_family, AddressFamily); DEFINE_STRING_TABLE_LOOKUP(duplicate_address_detection_address_family, AddressFamily); DEFINE_CONFIG_PARSE_ENUM(config_parse_link_local_address_family, link_local_address_family, diff --git a/src/network/networkd-util.h b/src/network/networkd-util.h index 6100a0031c..7b48046c35 100644 --- a/src/network/networkd-util.h +++ b/src/network/networkd-util.h @@ -16,8 +16,6 @@ typedef enum AddressFamily { ADDRESS_FAMILY_IPV4 = 1 << 0, ADDRESS_FAMILY_IPV6 = 1 << 1, ADDRESS_FAMILY_YES = ADDRESS_FAMILY_IPV4 | ADDRESS_FAMILY_IPV6, - ADDRESS_FAMILY_FALLBACK_IPV4 = 1 << 2, - ADDRESS_FAMILY_FALLBACK = ADDRESS_FAMILY_FALLBACK_IPV4 | ADDRESS_FAMILY_IPV6, _ADDRESS_FAMILY_MAX, _ADDRESS_FAMILY_INVALID = -1, } AddressFamily; @@ -34,7 +32,6 @@ CONFIG_PARSER_PROTOTYPE(config_parse_address_family_with_kernel); const char *address_family_to_string(AddressFamily b) _const_; AddressFamily address_family_from_string(const char *s) _pure_; -const char *link_local_address_family_to_string(AddressFamily b) _const_; AddressFamily link_local_address_family_from_string(const char *s) _pure_; const char *routing_policy_rule_address_family_to_string(AddressFamily b) _const_; diff --git a/test/test-network/conf/dhcp-client-with-ipv4ll-fallback-with-dhcp-server.network b/test/test-network/conf/dhcp-client-with-ipv4ll-with-dhcp-server.network similarity index 66% rename from test/test-network/conf/dhcp-client-with-ipv4ll-fallback-with-dhcp-server.network rename to test/test-network/conf/dhcp-client-with-ipv4ll-with-dhcp-server.network index 9ebdbb4f8d..0455e09b4c 100644 --- a/test/test-network/conf/dhcp-client-with-ipv4ll-fallback-with-dhcp-server.network +++ b/test/test-network/conf/dhcp-client-with-ipv4ll-with-dhcp-server.network @@ -3,5 +3,5 @@ Name=veth99 [Network] DHCP=ipv4 -LinkLocalAddressing=fallback +LinkLocalAddressing=yes IPv6AcceptRA=no diff --git a/test/test-network/conf/dhcp-client-with-ipv4ll-fallback-without-dhcp-server.network b/test/test-network/conf/dhcp-client-with-ipv4ll-without-dhcp-server.network similarity index 73% rename from test/test-network/conf/dhcp-client-with-ipv4ll-fallback-without-dhcp-server.network rename to test/test-network/conf/dhcp-client-with-ipv4ll-without-dhcp-server.network index 5489c62a54..2723e0a273 100644 --- a/test/test-network/conf/dhcp-client-with-ipv4ll-fallback-without-dhcp-server.network +++ b/test/test-network/conf/dhcp-client-with-ipv4ll-without-dhcp-server.network @@ -3,7 +3,7 @@ Name=veth99 [Network] DHCP=ipv4 -LinkLocalAddressing=fallback +LinkLocalAddressing=yes IPv6AcceptRA=no [DHCPv4] diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index 688238777d..1262611d28 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -3428,8 +3428,8 @@ class NetworkdDHCPClientTests(unittest.TestCase, Utilities): 'dhcp-client-use-dns-yes.network', 'dhcp-client-use-domains.network', 'dhcp-client-vrf.network', - 'dhcp-client-with-ipv4ll-fallback-with-dhcp-server.network', - 'dhcp-client-with-ipv4ll-fallback-without-dhcp-server.network', + 'dhcp-client-with-ipv4ll-with-dhcp-server.network', + 'dhcp-client-with-ipv4ll-without-dhcp-server.network', 'dhcp-client-with-static-address.network', 'dhcp-client.network', 'dhcp-server-decline.network', @@ -3995,9 +3995,9 @@ class NetworkdDHCPClientTests(unittest.TestCase, Utilities): print(output) self.assertRegex(output, 'onlink') - def test_dhcp_client_with_ipv4ll_fallback_with_dhcp_server(self): + def test_dhcp_client_with_ipv4ll_with_dhcp_server(self): copy_unit_to_networkd_unit_path('25-veth.netdev', 'dhcp-server-veth-peer.network', - 'dhcp-client-with-ipv4ll-fallback-with-dhcp-server.network') + 'dhcp-client-with-ipv4ll-with-dhcp-server.network') start_networkd() self.wait_online(['veth-peer:carrier']) start_dnsmasq(lease_time='2m') @@ -4032,9 +4032,9 @@ class NetworkdDHCPClientTests(unittest.TestCase, Utilities): search_words_in_dnsmasq_log('DHCPOFFER', show_all=True) - def test_dhcp_client_with_ipv4ll_fallback_without_dhcp_server(self): + def test_dhcp_client_with_ipv4ll_without_dhcp_server(self): copy_unit_to_networkd_unit_path('25-veth.netdev', 'dhcp-server-veth-peer.network', - 'dhcp-client-with-ipv4ll-fallback-without-dhcp-server.network') + 'dhcp-client-with-ipv4ll-without-dhcp-server.network') start_networkd() self.wait_online(['veth99:degraded', 'veth-peer:routable']) From fb536bc5daf4a57b3bdd277480d599cbf785b37d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 8 Oct 2020 16:51:25 +0200 Subject: [PATCH 04/10] sd-dhcp-client: report transient DHCP failure to the caller So far we only reported major state transitions like failure to acquire the message. Let's report the initial failure after a few timeouts in a new event type. The number of timeouts is hardcoded as 3, since Windows seems to be using that. I don't think we need to make this configurable out of the box. A reasonable default may be enough. --- src/libsystemd-network/sd-dhcp-client.c | 6 ++++++ src/systemd/sd-dhcp-client.h | 2 ++ 2 files changed, 8 insertions(+) diff --git a/src/libsystemd-network/sd-dhcp-client.c b/src/libsystemd-network/sd-dhcp-client.c index 76f762cf96..f47a542483 100644 --- a/src/libsystemd-network/sd-dhcp-client.c +++ b/src/libsystemd-network/sd-dhcp-client.c @@ -38,6 +38,9 @@ #define RESTART_AFTER_NAK_MIN_USEC (1 * USEC_PER_SEC) #define RESTART_AFTER_NAK_MAX_USEC (30 * USEC_PER_MINUTE) +#define TRANSIENT_FAILURE_ATTEMPTS 3 /* Arbitrary limit: how many attempts are considered enough to report + * transient failure. */ + typedef struct sd_dhcp_client_id { uint8_t type; union { @@ -1299,6 +1302,9 @@ static int client_timeout_resend( goto error; } + if (client->attempt >= TRANSIENT_FAILURE_ATTEMPTS) + client_notify(client, SD_DHCP_CLIENT_EVENT_TRANSIENT_FAILURE); + return 0; error: diff --git a/src/systemd/sd-dhcp-client.h b/src/systemd/sd-dhcp-client.h index 44bafe6df5..c35328a9a6 100644 --- a/src/systemd/sd-dhcp-client.h +++ b/src/systemd/sd-dhcp-client.h @@ -40,6 +40,8 @@ enum { SD_DHCP_CLIENT_EVENT_EXPIRED = 3, SD_DHCP_CLIENT_EVENT_RENEW = 4, SD_DHCP_CLIENT_EVENT_SELECTING = 5, + SD_DHCP_CLIENT_EVENT_TRANSIENT_FAILURE = 6, /* Sent when we have not received a reply after the first few attempts. + * The client may want to start acquiring link-local addresses. */ }; enum { From 0107b769b11dcd44b05981be440d21b1923b25f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 8 Oct 2020 20:14:51 +0200 Subject: [PATCH 05/10] networkd: start ipv4ll when dhcp has trouble getting a lease Fixes #13316. --- man/systemd.network.xml | 7 +++++-- src/network/networkd-dhcp4.c | 15 +++++++++++++++ src/network/networkd-link.c | 17 ++++++++--------- test/test-network/conf/dhcp-client-vrf.network | 1 - test/test-network/systemd-networkd-tests.py | 3 --- 5 files changed, 28 insertions(+), 15 deletions(-) diff --git a/man/systemd.network.xml b/man/systemd.network.xml index 3b029dca63..27263c34eb 100644 --- a/man/systemd.network.xml +++ b/man/systemd.network.xml @@ -367,7 +367,9 @@ Enables link-local address autoconfiguration. Accepts , , , and . An IPv6 link-local address is configured when or . An IPv4 link-local address is - configured when or . + configured when or and when DHCPv4 autoconfiguration + has been unsuccessful for some time. (IPv4 link-local address autoconfiguration will usually + happen in parallel with repeated attempts to acquire a DHCPv4 lease). Defaults to when Bridge=yes is set, and otherwise. @@ -1662,7 +1664,8 @@ IPv6Token=prefixstable:2002:da8:1:: Specifies how many times the DHCPv4 client configuration should be attempted. Takes a number or infinity. Defaults to infinity. Note that the time between retries is increased exponentially, up to approximately one per minute, so the - network will not be overloaded even if this number is high. + network will not be overloaded even if this number is high. The default is suitable in most + circumstances. diff --git a/src/network/networkd-dhcp4.c b/src/network/networkd-dhcp4.c index 8a2292b1a6..78af53c381 100644 --- a/src/network/networkd-dhcp4.c +++ b/src/network/networkd-dhcp4.c @@ -1135,6 +1135,21 @@ static int dhcp4_handler(sd_dhcp_client *client, int event, void *userdata) { return -ENOMSG; } break; + + case SD_DHCP_CLIENT_EVENT_TRANSIENT_FAILURE: + if (link_ipv4ll_enabled(link)) { + assert(link->ipv4ll); + + if (!sd_ipv4ll_is_running(link->ipv4ll)) { + log_link_debug(link, "Problems acquiring DHCP lease, acquiring IPv4 link-local address"); + + r = sd_ipv4ll_start(link->ipv4ll); + if (r < 0) + return log_link_warning_errno(link, r, "Could not acquire IPv4 link-local address: %m"); + } + } + break; + default: if (event < 0) log_link_warning_errno(link, event, "DHCP error: Client failed: %m"); diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index baaf0b8812..07547d032b 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -1229,7 +1229,14 @@ static int link_acquire_ipv4_conf(Link *link) { assert(link->manager); assert(link->manager->event); - if (link_ipv4ll_enabled(link)) { + if (link->dhcp_client) { + log_link_debug(link, "Acquiring DHCPv4 lease"); + + r = sd_dhcp_client_start(link->dhcp_client); + if (r < 0) + return log_link_warning_errno(link, r, "Could not acquire DHCPv4 lease: %m"); + + } else if (link_ipv4ll_enabled(link)) { assert(link->ipv4ll); log_link_debug(link, "Acquiring IPv4 link-local address"); @@ -1239,14 +1246,6 @@ static int link_acquire_ipv4_conf(Link *link) { return log_link_warning_errno(link, r, "Could not acquire IPv4 link-local address: %m"); } - if (link->dhcp_client) { - log_link_debug(link, "Acquiring DHCPv4 lease"); - - r = sd_dhcp_client_start(link->dhcp_client); - if (r < 0) - return log_link_warning_errno(link, r, "Could not acquire DHCPv4 lease: %m"); - } - return 0; } diff --git a/test/test-network/conf/dhcp-client-vrf.network b/test/test-network/conf/dhcp-client-vrf.network index bb1d2e09c5..a277d5ecbf 100644 --- a/test/test-network/conf/dhcp-client-vrf.network +++ b/test/test-network/conf/dhcp-client-vrf.network @@ -4,5 +4,4 @@ Name=veth99 [Network] DHCP=yes IPv6AcceptRA=yes -LinkLocalAddressing=yes VRF=vrf99 diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index 1262611d28..2c2ffcb9d0 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -3925,7 +3925,6 @@ class NetworkdDHCPClientTests(unittest.TestCase, Utilities): print('## ip address show vrf vrf99') output = check_output('ip address show vrf vrf99') print(output) - self.assertRegex(output, 'inet 169.254.[0-9]*.[0-9]*/16 brd 169.254.255.255 scope link veth99') self.assertRegex(output, 'inet 192.168.5.[0-9]*/24 brd 192.168.5.255 scope global dynamic veth99') self.assertRegex(output, 'inet6 2600::[0-9a-f]*/128 scope global (dynamic noprefixroute|noprefixroute dynamic)') self.assertRegex(output, 'inet6 .* scope link') @@ -3933,7 +3932,6 @@ class NetworkdDHCPClientTests(unittest.TestCase, Utilities): print('## ip address show dev veth99') output = check_output('ip address show dev veth99') print(output) - self.assertRegex(output, 'inet 169.254.[0-9]*.[0-9]*/16 brd 169.254.255.255 scope link veth99') self.assertRegex(output, 'inet 192.168.5.[0-9]*/24 brd 192.168.5.255 scope global dynamic veth99') self.assertRegex(output, 'inet6 2600::[0-9a-f]*/128 scope global (dynamic noprefixroute|noprefixroute dynamic)') self.assertRegex(output, 'inet6 .* scope link') @@ -3942,7 +3940,6 @@ class NetworkdDHCPClientTests(unittest.TestCase, Utilities): output = check_output('ip route show vrf vrf99') print(output) self.assertRegex(output, 'default via 192.168.5.1 dev veth99 proto dhcp src 192.168.5.') - self.assertRegex(output, '169.254.0.0/16 dev veth99 proto kernel scope link src 169.254') self.assertRegex(output, '192.168.5.0/24 dev veth99 proto kernel scope link src 192.168.5') self.assertRegex(output, '192.168.5.0/24 via 192.168.5.5 dev veth99 proto dhcp') self.assertRegex(output, '192.168.5.1 dev veth99 proto dhcp scope link src 192.168.5') From 8ccae2dd2d68f715d4c962ee4bab4827b4500540 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 23 Nov 2020 13:28:47 +0900 Subject: [PATCH 06/10] network: stop IPv4LL engine when DHCPv4 address is successfully acquired --- src/network/networkd-dhcp4.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/network/networkd-dhcp4.c b/src/network/networkd-dhcp4.c index 78af53c381..c4024499f1 100644 --- a/src/network/networkd-dhcp4.c +++ b/src/network/networkd-dhcp4.c @@ -70,6 +70,10 @@ static void dhcp4_check_ready(Link *link) { return; } + r = sd_ipv4ll_stop(link->ipv4ll); + if (r < 0) + log_link_warning_errno(link, r, "Failed to drop IPv4 link-local address, ignoring: %m"); + link_check_ready(link); } From d19b993983fec735e0c7c1222c60e805983b3cf5 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 23 Nov 2020 13:55:26 +0900 Subject: [PATCH 07/10] network: shorten link_check_ready() a bit --- src/network/networkd-link.c | 83 ++++++++++++------------------------- 1 file changed, 27 insertions(+), 56 deletions(-) diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index 07547d032b..fc47d33cbb 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -733,77 +733,54 @@ void link_check_ready(Link *link) { if (link->state == LINK_STATE_CONFIGURED) return; - if (link->state != LINK_STATE_CONFIGURING) { - log_link_debug(link, "%s(): link is in %s state.", __func__, link_state_to_string(link->state)); - return; - } + if (link->state != LINK_STATE_CONFIGURING) + return (void) log_link_debug(link, "%s(): link is in %s state.", __func__, link_state_to_string(link->state)); if (!link->network) return; - if (!link->addresses_configured) { - log_link_debug(link, "%s(): static addresses are not configured.", __func__); - return; - } + if (!link->addresses_configured) + return (void) log_link_debug(link, "%s(): static addresses are not configured.", __func__); - if (!link->neighbors_configured) { - log_link_debug(link, "%s(): static neighbors are not configured.", __func__); - return; - } + if (!link->neighbors_configured) + return (void) log_link_debug(link, "%s(): static neighbors are not configured.", __func__); SET_FOREACH(a, link->addresses) if (!address_is_ready(a)) { _cleanup_free_ char *str = NULL; (void) in_addr_to_string(a->family, &a->in_addr, &str); - log_link_debug(link, "%s(): an address %s/%d is not ready.", __func__, strnull(str), a->prefixlen); - return; + return (void) log_link_debug(link, "%s(): an address %s/%d is not ready.", __func__, strnull(str), a->prefixlen); } - if (!link->static_routes_configured) { - log_link_debug(link, "%s(): static routes are not configured.", __func__); - return; - } + if (!link->static_routes_configured) + return (void) log_link_debug(link, "%s(): static routes are not configured.", __func__); - if (!link->static_nexthops_configured) { - log_link_debug(link, "%s(): static nexthops are not configured.", __func__); - return; - } + if (!link->static_nexthops_configured) + return (void) log_link_debug(link, "%s(): static nexthops are not configured.", __func__); - if (!link->routing_policy_rules_configured) { - log_link_debug(link, "%s(): static routing policy rules are not configured.", __func__); - return; - } + if (!link->routing_policy_rules_configured) + return (void) log_link_debug(link, "%s(): static routing policy rules are not configured.", __func__); - if (!link->tc_configured) { - log_link_debug(link, "%s(): traffic controls are not configured.", __func__); - return; - } + if (!link->tc_configured) + return (void) log_link_debug(link, "%s(): traffic controls are not configured.", __func__); - if (!link->sr_iov_configured) { - log_link_debug(link, "%s(): SR-IOV is not configured.", __func__); - return; - } + if (!link->sr_iov_configured) + return (void) log_link_debug(link, "%s(): SR-IOV is not configured.", __func__); - if (!link->bridge_mdb_configured) { - log_link_debug(link, "%s(): Bridge MDB is not configured.", __func__); - return; - } + if (!link->bridge_mdb_configured) + return (void) log_link_debug(link, "%s(): Bridge MDB is not configured.", __func__); if (link_has_carrier(link) || !link->network->configure_without_carrier) { bool has_ndisc_address = false; NDiscAddress *n; - if (link_ipv4ll_enabled(link) && !link->ipv4ll_address_configured) { - log_link_debug(link, "%s(): IPv4LL is not configured.", __func__); - return; - } + if (link_ipv4ll_enabled(link) && !link->ipv4ll_address_configured) + return (void) log_link_debug(link, "%s(): IPv4LL is not configured.", __func__); if (link_ipv6ll_enabled(link) && - in_addr_is_null(AF_INET6, (const union in_addr_union*) &link->ipv6ll_address)) { - log_link_debug(link, "%s(): IPv6LL is not configured.", __func__); - return; - } + in_addr_is_null(AF_INET6, (const union in_addr_union*) &link->ipv6ll_address)) + return (void) log_link_debug(link, "%s(): IPv6LL is not configured.", __func__); SET_FOREACH(n, link->ndisc_addresses) if (!n->marked) { @@ -813,22 +790,18 @@ void link_check_ready(Link *link) { if ((link_dhcp4_enabled(link) || link_dhcp6_enabled(link)) && !link->dhcp_address && set_isempty(link->dhcp6_addresses) && !has_ndisc_address && - !(link_ipv4ll_enabled(link) && link->ipv4ll_address_configured)) { - log_link_debug(link, "%s(): DHCP4 or DHCP6 is enabled but no dynamic address is assigned yet.", __func__); - return; - } + !(link_ipv4ll_enabled(link) && link->ipv4ll_address_configured)) + return (void) log_link_debug(link, "%s(): DHCP4 or DHCP6 is enabled but no dynamic address is assigned yet.", __func__); if (link_dhcp4_enabled(link) || link_dhcp6_enabled(link) || link_dhcp6_pd_is_enabled(link) || link_ipv6_accept_ra_enabled(link)) { if (!link->dhcp4_configured && !(link->dhcp6_address_configured && link->dhcp6_route_configured) && !(link->dhcp6_pd_address_configured && link->dhcp6_pd_route_configured) && !(link->ndisc_addresses_configured && link->ndisc_routes_configured) && - !(link_ipv4ll_enabled(link) && link->ipv4ll_address_configured)) { + !(link_ipv4ll_enabled(link) && link->ipv4ll_address_configured)) /* When DHCP or RA is enabled, at least one protocol must provide an address, or * an IPv4ll fallback address must be configured. */ - log_link_debug(link, "%s(): dynamic addresses or routes are not configured.", __func__); - return; - } + return (void) log_link_debug(link, "%s(): dynamic addresses or routes are not configured.", __func__); log_link_debug(link, "%s(): dhcp4:%s dhcp6_addresses:%s dhcp_routes:%s dhcp_pd_addresses:%s dhcp_pd_routes:%s ndisc_addresses:%s ndisc_routes:%s", __func__, @@ -843,8 +816,6 @@ void link_check_ready(Link *link) { } link_enter_configured(link); - - return; } static int link_set_static_configs(Link *link) { From 0b4b66cc53c3b6315a1dcb66a8bc7211a562ab5a Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 23 Nov 2020 13:44:29 +0900 Subject: [PATCH 08/10] network: simplify the condition about ipv4ll is enabled or not --- src/network/networkd-dhcp4.c | 18 ++++++------------ src/network/networkd-link.c | 4 +--- 2 files changed, 7 insertions(+), 15 deletions(-) diff --git a/src/network/networkd-dhcp4.c b/src/network/networkd-dhcp4.c index c4024499f1..74c40559a7 100644 --- a/src/network/networkd-dhcp4.c +++ b/src/network/networkd-dhcp4.c @@ -1052,9 +1052,7 @@ static int dhcp4_handler(sd_dhcp_client *client, int event, void *userdata) { switch (event) { case SD_DHCP_CLIENT_EVENT_STOP: - if (link_ipv4ll_enabled(link)) { - assert(link->ipv4ll); - + if (link->ipv4ll) { log_link_debug(link, "DHCP client is stopped. Acquiring IPv4 link-local address"); r = sd_ipv4ll_start(link->ipv4ll); @@ -1141,16 +1139,12 @@ static int dhcp4_handler(sd_dhcp_client *client, int event, void *userdata) { break; case SD_DHCP_CLIENT_EVENT_TRANSIENT_FAILURE: - if (link_ipv4ll_enabled(link)) { - assert(link->ipv4ll); + if (link->ipv4ll && !sd_ipv4ll_is_running(link->ipv4ll)) { + log_link_debug(link, "Problems acquiring DHCP lease, acquiring IPv4 link-local address"); - if (!sd_ipv4ll_is_running(link->ipv4ll)) { - log_link_debug(link, "Problems acquiring DHCP lease, acquiring IPv4 link-local address"); - - r = sd_ipv4ll_start(link->ipv4ll); - if (r < 0) - return log_link_warning_errno(link, r, "Could not acquire IPv4 link-local address: %m"); - } + r = sd_ipv4ll_start(link->ipv4ll); + if (r < 0) + return log_link_warning_errno(link, r, "Could not acquire IPv4 link-local address: %m"); } break; diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index fc47d33cbb..d122af4d59 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -1207,9 +1207,7 @@ static int link_acquire_ipv4_conf(Link *link) { if (r < 0) return log_link_warning_errno(link, r, "Could not acquire DHCPv4 lease: %m"); - } else if (link_ipv4ll_enabled(link)) { - assert(link->ipv4ll); - + } else if (link->ipv4ll) { log_link_debug(link, "Acquiring IPv4 link-local address"); r = sd_ipv4ll_start(link->ipv4ll); From 0d0799daf483946d88254342ed3a89254d5eb957 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 23 Nov 2020 13:42:22 +0900 Subject: [PATCH 09/10] network: treat IPv4LL is one of dynamic addressing protocol This makes an IPv4LL address optional when multiple dynamic addressing protocols are enabled. --- src/network/networkd-link.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index d122af4d59..2b7055dd0f 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -775,9 +775,6 @@ void link_check_ready(Link *link) { bool has_ndisc_address = false; NDiscAddress *n; - if (link_ipv4ll_enabled(link) && !link->ipv4ll_address_configured) - return (void) log_link_debug(link, "%s(): IPv4LL is not configured.", __func__); - if (link_ipv6ll_enabled(link) && in_addr_is_null(AF_INET6, (const union in_addr_union*) &link->ipv6ll_address)) return (void) log_link_debug(link, "%s(): IPv6LL is not configured.", __func__); @@ -788,24 +785,26 @@ void link_check_ready(Link *link) { break; } - if ((link_dhcp4_enabled(link) || link_dhcp6_enabled(link)) && + if ((link_dhcp4_enabled(link) || link_dhcp6_enabled(link) || link_ipv4ll_enabled(link)) && !link->dhcp_address && set_isempty(link->dhcp6_addresses) && !has_ndisc_address && - !(link_ipv4ll_enabled(link) && link->ipv4ll_address_configured)) - return (void) log_link_debug(link, "%s(): DHCP4 or DHCP6 is enabled but no dynamic address is assigned yet.", __func__); + !link->ipv4ll_address_configured) + /* When DHCP[46] or IPv4LL is enabled, at least one address is acquired by them. */ + return (void) log_link_debug(link, "%s(): DHCP4, DHCP6 or IPv4LL is enabled but no dynamic address is assigned yet.", __func__); - if (link_dhcp4_enabled(link) || link_dhcp6_enabled(link) || link_dhcp6_pd_is_enabled(link) || link_ipv6_accept_ra_enabled(link)) { + if (link_dhcp4_enabled(link) || link_dhcp6_enabled(link) || link_dhcp6_pd_is_enabled(link) || + link_ipv6_accept_ra_enabled(link) || link_ipv4ll_enabled(link)) { if (!link->dhcp4_configured && !(link->dhcp6_address_configured && link->dhcp6_route_configured) && !(link->dhcp6_pd_address_configured && link->dhcp6_pd_route_configured) && !(link->ndisc_addresses_configured && link->ndisc_routes_configured) && - !(link_ipv4ll_enabled(link) && link->ipv4ll_address_configured)) - /* When DHCP or RA is enabled, at least one protocol must provide an address, or - * an IPv4ll fallback address must be configured. */ + !link->ipv4ll_address_configured) + /* When DHCP[46], NDisc, or IPv4LL is enabled, at least one protocol must be finished. */ return (void) log_link_debug(link, "%s(): dynamic addresses or routes are not configured.", __func__); - log_link_debug(link, "%s(): dhcp4:%s dhcp6_addresses:%s dhcp_routes:%s dhcp_pd_addresses:%s dhcp_pd_routes:%s ndisc_addresses:%s ndisc_routes:%s", + log_link_debug(link, "%s(): dhcp4:%s ipv4ll:%s dhcp6_addresses:%s dhcp_routes:%s dhcp_pd_addresses:%s dhcp_pd_routes:%s ndisc_addresses:%s ndisc_routes:%s", __func__, yes_no(link->dhcp4_configured), + yes_no(link->ipv4ll_address_configured), yes_no(link->dhcp6_address_configured), yes_no(link->dhcp6_route_configured), yes_no(link->dhcp6_pd_address_configured), From 53ec5dd028503f48be09e936f013ff54678b2359 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 23 Nov 2020 22:34:43 +0900 Subject: [PATCH 10/10] network: use IN_SET() macro Follow-up for 1d370b2c182505ff8033fccbebcc56621d305220. --- src/network/networkd-dhcp-common.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/network/networkd-dhcp-common.c b/src/network/networkd-dhcp-common.c index c338c775a7..904cb34d13 100644 --- a/src/network/networkd-dhcp-common.c +++ b/src/network/networkd-dhcp-common.c @@ -62,11 +62,9 @@ static struct DUID fallback_duid = { .type = DUID_TYPE_EN }; DUID* link_get_duid(Link *link) { if (link->network->duid.type != _DUID_TYPE_INVALID) return &link->network->duid; - else if (link->hw_addr.length == 0 && - (link->manager->duid.type == DUID_TYPE_LLT || - link->manager->duid.type == DUID_TYPE_LL)) - /* Fallback to DUID that works without mac addresses. - * This is useful for tunnel devices without mac address. */ + else if (link->hw_addr.length == 0 && IN_SET(link->manager->duid.type, DUID_TYPE_LLT, DUID_TYPE_LL)) + /* Fallback to DUID that works without MAC address. + * This is useful for tunnel devices without MAC address. */ return &fallback_duid; else return &link->manager->duid;