On Wed, 22 Apr 2009, Bruce Simpson wrote:
Start to address a number of races relating to use of ifnet pointers
after the corresponding interface has been destroyed:
(1) Add an ifnet refcount, ifp->if_refcount. Initialize it to 1 in
if_alloc(), and modify if_free_type() to decrement and check the
refcount.
(2) Add new if_ref() and if_rele() interfaces to allow kernel code
walking global interface lists to release IFNET_[RW]LOCK() yet
keep the ifnet stable. Currently, if_rele() is a no-op wrapper
around if_free(), but this may change in the future.
Thanks. I'll try to digest this badly needed work, but might not get around
to updating SSM to use it straight away. As you probably saw from doco, it's
something which SSM bangs right into, inpcbs after all last longer than
ifnets.
There are a few parts to our general problem with ifnets going away:
(1) Our general ifnet life cycle is poorly defined, so intermediate states may
be visible that shouldn't be (for example, if_detach is a bit chaotic, and
we've now stretched out the gap between if_detach and if_free using a
refcount). We probably need an IFF_DEAD flag to better handle this case,
so that once an ifnet is marked as free but still dying due to outstanding
references, it can'e bt looked up. Brooks and I have plans to try to
hash some of this out at the BSDCan devsummit.
(2) Some consumers of ifnets need to be taught to acquire refcounts on the
ifnet to prevent it going away during certain operations -- mainly a class
of things similar to the one I just fixed relating to user requests that
perform operations that you can't hold IFNET_RLOCK() over.
(3) Some consumers of ifnets need to be taught to register the ifnet removal
event and properly detach themselves. For example, DUMMYNET is a
well-known component that likes to hold onto mbufs for a long time,
increasing the chances of the "oh, my ifnet is gone" race. Rather than
trying to refcount from each mbuf, which would add excessive overhead, I
think the answer is to modify these consumers to detect ifnet removal and
GC all the mbufs that refer to it. For example, DUMMYNET should probably
walk all its queues and m_freem() packets refering to the dying ifnet.
I'm not familiar with the workings of SSM, but my feeling is that it probably
needs to take the (3) approach rather than (2) -- rather than preventing the
ifnet from going away until a socket closes, it should detect that the ifnet
is going away and take appropriate remedial action. Possibly it should detect
when a similarly configured ifnet re-appears and consider attaching to that,
but it will most likely be a different instance of struct ifnet, and there are
semantic considerations.
Robert N M Watson
Computer Laboratory
University of Cambridge
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
