Re: [PATCH net-next v2 1/7] net: ethtool: add hardware pacing offload support to rings

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

On 7/22/26 1:43 PM, Willem de Bruijn wrote:
> From: Willem de Bruijn <willemb@google.com>
> 
> Add two ethtool rings operations:
> 
> - get pacing offload horizon: active and max
> - set pacing offload horizon: active
> 
> Replace u64 max_pacing_offload_horizon with u32 active and max fields.
> This occupies the same original 8 bytes. Reduce precision from nsec to
> usec, which is sufficient.
> 
> Update the FQ scheduler to test against the active limit instead of
> max. It is the administrator responsibility to set the active limit
> before installing FQ.
> 
> Assisted-by: Gemini:gemini-3
> Signed-off-by: Willem de Bruijn <willemb@google.com>
> 

If I understand it correctly, the horizon lives in dev and is read at Tx 
time, so it shouldn't require touching ring config. But some drivers 
reallocate rings unconditionally in set_ringparam. Can we highlight 
somewhere (maybe in commit message) that a horizon-only change is meant 
to be non-disruptive? I may be missing the underlying reason though.

> ---
> 
> Changes
>    v1 -> v2
>      - (minor) style: remove period from end of ethtool comments
>      - (minor) style: fix missing space at end of HDS_THRESH comment
>    v1: https://lore.kernel.org/netdev/20260706133433.3142805-2-willemdebruijn.kernel@gmail.com/
> ---
>   Documentation/netlink/specs/ethtool.yaml      |  6 ++++
>   .../networking/net_cachelines/net_device.rst  |  3 +-
>   include/linux/ethtool.h                       |  6 ++++
>   include/linux/netdevice.h                     |  6 ++--
>   .../uapi/linux/ethtool_netlink_generated.h    |  2 ++
>   net/core/rtnetlink.c                          |  2 +-
>   net/ethtool/common.c                          |  3 ++
>   net/ethtool/netlink.h                         |  2 +-
>   net/ethtool/rings.c                           | 30 +++++++++++++++++--
>   net/sched/sch_fq.c                            |  3 +-
>   10 files changed, 55 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/netlink/specs/ethtool.yaml b/Documentation/netlink/specs/ethtool.yaml
> index 5dd4d1b5d94b..48f43dda2e6e 100644
> --- a/Documentation/netlink/specs/ethtool.yaml
> +++ b/Documentation/netlink/specs/ethtool.yaml
> @@ -448,6 +448,12 @@ attribute-sets:
>         -
>           name: hds-thresh-max
>           type: u32
> +      -
> +        name: pacing-offload-horizon
> +        type: u32
> +      -
> +        name: pacing-offload-horizon-max
> +        type: u32
>   
>     -
>       name: mm-stat
> diff --git a/Documentation/networking/net_cachelines/net_device.rst b/Documentation/networking/net_cachelines/net_device.rst
> index 512f6d6fa3d8..3b9ce1c6e105 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                                 pacing_offload_horizon
> +u32                                 max_pacing_offload_horizon
>   struct_napi_config*                 napi_config
>   unsigned_long                       gro_flush_timeout
>   u32                                 napi_defer_hard_irqs
> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
> index 5d491a98265e..adeebbf53dad 100644
> --- a/include/linux/ethtool.h
> +++ b/include/linux/ethtool.h
> @@ -85,6 +85,8 @@ enum {
>    * @tx_push_buf_max_len: Maximum allowed size of TX push buffer
>    * @hds_thresh: Packet size threshold for header data split (HDS)
>    * @hds_thresh_max: Maximum supported setting for @hds_threshold
> + * @pacing_offload_horizon: pacing offload horizon value in usec
> + * @max_pacing_offload_horizon: max pacing offload horizon value in usec
>    *
>    */
>   struct kernel_ethtool_ringparam {
> @@ -97,6 +99,8 @@ struct kernel_ethtool_ringparam {
>   	u32	tx_push_buf_max_len;
>   	u32	hds_thresh;
>   	u32	hds_thresh_max;
> +	u32	pacing_offload_horizon;
> +	u32	max_pacing_offload_horizon;
>   };
>   
>   /**
> @@ -108,6 +112,7 @@ struct kernel_ethtool_ringparam {
>    * @ETHTOOL_RING_USE_TX_PUSH_BUF_LEN: capture for setting tx_push_buf_len
>    * @ETHTOOL_RING_USE_TCP_DATA_SPLIT: capture for setting tcp_data_split
>    * @ETHTOOL_RING_USE_HDS_THRS: capture for setting header-data-split-thresh
> + * @ETHTOOL_RING_USE_PACING_OFFLOAD_HORIZON: capture for setting pacing offload horizon
>    */
>   enum ethtool_supported_ring_param {
>   	ETHTOOL_RING_USE_RX_BUF_LEN		= BIT(0),
> @@ -117,6 +122,7 @@ enum ethtool_supported_ring_param {
>   	ETHTOOL_RING_USE_TX_PUSH_BUF_LEN	= BIT(4),
>   	ETHTOOL_RING_USE_TCP_DATA_SPLIT		= BIT(5),
>   	ETHTOOL_RING_USE_HDS_THRS		= BIT(6),
> +	ETHTOOL_RING_USE_PACING_OFFLOAD_HORIZON	= BIT(7),
>   };
>   
>   #define __ETH_RSS_HASH_BIT(bit)	((u32)1 << (bit))
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 8db25b79573e..3c2ed45b55dc 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -2129,7 +2129,8 @@ enum netdev_reg_state {
>    *	@dpll_pin: Pointer to the SyncE source pin of a DPLL subsystem,
>    *		   where the clock is recovered.
>    *
> - *	@max_pacing_offload_horizon: max EDT offload horizon in nsec.
> + *	@pacing_offload_horizon: active pacing offload horizon in usec.
> + *	@max_pacing_offload_horizon: max pacing offload horizon in usec.
>    *	@napi_config: An array of napi_config structures containing per-NAPI
>    *		      settings.
>    *	@num_napi_configs:	number of allocated NAPI config structs,
> @@ -2552,7 +2553,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			pacing_offload_horizon;
> +	u32			max_pacing_offload_horizon;
>   	struct napi_config	*napi_config;
>   	u32			num_napi_configs;
>   	u32			napi_defer_hard_irqs;
> diff --git a/include/uapi/linux/ethtool_netlink_generated.h b/include/uapi/linux/ethtool_netlink_generated.h
> index 8134baf7860f..6f1a2fa36157 100644
> --- a/include/uapi/linux/ethtool_netlink_generated.h
> +++ b/include/uapi/linux/ethtool_netlink_generated.h
> @@ -193,6 +193,8 @@ enum {
>   	ETHTOOL_A_RINGS_TX_PUSH_BUF_LEN_MAX,
>   	ETHTOOL_A_RINGS_HDS_THRESH,
>   	ETHTOOL_A_RINGS_HDS_THRESH_MAX,
> +	ETHTOOL_A_RINGS_PACING_OFFLOAD_HORIZON,
> +	ETHTOOL_A_RINGS_PACING_OFFLOAD_HORIZON_MAX,
>   
>   	__ETHTOOL_A_RINGS_CNT,
>   	ETHTOOL_A_RINGS_MAX = (__ETHTOOL_A_RINGS_CNT - 1)
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index 31c65a545a10..2c7a245c5ffc 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -2155,7 +2155,7 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb,
>   	    nla_put_u32(skb, IFLA_TSO_MAX_SEGS,
>   			READ_ONCE(dev->tso_max_segs)) ||
>   	    nla_put_uint(skb, IFLA_MAX_PACING_OFFLOAD_HORIZON,
> -			 READ_ONCE(dev->max_pacing_offload_horizon)) ||
> +			 (u64)READ_ONCE(dev->max_pacing_offload_horizon) * NSEC_PER_USEC) ||
>   #ifdef CONFIG_RPS
>   	    nla_put_u32(skb, IFLA_NUM_RX_QUEUES,
>   			READ_ONCE(dev->num_rx_queues)) ||
> diff --git a/net/ethtool/common.c b/net/ethtool/common.c
> index 23db40618fed..5c2323aaa02b 100644
> --- a/net/ethtool/common.c
> +++ b/net/ethtool/common.c
> @@ -954,6 +954,9 @@ void ethtool_ringparam_get_cfg(struct net_device *dev,
>   	/* Driver gives us current state, we want to return current config */
>   	kparam->tcp_data_split = dev->cfg->hds_config;
>   	kparam->hds_thresh = dev->cfg->hds_thresh;
> +
> +	kparam->pacing_offload_horizon = dev->pacing_offload_horizon;
> +	kparam->max_pacing_offload_horizon = dev->max_pacing_offload_horizon;
>   }
>   
>   static void ethtool_init_tsinfo(struct kernel_ethtool_ts_info *info)
> diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h
> index 3e969a070f9f..ae7b651c658f 100644
> --- a/net/ethtool/netlink.h
> +++ b/net/ethtool/netlink.h
> @@ -494,7 +494,7 @@ extern const struct nla_policy ethnl_features_set_policy[ETHTOOL_A_FEATURES_WANT
>   extern const struct nla_policy ethnl_privflags_get_policy[ETHTOOL_A_PRIVFLAGS_HEADER + 1];
>   extern const struct nla_policy ethnl_privflags_set_policy[ETHTOOL_A_PRIVFLAGS_FLAGS + 1];
>   extern const struct nla_policy ethnl_rings_get_policy[ETHTOOL_A_RINGS_HEADER + 1];
> -extern const struct nla_policy ethnl_rings_set_policy[ETHTOOL_A_RINGS_HDS_THRESH_MAX + 1];
> +extern const struct nla_policy ethnl_rings_set_policy[ETHTOOL_A_RINGS_PACING_OFFLOAD_HORIZON_MAX + 1];
>   extern const struct nla_policy ethnl_channels_get_policy[ETHTOOL_A_CHANNELS_HEADER + 1];
>   extern const struct nla_policy ethnl_channels_set_policy[ETHTOOL_A_CHANNELS_COMBINED_COUNT + 1];
>   extern const struct nla_policy ethnl_coalesce_get_policy[ETHTOOL_A_COALESCE_HEADER + 1];
> diff --git a/net/ethtool/rings.c b/net/ethtool/rings.c
> index 9054c89c5d7b..52158572fe10 100644
> --- a/net/ethtool/rings.c
> +++ b/net/ethtool/rings.c
> @@ -42,6 +42,8 @@ static int rings_prepare_data(const struct ethnl_req_info *req_base,
>   
>   	data->kernel_ringparam.tcp_data_split = dev->cfg->hds_config;
>   	data->kernel_ringparam.hds_thresh = dev->cfg->hds_thresh;
> +	data->kernel_ringparam.pacing_offload_horizon = dev->pacing_offload_horizon;
> +	data->kernel_ringparam.max_pacing_offload_horizon = dev->max_pacing_offload_horizon;
>   
>   	dev->ethtool_ops->get_ringparam(dev, &data->ringparam,
>   					&data->kernel_ringparam, info->extack);
> @@ -69,7 +71,9 @@ static int rings_reply_size(const struct ethnl_req_info *req_base,
>   	       nla_total_size(sizeof(u32)) +	/* _RINGS_TX_PUSH_BUF_LEN */
>   	       nla_total_size(sizeof(u32)) +	/* _RINGS_TX_PUSH_BUF_LEN_MAX */
>   	       nla_total_size(sizeof(u32)) +	/* _RINGS_HDS_THRESH */
> -	       nla_total_size(sizeof(u32));	/* _RINGS_HDS_THRESH_MAX*/
> +	       nla_total_size(sizeof(u32)) +	/* _RINGS_HDS_THRESH_MAX */
> +	       nla_total_size(sizeof(u32)) +	/* _RINGS_PACING_OFFLOAD_HORIZON */
> +	       nla_total_size(sizeof(u32));	/* _RINGS_PACING_OFFLOAD_HORIZON_MAX */
>   }
>   
>   static int rings_fill_reply(struct sk_buff *skb,
> @@ -121,7 +125,12 @@ static int rings_fill_reply(struct sk_buff *skb,
>   	     (nla_put_u32(skb, ETHTOOL_A_RINGS_HDS_THRESH,
>   			  kr->hds_thresh) ||
>   	      nla_put_u32(skb, ETHTOOL_A_RINGS_HDS_THRESH_MAX,
> -			  kr->hds_thresh_max))))
> +			  kr->hds_thresh_max))) ||
> +	    ((supported_ring_params & ETHTOOL_RING_USE_PACING_OFFLOAD_HORIZON) &&
> +	     (nla_put_u32(skb, ETHTOOL_A_RINGS_PACING_OFFLOAD_HORIZON,
> +			  kr->pacing_offload_horizon) ||
> +	      nla_put_u32(skb, ETHTOOL_A_RINGS_PACING_OFFLOAD_HORIZON_MAX,
> +			  kr->max_pacing_offload_horizon))))
>   		return -EMSGSIZE;
>   
>   	return 0;
> @@ -144,6 +153,7 @@ const struct nla_policy ethnl_rings_set_policy[] = {
>   	[ETHTOOL_A_RINGS_RX_PUSH]		= NLA_POLICY_MAX(NLA_U8, 1),
>   	[ETHTOOL_A_RINGS_TX_PUSH_BUF_LEN]	= { .type = NLA_U32 },
>   	[ETHTOOL_A_RINGS_HDS_THRESH]		= { .type = NLA_U32 },
> +	[ETHTOOL_A_RINGS_PACING_OFFLOAD_HORIZON]	= { .type = NLA_U32 },
>   };
>   
>   static int
> @@ -177,6 +187,14 @@ ethnl_set_rings_validate(struct ethnl_req_info *req_info,
>   		return -EOPNOTSUPP;
>   	}
>   
> +	if (tb[ETHTOOL_A_RINGS_PACING_OFFLOAD_HORIZON] &&
> +	    !(ops->supported_ring_params & ETHTOOL_RING_USE_PACING_OFFLOAD_HORIZON)) {
> +		NL_SET_ERR_MSG_ATTR(info->extack,
> +				    tb[ETHTOOL_A_RINGS_PACING_OFFLOAD_HORIZON],
> +				    "setting pacing offload horizon is not supported");
> +		return -EOPNOTSUPP;
> +	}
> +
>   	if (tb[ETHTOOL_A_RINGS_CQE_SIZE] &&
>   	    !(ops->supported_ring_params & ETHTOOL_RING_USE_CQE_SIZE)) {
>   		NL_SET_ERR_MSG_ATTR(info->extack,
> @@ -246,6 +264,8 @@ ethnl_set_rings(struct ethnl_req_info *req_info, struct genl_info *info)
>   			 tb[ETHTOOL_A_RINGS_TX_PUSH_BUF_LEN], &mod);
>   	ethnl_update_u32(&kernel_ringparam.hds_thresh,
>   			 tb[ETHTOOL_A_RINGS_HDS_THRESH], &mod);
> +	ethnl_update_u32(&kernel_ringparam.pacing_offload_horizon,
> +			 tb[ETHTOOL_A_RINGS_PACING_OFFLOAD_HORIZON], &mod);
>   	if (!mod)
>   		return 0;
>   
> @@ -281,6 +301,9 @@ ethnl_set_rings(struct ethnl_req_info *req_info, struct genl_info *info)
>   		err_attr = tb[ETHTOOL_A_RINGS_TX];
>   	else if (kernel_ringparam.hds_thresh > kernel_ringparam.hds_thresh_max)
>   		err_attr = tb[ETHTOOL_A_RINGS_HDS_THRESH];
> +	else if (kernel_ringparam.pacing_offload_horizon >
> +		 kernel_ringparam.max_pacing_offload_horizon)
> +		err_attr = tb[ETHTOOL_A_RINGS_PACING_OFFLOAD_HORIZON];
>   	else
>   		err_attr = NULL;
>   	if (err_attr) {
> @@ -302,6 +325,9 @@ ethnl_set_rings(struct ethnl_req_info *req_info, struct genl_info *info)
>   
>   	ret = dev->ethtool_ops->set_ringparam(dev, &ringparam,
>   					      &kernel_ringparam, info->extack);
> +	if (!ret)
> +		dev->pacing_offload_horizon = kernel_ringparam.pacing_offload_horizon;

Since a driver may be reading dev->pacing_offload_horizon on the hotpath 
without a lock, should we use WRITE_ONCE() here? and have the driver use 
READ_ONCE()?

> +
>   	return ret < 0 ? ret : 1;
>   }
>   
> diff --git a/net/sched/sch_fq.c b/net/sched/sch_fq.c
> index 7cae082a9847..d4cc8bc9fa06 100644
> --- a/net/sched/sch_fq.c
> +++ b/net/sched/sch_fq.c
> @@ -1179,7 +1179,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 <=
> +		    (u64)qdisc_dev(sch)->pacing_offload_horizon * NSEC_PER_USEC) {
>   			WRITE_ONCE(q->offload_horizon, offload_horizon);
>   		} else {
>   			NL_SET_ERR_MSG_MOD(extack, "invalid offload_horizon");