cvs commit: src/sys/netinet ip_divert.c

[Robert Watson]

On Tue, 19 Oct 2004, Andre Oppermann wrote:

> >   Annotate a newly introduced race present due to the unloading of
> >   protocols: it is possible for sockets to be created and attached
> >   to the divert protocol between the test for sockets present and
> >   successful unload of the registration handler.  We will need to
> >   explore more mature APIs for unregistering the protocol and then
> >   draining consumers, or an atomic test-and-unregister mechanism.
> 
> Thanks. 
> 
> I'll commit a little extended locking of that section in a few minutes. 
> 
> Is it possible for someone else to spin on the lock while we are holding
> it?  If yes, it would be impossible to destroy the lock on the next
> line.  The same problem is then resident in ipfw unloading as well. 

Yes.  In fact, that's precisely what adaptive mutexes do, as our context
switch time appears to be more expensive than to spin in most cases.

We have started to address this and related problems in a couple of ways
in a couple of places -- for example, with the MAC Framework, we count the
number of threads that have entered the framework and wait until all
threads drain before unloading (potentially starving the unloader).  In
PFIL, we drop packets that try to enter while a module is waiting for
threads to drain so it can unload, avoiding the starvation problem.  I
believe Poul-Henning has also been working along these lines in the device
driver code so that during unload there's an atomic decision point such
that we stop letting new threads in once the decision to unload has been
finalized.  These models appear to be easier to implement than atomic "are
we busy or unload" tests, and by putting part of the decision logic in the
framework we avoid impossible-to-address races since we can quench the
flow of calls into the module (avoiding "unload while the instruction
pointer is in a function testing to see if it can be there").

As you point out, we don't have this problem a whole lot right now since
load and unload events are pretty infrequent, but it would be nice to
avoid adding new APIs that aren't designed to handle safe unload, in as
much as is reasonable.  For my own part, I'd be willing to simply say "you
can't unload protocols once you've loaded them" because it simplifies the
assumptions a great deal, but I recognize that that limits the usefulness
of loadable protocols for the purposes of a rapid development cycle (load,
unload, load, unload, ...)

Robert N M Watson             FreeBSD Core Team, TrustedBSD Projects
robert at fledge.watson.org      Principal Research Scientist, McAfee Research