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

[Willem de Bruijn <willemdebruijn.kernel@gmail.com>]

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.

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.
> 
> >  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