From f5c615889a7340a7a06508bab8ff33dbcb729a7b Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 1 Oct 2023 12:04:52 +0900 Subject: [PATCH 1/2] sd-netlink: make calc_elapse() return USEC_INFINITY when no timeout is requested Then, timout_compare() becomes simplar, the timeout value becomes consistent with what sd_netlink_get_timeout() provides. This also drop unnecessary assignment of reply_callback.timeout after the slot is dropped from the prioq. --- src/libsystemd/sd-netlink/netlink-slot.c | 2 +- src/libsystemd/sd-netlink/sd-netlink.c | 24 ++++++------------------ 2 files changed, 7 insertions(+), 19 deletions(-) diff --git a/src/libsystemd/sd-netlink/netlink-slot.c b/src/libsystemd/sd-netlink/netlink-slot.c index b237db3c2f8..d85d2cd2f40 100644 --- a/src/libsystemd/sd-netlink/netlink-slot.c +++ b/src/libsystemd/sd-netlink/netlink-slot.c @@ -63,7 +63,7 @@ void netlink_slot_disconnect(sd_netlink_slot *slot, bool unref) { case NETLINK_REPLY_CALLBACK: (void) hashmap_remove(nl->reply_callbacks, &slot->reply_callback.serial); - if (slot->reply_callback.timeout != 0) + if (slot->reply_callback.timeout != USEC_INFINITY) prioq_remove(nl->reply_callbacks_prioq, &slot->reply_callback, &slot->reply_callback.prioq_idx); break; diff --git a/src/libsystemd/sd-netlink/sd-netlink.c b/src/libsystemd/sd-netlink/sd-netlink.c index ce0687eb57a..6028d24c380 100644 --- a/src/libsystemd/sd-netlink/sd-netlink.c +++ b/src/libsystemd/sd-netlink/sd-netlink.c @@ -215,7 +215,6 @@ static int process_timeout(sd_netlink *nl) { return r; assert_se(prioq_pop(nl->reply_callbacks_prioq) == c); - c->timeout = 0; hashmap_remove(nl->reply_callbacks, UINT32_TO_PTR(c->serial)); slot = container_of(c, sd_netlink_slot, reply_callback); @@ -248,10 +247,8 @@ static int process_reply(sd_netlink *nl, sd_netlink_message *m) { if (!c) return 0; - if (c->timeout != 0) { + if (c->timeout != USEC_INFINITY) prioq_remove(nl->reply_callbacks_prioq, c, &c->prioq_idx); - c->timeout = 0; - } r = sd_netlink_message_get_type(m, &type); if (r < 0) @@ -380,10 +377,7 @@ int sd_netlink_process(sd_netlink *nl, sd_netlink_message **ret) { return r; } -static usec_t calc_elapse(uint64_t usec) { - if (usec == UINT64_MAX) - return 0; - +static usec_t timespan_to_timestamp(usec_t usec) { if (usec == 0) usec = NETLINK_DEFAULT_TIMEOUT_USEC; @@ -442,12 +436,6 @@ int sd_netlink_wait(sd_netlink *nl, uint64_t timeout_usec) { static int timeout_compare(const void *a, const void *b) { const struct reply_callback *x = a, *y = b; - if (x->timeout != 0 && y->timeout == 0) - return -1; - - if (x->timeout == 0 && y->timeout != 0) - return 1; - return CMP(x->timeout, y->timeout); } @@ -487,7 +475,7 @@ int sd_netlink_call_async( return r; slot->reply_callback.callback = callback; - slot->reply_callback.timeout = calc_elapse(usec); + slot->reply_callback.timeout = timespan_to_timestamp(usec); k = sd_netlink_send(nl, m, &slot->reply_callback.serial); if (k < 0) @@ -497,7 +485,7 @@ int sd_netlink_call_async( if (r < 0) return r; - if (slot->reply_callback.timeout != 0) { + if (slot->reply_callback.timeout != USEC_INFINITY) { r = prioq_put(nl->reply_callbacks_prioq, &slot->reply_callback, &slot->reply_callback.prioq_idx); if (r < 0) { (void) hashmap_remove(nl->reply_callbacks, UINT32_TO_PTR(slot->reply_callback.serial)); @@ -528,7 +516,7 @@ int sd_netlink_read( assert_return(nl, -EINVAL); assert_return(!netlink_pid_changed(nl), -ECHILD); - timeout = calc_elapse(usec); + timeout = timespan_to_timestamp(usec); for (;;) { _cleanup_(sd_netlink_message_unrefp) sd_netlink_message *m = NULL; @@ -567,7 +555,7 @@ int sd_netlink_read( /* received message, so try to process straight away */ continue; - if (timeout > 0) { + if (timeout != USEC_INFINITY) { usec_t n; n = now(CLOCK_MONOTONIC); From 52afaee74b40a765b8118393bff92717f78d0a51 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 1 Oct 2023 12:04:59 +0900 Subject: [PATCH 2/2] sd-netlink: make the default timeout configurable by environment variable On normal systems, triggering a timeout should be a bug in code or configuration error, so I do not think we should extend the default timeout. Also, we should not introduce a 'first class' configuration option about that. But, making it configurable may be useful for cases such that "an extremely highly utilized system (lots of OOM kills, very high CPU utilization, etc)". Closes #25441. --- docs/ENVIRONMENT.md | 5 ++++- src/libsystemd/sd-netlink/sd-netlink.c | 23 +++++++++++++++++++++-- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/docs/ENVIRONMENT.md b/docs/ENVIRONMENT.md index 7517d15fdab..6396789e4a2 100644 --- a/docs/ENVIRONMENT.md +++ b/docs/ENVIRONMENT.md @@ -121,7 +121,10 @@ All tools: kernel supports this. * `$SYSTEMD_ENABLE_LOG_CONTEXT` — if set, extra fields will always be logged to -the journal instead of only when logging in debug mode. + the journal instead of only when logging in debug mode. + +* `$SYSTEMD_NETLINK_DEFAULT_TIMEOUT` — specifies the default timeout of waiting + replies for netlink messages from the kernel. Defaults to 25 seconds. `systemctl`: diff --git a/src/libsystemd/sd-netlink/sd-netlink.c b/src/libsystemd/sd-netlink/sd-netlink.c index 6028d24c380..d3c906de150 100644 --- a/src/libsystemd/sd-netlink/sd-netlink.c +++ b/src/libsystemd/sd-netlink/sd-netlink.c @@ -378,8 +378,27 @@ int sd_netlink_process(sd_netlink *nl, sd_netlink_message **ret) { } static usec_t timespan_to_timestamp(usec_t usec) { - if (usec == 0) - usec = NETLINK_DEFAULT_TIMEOUT_USEC; + static bool default_timeout_set = false; + static usec_t default_timeout; + int r; + + if (usec == 0) { + if (!default_timeout_set) { + const char *e; + + default_timeout_set = true; + default_timeout = NETLINK_DEFAULT_TIMEOUT_USEC; + + e = getenv("SYSTEMD_NETLINK_DEFAULT_TIMEOUT"); + if (e) { + r = parse_sec(e, &default_timeout); + if (r < 0) + log_debug_errno(r, "sd-netlink: Failed to parse $SYSTEMD_NETLINK_DEFAULT_TIMEOUT environment variable, ignoring: %m"); + } + } + + usec = default_timeout; + } return usec_add(now(CLOCK_MONOTONIC), usec); }