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

Jiri Pirko
2021-11-09T16:31:00Z

Tue, Nov 09, 2021 at 05:26:48PM CET, kuba@kernel.org wrote:
>On Tue, 9 Nov 2021 17:15:24 +0100 Jiri Pirko wrote:
>> Tue, Nov 09, 2021 at 04:07:02PM CET, kuba@kernel.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).