carp: Unregister the netlink family without holding carp_mtx

[rcm]

Summary

carp_modevent() calls carp_mod_cleanup() with carp_mtx held, and the
cleanup unregisters the generic netlink family, which takes the
generic netlink sx: a sleepable lock taken with a mutex held, which
WITNESS reports on the first kldunload. A later change makes the
unregister also wait for running handlers, which would deadlock with
carp_nl_set() taking carp_mtx in carp_alloc().

At MOD_QUIESCE, refuse the unload while vhids exist, and otherwise set
carp_unloading under carp_sx, after which carp_nl_set() fails with
ENOPROTOOPT instead of creating a vhid, so MOD_UNLOAD no longer finds
one created after MOD_QUIESCE allowed the unload. MOD_UNLOAD repeats
the check for kldunload -f, which ignores the veto, then unregisters
the family and cleans up without carp_mtx. carp_mod_cleanup() no
longer unlocks carp_mtx, which carp_mod_load() did not hold when a
protocol registration failed, nor unregisters a family that
carp_mod_load() had not registered yet.

Fixes: 40e043596409 ("carp: add netlink interface")
MFC after: 1 week
Sponsored by: Rubicon Communications, LLC ("Netgate")

[glebius]

I don't like that we may return 0 from MOD_QUIESCE and then return EBUSY from MOD_UNLOAD. I would suggest to keep a global static bool carp_unloading and set it at MOD_QUIESCE when we return 0. Then carp_nl_set shall check this boolean. It should be protected by the carp_sx and carp_modevent should also use this lock.

[rcm]

Replace the MOD_UNLOAD re-register with a carp_unloading flag: MOD_QUIESCE sets it under carp_sx when no vhid exists, carp_nl_set() returns ENOPROTOOPT while it is set, and carp_modevent() uses carp_sx instead of carp_mtx.

[glebius]

Here is another idea. I'm sorry that I didn't suggest that before and you already produced a version.

What if on MOD_QUIESCE() we take the carp_sx, confirm that list is empty and call genl_unregister_family? This removes need for the ugly boolean. However, this leaves a race where we would have another thread already working on a netlink message and already entered carp_nl_set and blocked on carp_sx. This definitely resonates with the problem that D60491 tries to address.

P.S. I'd suggest not to rush with all these ad-hoc fixes to problems associated with unloading and unregistering. They do not hurt production use, so there is no emergency. We should think of a generic way to address them instead of playing a whack-a-mole a module by module.