Thread (12 messages) flat view 12 messages, 3 authors, 16d 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 05:13:13
Subsystem: networking [general], tc subsystem, the rest · Maintainers: "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Jamal Hadi Salim, Jiri Pirko, Linus Torvalds

On Thu, Aug 13, 2026 at 4:05 AM Willem de Bruijn
[off-list ref] wrote:
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 hunk ↗ jump to hunk
@@ -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:
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help