From 981278e1441ff9ee803c3c3b2efe6d17d482d986 Mon Sep 17 00:00:00 2001 From: Yu Watanabe <watanabe.yu+github@gmail.com> Date: Fri, 5 Jan 2024 00:13:40 +0900 Subject: [PATCH 1/2] network/queue: do not check if a request is ready multiple times in a single event Some checks are slightly heavy, and there may be huge number of interfaces. So, prcessing whole queue multiple times in a single event may decrease the performance. Let's process the queued requests once per event. --- src/network/networkd-queue.c | 60 +++++++++++++++--------------------- 1 file changed, 25 insertions(+), 35 deletions(-) diff --git a/src/network/networkd-queue.c b/src/network/networkd-queue.c index 6fafe42c0f8..7c626309686 100644 --- a/src/network/networkd-queue.c +++ b/src/network/networkd-queue.c @@ -215,52 +215,42 @@ int link_queue_request_full( } int manager_process_requests(Manager *manager) { + Request *req; int r; assert(manager); - for (;;) { - bool processed = false; - Request *req; + ORDERED_SET_FOREACH(req, manager->request_queue) { + _cleanup_(link_unrefp) Link *link = link_ref(req->link); - ORDERED_SET_FOREACH(req, manager->request_queue) { - _cleanup_(link_unrefp) Link *link = link_ref(req->link); + assert(req->process); - assert(req->process); + if (req->waiting_reply) + continue; /* Waiting for netlink reply. */ - if (req->waiting_reply) - continue; /* Waiting for netlink reply. */ + /* 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; - /* 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; + r = req->process(req, link, req->userdata); + if (r == 0) + continue; /* The request is not ready. */ - r = req->process(req, link, req->userdata); - if (r == 0) - continue; + /* 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); - processed = true; - - /* 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); - - if (r < 0 && link) { - link_enter_failed(link); - /* link_enter_failed() may remove multiple requests, - * hence we need to exit from the loop. */ - break; - } - } - - /* When at least one request is processed, then another request may be ready now. */ - if (!processed) + if (r < 0 && link) { + link_enter_failed(link); + /* link_enter_failed() may remove multiple requests, + * hence we need to exit from the loop. */ break; + } } return 0; From 4f6b801b0d69a01971cdb3745b7e9df0e2fe6c6f Mon Sep 17 00:00:00 2001 From: Yu Watanabe <watanabe.yu+github@gmail.com> Date: Thu, 4 Jan 2024 23:53:27 +0900 Subject: [PATCH 2/2] network/queue: stop processing requests when a new request is queued Otherwise, the loop triggers assertion: ``` Assertion 'e->p.b.key == i->next_key' failed at src/basic/hashmap.c:614, function hashmap_iterate_in_insertion_order(). Aborting. ``` --- src/network/networkd-manager.h | 1 + src/network/networkd-queue.c | 16 ++++++++++++++-- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/network/networkd-manager.h b/src/network/networkd-manager.h index 3f3569f44d5..a2edfd0e79c 100644 --- a/src/network/networkd-manager.h +++ b/src/network/networkd-manager.h @@ -100,6 +100,7 @@ struct Manager { FirewallContext *fw_ctx; + bool request_queued; OrderedSet *request_queue; Hashmap *tuntap_fds_by_name; diff --git a/src/network/networkd-queue.c b/src/network/networkd-queue.c index 7c626309686..1678510d522 100644 --- a/src/network/networkd-queue.c +++ b/src/network/networkd-queue.c @@ -168,6 +168,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; @@ -220,6 +224,8 @@ int manager_process_requests(Manager *manager) { assert(manager); + manager->request_queued = false; + ORDERED_SET_FOREACH(req, manager->request_queue) { _cleanup_(link_unrefp) Link *link = link_ref(req->link); @@ -236,8 +242,11 @@ int manager_process_requests(Manager *manager) { return 0; r = req->process(req, link, req->userdata); - if (r == 0) - continue; /* The request is not ready. */ + if (r == 0) { /* The request is not ready. */ + if (manager->request_queued) + break; /* a new request is queued during processing the request. */ + continue; + } /* 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. @@ -251,6 +260,9 @@ int manager_process_requests(Manager *manager) { * hence we need to exit from the loop. */ break; } + + if (manager->request_queued) + break; } return 0;