From f60e65583e2d5b34a97ce1bfa40b6af3bcc24f24 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 9 Jul 2023 07:15:21 +0900 Subject: [PATCH 1/4] network/address: free Address object by caller that passed to link_request_address() Follow-up for 9684a8ded083dd427f843b0c40aa0292e6c7ae06. Now, the input Address object is always copied, hence it is not necessary to free it in link_request_address(). --- src/network/networkd-address.c | 13 ++++--------- src/network/networkd-address.h | 5 ++--- src/network/networkd-dhcp-prefix-delegation.c | 2 +- src/network/networkd-dhcp-server.c | 2 +- src/network/networkd-dhcp4.c | 2 +- src/network/networkd-dhcp6.c | 2 +- src/network/networkd-ipv4ll.c | 2 +- src/network/networkd-ndisc.c | 2 +- src/network/networkd-radv.c | 2 +- 9 files changed, 13 insertions(+), 19 deletions(-) diff --git a/src/network/networkd-address.c b/src/network/networkd-address.c index c33ffa9576..276aad0989 100644 --- a/src/network/networkd-address.c +++ b/src/network/networkd-address.c @@ -1246,13 +1246,11 @@ static int address_process_request(Request *req, Link *link, Address *address) { int link_request_address( Link *link, - Address *address, - bool consume_object, + const Address *address, unsigned *message_counter, address_netlink_handler_t netlink_handler, Request **ret) { - _unused_ _cleanup_(address_freep) Address *address_will_be_freed = NULL; Address *existing; int r; @@ -1260,9 +1258,6 @@ int link_request_address( assert(address); assert(address->source != NETWORK_CONFIG_SOURCE_FOREIGN); - if (consume_object) - address_will_be_freed = address; - if (address_get(link, address, &existing) < 0) { _cleanup_(address_freep) Address *tmp = NULL; @@ -1341,12 +1336,12 @@ static int static_address_handler(sd_netlink *rtnl, sd_netlink_message *m, Reque return 1; } -int link_request_static_address(Link *link, Address *address, bool consume) { +int link_request_static_address(Link *link, const Address *address) { assert(link); assert(address); assert(address->source == NETWORK_CONFIG_SOURCE_STATIC); - return link_request_address(link, address, consume, &link->static_address_messages, + return link_request_address(link, address, &link->static_address_messages, static_address_handler, NULL); } @@ -1360,7 +1355,7 @@ int link_request_static_addresses(Link *link) { link->static_addresses_configured = false; ORDERED_HASHMAP_FOREACH(a, link->network->addresses_by_section) { - r = link_request_static_address(link, a, false); + r = link_request_static_address(link, a); if (r < 0) return r; } diff --git a/src/network/networkd-address.h b/src/network/networkd-address.h index 766da8ee75..5cb1cc388e 100644 --- a/src/network/networkd-address.h +++ b/src/network/networkd-address.h @@ -107,12 +107,11 @@ bool manager_has_address(Manager *manager, int family, const union in_addr_union void address_cancel_request(Address *address); int link_request_address( Link *link, - Address *address, - bool consume_object, + const Address *address, unsigned *message_counter, address_netlink_handler_t netlink_handler, Request **ret); -int link_request_static_address(Link *link, Address *address, bool consume); +int link_request_static_address(Link *link, const Address *address); int link_request_static_addresses(Link *link); int manager_rtnl_process_address(sd_netlink *nl, sd_netlink_message *message, Manager *m); diff --git a/src/network/networkd-dhcp-prefix-delegation.c b/src/network/networkd-dhcp-prefix-delegation.c index 66c5e979d9..9e39fa649e 100644 --- a/src/network/networkd-dhcp-prefix-delegation.c +++ b/src/network/networkd-dhcp-prefix-delegation.c @@ -421,7 +421,7 @@ static int dhcp_pd_request_address( else address_unmark(existing); - r = link_request_address(link, TAKE_PTR(address), true, &link->dhcp_pd_messages, + r = link_request_address(link, address, &link->dhcp_pd_messages, dhcp_pd_address_handler, NULL); if (r < 0) return log_link_error_errno(link, r, "Failed to request DHCP delegated prefix address: %m"); diff --git a/src/network/networkd-dhcp-server.c b/src/network/networkd-dhcp-server.c index 620fbbddc7..50d84893c6 100644 --- a/src/network/networkd-dhcp-server.c +++ b/src/network/networkd-dhcp-server.c @@ -115,7 +115,7 @@ int link_request_dhcp_server_address(Link *link) { * Configure the DHCP server address only when it is not. */ return 0; - return link_request_static_address(link, TAKE_PTR(address), true); + return link_request_static_address(link, address); } static int link_find_dhcp_server_address(Link *link, Address **ret) { diff --git a/src/network/networkd-dhcp4.c b/src/network/networkd-dhcp4.c index fd8ef402bd..fd72f73413 100644 --- a/src/network/networkd-dhcp4.c +++ b/src/network/networkd-dhcp4.c @@ -984,7 +984,7 @@ static int dhcp4_request_address(Link *link, bool announce) { else address_unmark(existing); - r = link_request_address(link, TAKE_PTR(addr), true, &link->dhcp4_messages, + r = link_request_address(link, addr, &link->dhcp4_messages, dhcp4_address_handler, NULL); if (r < 0) return log_link_error_errno(link, r, "Failed to request DHCPv4 address: %m"); diff --git a/src/network/networkd-dhcp6.c b/src/network/networkd-dhcp6.c index 57272e7bf6..44c9a95f8d 100644 --- a/src/network/networkd-dhcp6.c +++ b/src/network/networkd-dhcp6.c @@ -232,7 +232,7 @@ static int dhcp6_request_address( else address_unmark(existing); - r = link_request_address(link, TAKE_PTR(addr), true, &link->dhcp6_messages, + r = link_request_address(link, addr, &link->dhcp6_messages, dhcp6_address_handler, NULL); if (r < 0) return log_link_error_errno(link, r, "Failed to request DHCPv6 address %s/128: %m", diff --git a/src/network/networkd-ipv4ll.c b/src/network/networkd-ipv4ll.c index 8450b346b1..ee78fffdfb 100644 --- a/src/network/networkd-ipv4ll.c +++ b/src/network/networkd-ipv4ll.c @@ -120,7 +120,7 @@ static int ipv4ll_address_claimed(sd_ipv4ll *ll, Link *link) { log_link_debug(link, "IPv4 link-local claim "IPV4_ADDRESS_FMT_STR, IPV4_ADDRESS_FMT_VAL(address->in_addr.in)); - return link_request_address(link, TAKE_PTR(address), true, NULL, ipv4ll_address_handler, NULL); + return link_request_address(link, address, NULL, ipv4ll_address_handler, NULL); } static void ipv4ll_handler(sd_ipv4ll *ll, int event, void *userdata) { diff --git a/src/network/networkd-ndisc.c b/src/network/networkd-ndisc.c index 8e6389d202..972ba52b9e 100644 --- a/src/network/networkd-ndisc.c +++ b/src/network/networkd-ndisc.c @@ -250,7 +250,7 @@ static int ndisc_request_address(Address *in, Link *link, sd_ndisc_router *rt) { is_new = address_get(link, address, NULL) < 0; - r = link_request_address(link, TAKE_PTR(address), true, &link->ndisc_messages, + r = link_request_address(link, address, &link->ndisc_messages, ndisc_address_handler, NULL); if (r < 0) return r; diff --git a/src/network/networkd-radv.c b/src/network/networkd-radv.c index db1ecb4c97..0f67ff0973 100644 --- a/src/network/networkd-radv.c +++ b/src/network/networkd-radv.c @@ -214,7 +214,7 @@ int link_request_radv_addresses(Link *link) { address->prefixlen = p->prefixlen; address->route_metric = p->route_metric; - r = link_request_static_address(link, TAKE_PTR(address), true); + r = link_request_static_address(link, address); if (r < 0) return r; } From 5fb1f929d5d191670bce602ca02dcbb2fbfde7af Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 10 Jul 2023 09:30:58 +0900 Subject: [PATCH 2/4] network: use address_remove_and_drop() --- src/network/networkd-dhcp-prefix-delegation.c | 4 +--- src/network/networkd-dhcp4.c | 4 +--- src/network/networkd-dhcp6.c | 4 +--- 3 files changed, 3 insertions(+), 9 deletions(-) diff --git a/src/network/networkd-dhcp-prefix-delegation.c b/src/network/networkd-dhcp-prefix-delegation.c index 9e39fa649e..8d50da042c 100644 --- a/src/network/networkd-dhcp-prefix-delegation.c +++ b/src/network/networkd-dhcp-prefix-delegation.c @@ -216,11 +216,9 @@ int dhcp_pd_remove(Link *link, bool only_marked) { link_remove_dhcp_pd_subnet_prefix(link, &prefix); - k = address_remove(address); + k = address_remove_and_drop(address); if (k < 0) r = k; - - address_cancel_request(address); } } diff --git a/src/network/networkd-dhcp4.c b/src/network/networkd-dhcp4.c index fd72f73413..eeb3585f29 100644 --- a/src/network/networkd-dhcp4.c +++ b/src/network/networkd-dhcp4.c @@ -258,11 +258,9 @@ static int dhcp4_remove_address_and_routes(Link *link, bool only_marked) { if (only_marked && !address_is_marked(address)) continue; - k = address_remove(address); + k = address_remove_and_drop(address); if (k < 0) r = k; - - address_cancel_request(address); } return r; diff --git a/src/network/networkd-dhcp6.c b/src/network/networkd-dhcp6.c index 44c9a95f8d..755957f5b1 100644 --- a/src/network/networkd-dhcp6.c +++ b/src/network/networkd-dhcp6.c @@ -72,11 +72,9 @@ static int dhcp6_remove(Link *link, bool only_marked) { if (only_marked && !address_is_marked(address)) continue; - k = address_remove(address); + k = address_remove_and_drop(address); if (k < 0) r = k; - - address_cancel_request(address); } return r; From a3a25d01ced27efb40931935e0069291e36bbe7b Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 10 Jul 2023 09:38:38 +0900 Subject: [PATCH 3/4] network: drop unnecessary conditions When link_get_by_index() succeeds, the result is always non-NULL. --- src/network/networkd-address.c | 4 ++-- src/network/networkd-neighbor.c | 2 +- src/network/networkd-nexthop.c | 2 +- src/network/networkd-route.c | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/network/networkd-address.c b/src/network/networkd-address.c index 276aad0989..7ddc58337b 100644 --- a/src/network/networkd-address.c +++ b/src/network/networkd-address.c @@ -1403,7 +1403,7 @@ void address_cancel_request(Address *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; - Link *link = NULL; + Link *link; uint16_t type; Address *address = NULL; int ifindex, r; @@ -1439,7 +1439,7 @@ int manager_rtnl_process_address(sd_netlink *rtnl, sd_netlink_message *message, } r = link_get_by_index(m, ifindex, &link); - if (r < 0 || !link) { + if (r < 0) { /* when enumerating we might be out of sync, but we will get the address again, so just * ignore it */ if (!m->enumerating) diff --git a/src/network/networkd-neighbor.c b/src/network/networkd-neighbor.c index e3c332eadb..05ad0738e6 100644 --- a/src/network/networkd-neighbor.c +++ b/src/network/networkd-neighbor.c @@ -490,7 +490,7 @@ int manager_rtnl_process_neighbor(sd_netlink *rtnl, sd_netlink_message *message, } r = link_get_by_index(m, ifindex, &link); - if (r < 0 || !link) { + if (r < 0) { /* when enumerating we might be out of sync, but we will get the neighbor again. Also, * kernel sends messages about neighbors after a link is removed. So, just ignore it. */ log_debug("rtnl: received neighbor for link '%d' we don't know about, ignoring.", ifindex); diff --git a/src/network/networkd-nexthop.c b/src/network/networkd-nexthop.c index eed28440c2..3eda1f1b7d 100644 --- a/src/network/networkd-nexthop.c +++ b/src/network/networkd-nexthop.c @@ -835,7 +835,7 @@ int manager_rtnl_process_nexthop(sd_netlink *rtnl, sd_netlink_message *message, } r = link_get_by_index(m, ifindex, &link); - if (r < 0 || !link) { + if (r < 0) { if (!m->enumerating) log_warning("rtnl: received nexthop message for link (%"PRIu32") we do not know about, ignoring", ifindex); return 0; diff --git a/src/network/networkd-route.c b/src/network/networkd-route.c index 44dc63b642..283d515b81 100644 --- a/src/network/networkd-route.c +++ b/src/network/networkd-route.c @@ -1719,7 +1719,7 @@ int manager_rtnl_process_route(sd_netlink *rtnl, sd_netlink_message *message, Ma } r = link_get_by_index(m, ifindex, &link); - if (r < 0 || !link) { + if (r < 0) { /* when enumerating we might be out of sync, but we will * get the route again, so just ignore it */ if (!m->enumerating) From a2532c9db871e9d64ed47ad53a7fbd06fc2df530 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 9 Jul 2023 07:10:57 +0900 Subject: [PATCH 4/4] network/address: split-out address_match_null() No functional change, preparation for later commits. --- src/network/networkd-address.c | 33 ++++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/src/network/networkd-address.c b/src/network/networkd-address.c index 7ddc58337b..3345f83506 100644 --- a/src/network/networkd-address.c +++ b/src/network/networkd-address.c @@ -587,6 +587,27 @@ static int address_drop(Address *address) { return 0; } +static bool address_match_null(const Address *a, const Address *null_address) { + assert(a); + assert(null_address); + + if (!a->requested_as_null) + return false; + + /* Currently, null address is supported only by static addresses. Note that static + * address may be set as foreign during reconfiguring the interface. */ + if (!IN_SET(a->source, NETWORK_CONFIG_SOURCE_FOREIGN, NETWORK_CONFIG_SOURCE_STATIC)) + return false; + + if (a->family != null_address->family) + return false; + + if (a->prefixlen != null_address->prefixlen) + return false; + + return true; +} + int address_get(Link *link, const Address *in, Address **ret) { Address *a; @@ -603,17 +624,7 @@ int address_get(Link *link, const Address *in, Address **ret) { /* Find matching address that originally requested as null address. */ if (address_is_static_null(in)) SET_FOREACH(a, link->addresses) { - if (!a->requested_as_null) - continue; - - /* Currently, null address is supported only by static addresses. Note that static - * address may be set as foreign during reconfiguring the interface. */ - if (!IN_SET(a->source, NETWORK_CONFIG_SOURCE_FOREIGN, NETWORK_CONFIG_SOURCE_STATIC)) - continue; - - if (a->family != in->family) - continue; - if (a->prefixlen != in->prefixlen) + if (!address_match_null(a, in)) continue; if (ret)