From e402e99eaa59cd15e00c71fb7168f001a03e33ca Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 18 Aug 2022 14:15:23 +0900 Subject: [PATCH 1/5] network: fix DHCPv4 address renewal with IPv4ACD Previously, when a DHCP address is renewed and if the IPv4ACD for the address is enabled, the address will never drop the probing flag, thus the lifetime of the address will never be updated. This drops NETWORK_CONFIG_STATE_PROBING, and the IPv4ACD status is managed another bit, Address.acd_bound. And, the flag is updated only when the IPv4ACD announced the address or detects conflict. --- src/network/networkd-address.c | 8 ++--- src/network/networkd-address.h | 7 +--- src/network/networkd-ipv4acd.c | 62 +++++++++++++++++++--------------- src/network/networkd-ipv4acd.h | 1 + src/network/networkd-util.c | 1 - src/network/networkd-util.h | 11 +++--- 6 files changed, 46 insertions(+), 44 deletions(-) diff --git a/src/network/networkd-address.c b/src/network/networkd-address.c index ccb4a426fa..9122c644e8 100644 --- a/src/network/networkd-address.c +++ b/src/network/networkd-address.c @@ -143,15 +143,15 @@ Address *address_free(Address *address) { bool address_is_ready(const Address *a) { assert(a); + if (!ipv4acd_bound(a)) + return false; + if (FLAGS_SET(a->flags, IFA_F_TENTATIVE)) return false; if (FLAGS_SET(a->state, NETWORK_CONFIG_STATE_REMOVING)) return false; - if (FLAGS_SET(a->state, NETWORK_CONFIG_STATE_PROBING)) - return false; - if (!FLAGS_SET(a->state, NETWORK_CONFIG_STATE_CONFIGURED)) return false; @@ -1089,7 +1089,7 @@ static bool address_is_ready_to_configure(Link *link, const Address *address) { if (!link_is_ready_to_configure(link, false)) return false; - if (FLAGS_SET(address->state, NETWORK_CONFIG_STATE_PROBING)) + if (!ipv4acd_bound(address)) return false; /* Refuse adding more than the limit */ diff --git a/src/network/networkd-address.h b/src/network/networkd-address.h index 0237c1cb98..f97342504c 100644 --- a/src/network/networkd-address.h +++ b/src/network/networkd-address.h @@ -58,6 +58,7 @@ struct Address { * To control DAD for IPv6 dynamic addresses, set IFA_F_NODAD to flags. */ AddressFamily duplicate_address_detection; sd_ipv4acd *acd; + bool acd_bound; /* Called when address become ready */ address_ready_callback_t callback; @@ -118,12 +119,6 @@ int network_drop_invalid_addresses(Network *network); int address_compare_func(const Address *a1, const Address *a2); DEFINE_NETWORK_CONFIG_STATE_FUNCTIONS(Address, address); -static inline void address_enter_probing(Address *address) { - address_update_state(address, NETWORK_CONFIG_STATE_PROBING, NETWORK_CONFIG_STATE_PROBING); -} -static inline void address_cancel_probing(Address *address) { - address_update_state(address, NETWORK_CONFIG_STATE_PROBING, 0); -} void link_mark_addresses(Link *link, NetworkConfigSource source, const struct in6_addr *router); diff --git a/src/network/networkd-ipv4acd.c b/src/network/networkd-ipv4acd.c index 4f2563ff9c..f55e998f23 100644 --- a/src/network/networkd-ipv4acd.c +++ b/src/network/networkd-ipv4acd.c @@ -39,15 +39,21 @@ bool link_ipv4acd_supported(Link *link) { return true; } +bool ipv4acd_bound(const Address *address) { + assert(address); + + if (!address->acd) + return true; + + return address->acd_bound; +} + static int static_ipv4acd_address_remove(Link *link, Address *address, bool on_conflict) { int r; assert(link); assert(address); - /* Prevent form the address being freed. */ - address_enter_probing(address); - if (!address_exists(address)) return 0; /* Not assigned. */ @@ -85,9 +91,6 @@ static int dhcp4_address_on_conflict(Link *link, Address *address) { if (r < 0) return log_link_warning_errno(link, r, "Failed to drop DHCPv4 lease: %m"); - /* make the address will be freed. */ - address_cancel_probing(address); - /* It is not necessary to call address_remove() here, as dhcp4_lease_lost() removes it. */ return 0; } @@ -108,6 +111,8 @@ static void on_acd(sd_ipv4acd *acd, int event, void *userdata) { switch (event) { case SD_IPV4ACD_EVENT_STOP: + address->acd_bound = false; + if (address->source == NETWORK_CONFIG_SOURCE_STATIC) { r = static_ipv4acd_address_remove(link, address, /* on_conflict = */ false); if (r < 0) @@ -119,13 +124,15 @@ static void on_acd(sd_ipv4acd *acd, int event, void *userdata) { break; case SD_IPV4ACD_EVENT_BIND: + address->acd_bound = true; + log_link_debug(link, "Successfully claimed address "IPV4_ADDRESS_FMT_STR, IPV4_ADDRESS_FMT_VAL(address->in_addr.in)); - - address_cancel_probing(address); break; case SD_IPV4ACD_EVENT_CONFLICT: + address->acd_bound = false; + log_link_warning(link, "Dropping address "IPV4_ADDRESS_FMT_STR", as an address conflict was detected.", IPV4_ADDRESS_FMT_VAL(address->in_addr.in)); @@ -157,6 +164,22 @@ static int ipv4acd_check_mac(sd_ipv4acd *acd, const struct ether_addr *mac, void return link_get_by_hw_addr(m, &hw_addr, NULL) >= 0; } +static int address_ipv4acd_start(Address *address) { + assert(address); + assert(address->link); + + if (!address->acd) + return 0; + + if (sd_ipv4acd_is_running(address->acd)) + return 0; + + if (!link_has_carrier(address->link)) + return 0; + + return sd_ipv4acd_start(address->acd, true); +} + int ipv4acd_configure(Address *address) { Link *link; int r; @@ -177,10 +200,8 @@ int ipv4acd_configure(Address *address) { /* Currently, only static and DHCP4 addresses are supported. */ assert(IN_SET(address->source, NETWORK_CONFIG_SOURCE_STATIC, NETWORK_CONFIG_SOURCE_DHCP4)); - if (address->acd) { - address_enter_probing(address); - return 0; - } + if (address->acd) + return address_ipv4acd_start(address); log_link_debug(link, "Configuring IPv4ACD for address "IPV4_ADDRESS_FMT_STR, IPV4_ADDRESS_FMT_VAL(address->in_addr.in)); @@ -213,14 +234,7 @@ int ipv4acd_configure(Address *address) { if (r < 0) return r; - if (link_has_carrier(link)) { - r = sd_ipv4acd_start(address->acd, true); - if (r < 0) - return r; - } - - address_enter_probing(address); - return 0; + return address_ipv4acd_start(address); } int ipv4acd_update_mac(Link *link) { @@ -255,13 +269,7 @@ int ipv4acd_start(Link *link) { assert(link); SET_FOREACH(address, link->addresses) { - if (!address->acd) - continue; - - if (sd_ipv4acd_is_running(address->acd)) - continue; - - r = sd_ipv4acd_start(address->acd, true); + r = address_ipv4acd_start(address); if (r < 0) return r; } diff --git a/src/network/networkd-ipv4acd.h b/src/network/networkd-ipv4acd.h index 7bd6a26b40..1ec9481b99 100644 --- a/src/network/networkd-ipv4acd.h +++ b/src/network/networkd-ipv4acd.h @@ -5,6 +5,7 @@ typedef struct Address Address; typedef struct Link Link; bool link_ipv4acd_supported(Link *link); +bool ipv4acd_bound(const Address *address); int ipv4acd_configure(Address *address); int ipv4acd_update_mac(Link *link); int ipv4acd_start(Link *link); diff --git a/src/network/networkd-util.c b/src/network/networkd-util.c index 1c0987bc33..aa1b5ed172 100644 --- a/src/network/networkd-util.c +++ b/src/network/networkd-util.c @@ -26,7 +26,6 @@ DEFINE_STRING_TABLE_LOOKUP_TO_STRING(network_config_source, NetworkConfigSource) int network_config_state_to_string_alloc(NetworkConfigState s, char **ret) { static const char* states[] = { - [LOG2U(NETWORK_CONFIG_STATE_PROBING)] = "probing", [LOG2U(NETWORK_CONFIG_STATE_REQUESTING)] = "requesting", [LOG2U(NETWORK_CONFIG_STATE_CONFIGURING)] = "configuring", [LOG2U(NETWORK_CONFIG_STATE_CONFIGURED)] = "configured", diff --git a/src/network/networkd-util.h b/src/network/networkd-util.h index 373184a3ed..84fdc99958 100644 --- a/src/network/networkd-util.h +++ b/src/network/networkd-util.h @@ -27,14 +27,13 @@ typedef enum NetworkConfigSource { } NetworkConfigSource; typedef enum NetworkConfigState { - NETWORK_CONFIG_STATE_PROBING = 1 << 0, /* address is probing by IPv4ACD */ - NETWORK_CONFIG_STATE_REQUESTING = 1 << 1, /* request is queued */ - NETWORK_CONFIG_STATE_CONFIGURING = 1 << 2, /* e.g. address_configure() is called, but no response is received yet */ - NETWORK_CONFIG_STATE_CONFIGURED = 1 << 3, /* e.g. address_configure() is called and received a response from kernel. + NETWORK_CONFIG_STATE_REQUESTING = 1 << 0, /* request is queued */ + NETWORK_CONFIG_STATE_CONFIGURING = 1 << 1, /* e.g. address_configure() is called, but no response is received yet */ + NETWORK_CONFIG_STATE_CONFIGURED = 1 << 2, /* e.g. address_configure() is called and received a response from kernel. * Note that address may not be ready yet, so please use address_is_ready() * to check whether the address can be usable or not. */ - NETWORK_CONFIG_STATE_MARKED = 1 << 4, /* used GC'ing the old config */ - NETWORK_CONFIG_STATE_REMOVING = 1 << 5, /* e.g. address_remove() is called, but no response is received yet */ + NETWORK_CONFIG_STATE_MARKED = 1 << 3, /* used GC'ing the old config */ + NETWORK_CONFIG_STATE_REMOVING = 1 << 4, /* e.g. address_remove() is called, but no response is received yet */ } NetworkConfigState; static inline usec_t sec16_to_usec(uint16_t sec, usec_t timestamp_usec) { From 3c45d0d099927083b906c2ef5e4659fce11ea06a Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 18 Aug 2022 14:32:47 +0900 Subject: [PATCH 2/5] network: update setting for IPv4ACD or IPv6DAD in existing Address objects Otherwise, if the setting is changed on reconfigure, the new setting is not applied to the existing addresses. --- src/network/networkd-address.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/network/networkd-address.c b/src/network/networkd-address.c index 9122c644e8..1e6371c1ef 100644 --- a/src/network/networkd-address.c +++ b/src/network/networkd-address.c @@ -1190,6 +1190,7 @@ int link_request_address( } else { existing->source = address->source; existing->provider = address->provider; + existing->duplicate_address_detection = address->duplicate_address_detection; existing->lifetime_valid_usec = address->lifetime_valid_usec; existing->lifetime_preferred_usec = address->lifetime_preferred_usec; if (consume_object) From 03ff3c5a466cef45ebfd9353eecdfba89281886a Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 18 Aug 2022 15:39:22 +0900 Subject: [PATCH 3/5] network: unref existing sd_ipv4acd object when not necessary On reconfiguring an interface, the new setting may not enable IPv4ACD for an existing address anymore. Hence, we need to unref it. Otherwise, newly requested addresses may never be ready for (re-)configuring. --- src/network/networkd-ipv4acd.c | 34 ++++++++++++++++++++++++---------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/src/network/networkd-ipv4acd.c b/src/network/networkd-ipv4acd.c index f55e998f23..a633575444 100644 --- a/src/network/networkd-ipv4acd.c +++ b/src/network/networkd-ipv4acd.c @@ -39,6 +39,26 @@ bool link_ipv4acd_supported(Link *link) { return true; } +static bool address_ipv4acd_enabled(Address *address) { + assert(address); + assert(address->link); + + if (address->family != AF_INET) + return false; + + if (!FLAGS_SET(address->duplicate_address_detection, ADDRESS_FAMILY_IPV4)) + return false; + + /* Currently, only static and DHCP4 addresses are supported. */ + if (!IN_SET(address->source, NETWORK_CONFIG_SOURCE_STATIC, NETWORK_CONFIG_SOURCE_DHCP4)) + return false; + + if (!link_ipv4acd_supported(address->link)) + return false; + + return true; +} + bool ipv4acd_bound(const Address *address) { assert(address); @@ -188,17 +208,11 @@ int ipv4acd_configure(Address *address) { link = ASSERT_PTR(address->link); - if (address->family != AF_INET) + if (!address_ipv4acd_enabled(address)) { + address->acd = sd_ipv4acd_unref(address->acd); + address->acd_bound = false; return 0; - - if (!FLAGS_SET(address->duplicate_address_detection, ADDRESS_FAMILY_IPV4)) - return 0; - - if (!link_ipv4acd_supported(link)) - return 0; - - /* Currently, only static and DHCP4 addresses are supported. */ - assert(IN_SET(address->source, NETWORK_CONFIG_SOURCE_STATIC, NETWORK_CONFIG_SOURCE_DHCP4)); + } if (address->acd) return address_ipv4acd_start(address); From fd022b1da54f17cba79e453b805ead5736de753c Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 18 Aug 2022 14:49:42 +0900 Subject: [PATCH 4/5] test-network: drop unused .network file --- test/test-network/conf/25-dhcp-client-decline.network | 10 ---------- 1 file changed, 10 deletions(-) delete mode 100644 test/test-network/conf/25-dhcp-client-decline.network diff --git a/test/test-network/conf/25-dhcp-client-decline.network b/test/test-network/conf/25-dhcp-client-decline.network deleted file mode 100644 index ea1a2d0506..0000000000 --- a/test/test-network/conf/25-dhcp-client-decline.network +++ /dev/null @@ -1,10 +0,0 @@ -# SPDX-License-Identifier: LGPL-2.1-or-later -[Match] -Name=veth99 - -[Network] -DHCP=ipv4 -IPv6AcceptRA=false - -[DHCPv4] -SendDecline=yes From 3af934bcebe2686564a58312211f2fc44536cee5 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 18 Aug 2022 15:16:27 +0900 Subject: [PATCH 5/5] test-network: add tests for IPv4ACD and renewing DHCP address --- test/test-network/conf/25-dhcp4-6rd-upstream.network | 1 + test/test-network/systemd-networkd-tests.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/test/test-network/conf/25-dhcp4-6rd-upstream.network b/test/test-network/conf/25-dhcp4-6rd-upstream.network index e146337c5e..fd0d8942bd 100644 --- a/test/test-network/conf/25-dhcp4-6rd-upstream.network +++ b/test/test-network/conf/25-dhcp4-6rd-upstream.network @@ -10,6 +10,7 @@ DHCPPrefixDelegation=yes [DHCPv4] Use6RD=yes +SendDecline=yes [DHCPPrefixDelegation] UplinkInterface=:self diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index 978cc82829..3c1b3e0c1c 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -5289,7 +5289,7 @@ class NetworkdDHCPPDTests(unittest.TestCase, Utilities): self.verify_dhcp4_6rd(tunnel_name) - print('Wait for the DHCP lease to be expired') + print('Wait for the DHCP lease to be renewed/rebind') time.sleep(120) self.wait_online(['veth99:routable', 'test1:routable', 'dummy97:routable', 'dummy98:routable', 'dummy99:degraded',