From 0a35458f5eff59225aaa53734e6194f3e548ba03 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 2 Sep 2024 12:27:04 +0900 Subject: [PATCH] network/qdisc: do not save qdisc to Link before it is configured Otherwise, if the same kind of qdisc is already assigned, parameters configured in .network file will not be used. So, let's first copy the qdisc and put it on Request, then on success generate a new copy based on the netlink notification and store it to Link. This is the same as 0a0c2672dbd22dc85d660e5baa7e1bef701beb88, 65f5f581568448d6098358b704cae10a656d09f0, and friends, but for qdisc. Preparation for fixing #31226. --- src/network/tc/qdisc.c | 115 ++++++++++++++++++++++------------------- src/network/tc/qdisc.h | 2 +- src/network/tc/tc.c | 2 +- 3 files changed, 65 insertions(+), 54 deletions(-) diff --git a/src/network/tc/qdisc.c b/src/network/tc/qdisc.c index 851f624c459..0f89d844f58 100644 --- a/src/network/tc/qdisc.c +++ b/src/network/tc/qdisc.c @@ -378,11 +378,15 @@ void link_qdisc_drop_marked(Link *link) { assert(link); SET_FOREACH(qdisc, link->qdiscs) { + Request *req; + if (!qdisc_is_marked(qdisc)) continue; qdisc_unmark(qdisc); qdisc_enter_removed(qdisc); + if (qdisc_get_request(link, qdisc, &req) >= 0) + qdisc_enter_removed(req->userdata); if (qdisc->state == 0) { log_qdisc_debug(qdisc, link, "Forgetting"); @@ -483,6 +487,7 @@ static bool qdisc_is_ready_to_configure(QDisc *qdisc, Link *link) { } static int qdisc_process_request(Request *req, Link *link, QDisc *qdisc) { + QDisc *existing; int r; assert(req); @@ -497,54 +502,56 @@ static int qdisc_process_request(Request *req, Link *link, QDisc *qdisc) { return log_link_warning_errno(link, r, "Failed to configure QDisc: %m"); qdisc_enter_configuring(qdisc); + if (qdisc_get(link, qdisc, &existing) >= 0) + qdisc_enter_configuring(existing); + return 1; } -int link_request_qdisc(Link *link, QDisc *qdisc) { - QDisc *existing; +int link_request_qdisc(Link *link, const QDisc *qdisc) { + _cleanup_(qdisc_unrefp) QDisc *tmp = NULL; + QDisc *existing = NULL; int r; assert(link); assert(qdisc); + assert(qdisc->source != NETWORK_CONFIG_SOURCE_FOREIGN); if (qdisc_get_request(link, qdisc, NULL) >= 0) return 0; /* already requested, skipping. */ - if (qdisc_get(link, qdisc, &existing) < 0) { - _cleanup_(qdisc_unrefp) QDisc *tmp = NULL; - - r = qdisc_dup(qdisc, &tmp); - if (r < 0) - return log_oom(); - - r = qdisc_attach(link, tmp); - if (r < 0) - return log_link_warning_errno(link, r, "Failed to store QDisc: %m"); + r = qdisc_dup(qdisc, &tmp); + if (r < 0) + return r; - existing = tmp; - } else - existing->source = qdisc->source; + if (qdisc_get(link, qdisc, &existing) >= 0) + /* Copy state for logging below. */ + tmp->state = existing->state; - log_qdisc_debug(existing, link, "Requesting"); + log_qdisc_debug(tmp, link, "Requesting"); r = link_queue_request_safe(link, REQUEST_TYPE_TC_QDISC, - existing, NULL, + tmp, + qdisc_unref, qdisc_hash_func, qdisc_compare_func, qdisc_process_request, &link->tc_messages, qdisc_handler, NULL); - if (r < 0) - return log_link_warning_errno(link, r, "Failed to request QDisc: %m"); - if (r == 0) - return 0; + if (r <= 0) + return r; - qdisc_enter_requesting(existing); + qdisc_enter_requesting(tmp); + if (existing) + qdisc_enter_requesting(existing); + + TAKE_PTR(tmp); return 1; } int manager_rtnl_process_qdisc(sd_netlink *rtnl, sd_netlink_message *message, Manager *m) { _cleanup_(qdisc_unrefp) QDisc *tmp = NULL; + Request *req = NULL; QDisc *qdisc = NULL; Link *link; uint16_t type; @@ -609,45 +616,49 @@ int manager_rtnl_process_qdisc(sd_netlink *rtnl, sd_netlink_message *message, Ma } (void) qdisc_get(link, tmp, &qdisc); + (void) qdisc_get_request(link, tmp, &req); - switch (type) { - case RTM_NEWQDISC: - if (qdisc) { - qdisc_enter_configured(qdisc); - log_qdisc_debug(qdisc, link, "Received remembered"); - } else { - qdisc_enter_configured(tmp); - log_qdisc_debug(tmp, link, "Received new"); + if (type == RTM_DELQDISC) { + if (qdisc) + qdisc_drop(qdisc); + else + log_qdisc_debug(tmp, link, "Kernel removed unknown"); - r = qdisc_attach(link, tmp); - if (r < 0) { - log_link_warning_errno(link, r, "Failed to remember QDisc, ignoring: %m"); - return 0; - } + return 0; + } - qdisc = tmp; + bool is_new = false; + if (!qdisc) { + /* If we did not know the qdisc, then save it. */ + r = qdisc_attach(link, tmp); + if (r < 0) { + log_link_warning_errno(link, r, "Failed to remember QDisc, ignoring: %m"); + return 0; } - if (!m->enumerating) { - /* Some kind of QDisc (e.g. tbf) also create an implicit class under the qdisc, but - * the kernel may not notify about the class. Hence, we need to enumerate classes. */ - r = link_enumerate_tclass(link, qdisc->handle); - if (r < 0) - log_link_warning_errno(link, r, "Failed to enumerate TClass, ignoring: %m"); - } + qdisc = tmp; + is_new = true; + } - break; + /* Also update information that cannot be obtained through netlink notification. */ + if (req && req->waiting_reply) { + QDisc *q = ASSERT_PTR(req->userdata); - case RTM_DELQDISC: - if (qdisc) - qdisc_drop(qdisc); - else - log_qdisc_debug(tmp, link, "Kernel removed unknown"); + qdisc->source = q->source; + } - break; + qdisc_enter_configured(qdisc); + if (req) + qdisc_enter_configured(req->userdata); - default: - assert_not_reached(); + log_qdisc_debug(qdisc, link, is_new ? "Remembering" : "Received remembered"); + + if (!m->enumerating) { + /* Some kind of QDisc (e.g. tbf) also create an implicit class under the qdisc, but + * the kernel may not notify about the class. Hence, we need to enumerate classes. */ + r = link_enumerate_tclass(link, qdisc->handle); + if (r < 0) + log_link_warning_errno(link, r, "Failed to enumerate TClass, ignoring: %m"); } return 1; diff --git a/src/network/tc/qdisc.h b/src/network/tc/qdisc.h index 75351fb8ada..50a8f4ead19 100644 --- a/src/network/tc/qdisc.h +++ b/src/network/tc/qdisc.h @@ -85,7 +85,7 @@ void link_qdisc_drop_marked(Link *link); int link_find_qdisc(Link *link, uint32_t handle, const char *kind, QDisc **qdisc); -int link_request_qdisc(Link *link, QDisc *qdisc); +int link_request_qdisc(Link *link, const QDisc *qdisc); void network_drop_invalid_qdisc(Network *network); diff --git a/src/network/tc/tc.c b/src/network/tc/tc.c index 8a1c5b3a3b3..37e30441e16 100644 --- a/src/network/tc/tc.c +++ b/src/network/tc/tc.c @@ -20,7 +20,7 @@ int link_request_traffic_control(Link *link) { HASHMAP_FOREACH(qdisc, link->network->qdiscs_by_section) { r = link_request_qdisc(link, qdisc); if (r < 0) - return r; + return log_link_warning_errno(link, r, "Failed to request QDisc: %m"); } HASHMAP_FOREACH(tclass, link->network->tclasses_by_section) { -- 2.47.3