When we started calling if_findindex() from if_alloc() with an empty

struct ifnet most of if_findindex() become a complex no-op.  Remove it
and replace it with a corrected version of the four line for loop it
devolved to plus some error handling.  This should probably be replaced
with subr_unit at some point.

Switch from checking ifaddr_byindex to ifnet_byindex when looking for
empty indexes.  Since we're doing this from if_alloc/if_free, we can
only be sure that ifnet_byindex will be correct.  This fixes panics when
loading the ef(4) module.  The panics were caused by the fact that
if_alloc was called four time before if_attach was called and thus
ifaddr_byindex was not set and the same unit was allocated again.  This
in turn caused the first if_attach to fail because the ifp was not the
one in ifnet_byindex(ifp->if_index).

Reported by:	"Wojciech A. Koszek" <dunstan at freebsd dot czest dot pl>
PR:		kern/84987
MFC After:	1 day

1 files changed, 16 insertions, 56 deletions

diff --git a/sys/net/if.c b/sys/net/if.c
index 7594636..7090648 100644
--- a/sys/net/if.c
+++ b/sys/net/if.c
@@ -104,7 +104,6 @@ static int	ifconf(u_long, caddr_t);
 static void	if_grow(void);
 static void	if_init(void *);
 static void	if_check(void *);
-static int	if_findindex(struct ifnet *);
 static void	if_qflush(struct ifaltq *);
 static void	if_route(struct ifnet *, int flag, int fam);
 static int	if_setflag(struct ifnet *, int, int, int *, int);
@@ -329,57 +328,6 @@ if_check(void *dummy __unused)
 	if_slowtimo(0);
 }
 
-/* XXX: should be locked. */
-static int
-if_findindex(struct ifnet *ifp)
-{
-	int i, unit;
-	char eaddr[18], devname[32];
-	const char *name, *p;
-
-	switch (ifp->if_type) {
-	case IFT_ETHER:			/* these types use struct arpcom */
-	case IFT_FDDI:
-	case IFT_XETHER:
-	case IFT_ISO88025:
-	case IFT_L2VLAN:
-	case IFT_BRIDGE:
-		snprintf(eaddr, 18, "%6D", IFP2ENADDR(ifp), ":");
-		break;
-	default:
-		eaddr[0] = '\0';
-		break;
-	}
-	strlcpy(devname, ifp->if_xname, sizeof(devname));
-	name = net_cdevsw.d_name;
-	i = 0;
-	while ((resource_find_dev(&i, name, &unit, NULL, NULL)) == 0) {
-		if (resource_string_value(name, unit, "ether", &p) == 0)
-			if (strcmp(p, eaddr) == 0)
-				goto found;
-		if (resource_string_value(name, unit, "dev", &p) == 0)
-			if (strcmp(p, devname) == 0)
-				goto found;
-	}
-	unit = 0;
-found:
-	if (unit != 0) {
-		if (ifaddr_byindex(unit) == NULL)
-			return (unit);
-		printf("%s%d in use, cannot hardwire it to %s.\n",
-		    name, unit, devname);
-	}
-	for (unit = 1; ; unit++) {
-		if (unit <= if_index && ifaddr_byindex(unit) != NULL)
-			continue;
-		if (resource_string_value(name, unit, "ether", &p) == 0 ||
-		    resource_string_value(name, unit, "dev", &p) == 0)
-			continue;
-		break;
-	}
-	return (unit);
-}
-
 /*
  * Allocate a struct ifnet and in index for an interface.
  */
@@ -390,13 +338,25 @@ if_alloc(u_char type)
 
 	ifp = malloc(sizeof(struct ifnet), M_IFNET, M_WAITOK|M_ZERO);
 
-	/* XXX: This should fail if if_index is too big */
-	ifp->if_index = if_findindex(ifp);
+	/*
+	 * Try to find an empty slot below if_index.  If we fail, take
+	 * the next slot.
+	 *
+	 * XXX: should be locked!
+	 */
+	for (ifp->if_index = 1; ifp->if_index <= if_index; ifp->if_index++) {
+		if (ifnet_byindex(ifp->if_index) == NULL)
+			break;
+	}
+	/* Catch if_index overflow. */
+	if (ifp->if_index < 1) {
+		free(ifp, M_IFNET);
+		return (NULL);
+	}
 	if (ifp->if_index > if_index)
 		if_index = ifp->if_index;
 	if (if_index >= if_indexlim)
 		if_grow();
-
 	ifnet_byindex(ifp->if_index) = ifp;
 
 	ifp->if_type = type;
@@ -436,7 +396,7 @@ if_free_type(struct ifnet *ifp, u_char type)
 	ifnet_byindex(ifp->if_index) = NULL;
 
 	/* XXX: should be locked with if_findindex() */
-	while (if_index > 0 && ifaddr_byindex(if_index) == NULL)
+	while (if_index > 0 && ifnet_byindex(if_index) == NULL)
 		if_index--;
 
 	if (if_com_free[type] != NULL)