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

[Jakub Kicinski <kuba@kernel.org>]

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?

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