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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190416085937.GC2122@nanopsycho>
Date:   Tue, 16 Apr 2019 10:59:37 +0200
From:   Jiri Pirko <jiri@...nulli.us>
To:     Jakub Kicinski <jakub.kicinski@...ronome.com>
Cc:     netdev@...r.kernel.org, davem@...emloft.net, mlxsw@...lanox.com
Subject: Re: [patch net-next rfc 00/15] netdevsim: impement proper device
 model

Mon, Apr 15, 2019 at 09:27:09PM CEST, jakub.kicinski@...ronome.com wrote:
>On Sat, 13 Apr 2019 18:20:57 +0200, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@...lanox.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 :)
>
>> 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.

Also, I plan to implement port splitting in follow-up patchset. All
flows are there as well.


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