Messages in this thread | | | Date | Fri, 6 Aug 2021 14:19:43 +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 12:12:03PM -0700, Jakub Kicinski wrote: > On Thu, 5 Aug 2021 21:02:23 +0300 Leon Romanovsky wrote: > > > > 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. > > I worry that netdevsim's gone unwieldy as a reflection of the quality of > the devlink APIs that got added, not by itself :/ > > > > It was intended for fast prototyping, but ended to be huge pile of > > > debugfs entries and selftest to execute random flows. > > It's for selftests, IDK what fast prototyping is in terms of driver > APIs. Fast prototyping makes me think of the "it works" attitude which > is not sufficiently high bar for core APIs IMO, I'm sure you'll agree. > > netdevsim was written specifically to be able to exercise HW APIs which > are implemented by small fraction of drivers. Especially offload APIs > as those can easily be broken by people changing the SW implementation > without capable HW at hand. > > BTW I wonder if there is a term in human science of situation like when > a recent contributor tells the guy who wrote the code what the code was > intended for :)
"Teaching grandmother to suck eggs" ? :)
> > > > 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. > > Right, but I think as you rightly pointed out the current protection > of reload is broken. I'm not saying you must make it perfect or else.. > just pointing out a gap you could address if you so choose.
I don't know, netdevsim needs some dedicated cleanup.
Thanks
| |