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: <20230329192733.7473953c@kernel.org>
Date:   Wed, 29 Mar 2023 19:27:33 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Shannon Nelson <shannon.nelson@....com>
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 Wed, 29 Mar 2023 13:53:23 -0700 Shannon Nelson wrote:
> 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.

It is precisely the painful experience of dealing with those drivers
when refactoring devlink code which makes me ask you to do it right.

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

Not all of the code, but don't wrap parts of probe/remove into
out-of-sight helpers. It will lead to other devlink code collecting
in the same functions regardless of whether it's the right stage of 
initialization. Having devlink.c as an entry point for the ops is 
perfectly fine, OTOH.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