Date: Fri, 22 Jul 2005 02:49:12 GMT From: Robert Watson <rwatson@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 80731 for review Message-ID: <200507220249.j6M2nCaP045985@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=80731 Change 80731 by rwatson@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)
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200507220249.j6M2nCaP045985>