Updated scope6.c locking patch...

[gnn at freebsd.org]

Hi Folks,

	Enclosed please find my updated patch for scope6 locking for
	the IPv6 code in FreeBSD-CURRENT.  The major change was to
	remove the IFNET list locking because this actually needs to
	be done outside of the IPv6 code and to add address family
	(AF) data locking because that's something we can and should
	do within the scope6 code.

	I have tested this locally in the following way:

1) Open a shell on the machine (shell1) and do:

shell1> ifconfig lo1 create
shell1> while 1
shell1> scope6config -l 1 -s 2 -o 3 lo1
shell1> end

2) Open a second shell on the same machine (shell2) and do:

shell2> ifconfig lo1 destroy
shell2> ifconfig lo1 create

etc.

The messages in the shell1 window should switch between:

lo1: 0, 6, 1, 0, 0, 2, 0, 0, 3, 0, 0, 0, 0, 0, 0, 0

and

scope6config: ioctl(SIOCSSCOPE6): Device not configured

and the kernel should not panic ;-)

Later,
George

---  sys/netinet6/scope6.c.orig
+++  sys/netinet6/scope6.c
@@ -71,7 +71,6 @@
 scope6_ifattach(ifp)
 	struct ifnet *ifp;
 {
-	int s = splnet();
 	struct scope6_id *sid;
 
 	sid = (struct scope6_id *)malloc(sizeof(*sid), M_IFADDR, M_WAITOK);
@@ -89,7 +88,6 @@
 	sid->s6id_list[IPV6_ADDR_SCOPE_ORGLOCAL] = 1;
 #endif
 
-	splx(s);
 	return sid;
 }
 
@@ -106,12 +104,17 @@
 	struct ifnet *ifp;
 	struct scope6_id *idlist;
 {
-	int i, s;
+	int i;
 	int error = 0;
-	struct scope6_id *sid = SID(ifp);
+	struct scope6_id *sid = NULL;
+
+	IF_AFDATA_LOCK(ifp);
+	sid = SID(ifp);
 
-	if (!sid)	/* paranoid? */
+	if (!sid) {	/* paranoid? */
+		IF_AFDATA_UNLOCK(ifp);
 		return (EINVAL);
+	}
 
 	/*
 	 * XXX: We need more consistency checks of the relationship among
@@ -123,8 +126,6 @@
 	 * interface addresses, routing table entries, PCB entries...
 	 */
 
-	s = splnet();
-
 	SCOPE6_LOCK();
 	for (i = 0; i < 16; i++) {
 		if (idlist->s6id_list[i] &&
@@ -135,7 +136,8 @@
 			 */
 			if (i == IPV6_ADDR_SCOPE_INTFACELOCAL &&
 			    idlist->s6id_list[i] != ifp->if_index) {
-				splx(s);
+				IF_AFDATA_UNLOCK(ifp);
+				SCOPE6_UNLOCK();
 				return (EINVAL);
 			}
 
@@ -147,7 +149,8 @@
 				 * IDs, but we check the consistency for
 				 * safety in later use.
 				 */
-				splx(s);
+				IF_AFDATA_UNLOCK(ifp);
+				SCOPE6_UNLOCK();
 				return (EINVAL);
 			}
 
@@ -160,7 +163,7 @@
 		}
 	}
 	SCOPE6_UNLOCK();
-	splx(s);
+	IF_AFDATA_UNLOCK(ifp);
 
 	return (error);
 }
@@ -170,15 +173,20 @@
 	struct ifnet *ifp;
 	struct scope6_id *idlist;
 {
+	/* We only need to lock the interface's afdata for SID() to work. */
+	IF_AFDATA_LOCK(ifp);
 	struct scope6_id *sid = SID(ifp);
 
-	if (sid == NULL)	/* paranoid? */
+	if (sid == NULL) {	/* paranoid? */
+		IF_AFDATA_UNLOCK(ifp);
 		return (EINVAL);
+	}
 
 	SCOPE6_LOCK();
 	*idlist = *sid;
 	SCOPE6_UNLOCK();
 
+	IF_AFDATA_UNLOCK(ifp);
 	return (0);
 }
 
@@ -259,7 +267,11 @@
 {
 	int scope;
 	u_int32_t zoneid = 0;
-	struct scope6_id *sid = SID(ifp);
+	struct scope6_id *sid = NULL;
+
+	IF_AFDATA_LOCK(ifp);
+
+	sid = SID(ifp);
 
 #ifdef DIAGNOSTIC
 	if (sid == NULL) { /* should not happen */
@@ -277,10 +289,12 @@
 	 * interface.
 	 */
 	if (IN6_IS_ADDR_LOOPBACK(addr)) {
-		if (!(ifp->if_flags & IFF_LOOPBACK))
+		if (!(ifp->if_flags & IFF_LOOPBACK)) {
+			IF_AFDATA_UNLOCK(ifp);
 			return (-1);
-		else {
+		} else {
 			*ret_id = 0; /* there's no ambiguity */
+			IF_AFDATA_UNLOCK(ifp);
 			return (0);
 		}
 	}
@@ -315,6 +329,9 @@
 	SCOPE6_UNLOCK();
 
 	*ret_id = zoneid;
+	
+	IF_AFDATA_UNLOCK(ifp);
+
 	return (0);
 }
 
@@ -323,7 +340,7 @@
 	struct ifnet *ifp;	/* note that this might be NULL */
 {
 	/*
-	 * Currently, this function just set the default "interfaces"
+	 * Currently, this function just sets the default "interfaces"
 	 * and "links" according to the given interface.
 	 * We might eventually have to separate the notion of "link" from
 	 * "interface" and provide a user interface to set the default.