Re: [PATCH net-next v6 2/6] net_sched: sch_fq: clear past skb->tstamp if offloading pacing
From: Eric Dumazet <edumazet@google.com>
Date: 2026-08-13 16:04:14
On Thu, Aug 13, 2026 at 6:02 PM Willem de Bruijn [off-list ref] wrote:
On Thu, Aug 13, 2026 at 1:13 AM Eric Dumazet [off-list ref] wrote:quoted
On Thu, Aug 13, 2026 at 4:05 AM Willem de Bruijn [off-list ref] wrote:quoted
From: Willem de Bruijn <willemb@google.com> 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 <willemb@google.com> ---quoted
@@ -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 [off-list ref] 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 [off-list ref] Signed-off-by: Eric Dumazet [off-list ref]diff --git a/net/sched/sch_fq.c b/net/sched/sch_fq.c index 7cae082a984721215b17fea2d094095155580dcb..f2a65a7a928d5838547285921ce0b42c99a96ef4100644--- 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.