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
>
> 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?
>
> PR: kern/162789 [1]
>
> Modified:
> head/sys/net/if_clone.c
> head/sys/net/if_clone.h
>
> Modified: head/sys/net/if_clone.c
> ==============================================================================
> --- 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 != NULL, ("%s: no name\n", __func__));
>
> - /*
> - * Compute bitmap size and allocate it.
> - */
> - maxclone = ifc->ifc_maxunit + 1;
> - len = maxclone >> 3;
> - if ((len << 3) < maxclone)
> - len++;
> - ifc->ifc_units = malloc(len, M_CLONE, M_WAITOK | M_ZERO);
> - ifc->ifc_bmlen = len;
> IF_CLONE_LOCK_INIT(ifc);
> IF_CLONE_ADDREF(ifc);
> + ifc->ifc_unrhdr = new_unrhdr(0, ifc->ifc_maxunit, &ifc->ifc_mtx);
> + LIST_INIT(&ifc->ifc_iflist);
>
> IF_CLONERS_LOCK();
> + LIST_FOREACH(ifc1, &V_if_cloners, ifc_list)
> + if (strcmp(ifc->ifc_name, ifc1->ifc_name) == 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();
>
> - LIST_INIT(&ifc->ifc_iflist);
> -
> if (ifc->ifc_attach != NULL)
> (*ifc->ifc_attach)(ifc);
> EVENTHANDLER_INVOKE(if_clone_event, ifc);
> +
> + return (0);
> }
>
> /*
> @@ -338,16 +339,12 @@ if_clone_detach(struct if_clone *ifc)
> static void
> if_clone_free(struct if_clone *ifc)
> {
> - for (int bytoff = 0; bytoff < ifc->ifc_bmlen; bytoff++) {
> - KASSERT(ifc->ifc_units[bytoff] == 0x00,
> - ("ifc_units[%d] is not empty", bytoff));
> - }
>
> KASSERT(LIST_EMPTY(&ifc->ifc_iflist),
> ("%s: ifc_iflist not empty", __func__));
>
> IF_CLONE_LOCK_DESTROY(ifc);
> - free(ifc->ifc_units, M_CLONE);
> + delete_unrhdr(ifc->ifc_unrhdr);
> }
>
> /*
> @@ -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 = 0;
> -
> - IF_CLONE_LOCK(ifc);
> + char name[IFNAMSIZ];
> + int wildcard;
>
> - bytoff = bitoff = 0;
> wildcard = (*unit < 0);
> - /*
> - * Find a free unit if none was given.
> - */
> +retry:
> if (wildcard) {
> - while ((bytoff < ifc->ifc_bmlen)
> - && (ifc->ifc_units[bytoff] == 0xff))
> - bytoff++;
> - if (bytoff >= ifc->ifc_bmlen) {
> - err = ENOSPC;
> - goto done;
> - }
> - while ((ifc->ifc_units[bytoff] & (1 << bitoff)) != 0)
> - bitoff++;
> - *unit = (bytoff << 3) + bitoff;
> - }
> -
> - if (*unit > ifc->ifc_maxunit) {
> - err = ENOSPC;
> - goto done;
> + *unit = alloc_unr(ifc->ifc_unrhdr);
> + if (*unit == -1)
> + return (ENOSPC);
> + } else {
> + *unit = alloc_unr_specific(ifc->ifc_unrhdr, *unit);
> + if (*unit == -1)
> + return (EEXIST);
> }
>
> - if (!wildcard) {
> - bytoff = *unit >> 3;
> - bitoff = *unit - (bytoff << 3);
> + snprintf(name, IFNAMSIZ, "%s%d", ifc->ifc_name, *unit);
> + if (ifunit(name) != NULL) {
> + if (wildcard)
> + goto retry; /* XXXGL: yep, it's a unit leak */
> + else
> + return (EEXIST);
> }
>
> - if((ifc->ifc_units[bytoff] & (1 << bitoff)) != 0) {
> - err = EEXIST;
> - goto done;
> - }
> - /*
> - * Allocate the unit in the bitmap.
> - */
> - KASSERT((ifc->ifc_units[bytoff] & (1 << bitoff)) == 0,
> - ("%s: bit is already set", __func__));
> - ifc->ifc_units[bytoff] |= (1 << bitoff);
> - IF_CLONE_ADDREF_LOCKED(ifc);
> + IF_CLONE_ADDREF(ifc);
>
> -done:
> - IF_CLONE_UNLOCK(ifc);
> - return (err);
> + return (0);
> }
>
> void
> ifc_free_unit(struct if_clone *ifc, int unit)
> {
> - int bytoff, bitoff;
> -
>
> - /*
> - * Compute offset in the bitmap and deallocate the unit.
> - */
> - bytoff = unit >> 3;
> - bitoff = unit - (bytoff << 3);
> -
> - IF_CLONE_LOCK(ifc);
> - KASSERT((ifc->ifc_units[bytoff] & (1 << bitoff)) != 0,
> - ("%s: bit is already cleared", __func__));
> - ifc->ifc_units[bytoff] &= ~(1 << bitoff);
> - IF_CLONE_REMREF_LOCKED(ifc); /* releases lock */
> + free_unr(ifc->ifc_unrhdr, unit);
> + IF_CLONE_REMREF(ifc);
> }
>
> void
>
> Modified: head/sys/net/if_clone.h
> ==============================================================================
> --- 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 @@
>
> #define IFC_CLONE_INITIALIZER(name, data, maxunit, \
> attach, match, create, destroy) \
> - { { 0 }, name, maxunit, NULL, 0, data, attach, match, create, destroy }
> + {
> \
> + .ifc_name = name, \
> + .ifc_maxunit = maxunit, \
> + .ifc_data = data, \
> + .ifc_attach = attach, \
> + .ifc_match = match, \
> + .ifc_create = create, \
> + .ifc_destroy = destroy, \
> + }
>
> /*
> * 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. */
>
> /* (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 *);
>
> 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 */
> };
>
> 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);
>
--
Bjoern A. Zeeb You have to have visions!
Stop bit received. Insert coin for new address family.
_______________________________________________
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
