Date: Sat, 19 Oct 2024 19:22:08 -0400 From: Mark Johnston <markj@freebsd.org> To: Konstantin Belousov <kostikbel@gmail.com> Cc: John Baldwin <jhb@freebsd.org>, src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Subject: Re: git: f4e35c044c89 - main - bus: Set the current VNET in device_attach() Message-ID: <ZxQ_IALrOAVAf09i@nuc> In-Reply-To: <ZxPaDc7jAORhM5O6@kib.kiev.ua> References: <202410191304.49JD4JoM084001@gitrepo.freebsd.org> <ed434405-26cc-4363-834e-9f31d7912589@FreeBSD.org> <ZxPaDc7jAORhM5O6@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
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. 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. > 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). Shouldn't something like this be handled by if_detach()/ether_ifdetach()? Posed another way, why does device detach itself need to care about the VNET? I tend to agree that having VNET knowledge in subr_bus.c is a hack. My aim was just to fix the panic without making the hack worse. > 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().
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?ZxQ_IALrOAVAf09i>