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.
