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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <a15acdf5-a551-4fb2-9118-770c37b47be6@redhat.com>
Date: Fri, 23 Aug 2024 16:23:30 +0200
From: Paolo Abeni <pabeni@...hat.com>
To: Jiri Pirko <jiri@...nulli.us>
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

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.

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.

>> 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