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