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  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, 10 Sep 2020 13:59:07 -0700
From:   Jacob Keller <jacob.e.keller@...el.com>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     netdev@...r.kernel.org, Jiri Pirko <jiri@...lanox.com>,
        Jonathan Corbet <corbet@....net>,
        Michael Chan <michael.chan@...adcom.com>,
        Bin Luo <luobin9@...wei.com>,
        Saeed Mahameed <saeedm@...lanox.com>,
        Leon Romanovsky <leon@...nel.org>,
        Ido Schimmel <idosch@...lanox.com>,
        Danielle Ratson <danieller@...lanox.com>
Subject: Re: [net-next v4 2/5] devlink: convert flash_update to use params
 structure



On 9/9/2020 5:55 PM, Jakub Kicinski wrote:
> On Wed,  9 Sep 2020 15:26:50 -0700 Jacob Keller wrote:
>> The devlink core recently gained support for checking whether the driver
>> supports a flash_update parameter, via `supported_flash_update_params`.
>> However, parameters are specified as function arguments. Adding a new
>> parameter still requires modifying the signature of the .flash_update
>> callback in all drivers.
>>
>> Convert the .flash_update function to take a new `struct
>> devlink_flash_update_params` instead. By using this structure, and the
>> `supported_flash_update_params` bit field, a new parameter to
>> flash_update can be added without requiring modification to existing
>> drivers.
>>
>> As before, all parameters except file_name will require driver opt-in.
>> Because file_name is a necessary field to for the flash_update to make
>> sense, no "SUPPORTED" bitflag is provided and it is always considered
>> valid. All future additional parameters will require a new bit in the
>> supported_flash_update_params bitfield.
> 
> I keep thinking we should also make the core do the
> request_firmware_direct(). What else is the driver gonna do with the file name..
> 
> But I don't want to drag your series out so:
> 
> Reviewed-by: Jakub Kicinski <kuba@...nel.org>
> 

Hmm. What does _direct do? I guess it means it won't fall back to the
userspace helper if it can't find the firmware? It looks like MLX
drivers use it, but others seem to just stick to regular request_firmware.

This seems like an improvement that we can handle in a follow up series
either way. Thanks for the review!

Powered by blists - more mailing lists