netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next v8 0/6] hardware pacing offload
@ 2026-09-02 18:17 Willem de Bruijn
  2026-09-02 18:17 ` [PATCH net-next v8 1/6] net: rtnetlink: add pacing_offload_horizon attribute to net_device Willem de Bruijn
                   ` (5 more replies)
  0 siblings, 6 replies; 25+ messages in thread
From: Willem de Bruijn @ 2026-09-02 18:17 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, edumazet, pabeni, horms, andrew+netdev,
	Willem de Bruijn

From: Willem de Bruijn <willemb@google.com>

Enable safe hardware pacing offload on a first device (idpf), plus support.

- Add rtnetlink support for admin control over the device feature
- Add device support to the idpf driver

Besides the main feature, also

- Small optimization to FQ offload: avoid unnecessary ktime_.._get().
- Expand so_txtime drv-net test with hw offload testcases.
- Expand pktgen with txtime feature.

Details and detailed changelog in the individual patches.

High level changes:
  v7 -> v8
    - 2 small fixes: unused variable in idpf and new ruff warning in test
  v6 -> v7
    - Rebase onto conflicting patches and address feedback
  v5 -> v6
    - Address substantial opus 5 feedback
  v4 -> v5
    - Rebase + small kdoc fix only
  v3 -> v4
    - Replace ethtool API with rtnetlink
    - Drop/defer pktgen patch: not essential

v7: https://lore.kernel.org/netdev/20260831213221.1370774-1-willemdebruijn.kernel@gmail.com/
v6: https://lore.kernel.org/netdev/20260813020529.3506539-1-willemdebruijn.kernel@gmail.com/
v5: https://lore.kernel.org/netdev/20260808155217.885299-1-willemdebruijn.kernel@gmail.com/
v4: https://lore.kernel.org/netdev/20260806232913.4020403-1-willemdebruijn.kernel@gmail.com/
v3: https://lore.kernel.org/netdev/20260726202902.760552-1-willemdebruijn.kernel@gmail.com/
v2: https://lore.kernel.org/netdev/20260722204454.3234605-1-willemdebruijn.kernel@gmail.com/
v1: https://lore.kernel.org/netdev/20260706133433.3142805-1-willemdebruijn.kernel@gmail.com/

Willem de Bruijn (6):
  net: rtnetlink: add pacing_offload_horizon attribute to net_device
  net_sched: sch_fq: clear past skb->tstamp if offloading pacing
  idpf: support pacing offload
  selftests: drv-net: refactor so_txtime errqueue handling
  selftests: drv-net: in so_txtime tell apart sw from hw pacing
  selftests: drv-net: extend so_txtime with hw offload

 Documentation/netlink/specs/rt-link.yaml      |   6 +
 .../networking/net_cachelines/net_device.rst  |   3 +-
 drivers/net/ethernet/intel/idpf/idpf.h        |  12 ++
 drivers/net/ethernet/intel/idpf/idpf_lib.c    |   6 +
 drivers/net/ethernet/intel/idpf/idpf_txrx.c   |  66 +++++++-
 drivers/net/ethernet/intel/idpf/idpf_txrx.h   |   8 +-
 .../net/ethernet/intel/idpf/idpf_virtchnl.c   |  60 ++++++-
 include/linux/netdevice.h                     |   4 +-
 include/uapi/linux/if_link.h                  |   1 +
 net/core/rtnetlink.c                          |  21 +++
 net/sched/sch_fq.c                            |  23 ++-
 .../testing/selftests/drivers/net/so_txtime.c | 159 +++++++++++++-----
 .../selftests/drivers/net/so_txtime.py        |  74 +++++++-
 13 files changed, 387 insertions(+), 56 deletions(-)

-- 
2.55.0.970.g62bdec98f9-goog


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH net-next v8 1/6] net: rtnetlink: add pacing_offload_horizon attribute to net_device
  2026-09-02 18:17 [PATCH net-next v8 0/6] hardware pacing offload Willem de Bruijn
@ 2026-09-02 18:17 ` Willem de Bruijn
  2026-09-04  0:19   ` netdev-bot+sashiko
  2026-09-04 23:01   ` Jakub Kicinski
  2026-09-02 18:17 ` [PATCH net-next v8 2/6] net_sched: sch_fq: clear past skb->tstamp if offloading pacing Willem de Bruijn
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 25+ messages in thread
From: Willem de Bruijn @ 2026-09-02 18:17 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, edumazet, pabeni, horms, andrew+netdev,
	Willem de Bruijn

From: Willem de Bruijn <willemb@google.com>

The 'max_pacing_offload_horizon' field of 'struct net_device' represents
the maximum pacing offload horizon supported by the device.

Add a new field 'pacing_offload_horizon' to store the active pacing
offload horizon.

The new attribute is initialized to 0 (disabled) and can be set from
userspace via RTM_SETLINK up to dev->max_pacing_offload_horizon. This
new default off behavior does not cause regressions, as no driver yet
advertises max_pacing_offload_horizon.

The attribute is omitted from the newlink request spec, because the
value may need to be bound by a device maximum that first needs to be
negotiated with firmware, as is the case for the idpf driver in this
series.

Make both fields u32, to maintain net_device cacheline layout. This
expresses up to 4s of pacing offload, which is sufficient.

Update the YNL specification ('rt-link.yaml') to add the
'pacing-offload-horizon' attribute and include it in link-all-attrs.

Both fields can be read with

    ynl --spec /usr/local/share/ynl/specs/rt-link.yaml \
        --do getlink \
        --json '{"ifname": "eth0"}' | grep pacing

And the active horizon set with

    ynl --spec /usr/local/share/ynl/specs/rt-link.yaml \
        --do setlink \
        --json '{"ifname": "eth0", "pacing-offload-horizon": 50000000}'

Signed-off-by: Willem de Bruijn <willemb@google.com>

----

Changes
  v6 -> v7
    - commit-msg: update ynl invocation to installed version
  v5 -> v6
    - bring back fq_change update
    - commit-msg: reword absense of RTM_NEWLINK -> omitted from newlink
---
 Documentation/netlink/specs/rt-link.yaml      |  6 ++++++
 .../networking/net_cachelines/net_device.rst  |  3 ++-
 include/linux/netdevice.h                     |  4 +++-
 include/uapi/linux/if_link.h                  |  1 +
 net/core/rtnetlink.c                          | 21 +++++++++++++++++++
 net/sched/sch_fq.c                            |  3 ++-
 6 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/Documentation/netlink/specs/rt-link.yaml b/Documentation/netlink/specs/rt-link.yaml
index 61ebb9a2bad5..2a2aad9a29c4 100644
--- a/Documentation/netlink/specs/rt-link.yaml
+++ b/Documentation/netlink/specs/rt-link.yaml
@@ -1089,6 +1089,10 @@ attribute-sets:
       -
         name: tailroom
         type: u16
+      -
+        name: pacing-offload-horizon
+        type: uint
+        doc: EDT offload horizon setting for the device (in nsec).
   -
     name: prop-list-link-attrs
     subset-of: link-attrs
@@ -2557,6 +2561,8 @@ operations:
             - devlink-port
             - gso-ipv4-max-size
             - gro-ipv4-max-size
+            - max-pacing-offload-horizon
+            - pacing-offload-horizon
       dump:
         request:
           value: 18
diff --git a/Documentation/networking/net_cachelines/net_device.rst b/Documentation/networking/net_cachelines/net_device.rst
index 512f6d6fa3d8..90e257e815b1 100644
--- a/Documentation/networking/net_cachelines/net_device.rst
+++ b/Documentation/networking/net_cachelines/net_device.rst
@@ -183,7 +183,8 @@ struct devlink_port*                devlink_port
 struct dpll_pin*                    dpll_pin
 struct hlist_head                   page_pools
 struct dim_irq_moder*               irq_moder
