Date: Mon, 25 Jul 2011 13:00:20 +0200 From: Daan Vreeken <Daan@vitsch.nl> To: freebsd-net@freebsd.org Cc: Ryan Stone <rysto32@gmail.com>, bz@freebsd.org, "Robert N. M. Watson" <rwatson@freebsd.org>, gnn@freebsd.org Subject: Re: m_pkthdr.rcvif dangling pointer problem Message-ID: <201107251300.20832.Daan@vitsch.nl> In-Reply-To: <E05FE767-1923-4D47-9759-FA040E403618@freebsd.org> References: <20110714154457.GI70776@FreeBSD.org> <CAFMmRNwBbxR-F7PjkwF8E4GjwFQy_0USKW-3u-ZRNxPMJSOQcA@mail.gmail.com> <E05FE767-1923-4D47-9759-FA040E403618@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Hi Robert, On Sunday 24 July 2011 10:43:59 Robert N. M. Watson wrote: > On 24 Jul 2011, at 04:51, Ryan Stone wrote: > > I ran headlong into this issue today when trying to test some network > > stack changes. It's depressingly easy to crash HEAD by periodically > > destroying vlan interfaces while you are sending traffic over those > > interfaces -- I get a panic within minutes. > > > >> http://people.freebsd.org/~glebius/patches/ifnet.no_free > > > > This patch makes my test system survive longer but does not resolve the > > issue. > > Unfortunately, I'm a bit preoccupied currently, so haven't had a chance to > follow up as yet, but just to follow up on the general issue here: this > problem existed pre-SMP as well, and could be easily triggered by DUMMYNET > and removable interfaces as well (as additional queuing delays just make > the problem worse). In general: we need a solution that penalises interface > removal, not common-case packet processing. As many packets have their > source ifnet looked up in common-case processing (worth checking this > assumption) because it's cheap, any solution that causes an interface > lookup on every input packet (with synchronisation) is also an issue. > > Instead, I think we should go for a more radical notion, which is a bit > harder to implement in our stack: the network stack needs a race-free way > to "drain" all mbufs referring to a particular ifnet, which does not cause > existing processing to become more expensive. This is easy in some > subsystems, but more complex in others -- and the composition of subsystems > makes it all much harder since we need to know that (to be 100% correct) > packets aren't getting passed between subsystems (and hence belong to > neither) in a way that races with a sweep through the subsystems. It may be > possible to get this 99.9% right simply by providing a series of callbacks > into subsystems that cause queues to be walked and drained of packets > matching the doomed ifnet. It may also be quite cheap to have subsystems > that "hold" packets outside of explicit queues for some period (i.e., in a > thread-local pointer out of the stack) add explicit invalidation tests > (i.e., for IFF_DYING) before handing off to prevent those packets from > traversing into other subsystems -- which can be done synchronisation-free, > but still wouldn't 100% prevent the race > > Just to give an example: netisr should offer a method for > netisr_drain_ifnet(struct ifnet *) that causes netisr to walk all of its > queues to find matching packets and free them. Due to direct dispatch and > thread-local queues during processing, netisr should also check IFF_DYING > before handing off. > > If we do that, I wonder how robust the system then becomes...? This may not > be too hard to test. But I'd rather we penalise ifnet removal than, say, > the IP input path when it needs to check a source interface property. Couldn't the dangling pointer problem be solved by adding a 'generation' field to the mbuf structure? The 'generation' could be a system-wide number that gets incremented whenever an interface is removed. The mbuf* functions could keep a (per CPU?) reference count on the number of mbufs allocated/freed during that 'generation'. After interface removal, the ifnet structure could be freed when all the reference counters of generations before the current generation reach zero (whenever that happens). -- Daan Vreeken Vitsch Electronics http://Vitsch.nl tel: +31-(0)40-7113051 / +31-(0)6-46210825 KvK nr: 17174380
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201107251300.20832.Daan>