* [PATCH] Bug 134: message rate limiting https://bugs.passt.top/show_bug.cgi?id=134
@ 2026-03-23 9:03 Anshu Kumari
2026-03-23 12:18 ` Laurent Vivier
2026-03-23 22:51 ` David Gibson
0 siblings, 2 replies; 4+ messages in thread
From: Anshu Kumari @ 2026-03-23 9:03 UTC (permalink / raw)
To: passt-dev; +Cc: sbrivio, dgibson, Anshu Kumari
Hi Everyone,
Please review patch for Bug 134.
Description:- Inorder to rate limit the messages, each logging function uses per-call-site static variables, so each macro expansion tracks its own rate independently. Allows up to LOG_RATELIMIT_BURST messages per window. When a new window starts after suppression, a summary of suppressed messages is logged.
Signed-off-by: Anshu Kumari <anskuma@redhat.com>
---
log.h | 41 +++++++++++++++++++++++++++++++++++++++++
tap.c | 19 ++++++-------------
2 files changed, 47 insertions(+), 13 deletions(-)
diff --git a/log.h b/log.h
index 6ceb686..e989760 100644
--- a/log.h
+++ b/log.h
@@ -48,6 +48,47 @@ void logmsg_perror(int pri, const char *format, ...)
passt_exit(EXIT_FAILURE); \
} while (0)
+#define LOG_RATELIMIT_BURST 5 /* Max messages per window per call site */
+#define LOG_RATELIMIT_INTERVAL 1 /* Default rate limit window in seconds */
+
+/**
+ * logmsg_ratelimit() - Rate-limited log message
+ * @fn: Logging function
+ * @now: current timestamp
+ * @intv: Minimum interval in seconds between allowed messages
+ */
+#define logmsg_ratelimit(fn, now, intv, ...) \
+ do { \
+ static time_t _rl_last; \
+ static unsigned int _rl_printed; \
+ static unsigned int _rl_suppressed; \
+ \
+ if ((now)->tv_sec - _rl_last > (intv)) { \
+ if (_rl_suppressed) \
+ fn("(suppressed %u similar messages)", \
+ _rl_suppressed); \
+ _rl_last = (now)->tv_sec; \
+ _rl_printed = 0; \
+ _rl_suppressed = 0; \
+ } \
+ \
+ if (_rl_printed < LOG_RATELIMIT_BURST) { \
+ fn(__VA_ARGS__); \
+ _rl_printed++; \
+ } else { \
+ _rl_suppressed++; \
+ } \
+ } while (0)
+
+#define err_ratelimit(now, intv, ...) \
+ logmsg_ratelimit(err, now, intv, __VA_ARGS__)
+#define warn_ratelimit(now, intv, ...) \
+ logmsg_ratelimit(warn, now, intv, __VA_ARGS__)
+#define info_ratelimit(now, intv, ...) \
+ logmsg_ratelimit(info, now, intv, __VA_ARGS__)
+#define debug_ratelimit(now, intv, ...) \
+ logmsg_ratelimit(debug, now, intv, __VA_ARGS__)
+
extern int log_file;
extern int log_trace;
extern bool log_conf_parsed;
diff --git a/tap.c b/tap.c
index 1049e02..0812a8a 100644
--- a/tap.c
+++ b/tap.c
@@ -686,17 +686,8 @@ static bool tap4_is_fragment(const struct iphdr *iph,
const struct timespec *now)
{
if (ntohs(iph->frag_off) & ~IP_DF) {
- /* Ratelimit messages */
- static time_t last_message;
- static unsigned num_dropped;
-
- num_dropped++;
- if (now->tv_sec - last_message > FRAGMENT_MSG_RATE) {
- warn("Can't process IPv4 fragments (%u dropped)",
- num_dropped);
- last_message = now->tv_sec;
- num_dropped = 0;
- }
+ warn_ratelimit(now, FRAGMENT_MSG_RATE,
+ "Can't process IPv4 fragment");
return true;
}
return false;
@@ -1115,8 +1106,10 @@ void tap_add_packet(struct ctx *c, struct iov_tail *data,
char bufmac[ETH_ADDRSTRLEN];
memcpy(c->guest_mac, eh->h_source, ETH_ALEN);
- debug("New guest MAC address observed: %s",
- eth_ntop(c->guest_mac, bufmac, sizeof(bufmac)));
+ info_ratelimit(now, LOG_RATELIMIT_INTERVAL,
+ "New guest MAC address observed: %s",
+ eth_ntop(c->guest_mac, bufmac,
+ sizeof(bufmac)));
proto_update_l2_buf(c->guest_mac);
}
--
2.53.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Bug 134: message rate limiting https://bugs.passt.top/show_bug.cgi?id=134
2026-03-23 9:03 [PATCH] Bug 134: message rate limiting https://bugs.passt.top/show_bug.cgi?id=134 Anshu Kumari
@ 2026-03-23 12:18 ` Laurent Vivier
2026-03-24 9:58 ` Anshu Kumari
2026-03-23 22:51 ` David Gibson
1 sibling, 1 reply; 4+ messages in thread
From: Laurent Vivier @ 2026-03-23 12:18 UTC (permalink / raw)
To: Anshu Kumari, passt-dev; +Cc: sbrivio, dgibson
On 3/23/26 10:03, Anshu Kumari wrote:
> Hi Everyone,
Hi Anshu,
> Please review patch for Bug 134.
>
> Description:- Inorder to rate limit the messages, each logging function uses per-call-site static variables, so each macro expansion tracks its own rate independently. Allows up to LOG_RATELIMIT_BURST messages per window. When a new window starts after suppression, a summary of suppressed messages is logged.
This should be a message describing the patch, not an email text.
This will be commited and part of the change log of passt.
If you want to introduce your patch with a message you can write a cover letter (you can
put " Hi Everyone, Please review patch for Bug 134" in it)
Put the bug link in the message body with Link: tag
Link: https://bugs.passt.top/show_bug.cgi?id=134
Something like:
"log: Add rate-limiting macros for log messages
In order to rate limit the messages, each logging function uses per-call-site static
variables, so each macro expansion tracks its own rate independently. Allows up to
LOG_RATELIMIT_BURST messages per window. When a new window starts after suppression, a
summary of suppressed messages is logged.
Link: https://bugs.passt.top/show_bug.cgi?id=134
Signed-off-by: Anshu Kumari <anskuma@redhat.com>
"
>
> Signed-off-by: Anshu Kumari <anskuma@redhat.com>
> ---
> log.h | 41 +++++++++++++++++++++++++++++++++++++++++
> tap.c | 19 ++++++-------------
> 2 files changed, 47 insertions(+), 13 deletions(-)
>
> diff --git a/log.h b/log.h
> index 6ceb686..e989760 100644
> --- a/log.h
> +++ b/log.h
> @@ -48,6 +48,47 @@ void logmsg_perror(int pri, const char *format, ...)
> passt_exit(EXIT_FAILURE); \
> } while (0)
>
> +#define LOG_RATELIMIT_BURST 5 /* Max messages per window per call site */
> +#define LOG_RATELIMIT_INTERVAL 1 /* Default rate limit window in seconds */
> +
> +/**
> + * logmsg_ratelimit() - Rate-limited log message
> + * @fn: Logging function
> + * @now: current timestamp
> + * @intv: Minimum interval in seconds between allowed messages
> + */
> +#define logmsg_ratelimit(fn, now, intv, ...) \
> + do { \
> + static time_t _rl_last; \
> + static unsigned int _rl_printed; \
> + static unsigned int _rl_suppressed; \
> + \
> + if ((now)->tv_sec - _rl_last > (intv)) { \
> + if (_rl_suppressed) \
> + fn("(suppressed %u similar messages)", \
> + _rl_suppressed); \
> + _rl_last = (now)->tv_sec; \
> + _rl_printed = 0; \
> + _rl_suppressed = 0; \
> + } \
> + \
> + if (_rl_printed < LOG_RATELIMIT_BURST) { \
> + fn(__VA_ARGS__); \
> + _rl_printed++; \
> + } else { \
> + _rl_suppressed++; \
> + } \
> + } while (0)
> +
> +#define err_ratelimit(now, intv, ...) \
> + logmsg_ratelimit(err, now, intv, __VA_ARGS__)
> +#define warn_ratelimit(now, intv, ...) \
> + logmsg_ratelimit(warn, now, intv, __VA_ARGS__)
> +#define info_ratelimit(now, intv, ...) \
> + logmsg_ratelimit(info, now, intv, __VA_ARGS__)
> +#define debug_ratelimit(now, intv, ...) \
> + logmsg_ratelimit(debug, now, intv, __VA_ARGS__)
> +
> extern int log_file;
> extern int log_trace;
> extern bool log_conf_parsed;
> diff --git a/tap.c b/tap.c
> index 1049e02..0812a8a 100644
> --- a/tap.c
> +++ b/tap.c
> @@ -686,17 +686,8 @@ static bool tap4_is_fragment(const struct iphdr *iph,
> const struct timespec *now)
> {
> if (ntohs(iph->frag_off) & ~IP_DF) {
> - /* Ratelimit messages */
> - static time_t last_message;
> - static unsigned num_dropped;
> -
> - num_dropped++;
> - if (now->tv_sec - last_message > FRAGMENT_MSG_RATE) {
I think you could remove FRAGMENT_MSG_RATE and use LOG_RATELIMIT_INTERVAL, or better
remove the intv parameter and use LOG_RATELIMIT_INTERVAL internally.
> - warn("Can't process IPv4 fragments (%u dropped)",
> - num_dropped);
> - last_message = now->tv_sec;
> - num_dropped = 0;
> - }
> + warn_ratelimit(now, FRAGMENT_MSG_RATE,
> + "Can't process IPv4 fragment");
> return true;
> }
> return false;
> @@ -1115,8 +1106,10 @@ void tap_add_packet(struct ctx *c, struct iov_tail *data,
> char bufmac[ETH_ADDRSTRLEN];
>
> memcpy(c->guest_mac, eh->h_source, ETH_ALEN);
> - debug("New guest MAC address observed: %s",
> - eth_ntop(c->guest_mac, bufmac, sizeof(bufmac)));
> + info_ratelimit(now, LOG_RATELIMIT_INTERVAL,
You changed "debug" to "info". Is this intentional?
> + "New guest MAC address observed: %s",
> + eth_ntop(c->guest_mac, bufmac,
> + sizeof(bufmac)));
> proto_update_l2_buf(c->guest_mac);
> }
>
Thanks,
Laurent
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Bug 134: message rate limiting https://bugs.passt.top/show_bug.cgi?id=134
2026-03-23 9:03 [PATCH] Bug 134: message rate limiting https://bugs.passt.top/show_bug.cgi?id=134 Anshu Kumari
2026-03-23 12:18 ` Laurent Vivier
@ 2026-03-23 22:51 ` David Gibson
1 sibling, 0 replies; 4+ messages in thread
From: David Gibson @ 2026-03-23 22:51 UTC (permalink / raw)
To: Anshu Kumari; +Cc: passt-dev, sbrivio, dgibson
[-- Attachment #1: Type: text/plain, Size: 4492 bytes --]
On Mon, Mar 23, 2026 at 02:33:07PM +0530, Anshu Kumari wrote:
> Hi Everyone,
>
> Please review patch for Bug 134.
>
> Description:- Inorder to rate limit the messages, each logging function uses per-call-site static variables, so each macro expansion tracks its own rate independently. Allows up to LOG_RATELIMIT_BURST messages per window. When a new window starts after suppression, a summary of suppressed messages is logged.
>
> Signed-off-by: Anshu Kumari <anskuma@redhat.com>
Logic looks good to me, and more straightforward than I feared. Nice!
Stefano had a bunch of boring procedural things to fix, and I have one
more, see below.
> ---
> log.h | 41 +++++++++++++++++++++++++++++++++++++++++
> tap.c | 19 ++++++-------------
> 2 files changed, 47 insertions(+), 13 deletions(-)
>
> diff --git a/log.h b/log.h
> index 6ceb686..e989760 100644
> --- a/log.h
> +++ b/log.h
> @@ -48,6 +48,47 @@ void logmsg_perror(int pri, const char *format, ...)
> passt_exit(EXIT_FAILURE); \
> } while (0)
>
> +#define LOG_RATELIMIT_BURST 5 /* Max messages per window per call site */
> +#define LOG_RATELIMIT_INTERVAL 1 /* Default rate limit window in seconds */
> +
> +/**
> + * logmsg_ratelimit() - Rate-limited log message
> + * @fn: Logging function
> + * @now: current timestamp
> + * @intv: Minimum interval in seconds between allowed messages
> + */
> +#define logmsg_ratelimit(fn, now, intv, ...) \
> + do { \
> + static time_t _rl_last; \
Identifiers starting with _ are reserved for "the system" (vague, I
know). The kernel can get away with this, because it controls every
part of the build. We're a normal userspace binary, though, linked
against the C library, so we should avoid this. Doing something to
avoiding shadowing regular variables in the macro is wise though. For
this, I usually use _ as a suffix instead of a prefix (see,
e.g. flow_log()).
> + static unsigned int _rl_printed; \
> + static unsigned int _rl_suppressed; \
> + \
> + if ((now)->tv_sec - _rl_last > (intv)) { \
> + if (_rl_suppressed) \
> + fn("(suppressed %u similar messages)", \
> + _rl_suppressed); \
> + _rl_last = (now)->tv_sec; \
> + _rl_printed = 0; \
> + _rl_suppressed = 0; \
> + } \
> + \
> + if (_rl_printed < LOG_RATELIMIT_BURST) { \
> + fn(__VA_ARGS__); \
> + _rl_printed++; \
> + } else { \
> + _rl_suppressed++; \
> + } \
> + } while (0)
> +
> +#define err_ratelimit(now, intv, ...) \
> + logmsg_ratelimit(err, now, intv, __VA_ARGS__)
> +#define warn_ratelimit(now, intv, ...) \
> + logmsg_ratelimit(warn, now, intv, __VA_ARGS__)
> +#define info_ratelimit(now, intv, ...) \
> + logmsg_ratelimit(info, now, intv, __VA_ARGS__)
> +#define debug_ratelimit(now, intv, ...) \
> + logmsg_ratelimit(debug, now, intv, __VA_ARGS__)
> +
> extern int log_file;
> extern int log_trace;
> extern bool log_conf_parsed;
> diff --git a/tap.c b/tap.c
> index 1049e02..0812a8a 100644
> --- a/tap.c
> +++ b/tap.c
> @@ -686,17 +686,8 @@ static bool tap4_is_fragment(const struct iphdr *iph,
> const struct timespec *now)
> {
> if (ntohs(iph->frag_off) & ~IP_DF) {
> - /* Ratelimit messages */
> - static time_t last_message;
> - static unsigned num_dropped;
> -
> - num_dropped++;
> - if (now->tv_sec - last_message > FRAGMENT_MSG_RATE) {
> - warn("Can't process IPv4 fragments (%u dropped)",
> - num_dropped);
> - last_message = now->tv_sec;
> - num_dropped = 0;
> - }
> + warn_ratelimit(now, FRAGMENT_MSG_RATE,
> + "Can't process IPv4 fragment");
> return true;
> }
> return false;
> @@ -1115,8 +1106,10 @@ void tap_add_packet(struct ctx *c, struct iov_tail *data,
> char bufmac[ETH_ADDRSTRLEN];
>
> memcpy(c->guest_mac, eh->h_source, ETH_ALEN);
> - debug("New guest MAC address observed: %s",
> - eth_ntop(c->guest_mac, bufmac, sizeof(bufmac)));
> + info_ratelimit(now, LOG_RATELIMIT_INTERVAL,
> + "New guest MAC address observed: %s",
> + eth_ntop(c->guest_mac, bufmac,
> + sizeof(bufmac)));
> proto_update_l2_buf(c->guest_mac);
> }
>
> --
> 2.53.0
>
--
David Gibson (he or they) | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you, not the other way
| around.
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Bug 134: message rate limiting https://bugs.passt.top/show_bug.cgi?id=134
2026-03-23 12:18 ` Laurent Vivier
@ 2026-03-24 9:58 ` Anshu Kumari
0 siblings, 0 replies; 4+ messages in thread
From: Anshu Kumari @ 2026-03-24 9:58 UTC (permalink / raw)
To: Laurent Vivier; +Cc: passt-dev, sbrivio, dgibson
[-- Attachment #1: Type: text/plain, Size: 7038 bytes --]
> + info_ratelimit(now, LOG_RATELIMIT_INTERVAL,
You changed "debug" to "info". Is this intentional?
> + "New guest MAC address observed: %s",
> + eth_ntop(c->guest_mac, bufmac,
> + sizeof(bufmac)));
> proto_update_l2_buf(c->guest_mac);
> }
>
Yes, this log-level is changed to info based on the discusion attached with
the ticket
https://archives.passt.top/passt-dev/20250912082344.01888466@elisabeth/
On Mon, Mar 23, 2026 at 5:48 PM Laurent Vivier <lvivier@redhat.com> wrote:
> On 3/23/26 10:03, Anshu Kumari wrote:
> > Hi Everyone,
>
> Hi Anshu,
>
> > Please review patch for Bug 134.
> >
> > Description:- Inorder to rate limit the messages, each logging function
> uses per-call-site static variables, so each macro expansion tracks its own
> rate independently. Allows up to LOG_RATELIMIT_BURST messages per window.
> When a new window starts after suppression, a summary of suppressed
> messages is logged.
>
> This should be a message describing the patch, not an email text.
> This will be commited and part of the change log of passt.
>
> If you want to introduce your patch with a message you can write a cover
> letter (you can
> put " Hi Everyone, Please review patch for Bug 134" in it)
>
> Put the bug link in the message body with Link: tag
>
> Link: https://bugs.passt.top/show_bug.cgi?id=134
>
> Something like:
>
> "log: Add rate-limiting macros for log messages
>
> In order to rate limit the messages, each logging function uses
> per-call-site static
> variables, so each macro expansion tracks its own rate independently.
> Allows up to
> LOG_RATELIMIT_BURST messages per window. When a new window starts after
> suppression, a
> summary of suppressed messages is logged.
>
> Link: https://bugs.passt.top/show_bug.cgi?id=134
> Signed-off-by: Anshu Kumari <anskuma@redhat.com>
> "
>
> >
> > Signed-off-by: Anshu Kumari <anskuma@redhat.com>
> > ---
> > log.h | 41 +++++++++++++++++++++++++++++++++++++++++
> > tap.c | 19 ++++++-------------
> > 2 files changed, 47 insertions(+), 13 deletions(-)
> >
> > diff --git a/log.h b/log.h
> > index 6ceb686..e989760 100644
> > --- a/log.h
> > +++ b/log.h
> > @@ -48,6 +48,47 @@ void logmsg_perror(int pri, const char *format, ...)
> > passt_exit(EXIT_FAILURE); \
> > } while (0)
> >
> > +#define LOG_RATELIMIT_BURST 5 /* Max messages per window per call
> site */
> > +#define LOG_RATELIMIT_INTERVAL 1 /* Default rate limit window in
> seconds */
> > +
> > +/**
> > + * logmsg_ratelimit() - Rate-limited log message
> > + * @fn: Logging function
> > + * @now: current timestamp
> > + * @intv: Minimum interval in seconds between allowed messages
> > + */
> > +#define logmsg_ratelimit(fn, now, intv, ...) \
> > + do { \
> > + static time_t _rl_last; \
> > + static unsigned int _rl_printed; \
> > + static unsigned int _rl_suppressed; \
> > + \
> > + if ((now)->tv_sec - _rl_last > (intv)) { \
> > + if (_rl_suppressed) \
> > + fn("(suppressed %u similar messages)", \
> > + _rl_suppressed); \
> > + _rl_last = (now)->tv_sec; \
> > + _rl_printed = 0; \
> > + _rl_suppressed = 0; \
> > + } \
> > + \
> > + if (_rl_printed < LOG_RATELIMIT_BURST) { \
> > + fn(__VA_ARGS__); \
> > + _rl_printed++; \
> > + } else { \
> > + _rl_suppressed++; \
> > + } \
> > + } while (0)
> > +
> > +#define err_ratelimit(now, intv, ...)
> \
> > + logmsg_ratelimit(err, now, intv, __VA_ARGS__)
> > +#define warn_ratelimit(now, intv, ...)
> \
> > + logmsg_ratelimit(warn, now, intv, __VA_ARGS__)
> > +#define info_ratelimit(now, intv, ...)
> \
> > + logmsg_ratelimit(info, now, intv, __VA_ARGS__)
> > +#define debug_ratelimit(now, intv, ...)
> \
> > + logmsg_ratelimit(debug, now, intv, __VA_ARGS__)
> > +
> > extern int log_file;
> > extern int log_trace;
> > extern bool log_conf_parsed;
> > diff --git a/tap.c b/tap.c
> > index 1049e02..0812a8a 100644
> > --- a/tap.c
> > +++ b/tap.c
> > @@ -686,17 +686,8 @@ static bool tap4_is_fragment(const struct iphdr
> *iph,
> > const struct timespec *now)
> > {
> > if (ntohs(iph->frag_off) & ~IP_DF) {
> > - /* Ratelimit messages */
> > - static time_t last_message;
> > - static unsigned num_dropped;
> > -
> > - num_dropped++;
> > - if (now->tv_sec - last_message > FRAGMENT_MSG_RATE) {
>
> I think you could remove FRAGMENT_MSG_RATE and use LOG_RATELIMIT_INTERVAL,
> or better
> remove the intv parameter and use LOG_RATELIMIT_INTERVAL internally.
>
> > - warn("Can't process IPv4 fragments (%u dropped)",
> > - num_dropped);
> > - last_message = now->tv_sec;
> > - num_dropped = 0;
> > - }
> > + warn_ratelimit(now, FRAGMENT_MSG_RATE,
> > + "Can't process IPv4 fragment");
> > return true;
> > }
> > return false;
> > @@ -1115,8 +1106,10 @@ void tap_add_packet(struct ctx *c, struct
> iov_tail *data,
> > char bufmac[ETH_ADDRSTRLEN];
> >
> > memcpy(c->guest_mac, eh->h_source, ETH_ALEN);
> > - debug("New guest MAC address observed: %s",
> > - eth_ntop(c->guest_mac, bufmac, sizeof(bufmac)));
> > + info_ratelimit(now, LOG_RATELIMIT_INTERVAL,
>
> You changed "debug" to "info". Is this intentional?
>
> > + "New guest MAC address observed: %s",
> > + eth_ntop(c->guest_mac, bufmac,
> > + sizeof(bufmac)));
> > proto_update_l2_buf(c->guest_mac);
> > }
> >
>
> Thanks,
> Laurent
>
>
[-- Attachment #2: Type: text/html, Size: 9705 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-03-24 9:58 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-03-23 9:03 [PATCH] Bug 134: message rate limiting https://bugs.passt.top/show_bug.cgi?id=134 Anshu Kumari
2026-03-23 12:18 ` Laurent Vivier
2026-03-24 9:58 ` Anshu Kumari
2026-03-23 22:51 ` David Gibson
Code repositories for project(s) associated with this public inbox
https://passt.top/passt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).