Re: [PATCH v1 2/2] tap, tcp, udp: Use rate-limited logging

[Stefano Brivio <sbrivio@redhat.com>]

On Thu, 2 Apr 2026 17:10:12 +0200
Laurent Vivier <lvivier@redhat.com> wrote:

> On 4/1/26 20:29, Anshu Kumari wrote:
>
> [...]
>
> > +++ b/tap.c
> > @@ -686,17 +686,11 @@ 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) {  
> 
> the #define FRAGMENT_MSG_RATE can be removed from the file as it is unused now.
> 
> > -			warn("Can't process IPv4 fragments (%u dropped)",
> > -			     num_dropped);
> > -			last_message = now->tv_sec;
> > -			num_dropped = 0;
> > -		}
> > +		warn_ratelimit(now, "Can't process IPv4 fragments (%u dropped)",
> > +			       num_dropped);  
> 
> I don't think we should keep the num_dropped value here as it is never reset, the 
> "suppressed %u similar messages" will give the information.

Wait, that was actually my suggestion based on the previous patch:

  https://archives.passt.top/passt-dev/20260324210145.1f530a59@elisabeth/

reporting here for convenience:

---
If you just drop "(%u dropped)", it will be pretty hard to understand
the rate at which fragments are being sent: is it one per second or a
thousand? That makes a difference while debugging things.

I would suggest to keep the 'num_dropped' counter, keep printing the
number (without zeroing it, because you don't know when to do it here),
and yes, it will wrap eventually (become 0 again), but the wrapping
should be very obvious, and it's better than having no indication at
all.
---

-- 
Stefano