-u64                                 max_pacing_offload_horizon
+u32                                 max_pacing_offload_horizon
+u32                                 pacing_offload_horizon
 struct_napi_config*                 napi_config
 unsigned_long                       gro_flush_timeout
 u32                                 napi_defer_hard_irqs
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 87cafc932e9e..82a74ad284a1 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2139,6 +2139,7 @@ enum netdev_reg_state {
  *		   where the clock is recovered.
  *
  *	@max_pacing_offload_horizon: max EDT offload horizon in nsec.
+ *	@pacing_offload_horizon: EDT offload horizon setting in nsec.
  *	@napi_config: An array of napi_config structures containing per-NAPI
  *		      settings.
  *	@num_napi_configs:	number of allocated NAPI config structs,
@@ -2561,7 +2562,8 @@ struct net_device {
 	/** @irq_moder: dim parameters used if IS_ENABLED(CONFIG_DIMLIB). */
 	struct dim_irq_moder	*irq_moder;
 
-	u64			max_pacing_offload_horizon;
+	u32			max_pacing_offload_horizon;
+	u32			pacing_offload_horizon;
 	struct napi_config	*napi_config;
 	u32			num_napi_configs;
 	u32			napi_defer_hard_irqs;
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index 43cecca49f01..7c1d7754a670 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -381,6 +381,7 @@ enum {
 	IFLA_NETNS_IMMUTABLE,
 	IFLA_HEADROOM,
 	IFLA_TAILROOM,
+	IFLA_PACING_OFFLOAD_HORIZON,
 	__IFLA_MAX
 };
 
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 81c5a6104dea..eeaf953b397f 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1396,6 +1396,7 @@ static noinline size_t if_nlmsg_size(const struct net_device *dev,
 	       + rtnl_devlink_port_size(dev)
 	       + rtnl_dpll_pin_size()
 	       + nla_total_size(8)  /* IFLA_MAX_PACING_OFFLOAD_HORIZON */
+	       + nla_total_size(8)  /* IFLA_PACING_OFFLOAD_HORIZON */
 	       + nla_total_size(2)  /* IFLA_HEADROOM */
 	       + nla_total_size(2)  /* IFLA_TAILROOM */
 	       + rtnl_dev_parent_size(dev)
@@ -2176,6 +2177,8 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb,
 			READ_ONCE(dev->tso_max_segs)) ||
 	    nla_put_uint(skb, IFLA_MAX_PACING_OFFLOAD_HORIZON,
 			 READ_ONCE(dev->max_pacing_offload_horizon)) ||
+	    nla_put_uint(skb, IFLA_PACING_OFFLOAD_HORIZON,
+			 READ_ONCE(dev->pacing_offload_horizon)) ||
 #ifdef CONFIG_RPS
 	    nla_put_u32(skb, IFLA_NUM_RX_QUEUES,
 			READ_ONCE(dev->num_rx_queues)) ||
@@ -2344,9 +2347,11 @@ static const struct nla_policy ifla_policy[IFLA_MAX+1] = {
 	[IFLA_ALLMULTI]		= { .type = NLA_REJECT },
 	[IFLA_GSO_IPV4_MAX_SIZE]	= NLA_POLICY_MIN(NLA_U32, MAX_TCP_HEADER + 1),
 	[IFLA_GRO_IPV4_MAX_SIZE]	= { .type = NLA_U32 },
+	[IFLA_MAX_PACING_OFFLOAD_HORIZON] = { .type = NLA_REJECT },
 	[IFLA_NETNS_IMMUTABLE]	= { .type = NLA_REJECT },
 	[IFLA_HEADROOM]		= { .type = NLA_REJECT },
 	[IFLA_TAILROOM]		= { .type = NLA_REJECT },
+	[IFLA_PACING_OFFLOAD_HORIZON] = { .type = NLA_UINT },
 };
 
 static const struct nla_policy ifla_info_policy[IFLA_INFO_MAX+1] = {
@@ -2820,6 +2825,13 @@ static int validate_linkmsg(struct net_device *dev, struct nlattr *tb[],
 		return -EINVAL;
 	}
 
+	if (tb[IFLA_PACING_OFFLOAD_HORIZON] &&
+	    nla_get_uint(tb[IFLA_PACING_OFFLOAD_HORIZON]) >
+	    dev->max_pacing_offload_horizon) {
+		NL_SET_ERR_MSG(extack, "too big pacing_offload_horizon");
+		return -EINVAL;
+	}
+
 	if (tb[IFLA_AF_SPEC]) {
 		struct nlattr *af;
 		int rem, err;
@@ -3337,6 +3349,15 @@ static int do_setlink(const struct sk_buff *skb, struct net_device *dev,
 		}
 	}
 
+	if (tb[IFLA_PACING_OFFLOAD_HORIZON]) {
+		u64 horizon = nla_get_uint(tb[IFLA_PACING_OFFLOAD_HORIZON]);
+
+		if (dev->pacing_offload_horizon ^ horizon) {
+			WRITE_ONCE(dev->pacing_offload_horizon, horizon);
+			status |= DO_SETLINK_MODIFIED;
+		}
+	}
+
 	if (tb[IFLA_OPERSTATE])
 		set_operstate(dev, nla_get_u8(tb[IFLA_OPERSTATE]));
 
diff --git a/net/sched/sch_fq.c b/net/sched/sch_fq.c
index 6144b5686f13..7cf7afda699f 100644
--- a/net/sched/sch_fq.c
+++ b/net/sched/sch_fq.c
@@ -1183,7 +1183,8 @@ static int fq_change(struct Qdisc *sch, struct nlattr *opt,
 		u64 offload_horizon = (u64)NSEC_PER_USEC *
 				      nla_get_u32(tb[TCA_FQ_OFFLOAD_HORIZON]);
 
-		if (offload_horizon <= qdisc_dev(sch)->max_pacing_offload_horizon) {
+		if (offload_horizon <=
+		    READ_ONCE(qdisc_dev(sch)->pacing_offload_horizon)) {
 			WRITE_ONCE(q->offload_horizon, offload_horizon);
 		} else {
 			NL_SET_ERR_MSG_MOD(extack, "invalid offload_horizon");
-- 
2.55.0.970.g62bdec98f9-goog


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH net-next v8 2/6] net_sched: sch_fq: clear past skb->tstamp if offloading pacing
  2026-09-02 18:17 [PATCH net-next v8 0/6] hardware pacing offload Willem de Bruijn
  2026-09-02 18:17 ` [PATCH net-next v8 1/6] net: rtnetlink: add pacing_offload_horizon attribute to net_device Willem de Bruijn
@ 2026-09-02 18:17 ` Willem de Bruijn
  2026-09-02 18:17 ` [PATCH net-next v8 3/6] idpf: support pacing offload Willem de Bruijn
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 25+ messages in thread
From: Willem de Bruijn @ 2026-09-02 18:17 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, edumazet, pabeni, horms, andrew+netdev,
	Willem de Bruijn

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.

Signed-off-by: Willem de Bruijn <willemb@google.com>

---

Changes
  v6 -> v7
    - timer drift logic fix moved to separate commit
  v5 -> v6
    - switch to skb_set_delivery_time to also clear SKB_CLOCK_REALTIME
    - add fq_offload_slack_ns
  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 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.
---
 net/sched/sch_fq.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/net/sched/sch_fq.c b/net/sched/sch_fq.c
index 7cf7afda699f..322120760507 100644
--- a/net/sched/sch_fq.c
+++ b/net/sched/sch_fq.c
@@ -159,6 +159,9 @@ struct fq_sched_data {
 	u64		stat_allocation_errors;
 };
 
+/* EDT timestamps to clear beyond now. */
+static const int fq_offload_slack_ns = 400;
+
 /* return the i-th 2-bit value ("crumb") */
 static u8 fq_prio2band(const u8 *prio2band, unsigned int prio)
 {
@@ -399,6 +402,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 + fq_offload_slack_ns)
+			skb_set_delivery_time(skb, 0, 0);
+
 		return &q->internal;
 	}
 
@@ -707,6 +715,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 +730,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 +767,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;
@@ -836,7 +845,12 @@ static struct sk_buff *fq_dequeue(struct Qdisc *sch)
 		}
 		f->time_next_packet = now + len;
 	}
+
 out:
+	if (q->offload_horizon &&
+	    time_next_packet && time_next_packet <= now + fq_offload_slack_ns)
+		skb_set_delivery_time(skb, 0, 0);
+
 	return skb;
 }
 
-- 
2.55.0.970.g62bdec98f9-goog


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH net-next v8 3/6] idpf: support pacing offload
  2026-09-02 18:17 [PATCH net-next v8 0/6] hardware pacing offload Willem de Bruijn
  2026-09-02 18:17 ` [PATCH net-next v8 1/6] net: rtnetlink: add pacing_offload_horizon attribute to net_device Willem de Bruijn
  2026-09-02 18:17 ` [PATCH net-next v8 2/6] net_sched: sch_fq: clear past skb->tstamp if offloading pacing Willem de Bruijn
@ 2026-09-02 18:17 ` Willem de Bruijn
  2026-09-04  0:19   ` netdev-bot+sashiko
  2026-09-07  9:30   ` Loktionov, Aleksandr
  2026-09-02 18:17 ` [PATCH net-next v8 4/6] selftests: drv-net: refactor so_txtime errqueue handling Willem de Bruijn
                   ` (2 subsequent siblings)
  5 siblings, 2 replies; 25+ messages in thread
From: Willem de Bruijn @ 2026-09-02 18:17 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, edumazet, pabeni, horms, andrew+netdev,
	Willem de Bruijn, Tony Nguyen, Przemek Kitszel, Joshua A Hay,
	intel-wired-lan

From: Willem de Bruijn <willemb@google.com>

If skb->tstamp is in the future, program this future delivery txtime
in the transmit descriptor.

TCP pacing offload is only offloaded if SK_PACING_FQ is negotiated and
the FQ offload_horizon is configured. But device support for pacing
offload must be more robust: it can also be reached through SO_TXTIME.

Bounds check txtime. Only packets with timestamp between now and the
horizon (pacing_offload_horizon) are offloaded.

Negotiate the feature with the device using virtchnl. Support is
conditional on
- splitq mode, where tx and tx completion queues are separate, so
  completions can be returned out of order.
- flow scheduling mode, where completions can arrive out of order.
- PTP to ensure the NIC clock is synced to CLOCK_TAI.

These features are negotiated per adapter, but expect all vports to
uniformly request splitq (req_[rt]x_splitq) and flow scheduling
(flow_sch_en) when available.

Do not explicitly check all preconditions. Trust the firmware to
only advertise EDT when all are met. In general, firmware response is
trusted to be correct. If not, EDT would be the least of the worries.
In particular, granularity is trusted to be a power of two. And
time_horizon_ns is a range expressible given the chosen granularity.

Packets beyond the horizon are sent immediately with the overflow bit
set.

On device reset, dev->pacing_offload_horizon, fq offload_horizon and
granularity are not re-negotiated. It is safe to assume that firmware
does not change these EDT capabilities across resets.

Must not be called from netpoll due to ktime_get. But netpoll does not
generate packets with EDT, so no explicit test is needed.

Do not fail device initialization on EDT init error. Log an error, but
continue without EDT, similar to PTP.

Cc: Tony Nguyen <anthony.l.nguyen@intel.com>
Cc: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Cc: Joshua A Hay <joshua.a.hay@intel.com>
Cc: intel-wired-lan@lists.osuosl.org
Signed-off-by: Willem de Bruijn <willemb@google.com>

---

Changes
  v7 -> v8
    - remove unused variable np in idpf_tx_splitq_set_txtime
  v6 -> v7
    - rebase onto libie changes: adjust idpf_send_get_edt_caps_msg
    - only set horizon if splitq with flow scheduling
    - fail on illegal granularity 0, rather than use hardcoded default
    - zero edt_caps on negotation error, in case of call after reset
    - cache ts_gran_pow2 in idpf_tx_queue to be in hot cache line
    - similar to FQ, add offload slack to avoid pacing < 400ns of now
    - remove now superfluous include netpoll.h
    - reverse xmas tree (1x)
  v5 -> v6
    - move EDT caps from virtchnl2_edt_caps_ilog2 to idpf_edt_caps_ilog2
    - do not fail device init if EDT init failed, same as PTP
    - replace netpoll branch with commit-msg comment
    - add commit-msg comment about device reset
    - add commit-msg about behavior beyond horizon
    - add commit-msg about trusting EDT capability response
  v4 -> v5
    - fix kdoc: idpf.h:738 struct member 'edt_caps' not described..
  v3 -> v4
    - add EDT virtchnl negotiation
    - move endianness fix to its own iwl-net patch
    - update commit-msg: check is against pacing_offload_horizon, not max_..
    - simplify assignment, avoid the le32 assignment in favor of u8 inits
    - replace open coded constant with IDPF_TXD_FLOW_SCH_HORIZON_OVERFLOW_M
    - (minor) fix 24b timestamp comment: is 23b + overflow bit
    - (minor) remove unused parameter
  v2 -> v3
    - use READ_ONCE and read pacing_offload_horizon only once
  v1 -> v2
    - move special zero case up and return early

Sashiko, ignore pre-existing issues.

Sashiko, ignore that idpf_tx_splitq_set_txtime may have a benign race
by calling ktime_mono_to_any twice to get TAI to REALTIME offset.
---
 drivers/net/ethernet/intel/idpf/idpf.h        | 12 ++++
 drivers/net/ethernet/intel/idpf/idpf_lib.c    |  6 ++
 drivers/net/ethernet/intel/idpf/idpf_txrx.c   | 66 ++++++++++++++++++-
 drivers/net/ethernet/intel/idpf/idpf_txrx.h   |  8 ++-
 .../net/ethernet/intel/idpf/idpf_virtchnl.c   | 60 ++++++++++++++++-
 5 files changed, 149 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/idpf/idpf.h b/drivers/net/ethernet/intel/idpf/idpf.h
index 470bc23c844c..f214023095ee 100644
--- a/drivers/net/ethernet/intel/idpf/idpf.h
+++ b/drivers/net/ethernet/intel/idpf/idpf.h
@@ -604,6 +604,16 @@ struct idpf_vport_config {
 	DECLARE_BITMAP(flags, IDPF_VPORT_CONFIG_FLAGS_NBITS);
 };
 
+/**
+ * struct idpf_edt_caps_ilog2 - Host parsed EDT capabilities.
+ * @time_horizon_ns: Total time window in nanoseconds.
+ * @tstamp_granularity_pow2: Log2 of timestamp granularity in nanoseconds.
+ */
+struct idpf_edt_caps_ilog2 {
+	u32 time_horizon_ns;
+	u8 tstamp_granularity_pow2;
+};
+
 #define idpf_for_each_vport(adapter, iter) \
 	for (struct idpf_vport **__##iter = &(adapter)->vports[0], \
 	     *iter = (adapter)->max_vports ? *__##iter : NULL; \
@@ -657,6 +667,7 @@ struct idpf_vport_config {
  * @stats_task: Periodic statistics retrieval task
  * @stats_wq: Workqueue for statistics task
  * @caps: Negotiated capabilities with device
+ * @edt_caps: Negotiated EDT capabilities with device
  * @dev_ops: See idpf_dev_ops
  * @cdev_info: IDC core device info pointer
  * @num_vfs: Number of allocated VFs through sysfs. PF does not directly talk
@@ -720,6 +731,7 @@ struct idpf_adapter {
 	struct delayed_work stats_task;
 	struct workqueue_struct *stats_wq;
 	struct virtchnl2_get_capabilities caps;
+	struct idpf_edt_caps_ilog2 edt_caps;
 
 	struct idpf_dev_ops dev_ops;
 	struct iidc_rdma_core_dev_info *cdev_info;
diff --git a/drivers/net/ethernet/intel/idpf/idpf_lib.c b/drivers/net/ethernet/intel/idpf/idpf_lib.c
index 827c795afcb6..c2ad3fdf72aa 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_lib.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_lib.c
@@ -890,6 +890,12 @@ static int idpf_cfg_netdev(struct idpf_vport *vport)
 	netdev->min_mtu = ETH_MIN_MTU;
 	netdev->max_mtu = vport->max_mtu;
 
+	if (adapter->edt_caps.time_horizon_ns &&
+	    idpf_is_queue_model_split(vport->dflt_qv_rsrc.txq_model) &&
+	    !idpf_is_cap_ena(adapter, IDPF_OTHER_CAPS,
+			     VIRTCHNL2_CAP_SPLITQ_QSCHED))
+		netdev->max_pacing_offload_horizon = adapter->edt_caps.time_horizon_ns;
+
 	dflt_features = NETIF_F_SG	|
 			NETIF_F_HIGHDMA;
 
diff --git a/drivers/net/ethernet/intel/idpf/idpf_txrx.c b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
index 24b91be25676..d80b6f5a0fde 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_txrx.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
@@ -1742,6 +1742,7 @@ static int idpf_txq_group_alloc(struct idpf_vport *vport,
 			q->desc_count = rsrc->txq_desc_count;
 			q->tx_max_bufs = idpf_get_max_tx_bufs(adapter);
 			q->tx_min_pkt_len = idpf_get_min_tx_pkt_len(adapter);
+			q->ts_gran_pow2 = adapter->edt_caps.tstamp_granularity_pow2;
 			q->netdev = vport->netdev;
 			q->txq_grp = tx_qgrp;
 			q->rel_q_id = j;
@@ -2408,7 +2409,12 @@ void idpf_tx_splitq_build_flow_desc(union idpf_tx_flex_desc *desc,
 				    struct idpf_tx_splitq_params *params,
 				    u16 td_cmd, u16 size)
 {
-	*(__le32 *)&desc->flow.qw1.cmd_dtype = cpu_to_le32((u8)(params->dtype | td_cmd));
+	desc->flow.qw1.cmd_dtype = (u8)(params->dtype | td_cmd);
+
+	desc->flow.qw1.ts[0] = params->offload.desc_ts[0];
+	desc->flow.qw1.ts[1] = params->offload.desc_ts[1];
+	desc->flow.qw1.ts[2] = params->offload.desc_ts[2];
+
 	desc->flow.qw1.rxr_bufsize = cpu_to_le16((u16)size);
 	desc->flow.qw1.compl_tag = cpu_to_le16(params->compl_tag);
 }
@@ -3011,6 +3017,60 @@ static bool idpf_tx_splitq_need_re(struct idpf_tx_queue *tx_q)
 	return gap >= IDPF_TX_SPLITQ_RE_MIN_GAP;
 }
 
+static void idpf_tx_splitq_set_txtime(const struct sk_buff *skb,
+				      const struct idpf_tx_queue *tx_q,
+				      struct idpf_tx_splitq_params *tx_params)
+{
+	const int offload_slack_ns = 400;
+	u64 ts, now, horizon;
+
+	horizon = READ_ONCE(skb->dev->pacing_offload_horizon);
+	if (!horizon)
+		return;
+
+	switch (skb->tstamp_type) {
+	case SKB_CLOCK_REALTIME:
+		ts = ktime_to_ns(ktime_add(skb->tstamp,
+					   ktime_mono_to_any(0, TK_OFFS_TAI) -
+					   ktime_mono_to_any(0, TK_OFFS_REAL)));
+		break;
+	case SKB_CLOCK_MONOTONIC:
+		ts = ktime_to_ns(ktime_mono_to_any(skb->tstamp, TK_OFFS_TAI));
+		break;
+	case SKB_CLOCK_TAI:
+		ts = ktime_to_ns(skb->tstamp);
+		break;
+	default:
+		WARN_ON_ONCE(1);
+		return;
+	}
+
+	now = ktime_get_clocktai_ns();
+	if (ts < now + offload_slack_ns)
+		return;
+
+	/* beyond offload horizon? set overflow bit only */
+	if (ts > now + horizon) {
+		tx_params->offload.desc_ts[2] =
+			IDPF_TXD_FLOW_SCH_HORIZON_OVERFLOW_M;
+		return;
+	}
+
+	ts >>= tx_q->ts_gran_pow2;
+
+	/* 0 is valid 23b timestamp, but also means field unset.
+	 * Increase by one to avoid this case
+	 */
+	if ((ts & 0x7fffff) == 0) {
+		tx_params->offload.desc_ts[0] = 1;
+		return;
+	}
+
+	tx_params->offload.desc_ts[0] = ts & 0xff;
+	tx_params->offload.desc_ts[1] = (ts >> 8) & 0xff;
+	tx_params->offload.desc_ts[2] = ((ts >> 16) & 0x7f);
+}
+
 /**
  * idpf_tx_splitq_frame - Sends buffer on Tx ring using flex descriptors
  * @skb: send buffer
@@ -3097,6 +3157,10 @@ static netdev_tx_t idpf_tx_splitq_frame(struct sk_buff *skb,
 
 		tx_params.dtype = IDPF_TX_DESC_DTYPE_FLEX_FLOW_SCHE;
 		tx_params.eop_cmd = IDPF_TXD_FLEX_FLOW_CMD_EOP;
+
+		if (skb->tstamp)
+			idpf_tx_splitq_set_txtime(skb, tx_q, &tx_params);
+
 		/* Set the RE bit periodically to "clean" the descriptor ring */
 		if (idpf_tx_splitq_need_re(tx_q)) {
 			tx_params.eop_cmd |= IDPF_TXD_FLEX_FLOW_CMD_RE;
diff --git a/drivers/net/ethernet/intel/idpf/idpf_txrx.h b/drivers/net/ethernet/intel/idpf/idpf_txrx.h
index 93547597efd2..fe0c913f9bb9 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_txrx.h
+++ b/drivers/net/ethernet/intel/idpf/idpf_txrx.h
@@ -161,6 +161,7 @@ union idpf_tx_flex_desc {
  * @tso_segs: Number of segments to be sent
  * @tso_hdr_len: Length of headers to be duplicated
  * @td_cmd: Command field to be inserted into descriptor
+ * @desc_ts: Flow scheduling offload timestamp
  */
 struct idpf_tx_offload_params {
 	u32 tx_flags;
@@ -174,6 +175,7 @@ struct idpf_tx_offload_params {
 	u16 tso_hdr_len;
 
 	u16 td_cmd;
+	u8 desc_ts[3];
 };
 
 /**
@@ -608,6 +610,7 @@ libeth_cacheline_set_assert(struct idpf_rx_queue,
  *	 hot path TX pointers stored in vport. Used in both singleq/splitq.
  * @desc_count: Number of descriptors
  * @tx_min_pkt_len: Min supported packet length
+ * @ts_gran_pow2: Txtime timestamp granularity in nanoseconds (log2).
  * @thresh: XDP queue cleaning threshold
  * @netdev: &net_device corresponding to this queue
  * @next_to_use: Next descriptor to use
@@ -666,7 +669,10 @@ struct idpf_tx_queue {
 	u16 desc_count;
 
 	union {
-		u16 tx_min_pkt_len;
+		struct {
+			u16 tx_min_pkt_len;
+			u8 ts_gran_pow2;
+		};
 		u32 thresh;
 	};
 
diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
index 1caf52706973..030ecd9b6c41 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
@@ -3,6 +3,7 @@
 
 #include <linux/export.h>
 #include <linux/net/intel/libie/pci.h>
+#include <linux/log2.h>
 #include <net/libeth/rx.h>
 
 #include "idpf.h"
@@ -553,7 +554,8 @@ static int idpf_send_get_caps_msg(struct idpf_adapter *adapter)
 			    VIRTCHNL2_CAP_SPLITQ_QSCHED		|
 			    VIRTCHNL2_CAP_PROMISC		|
 			    VIRTCHNL2_CAP_LOOPBACK		|
-			    VIRTCHNL2_CAP_PTP);
+			    VIRTCHNL2_CAP_PTP			|
+			    VIRTCHNL2_CAP_EDT);
 
 	err = idpf_send_mb_msg_stack(adapter, &xn_params, &caps);
 	if (err)
@@ -573,6 +575,54 @@ static int idpf_send_get_caps_msg(struct idpf_adapter *adapter)
 	return err;
 }
 
+/**
+ * idpf_send_get_edt_caps_msg - Send virtchnl get EDT caps msg
+ * @adapter: Driver specific private struct
+ *
+ * Return: 0 on success or error code on failure.
+ */
+static int idpf_send_get_edt_caps_msg(struct idpf_adapter *adapter)
+{
+	struct libie_ctlq_xn_send_params xn_params = {
+		.timeout_ms	= IDPF_VC_XN_DEFAULT_TIMEOUT_MSEC,
+		.chnl_opcode	= VIRTCHNL2_OP_GET_EDT_CAPS,
+	};
+	struct virtchnl2_edt_caps caps = {};
+	u64 gran_ns, horizon_ns;
+	int err;
+
+	err = idpf_send_mb_msg_stack(adapter, &xn_params, &caps);
+	if (err)
+		return err;
+
+	if (xn_params.recv_mem.iov_len < sizeof(caps)) {
+		err = -EIO;
+		goto free_rx_buf;
+	}
+
+	memcpy(&caps, xn_params.recv_mem.iov_base, sizeof(caps));
+	horizon_ns = le64_to_cpu(caps.time_horizon_ns);
+	gran_ns = le64_to_cpu(caps.tstamp_granularity_ns);
+	if (horizon_ns > U32_MAX) {
+		dev_warn(&adapter->pdev->dev, "EDT horizon exceeds U32\n");
+		err = -EINVAL;
+		goto free_rx_buf;
+	}
+	if (!gran_ns) {
+		dev_warn(&adapter->pdev->dev, "EDT granularity undefined\n");
+		err = -EINVAL;
+		goto free_rx_buf;
+	}
+
+	adapter->edt_caps.time_horizon_ns = horizon_ns;
+	adapter->edt_caps.tstamp_granularity_pow2 = ilog2(gran_ns);
+
+free_rx_buf:
+	libie_ctlq_release_rx_buf(&xn_params.recv_mem);
+
+	return err;
+}
+
 /**
  * idpf_mmio_region_non_static - Check if region is not static
  * @mmio_info: PCI resources info
@@ -3085,6 +3135,14 @@ int idpf_vc_core_init(struct idpf_adapter *adapter)
 		}
 	}
 
+	if (idpf_is_cap_ena(adapter, IDPF_OTHER_CAPS, VIRTCHNL2_CAP_EDT)) {
+		err = idpf_send_get_edt_caps_msg(adapter);
+		if (err) {
+			dev_err(&adapter->pdev->dev, "EDT init failed, err=%d\n", err);
+			memset(&adapter->edt_caps, 0, sizeof(adapter->edt_caps));
+		}
+	}
+
 	pci_sriov_set_totalvfs(adapter->pdev, idpf_get_max_vfs(adapter));
 	num_max_vports = idpf_get_max_vports(adapter);
 	adapter->vports = kzalloc_objs(*adapter->vports, num_max_vports);
-- 
2.55.0.970.g62bdec98f9-goog


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH net-next v8 4/6] selftests: drv-net: refactor so_txtime errqueue handling
  2026-09-02 18:17 [PATCH net-next v8 0/6] hardware pacing offload Willem de Bruijn
                   ` (2 preceding siblings ...)
  2026-09-02 18:17 ` [PATCH net-next v8 3/6] idpf: support pacing offload Willem de Bruijn
@ 2026-09-02 18:17 ` Willem de Bruijn
  2026-09-04  0:19   ` netdev-bot+sashiko
  2026-09-02 18:17 ` [PATCH net-next v8 5/6] selftests: drv-net: in so_txtime tell apart sw from hw pacing Willem de Bruijn
  2026-09-02 18:17 ` [PATCH net-next v8 6/6] selftests: drv-net: extend so_txtime with hw offload Willem de Bruijn
  5 siblings, 1 reply; 25+ messages in thread
From: Willem de Bruijn @ 2026-09-02 18:17 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, edumazet, pabeni, horms, andrew+netdev,
	Willem de Bruijn

From: Willem de Bruijn <willemb@google.com>

Prepare error queue handling for upcoming SO_EE_ORIGIN_TIMESTAMPING
messages in the next patch in this series.

Convert do_recv_errqueue_timeout into dispatcher do_recv_errqueue
and move SO_EE_ORIGIN_TXTIME specific code into a separate helper.

This will make the next patch a lot more readable.

No functional changes.

Signed-off-by: Willem de Bruijn <willemb@google.com>

--

Changes
  v3 -> v4
    - rename func to do_recv_errqueue_txtime
---
 .../testing/selftests/drivers/net/so_txtime.c | 66 ++++++++++---------
 1 file changed, 36 insertions(+), 30 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/so_txtime.c b/tools/testing/selftests/drivers/net/so_txtime.c
index 55a386f3d1b9..f6f3253ed174 100644
--- a/tools/testing/selftests/drivers/net/so_txtime.c
+++ b/tools/testing/selftests/drivers/net/so_txtime.c
@@ -158,7 +158,40 @@ static void do_recv_verify_empty(int fdr)
 		error(1, 0, "recv: not empty as expected (%d, %d)", ret, errno);
 }
 
-static int do_recv_errqueue_timeout(int fdt)
+static int do_recv_errqueue_txtime(struct sock_extended_err *err,
+				   const char payload_char)
+{
+	const char *reason = NULL;
+	int64_t tstamp = 0;
+
+	switch (err->ee_errno) {
+	case ECANCELED:
+		if (err->ee_code != SO_EE_CODE_TXTIME_MISSED)
+			error(1, 0, "errqueue: unknown ECANCELED %u\n",
+			      err->ee_code);
+		reason = "missed txtime";
+	break;
+	case EINVAL:
+		if (err->ee_code != SO_EE_CODE_TXTIME_INVALID_PARAM)
+			error(1, 0, "errqueue: unknown EINVAL %u\n",
+			      err->ee_code);
+		reason = "invalid txtime";
+	break;
+	default:
+		error(1, 0, "errqueue: errno %u code %u\n",
+		      err->ee_errno, err->ee_code);
+	}
+
+	tstamp = ((int64_t)err->ee_data) << 32 | err->ee_info;
+	tstamp -= (int64_t)glob_tstart;
+	tstamp /= 1000 * 1000;
+	fprintf(stderr, "send: pkt %c at %" PRId64 "ms dropped: %s\n",
+		payload_char, tstamp, reason);
+
+	return 1;
+}
+
+static int do_recv_errqueue(int fdt)
 {
 	char control[CMSG_SPACE(sizeof(struct sock_extended_err)) +
 		     CMSG_SPACE(sizeof(struct sockaddr_in6))] = {0};
@@ -169,7 +202,6 @@ static int do_recv_errqueue_timeout(int fdt)
 	struct msghdr msg = {0};
 	struct iovec iov = {0};
 	struct cmsghdr *cm;
-	int64_t tstamp = 0;
 
 	iov.iov_base = data;
 	iov.iov_len = sizeof(data);
@@ -181,8 +213,6 @@ static int do_recv_errqueue_timeout(int fdt)
 	msg.msg_controllen = sizeof(control);
 
 	while (1) {
-		const char *reason = NULL;
-
 		ret = recvmsg(fdt, &msg, MSG_ERRQUEUE);
 		if (ret == -1 && errno == EAGAIN)
 			break;
@@ -201,33 +231,9 @@ static int do_recv_errqueue_timeout(int fdt)
 		if (err->ee_origin != SO_EE_ORIGIN_TXTIME)
 			error(1, 0, "errqueue: origin 0x%x\n", err->ee_origin);
 
-		switch (err->ee_errno) {
-		case ECANCELED:
-			if (err->ee_code != SO_EE_CODE_TXTIME_MISSED)
-				error(1, 0, "errqueue: unknown ECANCELED %u\n",
-				      err->ee_code);
-			reason = "missed txtime";
-		break;
-		case EINVAL:
-			if (err->ee_code != SO_EE_CODE_TXTIME_INVALID_PARAM)
-				error(1, 0, "errqueue: unknown EINVAL %u\n",
-				      err->ee_code);
-			reason = "invalid txtime";
-		break;
-		default:
-			error(1, 0, "errqueue: errno %u code %u\n",
-			      err->ee_errno, err->ee_code);
-		}
-
-		tstamp = ((int64_t) err->ee_data) << 32 | err->ee_info;
-		tstamp -= (int64_t) glob_tstart;
-		tstamp /= 1000 * 1000;
-		fprintf(stderr, "send: pkt %c at %" PRId64 "ms dropped: %s\n",
-			data[ret - 1], tstamp, reason);
-
+		num_tstamp += do_recv_errqueue_txtime(err, data[ret - 1]);
 		msg.msg_flags = 0;
 		msg.msg_controllen = sizeof(control);
-		num_tstamp++;
 	}
 
 	return num_tstamp;
@@ -245,7 +251,7 @@ static void recv_errqueue_msgs(int fdt)
 			error(1, errno, "poll");
 
 		if (ret && (pfd.revents & POLLERR))
-			num_tstamp += do_recv_errqueue_timeout(fdt);
+			num_tstamp += do_recv_errqueue(fdt);
 
 		if (num_tstamp == cfg_num_pkt)
 			break;
-- 
2.55.0.970.g62bdec98f9-goog


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH net-next v8 5/6] selftests: drv-net: in so_txtime tell apart sw from hw pacing
  2026-09-02 18:17 [PATCH net-next v8 0/6] hardware pacing offload Willem de Bruijn
                   ` (3 preceding siblings ...)
  2026-09-02 18:17 ` [PATCH net-next v8 4/6] selftests: drv-net: refactor so_txtime errqueue handling Willem de Bruijn
@ 2026-09-02 18:17 ` Willem de Bruijn
  2026-09-04  0:19   ` netdev-bot+sashiko
  2026-09-02 18:17 ` [PATCH net-next v8 6/6] selftests: drv-net: extend so_txtime with hw offload Willem de Bruijn
  5 siblings, 1 reply; 25+ messages in thread
From: Willem de Bruijn @ 2026-09-02 18:17 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, edumazet, pabeni, horms, andrew+netdev,
	Willem de Bruijn

From: Willem de Bruijn <willemb@google.com>

Detect software pacing in so_txtime.c using SO_TIMESTAMPING.

If '-H' (hw) is passed

1. measure sw tx delay with SOF_TIMESTAMPING_TX_SOFTWARE, and
2. fail if delay exceeds a threshold, indicating pacing

Also extend while condition to account for possible variance.
This applies to all tests, not just the new '-H' variants.

Also reorder getopt parameters to make them alphabetical.

Signed-off-by: Willem de Bruijn <willemb@google.com>

---

Changes
  v6 -> v7
    - gate missing tstamp errors++ on !KSFT_MACHINE_SLOW
  v5 -> v6
    - initialize tdeliver_max to now (glob_tstart),
      as otherwise not set for -H tests without delay
  v3 -> v4
    - rename -h to -H to avoid aliasing implicit getopt -h
    - count sw timestamps to verify that they were actually checked
    - (minor) do not ask for SOF_TIMESTAMPING_OPT_ID as not used
    - (minor) remove unused parameter
---
 .../testing/selftests/drivers/net/so_txtime.c | 95 +++++++++++++++----
 1 file changed, 79 insertions(+), 16 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/so_txtime.c b/tools/testing/selftests/drivers/net/so_txtime.c
index f6f3253ed174..9ebe9f91227c 100644
--- a/tools/testing/selftests/drivers/net/so_txtime.c
+++ b/tools/testing/selftests/drivers/net/so_txtime.c
@@ -42,8 +42,10 @@ static bool	cfg_machine_slow;
 static uint64_t	cfg_start_time_ns;
 static int	cfg_mark;
 static bool	cfg_rx;
+static bool	cfg_verify_hw_offload;
 
 static uint64_t glob_tstart;
+static uint64_t glob_tstart_real;
 static uint64_t tdeliver_max;
 
 static int errors;
@@ -191,12 +193,32 @@ static int do_recv_errqueue_txtime(struct sock_extended_err *err,
 	return 1;
 }
 
-static int do_recv_errqueue(int fdt)
+static int do_recv_errqueue_timestamping(struct scm_timestamping *tss)
+{
+	int64_t ts;
+
+	ts = tss->ts[0].tv_sec * 1000ULL * 1000 * 1000;
+	ts += tss->ts[0].tv_nsec;
+	ts -= glob_tstart_real;
+	ts /= 1000;
+
+	if (ts > cfg_variance_us) {
+		fprintf(stderr, "sw delay %" PRId64 "us exceeds bounds\n", ts);
+		if (!cfg_machine_slow)
+			errors++;
+	}
+
+	return 1;
+}
+
+static int do_recv_errqueue(int fdt, int *num_ts)
 {
 	char control[CMSG_SPACE(sizeof(struct sock_extended_err)) +
+		     CMSG_SPACE(sizeof(struct scm_timestamping)) +
 		     CMSG_SPACE(sizeof(struct sockaddr_in6))] = {0};
 	char data[sizeof(struct ethhdr) + sizeof(struct ipv6hdr) +
 		  sizeof(struct udphdr) + 1];
+	struct scm_timestamping *tss;
 	struct sock_extended_err *err;
 	int ret, num_tstamp = 0;
 	struct msghdr msg = {0};
@@ -222,16 +244,29 @@ static int do_recv_errqueue(int fdt)
 			error(1, 0, "errqueue: flags 0x%x\n", msg.msg_flags);
 
 		cm = CMSG_FIRSTHDR(&msg);
+		tss = NULL;
+
+		if (cm->cmsg_level == SOL_SOCKET &&
+		    cm->cmsg_type == SCM_TIMESTAMPING) {
+			tss = (void *)CMSG_DATA(cm);
+			cm = CMSG_NXTHDR(&msg, cm);
+			if (!cm)
+				error(1, 0, "timestamp missing ip err\n");
+		}
+
 		if (cm->cmsg_level != cfg_errq_level ||
 		    cm->cmsg_type != cfg_errq_type)
 			error(1, 0, "errqueue: type 0x%x.0x%x\n",
 				    cm->cmsg_level, cm->cmsg_type);
 
 		err = (struct sock_extended_err *)CMSG_DATA(cm);
-		if (err->ee_origin != SO_EE_ORIGIN_TXTIME)
+		if (err->ee_origin == SO_EE_ORIGIN_TXTIME)
+			num_tstamp += do_recv_errqueue_txtime(err, data[ret - 1]);
+		else if (err->ee_origin == SO_EE_ORIGIN_TIMESTAMPING && tss)
+			*num_ts += do_recv_errqueue_timestamping(tss);
+		else
 			error(1, 0, "errqueue: origin 0x%x\n", err->ee_origin);
 
-		num_tstamp += do_recv_errqueue_txtime(err, data[ret - 1]);
 		msg.msg_flags = 0;
 		msg.msg_controllen = sizeof(control);
 	}
@@ -243,7 +278,7 @@ static void recv_errqueue_msgs(int fdt)
 {
 	struct pollfd pfd = { .fd = fdt, .events = POLLERR };
 	const int timeout_ms = 10;
-	int ret, num_tstamp = 0;
+	int ret, num_tstamp = 0, num_ts = 0;
 
 	do {
 		ret = poll(&pfd, 1, timeout_ms);
@@ -251,12 +286,20 @@ static void recv_errqueue_msgs(int fdt)
 			error(1, errno, "poll");
 
 		if (ret && (pfd.revents & POLLERR))
-			num_tstamp += do_recv_errqueue(fdt);
+			num_tstamp += do_recv_errqueue(fdt, &num_ts);
 
-		if (num_tstamp == cfg_num_pkt)
+		if (num_tstamp == cfg_num_pkt || num_ts == cfg_num_pkt)
 			break;
 
-	} while (gettime_ns(cfg_clockid) < tdeliver_max);
+	} while (gettime_ns(cfg_clockid) <
+		 tdeliver_max + (cfg_variance_us * 1000));
+
+	if (cfg_verify_hw_offload && num_ts != cfg_num_pkt) {
+		fprintf(stderr, "missing timestamps: expected %d, got %d\n",
+			cfg_num_pkt, num_ts);
+		if (!cfg_machine_slow)
+			errors++;
+	}
 }
 
 static void start_time_wait(void)
@@ -301,6 +344,17 @@ static void setsockopt_txtime(int fd)
 		error(1, 0, "getsockopt txtime: mismatch");
 }
 
+static void setsockopt_txtimestamping(int fd)
+{
+	int val = SOF_TIMESTAMPING_TX_SOFTWARE |
+		  SOF_TIMESTAMPING_SOFTWARE |
+		  SOF_TIMESTAMPING_OPT_TSONLY;
+
+	if (setsockopt(fd, SOL_SOCKET, SO_TIMESTAMPING,
+		       &val, sizeof(val)))
+		error(1, errno, "setsockopt timestamping");
+}
+
 static int setup_tx(struct sockaddr *addr, socklen_t alen)
 {
 	int fd;
@@ -314,6 +368,9 @@ static int setup_tx(struct sockaddr *addr, socklen_t alen)
 
 	setsockopt_txtime(fd);
 
+	if (cfg_verify_hw_offload)
+		setsockopt_txtimestamping(fd);
+
 	if (cfg_mark &&
 	    setsockopt(fd, SOL_SOCKET, SO_MARK, &cfg_mark, sizeof(cfg_mark)))
 		error(1, errno, "setsockopt mark");
@@ -354,6 +411,8 @@ static void do_test_tx(struct sockaddr *addr, socklen_t alen)
 
 	start_time_wait();
 	glob_tstart = gettime_ns(cfg_clockid);
+	glob_tstart_real = gettime_ns(CLOCK_REALTIME);
+	tdeliver_max = glob_tstart;
 
 	for (i = 0; i < cfg_num_pkt; i++)
 		do_send_one(fdt, &cfg_buf[i]);
@@ -446,10 +505,11 @@ static void usage(const char *progname)
 			"  -6            only IPv6\n"
 			"  -c <clock>    monotonic or tai (default)\n"
 			"  -D <addr>     destination IP address (server)\n"
-			"  -S <addr>     source IP address (client)\n"
+			"  -H            verify hardware offload (tx)\n"
+			"  -m <mark>     socket mark\n"
 			"  -r            run rx mode\n"
+			"  -S <addr>     source IP address (client)\n"
 			"  -t <nsec>     start time (UTC nanoseconds)\n"
-			"  -m <mark>     socket mark\n"
 			"\n",
 			progname);
 	exit(1);
@@ -461,7 +521,7 @@ static void parse_opts(int argc, char **argv)
 	int domain = PF_UNSPEC;
 	int c;
 
-	while ((c = getopt(argc, argv, "46c:S:D:rt:m:")) != -1) {
+	while ((c = getopt(argc, argv, "46c:D:Hm:rS:t:")) != -1) {
 		switch (c) {
 		case '4':
 			if (domain != PF_UNSPEC)
@@ -488,21 +548,24 @@ static void parse_opts(int argc, char **argv)
 			else
 				error(1, 0, "unknown clock id %s", optarg);
 			break;
-		case 'S':
-			saddr = optarg;
-			break;
 		case 'D':
 			daddr = optarg;
 			break;
+		case 'H':
+			cfg_verify_hw_offload = true;
+			break;
+		case 'm':
+			cfg_mark = strtol(optarg, NULL, 0);
+			break;
 		case 'r':
 			cfg_rx = true;
 			break;
+		case 'S':
+			saddr = optarg;
+			break;
 		case 't':
 			cfg_start_time_ns = strtoll(optarg, NULL, 0);
 			break;
-		case 'm':
-			cfg_mark = strtol(optarg, NULL, 0);
-			break;
 		default:
 			usage(argv[0]);
 		}
-- 
2.55.0.970.g62bdec98f9-goog


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH net-next v8 6/6] selftests: drv-net: extend so_txtime with hw offload
  2026-09-02 18:17 [PATCH net-next v8 0/6] hardware pacing offload Willem de Bruijn
                   ` (4 preceding siblings ...)
  2026-09-02 18:17 ` [PATCH net-next v8 5/6] selftests: drv-net: in so_txtime tell apart sw from hw pacing Willem de Bruijn
@ 2026-09-02 18:17 ` Willem de Bruijn
  2026-09-04  0:19   ` netdev-bot+sashiko
  5 siblings, 1 reply; 25+ messages in thread
From: Willem de Bruijn @ 2026-09-02 18:17 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, edumazet, pabeni, horms, andrew+netdev,
	Willem de Bruijn

From: Willem de Bruijn <willemb@google.com>

Add two pacing hardware offload variants

1. one that uses FQ to safely offload when within bounds.
2. one that uses pfifo_fast and thus forwards all packets.

Verify that the packets are paced in hardware with new flag '-H'.

Also increase rcvtimeout significantly to reduce flakiness. Especially
for the new beyond_hw_horizon test, which is close to the 100ms limit.
But update recv_verify_empty to take MSG_DONTWAIT. That last empty
check must not delay each testcase by the receive timeout.

Hardware pacing offload can complete packets out of order. So the
reverse_order test is expected to pass with pfifo_fast too.

Do not test ETF, which does not change its dequeue behavior based on
pacing_offload_horizon.

Signed-off-by: Willem de Bruijn <willemb@google.com>

---

Changes
  v7 -> v8
    - simplify "not a == b" test to "a != b" (ruff)
  v5 -> v6
    - do not suppress tx failures on expected failure:
      all expect the receiver process to signal failure
  v3 -> v4
    - replace ethtool with rtnetlink APIs
    - expect_fail: correctly handle negative test pfifofast beyond_hw_horizon,
      also when KSFT_MACHINE_SLOW suppresses timing errors
    - commit-msg: clarify that rcvtimeout increase is also needed for
      beyond_hw_horizon test
    - define the horizon (50ms) once, rather than three times
    - leave cfg.require_ipver in place
  v2 -> v3
    - remove drivers/net/settings timeout change: superseded by recent commit
    - add reverse_order comment
  v1 -> v2
    - re-raise NlError from e (patchwork pylint)
    - simplify expect_pass test (patchwork pylint)
---
 .../testing/selftests/drivers/net/so_txtime.c |  4 +-
 .../selftests/drivers/net/so_txtime.py        | 74 ++++++++++++++++++-
 2 files changed, 74 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/so_txtime.c b/tools/testing/selftests/drivers/net/so_txtime.c
index 9ebe9f91227c..5bb15498da30 100644
--- a/tools/testing/selftests/drivers/net/so_txtime.c
+++ b/tools/testing/selftests/drivers/net/so_txtime.c
@@ -155,7 +155,7 @@ static void do_recv_verify_empty(int fdr)
 	char rbuf[1];
 	int ret;
 
-	ret = recv(fdr, rbuf, sizeof(rbuf), 0);
+	ret = recv(fdr, rbuf, sizeof(rbuf), MSG_DONTWAIT);
 	if (ret != -1 || errno != EAGAIN)
 		error(1, 0, "recv: not empty as expected (%d, %d)", ret, errno);
 }
@@ -380,7 +380,7 @@ static int setup_tx(struct sockaddr *addr, socklen_t alen)
 
 static int setup_rx(struct sockaddr *addr, socklen_t alen)
 {
-	struct timeval tv = { .tv_usec = 100 * 1000 };
+	struct timeval tv = { .tv_usec = 600 * 1000 };
 	int fd;
 
 	fd = socket(addr->sa_family, SOCK_DGRAM, 0);
diff --git a/tools/testing/selftests/drivers/net/so_txtime.py b/tools/testing/selftests/drivers/net/so_txtime.py
index a097fae0b335..873d5fd6a8eb 100755
--- a/tools/testing/selftests/drivers/net/so_txtime.py
+++ b/tools/testing/selftests/drivers/net/so_txtime.py
@@ -12,8 +12,9 @@ import time
 from lib.py import ksft_exit, ksft_run, ksft_variants
 from lib.py import KsftNamedVariant, KsftSkipEx
 from lib.py import NetDrvEpEnv, bkg, cmd, defer, tc
-from lib.py import CmdExitFailure
+from lib.py import CmdExitFailure, RtnlFamily, NlError
 
+_HW_OFFLOAD_HORIZON_MS = 50
 
 def test_so_txtime(cfg, clockid, ipver, args_tx, args_rx, expect_success):
     """Main function. Run so_txtime as sender and receiver."""
@@ -33,12 +34,41 @@ def test_so_txtime(cfg, clockid, ipver, args_tx, args_rx, expect_success):
     expect_fail = not expect_success
     if slow_machine:
         expect_success = False
+        expect_fail = None
 
     with bkg(cmd_rx, host=cfg.remote, fail=expect_success,
              expect_fail=expect_fail, exit_wait=True):
         cmd(cmd_tx)
 
 
+def _dev_setup_pacing_offload(cfg):
+    """Configure pacing-offload-horizon."""
+    rtnl = RtnlFamily()
+
+    try:
+        link = rtnl.getlink({'ifi-index': cfg.ifindex})
+    except NlError as e:
+        raise KsftSkipEx('getlink not supported by device') from e
+
+    if 'pacing-offload-horizon' not in link or \
+       'max-pacing-offload-horizon' not in link:
+        raise KsftSkipEx('pacing offload horizon not supported by device')
+
+    horizon = _HW_OFFLOAD_HORIZON_MS * 1000_000
+    if link['max-pacing-offload-horizon'] < horizon:
+        raise KsftSkipEx('pacing offload max horizon too small')
+
+    cur_horizon = link['pacing-offload-horizon']
+    rtnl.setlink({
+        'ifi-index': cfg.ifindex,
+        'pacing-offload-horizon': horizon,
+    })
+    defer(rtnl.setlink, {
+        'ifi-index': cfg.ifindex,
+        'pacing-offload-horizon': cur_horizon
+    })
+
+
 def _qdisc_setup(ifname, qdisc, optargs=""):
     """Replace root qdisc. Restore the original after the test.
 
@@ -61,6 +91,7 @@ def _test_variants_fq():
             ["one_pkt", "a,10", "a,10"],
             ["in_order", "a,10,b,20", "a,10,b,20"],
             ["reverse_order", "a,20,b,10", "b,10,a,20"],
+            ["beyond_hw_horizon", "a,70", "a,70"],
         ]:
             name = f"v{ipver}_{testcase[0]}"
             yield KsftNamedVariant(name, ipver, testcase[1], testcase[2])
@@ -74,6 +105,39 @@ def test_so_txtime_fq_mono(cfg, ipver, args_tx, args_rx):
     test_so_txtime(cfg, "mono", ipver, args_tx, args_rx, True)
 
 
+@ksft_variants(_test_variants_fq())
+def test_so_txtime_fq_mono_hw(cfg, ipver, args_tx, args_rx):
+    """Run all variants of monotonic fq tests, with offload horizon."""
+    cfg.require_ipver(ipver)
+    cfg.require_nsim(nsim_test=False)
+
+    _dev_setup_pacing_offload(cfg)
+    try:
+        _qdisc_setup(cfg.ifname, "fq", f"offload_horizon {_HW_OFFLOAD_HORIZON_MS}ms")
+    except Exception as e:
+        raise KsftSkipEx("netdev does not support offload. skipping") from e
+
+    # Expect all tests to use only hw pacing, except beyond_hw_horizon.
+    # Do not pass -H to that test so that with sw pacing fallback it passes.
+    hw_only = "-H" if args_tx != "a,70" else ""
+    test_so_txtime(cfg, "mono", ipver, f"{hw_only} {args_tx}", args_rx, True)
+
+
+@ksft_variants(_test_variants_fq())
+def test_so_txtime_pfifofast_mono_hw(cfg, ipver, args_tx, args_rx):
+    """Run all variants of monotonic tests, without fq pacing sw backup."""
+    cfg.require_ipver(ipver)
+    cfg.require_nsim(nsim_test=False)
+
+    _dev_setup_pacing_offload(cfg)
+    _qdisc_setup(cfg.ifname, "pfifo_fast")
+
+    # Expect all tests to pass, except beyond_hw_horizon without sw fallback.
+    # It will send immediately, failing the receiver arrival bounds check.
+    expect_pass = args_tx != "a,70"
+    test_so_txtime(cfg, "mono", ipver, f"-H {args_tx}", args_rx, expect_pass)
+
+
 @ksft_variants(_test_variants_fq())
 def test_so_txtime_fq_tai(cfg, ipver, args_tx, args_rx):
     """Run all variants of fq tests, but pass CLOCK_TAI to test conversion."""
@@ -123,7 +187,13 @@ def main() -> None:
     """Boilerplate ksft main."""
     with NetDrvEpEnv(__file__) as cfg:
         ksft_run(
-            [test_so_txtime_fq_mono, test_so_txtime_fq_tai, test_so_txtime_etf],
+            [
+                test_so_txtime_fq_mono,
+                test_so_txtime_fq_mono_hw,
+                test_so_txtime_pfifofast_mono_hw,
+                test_so_txtime_fq_tai,
+                test_so_txtime_etf,
+            ],
             args=(cfg,),
         )
     ksft_exit()
-- 
2.55.0.970.g62bdec98f9-goog


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH net-next v8 1/6] net: rtnetlink: add pacing_offload_horizon attribute to net_device
  2026-09-02 18:17 ` [PATCH net-next v8 1/6] net: rtnetlink: add pacing_offload_horizon attribute to net_device Willem de Bruijn
@ 2026-09-04  0:19   ` netdev-bot+sashiko
  2026-09-04 23:01   ` Jakub Kicinski
  1 sibling, 0 replies; 25+ messages in thread
From: netdev-bot+sashiko @ 2026-09-04  0:19 UTC (permalink / raw)
  To: willemdebruijn.kernel
  Cc: netdev, davem, kuba, edumazet, pabeni, horms, andrew+netdev,
	willemb

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 1 · Low: 1

- [Medium] Stale cached bound: sch_fq latches its offload horizon in
  q->offload_horizon at configuration time, validated once against the…
- [Low] uAPI spec vs kernel policy mismatch: rt-link.yaml adds 'max-
  pacing-offload-horizon' to the '&link-all-attrs' anchor, and that same…

--

net: rtnetlink: add pacing_offload_horizon attribute to net_device

Adds a new writable net_device field pacing_offload_horizon, exposed as
IFLA_PACING_OFFLOAD_HORIZON and settable via RTM_SETLINK up to
dev->max_pacing_offload_horizon.  Both fields become u32, and sch_fq now
validates TCA_FQ_OFFLOAD_HORIZON against the new active horizon instead of
the device maximum.

> diff --git a/Documentation/netlink/specs/rt-link.yaml b/Documentation/netlink/specs/rt-link.yaml
> index 61ebb9a2bad5..2a2aad9a29c4 100644
> --- a/Documentation/netlink/specs/rt-link.yaml
> +++ b/Documentation/netlink/specs/rt-link.yaml

[ ... ]

> @@ -2557,6 +2561,8 @@ operations:
>              - devlink-port
>              - gso-ipv4-max-size
>              - gro-ipv4-max-size
> +            - max-pacing-offload-horizon
> +            - pacing-offload-horizon
>        dump:
>          request:
>            value: 18

[Severity: Low]
Should max-pacing-offload-horizon be added to this list?  The list this hunk
extends is the anchor definition in the getlink do-reply:

  Documentation/netlink/specs/rt-link.yaml
        reply:
          value: 16
          attributes: &link-all-attrs

and the same anchor is dereferenced as the setlink request list:

    -
      name: setlink
      doc: Set information about a link.
      attribute-set: link-attrs
      fixed-header: ifinfomsg
      do:
        request:
          value: 19
          attributes: *link-all-attrs

So the spec now declares the read-only maximum as settable, while the same
patch adds [IFLA_MAX_PACING_OFFLOAD_HORIZON] = { .type = NLA_REJECT } to
ifla_policy, which makes a ynl-generated setlink carrying that attribute fail
the whole message with -EINVAL.

The same mismatch already exists in this shared list for perm-address,
tso-max-size, tso-max-segs and allmulti, but the more recent read-only
additions (netns-immutable, headroom, tailroom) were deliberately left out of
it.  Would listing max-pacing-offload-horizon only in the read-only/reply set
be closer to the intent here?  pacing-offload-horizon itself is genuinely
settable, so only the max- attribute is affected.

> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index 81c5a6104dea..eeaf953b397f 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c

[ ... ]

> @@ -2344,9 +2347,11 @@ static const struct nla_policy ifla_policy[IFLA_MAX+1] = {
>  	[IFLA_ALLMULTI]		= { .type = NLA_REJECT },
>  	[IFLA_GSO_IPV4_MAX_SIZE]	= NLA_POLICY_MIN(NLA_U32, MAX_TCP_HEADER + 1),
>  	[IFLA_GRO_IPV4_MAX_SIZE]	= { .type = NLA_U32 },
> +	[IFLA_MAX_PACING_OFFLOAD_HORIZON] = { .type = NLA_REJECT },
>  	[IFLA_NETNS_IMMUTABLE]	= { .type = NLA_REJECT },
>  	[IFLA_HEADROOM]		= { .type = NLA_REJECT },
>  	[IFLA_TAILROOM]		= { .type = NLA_REJECT },
> +	[IFLA_PACING_OFFLOAD_HORIZON] = { .type = NLA_UINT },
>  };

[ ... ]

> @@ -3337,6 +3349,15 @@ static int do_setlink(const struct sk_buff *skb, struct net_device *dev,
>  		}
>  	}
>  
> +	if (tb[IFLA_PACING_OFFLOAD_HORIZON]) {
> +		u64 horizon = nla_get_uint(tb[IFLA_PACING_OFFLOAD_HORIZON]);
> +
> +		if (dev->pacing_offload_horizon ^ horizon) {
> +			WRITE_ONCE(dev->pacing_offload_horizon, horizon);
> +			status |= DO_SETLINK_MODIFIED;
> +		}
> +	}
> +
>  	if (tb[IFLA_OPERSTATE])
>  		set_operstate(dev, nla_get_u8(tb[IFLA_OPERSTATE]));

[Severity: Medium]
What happens to an already configured fq qdisc when this write lowers the
device horizon, including back to 0?

sch_fq latches its horizon once in fq_change() (see the sch_fq.c hunk quoted
below) and then uses the cached copy in the datapath:

net/sched/sch_fq.c:fq_fastpath_check() {
	if (fq_skb_cb(skb)->time_to_send > now + q->offload_horizon)
		return false;
	...
}

net/sched/sch_fq.c:fq_dequeue() {
	...
	if (now + q->offload_horizon < time_next_packet) {
	...
}

do_setlink() does not walk the device's qdiscs and does not clamp
q->offload_horizon, and sch_fq registers no netdevice notifier (only
act_mirred, sch_cbs and sch_taprio do), so nothing re-validates the cached
value.

Does fq then keep releasing packets up to the old horizon ahead of their EDT
while the device no longer paces them?  With the idpf patch from this series
applied:

drivers/net/ethernet/intel/idpf/idpf_txrx.c:idpf_tx_splitq_set_txtime() {
	horizon = READ_ONCE(skb->dev->pacing_offload_horizon);
	if (!horizon)
		return;
	...
}

no EDT is programmed once the horizon is 0, so packets handed over early
appear to be sent immediately until userspace also reconfigures the qdisc.

For comparison, netif_set_tso_max_size() clamps dev->gso_max_size when the
device maximum shrinks.  Would something similar, or a notifier, be
appropriate here?

Both knobs require CAP_NET_ADMIN, so this is a configuration-ordering issue
rather than something an unprivileged user can trigger.

> diff --git a/net/sched/sch_fq.c b/net/sched/sch_fq.c
> index 6144b5686f13..7cf7afda699f 100644
> --- a/net/sched/sch_fq.c
> +++ b/net/sched/sch_fq.c
> @@ -1183,7 +1183,8 @@ static int fq_change(struct Qdisc *sch, struct nlattr *opt,
>  		u64 offload_horizon = (u64)NSEC_PER_USEC *
>  				      nla_get_u32(tb[TCA_FQ_OFFLOAD_HORIZON]);
>  
> -		if (offload_horizon <= qdisc_dev(sch)->max_pacing_offload_horizon) {
> +		if (offload_horizon <=
> +		    READ_ONCE(qdisc_dev(sch)->pacing_offload_horizon)) {
>  			WRITE_ONCE(q->offload_horizon, offload_horizon);
>  		} else {
>  			NL_SET_ERR_MSG_MOD(extack, "invalid offload_horizon");

Before this patch the bound compared here had no in-tree writer, so an
accepted value could never become invalid afterwards.  Is the "invalid
offload_horizon" invariant still enforced once the bound is user-mutable?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260902181747.2483351-1-willemdebruijn.kernel%40gmail.com

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH net-next v8 3/6] idpf: support pacing offload
  2026-09-02 18:17 ` [PATCH net-next v8 3/6] idpf: support pacing offload Willem de Bruijn
@ 2026-09-04  0:19   ` netdev-bot+sashiko
  2026-09-04 15:22     ` Willem de Bruijn
  2026-09-07  9:30   ` Loktionov, Aleksandr
  1 sibling, 1 reply; 25+ messages in thread
From: netdev-bot+sashiko @ 2026-09-04  0:19 UTC (permalink / raw)
  To: willemdebruijn.kernel
  Cc: netdev, davem, kuba, edumazet, pabeni, horms, andrew+netdev,
	willemb

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 1 · Low: 0

- [Medium] Divergent EDT state across a hard reset: the netdev-side
  pacing horizon survives while the adapter-side timestamp granularity…

--

idpf: support pacing offload

Adds EDT (earliest departure time) support to idpf: the txtime from
skb->tstamp is bounds checked and programmed into the flow scheduling Tx
descriptor. The feature is negotiated with the device through a new
VIRTCHNL2_OP_GET_EDT_CAPS exchange, and the resulting horizon is exposed
as netdev->max_pacing_offload_horizon.

> diff --git a/drivers/net/ethernet/intel/idpf/idpf_lib.c b/drivers/net/ethernet/intel/idpf/idpf_lib.c
> index 827c795afcb6..c2ad3fdf72aa 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_lib.c
> +++ b/drivers/net/ethernet/intel/idpf/idpf_lib.c
> @@ -890,6 +890,12 @@ static int idpf_cfg_netdev(struct idpf_vport *vport)
>  	netdev->min_mtu = ETH_MIN_MTU;
>  	netdev->max_mtu = vport->max_mtu;
>  
> +	if (adapter->edt_caps.time_horizon_ns &&
> +	    idpf_is_queue_model_split(vport->dflt_qv_rsrc.txq_model) &&
> +	    !idpf_is_cap_ena(adapter, IDPF_OTHER_CAPS,
> +			     VIRTCHNL2_CAP_SPLITQ_QSCHED))
> +		netdev->max_pacing_offload_horizon = adapter->edt_caps.time_horizon_ns;
> +
>  	dflt_features = NETIF_F_SG	|
>  			NETIF_F_HIGHDMA;
>  
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_txrx.c b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
> index 24b91be25676..d80b6f5a0fde 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_txrx.c
> +++ b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
> @@ -1742,6 +1742,7 @@ static int idpf_txq_group_alloc(struct idpf_vport *vport,
>  			q->desc_count = rsrc->txq_desc_count;
>  			q->tx_max_bufs = idpf_get_max_tx_bufs(adapter);
>  			q->tx_min_pkt_len = idpf_get_min_tx_pkt_len(adapter);
> +			q->ts_gran_pow2 = adapter->edt_caps.tstamp_granularity_pow2;
>  			q->netdev = vport->netdev;
>  			q->txq_grp = tx_qgrp;
>  			q->rel_q_id = j;

[ ... ]

> @@ -3011,6 +3017,60 @@ static bool idpf_tx_splitq_need_re(struct idpf_tx_queue *tx_q)
>  	return gap >= IDPF_TX_SPLITQ_RE_MIN_GAP;
>  }
>  
> +static void idpf_tx_splitq_set_txtime(const struct sk_buff *skb,
> +				      const struct idpf_tx_queue *tx_q,
> +				      struct idpf_tx_splitq_params *tx_params)
> +{
> +	const int offload_slack_ns = 400;
> +	u64 ts, now, horizon;
> +
> +	horizon = READ_ONCE(skb->dev->pacing_offload_horizon);
> +	if (!horizon)
> +		return;
> +
[ ... ]
> +	ts >>= tx_q->ts_gran_pow2;
> +
> +	/* 0 is valid 23b timestamp, but also means field unset.
> +	 * Increase by one to avoid this case
> +	 */
> +	if ((ts & 0x7fffff) == 0) {
> +		tx_params->offload.desc_ts[0] = 1;
> +		return;
> +	}
> +
> +	tx_params->offload.desc_ts[0] = ts & 0xff;
> +	tx_params->offload.desc_ts[1] = (ts >> 8) & 0xff;
> +	tx_params->offload.desc_ts[2] = ((ts >> 16) & 0x7f);
> +}
> +

[ ... ]

> diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
> index 1caf52706973..030ecd9b6c41 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
> +++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
> @@ -573,6 +575,54 @@ static int idpf_send_get_caps_msg(struct idpf_adapter *adapter)
>  	return err;
>  }
>  
[ ... ]
> +	if (xn_params.recv_mem.iov_len < sizeof(caps)) {
> +		err = -EIO;
> +		goto free_rx_buf;
> +	}
> +
> +	memcpy(&caps, xn_params.recv_mem.iov_base, sizeof(caps));
> +	horizon_ns = le64_to_cpu(caps.time_horizon_ns);
> +	gran_ns = le64_to_cpu(caps.tstamp_granularity_ns);
> +	if (horizon_ns > U32_MAX) {
> +		dev_warn(&adapter->pdev->dev, "EDT horizon exceeds U32\n");
> +		err = -EINVAL;
> +		goto free_rx_buf;
> +	}
> +	if (!gran_ns) {
> +		dev_warn(&adapter->pdev->dev, "EDT granularity undefined\n");
> +		err = -EINVAL;
> +		goto free_rx_buf;
> +	}
> +
> +	adapter->edt_caps.time_horizon_ns = horizon_ns;
> +	adapter->edt_caps.tstamp_granularity_pow2 = ilog2(gran_ns);
> +
> +free_rx_buf:
> +	libie_ctlq_release_rx_buf(&xn_params.recv_mem);
> +
> +	return err;
> +}
> +
[ ... ]
> @@ -3085,6 +3135,14 @@ int idpf_vc_core_init(struct idpf_adapter *adapter)
>  		}
>  	}
>  
> +	if (idpf_is_cap_ena(adapter, IDPF_OTHER_CAPS, VIRTCHNL2_CAP_EDT)) {
> +		err = idpf_send_get_edt_caps_msg(adapter);
> +		if (err) {
> +			dev_err(&adapter->pdev->dev, "EDT init failed, err=%d\n", err);
> +			memset(&adapter->edt_caps, 0, sizeof(adapter->edt_caps));
> +		}
> +	}
> +

[Severity: Medium]
When this re-negotiation fails on a reset, edt_caps is zeroed and init
continues, but nothing revokes the pacing horizon already published on the
netdev.  Can that leave the two halves of the EDT state inconsistent?

idpf_init_hard_reset() calls idpf_vc_core_init() again on the same adapter
while the netdevs stay registered (IDPF_VPORT_REG_NETDEV is only cleared in
idpf_decfg_netdev(), on driver remove), so idpf_cfg_netdev() takes its early
return:

	if (test_bit(IDPF_VPORT_REG_NETDEV, vport_config->flags)) {
		netdev = adapter->netdevs[idx];
		...
		return idpf_init_mac_addr(vport, netdev);
	}

and never re-evaluates the new assignment:

	if (adapter->edt_caps.time_horizon_ns && ...)
		netdev->max_pacing_offload_horizon = adapter->edt_caps.time_horizon_ns;

The dev->pacing_offload_horizon value the admin set earlier through
RTM_SETLINK also survives, since it is only validated at set time.

The re-created Tx queues do pick up the zeroed granularity in
idpf_txq_group_alloc():

	q->ts_gran_pow2 = adapter->edt_caps.tstamp_granularity_pow2;

and idpf_tx_splitq_set_txtime() gates only on the netdev horizon:

	horizon = READ_ONCE(skb->dev->pacing_offload_horizon);
	if (!horizon)
		return;
	...
	ts >>= tx_q->ts_gran_pow2;

With ts_gran_pow2 of 0 the shift is a no-op, so the low 23 bits of a raw
CLOCK_TAI nanosecond value end up in the descriptor ts[] field while the
device still interprets that field in its own unchanged granularity.  Would
departure times then land on effectively arbitrary points inside the hardware
window (up to roughly 4.29 s), holding a descriptor and buf_id each, after
only the single dev_err above?

Note that granularity 0 is a value the driver itself rejects from firmware
("EDT granularity undefined"), so it is being used as an invalid state here.

Would it be reasonable to clear dev->pacing_offload_horizon and
max_pacing_offload_horizon on this failure path, or to have
idpf_tx_splitq_set_txtime() also check that tx_q->ts_gran_pow2 is valid?

One more thing on this part of the commit message:

    On device reset, dev->pacing_offload_horizon, fq offload_horizon and
    granularity are not re-negotiated. It is safe to assume that firmware
    does not change these EDT capabilities across resets.

idpf_vc_core_init() runs again on every hard reset and does re-issue
VIRTCHNL2_OP_GET_EDT_CAPS, so granularity is re-negotiated.  Could the
wording be adjusted to match what the code does?

>  	pci_sriov_set_totalvfs(adapter->pdev, idpf_get_max_vfs(adapter));
>  	num_max_vports = idpf_get_max_vports(adapter);
>  	adapter->vports = kzalloc_objs(*adapter->vports, num_max_vports);

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260902181747.2483351-1-willemdebruijn.kernel%40gmail.com

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH net-next v8 4/6] selftests: drv-net: refactor so_txtime errqueue handling
  2026-09-02 18:17 ` [PATCH net-next v8 4/6] selftests: drv-net: refactor so_txtime errqueue handling Willem de Bruijn
@ 2026-09-04  0:19   ` netdev-bot+sashiko
  0 siblings, 0 replies; 25+ messages in thread
From: netdev-bot+sashiko @ 2026-09-04  0:19 UTC (permalink / raw)
  To: willemdebruijn.kernel
  Cc: netdev, davem, kuba, edumazet, pabeni, horms, andrew+netdev,
	willemb

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 0 · Low: 1

- [Low] Commit message hygiene: the per-revision changelog block of
  commit "selftests: drv-net: refactor so_txtime errqueue handling"…

--

selftests: drv-net: refactor so_txtime errqueue handling

This patch splits do_recv_errqueue_timeout() in
tools/testing/selftests/drivers/net/so_txtime.c into a dispatcher
do_recv_errqueue() plus a new helper do_recv_errqueue_txtime() that keeps the
SO_EE_ORIGIN_TXTIME specific handling.  It is preparation for handling
SO_EE_ORIGIN_TIMESTAMPING messages in the following patch, with no functional
change intended.

The only comment below concerns the recorded commit message, so the diff hunks
are omitted.

> Signed-off-by: Willem de Bruijn <willemb@google.com>
>
> --
>
> Changes
>   v3 -> v4
>     - rename func to do_recv_errqueue_txtime

[Severity: Low]
This isn't a code bug, but should the per-revision changelog be separated by
a three-dash line instead of "--"?

git mailinfo only truncates the message at a line of exactly three dashes or
at an explicit scissors marker, so a "--" line is kept as ordinary body text.
The recorded commit for d88dea26ea55958d0982df4811107d0743ccf08d ends with:

    Signed-off-by: Willem de Bruijn <willemb@google.com>

    --

    Changes
      v3 -> v4
        - rename func to do_recv_errqueue_txtime

That leaves the changelog permanently in the git history, and it sits after
the Signed-off-by trailer, so the trailer block is no longer the last section
of the message.

Could this be resent with "---" before the changelog block, or with the block
dropped?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260902181747.2483351-1-willemdebruijn.kernel%40gmail.com

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH net-next v8 5/6] selftests: drv-net: in so_txtime tell apart sw from hw pacing
  2026-09-02 18:17 ` [PATCH net-next v8 5/6] selftests: drv-net: in so_txtime tell apart sw from hw pacing Willem de Bruijn
@ 2026-09-04  0:19   ` netdev-bot+sashiko
  0 siblings, 0 replies; 25+ messages in thread
From: netdev-bot+sashiko @ 2026-09-04  0:19 UTC (permalink / raw)
  To: willemdebruijn.kernel
  Cc: netdev, davem, kuba, edumazet, pabeni, horms, andrew+netdev,
	willemb

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 0 · Low: 1

- [Low] Claim: the new '-H' mode (cfg_verify_hw_offload) added to
  tools/testing/selftests/drivers/net/so_txtime.c is dead code because…

--

selftests: drv-net: in so_txtime tell apart sw from hw pacing

This adds an optional '-H' mode to so_txtime.c that enables
SO_TIMESTAMPING with SOF_TIMESTAMPING_TX_SOFTWARE, measures the software
transmit delay reported on the error queue, and flags software pacing when
the delay is too large or timestamps are missing. It also extends the error
queue receive deadline by cfg_variance_us and reorders the getopt string
and usage text alphabetically.

No regressions to report on this one.

The one thing that came up during review was whether the new '-H' path is
reachable at this point in the series:

> diff --git a/tools/testing/selftests/drivers/net/so_txtime.c b/tools/testing/selftests/drivers/net/so_txtime.c
> index f6f3253ed174..9ebe9f91227c 100644
> --- a/tools/testing/selftests/drivers/net/so_txtime.c
> +++ b/tools/testing/selftests/drivers/net/so_txtime.c

[ ... ]

> @@ -251,12 +286,20 @@ static void recv_errqueue_msgs(int fdt)
> +	if (cfg_verify_hw_offload && num_ts != cfg_num_pkt) {
> +		fprintf(stderr, "missing timestamps: expected %d, got %d\n",
> +			cfg_num_pkt, num_ts);
> +		if (!cfg_machine_slow)
> +			errors++;
> +	}
>  }

[ ... ]

> @@ -488,21 +548,24 @@ static void parse_opts(int argc, char **argv)
> +		case 'H':
> +			cfg_verify_hw_offload = true;
> +			break;

Nothing in the tree passes '-H' as of this commit, so setsockopt_txtimestamping(),
do_recv_errqueue_timestamping() and the new "missing timestamps" check in
recv_errqueue_msgs() are not exercised yet. That is resolved later in the same
series: the following patch adds test_so_txtime_fq_mono_hw() and
test_so_txtime_pfifofast_mono_hw() to so_txtime.py, both of which pass '-H' in
args_tx and are registered in main()'s ksft_run() list, and args_tx is placed
before the payload operand in cmd_tx so getopt() picks the flag up. So no
action needed here.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260902181747.2483351-1-willemdebruijn.kernel%40gmail.com

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH net-next v8 6/6] selftests: drv-net: extend so_txtime with hw offload
  2026-09-02 18:17 ` [PATCH net-next v8 6/6] selftests: drv-net: extend so_txtime with hw offload Willem de Bruijn
@ 2026-09-04  0:19   ` netdev-bot+sashiko
  0 siblings, 0 replies; 25+ messages in thread
From: netdev-bot+sashiko @ 2026-09-04  0:19 UTC (permalink / raw)
  To: willemdebruijn.kernel
  Cc: netdev, davem, kuba, edumazet, pabeni, horms, andrew+netdev,
	willemb

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 1 · Low: 0

- [Medium] In `test_so_txtime()`
  (tools/testing/selftests/drivers/net/so_txtime.py:34-37) the patch…

--

selftests: drv-net: extend so_txtime with hw offload

Adds two new so_txtime.py variants that exercise EDT pacing offload, one
on top of fq with offload_horizon and one on top of pfifo_fast, together
with a new beyond_hw_horizon testcase. Also raises the receiver socket
timeout to 600ms and makes the final emptiness check non-blocking.

> diff --git a/tools/testing/selftests/drivers/net/so_txtime.py b/tools/testing/selftests/drivers/net/so_txtime.py
> index a097fae0b335..873d5fd6a8eb 100755
> --- a/tools/testing/selftests/drivers/net/so_txtime.py
> +++ b/tools/testing/selftests/drivers/net/so_txtime.py

[ ... ]

> @@ -33,12 +34,41 @@ def test_so_txtime(cfg, clockid, ipver, args_tx, args_rx, expect_success):
>      expect_fail = not expect_success
>      if slow_machine:
>          expect_success = False
> +        expect_fail = None
>  
>      with bkg(cmd_rx, host=cfg.remote, fail=expect_success,
>               expect_fail=expect_fail, exit_wait=True):
>          cmd(cmd_tx)

[Severity: Medium]

Is this new expect_fail = None intended to apply to every caller of
test_so_txtime(), or only to the new pfifo_fast beyond_hw_horizon case?

With slow_machine set, both fail and expect_fail become falsy, and
cmd.process() in tools/testing/selftests/net/lib/py/utils.py then takes
neither branch:

    # Fail on unexpected test failure if fail.
    # Fail on unexpected test success if expect_fail.
    # Fail on negative returncode if either:
    # Set by subprocess on crash or signal, this is never expected failure.
    if (self.proc.returncode != 0 and fail or
        (self.proc.returncode < 0 and expect_fail)):
        ...
        raise CmdExitFailure("Command failed", self)
    elif self.proc.returncode == 0 and expect_fail:
        raise CmdExitZeroFailure("Command succeeded (expected fail)", self)

So the remote receiver's exit status is not checked at all, including the
negative returncode from a crash or signal that the comment above says is
never an expected failure.

Does this silently drop the only assertion the pre-existing ETF negative
variants have? test_so_txtime_etf() passes expect_fail through for:

    ["no_delay", "a,-1", "a,-1", False],
    ["zero_delay", "a,0", "a,0", False],

Those failures are deterministic rather than timing related: sch_etf drops
the packet in is_packet_valid(), and the receiver then hits the
error(1, EAGAIN, "recv: timeout") path in do_recv_one(), which is not
guarded by cfg_machine_slow. Previously expect_success was cleared but
expect_fail stayed True, so a receiver that unexpectedly succeeded was
still reported. Would clearing expect_fail per-variant, or only for the
new hw variants, keep that coverage?

> +@ksft_variants(_test_variants_fq())
> +def test_so_txtime_fq_mono_hw(cfg, ipver, args_tx, args_rx):
> +    """Run all variants of monotonic fq tests, with offload horizon."""
> +    cfg.require_ipver(ipver)
> +    cfg.require_nsim(nsim_test=False)
> +
> +    _dev_setup_pacing_offload(cfg)
> +    try:
> +        _qdisc_setup(cfg.ifname, "fq", f"offload_horizon {_HW_OFFLOAD_HORIZON_MS}ms")
> +    except Exception as e:
> +        raise KsftSkipEx("netdev does not support offload. skipping") from e
> +
> +    # Expect all tests to use only hw pacing, except beyond_hw_horizon.
> +    # Do not pass -H to that test so that with sw pacing fallback it passes.
> +    hw_only = "-H" if args_tx != "a,70" else ""
> +    test_so_txtime(cfg, "mono", ipver, f"{hw_only} {args_tx}", args_rx, True)
> +
> +
> +@ksft_variants(_test_variants_fq())
> +def test_so_txtime_pfifofast_mono_hw(cfg, ipver, args_tx, args_rx):
> +    """Run all variants of monotonic tests, without fq pacing sw backup."""
> +    cfg.require_ipver(ipver)
> +    cfg.require_nsim(nsim_test=False)
> +
> +    _dev_setup_pacing_offload(cfg)
> +    _qdisc_setup(cfg.ifname, "pfifo_fast")
> +
> +    # Expect all tests to pass, except beyond_hw_horizon without sw fallback.
> +    # It will send immediately, failing the receiver arrival bounds check.
> +    expect_pass = args_tx != "a,70"
> +    test_so_txtime(cfg, "mono", ipver, f"-H {args_tx}", args_rx, expect_pass)

On the positive side of these new tests, do they still verify anything on
a KSFT_MACHINE_SLOW run? Both checks the -H flag relies on skip the error
count when cfg_machine_slow is set:

tools/testing/selftests/drivers/net/so_txtime.c:do_recv_one() {
	if (llabs(tstop - texpect) > cfg_variance_us) {
		fprintf(stderr, "exceeds variance (%d us)\n", cfg_variance_us);
		if (!cfg_machine_slow)
			errors++;
	}
}

tools/testing/selftests/drivers/net/so_txtime.c:recv_errqueue_msgs() {
	if (cfg_verify_hw_offload && num_ts != cfg_num_pkt) {
		fprintf(stderr, "missing timestamps: expected %d, got %d\n",
			cfg_num_pkt, num_ts);
		if (!cfg_machine_slow)
			errors++;
	}
}

Combined with expect_fail = None, would test_so_txtime_fq_mono_hw and
test_so_txtime_pfifofast_mono_hw report pass on such a machine even if the
device performs no EDT pacing at all?

One more note: the reason for relaxing the expectation (the timing
sensitive pfifo_fast beyond_hw_horizon case) appears only in the changelog
below the --- line. Could that rationale move into the commit message?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260902181747.2483351-1-willemdebruijn.kernel%40gmail.com

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH net-next v8 3/6] idpf: support pacing offload
  2026-09-04  0:19   ` netdev-bot+sashiko
@ 2026-09-04 15:22     ` Willem de Bruijn
  0 siblings, 0 replies; 25+ messages in thread
From: Willem de Bruijn @ 2026-09-04 15:22 UTC (permalink / raw)
  To: netdev-bot+sashiko, willemdebruijn.kernel
  Cc: netdev, davem, kuba, edumazet, pabeni, horms, andrew+netdev,
	willemb

netdev-bot+sashiko@ wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider:
> 
> Critical: 0 · High: 0 · Medium: 1 · Low: 0
> 
> - [Medium] Divergent EDT state across a hard reset: the netdev-side
>   pacing horizon survives while the adapter-side timestamp granularity…

The main AI feedback to this series continues to be that the
offload_horizon can remain set after a device reset would modify the
max_offload_horizon. That is mentioned as a known caveat in the
commit message. But indeed we can do better.

For v9

- I will reset the device offload_horizon here as suggested:
  Disable offload_horizon if the new max_offload_horizon is lower.

- Am considering a device notifier in FQ to do the same for
  q->offload_horizon.

Having both a configurable dev->offload_horizon and q->offload_horizon
is perhaps overkill. But we need the new dev one to safely roll out
the idpf feature default off. We cannot rely on FQ alone, as there are
other paths to pass packets with txtime to the driver: SO_TXTIME, BPF,
ETF.

For the other feedback, also for v9:

- Will restrict the 'expect_fail = None' to the new tests that need it.

- Will address the few other low severity points.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH net-next v8 1/6] net: rtnetlink: add pacing_offload_horizon attribute to net_device
  2026-09-02 18:17 ` [PATCH net-next v8 1/6] net: rtnetlink: add pacing_offload_horizon attribute to net_device Willem de Bruijn
  2026-09-04  0:19   ` netdev-bot+sashiko
