Thu, Apr 18, 2019 at 07:07:48PM CEST, jakub.kicin...@netronome.com wrote: >On Thu, 18 Apr 2019 09:22:56 +0200, Jiri Pirko wrote: >> 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? > >BPF offload tries to leave as much code as possible in the core, and >make the drivers simple. I'm testing whether the core reacts correctly, >netdevsim just calls register/unregister. > >> >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.. > >Hm.. I'm getting lost, sorry, I'm probably confusing myself here.. > >Netdevsim is supposed to test real, existing kernel interfaces and core >code. What we do today with linking based on netdevs is quite simple >and works very well for the BPF offload tests. > >If you want to test some devlink code, that's also real, perfect. > >For BPF tests we want the ability to add and remove netdevs to a sdev >during tests, yes. That ability cannot be lost.
Yeah, but since the api is changing, the original approach of using IFLA_LINK cannot be used anymore. Not to mention is is abuse of the API from the very beginning. You want to add/del ports during lifetime, I'm trying to find interface. Basically whe have two options: 1) devlink extension I suggested above 2) sysfs files to add/del ports. Similar to "new_device" and "del_device". I like the idea of having 2) better, the reason is this is very speficic to netdevsim and not really applicable on real devices. > > >> >"port splitting" or device slicing, which should be discussed over real >> >code, not netdevsim.
