All of lore.kernel.org
 help / color / mirror / Atom feed
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
To: netdev@vger.kernel.org
Cc: intel-wired-lan@lists.osuosl.org, anthony.l.nguyen@intel.com,
	joshua.a.hay@intel.com, przemyslaw.kitszel@intel.com,
	Willem de Bruijn <willemb@google.com>
Subject: [Intel-wired-lan] [PATCH net-next v4 2/6] net_sched: sch_fq: clear past skb->tstamp if offloading pacing
Date: Thu,  6 Aug 2026 19:25:59 -0400	[thread overview]
Message-ID: <20260806232913.4020403-3-willemdebruijn.kernel@gmail.com> (raw)
In-Reply-To: <20260806232913.4020403-1-willemdebruijn.kernel@gmail.com>

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

---

Changes
  v3 -> v4
    - also reset tstamp_type
    - minor: initialize time_next_packet for more obvious correctness

Sashiko, ignore pre-existing issues. In particular, effects on
non-EDT packets and when queue or sk rate limit is set.

Sashiko, pacing offload is an optimization. Ignore that some packets
may not get offloaded, e.g., when txtime is a few usec in the future.

Claude suggests to only call __skb_clear_delivery_time in one location
in fq_dequeue. Unfortunately the separate fastpath location is needed
as that avoids computing now in fq_dequeue for these fastpath packets.
---
 include/linux/skbuff.h | 17 ++++++++++++-----
 net/sched/sch_fq.c     | 19 +++++++++++++++----
 2 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 22eda1d54a0e..b3445ad5a35c 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -4490,20 +4490,27 @@ static inline void skb_set_delivery_type_by_clockid(struct sk_buff *skb,
 
 DECLARE_STATIC_KEY_FALSE(netstamp_needed_key);
 
-/* It is used in the ingress path to clear the delivery_time.
- * If needed, set the skb->tstamp to the (rcv) timestamp.
- */
-static __always_inline void skb_clear_delivery_time(struct sk_buff *skb)
+static __always_inline void __skb_clear_delivery_time(struct sk_buff *skb,
+						      bool want_tstamp)
 {
 	if (skb->tstamp_type) {
 		skb->tstamp_type = SKB_CLOCK_REALTIME;
-		if (static_branch_unlikely(&netstamp_needed_key))
+		if (want_tstamp &&
+		    static_branch_unlikely(&netstamp_needed_key))
 			skb->tstamp = ktime_get_real();
 		else
 			skb->tstamp = 0;
 	}
 }
 
+/* It is used in the ingress path to clear the delivery_time.
+ * If needed, set the skb->tstamp to the (rcv) timestamp.
+ */
+static __always_inline void skb_clear_delivery_time(struct sk_buff *skb)
+{
+	__skb_clear_delivery_time(skb, true);
+}
+
 static inline void skb_clear_tstamp(struct sk_buff *skb)
 {
 	if (skb->tstamp_type)
diff --git a/net/sched/sch_fq.c b/net/sched/sch_fq.c
index 7cae082a9847..8d7458c38409 100644
--- a/net/sched/sch_fq.c
+++ b/net/sched/sch_fq.c
@@ -399,6 +399,11 @@ static struct fq_flow *fq_classify(struct Qdisc *sch, struct sk_buff *skb,
 		    READ_ONCE(sk->sk_pacing_status) != SK_PACING_FQ)
 			smp_store_release(&sk->sk_pacing_status,
 					  SK_PACING_FQ);
+
+		if (q->offload_horizon &&
+		    fq_skb_cb(skb)->time_to_send <= now)
+			__skb_clear_delivery_time(skb, false);
+
 		return &q->internal;
 	}
 
@@ -707,6 +712,7 @@ static struct sk_buff *fq_dequeue(struct Qdisc *sch)
 	struct fq_sched_data *q = qdisc_priv(sch);
 	struct fq_perband_flows *pband;
 	struct fq_flow_head *head;
+	u64 time_next_packet = 0;
 	struct sk_buff *skb;
 	struct fq_flow *f;
 	unsigned long rate;
@@ -721,7 +727,7 @@ static struct sk_buff *fq_dequeue(struct Qdisc *sch)
 	if (skb) {
 		q->internal.qlen--;
 		fq_dequeue_skb(sch, &q->internal, skb);
-		goto out;
+		return skb;
 	}
 
 	now = ktime_get_ns();
@@ -758,8 +764,8 @@ static struct sk_buff *fq_dequeue(struct Qdisc *sch)
 
 	skb = fq_peek(f);
 	if (skb) {
-		u64 time_next_packet = max_t(u64, fq_skb_cb(skb)->time_to_send,
-					     f->time_next_packet);
+		time_next_packet = max_t(u64, fq_skb_cb(skb)->time_to_send,
+					 f->time_next_packet);
 
 		if (now + q->offload_horizon < time_next_packet) {
 			head->first = f->next;
@@ -828,11 +834,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);
 		f->time_next_packet = now + len;
 	}
+
 out:
+	if (q->offload_horizon &&
+	    time_next_packet && time_next_packet <= now)
+		__skb_clear_delivery_time(skb, false);
+
 	return skb;
 }
 
