Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 19 Jul 2005 21:23:34 GMT
From:      Robert Watson <rwatson@FreeBSD.org>
To:        Perforce Change Reviews <perforce@freebsd.org>
Subject:   PERFORCE change 80553 for review
Message-ID:  <200507192123.j6JLNXiL090767@repoman.freebsd.org>

next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=80553

Change 80553 by rwatson@rwatson_zoo on 2005/07/19 21:22:39

	Merge change 80537 from rwatson_netperf to netsmp:
	
	Re-formulate and lock down if_delmulti().
	
	Comment on an inconsistency or two in the implementation.

Affected files ...

.. //depot/projects/netsmp/src/sys/net/if.c#3 edit

Differences ...

==== //depot/projects/netsmp/src/sys/net/if.c#3 (text+ko) ====

@@ -2023,6 +2023,8 @@
 	/*
 	 * Must generate the message while holding the lock so that 'ifma'
 	 * pointer is still valid.
+	 *
+	 * XXXRW: How come we don't announce ll_ifma?
 	 */
 	rt_newmaddrmsg(RTM_NEWMADDR, ifma);
 	IF_ADDR_UNLOCK(ifp);
@@ -2049,72 +2051,50 @@
 int
 if_delmulti(struct ifnet *ifp, struct sockaddr *sa)
 {
-	struct ifmultiaddr *ifma;
-	int s;
+	struct ifmultiaddr *ifma, *ll_ifma;
 
-	TAILQ_FOREACH(ifma, &ifp->if_multiaddrs, ifma_link)
-		if (sa_equal(sa, ifma->ifma_addr))
-			break;
-	if (ifma == NULL)
+	IF_ADDR_LOCK(ifp);
+	ifma = if_findmulti(ifp, sa);
+	if (ifma == NULL) {
+		IF_ADDR_UNLOCK(ifp);
 		return ENOENT;
+	}
 
 	if (ifma->ifma_refcount > 1) {
 		ifma->ifma_refcount--;
+		IF_ADDR_UNLOCK(ifp);
 		return 0;
 	}
 
-	rt_newmaddrmsg(RTM_DELMADDR, ifma);
 	sa = ifma->ifma_lladdr;
-	s = splimp();
-	TAILQ_REMOVE(&ifp->if_multiaddrs, ifma, ifma_link);
+	ll_ifma = if_findmulti(ifp, sa);
+
 	/*
-	 * Make sure the interface driver is notified
-	 * in the case of a link layer mcast group being left.
+	 * XXXRW: How come we don't announce ll_ifma?
 	 */
-	if (ifp->if_ioctl && ifma->ifma_addr->sa_family == AF_LINK && sa == 0) {
-		IFF_LOCKGIANT(ifp);
-		(void) (*ifp->if_ioctl)(ifp, SIOCDELMULTI, 0);
-		IFF_UNLOCKGIANT(ifp);
+	rt_newmaddrmsg(RTM_DELMADDR, ifma);
+
+	TAILQ_REMOVE(&ifp->if_multiaddrs, ifma, ifma_link);
+	if_freemulti(ifma);
+
+	if (ll_ifma != NULL) {
+		if (ll_ifma->ifma_refcount == 1) {
+			TAILQ_REMOVE(&ifp->if_multiaddrs, ll_ifma, ifma_link);
+			if_freemulti(ll_ifma);
+		} else
+			ll_ifma->ifma_refcount--;
 	}
-	splx(s);
-	free(ifma->ifma_addr, M_IFMADDR);
-	free(ifma, M_IFMADDR);
-	if (sa == NULL)
-		return 0;
+	IF_ADDR_UNLOCK(ifp);
 
 	/*
-	 * Now look for the link-layer address which corresponds to
-	 * this network address.  It had been squirreled away in
-	 * ifma->ifma_lladdr for this purpose (so we don't have
-	 * to call ifp->if_resolvemulti() again), and we saved that
-	 * value in sa above.  If some nasty deleted the
-	 * link-layer address out from underneath us, we can deal because
-	 * the address we stored was is not the same as the one which was
-	 * in the record for the link-layer address.  (So we don't complain
-	 * in that case.)
+	 * Make sure the interface driver is notified
+	 * in the case of a link layer mcast group being left.
 	 */
-	TAILQ_FOREACH(ifma, &ifp->if_multiaddrs, ifma_link)
-		if (sa_equal(sa, ifma->ifma_addr))
-			break;
-	if (ifma == NULL)
-		return 0;
-
-	if (ifma->ifma_refcount > 1) {
-		ifma->ifma_refcount--;
-		return 0;
-	}
-
-	s = splimp();
-	TAILQ_REMOVE(&ifp->if_multiaddrs, ifma, ifma_link);
-	if (ifp->if_ioctl != NULL) {
+	if (ifp->if_ioctl) {
 		IFF_LOCKGIANT(ifp);
 		(void) (*ifp->if_ioctl)(ifp, SIOCDELMULTI, 0);
 		IFF_UNLOCKGIANT(ifp);
 	}
-	splx(s);
-	free(ifma->ifma_addr, M_IFMADDR);
-	free(sa, M_IFMADDR);
-	free(ifma, M_IFMADDR);
 
 	return 0;
 }



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200507192123.j6JLNXiL090767>