From 7f74b00ab0040e1fcbd30c53fef5f5579a499e0f Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 3 Jan 2024 04:40:08 +0900 Subject: [PATCH 1/3] network/address: make address_remove() take Link object that the address assigned to No functional change. Preparation for later commits. --- src/network/networkd-address.c | 23 ++++++++++------------- src/network/networkd-address.h | 2 +- src/network/networkd-ipv4acd.c | 2 +- src/network/networkd-ipv4ll.c | 2 +- 4 files changed, 13 insertions(+), 16 deletions(-) diff --git a/src/network/networkd-address.c b/src/network/networkd-address.c index 4f27e3845fb..cfa57aaa71f 100644 --- a/src/network/networkd-address.c +++ b/src/network/networkd-address.c @@ -1087,20 +1087,17 @@ static int address_remove_handler(sd_netlink *rtnl, sd_netlink_message *m, Link return 1; } -int address_remove(Address *address) { +int address_remove(Address *address, Link *link) { _cleanup_(sd_netlink_message_unrefp) sd_netlink_message *m = NULL; Request *req; - Link *link; int r; assert(address); assert(IN_SET(address->family, AF_INET, AF_INET6)); - assert(address->link); - assert(address->link->ifindex > 0); - assert(address->link->manager); - assert(address->link->manager->rtnl); - - link = address->link; + assert(link); + assert(link->ifindex > 0); + assert(link->manager); + assert(link->manager->rtnl); log_address_debug(address, "Removing", link); @@ -1132,13 +1129,13 @@ int address_remove(Address *address) { } int address_remove_and_drop(Address *address) { - if (!address) + if (!address || !address->link) return 0; address_cancel_request(address); if (address_exists(address)) - return address_remove(address); + return address_remove(address, address->link); return address_drop(address); } @@ -1260,7 +1257,7 @@ int link_drop_ipv6ll_addresses(Link *link) { existing = TAKE_PTR(a); } - r = address_remove(existing); + r = address_remove(existing, link); if (r < 0) return r; } @@ -1320,7 +1317,7 @@ int link_drop_foreign_addresses(Link *link) { if (!address_is_marked(address)) continue; - RET_GATHER(r, address_remove(address)); + RET_GATHER(r, address_remove(address, link)); } return r; @@ -1341,7 +1338,7 @@ int link_drop_managed_addresses(Link *link) { if (!address_exists(address)) continue; - RET_GATHER(r, address_remove(address)); + RET_GATHER(r, address_remove(address, link)); } return r; diff --git a/src/network/networkd-address.h b/src/network/networkd-address.h index 8273184d868..786005c1e4b 100644 --- a/src/network/networkd-address.h +++ b/src/network/networkd-address.h @@ -89,7 +89,7 @@ Address* address_free(Address *address); int address_get(Link *link, const Address *in, Address **ret); int address_get_harder(Link *link, const Address *in, Address **ret); int address_configure_handler_internal(sd_netlink *rtnl, sd_netlink_message *m, Link *link, const char *error_msg); -int address_remove(Address *address); +int address_remove(Address *address, Link *link); int address_remove_and_drop(Address *address); int address_dup(const Address *src, Address **ret); bool address_is_ready(const Address *a); diff --git a/src/network/networkd-ipv4acd.c b/src/network/networkd-ipv4acd.c index 3d5e2036aa7..ee1f31ef0c6 100644 --- a/src/network/networkd-ipv4acd.c +++ b/src/network/networkd-ipv4acd.c @@ -92,7 +92,7 @@ static int static_ipv4acd_address_remove(Link *link, Address *address, bool on_c else log_link_debug(link, "Removing address %s, as the ACD client is stopped.", IN4_ADDR_TO_STRING(&address->in_addr.in)); - r = address_remove(address); + r = address_remove(address, link); if (r < 0) return log_link_warning_errno(link, r, "Failed to remove address %s: %m", IN4_ADDR_TO_STRING(&address->in_addr.in)); diff --git a/src/network/networkd-ipv4ll.c b/src/network/networkd-ipv4ll.c index c35738252bb..3fccb6d13c9 100644 --- a/src/network/networkd-ipv4ll.c +++ b/src/network/networkd-ipv4ll.c @@ -82,7 +82,7 @@ static int ipv4ll_address_lost(Link *link) { log_link_debug(link, "IPv4 link-local release "IPV4_ADDRESS_FMT_STR, IPV4_ADDRESS_FMT_VAL(address->in_addr.in)); - return address_remove(existing); + return address_remove(existing, link); } static int ipv4ll_address_handler(sd_netlink *rtnl, sd_netlink_message *m, Request *req, Link *link, Address *address) { From f1b73dbbc95d2414bc1acb35ccae25b94680d0df Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 3 Jan 2024 04:40:24 +0900 Subject: [PATCH 2/3] network/address: not necessary to remmber address before remove --- src/network/networkd-address.c | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/src/network/networkd-address.c b/src/network/networkd-address.c index cfa57aaa71f..dd1814bb994 100644 --- a/src/network/networkd-address.c +++ b/src/network/networkd-address.c @@ -1203,7 +1203,6 @@ int link_drop_ipv6ll_addresses(Link *link) { _cleanup_(address_freep) Address *a = NULL; unsigned char flags, prefixlen; struct in6_addr address; - Address *existing; int ifindex; /* NETLINK_GET_STRICT_CHK socket option is supported since kernel 4.20. To support @@ -1249,15 +1248,7 @@ int link_drop_ipv6ll_addresses(Link *link) { a->prefixlen = prefixlen; a->flags = flags; - if (address_get(link, a, &existing) < 0) { - r = address_add(link, a); - if (r < 0) - return r; - - existing = TAKE_PTR(a); - } - - r = address_remove(existing, link); + r = address_remove(a, link); if (r < 0) return r; } From f22b586a215962416bdbd692aabb89b1ac2999d0 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 3 Jan 2024 04:40:29 +0900 Subject: [PATCH 3/3] network/address: introduce address_remove_and_cancel() Then, replace address_remove_and_drop() with it. If an address is requested, and the request is already called, we may not received its reply and notification from the kernel, and the corresponding address object may not be remmbered. Even in such case, we need to remove the address, otherwise the address will come later after the function called. --- src/network/networkd-address.c | 52 ++++++++----------- src/network/networkd-address.h | 3 +- src/network/networkd-dhcp-prefix-delegation.c | 2 +- src/network/networkd-dhcp4.c | 2 +- src/network/networkd-dhcp6.c | 2 +- src/network/networkd-ipv4acd.c | 2 + src/network/networkd-ipv4ll.c | 12 +---- src/network/networkd-ndisc.c | 2 +- 8 files changed, 29 insertions(+), 48 deletions(-) diff --git a/src/network/networkd-address.c b/src/network/networkd-address.c index dd1814bb994..f0924c93228 100644 --- a/src/network/networkd-address.c +++ b/src/network/networkd-address.c @@ -1089,7 +1089,6 @@ static int address_remove_handler(sd_netlink *rtnl, sd_netlink_message *m, Link int address_remove(Address *address, Link *link) { _cleanup_(sd_netlink_message_unrefp) sd_netlink_message *m = NULL; - Request *req; int r; assert(address); @@ -1119,8 +1118,6 @@ int address_remove(Address *address, Link *link) { link_ref(link); address_enter_removing(address); - if (address_get_request(link, address, &req) >= 0) - address_enter_removing(req->userdata); /* The operational state is determined by address state and carrier state. Hence, if we remove * an address, the operational state may be changed. */ @@ -1128,16 +1125,30 @@ int address_remove(Address *address, Link *link) { return 0; } -int address_remove_and_drop(Address *address) { - if (!address || !address->link) - return 0; +int address_remove_and_cancel(Address *address, Link *link) { + bool waiting = false; + Request *req; - address_cancel_request(address); + assert(address); + assert(link); + assert(link->manager); - if (address_exists(address)) - return address_remove(address, address->link); + /* If the address is remembered by the link, then use the remembered object. */ + (void) address_get(link, address, &address); - return address_drop(address); + /* Cancel the request for the address. If the request is already called but we have not received the + * notification about the request, then explicitly remove the address. */ + if (address_get_request(link, address, &req) >= 0) { + waiting = req->waiting_reply; + request_detach(link->manager, req); + address_cancel_requesting(address); + } + + /* If we know the address will come or already exists, remove it. */ + if (waiting || (address->link && address_exists(address))) + return address_remove(address, link); + + return 0; } bool link_address_is_dynamic(const Link *link, const Address *address) { @@ -1632,27 +1643,6 @@ int link_request_static_addresses(Link *link) { return 0; } -void address_cancel_request(Address *address) { - Request req; - - assert(address); - assert(address->link); - - if (!address_is_requesting(address)) - return; - - req = (Request) { - .link = address->link, - .type = REQUEST_TYPE_ADDRESS, - .userdata = address, - .hash_func = (hash_func_t) address_hash_func, - .compare_func = (compare_func_t) address_compare_func, - }; - - request_detach(address->link->manager, &req); - address_cancel_requesting(address); -} - int manager_rtnl_process_address(sd_netlink *rtnl, sd_netlink_message *message, Manager *m) { _cleanup_(address_freep) Address *tmp = NULL; struct ifa_cacheinfo cinfo; diff --git a/src/network/networkd-address.h b/src/network/networkd-address.h index 786005c1e4b..53e7a798212 100644 --- a/src/network/networkd-address.h +++ b/src/network/networkd-address.h @@ -90,7 +90,7 @@ int address_get(Link *link, const Address *in, Address **ret); int address_get_harder(Link *link, const Address *in, Address **ret); int address_configure_handler_internal(sd_netlink *rtnl, sd_netlink_message *m, Link *link, const char *error_msg); int address_remove(Address *address, Link *link); -int address_remove_and_drop(Address *address); +int address_remove_and_cancel(Address *address, Link *link); int address_dup(const Address *src, Address **ret); bool address_is_ready(const Address *a); bool link_check_addresses_ready(Link *link, NetworkConfigSource source); @@ -114,7 +114,6 @@ static inline int link_get_ipv4_address(Link *link, const struct in_addr *addres int manager_get_address(Manager *manager, int family, const union in_addr_union *address, unsigned char prefixlen, Address **ret); bool manager_has_address(Manager *manager, int family, const union in_addr_union *address); -void address_cancel_request(Address *address); int link_request_address( Link *link, const Address *address, diff --git a/src/network/networkd-dhcp-prefix-delegation.c b/src/network/networkd-dhcp-prefix-delegation.c index ad49f2ee8b2..f5d97dd7af2 100644 --- a/src/network/networkd-dhcp-prefix-delegation.c +++ b/src/network/networkd-dhcp-prefix-delegation.c @@ -192,7 +192,7 @@ int dhcp_pd_remove(Link *link, bool only_marked) { link_remove_dhcp_pd_subnet_prefix(link, &prefix); - RET_GATHER(ret, address_remove_and_drop(address)); + RET_GATHER(ret, address_remove_and_cancel(address, link)); } } diff --git a/src/network/networkd-dhcp4.c b/src/network/networkd-dhcp4.c index bca426f6237..8aee30e7266 100644 --- a/src/network/networkd-dhcp4.c +++ b/src/network/networkd-dhcp4.c @@ -263,7 +263,7 @@ static int dhcp4_remove_address_and_routes(Link *link, bool only_marked) { if (only_marked && !address_is_marked(address)) continue; - RET_GATHER(ret, address_remove_and_drop(address)); + RET_GATHER(ret, address_remove_and_cancel(address, link)); } return ret; diff --git a/src/network/networkd-dhcp6.c b/src/network/networkd-dhcp6.c index 6854533eed6..6d8b3c3ce26 100644 --- a/src/network/networkd-dhcp6.c +++ b/src/network/networkd-dhcp6.c @@ -71,7 +71,7 @@ static int dhcp6_remove(Link *link, bool only_marked) { if (only_marked && !address_is_marked(address)) continue; - RET_GATHER(ret, address_remove_and_drop(address)); + RET_GATHER(ret, address_remove_and_cancel(address, link)); } return ret; diff --git a/src/network/networkd-ipv4acd.c b/src/network/networkd-ipv4acd.c index ee1f31ef0c6..de03293fff3 100644 --- a/src/network/networkd-ipv4acd.c +++ b/src/network/networkd-ipv4acd.c @@ -92,6 +92,8 @@ static int static_ipv4acd_address_remove(Link *link, Address *address, bool on_c else log_link_debug(link, "Removing address %s, as the ACD client is stopped.", IN4_ADDR_TO_STRING(&address->in_addr.in)); + /* Do not call address_remove_and_cancel() here. Otherwise, the request is cancelled, and the + * interface may be in configured state without the address. */ r = address_remove(address, link); if (r < 0) return log_link_warning_errno(link, r, "Failed to remove address %s: %m", IN4_ADDR_TO_STRING(&address->in_addr.in)); diff --git a/src/network/networkd-ipv4ll.c b/src/network/networkd-ipv4ll.c index 3fccb6d13c9..629a3734095 100644 --- a/src/network/networkd-ipv4ll.c +++ b/src/network/networkd-ipv4ll.c @@ -57,7 +57,6 @@ static int address_new_from_ipv4ll(Link *link, Address **ret) { static int ipv4ll_address_lost(Link *link) { _cleanup_(address_freep) Address *address = NULL; - Address *existing; int r; assert(link); @@ -70,19 +69,10 @@ static int ipv4ll_address_lost(Link *link) { if (r < 0) return r; - if (address_get(link, address, &existing) < 0) - return 0; - - if (existing->source != NETWORK_CONFIG_SOURCE_IPV4LL) - return 0; - - if (!address_exists(existing)) - return 0; - log_link_debug(link, "IPv4 link-local release "IPV4_ADDRESS_FMT_STR, IPV4_ADDRESS_FMT_VAL(address->in_addr.in)); - return address_remove(existing, link); + return address_remove_and_cancel(address, link); } static int ipv4ll_address_handler(sd_netlink *rtnl, sd_netlink_message *m, Request *req, Link *link, Address *address) { diff --git a/src/network/networkd-ndisc.c b/src/network/networkd-ndisc.c index ddd60a1e574..369e205a99a 100644 --- a/src/network/networkd-ndisc.c +++ b/src/network/networkd-ndisc.c @@ -1137,7 +1137,7 @@ static int ndisc_drop_outdated(Link *link, usec_t timestamp_usec) { if (address->lifetime_valid_usec >= timestamp_usec) continue; /* the address is still valid */ - r = address_remove_and_drop(address); + r = address_remove_and_cancel(address, link); if (r < 0) RET_GATHER(ret, log_link_warning_errno(link, r, "Failed to remove outdated SLAAC address, ignoring: %m")); }