Messages in this thread | | | Date | Thu, 5 Aug 2021 21:02:23 +0300 | From | Leon Romanovsky <> | Subject | Re: [PATCH net-next v1] netdevsim: Forbid devlink reload when adding or deleting ports |
| |
On Thu, Aug 05, 2021 at 08:35:59PM +0300, Leon Romanovsky wrote: > On Thu, Aug 05, 2021 at 08:27:56AM -0700, Jakub Kicinski wrote: > > On Thu, 5 Aug 2021 17:33:35 +0300 Leon Romanovsky wrote: > > > On Thu, Aug 05, 2021 at 07:23:42AM -0700, Jakub Kicinski wrote: > > > > > This is what devlink_reload_disable() returns, so I kept same error. > > > > > It is not important at all. > > > > > > > > > > What about the following change on top of this patch? > > > > > > > > LGTM, the only question is whether we should leave in_reload true > > > > if nsim_dev->fail_reload is set. > > > > > > I don't think so, it will block add/delete ports. > > > > As it should, given add/delete ports takes the port_list_lock which is > > destroyed by down but not (due to the forced failure) re-initialized by > > up. > > > > If we want to handle adding ports while down we can just bump port > > count and return, although I don't think there's a practical need > > to support that. > > Sorry, but for me netdevsim looks like complete dumpster. It was > intended for fast prototyping, but ended to be huge pile of debugfs > entries and selftest to execute random flows. > > Do you want me to move in_reload = false line to be after if (nsim_dev->fail_reload) > check?
BTW, the current implementation where in_reload before if, actually preserves same behaviour as was with devlink_reload_enable() implementation.
Thanks
> > Thanks
| |