Tue, Nov 01, 2022 at 05:25:42PM CET, kuba@xxxxxxxxxx wrote: >On Mon, 31 Oct 2022 13:42:40 +0100 Jiri Pirko wrote: >> +/* >> + * Driver should use this to assign devlink port instance to a netdevice >> + * before it registers the netdevice. Therefore devlink_port is static >> + * during the netdev lifetime after it is registered. >> + */ >> +#define SET_NETDEV_DEVLINK_PORT(dev, _devlink_port) \ >> +({ \ >> + WARN_ON(dev->reg_state != NETREG_UNINITIALIZED); \ >> + ((dev)->devlink_port = (_devlink_port)); \ >> +}) > >The argument wrapping is inconsistent here - dev is in brackets >on the second line but not on the first. _devlink_port is prefixed >with underscore and dev is not. Let's make this properly secure >and define a local var for dev. Ok. > >> @@ -10107,6 +10107,7 @@ int register_netdevice(struct net_device *dev) >> return ret; >> >> err_uninit: >> + call_netdevice_notifiers(NETDEV_PRE_UNINIT, dev); > >I think we should make this symmetric with POST_INIT. >IOW PRE_UNINIT should only be called if POST_INIT was called. Yep, will check and fix.