svn commit: r191367 - head/sys/net

Robert Watson rwatson at FreeBSD.org
Wed Apr 22 09:33:21 UTC 2009


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


More information about the svn-src-all mailing list