pushing NET_LOCK() further down from if_clone_{create,destroy}()

OK mpi@

4 files changed, 23 insertions, 33 deletions

diff --git a/sys/net/if.c b/sys/net/if.c
index 04ac3e9a862..e355f44e80d 100644
--- a/sys/net/if.c
+++ b/sys/net/if.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: if.c,v 1.582 2019/05/11 23:36:40 mpi Exp $	*/
+/*	$OpenBSD: if.c,v 1.583 2019/05/12 16:38:02 sashan Exp $	*/
 /*	$NetBSD: if.c,v 1.35 1996/05/07 05:26:04 thorpej Exp $	*/
 
 /*
@@ -1215,8 +1215,6 @@ if_clone_create(const char *name, int rdomain)
 	struct ifnet *ifp;
 	int unit, ret;
 
-	NET_ASSERT_LOCKED();
-
 	ifc = if_clone_lookup(name, &unit);
 	if (ifc == NULL)
 		return (EINVAL);
@@ -1224,17 +1222,16 @@ if_clone_create(const char *name, int rdomain)
 	if (ifunit(name) != NULL)
 		return (EEXIST);
 
-	/* XXXSMP breaks atomicity */
-	NET_UNLOCK();
 	ret = (*ifc->ifc_create)(ifc, unit);
-	NET_LOCK();
 
 	if (ret != 0 || (ifp = ifunit(name)) == NULL)
 		return (ret);
 
+	NET_LOCK();
 	if_addgroup(ifp, ifc->ifc_name);
 	if (rdomain != 0)
 		if_setrdomain(ifp, rdomain);
+	NET_UNLOCK();
 
 	return (ret);
 }
@@ -1249,8 +1246,6 @@ if_clone_destroy(const char *name)
 	struct ifnet *ifp;
 	int ret;
 
-	NET_ASSERT_LOCKED();
-
 	ifc = if_clone_lookup(name, NULL);
 	if (ifc == NULL)
 		return (EINVAL);
@@ -1262,17 +1257,15 @@ if_clone_destroy(const char *name)
 	if (ifc->ifc_destroy == NULL)
 		return (EOPNOTSUPP);
 
+	NET_LOCK();
 	if (ifp->if_flags & IFF_UP) {
 		int s;
 		s = splnet();
 		if_down(ifp);
 		splx(s);
 	}
-
-	/* XXXSMP breaks atomicity */
 	NET_UNLOCK();
 	ret = (*ifc->ifc_destroy)(ifp);
-	NET_LOCK();
 
 	return (ret);
 }
