diff options
author | Yu Watanabe <watanabe.yu+github@gmail.com> | 2019-04-15 16:38:45 +0900 |
---|---|---|
committer | Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl> | 2019-04-25 10:47:17 +0200 |
commit | 5f707e1280d7c66d3adcffd47a23ad446257f355 (patch) | |
tree | 05ec491bc95e4ca841822456aff165d08e8b3587 | |
parent | bd08ce56156751d58584a44e766ef61340cdae2d (diff) | |
download | systemd-5f707e1280d7c66d3adcffd47a23ad446257f355.tar.gz |
network: fix ref/unref logic for Link object
- bridge or bonding master takes a reference of slave links.
- drop link from bridge or bonding master's slave list when slave link
is removed.
- change type of Link::slaves to Set*,
Fixes #12315.
-rw-r--r-- | src/network/networkd-link.c | 109 | ||||
-rw-r--r-- | src/network/networkd-link.h | 2 | ||||
-rw-r--r-- | src/network/networkd-manager.c | 7 |
3 files changed, 61 insertions, 57 deletions
diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index 3bc7f69cb3..d3928e2862 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -409,7 +409,7 @@ void link_update_operstate(Link *link, bool also_update_master) { if (operstate >= LINK_OPERSTATE_CARRIER) { Link *slave; - HASHMAP_FOREACH(slave, link->slaves, i) { + SET_FOREACH(slave, link->slaves, i) { link_update_operstate(slave, false); if (slave->operstate < LINK_OPERSTATE_CARRIER) @@ -605,17 +605,8 @@ static int link_new(Manager *manager, sd_netlink_message *message, Link **ret) { return 0; } -static void link_detach_from_manager(Link *link) { - if (!link || !link->manager) - return; - - hashmap_remove(link->manager->links, INT_TO_PTR(link->ifindex)); - set_remove(link->manager->links_requesting_uuid, link); - link_clean(link); -} - static Link *link_free(Link *link) { - Link *carrier, *master; + Link *carrier; Address *address; Route *route; Iterator i; @@ -663,10 +654,7 @@ static Link *link_free(Link *link) { sd_ndisc_unref(link->ndisc); sd_radv_unref(link->radv); - link_detach_from_manager(link); - free(link->ifname); - free(link->kind); (void) unlink(link->state_file); @@ -682,17 +670,7 @@ static Link *link_free(Link *link) { hashmap_remove(link->bound_by_links, INT_TO_PTR(carrier->ifindex)); hashmap_free(link->bound_by_links); - hashmap_free(link->slaves); - - if (link->network) { - if (link->network->bond && - link_get(link->manager, link->network->bond->ifindex, &master) >= 0) - (void) hashmap_remove(master->slaves, INT_TO_PTR(link->ifindex)); - - if (link->network->bridge && - link_get(link->manager, link->network->bridge->ifindex, &master) >= 0) - (void) hashmap_remove(master->slaves, INT_TO_PTR(link->ifindex)); - } + set_free_with_destructor(link->slaves, link_unref); return mfree(link); } @@ -1751,28 +1729,6 @@ static int link_set_bond(Link *link) { return r; } -static int link_append_to_master(Link *link, NetDev *netdev) { - Link *master; - int r; - - assert(link); - assert(netdev); - - r = link_get(link->manager, netdev->ifindex, &master); - if (r < 0) - return r; - - r = hashmap_ensure_allocated(&master->slaves, NULL); - if (r < 0) - return r; - - r = hashmap_put(master->slaves, INT_TO_PTR(link->ifindex), link); - if (r < 0) - return r; - - return 0; -} - static int link_lldp_save(Link *link) { _cleanup_free_ char *temp_path = NULL; _cleanup_fclose_ FILE *f = NULL; @@ -2535,6 +2491,55 @@ static void link_free_carrier_maps(Link *link) { return; } +static int link_append_to_master(Link *link, NetDev *netdev) { + Link *master; + int r; + + assert(link); + assert(netdev); + + r = link_get(link->manager, netdev->ifindex, &master); + if (r < 0) + return r; + + r = set_ensure_allocated(&master->slaves, NULL); + if (r < 0) + return r; + + r = set_put(master->slaves, link); + if (r < 0) + return r; + + link_ref(link); + return 0; +} + +static void link_drop_from_master(Link *link, NetDev *netdev) { + Link *master; + + assert(link); + + if (!link->manager || !netdev) + return; + + if (link_get(link->manager, netdev->ifindex, &master) < 0) + return; + + link_unref(set_remove(master->slaves, link)); +} + +static void link_detach_from_manager(Link *link) { + if (!link || !link->manager) + return; + + link_unref(set_remove(link->manager->links_requesting_uuid, link)); + link_clean(link); + + /* The following must be called at last. */ + assert_se(hashmap_remove(link->manager->links, INT_TO_PTR(link->ifindex)) == link); + link_unref(link); +} + void link_drop(Link *link) { if (!link || link->state == LINK_STATE_LINGER) return; @@ -2543,15 +2548,15 @@ void link_drop(Link *link) { link_free_carrier_maps(link); + if (link->network) { + link_drop_from_master(link, link->network->bridge); + link_drop_from_master(link, link->network->bond); + } + log_link_debug(link, "Link removed"); (void) unlink(link->state_file); - link_detach_from_manager(link); - - link_unref(link); - - return; } static int link_joined(Link *link) { diff --git a/src/network/networkd-link.h b/src/network/networkd-link.h index e322ec2262..e5497379d7 100644 --- a/src/network/networkd-link.h +++ b/src/network/networkd-link.h @@ -122,7 +122,7 @@ typedef struct Link { Hashmap *bound_by_links; Hashmap *bound_to_links; - Hashmap *slaves; + Set *slaves; } Link; typedef int (*link_netlink_message_handler_t)(sd_netlink*, sd_netlink_message*, Link*); diff --git a/src/network/networkd-manager.c b/src/network/networkd-manager.c index 677f66a478..c9579370cb 100644 --- a/src/network/networkd-manager.c +++ b/src/network/networkd-manager.c @@ -1450,13 +1450,12 @@ void manager_free(Manager *m) { } m->dirty_links = set_free_with_destructor(m->dirty_links, link_unref); - m->links = hashmap_free(m->links); - m->links_requesting_uuid = set_free(m->links_requesting_uuid); - set_free(m->duids_requesting_uuid); + m->links_requesting_uuid = set_free_with_destructor(m->links_requesting_uuid, link_unref); + m->links = hashmap_free_with_destructor(m->links, link_unref); + m->duids_requesting_uuid = set_free(m->duids_requesting_uuid); while ((network = m->networks)) network_free(network); - hashmap_free(m->networks_by_name); m->netdevs = hashmap_free_with_destructor(m->netdevs, netdev_unref); |