From ebd96906477aac2bbc6b9de0d6e9bd0f39db5581 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 3 Jan 2024 04:41:14 +0900 Subject: [PATCH] network/address: introduce ref/unref functions for Address object Then, Address object can live if it is detached from the owner (Link or Network object). --- src/network/networkd-address.c | 141 +++++++++++------- src/network/networkd-address.h | 8 +- 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 | 6 +- src/network/networkd-ndisc.c | 4 +- src/network/networkd-network.c | 4 +- src/network/networkd-radv.c | 2 +- 10 files changed, 109 insertions(+), 64 deletions(-) diff --git a/src/network/networkd-address.c b/src/network/networkd-address.c index f0924c93228..e54c1223614 100644 --- a/src/network/networkd-address.c +++ b/src/network/networkd-address.c @@ -133,14 +133,40 @@ void link_get_address_states( *ret_all = address_state_from_scope(MIN(ipv4_scope, ipv6_scope)); } +static void address_hash_func(const Address *a, struct siphash *state); +static int address_compare_func(const Address *a1, const Address *a2); +static void address_detach(Address *address); + +DEFINE_PRIVATE_HASH_OPS_WITH_KEY_DESTRUCTOR( + address_hash_ops_detach, + Address, + address_hash_func, + address_compare_func, + address_detach); + +DEFINE_HASH_OPS( + address_hash_ops, + Address, + address_hash_func, + address_compare_func); + +DEFINE_HASH_OPS_WITH_VALUE_DESTRUCTOR( + address_section_hash_ops, + ConfigSection, + config_section_hash_func, + config_section_compare_func, + Address, + address_detach); + int address_new(Address **ret) { - _cleanup_(address_freep) Address *address = NULL; + _cleanup_(address_unrefp) Address *address = NULL; address = new(Address, 1); if (!address) return -ENOMEM; *address = (Address) { + .n_ref = 1, .family = AF_UNSPEC, .scope = RT_SCOPE_UNIVERSE, .lifetime_valid_usec = USEC_INFINITY, @@ -155,7 +181,7 @@ int address_new(Address **ret) { int address_new_static(Network *network, const char *filename, unsigned section_line, Address **ret) { _cleanup_(config_section_freep) ConfigSection *n = NULL; - _cleanup_(address_freep) Address *address = NULL; + _cleanup_(address_unrefp) Address *address = NULL; int r; assert(network); @@ -186,7 +212,7 @@ int address_new_static(Network *network, const char *filename, unsigned section_ /* This will be adjusted in address_section_verify(). */ address->duplicate_address_detection = _ADDRESS_FAMILY_INVALID; - r = ordered_hashmap_ensure_put(&network->addresses_by_section, &config_section_hash_ops, address->section, address); + r = ordered_hashmap_ensure_put(&network->addresses_by_section, &address_section_hash_ops, address->section, address); if (r < 0) return r; @@ -194,9 +220,9 @@ int address_new_static(Network *network, const char *filename, unsigned section_ return 0; } -Address *address_free(Address *address) { - if (!address) - return NULL; +static Address* address_detach_impl(Address *address) { + assert(address); + assert(!address->link || !address->network); if (address->network) { assert(address->section); @@ -204,11 +230,33 @@ Address *address_free(Address *address) { if (address->network->dhcp_server_address == address) address->network->dhcp_server_address = NULL; + + address->network = NULL; + return address; } - if (address->link) + if (address->link) { set_remove(address->link->addresses, address); + address->link = NULL; + return address; + } + + return NULL; +} + +static void address_detach(Address *address) { + assert(address); + + address_unref(address_detach_impl(address)); +} + +static Address* address_free(Address *address) { + if (!address) + return NULL; + + address_detach_impl(address); + config_section_free(address->section); free(address->label); free(address->netlabel); @@ -216,6 +264,8 @@ Address *address_free(Address *address) { return mfree(address); } +DEFINE_TRIVIAL_REF_UNREF_FUNC(Address, address, address_free); + static bool address_lifetime_is_valid(const Address *a) { assert(a); @@ -466,19 +516,6 @@ static int address_compare_func(const Address *a1, const Address *a2) { } } -DEFINE_HASH_OPS( - address_hash_ops, - Address, - address_hash_func, - address_compare_func); - -DEFINE_PRIVATE_HASH_OPS_WITH_KEY_DESTRUCTOR( - address_hash_ops_free, - Address, - address_hash_func, - address_compare_func, - address_free); - static bool address_can_update(const Address *la, const Address *na) { assert(la); assert(la->link); @@ -557,7 +594,7 @@ static bool address_can_update(const Address *la, const Address *na) { } int address_dup(const Address *src, Address **ret) { - _cleanup_(address_freep) Address *dest = NULL; + _cleanup_(address_unrefp) Address *dest = NULL; int r; assert(src); @@ -567,7 +604,8 @@ int address_dup(const Address *src, Address **ret) { if (!dest) return -ENOMEM; - /* clear all pointers */ + /* clear the reference counter and all pointers */ + dest->n_ref = 1; dest->network = NULL; dest->section = NULL; dest->link = NULL; @@ -708,19 +746,21 @@ static void address_modify_nft_set(Address *address, bool add) { } } -static int address_add(Link *link, Address *address) { +static int address_attach(Link *link, Address *address) { int r; assert(link); assert(address); + assert(!address->link); - r = set_ensure_put(&link->addresses, &address_hash_ops_free, address); + r = set_ensure_put(&link->addresses, &address_hash_ops_detach, address); if (r < 0) return r; if (r == 0) return -EEXIST; address->link = link; + address_ref(address); return 0; } @@ -781,7 +821,7 @@ static int address_drop(Address *address) { ipv4acd_detach(link, address); - address_free(address); + address_detach(address); link_update_operstate(link, /* also_update_master = */ true); link_check_ready(link); @@ -910,7 +950,7 @@ int link_get_address(Link *link, int family, const union in_addr_union *address, * arbitrary prefixlen will be returned. */ if (family == AF_INET6 || prefixlen != 0) { - _cleanup_(address_freep) Address *tmp = NULL; + _cleanup_(address_unrefp) Address *tmp = NULL; /* In this case, we can use address_get(). */ @@ -1211,7 +1251,7 @@ int link_drop_ipv6ll_addresses(Link *link) { return r; for (sd_netlink_message *addr = reply; addr; addr = sd_netlink_message_next(addr)) { - _cleanup_(address_freep) Address *a = NULL; + _cleanup_(address_unrefp) Address *a = NULL; unsigned char flags, prefixlen; struct in6_addr address; int ifindex; @@ -1356,7 +1396,7 @@ void link_foreignize_addresses(Link *link) { } static int address_acquire(Link *link, const Address *original, Address **ret) { - _cleanup_(address_freep) Address *na = NULL; + _cleanup_(address_unrefp) Address *na = NULL; union in_addr_union in_addr; int r; @@ -1523,7 +1563,7 @@ int link_request_address( address_netlink_handler_t netlink_handler, Request **ret) { - _cleanup_(address_freep) Address *tmp = NULL; + _cleanup_(address_unrefp) Address *tmp = NULL; Address *existing = NULL; int r; @@ -1568,7 +1608,7 @@ int link_request_address( log_address_debug(tmp, "Requesting", link); r = link_queue_request_safe(link, REQUEST_TYPE_ADDRESS, tmp, - address_free, + address_unref, address_hash_func, address_compare_func, address_process_request, @@ -1644,7 +1684,7 @@ int link_request_static_addresses(Link *link) { } int manager_rtnl_process_address(sd_netlink *rtnl, sd_netlink_message *message, Manager *m) { - _cleanup_(address_freep) Address *tmp = NULL; + _cleanup_(address_unrefp) Address *tmp = NULL; struct ifa_cacheinfo cinfo; Link *link; uint16_t type; @@ -1781,13 +1821,13 @@ int manager_rtnl_process_address(sd_netlink *rtnl, sd_netlink_message *message, if (!address) { /* If we did not know the address, then save it. */ - r = address_add(link, tmp); + r = address_attach(link, tmp); if (r < 0) { log_link_warning_errno(link, r, "Failed to save received address %s, ignoring: %m", IN_ADDR_PREFIX_TO_STRING(tmp->family, &tmp->in_addr, tmp->prefixlen)); return 0; } - address = TAKE_PTR(tmp); + address = tmp; is_new = true; @@ -1887,7 +1927,7 @@ int config_parse_broadcast( void *userdata) { Network *network = userdata; - _cleanup_(address_free_or_set_invalidp) Address *n = NULL; + _cleanup_(address_unref_or_set_invalidp) Address *n = NULL; union in_addr_union u; int r; @@ -1964,7 +2004,7 @@ int config_parse_address( void *userdata) { Network *network = userdata; - _cleanup_(address_free_or_set_invalidp) Address *n = NULL; + _cleanup_(address_unref_or_set_invalidp) Address *n = NULL; union in_addr_union buffer; unsigned char prefixlen; int r, f; @@ -1978,7 +2018,7 @@ int config_parse_address( if (streq(section, "Network")) { if (isempty(rvalue)) { /* If an empty string specified in [Network] section, clear previously assigned addresses. */ - network->addresses_by_section = ordered_hashmap_free_with_destructor(network->addresses_by_section, address_free); + network->addresses_by_section = ordered_hashmap_free(network->addresses_by_section); return 0; } @@ -2051,7 +2091,7 @@ int config_parse_label( void *data, void *userdata) { - _cleanup_(address_free_or_set_invalidp) Address *n = NULL; + _cleanup_(address_unref_or_set_invalidp) Address *n = NULL; Network *network = userdata; int r; @@ -2103,7 +2143,7 @@ int config_parse_lifetime( void *userdata) { Network *network = userdata; - _cleanup_(address_free_or_set_invalidp) Address *n = NULL; + _cleanup_(address_unref_or_set_invalidp) Address *n = NULL; usec_t k; int r; @@ -2152,7 +2192,7 @@ int config_parse_address_flags( void *userdata) { Network *network = userdata; - _cleanup_(address_free_or_set_invalidp) Address *n = NULL; + _cleanup_(address_unref_or_set_invalidp) Address *n = NULL; int r; assert(filename); @@ -2199,7 +2239,7 @@ int config_parse_address_scope( void *userdata) { Network *network = userdata; - _cleanup_(address_free_or_set_invalidp) Address *n = NULL; + _cleanup_(address_unref_or_set_invalidp) Address *n = NULL; int r; assert(filename); @@ -2243,7 +2283,7 @@ int config_parse_address_route_metric( void *userdata) { Network *network = userdata; - _cleanup_(address_free_or_set_invalidp) Address *n = NULL; + _cleanup_(address_unref_or_set_invalidp) Address *n = NULL; int r; assert(filename); @@ -2285,7 +2325,7 @@ int config_parse_duplicate_address_detection( void *userdata) { Network *network = userdata; - _cleanup_(address_free_or_set_invalidp) Address *n = NULL; + _cleanup_(address_unref_or_set_invalidp) Address *n = NULL; int r; assert(filename); @@ -2339,7 +2379,7 @@ int config_parse_address_netlabel( void *userdata) { Network *network = userdata; - _cleanup_(address_free_or_set_invalidp) Address *n = NULL; + _cleanup_(address_unref_or_set_invalidp) Address *n = NULL; int r; assert(filename); @@ -2481,8 +2521,8 @@ int network_drop_invalid_addresses(Network *network) { if (address_section_verify(address) < 0) { /* Drop invalid [Address] sections or Address= settings in [Network]. - * Note that address_free() will drop the address from addresses_by_section. */ - address_free(address); + * Note that address_detach() will drop the address from addresses_by_section. */ + address_detach(address); continue; } @@ -2495,12 +2535,13 @@ int network_drop_invalid_addresses(Network *network) { IN_ADDR_PREFIX_TO_STRING(address->family, &address->in_addr, address->prefixlen), address->section->line, dup->section->line, dup->section->line); - /* address_free() will drop the address from addresses_by_section. */ - address_free(dup); + + /* address_detach() will drop the address from addresses_by_section. */ + address_detach(dup); } - /* Use address_hash_ops, instead of address_hash_ops_free. Otherwise, the Address objects - * will be freed. */ + /* Use address_hash_ops, instead of address_hash_ops_detach. Otherwise, the Address objects + * will be detached. */ r = set_ensure_put(&addresses, &address_hash_ops, address); if (r < 0) return log_oom(); @@ -2527,7 +2568,7 @@ int config_parse_address_ip_nft_set( void *userdata) { Network *network = userdata; - _cleanup_(address_free_or_set_invalidp) Address *n = NULL; + _cleanup_(address_unref_or_set_invalidp) Address *n = NULL; int r; assert(filename); diff --git a/src/network/networkd-address.h b/src/network/networkd-address.h index 53e7a798212..cc2429094e7 100644 --- a/src/network/networkd-address.h +++ b/src/network/networkd-address.h @@ -34,6 +34,8 @@ struct Address { NetworkConfigState state; union in_addr_union provider; /* DHCP server or router address */ + unsigned n_ref; + int family; unsigned char prefixlen; unsigned char scope; @@ -83,9 +85,11 @@ void link_get_address_states( extern const struct hash_ops address_hash_ops; +Address* address_ref(Address *address); +Address* address_unref(Address *address); + int address_new(Address **ret); int address_new_static(Network *network, const char *filename, unsigned section_line, Address **ret); -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); @@ -95,7 +99,7 @@ int address_dup(const Address *src, Address **ret); bool address_is_ready(const Address *a); bool link_check_addresses_ready(Link *link, NetworkConfigSource source); -DEFINE_SECTION_CLEANUP_FUNCTIONS(Address, address_free); +DEFINE_SECTION_CLEANUP_FUNCTIONS(Address, address_unref); int link_drop_managed_addresses(Link *link); int link_drop_foreign_addresses(Link *link); diff --git a/src/network/networkd-dhcp-prefix-delegation.c b/src/network/networkd-dhcp-prefix-delegation.c index f5d97dd7af2..2158c773931 100644 --- a/src/network/networkd-dhcp-prefix-delegation.c +++ b/src/network/networkd-dhcp-prefix-delegation.c @@ -366,7 +366,7 @@ static int dhcp_pd_request_address( return log_link_warning_errno(link, r, "Failed to generate addresses for acquired DHCP delegated prefix: %m"); SET_FOREACH(a, addresses) { - _cleanup_(address_freep) Address *address = NULL; + _cleanup_(address_unrefp) Address *address = NULL; Address *existing; r = address_new(&address); diff --git a/src/network/networkd-dhcp-server.c b/src/network/networkd-dhcp-server.c index a486fd89275..81ef6d80f75 100644 --- a/src/network/networkd-dhcp-server.c +++ b/src/network/networkd-dhcp-server.c @@ -92,7 +92,7 @@ int network_adjust_dhcp_server(Network *network, Set **addresses) { } } else { - _cleanup_(address_freep) Address *a = NULL; + _cleanup_(address_unrefp) Address *a = NULL; Address *existing; unsigned line; diff --git a/src/network/networkd-dhcp4.c b/src/network/networkd-dhcp4.c index 8aee30e7266..dc55cc141d6 100644 --- a/src/network/networkd-dhcp4.c +++ b/src/network/networkd-dhcp4.c @@ -879,7 +879,7 @@ static int dhcp4_address_handler(sd_netlink *rtnl, sd_netlink_message *m, Reques } static int dhcp4_request_address(Link *link, bool announce) { - _cleanup_(address_freep) Address *addr = NULL; + _cleanup_(address_unrefp) Address *addr = NULL; struct in_addr address, server; uint8_t prefixlen; Address *existing; diff --git a/src/network/networkd-dhcp6.c b/src/network/networkd-dhcp6.c index 6d8b3c3ce26..0024adb816b 100644 --- a/src/network/networkd-dhcp6.c +++ b/src/network/networkd-dhcp6.c @@ -191,7 +191,7 @@ static int dhcp6_request_address( usec_t lifetime_preferred_usec, usec_t lifetime_valid_usec) { - _cleanup_(address_freep) Address *addr = NULL; + _cleanup_(address_unrefp) Address *addr = NULL; Address *existing; int r; diff --git a/src/network/networkd-ipv4ll.c b/src/network/networkd-ipv4ll.c index 629a3734095..299aaedd070 100644 --- a/src/network/networkd-ipv4ll.c +++ b/src/network/networkd-ipv4ll.c @@ -28,7 +28,7 @@ bool link_ipv4ll_enabled(Link *link) { } static int address_new_from_ipv4ll(Link *link, Address **ret) { - _cleanup_(address_freep) Address *address = NULL; + _cleanup_(address_unrefp) Address *address = NULL; struct in_addr addr; int r; @@ -56,7 +56,7 @@ static int address_new_from_ipv4ll(Link *link, Address **ret) { } static int ipv4ll_address_lost(Link *link) { - _cleanup_(address_freep) Address *address = NULL; + _cleanup_(address_unrefp) Address *address = NULL; int r; assert(link); @@ -92,7 +92,7 @@ static int ipv4ll_address_handler(sd_netlink *rtnl, sd_netlink_message *m, Reque } static int ipv4ll_address_claimed(sd_ipv4ll *ll, Link *link) { - _cleanup_(address_freep) Address *address = NULL; + _cleanup_(address_unrefp) Address *address = NULL; int r; assert(ll); diff --git a/src/network/networkd-ndisc.c b/src/network/networkd-ndisc.c index 369e205a99a..0da645cde5e 100644 --- a/src/network/networkd-ndisc.c +++ b/src/network/networkd-ndisc.c @@ -244,7 +244,7 @@ static int ndisc_address_handler(sd_netlink *rtnl, sd_netlink_message *m, Reques } static int ndisc_request_address(Address *in, Link *link, sd_ndisc_router *rt) { - _cleanup_(address_freep) Address *address = in; + _cleanup_(address_unrefp) Address *address = in; struct in6_addr router; bool is_new; int r; @@ -432,7 +432,7 @@ static int ndisc_router_process_autonomous_prefix(Link *link, sd_ndisc_router *r return log_link_warning_errno(link, r, "Failed to generate SLAAC addresses: %m"); SET_FOREACH(a, addresses) { - _cleanup_(address_freep) Address *address = NULL; + _cleanup_(address_unrefp) Address *address = NULL; r = address_new(&address); if (r < 0) diff --git a/src/network/networkd-network.c b/src/network/networkd-network.c index a2b3580ced1..591b4093cc3 100644 --- a/src/network/networkd-network.c +++ b/src/network/networkd-network.c @@ -187,7 +187,7 @@ int network_verify(Network *network) { log_warning("%s: Cannot set routes when Bond= is specified, ignoring routes.", network->filename); - network->addresses_by_section = ordered_hashmap_free_with_destructor(network->addresses_by_section, address_free); + network->addresses_by_section = ordered_hashmap_free(network->addresses_by_section); network->routes_by_section = hashmap_free_with_destructor(network->routes_by_section, route_free); } @@ -782,7 +782,7 @@ static Network *network_free(Network *network) { /* static configs */ set_free_free(network->ipv6_proxy_ndp_addresses); - ordered_hashmap_free_with_destructor(network->addresses_by_section, address_free); + ordered_hashmap_free(network->addresses_by_section); hashmap_free_with_destructor(network->routes_by_section, route_free); ordered_hashmap_free_with_destructor(network->nexthops_by_section, nexthop_free); hashmap_free_with_destructor(network->bridge_fdb_entries_by_section, bridge_fdb_free); diff --git a/src/network/networkd-radv.c b/src/network/networkd-radv.c index fc36a001f8f..7fd97e7206f 100644 --- a/src/network/networkd-radv.c +++ b/src/network/networkd-radv.c @@ -258,7 +258,7 @@ int link_request_radv_addresses(Link *link) { return r; SET_FOREACH(a, addresses) { - _cleanup_(address_freep) Address *address = NULL; + _cleanup_(address_unrefp) Address *address = NULL; r = address_new(&address); if (r < 0) -- 2.39.2