netdev - Re: [PATCH net-next 1/3] devlink: Add fw_version

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:   Tue, 6 Nov 2018 14:47:13 -0800
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     Ido Schimmel <idosch@...sch.org>
Cc:     Ido Schimmel <idosch@...lanox.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "davem@...emloft.net" <davem@...emloft.net>,
        Jiri Pirko <jiri@...lanox.com>,
        Shalom Toledo <shalomt@...lanox.com>,
        Moshe Shemesh <moshe@...lanox.com>,
        "dsahern@...il.com" <dsahern@...il.com>,
        "andrew@...n.ch" <andrew@...n.ch>,
        "f.fainelli@...il.com" <f.fainelli@...il.com>,
        mlxsw <mlxsw@...lanox.com>
Subject: Re: [PATCH net-next 1/3] devlink: Add fw_version_check generic
 parameter

On Tue, 6 Nov 2018 22:37:51 +0200, Ido Schimmel wrote:
> On Tue, Nov 06, 2018 at 12:19:13PM -0800, Jakub Kicinski wrote:
> > On Tue, 6 Nov 2018 20:05:00 +0000, Ido Schimmel wrote:  
> > > From: Shalom Toledo <shalomt@...lanox.com>
> > > 
> > > Many drivers checking the device's firmware version during the
> > > initialization flow and flashing a compatible version if the current
> > > version is not.
> > > 
> > > fw_version_check gives the ability to skip this check which allows to run
> > > the device with a different firmware version than required by the driver
> > > for testing and/or debugging purposes.
> > > 
> > > Signed-off-by: Shalom Toledo <shalomt@...lanox.com>
> > > Reviewed-by: Jiri Pirko <jiri@...lanox.com>
> > > Signed-off-by: Ido Schimmel <idosch@...lanox.com>  
> > 
> > The documentation is missing, so it's hard to comment on the definition
> > of the parameter...    
> 
> I assume you mean Documentation/networking/devlink-params.txt ?

Yes

> > We have a FW loading policy for NFP, too, so it'd be good to see if we
> > can find a common ground.  
> 
> If the parameter is set, then device runs with whatever firmware version
> was last flashed (via ethtool, for example). Otherwise, the driver will
> flash a version according to its policy. In mlxsw, it is a specific
> version.
> 
> Will that work for you?

Our FW is always backward compatible so there is no need to downgrade.

What we have is this more along these lines: there are two images one
on disk and second in the flash.  The FW loading policy can decide
which of those should be preferred, or should the versions be compared
and the newer one win (default).  But we don't flash the newer FW, just
potentially load it from disk today.

I'm not sure whether 'fw_version_check' describes the general behaviour
of not updating the FW in flash.  The policy of updating the FW in the
flash if the one on disk is newer seems to be something we could adopt
as well.  Can we come up with a more general parameter which could
select FW loading policy that'd for both cases?

Would values like these make any sense to you?
 - driver preferred (your default behaviour, we don't support since
   driver doesn't care);
 - newest (our default, device compares images and picks newer);
 - always disk (always run with what's on the disk, regardless of
   versions);
 - always flash (always run with what's already in flash, don't look at
   disk);

Separate bool parameter 'fw_flash_auto_update' would decide whether the
selected FW should be flashed to the device (always true for you AFAIU).

Let me know if that makes sense, it would be nice if we could converge
on a common solution, or at least name our parameters sufficiently
distinctly to avoid confusion :)