From a45e1c35e34c2568ea36c22fa01a1cb07194668b Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 14 Feb 2024 13:02:56 +0900 Subject: [PATCH 1/4] network/address: drop requirement of Address.link and .network in address_can_update() This also rename arguments. No functional change, just refactoring. --- src/network/networkd-address.c | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/src/network/networkd-address.c b/src/network/networkd-address.c index a7aa166b4b..d1087c5f12 100644 --- a/src/network/networkd-address.c +++ b/src/network/networkd-address.c @@ -516,11 +516,9 @@ static int address_compare_func(const Address *a1, const Address *a2) { } } -static bool address_can_update(const Address *la, const Address *na) { - assert(la); - assert(la->link); - assert(na); - assert(na->network); +static bool address_can_update(const Address *existing, const Address *requesting) { + assert(existing); + assert(requesting); /* * property | IPv4 | IPv6 @@ -545,32 +543,32 @@ static bool address_can_update(const Address *la, const Address *na) { * IPv6 : See inet6_addr_modify() in net/ipv6/addrconf.c. */ - if (la->family != na->family) + if (existing->family != requesting->family) return false; - if (la->prefixlen != na->prefixlen) + if (existing->prefixlen != requesting->prefixlen) return false; /* When a null address is requested, the address to be assigned/updated will be determined later. */ - if (!address_is_static_null(na) && - in_addr_equal(la->family, &la->in_addr, &na->in_addr) <= 0) + if (!address_is_static_null(requesting) && + in_addr_equal(existing->family, &existing->in_addr, &requesting->in_addr) <= 0) return false; - switch (la->family) { + switch (existing->family) { case AF_INET: { struct in_addr bcast; - if (la->scope != na->scope) + if (existing->scope != requesting->scope) return false; - if (((la->flags ^ na->flags) & KNOWN_FLAGS & ~IPV6ONLY_FLAGS & ~UNMANAGED_FLAGS) != 0) + if (((existing->flags ^ requesting->flags) & KNOWN_FLAGS & ~IPV6ONLY_FLAGS & ~UNMANAGED_FLAGS) != 0) return false; - if (!streq_ptr(la->label, na->label)) + if (!streq_ptr(existing->label, requesting->label)) return false; - if (!in4_addr_equal(&la->in_addr_peer.in, &na->in_addr_peer.in)) + if (!in4_addr_equal(&existing->in_addr_peer.in, &requesting->in_addr_peer.in)) return false; - if (address_get_broadcast(na, la->link, &bcast) >= 0) { + if (existing->link && address_get_broadcast(requesting, existing->link, &bcast) >= 0) { /* If the broadcast address can be determined now, check if they match. */ - if (!in4_addr_equal(&la->broadcast, &bcast)) + if (!in4_addr_equal(&existing->broadcast, &bcast)) return false; } else { /* When a null address is requested, then the broadcast address will be @@ -578,7 +576,7 @@ static bool address_can_update(const Address *la, const Address *na) { * 192.168.0.10/24 -> 192.168.0.255 * So, here let's only check if the broadcast is the last address in the range, e.g. * 0.0.0.0/24 -> 0.0.0.255 */ - if (!FLAGS_SET(la->broadcast.s_addr, htobe32(UINT32_C(0xffffffff) >> la->prefixlen))) + if (!FLAGS_SET(existing->broadcast.s_addr, htobe32(UINT32_C(0xffffffff) >> existing->prefixlen))) return false; } break; From 77de62f96c16592844374769c84932dc7c75eef2 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 14 Feb 2024 13:27:00 +0900 Subject: [PATCH 2/4] network/dhcp6: use address_can_update() at one more place In address_can_update(), only prefix length is checked for IPv6 addresses. So, this should not change any behavior. --- src/network/networkd-address.c | 2 +- src/network/networkd-address.h | 2 ++ src/network/networkd-dhcp6.c | 3 +-- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/network/networkd-address.c b/src/network/networkd-address.c index d1087c5f12..2aa8c88bb2 100644 --- a/src/network/networkd-address.c +++ b/src/network/networkd-address.c @@ -516,7 +516,7 @@ static int address_compare_func(const Address *a1, const Address *a2) { } } -static bool address_can_update(const Address *existing, const Address *requesting) { +bool address_can_update(const Address *existing, const Address *requesting) { assert(existing); assert(requesting); diff --git a/src/network/networkd-address.h b/src/network/networkd-address.h index cc2429094e..350d2f6b38 100644 --- a/src/network/networkd-address.h +++ b/src/network/networkd-address.h @@ -85,6 +85,8 @@ void link_get_address_states( extern const struct hash_ops address_hash_ops; +bool address_can_update(const Address *existing, const Address *requesting); + Address* address_ref(Address *address); Address* address_unref(Address *address); diff --git a/src/network/networkd-dhcp6.c b/src/network/networkd-dhcp6.c index eb1420be5c..de4588c93e 100644 --- a/src/network/networkd-dhcp6.c +++ b/src/network/networkd-dhcp6.c @@ -158,8 +158,7 @@ static int verify_dhcp6_address(Link *link, const Address *address) { } else log_level = LOG_DEBUG; - if (address->prefixlen == existing->prefixlen) - /* Currently, only conflict in prefix length is reported. */ + if (address_can_update(existing, address)) goto simple_log; if (existing->source == NETWORK_CONFIG_SOURCE_NDISC) From 9e79ef91207e760b5f8d257a8c73f52eda1d4831 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 14 Feb 2024 13:46:51 +0900 Subject: [PATCH 3/4] network/ndisc: drop unnecessary ownership transfer Follow-up for e217da13803518fb9592a18b0ec7dade513878a0. --- src/network/networkd-ndisc.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/network/networkd-ndisc.c b/src/network/networkd-ndisc.c index f5621e219b..7e6758714b 100644 --- a/src/network/networkd-ndisc.c +++ b/src/network/networkd-ndisc.c @@ -249,8 +249,7 @@ static int ndisc_address_handler(sd_netlink *rtnl, sd_netlink_message *m, Reques return 1; } -static int ndisc_request_address(Address *in, Link *link, sd_ndisc_router *rt) { - _cleanup_(address_unrefp) Address *address = in; +static int ndisc_request_address(Address *address, Link *link, sd_ndisc_router *rt) { struct in6_addr router; bool is_new; int r; @@ -485,7 +484,7 @@ static int ndisc_router_process_autonomous_prefix(Link *link, sd_ndisc_router *r address->lifetime_valid_usec = lifetime_valid_usec; address->lifetime_preferred_usec = lifetime_preferred_usec; - r = ndisc_request_address(TAKE_PTR(address), link, rt); + r = ndisc_request_address(address, link, rt); if (r < 0) return log_link_warning_errno(link, r, "Could not request SLAAC address: %m"); } From e720ad88f37e0c983df6b13d4e417968aa565b5e Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 14 Feb 2024 13:39:48 +0900 Subject: [PATCH 4/4] network/ndisc: check if there exists a conflicting address Follow-up for 0a0c2672dbd22dc85d660e5baa7e1bef701beb88. Before the commit, if a conflicting address exists or already requested, then the configuration of newly requested address (especially, prefix length) is mostly ignored silently. However, after the commit, even if there exists a conflicting address, networkd anyway tries to configure the newly requested address, and enter failed state. Such situation can be triggered, e.g. when the DHCPv6 client is started earlier than NDisc, by WithoutRA=solicit. Fixes #31263. --- src/network/networkd-ndisc.c | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/src/network/networkd-ndisc.c b/src/network/networkd-ndisc.c index 7e6758714b..030b4a4339 100644 --- a/src/network/networkd-ndisc.c +++ b/src/network/networkd-ndisc.c @@ -269,7 +269,26 @@ static int ndisc_request_address(Address *address, Link *link, sd_ndisc_router * if (r < 0) return r; - is_new = address_get(link, address, NULL) < 0; + Address *existing; + if (address_get_harder(link, address, &existing) < 0) + is_new = true; + else if (address_can_update(existing, address)) + is_new = false; + else if (existing->source == NETWORK_CONFIG_SOURCE_DHCP6) { + /* SLAAC address is preferred over DHCPv6 address. */ + log_link_debug(link, "Conflicting DHCPv6 address %s exists, removing.", + IN_ADDR_PREFIX_TO_STRING(existing->family, &existing->in_addr, existing->prefixlen)); + r = address_remove(existing, link); + if (r < 0) + return r; + + is_new = true; + } else { + /* Conflicting static address is configured?? */ + log_link_debug(link, "Conflicting address %s exists, ignoring request.", + IN_ADDR_PREFIX_TO_STRING(existing->family, &existing->in_addr, existing->prefixlen)); + return 0; + } r = link_request_address(link, address, &link->ndisc_messages, ndisc_address_handler, NULL);