netdev - Re: [patch net-next v3 3/3] devlink: implement flash status monitoring

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: <249eca9b-e62a-df02-7593-4492daf39183@gmail.com>
Date:   Mon, 10 Jun 2019 11:30:24 -0600
From:   David Ahern <dsahern@...il.com>
To:     Jakub Kicinski <jakub.kicinski@...ronome.com>
Cc:     Jiri Pirko <jiri@...nulli.us>, netdev@...r.kernel.org,
        davem@...emloft.net, mlxsw@...lanox.com, sthemmin@...rosoft.com,
        saeedm@...lanox.com, leon@...nel.org, f.fainelli@...il.com
Subject: Re: [patch net-next v3 3/3] devlink: implement flash status
 monitoring

On 6/10/19 11:24 AM, Jakub Kicinski wrote:
> On Mon, 10 Jun 2019 11:09:19 -0600, David Ahern wrote:
>> On 6/4/19 7:44 AM, Jiri Pirko wrote:
>>> diff --git a/man/man8/devlink-dev.8 b/man/man8/devlink-dev.8
>>> index 1804463b2321..1021ee8d064c 100644
>>> --- a/man/man8/devlink-dev.8
>>> +++ b/man/man8/devlink-dev.8
>>> @@ -244,6 +244,17 @@ Sets the parameter internal_error_reset of specified devlink device to true.
>>>  devlink dev reload pci/0000:01:00.0
>>>  .RS 4
>>>  Performs hot reload of specified devlink device.
>>> +.RE
>>> +.PP
>>> +devlink dev flash pci/0000:01:00.0 file firmware.bin
>>> +.RS 4
>>> +Flashes the specified devlink device with provided firmware file name. If the driver supports it, user gets updates about the flash status. For example:
>>> +.br
>>> +Preparing to flash
>>> +.br
>>> +Flashing 100%
>>> +.br
>>> +Flashing done
>>>  
>>>  .SH SEE ALSO
>>>  .BR devlink (8),  
>>
>> something is missing here from a user perspective at least:
>>
>> root@...-2700-05:~# ./devlink dev
>> pci/0000:03:00.0
>>
>> root@...-2700-05:~# ./devlink dev flash pci/0000:03:00.0 file
>> /lib/firmware/mellanox/mlxsw_spectrum-13.2000.1122.mfa2
>> devlink answers: No such file or directory
>>
>> root@...-2700-05:~# ls -l
>> /lib/firmware/mellanox/mlxsw_spectrum-13.2000.1122.mfa2
>> -rw-r--r-- 1 cumulus 1001 994184 May 14 22:44
>> /lib/firmware/mellanox/mlxsw_spectrum-13.2000.1122.mfa2
>>
>>
>> Why the 'no such file' response when the file exists?
> 
> I think the FW loader prepends /lib/firmware to the path (there is a
> CONFIG_ for the search paths, and / is usually not on it).  Perhaps try:
> 
> ./devlink dev flash pci/0000:03:00.0 file mellanox/mlxsw_spectrum-13.2000.1122.mfa2
> 

that worked, but if the user specifies fullpath that is confusing. So at
a minimum the documentation needs to be clear about the paths.

But, why the path limitation? why not allow a user to load a file from
any directory? For mlxsw at least the file in /lib/firmware will be
loaded automagically, so forcing the file to always be in /lib/firmware
seems counterintuitive when using a command to specify the file to load.