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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZsxLa0Ut7bWc0OmQ@nanopsycho.orion>
Date: Mon, 26 Aug 2024 11:31:23 +0200
From: Jiri Pirko <jiri@...nulli.us>
To: Paolo Abeni <pabeni@...hat.com>
Cc: Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org,
	Madhu Chittim <madhu.chittim@...el.com>,
	Sridhar Samudrala <sridhar.samudrala@...el.com>,
	Simon Horman <horms@...nel.org>,
	John Fastabend <john.fastabend@...il.com>,
	Sunil Kovvuri Goutham <sgoutham@...vell.com>,
	Jamal Hadi Salim <jhs@...atatu.com>
Subject: Re: [PATCH v3 03/12] net-shapers: implement NL get operation

Fri, Aug 23, 2024 at 04:23:30PM CEST, pabeni@...hat.com wrote:
>On 8/23/24 15:36, Jiri Pirko wrote:
>> Fri, Aug 23, 2024 at 02:58:27PM CEST, pabeni@...hat.com wrote:
>> > 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?

>
>It will at least scale better with the number of driver implementing the
>interface.
>
>> 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 driver implementation could still de-duplicate a lot of code, as far as
>> > the shaper-related arguments are the same.
>> > 
>> > Side note, if the intention is to allow the user to touch/modify the
>> > queue-level and queue-group-level shapers via the devlink object? if that is
>> > the intention, we will need to drop the shaper cache and (re-)introduce a
>> > get() callback, as the same shaper could be reached via multiple
>> > binding/handle pairs and the core will not know all of such pairs for a given
>> > shaper.
>> 
>> That is a good question, I don't know. But gut feeling is "no".
>
>Well, at least that is not in the direction of unlimited amount of additional
>time and pain ;)
>
>Thanks,
>
>Paolo
>