Re: [patch net-next rfc 00/15] netdevsim: impement proper device model

[Jakub Kicinski <jakub.kicinski@netronome.com>]

On Tue, 16 Apr 2019 10:53:12 +0200, Jiri Pirko wrote:
> Mon, Apr 15, 2019 at 09:27:09PM CEST, jakub.kicinski@netronome.com wrote:
> >On Sat, 13 Apr 2019 18:20:57 +0200, Jiri Pirko wrote:  
> >> From: Jiri Pirko <jiri@mellanox.com>
> >> 
> >> Currently the model of netdevsim is a bit odd in multiple ways.
> >> 1) devlink instance is not in any way related with actual netdevsim
> >>    netdevices. Instead, it is created per-namespace.
> >> 2) multi-port netdevsim device is done using "link" attribute.
> >> 3) netdevsim bus is there only to have something to bind the netdev to,
> >>    it really does not act as a bus.  
> >
> >Nope, it's there to expose SR-IOV ops :)  
> 
> They are not on the bus, they are on the device.

Right, I'm just explaining why there is a device in the first place.

> >> 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.  
> 
> The reason is the then the address, devlink handles and netdev names are
> predictable. If it would be auto-generated, how could you know what did
> you just created?
> 
> >I also dislike declaring ports from the start.  In real drivers ports
> >are never "atomically" registered, they are crated and destroyed one 
> >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...  
> 
> So what do you suggest?

I haven't looked at this in detail.  One quick idea is perhaps to
remember what  ID was used after the write on the file descriptor, 
and then next read would return the ID?

> >> Then devlink shows this:
> >> 
> >> $ devlink dev
> >> netdevsim/netdevsim10
> >> 
> >> $ devlink port
> >> netdevsim/netdevsim10/0: type eth netdev netdevsim10p1 flavour physical
> >> netdevsim/netdevsim10/1: type eth netdev netdevsim10p2 flavour physical
> >> netdevsim/netdevsim10/2: type eth netdev netdevsim10p3 flavour physical
> >> netdevsim/netdevsim10/3: type eth netdev netdevsim10p4 flavour physical
> >> 
> >> Debugfs topology is also adjusted a bit. The rest stays the same as
> >> before.
> >> 
> >> TODO:
> >> - teach udev to rename netdevsim netdevices similarly to pci netdevices  
> >
> >So we can test udev as well?  
> 
> https://github.com/systemd/systemd/pull/12328
> 
> >> - fix tools/testing/selftests/bpf/test_offload.py to work with new iface  
> >
> >That'd step 0 :)
> >
> >BTW are you testing all this with the various sysfs/kobject debug
> >checks?  I don't remember all the deets now, but there were certainly
> >ordering considerations coming from there.  
> 
> Yes.

Cool :)