1 files changed, 198 insertions, 66 deletions

diff --git a/src/network/networkd-queue.c b/src/network/networkd-queue.c
index 1128987..98c629f 100644
--- a/src/network/networkd-queue.c
+++ b/src/network/networkd-queue.c
@@ -9,14 +9,28 @@
 
 #define REPLY_CALLBACK_COUNT_THRESHOLD 128
 
+static Request* request_detach_impl(Request *req) {
+        assert(req);
+
+        if (!req->manager)
+                return NULL;
+
+        ordered_set_remove(req->manager->request_queue, req);
+        req->manager = NULL;
+        return req;
+}
+
+void request_detach(Request *req) {
+        request_unref(request_detach_impl(req));
+}
+
 static Request *request_free(Request *req) {
         if (!req)
                 return NULL;
 
         /* To prevent from triggering assertions in the hash and compare functions, remove this request
          * from the set before freeing userdata below. */
-        if (req->manager)
-                ordered_set_remove(req->manager->request_queue, req);
+        request_detach_impl(req);
 
         if (req->free_func)
                 req->free_func(req->userdata);
@@ -31,26 +45,10 @@ static Request *request_free(Request *req) {
 
 DEFINE_TRIVIAL_REF_UNREF_FUNC(Request, request, request_free);
 
-void request_detach(Manager *manager, Request *req) {
-        assert(manager);
-
-        if (!req)
-                return;
-
-        req = ordered_set_remove(manager->request_queue, req);
-        if (!req)
-                return;
-
-        req->manager = NULL;
-        request_unref(req);
-}
-
 static void request_destroy_callback(Request *req) {
         assert(req);
 
-        if (req->manager)
-                request_detach(req->manager, req);
-
+        request_detach(req);
         request_unref(req);
 }
 
@@ -58,14 +56,16 @@ static void request_hash_func(const Request *req, struct siphash *state) {
         assert(req);
         assert(state);
 
-        siphash24_compress_boolean(req->link, state);
-        if (req->link)
-                siphash24_compress(&req->link->ifindex, sizeof(req->link->ifindex), state);
+        siphash24_compress_typesafe(req->type, state);
 
-        siphash24_compress(&req->type, sizeof(req->type), state);
+        if (!IN_SET(req->type, REQUEST_TYPE_NEXTHOP, REQUEST_TYPE_ROUTE)) {
+                siphash24_compress_boolean(req->link, state);
+                if (req->link)
+                        siphash24_compress_typesafe(req->link->ifindex, state);
+        }
 
-        siphash24_compress(&req->hash_func, sizeof(req->hash_func), state);
-        siphash24_compress(&req->compare_func, sizeof(req->compare_func), state);
+        siphash24_compress_typesafe(req->hash_func, state);
+        siphash24_compress_typesafe(req->compare_func, state);
 
         if (req->hash_func)
                 req->hash_func(req->userdata, state);
@@ -77,19 +77,21 @@ static int request_compare_func(const struct Request *a, const struct Request *b
         assert(a);
         assert(b);
 
-        r = CMP(!!a->link, !!b->link);
+        r = CMP(a->type, b->type);
         if (r != 0)
                 return r;
 
-        if (a->link) {
-                r = CMP(a->link->ifindex, b->link->ifindex);
+        if (!IN_SET(a->type, REQUEST_TYPE_NEXTHOP, REQUEST_TYPE_ROUTE)) {
+                r = CMP(!!a->link, !!b->link);
                 if (r != 0)
                         return r;
-        }
 
-        r = CMP(a->type, b->type);
-        if (r != 0)
-                return r;
+                if (a->link) {
+                        r = CMP(a->link->ifindex, b->link->ifindex);
+                        if (r != 0)
+                                return r;
+                }
+        }
 
         r = CMP(PTR_TO_UINT64(a->hash_func), PTR_TO_UINT64(b->hash_func));
         if (r != 0)
@@ -110,7 +112,7 @@ DEFINE_PRIVATE_HASH_OPS_WITH_KEY_DESTRUCTOR(
                 Request,
                 request_hash_func,
                 request_compare_func,
-                request_unref);
+                request_detach);
 
 static int request_new(
                 Manager *manager,
@@ -164,6 +166,10 @@ static int request_new(
         if (req->counter)
                 (*req->counter)++;
 
+        /* If this is called in the ORDERED_SET_FOREACH() loop of manager_process_requests(), we need to
+         * exit from the loop, due to the limitation of the iteration on OrderedSet. */
+        manager->request_queued = true;
+
         if (ret)
                 *ret = req;
 
@@ -210,52 +216,70 @@ int link_queue_request_full(
                            process, counter, netlink_handler, ret);
 }
 
-int manager_process_requests(sd_event_source *s, void *userdata) {
-        Manager *manager = ASSERT_PTR(userdata);
-        int r;
+int link_requeue_request(Link *link, Request *req, void *userdata, Request **ret) {
+        assert(link);
+        assert(req);
 
-        for (;;) {
-                bool processed = false;
-                Request *req;
+        return link_queue_request_full(
+                        link,
+                        req->type,
+                        userdata,
+                        req->free_func,
+                        req->hash_func,
+                        req->compare_func,
+                        req->process,
+                        req->counter,
+                        req->netlink_handler,
+                        ret);
+}
 
-                ORDERED_SET_FOREACH(req, manager->request_queue) {
-                        _cleanup_(link_unrefp) Link *link = link_ref(req->link);
+int manager_process_requests(Manager *manager) {
+        Request *req;
+        int r;
 
-                        assert(req->process);
+        assert(manager);
 
-                        if (req->waiting_reply)
-                                continue; /* Waiting for netlink reply. */
+        /* Process only when no remove request is queued. */
+        if (!ordered_set_isempty(manager->remove_request_queue))
+                return 0;
 
-                        /* Typically, requests send netlink message asynchronously. If there are many requests
-                         * queued, then this event may make reply callback queue in sd-netlink full. */
-                        if (netlink_get_reply_callback_count(manager->rtnl) >= REPLY_CALLBACK_COUNT_THRESHOLD ||
-                            netlink_get_reply_callback_count(manager->genl) >= REPLY_CALLBACK_COUNT_THRESHOLD ||
-                            fw_ctx_get_reply_callback_count(manager->fw_ctx) >= REPLY_CALLBACK_COUNT_THRESHOLD)
-                                return 0;
+        manager->request_queued = false;
 
-                        r = req->process(req, link, req->userdata);
-                        if (r == 0)
-                                continue;
+        ORDERED_SET_FOREACH(req, manager->request_queue) {
+                if (req->waiting_reply)
+                        continue; /* Already processed, and waiting for netlink reply. */
 
-                        processed = true;
+                /* Typically, requests send netlink message asynchronously. If there are many requests
+                 * queued, then this event may make reply callback queue in sd-netlink full. */
+                if (netlink_get_reply_callback_count(manager->rtnl) >= REPLY_CALLBACK_COUNT_THRESHOLD ||
+                    netlink_get_reply_callback_count(manager->genl) >= REPLY_CALLBACK_COUNT_THRESHOLD ||
+                    fw_ctx_get_reply_callback_count(manager->fw_ctx) >= REPLY_CALLBACK_COUNT_THRESHOLD)
+                        break;
 
-                        /* If the request sends netlink message, e.g. for Address or so, the Request object
-                         * is referenced by the netlink slot, and will be detached later by its destroy callback.
-                         * Otherwise, e.g. for DHCP client or so, detach the request from queue now. */
-                        if (!req->waiting_reply)
-                                request_detach(manager, req);
+                /* Avoid the request and link freed by req->process() and request_detach(). */
+                _unused_ _cleanup_(request_unrefp) Request *req_unref = request_ref(req);
+                _cleanup_(link_unrefp) Link *link = link_ref(req->link);
 
-                        if (r < 0 && link) {
+                assert(req->process);
+                r = req->process(req, link, req->userdata);
+                if (r < 0) {
+                        request_detach(req);
+
+                        if (link) {
                                 link_enter_failed(link);
-                                /* link_enter_failed() may remove multiple requests,
-                                 * hence we need to exit from the loop. */
+                                /* link_enter_failed() may detach multiple requests from the queue.
+                                 * Hence, we need to exit from the loop. */
                                 break;
                         }
                 }
+                if (r > 0 && !req->waiting_reply)
+                        /* If the request sends netlink message, e.g. for Address or so, the Request object is
+                         * referenced by the netlink slot, and will be detached later by its destroy callback.
+                         * Otherwise, e.g. for DHCP client or so, detach the request from queue now. */
+                        request_detach(req);
 
-                /* When at least one request is processed, then another request may be ready now. */
-                if (!processed)
-                        break;
+                if (manager->request_queued)
+                        break; /* New request is queued. Exit from the loop. */
         }
 
         return 0;
@@ -316,7 +340,8 @@ static const char *const request_type_table[_REQUEST_TYPE_MAX] = {
         [REQUEST_TYPE_SET_LINK_ADDRESS_GENERATION_MODE] = "IPv6LL address generation mode",
         [REQUEST_TYPE_SET_LINK_BOND]                    = "bond configurations",
         [REQUEST_TYPE_SET_LINK_BRIDGE]                  = "bridge configurations",
-        [REQUEST_TYPE_SET_LINK_BRIDGE_VLAN]             = "bridge VLAN configurations",
+        [REQUEST_TYPE_SET_LINK_BRIDGE_VLAN]             = "bridge VLAN configurations (step 1)",
+        [REQUEST_TYPE_DEL_LINK_BRIDGE_VLAN]             = "bridge VLAN configurations (step 2)",
         [REQUEST_TYPE_SET_LINK_CAN]                     = "CAN interface configurations",
         [REQUEST_TYPE_SET_LINK_FLAGS]                   = "link flags",
         [REQUEST_TYPE_SET_LINK_GROUP]                   = "interface group",
@@ -331,3 +356,110 @@ static const char *const request_type_table[_REQUEST_TYPE_MAX] = {
 };
 
 DEFINE_STRING_TABLE_LOOKUP_TO_STRING(request_type, RequestType);
+
+static RemoveRequest* remove_request_free(RemoveRequest *req) {
+        if (!req)
+                return NULL;
+
+        if (req->manager)
+                ordered_set_remove(req->manager->remove_request_queue, req);
+
+        if (req->unref_func)
+                req->unref_func(req->userdata);
+
+        link_unref(req->link);
+        sd_netlink_unref(req->netlink);
+        sd_netlink_message_unref(req->message);
+
+        return mfree(req);
+}
+
+DEFINE_TRIVIAL_CLEANUP_FUNC(RemoveRequest*, remove_request_free);
+DEFINE_TRIVIAL_DESTRUCTOR(remove_request_destroy_callback, RemoveRequest, remove_request_free);
+DEFINE_PRIVATE_HASH_OPS_WITH_KEY_DESTRUCTOR(
+                remove_request_hash_ops,
+                void,
+                trivial_hash_func,
+                trivial_compare_func,
+                remove_request_free);
+
+int remove_request_add(
+                Manager *manager,
+                Link *link,
+                void *userdata,
+                mfree_func_t unref_func,
+                sd_netlink *netlink,
+                sd_netlink_message *message,
+                remove_request_netlink_handler_t netlink_handler) {
+
+        _cleanup_(remove_request_freep) RemoveRequest *req = NULL;
+        int r;
+
+        assert(manager);
+        assert(userdata);
+        assert(netlink);
+        assert(message);
+
+        req = new(RemoveRequest, 1);
+        if (!req)
+                return -ENOMEM;
+
+        *req = (RemoveRequest) {
+                .link = link_ref(link), /* link may be NULL, but link_ref() handles it gracefully. */
+                .userdata = userdata,
+                .netlink = sd_netlink_ref(netlink),
+                .message = sd_netlink_message_ref(message),
+                .netlink_handler = netlink_handler,
+        };
+
+        r = ordered_set_ensure_put(&manager->remove_request_queue, &remove_request_hash_ops, req);
+        if (r < 0)
+                return r;
+        assert(r > 0);
+
+        req->manager = manager;
+        req->unref_func = unref_func;
+
+        TAKE_PTR(req);
+        return 0;
+}
+
+int manager_process_remove_requests(Manager *manager) {
+        RemoveRequest *req;
+        int r;
+
+        assert(manager);
+
+        while ((req = ordered_set_first(manager->remove_request_queue))) {
+
+                /* Do not make the reply callback queue in sd-netlink full. */
+                if (netlink_get_reply_callback_count(req->netlink) >= REPLY_CALLBACK_COUNT_THRESHOLD)
+                        return 0;
+
+                r = netlink_call_async(
+                                req->netlink, NULL, req->message,
+                                req->netlink_handler,
+                                remove_request_destroy_callback,
+                                req);
+                if (r < 0) {
+                        _cleanup_(link_unrefp) Link *link = link_ref(req->link);
+
+                        log_link_warning_errno(link, r, "Failed to call netlink message: %m");
+
+                        /* First free the request. */
+                        remove_request_free(req);
+
+                        /* Then, make the link enter the failed state. */
+                        if (link)
+                                link_enter_failed(link);
+
+                } else {
+                        /* On success, netlink needs to be unref()ed. Otherwise, the netlink and remove
+                         * request may not freed on shutting down. */
+                        req->netlink = sd_netlink_unref(req->netlink);
+                        ordered_set_remove(manager->remove_request_queue, req);
+                }
+        }
+
+        return 0;
+}