svn commit: r257086 - user/ae/inet6/sys/netinet6

Andrey V. Elsukov ae at FreeBSD.org
Fri Oct 25 01:47:03 UTC 2013


Author: ae
Date: Fri Oct 25 01:47:02 2013
New Revision: 257086
URL: http://svnweb.freebsd.org/changeset/base/257086

Log:
  Scope related cleanup in mld6.c:
  * cleanup comments;
  * remove in6_clearscope() and in6_setscope() calls;

Modified:
  user/ae/inet6/sys/netinet6/mld6.c

Modified: user/ae/inet6/sys/netinet6/mld6.c
==============================================================================
--- user/ae/inet6/sys/netinet6/mld6.c	Fri Oct 25 01:10:07 2013	(r257085)
+++ user/ae/inet6/sys/netinet6/mld6.c	Fri Oct 25 01:47:02 2013	(r257086)
@@ -162,32 +162,9 @@ static int	sysctl_mld_ifinfo(SYSCTL_HAND
  *    per-link state iterators.
  *
  *  XXX LOR PREVENTION
- *  A special case for IPv6 is the in6_setscope() routine. ip6_output()
- *  will not accept an ifp; it wants an embedded scope ID, unlike
- *  ip_output(), which happily takes the ifp given to it. The embedded
- *  scope ID is only used by MLD to select the outgoing interface.
  *
  *  During interface attach and detach, MLD will take MLD_LOCK *after*
  *  the IF_AFDATA_LOCK.
- *  As in6_setscope() takes IF_AFDATA_LOCK then SCOPE_LOCK, we can't call
- *  it with MLD_LOCK held without triggering an LOR. A netisr with indirect
- *  dispatch could work around this, but we'd rather not do that, as it
- *  can introduce other races.
- *
- *  As such, we exploit the fact that the scope ID is just the interface
- *  index, and embed it in the IPv6 destination address accordingly.
- *  This is potentially NOT VALID for MLDv1 reports, as they
- *  are always sent to the multicast group itself; as MLDv2
- *  reports are always sent to ff02::16, this is not an issue
- *  when MLDv2 is in use.
- *
- *  This does not however eliminate the LOR when ip6_output() itself
- *  calls in6_setscope() internally whilst MLD_LOCK is held. This will
- *  trigger a LOR warning in WITNESS when the ifnet is detached.
- *
- *  The right answer is probably to make IF_AFDATA_LOCK an rwlock, given
- *  how it's used across the network stack. Here we're simply exploiting
- *  the fact that MLD runs at a similar layer in the stack to scope6.c.
  *
  * VIMAGE:
  *  * Each in6_multi corresponds to an ifp, and each ifp corresponds
@@ -196,11 +173,6 @@ static int	sysctl_mld_ifinfo(SYSCTL_HAND
 static struct mtx		 mld_mtx;
 static MALLOC_DEFINE(M_MLD, "mld", "mld state");
 
-#define	MLD_EMBEDSCOPE(pin6, zoneid)					\
-	if (IN6_IS_SCOPE_LINKLOCAL(pin6) ||				\
-	    IN6_IS_ADDR_MC_INTFACELOCAL(pin6))				\
-		(pin6)->s6_addr16[1] = htons((zoneid) & 0xFFFF)		\
-
 /*
  * VIMAGE-wide globals.
  */
@@ -426,11 +398,7 @@ mld_dispatch_queue(struct ifqueue *ifq, 
  * Filter outgoing MLD report state by group.
  *
  * Reports are ALWAYS suppressed for ALL-HOSTS (ff02::1)
- * and node-local addresses. However, kernel and socket consumers
- * always embed the KAME scope ID in the address provided, so strip it
- * when performing comparison.
- * Note: This is not the same as the *multicast* scope.
- *
+ * and node-local addresses.
  * Return zero if the given group is one for which MLD reports
  * should be suppressed, or non-zero if reports should be issued.
  */
@@ -440,16 +408,9 @@ mld_is_addr_reported(const struct in6_ad
 
 	KASSERT(IN6_IS_ADDR_MULTICAST(addr), ("%s: not multicast", __func__));
 
-	if (IPV6_ADDR_MC_SCOPE(addr) == IPV6_ADDR_SCOPE_NODELOCAL)
+	if (IPV6_ADDR_MC_SCOPE(addr) == IPV6_ADDR_SCOPE_NODELOCAL ||
+	    IN6_ARE_ADDR_EQUAL(addr, &in6addr_linklocal_allnodes))
 		return (0);
-
-	if (IPV6_ADDR_MC_SCOPE(addr) == IPV6_ADDR_SCOPE_LINKLOCAL) {
-		struct in6_addr tmp = *addr;
-		in6_clearscope(&tmp);
-		if (IN6_ARE_ADDR_EQUAL(&tmp, &in6addr_linklocal_allnodes))
-			return (0);
-	}
-
 	return (1);
 }
 
@@ -619,9 +580,6 @@ mli_delete_locked(const struct ifnet *if
 /*
  * Process a received MLDv1 general or address-specific query.
  * Assumes that the query header has been pulled up to sizeof(mld_hdr).
- *
- * NOTE: Can't be fully const correct as we temporarily embed scope ID in
- * mld_addr. This is OK as we own the mbuf chain.
  */
 static int
 mld_v1_input_query(struct ifnet *ifp, const struct ip6_hdr *ip6,
@@ -665,19 +623,10 @@ mld_v1_input_query(struct ifnet *ifp, co
 		 * MLDv1 General Query.
 		 * If this was not sent to the all-nodes group, ignore it.
 		 */
-		struct in6_addr		 dst;
-
-		dst = ip6->ip6_dst;
-		in6_clearscope(&dst);
-		if (!IN6_ARE_ADDR_EQUAL(&dst, &in6addr_linklocal_allnodes))
+		if (!IN6_ARE_ADDR_EQUAL(&ip6->ip6_dst,
+		    &in6addr_linklocal_allnodes))
 			return (EINVAL);
 		is_general_query = 1;
-	} else {
-		/*
-		 * Embed scope ID of receiving interface in MLD query for
-		 * lookup whilst we don't hold other locks.
-		 */
-		in6_setscope(&mld->mld_addr, ifp, NULL);
 	}
 
 	IN6_MULTI_LOCK();
@@ -722,8 +671,6 @@ mld_v1_input_query(struct ifnet *ifp, co
 			    ifp, ifp->if_xname);
 			mld_v1_update_group(inm, timer);
 		}
-		/* XXX Clear embedded scope ID as userland won't expect it. */
-		in6_clearscope(&mld->mld_addr);
 	}
 
 	IF_ADDR_RUNLOCK(ifp);
@@ -873,13 +820,6 @@ mld_v2_input_query(struct ifnet *ifp, co
 		if (nsrc > 0)
 			return (EINVAL);
 		is_general_query = 1;
-	} else {
-		/*
-		 * Embed scope ID of receiving interface in MLD query for
-		 * lookup whilst we don't hold other locks (due to KAME
-		 * locking lameness). We own this mbuf chain just now.
-		 */
-		in6_setscope(&mld->mld_addr, ifp, NULL);
 	}
 
 	IN6_MULTI_LOCK();
@@ -958,8 +898,6 @@ mld_v2_input_query(struct ifnet *ifp, co
 		if (mli->mli_v2_timer == 0 || mli->mli_v2_timer >= timer)
 			mld_v2_process_group_query(inm, mli, timer, m, off);
 
-		/* XXX Clear embedded scope ID as userland won't expect it. */
-		in6_clearscope(&mld->mld_addr);
 		IF_ADDR_RUNLOCK(ifp);
 	}
 
@@ -1086,15 +1024,11 @@ mld_v2_process_group_query(struct in6_mu
 /*
  * Process a received MLDv1 host membership report.
  * Assumes mld points to mld_hdr in pulled up mbuf chain.
- *
- * NOTE: Can't be fully const correct as we temporarily embed scope ID in
- * mld_addr. This is OK as we own the mbuf chain.
  */
 static int
 mld_v1_input_report(struct ifnet *ifp, const struct ip6_hdr *ip6,
     /*const*/ struct mld_hdr *mld)
 {
-	struct in6_addr		 src, dst;
 	struct in6_ifaddr	*ia;
 	struct in6_multi	*inm;
 #ifdef KTR
@@ -1115,9 +1049,8 @@ mld_v1_input_report(struct ifnet *ifp, c
 	 * MLDv1 reports must originate from a host's link-local address,
 	 * or the unspecified address (when booting).
 	 */
-	src = ip6->ip6_src;
-	in6_clearscope(&src);
-	if (!IN6_IS_SCOPE_LINKLOCAL(&src) && !IN6_IS_ADDR_UNSPECIFIED(&src)) {
+	if (!IN6_IS_SCOPE_LINKLOCAL(&ip6->ip6_src) &&
+	    !IN6_IS_ADDR_UNSPECIFIED(&ip6->ip6_src)) {
 		CTR3(KTR_MLD, "ignore v1 query src %s on ifp %p(%s)",
 		    ip6_sprintf(ip6tbuf, &ip6->ip6_src),
 		    ifp, ifp->if_xname);
@@ -1128,10 +1061,8 @@ mld_v1_input_report(struct ifnet *ifp, c
 	 * RFC2710 Section 4: MLDv1 reports must pertain to a multicast
 	 * group, and must be directed to the group itself.
 	 */
-	dst = ip6->ip6_dst;
-	in6_clearscope(&dst);
 	if (!IN6_IS_ADDR_MULTICAST(&mld->mld_addr) ||
-	    !IN6_ARE_ADDR_EQUAL(&mld->mld_addr, &dst)) {
+	    !IN6_ARE_ADDR_EQUAL(&mld->mld_addr, &ip6->ip6_dst)) {
 		CTR3(KTR_MLD, "ignore v1 query dst %s on ifp %p(%s)",
 		    ip6_sprintf(ip6tbuf, &ip6->ip6_dst),
 		    ifp, ifp->if_xname);
@@ -1150,7 +1081,7 @@ mld_v1_input_report(struct ifnet *ifp, c
 	 */
 	ia = in6ifa_ifpforlinklocal(ifp, IN6_IFF_NOTREADY|IN6_IFF_ANYCAST);
 	if ((ia && IN6_ARE_ADDR_EQUAL(&ip6->ip6_src, IA6_IN6(ia))) ||
-	    (ia == NULL && IN6_IS_ADDR_UNSPECIFIED(&src))) {
+	    (ia == NULL && IN6_IS_ADDR_UNSPECIFIED(&ip6->ip6_src))) {
 		if (ia != NULL)
 			ifa_free(&ia->ia_ifa);
 		return (0);
@@ -1161,13 +1092,6 @@ mld_v1_input_report(struct ifnet *ifp, c
 	CTR3(KTR_MLD, "process v1 report %s on ifp %p(%s)",
 	    ip6_sprintf(ip6tbuf, &mld->mld_addr), ifp, ifp->if_xname);
 
-	/*
-	 * Embed scope ID of receiving interface in MLD query for lookup
-	 * whilst we don't hold other locks (due to KAME locking lameness).
-	 */
-	if (!IN6_IS_ADDR_UNSPECIFIED(&mld->mld_addr))
-		in6_setscope(&mld->mld_addr, ifp, NULL);
-
 	IN6_MULTI_LOCK();
 	MLD_LOCK();
 	IF_ADDR_RLOCK(ifp);
@@ -1221,10 +1145,6 @@ out_locked:
 	IF_ADDR_RUNLOCK(ifp);
 	MLD_UNLOCK();
 	IN6_MULTI_UNLOCK();
-
-	/* XXX Clear embedded scope ID as userland won't expect it. */
-	in6_clearscope(&mld->mld_addr);
-
 	return (0);
 }
 
@@ -1839,7 +1759,6 @@ mld_v1_transmit_report(struct in6_multi 
 	mld->mld_maxdelay = 0;
 	mld->mld_reserved = 0;
 	mld->mld_addr = in6m->in6m_addr;
-	in6_clearscope(&mld->mld_addr);
 	mld->mld_cksum = in6_cksum(mh, IPPROTO_ICMPV6,
 	    sizeof(struct ip6_hdr), sizeof(struct mld_hdr));
 
@@ -2466,7 +2385,6 @@ mld_v2_enqueue_group_record(struct ifque
 	mr.mr_datalen = 0;
 	mr.mr_numsrc = 0;
 	mr.mr_addr = inm->in6m_addr;
-	in6_clearscope(&mr.mr_addr);
 	if (!m_append(m, sizeof(struct mldv2_record), (void *)&mr)) {
 		if (m != m0)
 			m_freem(m);
@@ -2752,7 +2670,6 @@ mld_v2_enqueue_filter_change(struct ifqu
 			 */
 			memset(&mr, 0, sizeof(mr));
 			mr.mr_addr = inm->in6m_addr;
-			in6_clearscope(&mr.mr_addr);
 			if (!m_append(m, sizeof(mr), (void *)&mr)) {
 				if (m != m0)
 					m_freem(m);
@@ -3051,7 +2968,6 @@ mld_dispatch_packet(struct mbuf *m)
 	struct ifnet		*oifp;
 	struct mbuf		*m0;
 	struct mbuf		*md;
-	struct ip6_hdr		*ip6;
 	struct mld_hdr		*mld;
 	int			 error;
 	int			 off;
@@ -3101,18 +3017,6 @@ mld_dispatch_packet(struct mbuf *m)
 	m_clrprotoflags(m);
 	m0->m_pkthdr.rcvif = V_loif;
 
-	ip6 = mtod(m0, struct ip6_hdr *);
-#if 0
-	(void)in6_setscope(&ip6->ip6_dst, ifp, NULL);	/* XXX LOR */
-#else
-	/*
-	 * XXX XXX Break some KPI rules to prevent an LOR which would
-	 * occur if we called in6_setscope() at transmission.
-	 * See comments at top of file.
-	 */
-	MLD_EMBEDSCOPE(&ip6->ip6_dst, ifp->if_index);
-#endif
-
 	/*
 	 * Retrieve the ICMPv6 type before handoff to ip6_output(),
 	 * so we can bump the stats.


More information about the svn-src-user mailing list