Messages in this thread | | | Date | Mon, 25 Oct 2021 22:12:42 +0300 | From | Leon Romanovsky <> | Subject | Re: [PATCH net-next] netdevsim: Register and unregister devlink traps on probe/remove device |
| |
On Mon, Oct 25, 2021 at 11:24:53AM -0700, Jakub Kicinski wrote: > On Mon, 25 Oct 2021 11:08:00 +0300 Ido Schimmel wrote: > > No, it's not correct. After your patch, trap properties like action are > > not set back to the default. Regardless of what you think is the "right > > design", you cannot introduce such regressions. > > > > Calling devlink_*_unregister() in reload_down() and devlink_*_register() > > in reload_up() is not new. It is done for multiple objects (e.g., ports, > > regions, shared buffer, etc). After your patch, netdevsim is still doing > > it. > > If we want to push forward in the direction that Leon advocates we'd > have to unregister the devlink instance before reload_down(), right?
Not really, we ensure that during "devlink reload", users can't send any get/set commands. So you don't need to unregister anything, because driver is safe to change any internal values it wants. The devlink instance is locked for exclusive access.
The protection is done now by devlink_mutex, but will be slightly different in the near future.
> > Otherwise it seems fundamentally incompatible with the idea of reload > for reconfig. And we'd be trading one partially correct model for > another partially correct model :/
I see devlink reload as command to reconfigure device and not devlink itself. This is the difference between my direction vs. mlxsw way.
Thanks
| |