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 <ma...@freebsd.org>
AuthorDate: 2024-10-19 13:03:56 +0000
Commit: Mark Johnston <ma...@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