@@ -1705,6 +1698,8 @@ ifunit(const char *name)
 {
 	struct ifnet *ifp;
 
+	KERNEL_ASSERT_LOCKED();
+
 	TAILQ_FOREACH(ifp, &ifnet, if_list) {
 		if (strcmp(ifp->if_xname, name) == 0)
 			return (ifp);
@@ -1878,16 +1873,12 @@ ifioctl(struct socket *so, u_long cmd, caddr_t data, struct proc *p)
 	case SIOCIFCREATE:
 		if ((error = suser(p)) != 0)
 			return (error);
-		NET_LOCK();
 		error = if_clone_create(ifr->ifr_name, 0);
-		NET_UNLOCK();
 		return (error);
 	case SIOCIFDESTROY:
 		if ((error = suser(p)) != 0)
 			return (error);
-		NET_LOCK();
 		error = if_clone_destroy(ifr->ifr_name);
-		NET_UNLOCK();
 		return (error);
 	case SIOCSIFGATTR:
 		if ((error = suser(p)) != 0)
@@ -2098,11 +2089,12 @@ ifioctl(struct socket *so, u_long cmd, caddr_t data, struct proc *p)
 	case SIOCSIFRDOMAIN:
 		if ((error = suser(p)) != 0)
 			break;
-		NET_LOCK();
 		error = if_createrdomain(ifr->ifr_rdomainid, ifp);
-		if (!error || error == EEXIST)
+		if (!error || error == EEXIST) {
+			NET_LOCK();
 			error = if_setrdomain(ifp, ifr->ifr_rdomainid);
-		NET_UNLOCK();
+			NET_UNLOCK();
+		}
 		break;
 
 	case SIOCAIFGROUP:
diff --git a/sys/net/if_bridge.c b/sys/net/if_bridge.c
index 24515e1ae32..ca082a07eab 100644
--- a/sys/net/if_bridge.c
+++ b/sys/net/if_bridge.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: if_bridge.c,v 1.330 2019/05/10 12:41:30 claudio Exp $	*/
+/*	$OpenBSD: if_bridge.c,v 1.331 2019/05/12 16:38:02 sashan Exp $	*/
 
 /*
  * Copyright (c) 1999, 2000 Jason L. Wright (jason@thought.net)
@@ -282,8 +282,14 @@ bridge_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data)
 		ifs = ifunit(req->ifbr_ifsname);
 
 		/* try to create the interface if it does't exist */
-		if (ifs == NULL && if_clone_create(req->ifbr_ifsname, 0) == 0)
-			ifs = ifunit(req->ifbr_ifsname);
+		if (ifs == NULL) {
+			/* XXXSMP breaks atomicity */
+			NET_UNLOCK();
+			error = if_clone_create(req->ifbr_ifsname, 0);
+			NET_LOCK();
+			if (error == 0)
+				ifs = ifunit(req->ifbr_ifsname);
+		}
 
 		if (ifs == NULL) {			/* no such interface */
 			error = ENOENT;
diff --git a/sys/net/if_tun.c b/sys/net/if_tun.c
index ac45bbac016..0e4fc9332b1 100644
--- a/sys/net/if_tun.c
+++ b/sys/net/if_tun.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: if_tun.c,v 1.185 2019/05/01 06:11:46 dlg Exp $	*/
+/*	$OpenBSD: if_tun.c,v 1.186 2019/05/12 16:38:02 sashan Exp $	*/
 /*	$NetBSD: if_tun.c,v 1.24 1996/05/07 02:40:48 thorpej Exp $	*/
 
 /*
@@ -316,9 +316,7 @@ tunopen(dev_t dev, int flag, int mode, struct proc *p)
 		int	error;
 
 		snprintf(xname, sizeof(xname), "%s%d", "tun", minor(dev));
-		NET_LOCK();
 		error = if_clone_create(xname, rdomain);
-		NET_UNLOCK();
 		if (error != 0)
 			return (error);
 
@@ -341,9 +339,7 @@ tapopen(dev_t dev, int flag, int mode, struct proc *p)
 		int	error;
 
 		snprintf(xname, sizeof(xname), "%s%d", "tap", minor(dev));
-		NET_LOCK();
 		error = if_clone_create(xname, rdomain);
-		NET_UNLOCK();
 		if (error != 0)
 			return (error);
 
@@ -418,11 +414,9 @@ tun_dev_close(struct tun_softc *tp, int flag, int mode, struct proc *p)
 
 	TUNDEBUG(("%s: closed\n", ifp->if_xname));
 
-	if (!(tp->tun_flags & TUN_STAYUP)) {
-		NET_LOCK();
+	if (!(tp->tun_flags & TUN_STAYUP))
 		error = if_clone_destroy(ifp->if_xname);
-		NET_UNLOCK();
-	} else {
+	else {
 		tp->tun_pgid = 0;
 		selwakeup(&tp->tun_rsel);
 	}
diff --git a/sys/net/switchctl.c b/sys/net/switchctl.c
index 6e40ad6a2a4..1c3b4c7c595 100644
--- a/sys/net/switchctl.c
+++ b/sys/net/switchctl.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: switchctl.c,v 1.14 2018/12/28 14:32:47 bluhm Exp $	*/
+/*	$OpenBSD: switchctl.c,v 1.15 2019/05/12 16:38:02 sashan Exp $	*/
 
 /*
  * Copyright (c) 2016 Kazuya GODA <goda@openbsd.org>
@@ -88,9 +88,7 @@ switchopen(dev_t dev, int flags, int mode, struct proc *p)
 
 	if ((sc = switch_dev2sc(dev)) == NULL) {
 		snprintf(name, sizeof(name), "switch%d", minor(dev));
-		NET_LOCK();
 		rv = if_clone_create(name, rdomain);
-		NET_UNLOCK();
 		if (rv != 0)
 			return (rv);
 		if ((sc = switch_dev2sc(dev)) == NULL)