@ 2026-09-04 23:01   ` Jakub Kicinski
  2026-09-06  2:22     ` Willem de Bruijn
  1 sibling, 1 reply; 25+ messages in thread
From: Jakub Kicinski @ 2026-09-04 23:01 UTC (permalink / raw)
  To: Willem de Bruijn
  Cc: netdev, davem, edumazet, pabeni, horms, andrew+netdev,
	Willem de Bruijn

Swapping in the conversation from v6, sorry, not sure why I missed your
reply..

On Mon, 17 Aug 2026 22:44:30 -0400 Willem de Bruijn wrote:
> Jakub Kicinski wrote:
> > On Wed, 12 Aug 2026 22:03:56 -0400 Willem de Bruijn wrote:  
> > > The 'max_pacing_offload_horizon' field of 'struct net_device' represents
> > > the maximum pacing offload horizon supported by the device.
> > > 
> > > Add a new field 'pacing_offload_horizon' to store the active pacing
> > > offload horizon.
> > > 
> > > The new attribute is initialized to 0 (disabled) and can be set from
> > > userspace via RTM_SETLINK up to dev->max_pacing_offload_horizon. This
> > > new default off behavior does not cause regressions, as no driver yet
> > > advertises max_pacing_offload_horizon.
> > > 
> > > The attribute is omitted from the newlink request spec, because the
> > > value may need to be bound by a device maximum that first needs to be
> > > negotiated with firmware, as is the case for the idpf driver in this
> > > series.
> > > 
> > > Make both fields u32, to maintain net_device cacheline layout. This
> > > expresses up to 4s of pacing offload, which is sufficient.
> > > 
> > > Update the YNL specification ('rt-link.yaml') to add the
> > > 'pacing-offload-horizon' attribute and include it in link-all-attrs.  
> > 
> > Forgive my slowness but I don't get how the new param squares against
> > TCA_FQ_OFFLOAD_HORIZON. IIRC in v5 review I asked something like "should 
> > this new option be a boolean" because the exact time horizon already
> > exists in the qdisc uAPI. As AI points out (among other things),
> > the two params are not synced in anyway. User can configure qdisc
> > offload higher than the device level one.  
> 
> They cannot. Or at least that sure is the intent.
> 
> After this patch fq tests against active limit
> dev->pacing_offload_horizon:
> 
> -               if (offload_horizon <= qdisc_dev(sch)->max_pacing_offload_horizon) {
> +               if (offload_horizon <=
> +                   READ_ONCE(qdisc_dev(sch)->pacing_offload_horizon)) {
>                         WRITE_ONCE(q->offload_horizon, offload_horizon);
> 
> A manual test to replace the root qdisc with fq offload_horizon 50ms
> seems to verify this: the command fails unless a device limit of >= 50ms
> is configured.

The other way around. Configure the Qdisc and device to horizon of 100ms
Then lower the device horizon to 50ms. Now the qdisc has a longer horizon
than the device.

> Perhaps I don't understand how dev->pacing_offload_horizon
> would function as a boolean.

The only uses of the new value are:
 - as the qdisc bound, replacing the max_ value
   -> Leave the qdisc as is, let qdisc config define the active horizon
 - in the driver

+static void idpf_tx_splitq_set_txtime(const struct sk_buff *skb,
+				      const struct idpf_tx_queue *tx_q,
+				      struct idpf_tx_splitq_params *tx_params)
+{
+	const int offload_slack_ns = 400;
+	u64 ts, now, horizon;
+
+	horizon = READ_ONCE(skb->dev->pacing_offload_horizon);
+	if (!horizon)
+		return;

   -> which already functions as a boolean, hence my suggestion of boolean

> > In fact any non-zero value of
> > the device one acts the same - hence the bool question.
> >
> >
> > Why do we need both? How are you going to use this new knob?
> > The commit msg explains the what not the why.
> > 
> > My naive understanding is that the main missing piece is a handshake
> > between the driver and qdisc to tell the driver that the qdisc is
> > indeed offloading pacing on queue X. And therefore the driver should
> > pay attention to the timestamps. This does not require uAPI changes.
> >   
> > >  python3 tools/net/ynl/pyynl/cli.py \  
> > 
> > uber-nit: python3 tools/net/ynl/pyynl/cli.py -> ynl
> > (the CLI is named ynl when packaged for end users)  
> 
> Should this also then point to the (default) installed spec path:
> 
>        ynl --spec /usr/local/share/ynl/specs/rt-link.yaml 

Use:

  ynl --family rt-link

The expectation is that the person copy/pasting from the commit message
already has the kernel and user space updated to include your changes.
Also it's easier to read the shorter format..


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH net-next v8 1/6] net: rtnetlink: add pacing_offload_horizon attribute to net_device
  2026-09-04 23:01   ` Jakub Kicinski
@ 2026-09-06  2:22     ` Willem de Bruijn
  2026-09-07 23:12       ` Jakub Kicinski
  0 siblings, 1 reply; 25+ messages in thread
From: Willem de Bruijn @ 2026-09-06  2:22 UTC (permalink / raw)
  To: Jakub Kicinski, Willem de Bruijn
  Cc: netdev, davem, edumazet, pabeni, horms, andrew+netdev,
	Willem de Bruijn

Jakub Kicinski wrote:
> Swapping in the conversation from v6, sorry, not sure why I missed your
> reply..
> 
> On Mon, 17 Aug 2026 22:44:30 -0400 Willem de Bruijn wrote:
> > Jakub Kicinski wrote:
> > > On Wed, 12 Aug 2026 22:03:56 -0400 Willem de Bruijn wrote:  
> > > > The 'max_pacing_offload_horizon' field of 'struct net_device' represents
> > > > the maximum pacing offload horizon supported by the device.
> > > > 
> > > > Add a new field 'pacing_offload_horizon' to store the active pacing
> > > > offload horizon.
> > > > 
> > > > The new attribute is initialized to 0 (disabled) and can be set from
> > > > userspace via RTM_SETLINK up to dev->max_pacing_offload_horizon. This
> > > > new default off behavior does not cause regressions, as no driver yet
> > > > advertises max_pacing_offload_horizon.
> > > > 
> > > > The attribute is omitted from the newlink request spec, because the
> > > > value may need to be bound by a device maximum that first needs to be
> > > > negotiated with firmware, as is the case for the idpf driver in this
> > > > series.
> > > > 
> > > > Make both fields u32, to maintain net_device cacheline layout. This
> > > > expresses up to 4s of pacing offload, which is sufficient.
> > > > 
> > > > Update the YNL specification ('rt-link.yaml') to add the
> > > > 'pacing-offload-horizon' attribute and include it in link-all-attrs.  
> > > 
> > > Forgive my slowness but I don't get how the new param squares against
> > > TCA_FQ_OFFLOAD_HORIZON. IIRC in v5 review I asked something like "should 
> > > this new option be a boolean" because the exact time horizon already
> > > exists in the qdisc uAPI. As AI points out (among other things),
> > > the two params are not synced in anyway. User can configure qdisc
> > > offload higher than the device level one.  
> > 
> > They cannot. Or at least that sure is the intent.
> > 
> > After this patch fq tests against active limit
> > dev->pacing_offload_horizon:
> > 
> > -               if (offload_horizon <= qdisc_dev(sch)->max_pacing_offload_horizon) {
> > +               if (offload_horizon <=
> > +                   READ_ONCE(qdisc_dev(sch)->pacing_offload_horizon)) {
> >                         WRITE_ONCE(q->offload_horizon, offload_horizon);
> > 
> > A manual test to replace the root qdisc with fq offload_horizon 50ms
> > seems to verify this: the command fails unless a device limit of >= 50ms
> > is configured.
> 
> The other way around. Configure the Qdisc and device to horizon of 100ms
> Then lower the device horizon to 50ms. Now the qdisc has a longer horizon
> than the device.
> 
> > Perhaps I don't understand how dev->pacing_offload_horizon
> > would function as a boolean.
> 
> The only uses of the new value are:
>  - as the qdisc bound, replacing the max_ value
>    -> Leave the qdisc as is, let qdisc config define the active horizon
>  - in the driver

I see your point now, thanks. A flag NETIF_F_PACING_OFFLOAD?

The two configurable offload_horizon fields is definitely redundant.
I do not want to ship idpf with the feature on by default, because of
SO_TXTIME. But a boolean will do.

Plus, a netdevice_notifier in FQ to clear q->offload_horizon
- when this feature flips to off or
- when dev->max_pacing_hardware_offload changes to a value smaller
  than then configured q->offload_horizon (e.g., on device reset).

> +static void idpf_tx_splitq_set_txtime(const struct sk_buff *skb,
> +				      const struct idpf_tx_queue *tx_q,
> +				      struct idpf_tx_splitq_params *tx_params)
> +{
> +	const int offload_slack_ns = 400;
> +	u64 ts, now, horizon;
> +
> +	horizon = READ_ONCE(skb->dev->pacing_offload_horizon);
> +	if (!horizon)
> +		return;
> 
>    -> which already functions as a boolean, hence my suggestion of boolean
> 
> > > In fact any non-zero value of
> > > the device one acts the same - hence the bool question.
> > >
> > >
> > > Why do we need both? How are you going to use this new knob?
> > > The commit msg explains the what not the why.
> > > 
> > > My naive understanding is that the main missing piece is a handshake
> > > between the driver and qdisc to tell the driver that the qdisc is
> > > indeed offloading pacing on queue X. And therefore the driver should
> > > pay attention to the timestamps. This does not require uAPI changes.
> > >   
> > > >  python3 tools/net/ynl/pyynl/cli.py \  
> > > 
> > > uber-nit: python3 tools/net/ynl/pyynl/cli.py -> ynl
> > > (the CLI is named ynl when packaged for end users)  
> > 
> > Should this also then point to the (default) installed spec path:
> > 
> >        ynl --spec /usr/local/share/ynl/specs/rt-link.yaml 
> 
> Use:
> 
>   ynl --family rt-link
> 
> The expectation is that the person copy/pasting from the commit message
> already has the kernel and user space updated to include your changes.
> Also it's easier to read the shorter format..

Will do. Definitely a lot cleaner.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [PATCH net-next v8 3/6] idpf: support pacing offload
  2026-09-02 18:17 ` [PATCH net-next v8 3/6] idpf: support pacing offload Willem de Bruijn
  2026-09-04  0:19   ` netdev-bot+sashiko
@ 2026-09-07  9:30   ` Loktionov, Aleksandr
  2026-09-10 17:14     ` Willem de Bruijn
  1 sibling, 1 reply; 25+ messages in thread
From: Loktionov, Aleksandr @ 2026-09-07  9:30 UTC (permalink / raw)
  To: Willem de Bruijn, netdev@vger.kernel.org
  Cc: davem@davemloft.net, kuba@kernel.org, edumazet@google.com,
	pabeni@redhat.com, horms@kernel.org, andrew+netdev@lunn.ch,
	Willem de Bruijn, Nguyen, Anthony L, Kitszel, Przemyslaw,
	Hay, Joshua A, intel-wired-lan@lists.osuosl.org



> -----Original Message-----
> From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
> Sent: Wednesday, September 2, 2026 8:17 PM
> To: netdev@vger.kernel.org
> Cc: davem@davemloft.net; kuba@kernel.org; edumazet@google.com;
> pabeni@redhat.com; horms@kernel.org; andrew+netdev@lunn.ch; Willem de
> Bruijn <willemb@google.com>; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>; Hay, Joshua A
> <joshua.a.hay@intel.com>; intel-wired-lan@lists.osuosl.org
> Subject: [PATCH net-next v8 3/6] idpf: support pacing offload
> 
> From: Willem de Bruijn <willemb@google.com>
> 
> If skb->tstamp is in the future, program this future delivery txtime
> in the transmit descriptor.
> 
> TCP pacing offload is only offloaded if SK_PACING_FQ is negotiated and
> the FQ offload_horizon is configured. But device support for pacing
> offload must be more robust: it can also be reached through SO_TXTIME.
> 
> Bounds check txtime. Only packets with timestamp between now and the
> horizon (pacing_offload_horizon) are offloaded.
> 
> Negotiate the feature with the device using virtchnl. Support is
> conditional on
> - splitq mode, where tx and tx completion queues are separate, so
>   completions can be returned out of order.
> - flow scheduling mode, where completions can arrive out of order.
> - PTP to ensure the NIC clock is synced to CLOCK_TAI.
> 
> These features are negotiated per adapter, but expect all vports to
> uniformly request splitq (req_[rt]x_splitq) and flow scheduling
> (flow_sch_en) when available.
> 
> Do not explicitly check all preconditions. Trust the firmware to only
> advertise EDT when all are met. In general, firmware response is
> trusted to be correct. If not, EDT would be the least of the worries.
> In particular, granularity is trusted to be a power of two. And
> time_horizon_ns is a range expressible given the chosen granularity.
> 
> Packets beyond the horizon are sent immediately with the overflow bit
> set.
> 
> On device reset, dev->pacing_offload_horizon, fq offload_horizon and
> granularity are not re-negotiated. It is safe to assume that firmware
> does not change these EDT capabilities across resets.
> 
> Must not be called from netpoll due to ktime_get. But netpoll does not
> generate packets with EDT, so no explicit test is needed.
> 
> Do not fail device initialization on EDT init error. Log an error, but
> continue without EDT, similar to PTP.
> 
> Cc: Tony Nguyen <anthony.l.nguyen@intel.com>
> Cc: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> Cc: Joshua A Hay <joshua.a.hay@intel.com>
> Cc: intel-wired-lan@lists.osuosl.org
> Signed-off-by: Willem de Bruijn <willemb@google.com>
> 
> ---
> 
> Changes
>   v7 -> v8
>     - remove unused variable np in idpf_tx_splitq_set_txtime
>   v6 -> v7
>     - rebase onto libie changes: adjust idpf_send_get_edt_caps_msg
>     - only set horizon if splitq with flow scheduling
>     - fail on illegal granularity 0, rather than use hardcoded default
>     - zero edt_caps on negotation error, in case of call after reset
negotation -> negotiation


Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>

>     - cache ts_gran_pow2 in idpf_tx_queue to be in hot cache line
>     - similar to FQ, add offload slack to avoid pacing < 400ns of now
>     - remove now superfluous include netpoll.h
>     - reverse xmas tree (1x)
>   v5 -> v6
>     - move EDT caps from virtchnl2_edt_caps_ilog2 to
> idpf_edt_caps_ilog2
>     - do not fail device init if EDT init failed, same as PTP
>     - replace netpoll branch with commit-msg comment
>     - add commit-msg comment about device reset
>     - add commit-msg about behavior beyond horizon
>     - add commit-msg about trusting EDT capability response
>   v4 -> v5
>     - fix kdoc: idpf.h:738 struct member 'edt_caps' not described..
>   v3 -> v4
>     - add EDT virtchnl negotiation
>     - move endianness fix to its own iwl-net patch
>     - update commit-msg: check is against pacing_offload_horizon, not
> max_..
>     - simplify assignment, avoid the le32 assignment in favor of u8
> inits
>     - replace open coded constant with
> IDPF_TXD_FLOW_SCH_HORIZON_OVERFLOW_M
>     - (minor) fix 24b timestamp comment: is 23b + overflow bit
>     - (minor) remove unused parameter
>   v2 -> v3
>     - use READ_ONCE and read pacing_offload_horizon only once
>   v1 -> v2
>     - move special zero case up and return early
> 
> Sashiko, ignore pre-existing issues.
> 
> Sashiko, ignore that idpf_tx_splitq_set_txtime may have a benign race
> by calling ktime_mono_to_any twice to get TAI to REALTIME offset.
> ---
>  drivers/net/ethernet/intel/idpf/idpf.h        | 12 ++++
>  drivers/net/ethernet/intel/idpf/idpf_lib.c    |  6 ++
>  drivers/net/ethernet/intel/idpf/idpf_txrx.c   | 66
> ++++++++++++++++++-
>  drivers/net/ethernet/intel/idpf/idpf_txrx.h   |  8 ++-
>  .../net/ethernet/intel/idpf/idpf_virtchnl.c   | 60 ++++++++++++++++-
>  5 files changed, 149 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/idpf/idpf.h
> b/drivers/net/ethernet/intel/idpf/idpf.h
> index 470bc23c844c..f214023095ee 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf.h
> +++ b/drivers/net/ethernet/intel/idpf/idpf.h
> @@ -604,6 +604,16 @@ struct idpf_vport_config {
>  	DECLARE_BITMAP(flags, IDPF_VPORT_CONFIG_FLAGS_NBITS);  };
> 

...

>  	pci_sriov_set_totalvfs(adapter->pdev,
> idpf_get_max_vfs(adapter));
>  	num_max_vports = idpf_get_max_vports(adapter);
>  	adapter->vports = kzalloc_objs(*adapter->vports,
> num_max_vports);
> --
> 2.55.0.970.g62bdec98f9-goog


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH net-next v8 1/6] net: rtnetlink: add pacing_offload_horizon attribute to net_device
  2026-09-06  2:22     ` Willem de Bruijn
@ 2026-09-07 23:12       ` Jakub Kicinski
  2026-09-08  0:50         ` Willem de Bruijn
  0 siblings, 1 reply; 25+ messages in thread
From: Jakub Kicinski @ 2026-09-07 23:12 UTC (permalink / raw)
  To: Willem de Bruijn
  Cc: netdev, davem, edumazet, pabeni, horms, andrew+netdev,
	Willem de Bruijn

On Sat, 05 Sep 2026 22:22:06 -0400 Willem de Bruijn wrote:
> > > Perhaps I don't understand how dev->pacing_offload_horizon
> > > would function as a boolean.  
> > 
> > The only uses of the new value are:
> >  - as the qdisc bound, replacing the max_ value  
> >    -> Leave the qdisc as is, let qdisc config define the active horizon  
> >  - in the driver  
> 
> I see your point now, thanks. A flag NETIF_F_PACING_OFFLOAD?

From the name I suspect you mean a feature / ethtool (-k/-K) ?
That would do, but presumably not as a real low bit within
dev->features ? I guess it is a datapath feature but somehow
those bits feel too precious.

> The two configurable offload_horizon fields is definitely redundant.
> I do not want to ship idpf with the feature on by default, because of
> SO_TXTIME. But a boolean will do.
> 
> Plus, a netdevice_notifier in FQ to clear q->offload_horizon
> - when this feature flips to off or

Not sure if we should be clearing user config or rejecting
the feature change if currently in use.

> - when dev->max_pacing_hardware_offload changes to a value smaller
>   than then configured q->offload_horizon (e.g., on device reset).

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH net-next v8 1/6] net: rtnetlink: add pacing_offload_horizon attribute to net_device
  2026-09-07 23:12       ` Jakub Kicinski
@ 2026-09-08  0:50         ` Willem de Bruijn
  2026-09-08 20:48           ` Willem de Bruijn
  0 siblings, 1 reply; 25+ messages in thread
From: Willem de Bruijn @ 2026-09-08  0:50 UTC (permalink / raw)
  To: Jakub Kicinski, Willem de Bruijn
  Cc: netdev, davem, edumazet, pabeni, horms, andrew+netdev,
	Willem de Bruijn

Jakub Kicinski wrote:
> On Sat, 05 Sep 2026 22:22:06 -0400 Willem de Bruijn wrote:
> > > > Perhaps I don't understand how dev->pacing_offload_horizon
> > > > would function as a boolean.  
> > > 
> > > The only uses of the new value are:
> > >  - as the qdisc bound, replacing the max_ value  
> > >    -> Leave the qdisc as is, let qdisc config define the active horizon  
> > >  - in the driver  
> > 
> > I see your point now, thanks. A flag NETIF_F_PACING_OFFLOAD?
> 
> From the name I suspect you mean a feature / ethtool (-k/-K) ?
> That would do, but presumably not as a real low bit within
> dev->features ? I guess it is a datapath feature but somehow
> those bits feel too precious.

Makes sense.

An ethtool -K that does not use dev->features seems like a hack.

Keep the existing ip link attribute and make that settable only to
zero or max_pacing_offload_horizon?
 
> > The two configurable offload_horizon fields is definitely redundant.
> > I do not want to ship idpf with the feature on by default, because of
> > SO_TXTIME. But a boolean will do.
> > 
> > Plus, a netdevice_notifier in FQ to clear q->offload_horizon
> > - when this feature flips to off or
> 
> Not sure if we should be clearing user config or rejecting
> the feature change if currently in use.

We cannot reject the feature change, if it's a device reset and on
re-negotiation the device capability changed. E.g., from a firmware
rollout.
 
> > - when dev->max_pacing_hardware_offload changes to a value smaller
> >   than then configured q->offload_horizon (e.g., on device reset).



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH net-next v8 1/6] net: rtnetlink: add pacing_offload_horizon attribute to net_device
  2026-09-08  0:50         ` Willem de Bruijn
@ 2026-09-08 20:48           ` Willem de Bruijn
  2026-09-08 21:48             ` Jakub Kicinski
  0 siblings, 1 reply; 25+ messages in thread
From: Willem de Bruijn @ 2026-09-08 20:48 UTC (permalink / raw)
  To: Jakub Kicinski, Willem de Bruijn, edumazet
  Cc: netdev, davem, pabeni, horms, andrew+netdev, Willem de Bruijn

On Mon, Sep 7, 2026 at 8:50 PM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> Jakub Kicinski wrote:
> > On Sat, 05 Sep 2026 22:22:06 -0400 Willem de Bruijn wrote:
> > > > > Perhaps I don't understand how dev->pacing_offload_horizon
> > > > > would function as a boolean.
> > > >
> > > > The only uses of the new value are:
> > > >  - as the qdisc bound, replacing the max_ value
> > > >    -> Leave the qdisc as is, let qdisc config define the active horizon
> > > >  - in the driver
> > >
> > > I see your point now, thanks. A flag NETIF_F_PACING_OFFLOAD?
> >
> > From the name I suspect you mean a feature / ethtool (-k/-K) ?
> > That would do, but presumably not as a real low bit within
> > dev->features ? I guess it is a datapath feature but somehow
> > those bits feel too precious.
>
> Makes sense.
>
> An ethtool -K that does not use dev->features seems like a hack.
>
> Keep the existing ip link attribute and make that settable only to
> zero or max_pacing_offload_horizon?

I've updated to a simple boolean pacing_offload in ip link.
Let me know if you had something else specific in mind.

+++ b/Documentation/netlink/specs/rt-link.yaml
@@ -1089,6 +1089,10 @@ attribute-sets:
+      -
+        name: pacing-offload
+        type: u8
+        doc: Enable EDT pacing offload (0 - disabled, 1 - enabled).

> > > The two configurable offload_horizon fields is definitely redundant.
> > > I do not want to ship idpf with the feature on by default, because of
> > > SO_TXTIME. But a boolean will do.
> > >
> > > Plus, a netdevice_notifier in FQ to clear q->offload_horizon
> > > - when this feature flips to off or
> >
> > Not sure if we should be clearing user config or rejecting
> > the feature change if currently in use.
>
> We cannot reject the feature change, if it's a device reset and on
> re-negotiation the device capability changed. E.g., from a firmware
> rollout.

I added a separate patch that adds a netdevice notifier to FQ.
To reset offload_horizon on device feature change if needed.
Or any qdisc feature that depends on device state in principle.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH net-next v8 1/6] net: rtnetlink: add pacing_offload_horizon attribute to net_device
  2026-09-08 20:48           ` Willem de Bruijn
@ 2026-09-08 21:48             ` Jakub Kicinski
  2026-09-08 22:39               ` Willem de Bruijn
  0 siblings, 1 reply; 25+ messages in thread
From: Jakub Kicinski @ 2026-09-08 21:48 UTC (permalink / raw)
  To: Willem de Bruijn
  Cc: edumazet, netdev, davem, pabeni, horms, andrew+netdev,
	Willem de Bruijn

On Tue, 8 Sep 2026 16:48:03 -0400 Willem de Bruijn wrote:
> > > From the name I suspect you mean a feature / ethtool (-k/-K) ?
> > > That would do, but presumably not as a real low bit within
> > > dev->features ? I guess it is a datapath feature but somehow
> > > those bits feel too precious.  
> >
> > Makes sense.
> >
> > An ethtool -K that does not use dev->features seems like a hack.

It's just a way of avoiding netdev_features_t becoming larger than 64b.
At the uAPI level we have a bitmap so we can express any number of bits.
But in the kernel dev->features is u64 / ull so if we run out of bits
the refactor will be rather painful.

Is it really that much worse to add the support for "high feature bits"
which don't go thru netdev_features_t to ethtool, than sprinkling extra
one off booleans into already bloated RTM_SETLINK? Not sure.

> > Keep the existing ip link attribute and make that settable only to
> > zero or max_pacing_offload_horizon?  
> 
> I've updated to a simple boolean pacing_offload in ip link.
> Let me know if you had something else specific in mind.
> 
> +++ b/Documentation/netlink/specs/rt-link.yaml
> @@ -1089,6 +1089,10 @@ attribute-sets:
> +      -
> +        name: pacing-offload
> +        type: u8

Netlink pads very value to 4B so just make it a u32

> +        doc: Enable EDT pacing offload (0 - disabled, 1 - enabled).

> > > Not sure if we should be clearing user config or rejecting
> > > the feature change if currently in use.  
> >
> > We cannot reject the feature change

Feature change means user requesting the feature to be disabled.

> > if it's a device reset and on re-negotiation the device capability
> > changed. E.g., from a firmware rollout.  
> 
> I added a separate patch that adds a netdevice notifier to FQ.
> To reset offload_horizon on device feature change if needed.
> Or any qdisc feature that depends on device state in principle.

I hate both notifiers everywhere and the idea that we have to be able
to quietly revoke device features "on firmware rollout". It leads to
unmaintainable code which almost never runs so it's buggy half of the
time. Whatever.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH net-next v8 1/6] net: rtnetlink: add pacing_offload_horizon attribute to net_device
  2026-09-08 21:48             ` Jakub Kicinski
