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

[Jacob Keller <jacob.e.keller@intel.com>]

On 7/16/2020 2:42 PM, Jakub Kicinski wrote:
> On Thu, 16 Jul 2020 14:29:40 -0700 Jacob Keller wrote:
>> On 7/15/2020 5:21 PM, Jacob Keller wrote:
>>> 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.
> 
> Sorry but I'm still not 100% sure of what the use for this option is
> beyond an OEM. Is it possible to reset the VPD, board serial, MAC
> address etc. while flashing a FW image downloaded from a support site?
> Would that mean that if I flash a rack with one FW image all NICs will
> start reporting the same serial numbers and use the same MACs?
> 

I think the intent here is for OEMs which would generate/customize the
images, though I've also been told it may be useful to get a card out of
some situation where firmware preservation was broken.. (No, I don't
really have more details on what specifically the situation might be).
Obviously in most update cases I don't think we expect this to be used.

>> 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"?
> 
> As a user I'd expect "nothing" to be the default. Same as your OS
> update does not wipe out your settings. I think it's also better 
> if the default is decided by Linux, not the drivers.
> 

Right, but I wasn't sure what other drivers/devices implement today and
didn't want  to end up in a "well we don't behave that way so you just
changed our behavior"..? Hmm. If they all behave this way today then
it's fine to make "nothing" the default and modify all implementations
to reject other options.

>> 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
> 
> Probably best if we understand the use case more clearly, too. Since
> you have this implemented in your tooling what are the scenarios where
> factory is expected to be preferred over FW default?
> 

I'll see if I can gather any further information on both this and the
overwrite-all mode.

My understanding so far is that it is intended as a way to restore the
device settings/config to what was written in the factory. I agree from
Linux perspective having this be a separate command (without requiring
an update) would make the most sense, but that isn't how it was
implemented today.

The factory settings are stored in a separate section of flash so they
aren't modified by normal update flows. I am not sure if there is a
procedure for updating them or if it truly is write-once.

Thanks,
Jake