Date: Tue, 29 Nov 2011 09:43:37 +0000 (UTC) From: "Bjoern A. Zeeb" <bz@FreeBSD.org> To: Gleb Smirnoff <glebius@FreeBSD.org> Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org Subject: Re: svn commit: r228071 - head/sys/net Message-ID: <alpine.BSF.2.00.1111290942500.4603@ai.fobar.qr> In-Reply-To: <20111129093550.GZ44498@FreeBSD.org> References: <201111281444.pASEixdO095604@svn.freebsd.org> <E15FE643-3360-4D42-8736-827104FDD128@FreeBSD.org> <20111129093550.GZ44498@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?alpine.BSF.2.00.1111290942500.4603>