Re: git: f4e35c044c89 - main - bus: Set the current VNET in device_attach()

On Sat, Oct 19, 2024 at 11:50:40PM +0000, Bjoern A. Zeeb wrote:
> On Sat, 19 Oct 2024, Mark Johnston wrote:
> 
> > On Sat, Oct 19, 2024 at 07:10:53PM +0300, Konstantin Belousov wrote:
> > > On Sat, Oct 19, 2024 at 11:36:32AM -0400, John Baldwin wrote:
> > > > On 10/19/24 09:04, Mark Johnston wrote:
> > > > > The branch main has been updated by markj:
> > > > > 
> > > > > URL: https://cgit.FreeBSD.org/src/commit/?id=f4e35c044c8988b7452cefbdcc417f5fd723e021
> > > > > 
> > > > > commit f4e35c044c8988b7452cefbdcc417f5fd723e021
> > > > > Author:     Mark Johnston <markj@FreeBSD.org>
> > > > > AuthorDate: 2024-10-19 13:03:56 +0000
> > > > > Commit:     Mark Johnston <markj@FreeBSD.org>
> > > > > CommitDate: 2024-10-19 13:03:56 +0000
> > > > > 
> > > > >      bus: Set the current VNET in device_attach()
> > > > >      Some drivers, in particular anything which creates an ifnet during
> > > > >      attach, need to have the current VNET set, as if_attach_internal() and
> > > > >      its callees access VNET-global variables.
> > > > >      device_probe_and_attach() handles this, but this is not the only way to
> > > > >      arrive in DEVICE_ATTACH.  In particular, bus drivers may invoke
> > > > >      device_attach() directly, as does devctl2's DEV_ENABLE ioctl handler.
> > > > >      So, set the current VNET in device_attach() instead.
> > > > >      I believe it is always safe to use vnet0, as devctl2 ioctls are not
> > > > >      permitted within a jail.
> > > > >      PR:             282168
> > > > >      Reviewed by:    zlei, kevans, bz, imp, glebius
> > > > >      MFC after:      1 week
> > > > >      Differential Revision:  https://reviews.freebsd.org/D47174
> > > > 
> > > > Hmm, there was some other review I thought that had a completely different change.
> > > > That change removed all the vnet stuff from new-bus and instead handled it in
> > > > if.c.  Specifically, that if_attach would set a default vnet to vnet0 if there
> > > > wasn't an active vnet at the time.  See all the discussion in
> > > > https://reviews.freebsd.org/D42678 which has a patch that I think is correct
> > > > in the comments.
> 
> There it was; thanks I didn't misremeber but couldn't find it.
> 
> 
> > Gleb's proposal, described a bit in D47147, is to require device-based
> > ifnet drivers to fully detach themselves from the parent bus in order to
> > change VNETs.  The intent is to eliminate the need for if_vmove() and
> > all the complexity it entails.  This would also eliminate the need for a
> > "home" VNET, referenced in the patch that you reference here.
> 
> Will it?
> 
> I asked for a discussion elsewhere but it seems we are having it here now...

I'm responding to John's question and Kostik's follow-up, nothing else.
The inline patch in D42678 seems fine, I don't have strong feelings
about it, but I believe it is not sufficient to fix the PR in question
(it still assumes that the current vnet is already set).

> I am still inclined to ask:
> - how do you want a vnet to attach an unknown to itself device?  From
>   the outside?
> - How to you pass it to a child-vnet without escalating priviledges to
>   outside of the host system (vnet0)?
> - Is, e.g., a vcc device [CXGBE(4)] a physical interface?
> - How do you pass a controlled set of other non-clonable devices in (or
>   did we get rid of them all)?   The inital idea was still that the
>   "host" has somehow control over what child can create...
>   { I recently tried tuntap in a vnet and it blew up badly by not going
>   away }
> - exmaple: I really would love, e.g., a vlan interface to be passed to a
>   vnet but but not the pyhsical interface.  Can we?
> - How will we do with wlan interfaces (which are cloned) but may not own
>   the hardware (kind-of similar to the vcc example)?  I know there are
>   special PRIV checks for those currently.
> - how does a detach in a vnet work and where will the physical interface
>   re-appear for (automatic) attachment?  just detached in that jail?
>   vnet0?  the parent jail?
> - what happens on vnet destroy? (same as last question)?
> - Are we just going to build a vmove on a layer which doesn't have
>   anything to do with networking per-se as a special case for some
>   interfaces but not others?

These are excellent questions which should be posed to Gleb when his
proposal is fleshed out.  In the meantime, I only aimed to fix an
obvious shortcoming of an existing hack which has been around for over
10 years.