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

Mohsin Bashir wrote:

On 7/22/26 1:43 PM, Willem de Bruijn wrote:

quoted

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.

What do you mean by underlying reason? 

quoted

@@ -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()?

Will fix in v3