Re: [PATCH net-next v2 3/7] idpf: support pacing offload

[Mohsin Bashir <mohsin.bashr@gmail.com>]

On 7/22/26 1:43 PM, Willem de Bruijn wrote:
> 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.
> 
> Bound check txtime. Only packets with timestamp between now and the
> horizon (max_pacing_offload_horizon) are offloaded.
> 
> Support only in splitq mode, where tx and tx completion queues are
> separate and so completions can be returned out of order.
> 
> Assume that the NIC clock is PTP synchronized to CLOCK_TAI. This
> can later be refined, e.g., to a custom CLOCK_AUX.
> 
> Disable if in netpoll. It does not need the feature, and the ktime
> functions are not safe to call in this context.
> 
> Cc: Tony Nguyen <anthony.l.nguyen@intel.com>
> Cc: Joshua A Hay <joshua.a.hay@intel.com>
> Signed-off-by: Willem de Bruijn <willemb@google.com>
> 
> ---
> 
> Changes
>    v1 -> v2
>      - move special zero case up and return early
>    v1: https://lore.kernel.org/netdev/20260706133433.3142805-4-willemdebruijn.kernel@gmail.com/
> 
> 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.
> 
> fixup idpf
> ---
>   .../net/ethernet/intel/idpf/idpf_ethtool.c    |  3 +-
>   drivers/net/ethernet/intel/idpf/idpf_lib.c    |  5 ++
>   drivers/net/ethernet/intel/idpf/idpf_txrx.c   | 65 ++++++++++++++++++-
>   drivers/net/ethernet/intel/idpf/idpf_txrx.h   |  2 +
>   4 files changed, 73 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_ethtool.c b/drivers/net/ethernet/intel/idpf/idpf_ethtool.c
> index bb99d9e7c65d..5ddf347927dc 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_ethtool.c
> +++ b/drivers/net/ethernet/intel/idpf/idpf_ethtool.c
> @@ -1801,7 +1801,8 @@ static void idpf_get_ts_stats(struct net_device *netdev,
>   static const struct ethtool_ops idpf_ethtool_ops = {
>   	.supported_coalesce_params = ETHTOOL_COALESCE_USECS |
>   				     ETHTOOL_COALESCE_USE_ADAPTIVE,
> -	.supported_ring_params	= ETHTOOL_RING_USE_TCP_DATA_SPLIT,
> +	.supported_ring_params	= ETHTOOL_RING_USE_TCP_DATA_SPLIT |
> +				  ETHTOOL_RING_USE_PACING_OFFLOAD_HORIZON,
>   	.get_msglevel		= idpf_get_msglevel,
>   	.set_msglevel		= idpf_set_msglevel,
>   	.get_link		= ethtool_op_get_link,
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_lib.c b/drivers/net/ethernet/intel/idpf/idpf_lib.c
> index cf966fe6c759..de4170c2aa3b 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_lib.c
> +++ b/drivers/net/ethernet/intel/idpf/idpf_lib.c
> @@ -888,6 +888,11 @@ static int idpf_cfg_netdev(struct idpf_vport *vport)
>   	netdev->min_mtu = ETH_MIN_MTU;
>   	netdev->max_mtu = vport->max_mtu;
>   
> +	if (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 = 128000;
> +
>   	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 566b08ca3a6c..1ccfaeb35193 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_txrx.c
> +++ b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
> @@ -1,6 +1,8 @@
>   // SPDX-License-Identifier: GPL-2.0-only
>   /* Copyright (C) 2023 Intel Corporation */
>   
> +#include <linux/netpoll.h>
> +
>   #include "idpf.h"
>   #include "idpf_ptp.h"
>   #include "idpf_virtchnl.h"
> @@ -2408,9 +2410,13 @@ void idpf_tx_splitq_build_flow_desc(union idpf_tx_flex_desc *desc,
>   				    struct idpf_tx_splitq_params *params,
>   				    u16 td_cmd, u16 size)
>   {
> -	*(u32 *)&desc->flow.qw1.cmd_dtype = (u8)(params->dtype | td_cmd);
> +	*(__le32 *)&desc->flow.qw1.cmd_dtype = cpu_to_le32((u8)(params->dtype | td_cmd));

Looks like an independent change (potentially a fix)?

>   	desc->flow.qw1.rxr_bufsize = cpu_to_le16((u16)size);
>   	desc->flow.qw1.compl_tag = cpu_to_le16(params->compl_tag);
> +
> +	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];
>   }
>   
>   /**
> @@ -3011,6 +3017,59 @@ 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 idpf_tx_queue *tx_q,
> +				      const struct sk_buff *skb,
> +				      struct idpf_tx_splitq_params *tx_params)
> +{
> +	const int ts_gran_pow2 = 9;
> +	u64 ts, now;
> +
> +	/* Skip if netpoll: not needed and not safe to call ktime helpers */
> +	if (netpoll_tx_running(skb->dev))
> +		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)
> +		return;
> +
> +	/* beyond offload horizon? set overflow bit only */
> +	if (ts > now + ((u64)skb->dev->pacing_offload_horizon * NSEC_PER_USEC)) {
> +		tx_params->offload.desc_ts[2] = 1 << 7;
> +		return;
> +	}
> +
> +	ts >>= ts_gran_pow2;
> +
> +	/* 0 is valid 24b timestamp, but also means field unset.
> +	 * Add 512 ns (ts_gran_pow2) 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 +3156,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 && skb->dev->pacing_offload_horizon)
> +			idpf_tx_splitq_set_txtime(tx_q, skb, &tx_params);
> +
>   		/* Set the RE bit to periodically "clean" the descriptor ring.
>   		 * MIN_GAP is set to MIN_RING size to ensure it will be set at
>   		 * least once each time around the ring.
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_txrx.h b/drivers/net/ethernet/intel/idpf/idpf_txrx.h
> index 4be5b3b6d3ed..865408b57b0d 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];
>   };
>   
>   /**