linux-kernel - Re: [PATCH net-next] devlink: Require devlink lock during device reload

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: <YYqiPIV9KoGTiTSV@nanopsycho>
Date:   Tue, 9 Nov 2021 17:30:52 +0100
From:   Jiri Pirko <jiri@...nulli.us>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     Jason Gunthorpe <jgg@...dia.com>,
        Leon Romanovsky <leon@...nel.org>,
        Ido Schimmel <idosch@...sch.org>,
        "David S . Miller" <davem@...emloft.net>,
        Jiri Pirko <jiri@...dia.com>, linux-kernel@...r.kernel.org,
        netdev@...r.kernel.org, edwin.peer@...adcom.com
Subject: Re: [PATCH net-next] devlink: Require devlink lock during device
 reload

Tue, Nov 09, 2021 at 05:26:48PM CET, kuba@...nel.org wrote:
>On Tue, 9 Nov 2021 17:15:24 +0100 Jiri Pirko wrote:
>> Tue, Nov 09, 2021 at 04:07:02PM CET, kuba@...nel.org wrote:
>> >On Tue, 9 Nov 2021 10:43:58 -0400 Jason Gunthorpe wrote:  
>> >> This becomes all entangled in the aux device stuff we did before.  
>> >
>> >So entangled in fact that neither of you is willing to elucidate 
>> >the exact need ;)
>> >  
>> >> devlink reload is defined, for reasons unrelated to netns, to do a
>> >> complete restart of the aux devices below the devlink. This happens
>> >> necessarily during actual reconfiguration operations, for instance.
>> >> 
>> >> So we have a situation, which seems like bad design, where reload is
>> >> also triggered by net namespace change that has nothing to do with
>> >> reconfiguring.  
>> >
>> >Agreed, it is somewhat uncomfortable that the same callback achieves
>> >two things. As clear as the need for reload-for-reset is (reconfig,
>> >recovery etc.) I'm not as clear on reload for netns.
>> >
>> >The main use case for reload for netns is placing a VF in a namespace,
>> >for a container to use. Is that right? I've not seen use cases
>> >requiring the PF to be moved, are there any?
>> >
>> >devlink now lives in a networking namespace yet it spans such
>> >namespaces (thru global notifiers). I think we need to define what it
>> >means for devlink to live in a namespace. Is it just about the
>> >configuration / notification channel? Or do we expect proper isolation?
>> >
>> >Jiri?  
>> 
>> Well honestly the primary motivation was to be able to run smoothly with
>> syzkaller for which the "configuration / notification channel" is
>> enough.
>
>Hm. And syzkaller runs in a namespace?

Correct.

>
>> By "proper isolation" you mean what exactly?
>
>For the devlink instance and all subordinate objects to be entirely
>contained to the namespace within which devlink resides, unless
>explicitly liked up with or delegated to another namespace.

What makes sense to me and that is actually how the current drivers
should behave (mlxsw, netdevsim are for sure).