@ 2026-09-08 22:39               ` Willem de Bruijn
  2026-09-08 23:34                 ` Jakub Kicinski
  0 siblings, 1 reply; 25+ messages in thread
From: Willem de Bruijn @ 2026-09-08 22:39 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: edumazet, netdev, davem, pabeni, horms, andrew+netdev,
	Willem de Bruijn

On Tue, Sep 8, 2026 at 5:48 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Tue, 8 Sep 2026 16:48:03 -0400 Willem de Bruijn wrote:
> > > > From the name I suspect you mean a feature / ethtool (-k/-K) ?
> > > > That would do, but presumably not as a real low bit within
> > > > dev->features ? I guess it is a datapath feature but somehow
> > > > those bits feel too precious.
> > >
> > > Makes sense.
> > >
> > > An ethtool -K that does not use dev->features seems like a hack.
>
> It's just a way of avoiding netdev_features_t becoming larger than 64b.
> At the uAPI level we have a bitmap so we can express any number of bits.
> But in the kernel dev->features is u64 / ull so if we run out of bits
> the refactor will be rather painful.
>
> Is it really that much worse to add the support for "high feature bits"
> which don't go thru netdev_features_t to ethtool, than sprinkling extra
> one off booleans into already bloated RTM_SETLINK? Not sure.

