Date: Wed, 22 Apr 2009 10:33:19 +0100 (BST) From: Robert Watson <rwatson@FreeBSD.org> To: Bruce Simpson <bms@incunabulum.net> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r191367 - head/sys/net Message-ID: <alpine.BSF.2.00.0904221028230.67705@fledge.watson.org> In-Reply-To: <49EEDB9C.8080409@incunabulum.net> References: <200904212243.n3LMhW48027008@svn.freebsd.org> <49EEDB9C.8080409@incunabulum.net>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?alpine.BSF.2.00.0904221028230.67705>