From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: passt.top; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=epdQcH7l; dkim-atps=neutral Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by passt.top (Postfix) with ESMTPS id BA8855A0271 for ; Thu, 26 Mar 2026 01:16:08 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1774484167; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=NUMJSZLJ8A2x2RTx7vVyWVoYEKnWG45vUvj2EtFwXq0=; b=epdQcH7locnBlJLJ83q/gDrUpqFmWatP1RipfLssZUS1YntR91kqg3IjM3/Eu1eqGiUODl w9P4ov3JtdgRSsayjMQ0j3dlAHyfvyGgE4FBMG2rhmopWZmGugTk7JPDOqW5Ow4yI5I/M/ b7Tpj613tMINOu4NqjeT7Ic0d5dv+cc= Received: from mail-wm1-f70.google.com (mail-wm1-f70.google.com [209.85.128.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-93-UQWZIeZGOW-xXfYFu6srjg-1; Wed, 25 Mar 2026 20:16:06 -0400 X-MC-Unique: UQWZIeZGOW-xXfYFu6srjg-1 X-Mimecast-MFC-AGG-ID: UQWZIeZGOW-xXfYFu6srjg_1774484165 Received: by mail-wm1-f70.google.com with SMTP id 5b1f17b1804b1-486fc42c83aso2789235e9.0 for ; Wed, 25 Mar 2026 17:16:05 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1774484164; x=1775088964; h=date:content-transfer-encoding:mime-version:organization:references :in-reply-to:message-id:subject:cc:to:from:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=NUMJSZLJ8A2x2RTx7vVyWVoYEKnWG45vUvj2EtFwXq0=; b=JVk8lA+Rrzm7zhxCGLB7uMaD+Ri3tHLTkKinBBzEVI75SUTeOrXB7114TJqFdKfsYT H9R6V8QOO8BYnTIND870RjqfJ42g59qxMuNCG7QYGR0+Sa9F79Z9cH1826zfZfygUdEJ vULzTRB3zpsv4tcpz7p5OK/56VYOVHgpU8RLZs+I8IOeoOsXxAia16CC4nUwVhJdcYI3 TXWrBHDk+0e1gdWhqWOTYZ0W7psI90mNpO8zSBHnrSB65pwlbarRqOy0HuAVf4f1J+Rv mgP2FBdKTSzOeyC1zqkPTwUBttCr2Q5wAp8BYLOSdqR79vQ8GA4Und1o4F5A7sRBn1PT o+pg== X-Gm-Message-State: AOJu0YwCYwA9K0IFyfx4hSJIdEnT2zBpg5YX7wWKRUtI6X08TmI9GKpF QarPePT5WJoTQQEESWFE0WiSQ5fO4F/BiuPcCa1hyx5Ny0znIpKUYzC7Ui8hqJlnOus8RuTEygV AsDq3MzXr6Q9yk16+4FPnTNRacdMpPpCUFpZ3x3340qP2LmQCx6pqIZoRLCHHWBWN8g7Qym8vHA HMT2OHtzl8sqZIepo3Xw1XHxnuK1BcpkcP79/6 X-Gm-Gg: ATEYQzyAAqmIhJoV6wBDqSekoWZYytUg1NPrUuD1zWhbdS75H7sjm1MFC0y2Omy6isN UhdxJT0wlzirFttB/usbpOO3JYPWcp/y4Lp+h8yF9TbQajkneoppq36WBZjM5lSvOh8JVqe3wj5 VUZNXJyWsPKQ12dpd1ZMvshT+JkaeXAoJn8dotb/NHsJ6MZE+P/Ery39LVIOKw7+KpFJUsTyZrA 5LTeO0R1ACPc6zJyJtThdnWhVpNY8tighDXdeCyd/tSL6c8DvItudL5MdSwfRydpiBJ1+sytG9A PcCj2ROZ+GVEjeplxjNVYV5j7jtFk/uIpTXlOxSCekkjcepAiLXPVXS14ugFIc/CDuTdBXO1gFF lHEJfWTIV97M/IiiQt2vDOm+C9uutortiD+Jz5Gi7XiBnSxElsQ== X-Received: by 2002:a05:600d:8453:b0:485:4eaf:eb53 with SMTP id 5b1f17b1804b1-48716051e4emr57225315e9.19.1774484163992; Wed, 25 Mar 2026 17:16:03 -0700 (PDT) X-Received: by 2002:a05:600d:8453:b0:485:4eaf:eb53 with SMTP id 5b1f17b1804b1-48716051e4emr57224985e9.19.1774484163303; Wed, 25 Mar 2026 17:16:03 -0700 (PDT) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [176.103.220.4]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-4872092cf34sm4015005e9.6.2026.03.25.17.16.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 25 Mar 2026 17:16:02 -0700 (PDT) From: Stefano Brivio To: Anshu Kumari Subject: Re: [PATCH v3] log: Add rate-limiting macros for log messages Message-ID: <20260326011601.38b9b1f4@elisabeth> In-Reply-To: <20260325131120.299924-2-anskuma@redhat.com> References: <20260325131120.299924-2-anskuma@redhat.com> Organization: Red Hat X-Mailer: Claws Mail 4.2.0 (GTK 3.24.49; x86_64-pc-linux-gnu) MIME-Version: 1.0 Date: Thu, 26 Mar 2026 01:16:01 +0100 (CET) X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: 5OMSUQMSRjM07yK8bFMlSMOTDv2AebYz-aK8i2DDbuM_1774484165 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: J6JCPLTRUTMVLNVRAO6YM2W2KZBPQ4EP X-Message-ID-Hash: J6JCPLTRUTMVLNVRAO6YM2W2KZBPQ4EP X-MailFrom: sbrivio@redhat.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top, lvivier@redhat.com, dgibson@redhat.com X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Thanks, this looks almost correct to me. Still two comments from my side: On Wed, 25 Mar 2026 18:41:21 +0530 Anshu Kumari wrote: > Currently, some log messages that would be useful at info or warn level > are kept at debug level because there is no way to throttle them, and a > guest could otherwise flood the host logs. > > Add a logmsg_ratelimit() macro that uses per-call-site static variables > to independently track each call site's rate. It allows up to > LOG_RATELIMIT_BURST (5) messages per LOG_RATELIMIT_INTERVAL (1 second) > window, then prints a suppression notice. When a new window opens and > messages were suppressed, the count is reported after the next allowed > message. > ...there should be the Link: tag here, before Signed-off-by:, that Laurent and myself suggested. > Signed-off-by: Anshu Kumari > --- > v3: > - Print suppressed count after the message, not before > - Add suppression notice when burst limit is hit > - Reverse Christmas tree variable ordering > - Fix tab/space alignment in macro > - swapped LOG_RATELIMIT_INTERVAL and LOG_RATELIMIT_BURST location > > v2: > - Use _ suffix for macro variables instead of prefix > - Remove intv parameter from convenience wrappers > --- > log.h | 45 +++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 45 insertions(+) > > diff --git a/log.h b/log.h > index 6ceb686..8e6d65e 100644 > --- a/log.h > +++ b/log.h > @@ -48,6 +48,51 @@ void logmsg_perror(int pri, const char *format, ...) > passt_exit(EXIT_FAILURE); \ > } while (0) > > +#define LOG_RATELIMIT_INTERVAL 1 /* Default rate limit window in seconds */ > +#define LOG_RATELIMIT_BURST 5 /* Max messages per window per call site */ > + > +/** > + * logmsg_ratelimit() - Log a message with rate limiting > + * @fn: Logging function name (e.g. warn, info, debug) > + * @now: Current timestamp > + */ > +#define logmsg_ratelimit(fn, now, ...) \ > + do { \ > + static unsigned int rl_suppressed_; \ > + static unsigned int rl_printed_; \ > + static time_t rl_last_; \ > + \ > + if ((now)->tv_sec - rl_last_ > LOG_RATELIMIT_INTERVAL) {\ > + rl_last_ = (now)->tv_sec; \ > + rl_printed_ = 0; \ > + } \ > + \ > + if (rl_printed_ < LOG_RATELIMIT_BURST) { \ > + fn(__VA_ARGS__); \ > + if (rl_suppressed_) { \ > + fn("(suppressed %u similar messages)", \ > + rl_suppressed_); \ > + rl_suppressed_ = 0; \ > + } \ > + rl_printed_++; \ > + } else if (rl_printed_ == LOG_RATELIMIT_BURST) { \ > + fn("(suppressing further similar messages)"); \ There's one remaining problem with this implementation: this message is printed only the first time the original logging message is *not* printed. There should be an overlap of the two cases instead, so that this message is printed *just after* the last logged message. Otherwise you could have the following situation: message A (#1) message A (#2) ... message A (#5) message B (#1) (suppressing further similar messages) ...as message A was being printed for the sixth time. But that seems to refer to message B that was printed in between. You should make sure it's either: message A (#1) message A (#2) ... message A (#5) (suppressing further similar messages) message B (#1) or: message A (#1) message A (#2) ... message A (#5) message B (#1) message A (#6) (suppressing further similar messages) I think the first version is slightly preferable as the limit is 5, as defined, and not 6. > + rl_printed_++; \ > + rl_suppressed_++; \ > + } else { \ > + rl_suppressed_++; \ > + } \ > + } while (0) > + > +#define err_ratelimit(now, ...) \ > + logmsg_ratelimit(err, now, __VA_ARGS__) > +#define warn_ratelimit(now, ...) \ > + logmsg_ratelimit(warn, now, __VA_ARGS__) > +#define info_ratelimit(now, ...) \ > + logmsg_ratelimit(info, now, __VA_ARGS__) > +#define debug_ratelimit(now, ...) \ > + logmsg_ratelimit(debug, now, __VA_ARGS__) > + > extern int log_file; > extern int log_trace; > extern bool log_conf_parsed; The rest looks good to me. -- Stefano