How would this work with hw_features, wanted_features, vlan_features
and other such feature masks?

There is potentially quite a bit of logic that needs to be duplicated
for a secondary set of features. Or, the risk that these features end
up not quite like normal features.

Some device attributes, such as lltx, did get moved out of
dev->features to free up space. But key there is that they are not
user configurable.

> > > Keep the existing ip link attribute and make that settable only to
> > > zero or max_pacing_offload_horizon?
> >
> > I've updated to a simple boolean pacing_offload in ip link.
> > Let me know if you had something else specific in mind.
> >
> > +++ b/Documentation/netlink/specs/rt-link.yaml
> > @@ -1089,6 +1089,10 @@ attribute-sets:
> > +      -
> > +        name: pacing-offload
> > +        type: u8
>
> Netlink pads very value to 4B so just make it a u32

Will do.

> > +        doc: Enable EDT pacing offload (0 - disabled, 1 - enabled).
>
> > > > Not sure if we should be clearing user config or rejecting
> > > > the feature change if currently in use.
> > >
> > > We cannot reject the feature change
>
> Feature change means user requesting the feature to be disabled.
>
> > > if it's a device reset and on re-negotiation the device capability
> > > changed. E.g., from a firmware rollout.
> >
> > I added a separate patch that adds a netdevice notifier to FQ.
> > To reset offload_horizon on device feature change if needed.
> > Or any qdisc feature that depends on device state in principle.
>
> I hate both notifiers everywhere and the idea that we have to be able
> to quietly revoke device features "on firmware rollout". It leads to
> unmaintainable code which almost never runs so it's buggy half of the
> time. Whatever.

