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

[Jiri Pirko <jiri@resnulli.us>]

Mon, Aug 19, 2024 at 06:52:22PM CEST, pabeni@redhat.com wrote:
>On 8/19/24 13:53, Jiri Pirko wrote:
>> Mon, Aug 19, 2024 at 11:33:28AM CEST, pabeni@redhat.com wrote:
>> > Isn't the whole point of devlink to configure objects that are directly
>> > related to any network device? Would be somewhat awkward accessing devlink
>> > port going through some net_device?
>> 
>> I'm not sure why you are asking that. I didn't suggest anything like
>> that. On contrary, as your API is netdev centric, I suggested to
>> disconnect from netdev to the shapers could be used not only with them.
>
>ndo_shaper_ops are basically net_device ndo. Any implementation of them will
>operate 'trough some net_device'.

I know, I see that in the code. But the question is, does it have to be
like that?

>
>I'm still not sure which one of the following you mean:
>
>1) the shaper NL interface must be able to manage devlink (rate) objects. The
>core will lookup the devlink obj and use devlink_ops to do the change.
>
>2) the shaper NL interface must be able to manage devlink (rate) objects, the
>core will use ndo_shaper_ops to do the actual change.
>
>3) something else?

I don't care about the shaper NL in case of devlink rate objects. I care
more about in-kernel api. I see shaper NL as one of the UAPIs to consume
the shaper infrastructure. The devlink rate is another one. If the
devlink rate shapers are visible over shaper NL, IDK. They may be RO
perhaps.


>
>In my previous reply, I assumed you wanted option 2). If so, which kind of
>object should implement the ndo_shaper_ops callbacks? net_device? devlink?
>other?

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.


>
>> This is what I understood was a plan from very beginning.
>
>Originally the scope was much more limited than what defined here. Jakub
>asked to implement an interface capable to unify the network device
>shaping/rate related callbacks.

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.


>
>In a previous revision, I stretched that to cover objects "above" the network
>device level (e.g. PF/VF/SFs groups), but then I left them out because:
>
>- it caused several inconsistencies (among other thing we can't use the
>'shaper cache' there and Jakub wants the cache in place).
>- we already have devlink for that.
>
>> > We could define additional scopes for each of such objects and use the id to
>> > discriminate the specific port or node within the relevant devlink.
>> 
>> But you still want to use some netdevice as a handle IIUC, is that
>> right?
>
>The revision of the series that I hope to share soon still used net_device
>ndos. Shapers are identified within the network device by an handle.
>
>Cheers,
>
>Paolo
>