From owner-freebsd-net@FreeBSD.ORG Thu Jun 29 17:21:48 2006 Return-Path: X-Original-To: freebsd-net@freebsd.org Delivered-To: freebsd-net@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 9149316A417; Thu, 29 Jun 2006 17:21:48 +0000 (UTC) (envelope-from julian@elischer.org) Received: from a50.ironport.com (a50.ironport.com [63.251.108.112]) by mx1.FreeBSD.org (Postfix) with ESMTP id 4454A44345; Thu, 29 Jun 2006 17:21:41 +0000 (GMT) (envelope-from julian@elischer.org) Received: from unknown (HELO [10.251.17.220]) ([10.251.17.220]) by a50.ironport.com with ESMTP; 29 Jun 2006 10:21:41 -0700 Message-ID: <44A40C25.904@elischer.org> Date: Thu, 29 Jun 2006 10:21:41 -0700 From: Julian Elischer User-Agent: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7.13) Gecko/20060414 X-Accept-Language: en-us, en MIME-Version: 1.0 To: Yar Tikhiy References: <200606290752.k5T7qU06021639@repoman.freebsd.org> <20060629132354.D73145@mp2.macomnet.net> <20060629131201.GA67682@comp.chem.msu.su> In-Reply-To: <20060629131201.GA67682@comp.chem.msu.su> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: FreeBSD Net , src-committers@FreeBSD.org Subject: Re: cvs commit: src/sys/net if_vlan.c 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: Thu, 29 Jun 2006 17:21:48 -0000 Yar Tikhiy wrote: >On Thu, Jun 29, 2006 at 01:24:56PM +0400, Maxim Konovalov wrote: > > >>On Thu, 29 Jun 2006, 07:52-0000, Yar Tikhiy wrote: >> >> >> >>>yar 2006-06-29 07:52:30 UTC >>> >>> FreeBSD src repository >>> >>> Modified files: >>> sys/net if_vlan.c >>> Log: >>> Detach the interface first, do vlan_unconfig() then. >>> Previously, another thread could get a pointer to the >>> interface by scanning the system-wide list and sleep >>> on the global vlan mutex held by vlan_unconfig(). >>> The interface was gone by the time the other thread >>> woke up. >>> >>> In order to be able to call vlan_unconfig() on a detached >>> interface, remove the purely cosmetic bzero'ing of IF_LLADDR >>> from the function because a detached interface has no addresses. >>> >>> Noticed by: a stress-testing script by maxim >>> Reviewed by: glebius >>> >>> >>Still no cookie :-) >> >>db> bt >>Tracing pid 75800 tid 100098 td 0xc2b0e960 >>in_control(c2a1c67c,c02069f6,c40eece0,c2e66000,c2b0e960) at in_control+0x114 >>ifioctl(c2a1c67c,c02069f6,c40eece0,c2b0e960,0,...) at ifioctl+0xee >>soo_ioctl(c27cb4c8,c02069f6,c40eece0,c2c04980,c2b0e960) at soo_ioctl+0x2db >>ioctl(c2b0e960,d56a4d04) at ioctl+0x370 >>syscall(3b,3b,3b,bfbfe2c4,0,...) at syscall+0x27e >>Xint0x80_syscall() at Xint0x80_syscall+0x1f >>--- syscall (54, FreeBSD ELF32, ioctl), eip = 0x2817cb43, esp = >>0xbfbfe28c, ebp = 0xbfbfe2d8 --- >> >>Let me know if you need more info. >> >> > >I stress tested gif(4) in the same manner for kicks and got a very >similar panic in in_control(). I suppose that my change eliminated >a concurrency problem in vlan(4) and we began to feel the lack of >refcounting at ifnet level. Indeed, a thread can keep a pointer >to an ifnet beyond its lifetime and panic the system on access to >the dead ifnet. > > > Unfortunatly, since mbufs point to ifnets it is almost impossible to "efficiently" refcount ifnets. Mbufs may persist almost indefinitly in a socket receive buffer, well after the given receive interface has gone away. I submitted patches to full real referenc counting of ifnets in 1995 but it was already too cumbersom then, and since then it has gotten worse. (due to SMP etc.) The solution I guess is to make mbufs reference ifnets by some indirect method that can be validity checked. The same solution was used to fix a reference problem with proc structures in 1992. (the PID was stored in some places instead of struct proc *). Basically, we assign a monatomically increasing number (assuming no wrap) to every interface. We keep that number in a hash with a pointer to the ifnet. We change the ifp in mbuf headers to hold that number, and always access it with a function (ifnum2ifp(ifnum)) and if it returns NULL then we drop the packet as its receive interface has gone away. There is already an interface number, but it gets re-used. (maybe that is not a problem?). You probably should still take out a refeence on the ifnet (or a lock) if you decide to make use of the ifnet. (e.g. probably ipfw should do something to stop it from going away while it is testing recv interface). this of course introduces some overhead.. how much we can afford is another question.