On 8/26/24 11:31, Jiri Pirko wrote:

Fri, Aug 23, 2024 at 04:23:30PM CEST, pabeni@redhat.com wrote:

quoted

On 8/23/24 15:36, Jiri Pirko wrote:

quoted

Fri, Aug 23, 2024 at 02:58:27PM CEST, pabeni@redhat.com wrote:

quoted

I personally think it would be much cleaner to have 2 separate set of
operations, with exactly the same semantic and argument list, except for the
first argument (struct net_device or struct devlink).

I think it is totally subjective. You like something, I like something
else. Both works. The amount of duplicity and need to change same
things on multiple places in case of bugfixes and extensions is what I
dislike on the 2 separate sets.

My guestimate is that the amount of deltas caused by bugfixes and extensions
will be much different in practice with the two approaches.

I guess that even with the net_shaper_ops between devlink and net_device,
there will be different callbacks implementation for devlink and net_device,
right?

If so, the differentiated operation list between devlink and net_device will
trade a:

{
	struct {net_device, netlink} =
net_shaper_binding_{netdevice_netlink}(binding);

preamble in every callback of every driver for a single additional operations
set definition.

So?

The amount of code we would need to change in case of core changes would 
probably be similar with either the differentiated operations list or not.

quoted

It will at least scale better with the number of driver implementing the
interface.

quoted

Plus, there might be another binding in
the future, will you copy the ops struct again then?

Yes. Same reasons of the above.

What's stopping anyone from diverging these 2-n sets? I mean, the whole
purpose it unification and finding common ground. Once you have ops
duplicated, sooner then later someone does change in A but ignore B.
Having the  "preamble" in every callback seems like very good tradeoff
to prevent this scenario.

The main fact is that we do not agree on the above point - unify the 
shaper_ops between struct net_device and struct devlink.

I think a 3rd party opinion could help moving forward.
@Jakub could you please share your view here?

Thanks,

Paolo