I don't like it, but firmware roll-outs that remove features
unfortunately are a real thing. Especially roll-backs.

For pacing offload specifically, I considered the risk low enough to
rely on the admin to manually revert the FQ settings when such an
event happens. But the bots kept complaining. And in fairness a
notifier based auto disable is indeed much more robust than a manual
correlated roll-out. OTOH, it is rarely exercised code in practice and
thus more prone to latent bugs.

A simpler approach for pacing offload is to check the dev fields
directly in fq. It is likely that that cacheline is warm.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH net-next v8 1/6] net: rtnetlink: add pacing_offload_horizon attribute to net_device
  2026-09-08 22:39               ` Willem de Bruijn
@ 2026-09-08 23:34                 ` Jakub Kicinski
  2026-09-09 15:47                   ` Willem de Bruijn
  0 siblings, 1 reply; 25+ messages in thread
From: Jakub Kicinski @ 2026-09-08 23:34 UTC (permalink / raw)
  To: Willem de Bruijn
  Cc: edumazet, netdev, davem, pabeni, horms, andrew+netdev,
	Willem de Bruijn

On Tue, 8 Sep 2026 18:39:20 -0400 Willem de Bruijn wrote:
> On Tue, Sep 8, 2026 at 5:48 PM Jakub Kicinski <kuba@kernel.org> wrote:
> > It's just a way of avoiding netdev_features_t becoming larger than 64b.
> > At the uAPI level we have a bitmap so we can express any number of bits.
> > But in the kernel dev->features is u64 / ull so if we run out of bits
> > the refactor will be rather painful.
> >
> > Is it really that much worse to add the support for "high feature bits"
> > which don't go thru netdev_features_t to ethtool, than sprinkling extra
> > one off booleans into already bloated RTM_SETLINK? Not sure.  
> 
> How would this work with hw_features, wanted_features, vlan_features
> and other such feature masks?
> 
> There is potentially quite a bit of logic that needs to be duplicated
> for a secondary set of features. Or, the risk that these features end
> up not quite like normal features.

No different than a loose bit in SETLINK?

What I was wondering is - we don't want to implement full handling for
high feature bits, agreed, it doesn't seem needed today. But is it
better to add such a bit in ethtool regardless, even tho it doesn't get
any infra for propagation to uppers etc. Or is it better to start adding
one-off bits in rt-link.

Think about it some more, since the max-horizon attr is already in
rt-link I guess putting this bit in rt-link does make more sense.

> > I hate both notifiers everywhere and the idea that we have to be able
> > to quietly revoke device features "on firmware rollout". It leads to
> > unmaintainable code which almost never runs so it's buggy half of the
> > time. Whatever.  
> 
> I don't like it, but firmware roll-outs that remove features
> unfortunately are a real thing. Especially roll-backs.
> 
> For pacing offload specifically, I considered the risk low enough to
> rely on the admin to manually revert the FQ settings when such an
> event happens. But the bots kept complaining. And in fairness a
> notifier based auto disable is indeed much more robust than a manual
> correlated roll-out. OTOH, it is rarely exercised code in practice and
> thus more prone to latent bugs.

TBH I'm not sure what you have in mind with the notifier.
What netdev event does the FW reset generate? 
 
> A simpler approach for pacing offload is to check the dev fields
> directly in fq. It is likely that that cacheline is warm.

Right, there's ~30b of unused flag space in the first cache line
of struct net_device. Should be warm.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH net-next v8 1/6] net: rtnetlink: add pacing_offload_horizon attribute to net_device
  2026-09-08 23:34                 ` Jakub Kicinski
