Messages in this thread | | | Date | Tue, 28 Jul 2020 11:44:58 -0700 | From | Jakub Kicinski <> | Subject | Re: [PATCH net-next RFC 01/13] devlink: Add reload level option to devlink reload command |
| |
On Tue, 28 Jul 2020 09:47:00 -0700 Jacob Keller wrote: > On 7/28/2020 6:58 AM, Jiri Pirko wrote: > > But this is needed to maintain the existing behaviour which is different > > for different drivers. > > Which drivers behave differently here?
I think Jiri refers to mlxsw vs mlx5.
mlxsw loads firmware on probe, by default at least. So reloading the driver implies a FW reset. NIC drivers OTOH don't generally load FW so they didn't reset FW.
Now since we're redefining the API from "do a reload so that driverinit params are applied" (or "so that all netdevs get spawned in a new netns") to "do a reset of depth X" we have to change the paradigm.
What I was trying to suggest is that we should not have to re-define the API like this.
From user perspective what's important is what the reset achieves (and perhaps how destructive it is). We can define the reset levels as:
$ devlink dev reload pci/0000:82:00.0 net-ns-respawn $ devlink dev reload pci/0000:82:00.0 driver-param-init $ devlink dev reload pci/0000:82:00.0 fw-activate
combining should be possible when user wants multiple things to happen:
$ devlink dev reload pci/0000:82:00.0 fw-activate driver-param-init
Then we have the use case of a "live reset" which is slightly under-defined right now IMHO, but we can extend it as:
$ devlink dev reload pci/0000:82:00.0 fw-activate --live
We can also add the "reset level" specifier - for the cases where device is misbehaving:
$ devlink dev reload pci/0000:82:00.0 level [driver|fw|hardware]
But I don't think that we can go from the current reload command cleanly to just a level reset. The driver-specific default is a bad smell which indicates we're changing semantics from what user wants to what the reset depth is. Our semantics with the patch as it stands are in fact: - if you want to load new params or change netns, don't pass the level - the "driver default" workaround dictates the right reset level for param init; - if you want to activate new firmware - select the reset level you'd like from the reset level options.
| |