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

On Thu, Aug 13, 2026 at 6:02 PM Willem de Bruijn
<[email protected]> wrote:
>
> 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?

No, just add it to the series, if a respin is needed.

>
> 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.

No worries, I do not think anyone is using pacing offload yet.

Thanks.