Merge pull request #30716 from yuwata/network-cancel-request

network: also remove configuration on cancelling request

src/network/networkd-link.c

@@ -931,15 +931,51 @@ static void link_drop_from_master(Link *link) {
         link_unref(set_remove(master->slaves, link));
 }
 
-static void link_drop_requests(Link *link) {
+static int link_drop_requests(Link *link) {
         Request *req;
+        int ret = 0;
 
         assert(link);
         assert(link->manager);
 
-        ORDERED_SET_FOREACH(req, link->manager->request_queue)
-                if (req->link == link)
-                        request_detach(link->manager, req);
+        ORDERED_SET_FOREACH(req, link->manager->request_queue) {
+                if (req->link != link)
+                        continue;
+
+                /* If the request is already called, but its reply is not received, then we need to
+                 * drop the configuration (e.g. address) here. Note, if the configuration is known,
+                 * it will be handled later by link_drop_foreign_addresses() or so. */
+                if (req->waiting_reply && link->state != LINK_STATE_LINGER)
+                        switch (req->type) {
+                        case REQUEST_TYPE_ADDRESS: {
+                                Address *address = ASSERT_PTR(req->userdata);
+
+                                if (address_get(link, address, NULL) < 0)
+                                        RET_GATHER(ret, address_remove(address, link));
+                                break;
+                        }
+                        case REQUEST_TYPE_NEIGHBOR: {
+                                Neighbor *neighbor = ASSERT_PTR(req->userdata);
+
+                                if (neighbor_get(link, neighbor, NULL) < 0)
+                                        RET_GATHER(ret, neighbor_remove(neighbor, link));
+                                break;
+                        }
+                        case REQUEST_TYPE_NEXTHOP: {
+                                NextHop *nexthop = ASSERT_PTR(req->userdata);
+
+                                if (nexthop_get_by_id(link->manager, nexthop->id, NULL) < 0)
+                                        RET_GATHER(ret, nexthop_remove(nexthop, link->manager));
+                                break;
+                        }
+                        default:
+                                ;
+                        }
+
+                request_detach(link->manager, req);
+        }
+
+        return ret;
 }
 
 static Link *link_drop(Link *link) {
@@ -953,7 +989,7 @@ static Link *link_drop(Link *link) {
         /* Drop all references from other links and manager. Note that async netlink calls may have
          * references to the link, and they will be dropped when we receive replies. */
 
-        link_drop_requests(link);
+        (void) link_drop_requests(link);
 
         link_free_bound_to_list(link);
         link_free_bound_by_list(link);
@@ -1263,7 +1299,9 @@ int link_reconfigure_impl(Link *link, bool force) {
         if (r < 0)
                 return r;
 
-        link_drop_requests(link);
+        r = link_drop_requests(link);
+        if (r < 0)
+                return r;
 
         if (network && !force && network->keep_configuration != KEEP_CONFIGURATION_YES)
                 /* When a new/updated .network file is assigned, first make all configs (addresses,

src/network/networkd-neighbor.c

@@ -152,7 +152,7 @@ static int neighbor_get_request(Link *link, const Neighbor *neighbor, Request **
         return 0;
 }
 
-static int neighbor_get(Link *link, const Neighbor *in, Neighbor **ret) {
+int neighbor_get(Link *link, const Neighbor *in, Neighbor **ret) {
         Neighbor *existing;
 
         assert(link);
@@ -370,18 +370,14 @@ static int neighbor_remove_handler(sd_netlink *rtnl, sd_netlink_message *m, Link
         return 1;
 }
 
-static int neighbor_remove(Neighbor *neighbor) {
+int neighbor_remove(Neighbor *neighbor, Link *link) {
         _cleanup_(sd_netlink_message_unrefp) sd_netlink_message *m = NULL;
-        Request *req;
-        Link *link;
         int r;
 
         assert(neighbor);
-        assert(neighbor->link);
-        assert(neighbor->link->manager);
-        assert(neighbor->link->manager->rtnl);
-
-        link = neighbor->link;
+        assert(link);
+        assert(link->manager);
+        assert(link->manager->rtnl);
 
         log_neighbor_debug(neighbor, "Removing", link);
 
@@ -402,9 +398,6 @@ static int neighbor_remove(Neighbor *neighbor) {
         link_ref(link);
 
         neighbor_enter_removing(neighbor);
-        if (neighbor_get_request(neighbor->link, neighbor, &req) >= 0)
-                neighbor_enter_removing(req->userdata);
-
         return 0;
 }
 
@@ -440,7 +433,7 @@ int link_drop_foreign_neighbors(Link *link) {
                 if (!neighbor_is_marked(neighbor))
                         continue;
 
-                RET_GATHER(r, neighbor_remove(neighbor));
+                RET_GATHER(r, neighbor_remove(neighbor, link));
         }
 
         return r;
@@ -461,7 +454,7 @@ int link_drop_managed_neighbors(Link *link) {
                 if (!neighbor_exists(neighbor))
                         continue;
 
-                RET_GATHER(r, neighbor_remove(neighbor));
+                RET_GATHER(r, neighbor_remove(neighbor, link));
         }
 
         return r;

src/network/networkd-neighbor.h

@@ -28,6 +28,9 @@ typedef struct Neighbor {
 
 Neighbor *neighbor_free(Neighbor *neighbor);
 
+int neighbor_get(Link *link, const Neighbor *in, Neighbor **ret);
+int neighbor_remove(Neighbor *neighbor, Link *link);
+
 int network_drop_invalid_neighbors(Network *network);
 
 int link_drop_managed_neighbors(Link *link);

src/network/networkd-nexthop.c

@@ -427,17 +427,14 @@ static int nexthop_remove_handler(sd_netlink *rtnl, sd_netlink_message *m, Link
         return 1;
 }
 
-static int nexthop_remove(NextHop *nexthop) {
+int nexthop_remove(NextHop *nexthop, Manager *manager) {
         _cleanup_(sd_netlink_message_unrefp) sd_netlink_message *m = NULL;
-        Manager *manager;
         Link *link = NULL;
-        Request *req;
         int r;
 
         assert(nexthop);
         assert(nexthop->id > 0);
-
-        manager = ASSERT_PTR(nexthop->manager);
+        assert(manager);
 
         /* link may be NULL. */
         (void) link_get_by_index(manager, nexthop->ifindex, &link);
@@ -460,9 +457,6 @@ static int nexthop_remove(NextHop *nexthop) {
         link_ref(link); /* link may be NULL, link_ref() is OK with that */
 
         nexthop_enter_removing(nexthop);
-        if (nexthop_get_request_by_id(manager, nexthop->id, &req) >= 0)
-                nexthop_enter_removing(req->userdata);
-
         return 0;
 }
 
@@ -809,7 +803,7 @@ int link_drop_nexthops(Link *link, bool foreign) {
                 if (!nexthop_is_marked(nexthop))
                         continue;
 
-                RET_GATHER(r, nexthop_remove(nexthop));
+                RET_GATHER(r, nexthop_remove(nexthop, link->manager));
         }
 
         return r;

src/network/networkd-nexthop.h

@@ -37,6 +37,8 @@ typedef struct NextHop {
 
 NextHop *nexthop_free(NextHop *nexthop);
 
+int nexthop_remove(NextHop *nexthop, Manager *manager);
+
 int network_drop_invalid_nexthops(Network *network);
 
 int link_drop_nexthops(Link *link, bool foreign);

test/test-network/systemd-networkd-tests.py

@@ -4024,7 +4024,11 @@ class NetworkdNetworkTests(unittest.TestCase, Utilities):
 
         remove_network_unit('25-nexthop-nothing.network', '25-nexthop-dummy-2.network')
         copy_network_unit('25-nexthop-1.network', '25-nexthop-dummy-1.network')
-        networkctl_reload()
+        # Of course, networkctl_reconfigure() below is unnecessary in normal operation, but it is intentional
+        # here to test reconfiguring with different .network files does not trigger race.
+        # See also comments in link_drop_requests().
+        networkctl_reconfigure('dummy98') # reconfigured with 25-nexthop-dummy-2.network
+        networkctl_reload()               # reconfigured with 25-nexthop-dummy-1.network
 
         self.check_nexthop(manage_foreign_nexthops, first=True)