@ 2026-09-09 15:47                   ` Willem de Bruijn
  2026-09-09 18:13                     ` Jakub Kicinski
  0 siblings, 1 reply; 25+ messages in thread
From: Willem de Bruijn @ 2026-09-09 15:47 UTC (permalink / raw)
  To: Jakub Kicinski, Willem de Bruijn
  Cc: edumazet, netdev, davem, pabeni, horms, andrew+netdev,
	Willem de Bruijn

Jakub Kicinski wrote:
> On Tue, 8 Sep 2026 18:39:20 -0400 Willem de Bruijn wrote:
> > On Tue, Sep 8, 2026 at 5:48 PM Jakub Kicinski <kuba@kernel.org> wrote:
> > > It's just a way of avoiding netdev_features_t becoming larger than 64b.
> > > At the uAPI level we have a bitmap so we can express any number of bits.
> > > But in the kernel dev->features is u64 / ull so if we run out of bits
> > > the refactor will be rather painful.
> > >
> > > Is it really that much worse to add the support for "high feature bits"
> > > which don't go thru netdev_features_t to ethtool, than sprinkling extra
> > > one off booleans into already bloated RTM_SETLINK? Not sure.  
> > 
> > How would this work with hw_features, wanted_features, vlan_features
> > and other such feature masks?
> > 
> > There is potentially quite a bit of logic that needs to be duplicated
> > for a secondary set of features. Or, the risk that these features end
> > up not quite like normal features.
> 
> No different than a loose bit in SETLINK?

