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

[Jiri Pirko <jiri@resnulli.us>]

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.


>
>> 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?


>
>> 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.