bus: Set the current VNET in device_attach()

[markj]

Summary

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

[bz]

While the change seems fine (hence accepted) some people always found it questionable that the device framework should care about VNETs and in some ways this seems to be wrong.

The problem, I believe, is that we do not have further abstractions for networking-related drivers which could in that case be used as a handler for setting/unsetting the vnet and pushing this into 40-ish (just guessed) drivers seems not right either... Probably a separate discussion to be had on net@ ?

[kevans]

sys/kern/subr_bus.c

2578

Seems like`TD_TO_VNET(curthread)` might be a slightly better way to spell the same thing, just in case

[markj]

In D47174#1075587, @bz wrote:

While the change seems fine (hence accepted) some people always found it questionable that the device framework should care about VNETs and in some ways this seems to be wrong.

The problem, I believe, is that we do not have further abstractions for networking-related drivers which could in that case be used as a handler for setting/unsetting the vnet and pushing this into 40-ish (just guessed) drivers seems not right either... Probably a separate discussion to be had on net@ ?

I'm not sure what abstractions you are suggesting? iflib happens to solve this problem already. I don't see any solution that doesn't involve churning all drivers though.

sys/kern/subr_bus.c

2578

I somewhat prefer to be explicit about it here, since we shouldn't be attaching devices from within a jail.

Probably we should assert IS_DEFAULT_VNET(TD_TO_VNET(curthread)) is true?

[kevans]

sys/kern/subr_bus.c

2578

+1

[markj]

Use TD_TO_VNET(curthread); add an assertion.

[kevans]

Thanks!

[imp]

I never liked the vnet stuff in newbus. So finding another way to deal with it would be ideal.
However, that's a somewhat large delta from where we are today, and beyond the scope of this fix.
The delta in scope of vnet shouldn't matter at all: resource_disabled() doesn't use it, nor does the device_sysctl_update() routine that I could see.
So this does make it better, I think.

sys/kern/subr_bus.c

2578

If we are attaching from within a jail, then this is a panic service.
None of the drivers attached at boot will be doing that, nor will the interrupt-driven or proc-deferred ones (since we don't put them in a jail today). So that leaves kldload from inside a jail (which is prohibited) and devctl, which I think disallows /dev/devctl2 in jails, so I think this is a good assert (eg, should never happen).

[markj]

sys/kern/subr_bus.c

2578

Yes, devctl2 ioctls are not permitted in jails, they require PRIV_DRIVER.

[markj]

Add a missing include.

[glebius]

I'm fine with this change, it tells it explicitly that attach is VNET-aware and currently is supposed to happen in the default vnet only.

I have a plan, that we discussed together with @melifaro and @imp, that we should get rid of if_vmove() and moving physical interfaces to vnet jail should happen with a full detach & attach. In case you want certain physical interface to be in a vnet jail right at the boot time, that would be specified by kenv. The work on this project hasn't started yet.

[bz]

In D47174#1075792, @glebius wrote:

I'm fine with this change, it tells it explicitly that attach is VNET-aware and currently is supposed to happen in the default vnet only.

I have a plan, that we discussed together with @melifaro and @imp, that we should get rid of if_vmove() and moving physical interfaces to vnet jail should happen with a full detach & attach. In case you want certain physical interface to be in a vnet jail right at the boot time, that would be specified by kenv. The work on this project hasn't started yet.

This needs a different discussion elsewhere; please Cc: me on it.

[zlei]

This should also fix https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=275381 . See also D42678 .

[zlei]

sys/kern/subr_bus.c

2580

I like this over vnet0 :)