Subject: Re: [PATCH v3 03/12] net-shapers: implement NL get operation

From: Jakub Kicinski <[email-masked]>
Date: Thu, 22 Aug 2024 15:56:08 -0700

On Thu, 22 Aug 2024 22:30:35 +0200 Paolo Abeni wrote:
> >> I'm not saying this is deal breaker for me. I just think that if the api
> >> is designed to be independent of the object shaper is bound to
> >> (netdev/devlink_port/etc), it would be much much easier to extend in the
> >> future. If you do everything netdev-centric from start, I'm sure no
> >> shaper consolidation will ever happen. And that I thought was one of the
> >> goals.
> >>
> >> Perhaps Jakub has opinion.  
> > 
> > I think you and I are on the same page :) Other than the "reference
> > object" (netdev / devlink port) the driver facing API should be
> > identical. Making it possible for the same driver code to handle
> > translating the parameters into HW config / FW requests, whether
> > they shape at the device (devlink) or port (netdev) level.
> > 
> > Shaper NL for netdevs is separate from internal representation and
> > driver API in my mind. My initial ask was to create the internal
> > representation first, make sure it can express devlink and handful of
> > exiting netdev APIs, and only once that's merged worry about exposing
> > it via a new NL.
> > 
> > I'm not opposed to showing devlink shapers in netdev NL (RO as you say)
> > but talking about it now strikes me as cart before the horse.  
> 
> FTR, I don't see both of you on the same page ?!?
> 
> I read the above as Jiri's preference is a single ndo set to control
> both devlink and device shapers, while I read Jakub's preference as for
> different sets of operations that will use the same arguments to specify
> the shaper informations.

Jiri replied:

  > which kind of object should implement the ndo_shaper_ops callbacks?

  Whoever implements the shaper in driver. If that is net_device tight
  shaper, driver should work with net_device. If that is devlink port
  related shaper, driver should work on top of devlink port based api.

I interpret this as having two almost identical versions of shaper ops,
the only difference is that one takes netdev and the other devlink port.
We could simplify it slightly, and call the ndo for getting devlink
port from netdev, and always pass devlink port in?

I _think_ (but I'm not 100% sure) that Jiri does _not_ mean that we
would be able to render the internal shaper tree as ops for the
existing devlink rate API. Because that may cause scope creep,
inconsistencies and duplication.

> Or to phrase the above differently, Jiri is focusing on the shaper
> "binding" (how to locate/access it) while Jakub is focusing on the 
> shaper "info" (content/definition/attributes). Please correct me If I 
> misread something.
> 
> Still for the record, I interpret the current proposal as not clashing
> with Jakub's preference, and being tolerated from Jiri, again please 
> correct me if I read too far.

One more thing, Jiri said:

  If you do everything netdev-centric from start, I'm sure no shaper
  consolidation will ever happen. And that I thought was one of the goals.

Consolidation was indeed one of the goals, and I share Jiri's concern :(