Tue, Apr 16, 2019 at 08:04:59PM CEST, jakub.kicin...@netronome.com wrote: >On Tue, 16 Apr 2019 10:59:37 +0200, Jiri Pirko wrote: >> >> 4) netdevsim instances are created by "ip link add" which is great for >> >> soft devices with no hw backend. The rtnl core allocates netdev and >> >> calls into driver holding rtnl mutex. For hw-backed devices, this >> >> flow is wrong as it breaks order in which things are done. >> >> >> >> This patchset adjust netdevsim to fix all above. >> >> >> >> In order to support proper devlink and devlink port instances and to be >> >> able to emulate real devices, there is need to implement bus probe and >> >> instantiate everything from there. User can specify device id and port >> >> count to be instantianted. For example: >> >> >> >> echo "10 4" > /sys/bus/netdevsim/new_device >> > >> >I really don't like the design where ID has to be allocated by user >> >space. It's a step back. >> > >> >I also dislike declaring ports from the start. In real drivers ports >> >are never "atomically" registered, they are crated and destroyed one >> >> Care to define "atomically" here? It is done in a very similar way >> to how it is done in mlxsw for example. Same flows. >> >> >> >by one, and a lot of races/UAFs/bugs lie in those small periods of >> >time where one netdev got unregistered, but other are still around... >> >> Same here. Not sure where do you see the differences. > >The difference is that today I can do this: > >create a netdevsim1 with shared dev 1 >create some state associated with shared dev 1 >create a netdevsim2 with shared dev 1 >check if all the shared dev 1 state created for netdevsim1 is visible > via netdevsim2
Hmm, you are testing netdevsim implementation then, not the kernel interfaces. What is the point of testing netdevsim? >destroy netdevsim1 >check the shared dev 1 state again > >If I say "give me 2 ports" from the start, that makes the testing >(which is the whole point of this code) harder. > >> Also, I plan to implement port splitting in follow-up patchset. All >> flows are there as well. > >Sure, let's just be clear that we won't be merging an ABI that has just >a netdevsim implementation, right? I have some reservations about the So what do you suggest? Allow to somehow add and remove ports during test? You can already do that with VFs. Do you want to do that with netdevsim "physical" ports? If yes, how? I can imagine to extend devlink port api with something like: $ sudo devlink dev netdevsim/netdevsim0 $ sudo devlink port netdevsim/netdevsim0/0: type eth netdev eth0 flavour physical $ sudo devlink dev port add netdevsim/netdevsim0 index 22 $ sudo devlink port netdevsim/netdevsim0/0: type eth netdev eni0p1 flavour physical netdevsim/netdevsim0/22: type eth netdev eni0p23 flavour physical $ sudo devlink port del netdevsim/netdevsim0/0 $ sudo devlink port netdevsim/netdevsim0/22: type eth netdev eni0p23 flavour physical But I see only usecase for this extension for netdevsim, not for real devices.. >"port splitting" or device slicing, which should be discussed over real >code, not netdevsim.