-- 
2.55.0.679.g6767b8d81c-goog


WARNING: multiple messages have this Message-ID (diff)
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
To: netdev@vger.kernel.org
Cc: intel-wired-lan@lists.osuosl.org, anthony.l.nguyen@intel.com,
	joshua.a.hay@intel.com, przemyslaw.kitszel@intel.com,
	Willem de Bruijn <willemb@google.com>
Subject: [PATCH net-next v4 2/6] net_sched: sch_fq: clear past skb->tstamp if offloading pacing
Date: Thu,  6 Aug 2026 19:25:59 -0400	[thread overview]
Message-ID: <20260806232913.4020403-3-willemdebruijn.kernel@gmail.com> (raw)
In-Reply-To: <20260806232913.4020403-1-willemdebruijn.kernel@gmail.com>

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

---

Changes
  v3 -> v4
    - also reset tstamp_type
    - minor: initialize time_next_packet for more obvious correctness

Sashiko, ignore pre-existing issues. In particular, effects on
non-EDT packets and when queue or sk rate limit is set.

Sashiko, pacing offload is an optimization. Ignore that some packets
may not get offloaded, e.g., when txtime is a few usec in the future.

Claude suggests to only call __skb_clear_delivery_time in one location
in fq_dequeue. Unfortunately the separate fastpath location is needed
as that avoids computing now in fq_dequeue for these fastpath packets.
---
 include/linux/skbuff.h | 17 ++++++++++++-----
 net/sched/sch_fq.c     | 19 +++++++++++++++----
 2 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 22eda1d54a0e..b3445ad5a35c 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -4490,20 +4490,27 @@ static inline void skb_set_delivery_type_by_clockid(struct sk_buff *skb,
 
 DECLARE_STATIC_KEY_FALSE(netstamp_needed_key);
 
-/* It is used in the ingress path to clear the delivery_time.
- * If needed, set the skb->tstamp to the (rcv) timestamp.
- */
-static __always_inline void skb_clear_delivery_time(struct sk_buff *skb)
+static __always_inline void __skb_clear_delivery_time(struct sk_buff *skb,
+						      bool want_tstamp)
 {
 	if (skb->tstamp_type) {
 		skb->tstamp_type = SKB_CLOCK_REALTIME;
-		if (static_branch_unlikely(&netstamp_needed_key))
+		if (want_tstamp &&
+		    static_branch_unlikely(&netstamp_needed_key))
 			skb->tstamp = ktime_get_real();
 		else
 			skb->tstamp = 0;
 	}
 }
 
+/* It is used in the ingress path to clear the delivery_time.
+ * If needed, set the skb->tstamp to the (rcv) timestamp.
+ */
+static __always_inline void skb_clear_delivery_time(struct sk_buff *skb)
+{
+	__skb_clear_delivery_time(skb, true);
+}
+
 static inline void skb_clear_tstamp(struct sk_buff *skb)
 {
 	if (skb->tstamp_type)
diff --git a/net/sched/sch_fq.c b/net/sched/sch_fq.c
index 7cae082a9847..8d7458c38409 100644
--- a/net/sched/sch_fq.c
+++ b/net/sched/sch_fq.c
@@ -399,6 +399,11 @@ static struct fq_flow *fq_classify(struct Qdisc *sch, struct sk_buff *skb,
 		    READ_ONCE(sk->sk_pacing_status) != SK_PACING_FQ)
 			smp_store_release(&sk->sk_pacing_status,
 					  SK_PACING_FQ);
+
+		if (q->offload_horizon &&
+		    fq_skb_cb(skb)->time_to_send <= now)
+			__skb_clear_delivery_time(skb, false);
+
 		return &q->internal;
 	}
 
@@ -707,6 +712,7 @@ static struct sk_buff *fq_dequeue(struct Qdisc *sch)
 	struct fq_sched_data *q = qdisc_priv(sch);
 	struct fq_perband_flows *pband;
 	struct fq_flow_head *head;
+	u64 time_next_packet = 0;
 	struct sk_buff *skb;
 	struct fq_flow *f;
 	unsigned long rate;
@@ -721,7 +727,7 @@ static struct sk_buff *fq_dequeue(struct Qdisc *sch)
 	if (skb) {
 		q->internal.qlen--;
 		fq_dequeue_skb(sch, &q->internal, skb);
-		goto out;
+		return skb;
 	}
 
 	now = ktime_get_ns();
@@ -758,8 +764,8 @@ static struct sk_buff *fq_dequeue(struct Qdisc *sch)
 
 	skb = fq_peek(f);
 	if (skb) {
-		u64 time_next_packet = max_t(u64, fq_skb_cb(skb)->time_to_send,
-					     f->time_next_packet);
+		time_next_packet = max_t(u64, fq_skb_cb(skb)->time_to_send,
+					 f->time_next_packet);
 
 		if (now + q->offload_horizon < time_next_packet) {
 			head->first = f->next;
@@ -828,11 +834,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);
 		f->time_next_packet = now + len;
 	}
+
 out:
+	if (q->offload_horizon &&
+	    time_next_packet && time_next_packet <= now)
+		__skb_clear_delivery_time(skb, false);
+
 	return skb;
 }
 
