netdev - Re: [PATCH net-next 0/4] bnxt_en: Add new "enable_hot_fw

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: <20200518164309.2065f489@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Mon, 18 May 2020 16:43:09 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Jiri Pirko <jiri@...nulli.us>
Cc:     Vasundhara Volam <vasundhara-v.volam@...adcom.com>,
        davem@...emloft.net, netdev@...r.kernel.org
Subject: Re: [PATCH net-next 0/4] bnxt_en: Add new "enable_hot_fw_reset"
 generic devlink parameter

On Mon, 18 May 2020 13:01:52 +0200 Jiri Pirko wrote:
> Mon, May 18, 2020 at 10:27:15AM CEST, vasundhara-v.volam@...adcom.com wrote:
> >This patchset adds support for a "enable_hot_fw_reset" generic devlink
> >parameter and use it in bnxt_en driver.
> >
> >Also, firmware spec. is updated to 1.10.1.40.  
> 
> Hi.
> 
> We've been discussing this internally for some time.
> I don't like to use params for this purpose.
> We already have "devlink dev flash" and "devlink dev reload" commands.
> Combination of these two with appropriate attributes should provide what
> you want. The "param" you are introducing is related to either "flash"
> or "reload", so I don't think it is good to have separate param, when we
> can extend the command attributes.
> 
> How does flash&reload work for mlxsw now:
> 
> # devlink flash
> Now new version is pending, old FW is running
> # devlink reload
> Driver resets the device, new FW is loaded
> 
> I propose to extend reload like this:
> 
>  devlink dev reload DEV [ level { driver-default | fw-reset | driver-only | fw-live-patch } ]
>    driver-default - means one of following to, according to what is
>                     default for the driver
>    fw-reset - does FW reset and driver entities re-instantiation
>    driver-only - does driver entities re-instantiation only
>    fw-live-patch - does only FW live patching - no effect on kernel
> 
> Could be an enum or bitfield. Does not matter. The point is to use
> reload with attribute to achieve what user wants. In your usecase, user
> would do:
> 
> # devlink flash
> # devlink reload level fw-live-patch

Unfortunately for SmartNICs and MultiHost systems the reset may not be
initiated locally. I agree it'd be great to have a normal netlink knob
for this instead of a param. But it has to be some form of a policy of
allowing the reset to happen, rather than an action/trigger kind of
thing.

Also user space notification should be generated when reset happens,
IMO. devlink dev info contents will likely change after reset, if
nothing else.

Plus this functionality will need proper documentation.

FWIW - I am unconvinced that applications will be happy to experience
network black outs, rather than being fully killed and re-spawned. For
a micro-service worker shutdown + re-spawn should be bread and butter.
But we already have ionic doing this, so seems like vendors are
convinced otherwise, so a common interface is probably a good step.