From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=none (p=none dis=none) header.from=gibson.dropbear.id.au Authentication-Results: passt.top; dkim=pass (2048-bit key; secure) header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.a=rsa-sha256 header.s=202602 header.b=Xzg2/8w1; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 803085A0652 for ; Mon, 23 Mar 2026 23:51:36 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202602; t=1774306292; bh=q+jdiN+7lH4Tw2ACa07qzzw9x8+xFjKAIBOpoCwPy/s=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=Xzg2/8w10/fKJckC5RMSJhr9VbGqqXW4F4ej5sDwt5GzPylyLV3J3rFkRWzH0fFr9 oINIp0CzoB79kfEgoqHG5Ukclgl3Mev5/pDI2VBA72UrZ3AQJibPixlt3rjShkRUJo GiPaDcM2Az9DuINjEJXJyRVyXrS7kExNb0osg0yjMHboWT51+DxOoG+OWfv0ujv5D0 R9maXoX50aLPOwmmJZlbR5c33+wjZEpTXbUMdqSmF7T/ykvCk7ZKX5XOEMsQ6vn0+1 ZTzBdrqxbPSv0efhl+bozFXvJIdmAag+xq2qxm1AZbfwZdK1gS6GDJp1G2rk9bERYk szd1wQK6uAsww== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4ffpLr6tkqz4wDN; Tue, 24 Mar 2026 09:51:32 +1100 (AEDT) Date: Tue, 24 Mar 2026 09:51:18 +1100 From: David Gibson To: Anshu Kumari Subject: Re: [PATCH] Bug 134: message rate limiting https://bugs.passt.top/show_bug.cgi?id=134 Message-ID: References: <20260323090305.319573-3-anskuma@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="A5AlsD+PAztMgdPJ" Content-Disposition: inline In-Reply-To: <20260323090305.319573-3-anskuma@redhat.com> Message-ID-Hash: 6TESQ4UJSXOHZHD2ESPOHZY22T5ETI4Y X-Message-ID-Hash: 6TESQ4UJSXOHZHD2ESPOHZY22T5ETI4Y X-MailFrom: dgibson@gandalf.ozlabs.org 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, sbrivio@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: --A5AlsD+PAztMgdPJ Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Mar 23, 2026 at 02:33:07PM +0530, Anshu Kumari wrote: > Hi Everyone, >=20 > Please review patch for Bug 134. >=20 > Description:- Inorder to rate limit the messages, each logging function u= ses 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 messag= es is logged. >=20 > Signed-off-by: Anshu Kumari 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(-) >=20 > 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) > =20 > +#define LOG_RATELIMIT_BURST 5 /* Max messages per window per call site = */ > +#define LOG_RATELIMIT_INTERVAL 1 /* Default rate limit window in second= s */ > + > +/** > + * 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 =3D (now)->tv_sec; \ > + _rl_printed =3D 0; \ > + _rl_suppressed =3D 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 =3D now->tv_sec; > - num_dropped =3D 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]; > =20 > 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); > } > =20 > --=20 > 2.53.0 >=20 --=20 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 --A5AlsD+PAztMgdPJ Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmnBw+UACgkQzQJF27ox 2GfLChAAkBerYI8qbfppFnHM0s99obziJD3tELlQsSPxQEHtpAU+kV4BYsEQWg2B C4hVkr+Ui/SlY3wXtXnYRdIU+LmSZd6OM6o3NLVpFZ9n0dKlaUcdw3wsXOVJ25eg /BF1WMf1cV38jWElYapQ67FiqKjSiEQfLe5VabTfW1esPYJEyjxXEHOqR+BZZUxN 7JNJTlPO5f/ieOz2U26DJqy+pDOsqAAUiTFmtu32K+kLu/DJPzhlPCi/YwMmD5v2 t4psaaBRLprJEfOLL+wmM0p7idvJ8QQrGy9mYHqRCwFeMNNy3KpiXN8eyp6PtkWk 5oqJQOqq1FsMu9KdRG/yAiNrUOhhuBETJ6eoGOZZaYcmhEUZ2eeF/ITsxhN3q568 k6VHJReab11IB/p9FCnhem6GfonTbLrcfucrH/xp+nc8fKNyVYgX7iWcdmWv4UXD UYN4L92B7ZB3gRJE7EDKZlxarRAwJ2hymOrLpeE2BJRLVfc7ifQodjpPaw2TRCo1 Tlh4jJ7Bo3D+H9WdAUs+nGtwqNfA14M1chJ9maEeYGw9qRgI6yiZrSXtMILp/DFU ikud4cl5j+sD0B9i5f90ZCQvMBgqobLHsjsBcjumzFWUKtAbO2CvgckoCwRtlmH3 vYuaDZL7YbuNeDneSwCbpqu1v+tUOXZcxcYJ29D0GvVDw4v1P2E= =flAO -----END PGP SIGNATURE----- --A5AlsD+PAztMgdPJ--