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

On Sat, 19 Oct 2024, Mark Johnston wrote:

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

ACK & ACK

-- 
Bjoern A. Zeeb                                                     r15:7