PERFORCE change 80731 for review

[Robert Watson]

http://perforce.freebsd.org/chv.cgi?CH=80731

Change 80731 by rwatson at rwatson_zoo on 2005/07/22 02:48:12

	Add a new mutex, in_multi_mtx, which will protect IPv4 multicast
	addresses, as well as the consistency of IPv4 multicast data
	pointed to by link level multicast address entries.  Initialize
	with a SYSINIT().  Provide accessor macros.
	
	Assert in_multi_mtx in IN_*_MULTI() macros.
	
	Acquire in_multi_mtx in in_addmulti(), in_delmulti().  Hold over
	calls to link layer multicast code, as well as IGMP code.
	
	NB: Need to look at lock order issues relating to how these calls
	are reached, and where the igmp code calls into.

Affected files ...

.. //depot/projects/netsmp/src/sys/netinet/in.c#2 edit
.. //depot/projects/netsmp/src/sys/netinet/in_var.h#5 edit

Differences ...

==== //depot/projects/netsmp/src/sys/netinet/in.c#2 (text+ko) ====

@@ -68,7 +68,14 @@
 SYSCTL_INT(_net_inet_ip, OID_AUTO, subnets_are_local, CTLFLAG_RW,
 	&subnetsarelocal, 0, "Treat all subnets as directly connected");
 
+/*
+ * The IPv4 multicast list (in_multihead and associated structures)
+ * are protected by the global in_multi_mtx.  See in_var.h for
+ * more details.
+ */
 struct in_multihead in_multihead; /* XXX BSS initialization */
+struct mtx in_multi_mtx;
+MTX_SYSINIT(in_multi_mtx, &in_multi_mtx, "in_multi_mtx", MTX_DEF);
 
 extern struct inpcbinfo ripcbinfo;
 extern struct inpcbinfo udbinfo;
@@ -951,6 +958,7 @@
 	struct ifmultiaddr *ifma;
 	int s = splnet();
 
+	IN_MULTI_LOCK();
 	/*
 	 * Call generic routine to add membership or increment
 	 * refcount.  It wants addresses in the form of a sockaddr,
@@ -962,6 +970,7 @@
 	sin.sin_addr = *ap;
 	error = if_addmulti(ifp, (struct sockaddr *)&sin, &ifma);
 	if (error) {
+		IN_MULTI_UNLOCK();
 		splx(s);
 		return 0;
 	}
@@ -971,15 +980,21 @@
 	 * a new record.  Otherwise, we are done.
 	 */
 	if (ifma->ifma_protospec != NULL) {
+		IN_MULTI_UNLOCK();
 		splx(s);
 		return ifma->ifma_protospec;
 	}
 
 	/* XXX - if_addmulti uses M_WAITOK.  Can this really be called
 	   at interrupt time?  If so, need to fix if_addmulti. XXX */
+	/*
+	 * XXXRW: This comment is right, and yes, we should use M_NOWAIT
+	 * here due to spl's and now mutexes.
+	 */
 	inm = (struct in_multi *)malloc(sizeof(*inm), M_IPMADDR,
 	    M_NOWAIT | M_ZERO);
 	if (inm == NULL) {
+		IN_MULTI_UNLOCK();
 		splx(s);
 		return (NULL);
 	}
@@ -994,6 +1009,7 @@
 	 * Let IGMP know that we have joined a new IP multicast group.
 	 */
 	igmp_joingroup(inm);
+	IN_MULTI_UNLOCK();
 	splx(s);
 	return (inm);
 }
@@ -1005,10 +1021,12 @@
 in_delmulti(inm)
 	register struct in_multi *inm;
 {
-	struct ifmultiaddr *ifma = inm->inm_ifma;
+	struct ifmultiaddr *ifma;
 	struct in_multi my_inm;
 	int s = splnet();
 
+	IN_MULTI_LOCK();
+	ifma = inm->inm_ifma;
 	my_inm.inm_ifp = NULL ; /* don't send the leave msg */
 	if (ifma->ifma_refcount == 1) {
 		/*
@@ -1026,5 +1044,6 @@
 	if_delmulti(ifma->ifma_ifp, ifma->ifma_addr);
 	if (my_inm.inm_ifp != NULL)
 		igmp_leavegroup(&my_inm);
+	IN_MULTI_UNLOCK();
 	splx(s);
 }

==== //depot/projects/netsmp/src/sys/netinet/in_var.h#5 (text+ko) ====

@@ -167,6 +167,17 @@
 extern LIST_HEAD(in_multihead, in_multi) in_multihead;
 
 /*
+ * Lock macros for IPv4 layer multicast address lists.  IPv4 lock goes
+ * before link layer multicast locks in the lock order.  In most cases,
+ * consumers of IN_*_MULTI() macros should acquire the locks before
+ * calling them; users of the in_{add,del}multi() functions should not.
+ */
+extern struct mtx in_multi_mtx;
+#define	IN_MULTI_LOCK()		mtx_lock(&in_multi_mtx)
+#define	IN_MULTI_UNLOCK()	mtx_unlock(&in_multi_mtx)
+#define	IN_MULTI_LOCK_ASSERT()	mtx_assert(&in_multi_mtx, MA_OWNED)
+
+/*
  * Structure used by macros below to remember position when stepping through
  * all of the in_multi records.
  */
@@ -187,6 +198,7 @@
 do { \
 	struct ifmultiaddr *ifma; \
 \
+	IN_MULTI_LOCK_ASSERT(); \
 	IF_ADDR_LOCK(ifp); \
 	TAILQ_FOREACH(ifma, &((ifp)->if_multiaddrs), ifma_link) { \
 		if (ifma->ifma_addr->sa_family == AF_INET \
@@ -209,6 +221,7 @@
 	/* struct in_multistep  step; */ \
 	/* struct in_multi *inm; */ \
 do { \
+	IN_MULTI_LOCK_ASSERT(); \
 	if (((inm) = (step).i_inm) != NULL) \
 		(step).i_inm = LIST_NEXT((step).i_inm, inm_link); \
 } while(0)
@@ -217,6 +230,7 @@
 	/* struct in_multistep step; */ \
 	/* struct in_multi *inm; */ \
 do { \
+	IN_MULTI_LOCK_ASSERT(); \
 	(step).i_inm = LIST_FIRST(&in_multihead); \
 	IN_NEXT_MULTI((step), (inm)); \
 } while(0)