netdev - Re: [PATCH v6 net-next 01/14] pds_core: initial framework for pds

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: <45c28c76-688c-5f49-4a30-f6cb6eab0dce@amd.com>
Date:   Wed, 29 Mar 2023 13:53:23 -0700
From:   Shannon Nelson <shannon.nelson@....com>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     brett.creeley@....com, davem@...emloft.net, netdev@...r.kernel.org,
        drivers@...sando.io, leon@...nel.org, jiri@...nulli.us
Subject: Re: [PATCH v6 net-next 01/14] pds_core: initial framework for
 pds_core PF driver

On 3/28/23 3:17 PM, Jakub Kicinski wrote:
> On Mon, 27 Mar 2023 23:19:28 -0700 Shannon Nelson wrote:
>>> What are you "abstracting away", exactly? Which "later patch"?
>>> I'm not going to read the 5k LoC submission to figure out what
>>> you're trying to say :(
>>
>> I'm saying that more code is added in later patches around the
>> devlink_register() for the health (patch 4) and parameters (patch 11).
>> This allows me to have a simple line in the main probe logic that does
>> the devlink-register related things, and then have the details collected
>> together off to the side.
> 
> It's not supposed to be off to the side, that's my point.
> It's a central interface for device control.
> 
>> Obviously, when I update the code for using the devl_* interfaces and
>> explicit locking, those two patches will change a little.

The devlink alloc and registration are obviously a part of the probe and 
thus device control setup, so I’m not sure why this is an issue.

As is suggested in coding style, the smaller functions make for easier 
reading, and keeps the related locking in a nice little package.  Having 
the devlink registration code gathered in one place in the devlink.c 
file seems to follow most conventions, which then allows the helper 
functions to be static to that file.  This seems to be what about half 
the drivers that use devlink have chosen to do.

Sure, I could move that function into main.c and make the helper 
functions more public if that is what you’re looking for.  This seems to 
be the choice for a few of the other drivers.

Or are you looking to have all of the devlink.c code get rolled into main.c?

sln