diff --git a/src/network/networkd-nexthop.c b/src/network/networkd-nexthop.c index 4de1e09999b..cf62e0e82dc 100644 --- a/src/network/networkd-nexthop.c +++ b/src/network/networkd-nexthop.c @@ -18,6 +18,48 @@ #include "stdio-util.h" #include "string-util.h" +static void nexthop_detach_from_group_members(NextHop *nexthop) { + assert(nexthop); + assert(nexthop->manager); + assert(nexthop->id > 0); + + struct nexthop_grp *nhg; + HASHMAP_FOREACH(nhg, nexthop->group) { + NextHop *nh; + + if (nexthop_get_by_id(nexthop->manager, nhg->id, &nh) < 0) + continue; + + set_remove(nh->nexthops, UINT32_TO_PTR(nexthop->id)); + } +} + +static void nexthop_attach_to_group_members(NextHop *nexthop) { + int r; + + assert(nexthop); + assert(nexthop->manager); + assert(nexthop->id > 0); + + struct nexthop_grp *nhg; + HASHMAP_FOREACH(nhg, nexthop->group) { + NextHop *nh; + + r = nexthop_get_by_id(nexthop->manager, nhg->id, &nh); + if (r < 0) { + if (nexthop->manager->manage_foreign_nexthops) + log_debug_errno(r, "Nexthop (id=%"PRIu32") has unknown group member (%"PRIu32"), ignoring.", + nexthop->id, nhg->id); + continue; + } + + r = set_ensure_put(&nh->nexthops, NULL, UINT32_TO_PTR(nexthop->id)); + if (r < 0) + log_debug_errno(r, "Failed to save nexthop ID (%"PRIu32") to group member (%"PRIu32"), ignoring: %m", + nexthop->id, nhg->id); + } +} + static NextHop* nexthop_detach_impl(NextHop *nexthop) { assert(nexthop); assert(!nexthop->manager || !nexthop->network); @@ -31,6 +73,9 @@ static NextHop* nexthop_detach_impl(NextHop *nexthop) { if (nexthop->manager) { assert(nexthop->id > 0); + + nexthop_detach_from_group_members(nexthop); + hashmap_remove(nexthop->manager->nexthops_by_id, UINT32_TO_PTR(nexthop->id)); nexthop->manager = NULL; return nexthop; @@ -51,6 +96,7 @@ static NextHop* nexthop_free(NextHop *nexthop) { config_section_free(nexthop->section); hashmap_free_free(nexthop->group); + set_free(nexthop->nexthops); return mfree(nexthop); } @@ -436,6 +482,29 @@ static void log_nexthop_debug(const NextHop *nexthop, const char *str, Manager * yes_no(nexthop->blackhole), strna(group), strna(flags)); } +static int nexthop_remove_dependents(NextHop *nexthop, Manager *manager) { + int r = 0; + + assert(nexthop); + assert(manager); + + /* If a nexthop is removed, the kernel silently removes nexthops that depend on the + * removed nexthop. Let's remove them for safety (though, they are already removed in the kernel, + * hence that should fail), and forget them. */ + + void *id; + SET_FOREACH(id, nexthop->nexthops) { + NextHop *nh; + + if (nexthop_get_by_id(manager, PTR_TO_UINT32(id), &nh) < 0) + continue; + + RET_GATHER(r, nexthop_remove(nh, manager)); + } + + return r; +} + static int nexthop_remove_handler(sd_netlink *rtnl, sd_netlink_message *m, RemoveRequest *rreq) { int r; @@ -451,6 +520,8 @@ static int nexthop_remove_handler(sd_netlink *rtnl, sd_netlink_message *m, Remov (r == -ENOENT || !nexthop->manager) ? LOG_DEBUG : LOG_WARNING, r, "Could not drop nexthop, ignoring"); + (void) nexthop_remove_dependents(nexthop, manager); + if (nexthop->manager) { /* If the nexthop cannot be removed, then assume the nexthop is already removed. */ log_nexthop_debug(nexthop, "Forgetting", manager); @@ -861,15 +932,22 @@ void link_foreignize_nexthops(Link *link) { } } -static int nexthop_update_group(NextHop *nexthop, const struct nexthop_grp *group, size_t size) { +static int nexthop_update_group(NextHop *nexthop, sd_netlink_message *message) { _cleanup_hashmap_free_free_ Hashmap *h = NULL; - size_t n_group; + _cleanup_free_ struct nexthop_grp *group = NULL; + size_t size = 0, n_group; int r; assert(nexthop); - assert(group || size == 0); + assert(message); - if (size == 0 || size % sizeof(struct nexthop_grp) != 0) + r = sd_netlink_message_read_data(message, NHA_GROUP, &size, (void**) &group); + if (r < 0 && r != -ENODATA) + return log_debug_errno(r, "rtnl: could not get NHA_GROUP attribute, ignoring: %m"); + + nexthop_detach_from_group_members(nexthop); + + if (size % sizeof(struct nexthop_grp) != 0) return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "rtnl: received nexthop message with invalid nexthop group size, ignoring."); @@ -908,12 +986,12 @@ static int nexthop_update_group(NextHop *nexthop, const struct nexthop_grp *grou hashmap_free_free(nexthop->group); nexthop->group = TAKE_PTR(h); + + nexthop_attach_to_group_members(nexthop); return 0; } int manager_rtnl_process_nexthop(sd_netlink *rtnl, sd_netlink_message *message, Manager *m) { - _cleanup_free_ void *raw_group = NULL; - size_t raw_group_size; uint16_t type; uint32_t id, ifindex; NextHop *nexthop = NULL; @@ -961,6 +1039,7 @@ int manager_rtnl_process_nexthop(sd_netlink *rtnl, sd_netlink_message *message, if (nexthop) { nexthop_enter_removed(nexthop); log_nexthop_debug(nexthop, "Forgetting removed", m); + (void) nexthop_remove_dependents(nexthop, m); nexthop_detach(nexthop); } else log_nexthop_debug(&(const NextHop) { .id = id }, "Kernel removed unknown", m); @@ -1001,13 +1080,7 @@ int manager_rtnl_process_nexthop(sd_netlink *rtnl, sd_netlink_message *message, if (r < 0) log_debug_errno(r, "rtnl: could not get nexthop flags, ignoring: %m"); - r = sd_netlink_message_read_data(message, NHA_GROUP, &raw_group_size, &raw_group); - if (r == -ENODATA) - nexthop->group = hashmap_free_free(nexthop->group); - else if (r < 0) - log_debug_errno(r, "rtnl: could not get NHA_GROUP attribute, ignoring: %m"); - else - (void) nexthop_update_group(nexthop, raw_group, raw_group_size); + (void) nexthop_update_group(nexthop, message); if (nexthop->family != AF_UNSPEC) { r = netlink_message_read_in_addr_union(message, NHA_GATEWAY, nexthop->family, &nexthop->gw); diff --git a/src/network/networkd-nexthop.h b/src/network/networkd-nexthop.h index bc2bacfa40d..74b23bd7720 100644 --- a/src/network/networkd-nexthop.h +++ b/src/network/networkd-nexthop.h @@ -26,15 +26,23 @@ typedef struct NextHop { unsigned n_ref; - uint8_t protocol; - int ifindex; - uint32_t id; - bool blackhole; + /* struct nhmsg */ int family; - union in_addr_union gw; + uint8_t protocol; uint8_t flags; - int onlink; /* Only used in conf parser and nexthop_section_verify(). */ - Hashmap *group; + + /* attributes */ + uint32_t id; /* NHA_ID */ + Hashmap *group; /* NHA_GROUP */ + bool blackhole; /* NHA_BLACKHOLE */ + int ifindex; /* NHA_OIF */ + union in_addr_union gw; /* NHA_GATEWAY */ + + /* Only used in conf parser and nexthop_section_verify(). */ + int onlink; + + /* For managing nexthops that depend on this nexthop. */ + Set *nexthops; } NextHop; NextHop* nexthop_ref(NextHop *nexthop); diff --git a/test/test-network/conf/25-nexthop-test1.network b/test/test-network/conf/25-nexthop-test1.network new file mode 100644 index 00000000000..5a4c596d3aa --- /dev/null +++ b/test/test-network/conf/25-nexthop-test1.network @@ -0,0 +1,12 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +[Match] +Name=test1 + +[Network] +Address=192.168.20.21/24 +IPv6AcceptRA=no + +[Route] +Destination=10.10.11.10 +# Nexthop 21 is configured as a group nexthop of 1 and 20 +NextHop=21 diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index edf09b81204..6d074e67ff5 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -4033,6 +4033,33 @@ class NetworkdNetworkTests(unittest.TestCase, Utilities): self.check_nexthop(manage_foreign_nexthops, first=True) + # Remove nexthop with ID 20 + check_output('ip nexthop del id 20') + copy_network_unit('11-dummy.netdev', '25-nexthop-test1.network') + networkctl_reload() + + # 25-nexthop-test1.network requests a route with nexthop ID 21, + # which is silently removed by the kernel when nexthop with ID 20 is removed in the above, + # hence test1 should be stuck in the configuring state. + self.wait_operstate('test1', operstate='routable', setup_state='configuring') + + # Wait for a while, and check if the interface is still in the configuring state. + time.sleep(1) + output = networkctl_status('test1') + self.assertIn('State: routable (configuring)', output) + + # Reconfigure the interface that has nexthop with ID 20 and 21, + # then the route requested by test1 can be configured. + networkctl_reconfigure('dummy98') + self.wait_online(['test1:routable']) + + # Check if the requested route actually configured. + output = check_output('ip route show 10.10.11.10') + print(output) + self.assertIn('10.10.11.10 nhid 21 proto static', output) + self.assertIn('nexthop via 192.168.5.1 dev veth99 weight 3', output) + self.assertIn('nexthop via 192.168.20.1 dev dummy98 weight 1', output) + remove_link('veth99') time.sleep(2)