netdev - Re: [RFC PATCH net-next 6/6] ice: implement devlink parameters to control flash update

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: <58840317-e818-af52-352a-19008b89bee7@intel.com>
Date:   Thu, 16 Jul 2020 14:29:40 -0700
From:   Jacob Keller <jacob.e.keller@...el.com>
To:     Jakub Kicinski <kubakici@...pl>
Cc:     netdev@...r.kernel.org, Jiri Pirko <jiri@...nulli.us>,
        Jesse Brandeburg <jesse.brandeburg@...el.com>,
        Tom Herbert <tom@...bertland.com>
Subject: Re: [RFC PATCH net-next 6/6] ice: implement devlink parameters to
 control flash update

On 7/15/2020 5:21 PM, Jacob Keller wrote:
> 
> 
> On 7/15/2020 4:23 PM, Jakub Kicinski wrote:
>> On Wed, 15 Jul 2020 14:41:04 -0700 Jacob Keller wrote:
>>> To summarize this discussion, the next spin will have the following changes:
>>>
>>> 1) remove all parameters except for the preservation_level. Both
>>> ignore_pending_flash_update and allow_downgrade_on_flash_update will be
>>> removed and change the default behavior to the most accepting case:
>>> updates will always be tried even if firmware says its a downgrade, and
>>> we will always cancel a pending update. We will now expect user space
>>> tools to be aware of this and handle the equivalent options themselves
>>> if they desire.
>>>
>>> 2) reset_after_flash_update will be removed, and we will replace it with
>>> a new interface, perhaps like the devlink reset command suggested in
>>> another thread.
>>>
>>> 3) preservation_level will remain, but I have updated the documentation
>>> slightly.
>>
>> Okay, then. But let's make it a parameter to the flash update operation
>> (extend the uAPI), rather than a devlink param, shall we?
>>
> 
> Ok, that seems reasonable. Ofcourse we'll need to find something generic
> enough that it can be re-used and isn't driver specific.
> 

Hi Jakub,

I think I have something that will be more clear and will be sending a
new RFC with the change this afternoon:

an extension to the DEVLINK_CMD_FLASH_UPDATE with a new parameter,
"overwrite" with these values:

a) "nothing" (or maybe, "firmware-only" or "binary-only"?, need a way to
clarify difference between settings/vital data and firmware program
binary) will request that we do not overwrite any settings or fields.
This is equivalent to the "PRESERVE_ALL" I had in the original proposal,
where we will maintain all settings and all vital data, but update the
firmware binary.

b) "settings" will request that the firmware overwrite all the settings
fields with the contents from the new image. However, vital data such as
the PCI Serial ID, VPD section, MAC Addresses, and similar "static" data
will be kept (not overwritten). This is the same as the
"PRESERVE_LIMITED" option I had in the original proposal

c) "all" or "everything" will request that firmware overwrite all
contents of the image. This means all settings and all vital data will
be overwritten by the contents in the new image.

d) if we need it, a "default" that would be the current behavior of
doing whatever the driver default is? (since it's not clear to me what
other implementations do but perhaps they all behavior as either
"nothing" or "all"?

I think I agree that "factory" settings doesn't really belong here, and
I will try to push for finding an alternative way to allow access to
that behavior. If we wanted it we could use "from_factory" to request
that we overwrite the settings and  vital data "from" the factory
portion, but I think that is pushing the boundary here a bit...

I am aiming to have a new patch up with this proposal

Thanks,
Jake