-- 
2.55.0.679.g6767b8d81c-goog


  parent reply	other threads:[~2026-08-06 23:29 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-06 23:25 [Intel-wired-lan] [PATCH net-next v4 0/6] hardware pacing offload Willem de Bruijn
2026-08-06 23:25 ` Willem de Bruijn
2026-08-06 23:25 ` [Intel-wired-lan] [PATCH net-next v4 1/6] net: rtnetlink: add pacing_offload_horizon attribute to net_device Willem de Bruijn
2026-08-06 23:25   ` Willem de Bruijn
2026-08-06 23:25 ` Willem de Bruijn [this message]
2026-08-06 23:25   ` [PATCH net-next v4 2/6] net_sched: sch_fq: clear past skb->tstamp if offloading pacing Willem de Bruijn
2026-08-06 23:26 ` [Intel-wired-lan] [PATCH net-next v4 3/6] idpf: support pacing offload Willem de Bruijn
2026-08-06 23:26   ` Willem de Bruijn
2026-08-06 23:26 ` [Intel-wired-lan] [PATCH net-next v4 4/6] selftests: drv-net: refactor so_txtime errqueue handling Willem de Bruijn
2026-08-06 23:26   ` Willem de Bruijn
2026-08-06 23:26 ` [Intel-wired-lan] [PATCH net-next v4 5/6] selftests: drv-net: in so_txtime tell apart sw from hw pacing Willem de Bruijn
2026-08-06 23:26   ` Willem de Bruijn
2026-08-06 23:26 ` [Intel-wired-lan] [PATCH net-next v4 6/6] selftests: drv-net: extend so_txtime with hw offload Willem de Bruijn
2026-08-06 23:26   ` Willem de Bruijn
2026-08-07  1:18 ` [Intel-wired-lan] [PATCH net-next v4 0/6] hardware pacing offload Willem de Bruijn
2026-08-07  1:18   ` Willem de Bruijn
2026-08-07 22:39   ` [Intel-wired-lan] " Jakub Kicinski
2026-08-07 22:39     ` Jakub Kicinski
2026-08-07 23:47     ` [Intel-wired-lan] " Willem de Bruijn
2026-08-07 23:47       ` Willem de Bruijn

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260806232913.4020403-3-willemdebruijn.kernel@gmail.com \
    --to=willemdebruijn.kernel@gmail.com \
    --cc=anthony.l.nguyen@intel.com \
    --cc=intel-wired-lan@lists.osuosl.org \
    --cc=joshua.a.hay@intel.com \
    --cc=netdev@vger.kernel.org \
    --cc=przemyslaw.kitszel@intel.com \
    --cc=willemb@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.