From owner-svn-src-all@FreeBSD.ORG Mon Nov 28 19:47:28 2011 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id BF3CB106564A; Mon, 28 Nov 2011 19:47:28 +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 2FB488FC12; Mon, 28 Nov 2011 19:47:28 +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 EFFFD25D3892; Mon, 28 Nov 2011 19:47:26 +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 BEBE7BD5E01; Mon, 28 Nov 2011 19:47:25 +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 o+WK4J4T2ajv; Mon, 28 Nov 2011 19:47:23 +0000 (UTC) Received: from orange-en1.sbone.de (orange-en1.sbone.de [IPv6:fde9:577b:c1a9:31:cabc:c8ff:fecf:e8e3]) (using TLSv1 with cipher AES128-SHA (128/128 bits)) (No client certificate requested) by mail.sbone.de (Postfix) with ESMTPSA id 56DFDBD5E00; Mon, 28 Nov 2011 19:47:23 +0000 (UTC) Mime-Version: 1.0 (Apple Message framework v1084) Content-Type: text/plain; charset=us-ascii From: "Bjoern A. Zeeb" In-Reply-To: <201111281444.pASEixdO095604@svn.freebsd.org> Date: Mon, 28 Nov 2011 19:47:22 +0000 Content-Transfer-Encoding: quoted-printable Message-Id: References: <201111281444.pASEixdO095604@svn.freebsd.org> To: Gleb Smirnoff X-Mailer: Apple Mail (2.1084) 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-all@freebsd.org X-Mailman-Version: 2.1.5 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: Mon, 28 Nov 2011 19:47:28 -0000 On 28. Nov 2011, at 14:44 , Gleb Smirnoff wrote: > Author: glebius > Date: Mon Nov 28 14:44:59 2011 > New Revision: 228071 > URL: http://svn.freebsd.org/changeset/base/228071 >=20 > Log: > - Use generic alloc_unr(9) allocator for if_clone, instead > of hand-made. > - When registering new cloner, check whether a cloner with > same name already exist. > - When allocating unit, also check with help of ifunit() > whether such interface already exist or not. [1] This forces packages to be recompiled; they might like to have a = __FreeBSD_version for that? It's not MFCable, at least I think - don't see a MFC after, just want to = be sure. See one more comment inline? >=20 > PR: kern/162789 [1] >=20 > Modified: > head/sys/net/if_clone.c > head/sys/net/if_clone.h >=20 > Modified: head/sys/net/if_clone.c > = =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D > --- head/sys/net/if_clone.c Mon Nov 28 14:39:56 2011 = (r228070) > +++ head/sys/net/if_clone.c Mon Nov 28 14:44:59 2011 = (r228071) > @@ -282,33 +282,34 @@ if_clone_destroyif(struct if_clone *ifc, > /* > * Register a network interface cloner. > */ > -void > +int > if_clone_attach(struct if_clone *ifc) > { > - int len, maxclone; > + struct if_clone *ifc1; > + > + KASSERT(ifc->ifc_name !=3D NULL, ("%s: no name\n", __func__)); >=20 > - /* > - * Compute bitmap size and allocate it. > - */ > - maxclone =3D ifc->ifc_maxunit + 1; > - len =3D maxclone >> 3; > - if ((len << 3) < maxclone) > - len++; > - ifc->ifc_units =3D malloc(len, M_CLONE, M_WAITOK | M_ZERO); > - ifc->ifc_bmlen =3D len; > IF_CLONE_LOCK_INIT(ifc); > IF_CLONE_ADDREF(ifc); > + ifc->ifc_unrhdr =3D new_unrhdr(0, ifc->ifc_maxunit, = &ifc->ifc_mtx); > + LIST_INIT(&ifc->ifc_iflist); >=20 > IF_CLONERS_LOCK(); > + LIST_FOREACH(ifc1, &V_if_cloners, ifc_list) > + if (strcmp(ifc->ifc_name, ifc1->ifc_name) =3D=3D 0) { > + IF_CLONERS_UNLOCK(); > + IF_CLONE_REMREF(ifc); > + return (EEXIST); At this point you may have a problem not freeing the unr? > + } > LIST_INSERT_HEAD(&V_if_cloners, ifc, ifc_list); > V_if_cloners_count++; > IF_CLONERS_UNLOCK(); >=20 > - LIST_INIT(&ifc->ifc_iflist); > - > if (ifc->ifc_attach !=3D NULL) > (*ifc->ifc_attach)(ifc); > EVENTHANDLER_INVOKE(if_clone_event, ifc); > + > + return (0); > } >=20 > /* > @@ -338,16 +339,12 @@ if_clone_detach(struct if_clone *ifc) > static void > if_clone_free(struct if_clone *ifc) > { > - for (int bytoff =3D 0; bytoff < ifc->ifc_bmlen; bytoff++) { > - KASSERT(ifc->ifc_units[bytoff] =3D=3D 0x00, > - ("ifc_units[%d] is not empty", bytoff)); > - } >=20 > KASSERT(LIST_EMPTY(&ifc->ifc_iflist), > ("%s: ifc_iflist not empty", __func__)); >=20 > IF_CLONE_LOCK_DESTROY(ifc); > - free(ifc->ifc_units, M_CLONE); > + delete_unrhdr(ifc->ifc_unrhdr); > } >=20 > /* > @@ -441,73 +438,40 @@ ifc_name2unit(const char *name, int *uni > int > ifc_alloc_unit(struct if_clone *ifc, int *unit) > { > - int wildcard, bytoff, bitoff; > - int err =3D 0; > - > - IF_CLONE_LOCK(ifc); > + char name[IFNAMSIZ]; > + int wildcard; >=20 > - bytoff =3D bitoff =3D 0; > wildcard =3D (*unit < 0); > - /* > - * Find a free unit if none was given. > - */ > +retry: > if (wildcard) { > - while ((bytoff < ifc->ifc_bmlen) > - && (ifc->ifc_units[bytoff] =3D=3D 0xff)) > - bytoff++; > - if (bytoff >=3D ifc->ifc_bmlen) { > - err =3D ENOSPC; > - goto done; > - } > - while ((ifc->ifc_units[bytoff] & (1 << bitoff)) !=3D 0) > - bitoff++; > - *unit =3D (bytoff << 3) + bitoff; > - } > - > - if (*unit > ifc->ifc_maxunit) { > - err =3D ENOSPC; > - goto done; > + *unit =3D alloc_unr(ifc->ifc_unrhdr); > + if (*unit =3D=3D -1) > + return (ENOSPC); > + } else { > + *unit =3D alloc_unr_specific(ifc->ifc_unrhdr, *unit); > + if (*unit =3D=3D -1) > + return (EEXIST); > } >=20 > - if (!wildcard) { > - bytoff =3D *unit >> 3; > - bitoff =3D *unit - (bytoff << 3); > + snprintf(name, IFNAMSIZ, "%s%d", ifc->ifc_name, *unit); > + if (ifunit(name) !=3D NULL) { > + if (wildcard) > + goto retry; /* XXXGL: yep, it's a unit leak = */ > + else > + return (EEXIST); > } >=20 > - if((ifc->ifc_units[bytoff] & (1 << bitoff)) !=3D 0) { > - err =3D EEXIST; > - goto done; > - } > - /* > - * Allocate the unit in the bitmap. > - */ > - KASSERT((ifc->ifc_units[bytoff] & (1 << bitoff)) =3D=3D 0, > - ("%s: bit is already set", __func__)); > - ifc->ifc_units[bytoff] |=3D (1 << bitoff); > - IF_CLONE_ADDREF_LOCKED(ifc); > + IF_CLONE_ADDREF(ifc); >=20 > -done: > - IF_CLONE_UNLOCK(ifc); > - return (err); > + return (0); > } >=20 > void > ifc_free_unit(struct if_clone *ifc, int unit) > { > - int bytoff, bitoff; > - >=20 > - /* > - * Compute offset in the bitmap and deallocate the unit. > - */ > - bytoff =3D unit >> 3; > - bitoff =3D unit - (bytoff << 3); > - > - IF_CLONE_LOCK(ifc); > - KASSERT((ifc->ifc_units[bytoff] & (1 << bitoff)) !=3D 0, > - ("%s: bit is already cleared", __func__)); > - ifc->ifc_units[bytoff] &=3D ~(1 << bitoff); > - IF_CLONE_REMREF_LOCKED(ifc); /* releases lock */ > + free_unr(ifc->ifc_unrhdr, unit); > + IF_CLONE_REMREF(ifc); > } >=20 > void >=20 > Modified: head/sys/net/if_clone.h > = =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D > --- head/sys/net/if_clone.h Mon Nov 28 14:39:56 2011 = (r228070) > +++ head/sys/net/if_clone.h Mon Nov 28 14:44:59 2011 = (r228071) > @@ -37,7 +37,15 @@ >=20 > #define IFC_CLONE_INITIALIZER(name, data, maxunit, = \ > attach, match, create, destroy) = \ > - { { 0 }, name, maxunit, NULL, 0, data, attach, match, create, = destroy } > + { = \ > + .ifc_name =3D name, = \ > + .ifc_maxunit =3D maxunit, = \ > + .ifc_data =3D data, = \ > + .ifc_attach =3D attach, = \ > + .ifc_match =3D match, = \ > + .ifc_create =3D create, = \ > + .ifc_destroy =3D destroy, = \ > + } >=20 > /* > * Structure describing a `cloning' interface. > @@ -52,10 +60,7 @@ struct if_clone { > LIST_ENTRY(if_clone) ifc_list; /* (e) On list of cloners */ > const char *ifc_name; /* (c) Name of device, e.g. = `gif' */ > int ifc_maxunit; /* (c) Maximum unit number */ > - unsigned char *ifc_units; /* (i) Bitmap to handle units. = */ > - /* Considered private, = access */ > - /* via = ifc_(alloc|free)_unit(). */ > - int ifc_bmlen; /* (c) Bitmap length. */ > + struct unrhdr *ifc_unrhdr; /* (c) alloc_unr(9) header */ > void *ifc_data; /* (*) Data for ifc_* functions. = */ >=20 > /* (c) Driver specific cloning functions. Called with no locks = held. */ > @@ -65,12 +70,12 @@ struct if_clone { > int (*ifc_destroy)(struct if_clone *, struct ifnet *); >=20 > long ifc_refcnt; /* (i) Refrence count. */ > - struct mtx ifc_mtx; /* Muted to protect members. */ > + struct mtx ifc_mtx; /* Mutex to protect members. */ > LIST_HEAD(, ifnet) ifc_iflist; /* (i) List of cloned interfaces = */ > }; >=20 > void if_clone_init(void); > -void if_clone_attach(struct if_clone *); > +int if_clone_attach(struct if_clone *); > void if_clone_detach(struct if_clone *); > void vnet_if_clone_init(void); >=20 --=20 Bjoern A. Zeeb You have to have visions! Stop bit received. Insert coin for new address family.