On 8/16/24 11:16, Jiri Pirko wrote:

Fri, Aug 16, 2024 at 10:52:58AM CEST, pabeni@redhat.com wrote:

quoted

On 8/14/24 10:37, Jiri Pirko wrote:

quoted

Tue, Aug 13, 2024 at 05:17:12PM CEST, pabeni@redhat.com wrote:

quoted

On 8/1/24 15:42, Jiri Pirko wrote:
[...]

quoted

quoted

int net_shaper_nl_get_doit(struct sk_buff *skb, struct genl_info *info)
{
-	return -EOPNOTSUPP;
+	struct net_shaper_info *shaper;
+	struct net_device *dev;
+	struct sk_buff *msg;
+	u32 handle;
+	int ret;
+
+	ret = fetch_dev(info, &dev);

This is quite net_device centric. Devlink rate shaper should be
eventually visible throught this api as well, won't they? How do you
imagine that?

I'm unsure we are on the same page. Do you foresee this to replace and
obsoleted the existing devlink rate API? It was not our so far.

Driver-api-wise, yes. I believe that was the goal, to have drivers to
implement one rate api.

I initially underlooked at this point, I'm sorry.

Re-reading this I think we are not on the same page.

The net_shaper_ops are per network device operations: they are aimed (also)
at consolidating network device shaping related callbacks, but they can't
operate on non-network device objects (devlink port).

Why not?

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?

Side note: I experimented adding the 'binging' abstraction to this API 
and gives a quite significant uglification to the user syntax (due to 
the additional nesting required) and the code.

Still, if there is a very strong need for controlling devlink rate via 
this API _and_ we can assume that each net_device "relates" 
(/references/is connected to) at most a single devlink object (out of 
sheer ignorance on my side I'm unsure about this point, but skimming 
over the existing implementations it looks so), the current API 
definition would be IMHO sufficient and clean enough to reach for both 
devlink port rate objects and devlink node rate objects.

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.

I think such scopes definition should come with related implementation, 
e.g. not with this series.

Thanks,

Paolo