From owner-svn-src-head@FreeBSD.ORG Tue Nov 29 09:43:41 2011 Return-Path: Delivered-To: svn-src-head@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 4FEB4106564A; Tue, 29 Nov 2011 09:43:41 +0000 (UTC) (envelope-from bz@FreeBSD.org) Received: from mx1.sbone.de (mx1.sbone.de [IPv6:2a01:4f8:130:3ffc::401:25]) by mx1.freebsd.org (Postfix) with ESMTP id 0026E8FC13; Tue, 29 Nov 2011 09:43:40 +0000 (UTC) Received: from mail.sbone.de (mail.sbone.de [IPv6:fde9:577b:c1a9:31::2013:587]) (using TLSv1 with cipher ADH-CAMELLIA256-SHA (256/256 bits)) (No client certificate requested) by mx1.sbone.de (Postfix) with ESMTPS id 17A7725D385E; Tue, 29 Nov 2011 09:43:40 +0000 (UTC) Received: from content-filter.sbone.de (content-filter.sbone.de [IPv6:fde9:577b:c1a9:31::2013:2742]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.sbone.de (Postfix) with ESMTPS id 45345BD5EA8; Tue, 29 Nov 2011 09:43:39 +0000 (UTC) X-Virus-Scanned: amavisd-new at sbone.de Received: from mail.sbone.de ([IPv6:fde9:577b:c1a9:31::2013:587]) by content-filter.sbone.de (content-filter.sbone.de [fde9:577b:c1a9:31::2013:2742]) (amavisd-new, port 10024) with ESMTP id pzQcOFXmkqqd; Tue, 29 Nov 2011 09:43:38 +0000 (UTC) Received: from nv.sbone.de (nv.sbone.de [IPv6:fde9:577b:c1a9:31::2013:138]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.sbone.de (Postfix) with ESMTPSA id D6E57BD5EA7; Tue, 29 Nov 2011 09:43:37 +0000 (UTC) Date: Tue, 29 Nov 2011 09:43:37 +0000 (UTC) From: "Bjoern A. Zeeb" To: Gleb Smirnoff In-Reply-To: <20111129093550.GZ44498@FreeBSD.org> Message-ID: References: <201111281444.pASEixdO095604@svn.freebsd.org> <20111129093550.GZ44498@FreeBSD.org> X-OpenPGP-Key-Id: 0x14003F198FEFA3E77207EE8D2B58B8F83CCF1842 MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org Subject: Re: svn commit: r228071 - head/sys/net X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 29 Nov 2011 09:43:41 -0000 On Tue, 29 Nov 2011, Gleb Smirnoff wrote: > On Mon, Nov 28, 2011 at 07:47:22PM +0000, Bjoern A. Zeeb wrote: > B> On 28. Nov 2011, at 14:44 , Gleb Smirnoff wrote: > B> > B> > Author: glebius > B> > Date: Mon Nov 28 14:44:59 2011 > B> > New Revision: 228071 > B> > URL: http://svn.freebsd.org/changeset/base/228071 > B> > > B> > Log: > B> > - Use generic alloc_unr(9) allocator for if_clone, instead > B> > of hand-made. > B> > - When registering new cloner, check whether a cloner with > B> > same name already exist. > B> > - When allocating unit, also check with help of ifunit() > B> > whether such interface already exist or not. [1] > B> > B> This forces packages to be recompiled; they might like to have a __FreeBSD_version for that? > B> It's not MFCable, at least I think - don't see a MFC after, just want to be sure. > > No plans for MFC. > > btw, I don't like the static initializer of cloners, since it require re-compile of > dependencies. > > What about making an API change: remove the static initializer and make an initializer > function. Modules should have only a pointer to opaque structure. We will bump > __FreeBSD_version for that. But later with any change to the if_clone struct, we > won't have ABI change. If everyone agrees, I can go for that. I have some fairly intrusive changes to cloners sitting in p4 for the V_irtualization but it could make my life easier; I'll be happy to look at the patch. > B> See one more comment inline? > B> > + ifc->ifc_unrhdr = new_unrhdr(0, ifc->ifc_maxunit, &ifc->ifc_mtx); > B> > + LIST_INIT(&ifc->ifc_iflist); > B> > > B> > IF_CLONERS_LOCK(); > B> > + LIST_FOREACH(ifc1, &V_if_cloners, ifc_list) > B> > + if (strcmp(ifc->ifc_name, ifc1->ifc_name) == 0) { > B> > + IF_CLONERS_UNLOCK(); > B> > + IF_CLONE_REMREF(ifc); > B> > + return (EEXIST); > B> > B> At this point you may have a problem not freeing the unr? > > No, there is no problem. The code path that goes to delete_unrhdr() is the > following: > > IF_CLONE_REMREF calls IF_CLONE_REMREF_LOCKED, the latter calls if_clone_free(), > and the last one calls delete_unrhdr(). > > -- Bjoern A. Zeeb You have to have visions! Stop bit received. Insert coin for new address family.