Re: [Intel-wired-lan] [PATCH iwl-next v4 0/5] iavf: Add devlink and devlink rate support'

Jakub Kicinski
2023-12-15T22:41:57Z

On Fri, 15 Dec 2023 12:06:52 +0100 Paolo Abeni wrote:
> > uAPI aside, why would we use ndo_setup_tc(TC_SETUP_QDISC_TBF)
> > to implement common basis?
> >
> > Is it not cleaner to have a separate driver API, with its ops
> > and capabilities?
>
> We understand one of the end goal is consolidating the existing rate-
> related in kernel interfaces. Adding a new one does not feel a good
> starting to reach that goal, see [1] & [2] ;)

ndo_setup_tc(TC_SETUP_QDISC_TBF) is a new API, too, very much so.
These attempts to build on top of existing interfaces with small
tweaks is leading us to a fragmented and incompatible driver landscape.

I explained before (perhaps on the netdev call) - Qdiscs have two
different offload models. "local" and "switchdev", here we want "local"
AFAIU and TBF only has "switchdev" offload (take a look at the enqueue
method and which drivers support it today).

"We'll extend TBF" is very much adding a new API. You'll have to add
"local offload" support in TBF and no NIC driver today supports it.
I'm not saying TBF is bad, but I disagree that it's any different
than a new NDO for all practical purposes.

> ndo_setup_tc() feels like the natural choice for H/W offload and TBF
> is the existing interface IMHO nearest to the requirements here.

I question whether something as basic as scheduling and ACLs should
follow the "offload SW constructs" mantra. You are exposed to more
diverse users so please don't hesitate to disagree, but AFAICT
the transparent offload (user installs SW constructs and if offload
is available - offload, otherwise use SW is good enough) has not
played out like we have hoped.

Let's figure out what is the abstract model of scheduling / shaping
within a NIC that we want to target. And then come up with a way of
representing it in SW. Not which uAPI we can shoehorn into the use
case.