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

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.
> 
In fact, I think that bus level is better.  At least, I know that detach
also might need something by vnet (e.g. mce(4) needs to clear the IPSEC
offload database on detach, although it still does not do).

It sounds as if bus_topo_lock() is the right place.  May be some other
name for it is better, like bus_topo_changes_enter().