From owner-svn-src-all@FreeBSD.ORG Fri Oct 18 10:09:25 2013 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTP id CBB27FC6; Fri, 18 Oct 2013 10:09:25 +0000 (UTC) (envelope-from glebius@FreeBSD.org) Received: from cell.glebius.int.ru (glebius.int.ru [81.19.69.10]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id A28CE220C; Fri, 18 Oct 2013 10:09:24 +0000 (UTC) Received: from cell.glebius.int.ru (localhost [127.0.0.1]) by cell.glebius.int.ru (8.14.7/8.14.7) with ESMTP id r9IA9GUk089809; Fri, 18 Oct 2013 14:09:16 +0400 (MSK) (envelope-from glebius@FreeBSD.org) Received: (from glebius@localhost) by cell.glebius.int.ru (8.14.7/8.14.7/Submit) id r9IA9G9a089808; Fri, 18 Oct 2013 14:09:16 +0400 (MSK) (envelope-from glebius@FreeBSD.org) X-Authentication-Warning: cell.glebius.int.ru: glebius set sender to glebius@FreeBSD.org using -f Date: Fri, 18 Oct 2013 14:09:16 +0400 From: Gleb Smirnoff To: Marko Zec Subject: Re: svn commit: r228969 - head/sys/netinet Message-ID: <20131018100915.GH52889@FreeBSD.org> References: <201112292041.pBTKfGkj071711@svn.freebsd.org> <201310161709.04489.jhb@freebsd.org> <20131017193545.GA3683@gmail.com> <201310180007.36048.zec@fer.hr> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <201310180007.36048.zec@fer.hr> User-Agent: Mutt/1.5.21 (2010-09-15) Cc: src-committers@freebsd.org, John Baldwin , svn-src-all@freebsd.org, hrs@freebsd.org, Mikolaj Golub , Bjoern Zeeb , svn-src-head@freebsd.org X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 18 Oct 2013 10:09:25 -0000 On Fri, Oct 18, 2013 at 12:07:35AM +0200, Marko Zec wrote: M> > Concerning this more general solution from Gleb, with storing the vnet M> > pointer in the task, I wander how it is safe when the vnet is being M> > removed? M> > M> > In vnet_destroy() the vnet interfaces are moved back to their parent M> > vnets, so setting the vnet context from the interface looks safer. M> M> +1 M> M> Gleb, M> M> what exactly was the justification for embedding a vnet context inside M> struct task, when it could have been easily embedded, directly or M> indirectly, inside the object pointed to by ta_context? M> M> What is / was your plan to garbage collect such stale pointers? M> M> At least in the early days of VIMAGE / VNET hacking, all the people involved M> strived very hard to store backpointers to vnets in as few structs as M> possible, and for quite a while we managed to constrain this to only ifnets M> and sockets, and both of those references are refcounted in struct vnet. And my change is in the same direction, isn't it? We will store vnet pointer only in the struct task, not in all structs that ta_context may point at. M> Now, more (unreferenced) backpointers to struct vnet seem to be popping up M> here and there, but in most cases they are embedded in objects which are M> guaranteed to persist only as long as the referenced vnet is alive. One M> notable exception to this rule is struct tcpcb which apparently can outlive M> vnet teardowns under certain conditions, but this is a known issue which M> needs to be fixed, not a precedent which calls for more timebombs to be M> scattered around the source tree. M> M> So pls. reconsider solving the issue at hand using a different approach, and M> back out r256587. I don't see problem here. When a vnet is destroyed, every facility is notified to tear down its vnet related data. In pf(4) that would mean, that the task is freed, and would never be put again on taskqueue. So I don't see any leaks or dereferences of dead vnet here. I have just looked into multicast code and see that it takes another approach. The task is single and global, and it sets vnet context per-option. Well, my patch won't hurt its operation. Just orthogonal approach. -- Totus tuus, Glebius.