Re: [PATCH net-next v6 1/6] net: rtnetlink: add pacing_offload_horizon attribute to net_device

Jakub Kicinski wrote:

On Wed, 12 Aug 2026 22:03:56 -0400 Willem de Bruijn wrote:

quoted

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.

Perhaps I don't understand how dev->pacing_offload_horizon
would function as a 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.

quoted

 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