Date: Mon, 28 Nov 2011 19:47:22 +0000 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: <E15FE643-3360-4D42-8736-827104FDD128@FreeBSD.org> In-Reply-To: <201111281444.pASEixdO095604@svn.freebsd.org> References: <201111281444.pASEixdO095604@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?E15FE643-3360-4D42-8736-827104FDD128>