Page MenuHomeFreeBSD

domain: give domains a chance to probe for availability
ClosedPublic

Authored by kevans on May 30 2020, 3:00 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Apr 29, 2:01 AM
Unknown Object (File)
Sun, Apr 14, 3:59 AM
Unknown Object (File)
Mar 30 2024, 2:19 PM
Unknown Object (File)
Mar 11 2024, 7:46 AM
Unknown Object (File)
Mar 9 2024, 7:52 AM
Unknown Object (File)
Mar 7 2024, 1:41 AM
Unknown Object (File)
Mar 6 2024, 9:31 AM
Unknown Object (File)
Feb 25 2024, 4:28 PM
Subscribers

Details

Summary

This gives any given domain a chance to indicate that it's not actually supported on the current system. If dom_probe isn't supplied, we assume the domain is universally applicable as most of them are. Keeping fully-initialized and registered domains around that physically can't work on a large majority of FreeBSD deployments is sub-optimal and leads to errors that aren't consistent with the reality of why the socket can't be created (e.g. ESOCKTNOSUPPORT) because such scenario has to be caught upon pru_attach, at which point kicking back the more-appropriate EAFNOSUPPORT would seem weird.

The only initial consumer of this is hvsock. It could be debated that perhaps hvsock shouldn't be in GENERIC given that HyperV has a relatively low share of FreeBSD deployments, but simply not initializing the domain on non-HyperV machines feels like a reasonable compromise.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Seems broadly reasonable to me. I'm not super familiar with the domains logic myself but nothing looks wrong.

This revision is now accepted and ready to land.Jun 25 2020, 3:09 AM

I was hoping this was going to fix the bluetooth issue of adding domains post-boot from the initial mail, but it doesn't seem so? (I get a warning every time I boot on a laptop with bluetooth since the BT domain is pulled in via devmatch.) It seems like solving that problem cleanly would be a prerequisite for kicking this out of GENERIC and into a module as well.

sys/kern/uipc_domain.c
245

This is the warning you get from bluetooth due to adding a domain post-boot.

sys/kern/uipc_domain.c
245

I'd come to the conclusion that this warning was OBE and could be removed, but didn't have the network domain knowledge to just make the change. We should look into that with the appropriate domain expert...

sys/kern/uipc_domain.c
245

At that point you have to worry about racing pr_{slow,fast}timo callbacks against partially initialized protocols/domains, so it's not quite safe. IMO it's already pretty thoroughly broken with VIMAGE, where you may invoke the callbacks that sometimes do something with all vnets and you could be racing with vnet_domain_init/domain_init.

Extra care needs to be taken per-domain to handle that, when it should probably be done here instead so you can also load domains later.

sys/kern/uipc_domain.c
245

Actually, the more I think about it; we can't really do proper synchronization for VIMAGE at this level. We probably don't want to skip any pr_{slow,fast}timo callback invocations if the domain's already been init'd in one vnet and a new vnet is created, but we probably have to at least make sure that the domain we're about to invoke for has been fully constructed in domain_init for the default vnet to allow late loading.

Simplify; just add dom_probe and a dom_flags for the domain that will be used to indicate that a domain is supported (no probe routine or probe returned 0).

There will be a follow-up review to make it safer to add domains post-domfinalize and kill off the warning.

This revision now requires review to proceed.Jun 26 2020, 2:41 AM

Sprinkle some atomics into place

The current version of this didn't really need atomics, but it's not hard to see a future where a domain could become supported for one reason or another after boot.

Update domain(9)... this includes some updates for past changes (e.g. domain_add hasn't actually called dom_init for 11 years as of the time of writing, note about dom_destroy).

bcr added a subscriber: bcr.

OK from manpages.

markj added inline comments.
sys/kern/uipc_domain.c
227

Could you explain exactly what the acq/rel barriers are for?

sys/kern/uipc_domain.c
227

... I see there is some discussion of that in D25459, so ignore me.

This revision was not accepted when it landed; it landed in state Needs Review.Aug 16 2021, 6:08 AM
This revision was automatically updated to reflect the committed changes.