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=JYChv7rT; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 3EAF95A0265 for ; Mon, 13 Apr 2026 08:26:03 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202602; t=1776061559; bh=8xrnnDdlwaX/h+rv9BmhOoV5XQ6d8nGdsSHorhs1Ob0=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=JYChv7rT7QGE5fO82ZkxjvRZrTJvSJQNPoEMu9ZIj5M8i1jXVu5r3nfHF++SbK9Ga GFeh8fRJYJUuwOI5YNVMJJCJmN93WVXLfz6WZ72i4GI7IwmK2CaFHq/LbkHIYj54a4 4gSeEQ0Tgd35qJFpdCDIOMcXfo5rYfCHUW8pKJe43gUAKA+GeQSfcs4JYzopZTljCy o3Cx2coK1kMvfcTfrLoJOCb3VbIrmZndzgqTIhbcJDOBj+KlJ9wB9e3a2amhkMVpTY fXKDKUQKUXb6NDTiIpYoHznE/FhxfZzIfQj+EQ1IwX/kwS4xQUjtPGfEUZ+9uLHQUl TTpEufRMWrFKg== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4fvHTz6s5cz4wLG; Mon, 13 Apr 2026 16:25:59 +1000 (AEST) Date: Mon, 13 Apr 2026 16:25:52 +1000 From: David Gibson To: Anshu Kumari Subject: Re: [PATCH v3] tap, tcp, udp: Use rate-limited logging Message-ID: References: <20260410103737.1642986-1-anskuma@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="ZsE8dMtsGjOSD1ER" Content-Disposition: inline In-Reply-To: <20260410103737.1642986-1-anskuma@redhat.com> Message-ID-Hash: C4UX47JLRZYSTEPEU7PRP7VPN436FCFY X-Message-ID-Hash: C4UX47JLRZYSTEPEU7PRP7VPN436FCFY 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: sbrivio@redhat.com, passt-dev@passt.top, v@njh.eu, lvivier@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: --ZsE8dMtsGjOSD1ER Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Apr 10, 2026 at 04:07:37PM +0530, Anshu Kumari wrote: > Now that rate-limited logging macros are available, promote several > debug messages to higher severity levels. These messages were > previously kept at debug to prevent guests from flooding host > logs, but with rate limiting they can safely be made visible in > normal operation. >=20 > In tap.c, refactor tap4_is_fragment() to use warn_ratelimit() instead > of its ad-hoc rate limiting, and promote the guest MAC address change > message to info level. >=20 > In tcp.c, promote the invalid TCP SYN endpoint message to warn level. >=20 > In udp.c, promote dropped datagram messages to warn level, and > rate-limit the unrecoverable socket error message. >=20 > In udp_flow.c, promote flow allocation failures to err_ratelimit. >=20 > Link: https://bugs.passt.top/show_bug.cgi?id=3D134 > Signed-off-by: Anshu Kumari Reviewed-by: David Gibson > --- > v3: > - Promote flow allocation failures to err_ratelimit from > warn_ratelimit. Nit: doesn't affect the validity of the patch itself, but I'd consider "promotion" in this context as going from a less severe to more severe error level. So going from 'err' to 'warn' is a demotion, not a promotion. > - Fix remaining alignment issues. > - Cc Volker >=20 > --- > tap.c | 16 +++++----------- > tcp.c | 6 +++--- > udp.c | 12 ++++++------ > udp_flow.c | 13 +++++++------ > 4 files changed, 21 insertions(+), 26 deletions(-) >=20 > diff --git a/tap.c b/tap.c > index 1049e02..7d06189 100644 > --- a/tap.c > +++ b/tap.c > @@ -99,7 +99,6 @@ static PACKET_POOL_NOINIT(pool_tap4, TAP_MSGS_IP4); > static PACKET_POOL_NOINIT(pool_tap6, TAP_MSGS_IP6); > =20 > #define TAP_SEQS 128 /* Different L4 tuples in one batch */ > -#define FRAGMENT_MSG_RATE 10 /* # seconds between fragment warnings */ > =20 > /** > * tap_l2_max_len() - Maximum frame size (including L2 header) for curre= nt mode > @@ -686,17 +685,11 @@ static bool tap4_is_fragment(const struct iphdr *ip= h, > const struct timespec *now) > { > if (ntohs(iph->frag_off) & ~IP_DF) { > - /* Ratelimit messages */ > - static time_t last_message; > static unsigned num_dropped; > =20 > 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, "Can't process IPv4 fragments (%u dropped)", > + num_dropped); > return true; > } > return false; > @@ -1115,8 +1108,9 @@ 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, "New guest MAC address observed: %s", > + eth_ntop(c->guest_mac, bufmac, > + sizeof(bufmac))); > proto_update_l2_buf(c->guest_mac); > } > =20 > diff --git a/tcp.c b/tcp.c > index 8ea9be8..071bcae 100644 > --- a/tcp.c > +++ b/tcp.c > @@ -1688,9 +1688,9 @@ static void tcp_conn_from_tap(const struct ctx *c, = sa_family_t af, > !inany_is_unicast(&ini->oaddr) || ini->oport =3D=3D 0) { > char sstr[INANY_ADDRSTRLEN], dstr[INANY_ADDRSTRLEN]; > =20 > - debug("Invalid endpoint in TCP SYN: %s:%hu -> %s:%hu", > - inany_ntop(&ini->eaddr, sstr, sizeof(sstr)), ini->eport, > - inany_ntop(&ini->oaddr, dstr, sizeof(dstr)), ini->oport); > + warn_ratelimit(now, "Invalid endpoint in TCP SYN: %s:%hu -> %s:%hu", > + inany_ntop(&ini->eaddr, sstr, sizeof(sstr)), ini->eport, > + inany_ntop(&ini->oaddr, dstr, sizeof(dstr)), ini->oport); > goto cancel; > } > =20 > diff --git a/udp.c b/udp.c > index 1fc5a42..52260b9 100644 > --- a/udp.c > +++ b/udp.c > @@ -871,7 +871,7 @@ void udp_sock_fwd(const struct ctx *c, int s, int rul= e_hint, > /* Clear errors & carry on */ > if (udp_sock_errs(c, s, FLOW_SIDX_NONE, > frompif, port) < 0) { > - err( > + err_ratelimit(now, > "UDP: Unrecoverable error on listening socket: (%s port %hu)", > pif_name(frompif), port); > /* FIXME: what now? close/re-open socket? */ > @@ -898,7 +898,7 @@ void udp_sock_fwd(const struct ctx *c, int s, int rul= e_hint, > pif_name(frompif), pif_name(topif)); > discard =3D true; > } else { > - debug("Discarding datagram without flow"); > + warn_ratelimit(now, "Discarding datagram without flow"); > discard =3D true; > } > =20 > @@ -1042,10 +1042,10 @@ int udp_tap_handler(const struct ctx *c, uint8_t = pif, > if (!(uflow =3D udp_at_sidx(tosidx))) { > char sstr[INET6_ADDRSTRLEN], dstr[INET6_ADDRSTRLEN]; > =20 > - debug("Dropping datagram with no flow %s %s:%hu -> %s:%hu", > - pif_name(pif), > - inet_ntop(af, saddr, sstr, sizeof(sstr)), src, > - inet_ntop(af, daddr, dstr, sizeof(dstr)), dst); > + warn_ratelimit(now, "Dropping datagram with no flow %s %s:%hu -> %s:%h= u", > + pif_name(pif), > + inet_ntop(af, saddr, sstr, sizeof(sstr)), src, > + inet_ntop(af, daddr, dstr, sizeof(dstr)), dst); > return 1; > } > =20 > diff --git a/udp_flow.c b/udp_flow.c > index 7e2453e..35417bc 100644 > --- a/udp_flow.c > +++ b/udp_flow.c > @@ -235,8 +235,9 @@ flow_sidx_t udp_flow_from_sock(const struct ctx *c, u= int8_t pif, > if (!(flow =3D flow_alloc())) { > char sastr[SOCKADDR_STRLEN]; > =20 > - debug("Couldn't allocate flow for UDP datagram from %s %s", > - pif_name(pif), sockaddr_ntop(s_in, sastr, sizeof(sastr))); > + err_ratelimit(now, "Couldn't allocate flow for UDP datagram from %s %s= ", > + pif_name(pif), > + sockaddr_ntop(s_in, sastr, sizeof(sastr))); > return FLOW_SIDX_NONE; > } > =20 > @@ -292,10 +293,10 @@ flow_sidx_t udp_flow_from_tap(const struct ctx *c, > if (!(flow =3D flow_alloc())) { > char sstr[INET6_ADDRSTRLEN], dstr[INET6_ADDRSTRLEN]; > =20 > - debug("Couldn't allocate flow for UDP datagram from %s %s:%hu -> %s:%h= u", > - pif_name(pif), > - inet_ntop(af, saddr, sstr, sizeof(sstr)), srcport, > - inet_ntop(af, daddr, dstr, sizeof(dstr)), dstport); > + err_ratelimit(now, "Couldn't allocate flow for UDP datagram from %s %s= :%hu -> %s:%hu", > + pif_name(pif), > + inet_ntop(af, saddr, sstr, sizeof(sstr)), srcport, > + inet_ntop(af, daddr, dstr, sizeof(dstr)), dstport); > return FLOW_SIDX_NONE; > } > =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 --ZsE8dMtsGjOSD1ER Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmncjG8ACgkQzQJF27ox 2Gd9pA//UEz2oEBIdWdb2TdRifhPajSIzMRFF78xaoOtX4fJ5qPvbV9BOFWfHBtJ 1jrO0CTx3hbjGyyr/UsMmh/IKiMY3nIIQS44cAeV4+7Z06mk+sWLqv0268haZNGW PI5+jWgF5eEZ+RwBjq8Pa7IDmhKkLIA+gnvfM2RcS2LuJWUA6xu+8CX0wANOsLHk UxQfcRPIyCr3jiLFTn3/rVhHAKHA4mLje6hsIcqpri6ZgfnzTRi6C1x6QWpBxSpQ nkKh6pkLOCUnARPSR5BYI2h9UTzMg+Nvy2Dbhbfa+a2UQ1tEXYG/0mIKeBrRy00c Axn7K83VTow440fMTXq0w7Y4lreskAlO+WMMMPVFmys58u7byeEyYhyL9PnsCCb1 922RsI0cZ90KEFrfmznexqj/mXO5nMJTyIbpoMzxo0g3fuVsq3gIPQ/B2eLTtXp2 Z/0dtX/mYf62Os3A2g9ccxr2N+ZSdu/m1qhaU1ldzc0A6uj872z7XN2+7B0XYn8g QhT7DAkyxURvFmOcXOQ9SRDn2JR8vjjdaUd3c98nU4tEPuXHsKHLKu13eU9v/Dlp /uSxjdsROxDjgiILd37HoT/VOzFmMomw7/bUd2u4tBqeHG24rsQIjN75LWwtlg4B Er+d935+GMg+HeOqQ5w940jX2E1kTiA19soHn2eWquue8MtwsJc= =9yhz -----END PGP SIGNATURE----- --ZsE8dMtsGjOSD1ER--