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=KJ2JZLbD; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id A93255A0265 for ; Tue, 07 Apr 2026 05:31:28 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202602; t=1775532685; bh=p2u0uX8jgzy0STcBwH86OTqFNf/dSbZykc8idnfJQTM=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=KJ2JZLbD2K/sN7/TudWNEP6UNT+1awr9rZv1MOIW5pWFFzKQRizLmKFZFsIrV0ILG aNR0tMRfQ8ldNoMNUcLWwN8FSfWgqaSlWLTWMNRnrZjpLTl+SaVkMj0CVl5n+LBqn7 6KV0clUqUs36NOwl2HruniEixf1NJ+SrvUSWnIFp7h5rKWrBAoRcpGzf3yluIPH08J NqnLmlQvbsL2yWdu4xW9zbYbb5mZtXKrvFDtMeH8ak9fYauHxKQ4PTpK4gb7jTENR3 r0cBh1HNizmnX1B2v2ZhdQG808g6nkAXo6GN8tMVCC1vyfUZy14ACLdd5+gJT9l2Ra 7LO4H1++wfUfg== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4fqWvK2YZzz4wKR; Tue, 07 Apr 2026 13:31:25 +1000 (AEST) Date: Tue, 7 Apr 2026 13:28:19 +1000 From: David Gibson To: Anshu Kumari Subject: Re: [PATCH v2] tap, tcp, udp: Use rate-limited logging Message-ID: References: <20260406080557.858577-1-anskuma@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="scQjeS2iXelmsCRM" Content-Disposition: inline In-Reply-To: <20260406080557.858577-1-anskuma@redhat.com> Message-ID-Hash: O3VIQDJVOS3JEQQ3PTJGKRJI43XBFNYK X-Message-ID-Hash: O3VIQDJVOS3JEQQ3PTJGKRJI43XBFNYK 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, 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: --scQjeS2iXelmsCRM Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Apr 06, 2026 at 01:35:56PM +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 and udp_flow.c, promote flow allocation failures and dropped > datagram messages to warn level, and rate-limit the unrecoverable > socket error message. >=20 > Link: https://bugs.passt.top/show_bug.cgi?id=3D134 > Signed-off-by: Anshu Kumari Stefano noted the minor indenting nit, and I have a couple of suggestions for error level. Otherwise LGTM. > --- > v2: > - removed #define FRAGMENT_MSG_RATE > - fixed the indentation for "Invalid endpoint in TCP SYN" log. >=20 >=20 > - Inside udp.c > - David: should this be changed to warn_ratelimit? I am not sure about it > debug("%s error on UDP socket %i: %s", > str_ee_origin(ee), s, strerror_(ee->ee_errno)); >=20 > --- > tap.c | 15 ++++----------- > tcp.c | 6 +++--- > udp.c | 6 +++--- > udp_flow.c | 4 ++-- > 4 files changed, 12 insertions(+), 19 deletions(-) >=20 > diff --git a/tap.c b/tap.c > index 1049e02..b7270d9 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,8 @@ 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..964cfc2 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..1ef5e7a 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,7 +1042,7 @@ int udp_tap_handler(const struct ctx *c, uint8_t pi= f, > 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", > + 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); > diff --git a/udp_flow.c b/udp_flow.c > index 7e2453e..9343b4b 100644 > --- a/udp_flow.c > +++ b/udp_flow.c > @@ -235,7 +235,7 @@ 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", > + warn_ratelimit(now, "Couldn't allocate flow for UDP datagram from %s %= s", > pif_name(pif), sockaddr_ntop(s_in, sastr, sizeof(sastr))); I think this one should go up to err() status. The previous ones could be caused by bogus packets from guest or peer. This one definitely indicates we're not able to forward legitimate looking traffic. > return FLOW_SIDX_NONE; > } > @@ -292,7 +292,7 @@ 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", > + warn_ratelimit(now, "Couldn't allocate flow for UDP datagram from %s %= s:%hu -> %s:%hu", Same for this one. > pif_name(pif), > inet_ntop(af, saddr, sstr, sizeof(sstr)), srcport, > inet_ntop(af, daddr, dstr, sizeof(dstr)), dstport); > --=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 --scQjeS2iXelmsCRM Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmnUecUACgkQzQJF27ox 2Gd4ng/7BdpbwSvPhYbVthnX+ozkj4QRUAYExJxF4T8zKtOF6K1em2ysqajsVAvD ADXg5WevA2twKcrXnBT39kbZ2lZbghSiy9bMYE0PDXFgcB1mgZrXVvyiBuGs2OUK ux1vwld1et8hWkqrZIbr5o0hKAdNluSxJFA5CRA+lUoyQx8tJH0Eg+n39y43kmwr RY1ltuUJwHXBXU4DYIViVal/h4AUpho04kKv/PIcWuKTMRB3kbnM/eN7GZlR9Vh4 lcNmZ9IWekcBlF4rPfH1qvlZ/llrUit+dGMeqBt06CYC1SLY2i0rlX/Tjxdn2Qd7 jxGdJlzYTPynoErYonx+kVYLD0Vsl93jC+u2PaRAnUVUsgBOHu0coJje1SvsTlgd +OZXue0v+ySpRfOiBysAWlXd/A2q2U8xYouhJ6AJy/NgqHBwZxvDtpbENZSF4bGP G/+9q4/Gr8LqOW8CAmKVg6mN+ajBQ+SVBvTt+bGMibcKBcPgmiA0JeSf1/XU5Dy5 Y+yLJjxpZrIg5Ph+T5yDekvoI5QlL9rGu2DPqMI9I+w0p1kuS6799Au3FxvKBhfk UbzwfRS2ssARAxaNSaiLWNIejm1DTPu1nEEem2VLHDaZ//EG9qdG3HelVlwPAcBh s29zHsEgT5HjjQYwpJ5DVbOKd4T6HVM7CgZ7QpI3MU8VL7meo/c= =mTa8 -----END PGP SIGNATURE----- --scQjeS2iXelmsCRM--