From owner-freebsd-net@FreeBSD.ORG Mon Jul 25 11:00:21 2011 Return-Path: Delivered-To: freebsd-net@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id CCCE61065670; Mon, 25 Jul 2011 11:00:21 +0000 (UTC) (envelope-from Daan@vitsch.nl) Received: from VM01.VEHosting.nl (VM016.VEHosting.nl [IPv6:2001:1af8:2100:b020::140]) by mx1.freebsd.org (Postfix) with ESMTP id 691918FC08; Mon, 25 Jul 2011 11:00:21 +0000 (UTC) Received: from [192.168.72.11] (124-54.bbned.dsl.internl.net [92.254.54.124]) (authenticated bits=0) by VM01.VEHosting.nl (8.14.3/8.13.8) with ESMTP id p6PB0L7g051204; Mon, 25 Jul 2011 13:00:21 +0200 (CEST) (envelope-from Daan@vitsch.nl) From: Daan Vreeken Organization: Vitsch Electronics To: freebsd-net@freebsd.org Date: Mon, 25 Jul 2011 13:00:20 +0200 User-Agent: KMail/1.9.10 References: <20110714154457.GI70776@FreeBSD.org> In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <201107251300.20832.Daan@vitsch.nl> x-ve-auth-version: mi-1.1.5 2011-02-07 - Copyright (c) 2008, 2011 - Daan Vreeken - VEHosting x-ve-auth: authenticated as 'pa4dan' on VM01.VEHosting.nl Cc: Ryan Stone , bz@freebsd.org, "Robert N. M. Watson" , gnn@freebsd.org Subject: Re: m_pkthdr.rcvif dangling pointer problem X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 25 Jul 2011 11:00:22 -0000 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