From ea97ec6cd01776bd666316a0a157a36244791329 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 14 Nov 2023 18:31:30 +0100 Subject: [PATCH 1/6] man: document StartLimitIntervalSec=infinity This seems to work as expected. In the issue, doubts were raised whether it works fine with daemon-reload/daemon-reexec, and it seems to work fine. (The property cannot be set via set-property, the dbus property is 'const'. We could relax this, but that'd be a separate feature.) Closes #29574. --- man/systemd.unit.xml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/man/systemd.unit.xml b/man/systemd.unit.xml index 2e9b87645fe..301fe77ce93 100644 --- a/man/systemd.unit.xml +++ b/man/systemd.unit.xml @@ -1166,6 +1166,8 @@ interval is a time span with the default unit of seconds, but other units may be specified, see systemd.time5. + The special value infinity can be used to limit the total number of start + attempts, even if they happen at large time intervals. Defaults to DefaultStartLimitIntervalSec= in manager configuration file, and may be set to 0 to disable any kind of rate limiting. burst is a number and defaults to DefaultStartLimitBurst= in manager configuration file. From 07a6647abbd74288a00b1b8f17ec9a9cce080ae4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 15 Nov 2023 17:17:12 +0100 Subject: [PATCH 2/6] core: split out the helper to serialize/deserialize ratelimits --- src/core/manager-serialize.c | 27 ++++----------------------- src/shared/serialize.c | 27 +++++++++++++++++++++++++++ src/shared/serialize.h | 3 +++ 3 files changed, 34 insertions(+), 23 deletions(-) diff --git a/src/core/manager-serialize.c b/src/core/manager-serialize.c index 1d7a1bed359..e9d567a97b8 100644 --- a/src/core/manager-serialize.c +++ b/src/core/manager-serialize.c @@ -152,13 +152,7 @@ int manager_serialize( (void) serialize_item_format(f, "user-lookup", "%i %i", copy0, copy1); } - (void) serialize_item_format(f, - "dump-ratelimit", - USEC_FMT " " USEC_FMT " %u %u", - m->dump_ratelimit.begin, - m->dump_ratelimit.interval, - m->dump_ratelimit.num, - m->dump_ratelimit.burst); + (void) serialize_ratelimit(f, "dump-ratelimit", &m->dump_ratelimit); bus_track_serialize(m->subscribed, f, "subscribed"); @@ -519,22 +513,9 @@ int manager_deserialize(Manager *m, FILE *f, FDSet *fds) { * remains set until all serialized contents are handled. */ if (deserialize_varlink_sockets) (void) varlink_server_deserialize_one(m->varlink_server, val, fds); - } else if ((val = startswith(l, "dump-ratelimit="))) { - usec_t begin, interval; - unsigned num, burst; - - if (sscanf(val, USEC_FMT " " USEC_FMT " %u %u", &begin, &interval, &num, &burst) != 4) - log_notice("Failed to parse dump ratelimit, ignoring: %s", val); - else { - /* If we changed the values across versions, flush the counter */ - if (interval != m->dump_ratelimit.interval || burst != m->dump_ratelimit.burst) - m->dump_ratelimit.num = 0; - else - m->dump_ratelimit.num = num; - m->dump_ratelimit.begin = begin; - } - - } else { + } else if ((val = startswith(l, "dump-ratelimit="))) + deserialize_ratelimit(&m->dump_ratelimit, "dump-ratelimit", val); + else { ManagerTimestamp q; for (q = 0; q < _MANAGER_TIMESTAMP_MAX; q++) { diff --git a/src/shared/serialize.c b/src/shared/serialize.c index 5019dbf1815..7099f67f92c 100644 --- a/src/shared/serialize.c +++ b/src/shared/serialize.c @@ -201,6 +201,17 @@ int serialize_pidref(FILE *f, FDSet *fds, const char *key, PidRef *pidref) { return serialize_item_format(f, key, "@%i", copy); } +int serialize_ratelimit(FILE *f, const char *key, const RateLimit *rl) { + assert(rl); + + return serialize_item_format(f, key, + USEC_FMT " " USEC_FMT " %u %u", + rl->begin, + rl->interval, + rl->num, + rl->burst); +} + int serialize_item_hexmem(FILE *f, const char *key, const void *p, size_t l) { _cleanup_free_ char *encoded = NULL; int r; @@ -486,6 +497,22 @@ int deserialize_pidref(FDSet *fds, const char *value, PidRef *ret) { return 0; } +void deserialize_ratelimit(RateLimit *rl, const char *name, const char *value) { + usec_t begin, interval; + unsigned num, burst; + + assert(rl); + assert(name); + assert(value); + + if (sscanf(value, USEC_FMT " " USEC_FMT " %u %u", &begin, &interval, &num, &burst) != 4) + return log_notice("Failed to parse %s, ignoring: %s", name, value); + + /* Preserve the counter only if the configuration didn't change. */ + rl->num = (interval == rl->interval && burst == rl->burst) ? num : 0; + rl->begin = begin; +} + int open_serialization_fd(const char *ident) { int fd; diff --git a/src/shared/serialize.h b/src/shared/serialize.h index c5211191f03..355eff9b8ff 100644 --- a/src/shared/serialize.h +++ b/src/shared/serialize.h @@ -7,6 +7,7 @@ #include "image-policy.h" #include "macro.h" #include "pidref.h" +#include "ratelimit.h" #include "set.h" #include "string-util.h" #include "time-util.h" @@ -22,6 +23,7 @@ int serialize_usec(FILE *f, const char *key, usec_t usec); int serialize_dual_timestamp(FILE *f, const char *key, const dual_timestamp *t); int serialize_strv(FILE *f, const char *key, char **l); int serialize_pidref(FILE *f, FDSet *fds, const char *key, PidRef *pidref); +int serialize_ratelimit(FILE *f, const char *key, const RateLimit *rl); int serialize_string_set(FILE *f, const char *key, Set *s); int serialize_image_policy(FILE *f, const char *key, const ImagePolicy *p); @@ -45,6 +47,7 @@ int deserialize_dual_timestamp(const char *value, dual_timestamp *ret); int deserialize_environment(const char *value, char ***environment); int deserialize_strv(const char *value, char ***l); int deserialize_pidref(FDSet *fds, const char *value, PidRef *ret); +void deserialize_ratelimit(RateLimit *rl, const char *name, const char *value); int open_serialization_fd(const char *ident); int open_serialization_file(const char *ident, FILE **ret); From 6ef512c0bb7aeb2000588d7d05e23b4681da8657 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 15 Nov 2023 17:23:27 +0100 Subject: [PATCH 3/6] core: serialize and deserialize unit start ratelimits The logic is taken from dump ratelimit: if the config changes, we discard the counters. This allows the user apply new limits and "start from scratch" in that case. This actually makes StartLimitIntervalSec=infinity (or with a large interval) work as expected, because the counter is maintained even if daemon-reload operations are interleaved. --- src/core/unit-serialize.c | 6 ++++++ test/units/testsuite-04.SYSTEMD_JOURNAL_COMPRESS.sh | 4 ++++ test/units/testsuite-67.sh | 2 ++ 3 files changed, 12 insertions(+) diff --git a/src/core/unit-serialize.c b/src/core/unit-serialize.c index af3e1c20899..bef3654525d 100644 --- a/src/core/unit-serialize.c +++ b/src/core/unit-serialize.c @@ -140,6 +140,8 @@ int unit_serialize_state(Unit *u, FILE *f, FDSet *fds, bool switching_root) { (void) serialize_dual_timestamp(f, "condition-timestamp", &u->condition_timestamp); (void) serialize_dual_timestamp(f, "assert-timestamp", &u->assert_timestamp); + (void) serialize_ratelimit(f, "start-ratelimit", &u->start_ratelimit); + if (dual_timestamp_is_set(&u->condition_timestamp)) (void) serialize_bool(f, "condition-result", u->condition_result); @@ -343,6 +345,10 @@ int unit_deserialize_state(Unit *u, FILE *f, FDSet *fds) { (void) deserialize_dual_timestamp(v, &u->assert_timestamp); continue; + } else if (streq(l, "start-ratelimit")) { + deserialize_ratelimit(&u->start_ratelimit, l, v); + continue; + } else if (MATCH_DESERIALIZE("condition-result", l, v, parse_boolean, u->condition_result)) continue; diff --git a/test/units/testsuite-04.SYSTEMD_JOURNAL_COMPRESS.sh b/test/units/testsuite-04.SYSTEMD_JOURNAL_COMPRESS.sh index 8fdf1b132e0..96d096d9adf 100755 --- a/test/units/testsuite-04.SYSTEMD_JOURNAL_COMPRESS.sh +++ b/test/units/testsuite-04.SYSTEMD_JOURNAL_COMPRESS.sh @@ -7,6 +7,9 @@ set -o pipefail mkdir /run/systemd/system/systemd-journald.service.d MACHINE_ID="$(/run/systemd/system/systemd-journald.service.d/compress.conf < Date: Fri, 17 Nov 2023 17:53:00 +0100 Subject: [PATCH 4/6] core: serialize and deserialize auto start/stop ratelimit The limit is not configurable, so the logic in the helper will always update the counters. The helper is a bit overkill, but it doesn't really matter. --- src/core/unit-serialize.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/core/unit-serialize.c b/src/core/unit-serialize.c index bef3654525d..fe4221ca46b 100644 --- a/src/core/unit-serialize.c +++ b/src/core/unit-serialize.c @@ -141,6 +141,7 @@ int unit_serialize_state(Unit *u, FILE *f, FDSet *fds, bool switching_root) { (void) serialize_dual_timestamp(f, "assert-timestamp", &u->assert_timestamp); (void) serialize_ratelimit(f, "start-ratelimit", &u->start_ratelimit); + (void) serialize_ratelimit(f, "auto-start-stop-ratelimit", &u->auto_start_stop_ratelimit); if (dual_timestamp_is_set(&u->condition_timestamp)) (void) serialize_bool(f, "condition-result", u->condition_result); @@ -348,6 +349,9 @@ int unit_deserialize_state(Unit *u, FILE *f, FDSet *fds) { } else if (streq(l, "start-ratelimit")) { deserialize_ratelimit(&u->start_ratelimit, l, v); continue; + } else if (streq(l, "auto-start-stop-ratelimit")) { + deserialize_ratelimit(&u->auto_start_stop_ratelimit, l, v); + continue; } else if (MATCH_DESERIALIZE("condition-result", l, v, parse_boolean, u->condition_result)) continue; From fed25720efa0fb0b94a0b13a7bf2b1df971280e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 17 Nov 2023 17:55:35 +0100 Subject: [PATCH 5/6] core: use uniform style for RateLimit initialization RateLimit is designed so that we can always initialize only the first two fields explicitly. All other call sites use a single line for this. --- src/basic/ratelimit.h | 2 ++ src/core/manager.c | 5 +---- src/core/path.c | 3 +-- src/core/socket.c | 3 +-- src/core/unit.c | 9 +++------ 5 files changed, 8 insertions(+), 14 deletions(-) diff --git a/src/basic/ratelimit.h b/src/basic/ratelimit.h index bb7160a895b..492ea3b48dd 100644 --- a/src/basic/ratelimit.h +++ b/src/basic/ratelimit.h @@ -12,6 +12,8 @@ typedef struct RateLimit { usec_t begin; } RateLimit; +#define RATELIMIT_OFF (const RateLimit) { .interval = USEC_INFINITY, .burst = UINT_MAX } + static inline void ratelimit_reset(RateLimit *rl) { rl->num = rl->begin = 0; } diff --git a/src/core/manager.c b/src/core/manager.c index fcb7739b136..57ec5f38609 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -927,10 +927,7 @@ int manager_new(RuntimeScope runtime_scope, ManagerTestRunFlags test_run_flags, .first_boot = -1, .test_run_flags = test_run_flags, - .dump_ratelimit = { - .interval = 10 * USEC_PER_MINUTE, - .burst = 10, - }, + .dump_ratelimit = (const RateLimit) { .interval = 10 * USEC_PER_MINUTE, .burst = 10 }, .executor_fd = -EBADF, }; diff --git a/src/core/path.c b/src/core/path.c index 3ffa0acc625..c09f001d50a 100644 --- a/src/core/path.c +++ b/src/core/path.c @@ -279,8 +279,7 @@ static void path_init(Unit *u) { p->directory_mode = 0755; - p->trigger_limit.interval = USEC_INFINITY; - p->trigger_limit.burst = UINT_MAX; + p->trigger_limit = RATELIMIT_OFF; } void path_free_specs(Path *p) { diff --git a/src/core/socket.c b/src/core/socket.c index 156ac4a2d59..cb28c788407 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -100,8 +100,7 @@ static void socket_init(Unit *u) { s->control_pid = PIDREF_NULL; s->control_command_id = _SOCKET_EXEC_COMMAND_INVALID; - s->trigger_limit.interval = USEC_INFINITY; - s->trigger_limit.burst = UINT_MAX; + s->trigger_limit = RATELIMIT_OFF; s->poll_limit_interval = USEC_INFINITY; s->poll_limit_burst = UINT_MAX; diff --git a/src/core/unit.c b/src/core/unit.c index b37b1710f26..41f3bdb226a 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -132,15 +132,12 @@ Unit* unit_new(Manager *m, size_t size) { u->last_section_private = -1; - u->start_ratelimit = (RateLimit) { + u->start_ratelimit = (const RateLimit) { m->defaults.start_limit_interval, - m->defaults.start_limit_burst + m->defaults.start_limit_burst, }; - u->auto_start_stop_ratelimit = (const RateLimit) { - 10 * USEC_PER_SEC, - 16 - }; + u->auto_start_stop_ratelimit = (const RateLimit) { .interval = 10 * USEC_PER_SEC, .burst = 16 }; return u; } From 51ad723d209579817e008bd29f62a8925518f61d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 17 Nov 2023 18:10:50 +0100 Subject: [PATCH 6/6] core: serialize and deserialize trigger ratelimits for socket and path --- src/core/path.c | 7 ++++++- src/core/socket.c | 7 ++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/core/path.c b/src/core/path.c index c09f001d50a..44481a95d58 100644 --- a/src/core/path.c +++ b/src/core/path.c @@ -682,6 +682,8 @@ static int path_serialize(Unit *u, FILE *f, FDSet *fds) { escaped); } + (void) serialize_ratelimit(f, "trigger-ratelimit", &p->trigger_limit); + return 0; } @@ -743,7 +745,10 @@ static int path_deserialize_item(Unit *u, const char *key, const char *value, FD } } - } else + } else if (streq(key, "trigger-ratelimit")) + deserialize_ratelimit(&p->trigger_limit, key, value); + + else log_unit_debug(u, "Unknown serialization key: %s", key); return 0; diff --git a/src/core/socket.c b/src/core/socket.c index cb28c788407..388be623184 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -2589,6 +2589,8 @@ static int socket_serialize(Unit *u, FILE *f, FDSet *fds) { } } + (void) serialize_ratelimit(f, "trigger-ratelimit", &s->trigger_limit); + return 0; } @@ -2823,7 +2825,10 @@ static int socket_deserialize_item(Unit *u, const char *key, const char *value, if (!found) log_unit_debug(u, "No matching ffs socket found: %s", value); - } else + } else if (streq(key, "trigger-ratelimit")) + deserialize_ratelimit(&s->trigger_limit, key, value); + + else log_unit_debug(UNIT(s), "Unknown serialization key: %s", key); return 0;