Re: [PATCH net-next v2 4/4] docs: netconsole: document rate limit feature
flat view
From: netdev-bot+sashiko@kernel.org
Date: 2026-09-12 00:06:38
Also in:
linux-doc, linux-kselftest, lkml
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: Critical: 0 · High: 0 · Medium: 1 · Low: 1 - [Medium] The live-update path for the two rate-limit knobs does not reset the token bucket, so the behaviour advertised by the new… - [Low] Documentation/networking/netconsole.rst: the canonical configfs inventory for a netconsole target ("The interface exposes these… -- docs: netconsole: document rate limit feature This adds a "Rate limiting" section to the netconsole documentation describing the per-target token bucket and the two configfs files that drive it, ratelimit_interval_ms and ratelimit_burst. It also notes that messages dropped by the bucket never get a message ID assigned.
quoted hunk ↗ jump to hunk
diff --git a/Documentation/networking/netconsole.rst b/Documentation/networking/netconsole.rst index 4ab5d7b05cf10..a9ccf79834656 100644 --- a/Documentation/networking/netconsole.rst +++ b/Documentation/networking/netconsole.rst@@ -177,6 +177,41 @@ You can modify these targets in runtime by creating the following targets:: cat cmdline1/remote_ip 10.0.0.3 +Rate limiting +------------- + +Netconsole hands every console message to every enabled target, so a host that +logs continuously can saturate the receiving agent. Each target carries a token +bucket that drops messages once the configured rate is exceeded, controlled by +two files in the target directory: + + ===================== ================================================ + ratelimit_interval_ms Length of the accounting interval, in + milliseconds. Zero, the default, sends + everything. + ratelimit_burst Messages allowed per interval. Defaults to + 10; zero drops every message once an + interval is set. + ===================== ================================================ + +Unlike most target parameters, both knobs can be written while the target is +enabled, which is when a flooding target most likely needs them.
[Severity: Low] Should the target parameter table earlier in this file be extended too? That table is introduced with "The interface exposes these parameters of a netconsole target to userspace" and it does list read-only siblings such as transmit_errors, but it gains no entry for ratelimit_interval_ms or ratelimit_burst, even though netconsole_target_attrs[] in drivers/net/netconsole.c registers them right next to attr_transmit_errors. The sentence directly under that table also seems to conflict with the new paragraph above: The "enabled" attribute is also used to control whether the parameters of a target can be updated or not -- you can modify the parameters of only disabled targets (i.e. if "enabled" is 0). Neither ratelimit_interval_ms_store() nor ratelimit_burst_store() has the nt->state == STATE_ENABLED test that, for example, remote_port_store() uses before returning -EINVAL with "target (%s) is enabled, disable to update parameters". Would it be worth qualifying that blanket rule so the document does not describe the same ABI in two contradictory ways?
quoted hunk ↗ jump to hunk
+ +The limit is applied per message, not per packet, so a message big enough to be +split into several `ncfrag` packets is either sent whole or not at all. + +Crash output bypasses the bucket. While an oops, BUG() or panic() is in +progress every message is sent, whatever the limit says, so a small burst +cannot cost you part of a crash dump. + +A drop leaves nothing on the wire. On an extended target it shows up as a gap +in the sequence number the header carries; a basic target has no such marker. + +Capping a target at 500 messages a minute:: + + echo 60000 > ratelimit_interval_ms + echo 500 > ratelimit_burst
[Severity: Medium] Does this two-step example behave as described on the flooding target the section is aimed at? ratelimit_interval_ms_store() and ratelimit_burst_store() in drivers/net/netconsole.c only publish the new values: dynamic_netconsole_mutex_lock(); WRITE_ONCE(nt->ratelimit.interval, jifs); dynamic_netconsole_mutex_unlock(); dynamic_netconsole_mutex_lock(); WRITE_ONCE(nt->ratelimit.burst, burst); dynamic_netconsole_mutex_unlock(); In lib/ratelimit.c, ___ratelimit() re-seeds the live allowance from rs->burst only on first use or once the interval has expired: if (!(rs->flags & RATELIMIT_INITIALIZED)) { rs->begin = jiffies; rs->flags |= RATELIMIT_INITIALIZED; atomic_set(&rs->rs_n_left, rs->burst); } if (time_is_before_jiffies(rs->begin + interval)) { ... atomic_set(&rs->rs_n_left, rs->burst); So after the first echo starts a 60 s interval with the default burst of 10, a flooding target drains those 10 tokens before the second shell command runs. Once burst=500 is stored, rs_n_left stays at 0 and every message is dropped for the rest of the 60 s interval, while cat ratelimit_burst reports 500. Lowering the burst mid-interval leaves the larger allowance spendable, and the next interval is still measured from the old rs->begin. Should the stores go through ratelimit_state_reset_interval() in include/linux/ratelimit.h, which takes rs->lock, clears RATELIMIT_INITIALIZED, does atomic_set(&rs->rs_n_left, rs->burst) and resets the missed counter? Otherwise, should this section say that a change only takes effect at the next interval boundary? There is also the plain load of rs->burst in the two atomic_set() calls in ___ratelimit(), which run under rs->lock. The configfs writers hold only dynamic_netconsole_mutex, which the console write path netconsole_write() -> netconsole_ratelimited() -> __ratelimit() never takes, so the locksets do not intersect. Is that concurrent plain read against WRITE_ONCE() something KCSAN would flag?
Append User Data ----------------
[ ... ] -- Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260910-netcons_ratelimit-v2-0-ebf0dd91e26e%40debian.org