Thread (12 messages) flat view 12 messages, 3 authors, 13h ago

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..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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help