At least then there is no expectation of netdev_features_t behavior.
 
> What I was wondering is - we don't want to implement full handling for
> high feature bits, agreed, it doesn't seem needed today. But is it
> better to add such a bit in ethtool regardless, even tho it doesn't get
> any infra for propagation to uppers etc. Or is it better to start adding
> one-off bits in rt-link.
> 
> Think about it some more, since the max-horizon attr is already in
> rt-link I guess putting this bit in rt-link does make more sense.

Ok, I'll leave this in rt-link then.

Even then, running out of feature bits will come back to haunt us
soon enough. A good idea to look into it some more now.

The main issue here is not wanting to grow netdev_features_t for
hot cachelines, right? Which besides growing dev->features has a
cascading effect on all the other fields of that type in net_device
too.

Extending struct net_device itself is fine, as long as it happens in
some cold cacheline at the end.

Perhaps something like what Paolo did for virtio features in the
series of 3b17aa13015c ("virtio_net: add supports for extended
offloads"). With non-contiguous bitmap fields. Everything beyond 64
is mapped to a new field at the end of the struct. A thin API to
avoid open-coding that check everywhere. And selective conversion
only of code/drivers that need to access the extended features.

> > > I hate both notifiers everywhere and the idea that we have to be able
> > > to quietly revoke device features "on firmware rollout". It leads to
> > > unmaintainable code which almost never runs so it's buggy half of the
> > > time. Whatever.  
> > 
> > I don't like it, but firmware roll-outs that remove features
> > unfortunately are a real thing. Especially roll-backs.
> > 
> > For pacing offload specifically, I considered the risk low enough to
> > rely on the admin to manually revert the FQ settings when such an
> > event happens. But the bots kept complaining. And in fairness a
> > notifier based auto disable is indeed much more robust than a manual
> > correlated roll-out. OTOH, it is rarely exercised code in practice and
> > thus more prone to latent bugs.
> 
> TBH I'm not sure what you have in mind with the notifier.
> What netdev event does the FW reset generate? 
>
> > A simpler approach for pacing offload is to check the dev fields
> > directly in fq. It is likely that that cacheline is warm.
> 
> Right, there's ~30b of unused flag space in the first cache line
> of struct net_device. Should be warm.

I can move dev->pacing_offload there.

max_pacing_offload is in a cold line. Finding a way to squeeze that
somewhere warm can perhaps be left for later.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH net-next v8 1/6] net: rtnetlink: add pacing_offload_horizon attribute to net_device
  2026-09-09 15:47                   ` Willem de Bruijn
@ 2026-09-09 18:13                     ` Jakub Kicinski
  0 siblings, 0 replies; 25+ messages in thread
From: Jakub Kicinski @ 2026-09-09 18:13 UTC (permalink / raw)
  To: Willem de Bruijn
  Cc: edumazet, netdev, davem, pabeni, horms, andrew+netdev,
	Willem de Bruijn

On Wed, 09 Sep 2026 11:47:13 -0400 Willem de Bruijn wrote:
> > No different than a loose bit in SETLINK?  
>
> At least then there is no expectation of netdev_features_t behavior.

I'm not sure what "netdev_features_t behavior" is, different "types"
of features behave differently.

> > What I was wondering is - we don't want to implement full handling for
> > high feature bits, agreed, it doesn't seem needed today. But is it
> > better to add such a bit in ethtool regardless, even tho it doesn't get
> > any infra for propagation to uppers etc. Or is it better to start adding
> > one-off bits in rt-link.
> > 
> > Think about it some more, since the max-horizon attr is already in
> > rt-link I guess putting this bit in rt-link does make more sense.  
> 
> Ok, I'll leave this in rt-link then.
> 
> Even then, running out of feature bits will come back to haunt us
> soon enough. A good idea to look into it some more now.
> 
> The main issue here is not wanting to grow netdev_features_t for
> hot cachelines, right? Which besides growing dev->features has a
> cascading effect on all the other fields of that type in net_device
> too.
> 
> Extending struct net_device itself is fine, as long as it happens in
> some cold cacheline at the end.
> 
> Perhaps something like what Paolo did for virtio features in the
> series of 3b17aa13015c ("virtio_net: add supports for extended
> offloads"). With non-contiguous bitmap fields. Everything beyond 64
> is mapped to a new field at the end of the struct. A thin API to
> avoid open-coding that check everywhere. And selective conversion
> only of code/drivers that need to access the extended features.

Trying to code this up may reveal the correct shape of the solution.
My mental model so far has been that features are still a mix of
high touch datapath bits and random config stuff like LOOPBACK or HW_TC
which could as well live in a cold cacheline. So my thinking was that
the first step would be to divorce the internal kernel formats from
uAPI. 

^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [PATCH net-next v8 3/6] idpf: support pacing offload
  2026-09-07  9:30   ` Loktionov, Aleksandr
@ 2026-09-10 17:14     ` Willem de Bruijn
  0 siblings, 0 replies; 25+ messages in thread
From: Willem de Bruijn @ 2026-09-10 17:14 UTC (permalink / raw)
  To: Loktionov, Aleksandr, Willem de Bruijn, netdev@vger.kernel.org
  Cc: davem@davemloft.net, kuba@kernel.org, edumazet@google.com,
	pabeni@redhat.com, horms@kernel.org, andrew+netdev@lunn.ch,
	Willem de Bruijn, Nguyen, Anthony L, Kitszel, Przemyslaw,
	Hay, Joshua A, intel-wired-lan@lists.osuosl.org

Loktionov, Aleksandr wrote:
> 
> 
> > -----Original Message-----
> > From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
> > Sent: Wednesday, September 2, 2026 8:17 PM
> > To: netdev@vger.kernel.org
> > Cc: davem@davemloft.net; kuba@kernel.org; edumazet@google.com;
> > pabeni@redhat.com; horms@kernel.org; andrew+netdev@lunn.ch; Willem de
> > Bruijn <willemb@google.com>; Nguyen, Anthony L
> > <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw
> > <przemyslaw.kitszel@intel.com>; Hay, Joshua A
> > <joshua.a.hay@intel.com>; intel-wired-lan@lists.osuosl.org
> > Subject: [PATCH net-next v8 3/6] idpf: support pacing offload
> > 
> > From: Willem de Bruijn <willemb@google.com>
> > 
> > If skb->tstamp is in the future, program this future delivery txtime
> > in the transmit descriptor.
> > 
> > TCP pacing offload is only offloaded if SK_PACING_FQ is negotiated and
> > the FQ offload_horizon is configured. But device support for pacing
> > offload must be more robust: it can also be reached through SO_TXTIME.
> > 
> > Bounds check txtime. Only packets with timestamp between now and the
> > horizon (pacing_offload_horizon) are offloaded.
> > 
> > Negotiate the feature with the device using virtchnl. Support is
> > conditional on
> > - splitq mode, where tx and tx completion queues are separate, so
> >   completions can be returned out of order.
> > - flow scheduling mode, where completions can arrive out of order.
> > - PTP to ensure the NIC clock is synced to CLOCK_TAI.
> > 
> > These features are negotiated per adapter, but expect all vports to
> > uniformly request splitq (req_[rt]x_splitq) and flow scheduling
> > (flow_sch_en) when available.
> > 
> > Do not explicitly check all preconditions. Trust the firmware to only
> > advertise EDT when all are met. In general, firmware response is
> > trusted to be correct. If not, EDT would be the least of the worries.
> > In particular, granularity is trusted to be a power of two. And
> > time_horizon_ns is a range expressible given the chosen granularity.
> > 
> > Packets beyond the horizon are sent immediately with the overflow bit
> > set.
> > 
> > On device reset, dev->pacing_offload_horizon, fq offload_horizon and
> > granularity are not re-negotiated. It is safe to assume that firmware
> > does not change these EDT capabilities across resets.
> > 
> > Must not be called from netpoll due to ktime_get. But netpoll does not
> > generate packets with EDT, so no explicit test is needed.
> > 
> > Do not fail device initialization on EDT init error. Log an error, but
> > continue without EDT, similar to PTP.
> > 
> > Cc: Tony Nguyen <anthony.l.nguyen@intel.com>
> > Cc: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> > Cc: Joshua A Hay <joshua.a.hay@intel.com>
> > Cc: intel-wired-lan@lists.osuosl.org
> > Signed-off-by: Willem de Bruijn <willemb@google.com>
> > 
> > ---
> > 
> > Changes
> >   v7 -> v8
> >     - remove unused variable np in idpf_tx_splitq_set_txtime
> >   v6 -> v7
> >     - rebase onto libie changes: adjust idpf_send_get_edt_caps_msg
> >     - only set horizon if splitq with flow scheduling
> >     - fail on illegal granularity 0, rather than use hardcoded default
> >     - zero edt_caps on negotation error, in case of call after reset
> negotation -> negotiation

Fixed
 
> 
> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>

Thanks for the review Aleksandr!

I did not keep the Reviewed-by as sticky, because the patch saw some
changes in v9. 

https://lore.kernel.org/netdev/20260910171131.2532487-5-willemdebruijn.kernel@gmail.com/T/#u

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2026-09-10 17:14 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-02 18:17 [PATCH net-next v8 0/6] hardware pacing offload Willem de Bruijn
2026-09-02 18:17 ` [PATCH net-next v8 1/6] net: rtnetlink: add pacing_offload_horizon attribute to net_device Willem de Bruijn
2026-09-04  0:19   ` netdev-bot+sashiko
2026-09-04 23:01   ` Jakub Kicinski
2026-09-06  2:22     ` Willem de Bruijn
2026-09-07 23:12       ` Jakub Kicinski
2026-09-08  0:50         ` Willem de Bruijn
2026-09-08 20:48           ` Willem de Bruijn
2026-09-08 21:48             ` Jakub Kicinski
2026-09-08 22:39               ` Willem de Bruijn
2026-09-08 23:34                 ` Jakub Kicinski
2026-09-09 15:47                   ` Willem de Bruijn
2026-09-09 18:13                     ` Jakub Kicinski
2026-09-02 18:17 ` [PATCH net-next v8 2/6] net_sched: sch_fq: clear past skb->tstamp if offloading pacing Willem de Bruijn
2026-09-02 18:17 ` [PATCH net-next v8 3/6] idpf: support pacing offload Willem de Bruijn
2026-09-04  0:19   ` netdev-bot+sashiko
2026-09-04 15:22     ` Willem de Bruijn
2026-09-07  9:30   ` Loktionov, Aleksandr
2026-09-10 17:14     ` Willem de Bruijn
2026-09-02 18:17 ` [PATCH net-next v8 4/6] selftests: drv-net: refactor so_txtime errqueue handling Willem de Bruijn
2026-09-04  0:19   ` netdev-bot+sashiko
2026-09-02 18:17 ` [PATCH net-next v8 5/6] selftests: drv-net: in so_txtime tell apart sw from hw pacing Willem de Bruijn
2026-09-04  0:19   ` netdev-bot+sashiko
2026-09-02 18:17 ` [PATCH net-next v8 6/6] selftests: drv-net: extend so_txtime with hw offload Willem de Bruijn
2026-09-04  0:19   ` netdev-bot+sashiko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).