- 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]

PR:		kern/162789 [1]

2 files changed, 47 insertions, 78 deletions

diff --git a/sys/net/if_clone.c b/sys/net/if_clone.c
index c02737b..5720fc4 100644
--- a/sys/net/if_clone.c
+++ b/sys/net/if_clone.c
@@ -282,33 +282,34 @@ if_clone_destroyif(struct if_clone *ifc, struct ifnet *ifp)
 /*
  * 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);
+		}
 	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 *unit)
 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
diff --git a/sys/net/if_clone.h b/sys/net/if_clone.h
index f125f8b..4278814 100644
--- a/sys/net/if_clone.h
+++ b/sys/net/if_clone.h
@@ -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);