Re: [PATCH net-next v6 2/6] net_sched: sch_fq: clear past skb->tstamp if offloading pacing

On Thu, Aug 13, 2026 at 1:13 AM Eric Dumazet <[email protected]> wrote:
>
> On Thu, Aug 13, 2026 at 4:05 AM Willem de Bruijn
> <[email protected]> wrote:
> >
> > From: Willem de Bruijn <[email protected]>
> >
> > When hardware offload is enabled, FQ will forward packets to the
> > netdevice for pacing. The device has to test that skb->tstamp is
> > in the future.
> >
> > Avoid this cost for packets whose txtime has already passed, by
> > clearing skb->tstamp.
> >
> > Also clear slightly into the future, for EDT timestamps that are
> > so close to now that they fall within a reasonable normal Tx
> > latency. This slack is set to 400 nsec.
> >
> > Also disable timer drift logic when offload is enabled, because
> > time_next_packet can exceed now causing a negative value.
> >
> > Signed-off-by: Willem de Bruijn <[email protected]>
> >
> > ---
>
> > @@ -828,11 +837,16 @@ static struct sk_buff *fq_dequeue(struct Qdisc *sch)
> >                  * f->time_next_packet was set when prior packet was sent,
> >                  * and current time (@now) can be too late by tens of us.
> >                  */
> > -               if (f->time_next_packet)
> > +               if (f->time_next_packet && f->time_next_packet < now)
> >                         len -= min(len/2, now - f->time_next_packet);
>
> IMO this part deserves a patch on its own.
>
> Using a variable adds no cost (compiler will generate the same code),
> but helps to understand what is going on?
>
> commit e51f3936aba0e5887cf5884b18ffa769dcb38523
> Author: Eric Dumazet <[email protected]>
> Date:   Wed Jul 1 08:33:38 2026 +0000
>
>     net_sched: sch_fq: fix pacing delay underflow with pacing offload
>
>     When pacing offload is enabled (q->offload_horizon > 0),
>     FQ can dequeue packets early (now < f->time_next_packet).
>
>     In this case, the drift calculation (now - f->time_next_packet)
>     underflows to a large unsigned value.
>
>     min(len/2, now - f->time_next_packet) then evaluates to len/2,
>     incorrectly halving the pacing delay for the next packet.
>
>     Fix this by only applying drift compensation when now > f->time_next_packet.
>
>     This bug was triggered when flow_max_rate was set on the qdisc
>     or for non EDT packets (packets with a zero skb->tstamp).
>
>     Fixes: f26080d47007 ("net_sched: sch_fq: add the ability to offload pacing")
>     Reported-by: Willem de Bruijn <[email protected]>
>     Signed-off-by: Eric Dumazet <[email protected]>
>
> diff --git a/net/sched/sch_fq.c b/net/sched/sch_fq.c
> index 7cae082a984721215b17fea2d094095155580dcb..f2a65a7a928d5838547285921ce0b42c99a96ef4
> 100644
> --- a/net/sched/sch_fq.c
> +++ b/net/sched/sch_fq.c
> @@ -828,8 +828,12 @@ static struct sk_buff *fq_dequeue(struct Qdisc *sch)
>                  * f->time_next_packet was set when prior packet was sent,
>                  * and current time (@now) can be too late by tens of us.
>                  */
> -               if (f->time_next_packet)
> -                       len -= min(len/2, now - f->time_next_packet);
> +               if (f->time_next_packet) {
> +                       s64 drift = now - f->time_next_packet;
> +
> +                       if (drift > 0)
> +                               len -= min_t(u64, len/2, drift);
> +               }
>                 f->time_next_packet = now + len;
>         }
>  out:

Sounds good. Indeed clearer as an independent patch with its own
commit explanation.

Do you want to send that separately now?

Else I will add it to the series. But that is likely to slip to the
next release, as the current revision conflicts with other idpf
changes queued for net.