On 8/28/24 08:40, Jiri Pirko wrote:

Makes sense?

Almost! Tacking aside the (very significant) differences between your 
proposition and Jakub’s, we can't use devlink port here, just devlink, 
or we will have to drop the cache too[1]. Specific devlink port shapers 
will be reached via different handles (scope/id).

Additionally, I think we don't need strictly the ‘binding’ nested 
attribute to extend the NL API with different binding objects (devlink), 
we could append the new attributes needed to support (identify) devlink 
at the end of the net shaper attributes list. I agree that would be 
likely less ‘nice’.

What about:
- Refactor the core and the driver api to support the ‘binding’ thing
- Update the NL definition to nest the ‘ifindex’ attribute under the 
‘binding’ one. No mention/reference to devlink yet, so most of the 
documentation will be unchanged.
- devlink support will not be included, but there should be enough 
ground paved for it.

?

Thanks,

Paolo

[1] the cache container belongs to the ‘entry point’ inside the shaper 
hierarchy - i.e. currently, the struct net_device. If we add a 
devlink_port ‘entry point’, the cache there will have to manage even the 
shaper for devlink ports group. When accessing a group containing 
multiple ports, we will get multiple inconsistent cache values.