netdev - Re: [PATCH v4 net-next 1/8] sfc: add devlink support for ef100

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]

Date:   Thu, 2 Feb 2023 06:41:45 +0000
From:   "Lucero Palau, Alejandro" <alejandro.lucero-palau@....com>
To:     Jakub Kicinski <kuba@...nel.org>, Jiri Pirko <jiri@...nulli.us>
CC:     "Lucero Palau, Alejandro" <alejandro.lucero-palau@....com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-net-drivers (AMD-Xilinx)" <linux-net-drivers@....com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "pabeni@...hat.com" <pabeni@...hat.com>,
        "edumazet@...gle.com" <edumazet@...gle.com>,
        "habetsm.xilinx@...il.com" <habetsm.xilinx@...il.com>,
        "ecree.xilinx@...il.com" <ecree.xilinx@...il.com>,
        "linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
        "corbet@....net" <corbet@....net>,
        "jiri@...dia.com" <jiri@...dia.com>
Subject: Re: [PATCH v4 net-next 1/8] sfc: add devlink support for ef100



On 2/1/23 19:01, Jakub Kicinski wrote:
> On Wed, 1 Feb 2023 10:07:33 +0100 Jiri Pirko wrote:
>>> This is due to the recommended/required devlink lock/unlock during
>>> driver initialization/removal.
>>>
>>> I think it is better to keep the lock/unlock inside the specific driver
>>> devlink code, and the functions naming reflects a time window when
>>> devlink related/dependent processing is being done.
>>>
>>> I'm not against changing this, maybe adding the lock/unlock suffix would
>>> be preferable?:
>>>
>>> int efx_probe_devlink_and_lock(struct efx_nic *efx);
>>> void efx_probe_devlink_unlock(struct efx_nic *efx);
>>> void efx_fini_devlink_lock(struct efx_nic *efx);
>>> void efx_fini_devlink_and_unlock(struct efx_nic *efx);
>> Sounds better. Thanks!
> FWIW I'd just take the devl lock in the main driver code.
> devlink should be viewed as a layer between bus and driver rather
> than as another subsystem the driver registers with. Otherwise reloads
> and port creation get awkward.
>
> But the above sounds okay, too.

That is what I have tried to do with these extra functions invoked from 
the main driver code.

The problem is one part of the protection needs to be done inside the 
driver's devlink code (if we want to avoid different calls for 
allocating then registering or unregistering then releasing), so it 
looked awkward to me the other protection part being directly in main 
driver code. So, I think the extra functions are worth for avoiding 
confusion.