Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 03 Jul 2007 18:18:50 +0100
From:      "Bruce M. Simpson" <bms@FreeBSD.org>
To:        Ian FREISLICH <ianf@clue.co.za>
Cc:        current@freebsd.org
Subject:   Re: Multicast problems [PATCH]
Message-ID:  <468A84FA.2090703@FreeBSD.org>
In-Reply-To: <468A5B9D.6030401@FreeBSD.org>
References:  <E1I5iuL-0006kH-6s@clue.co.za> <468A5B9D.6030401@FreeBSD.org>

next in thread | previous in thread | raw e-mail | index | archive | help
This is a multi-part message in MIME format.
--------------060206070705040907050507
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit

Bruce M. Simpson wrote:
>
> I see now that Linux also supports ip_mreqn in its IP_ADD_MEMBERSHIP 
> path. I could potentially change the ASM API ioctl paths 
> (IP_ADD_MEMBERSHIP, IP_DEL_MEMBERSHIP) to detect and support the 
> ip_mreqn structure -- however -- I am loathe to do this as it 
> introduces another bunch of nested conditionals, as the same code now 
> has to support IP_ADD_SOURCE_MEMBERSHIP in FreeBSD, which has the same 
> structure size. It is also a retrograde change. 

I have attached a diff which emulates the Linux ip_mreqn kludge in the 
IP_ADD_MEMBERSHIP and IP_DROP_MEMBERSHIP paths; this includes the 
changes from the previous patch to workaround the non-existence of a 
default route on boot.

I do not plan to commit it at the moment and have not tested it. The 
right thing for applications to do is to use the RFC 3678 API if they 
need to join an interface by index, the legacy ASM API can only be 
relied upon if interfaces are assigned IPv4 addresses, and it breaks for 
point-to-point because of legacy BSD behaviour.

regards,
BMS

--------------060206070705040907050507
Content-Type: text/x-patch;
 name="add_drop_mreqn.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="add_drop_mreqn.diff"

Index: in_mcast.c
===================================================================
RCS file: /home/ncvs/src/sys/netinet/in_mcast.c,v
retrieving revision 1.1
diff -u -p -r1.1 in_mcast.c
--- in_mcast.c	12 Jun 2007 16:24:53 -0000	1.1
+++ in_mcast.c	3 Jul 2007 17:10:01 -0000
@@ -967,11 +967,40 @@ inp_join_group(struct inpcb *inp, struct
 		struct ip_mreq_source	 mreqs;
 
 		if (sopt->sopt_name == IP_ADD_MEMBERSHIP) {
-			error = sooptcopyin(sopt, &mreqs,
-			    sizeof(struct ip_mreq),
-			    sizeof(struct ip_mreq));
 			/*
-			 * Do argument switcharoo from ip_mreq into
+			 * Handle the case where a struct ip_mreqn
+			 * was passed to the IP_ADD_MEMBERSHIP ioctl to
+			 * specify an interface index. This is an
+			 * undocumented modification of the IPv4 ASM API
+			 * obtained from Linux.
+			 */
+			if (sopt->sopt_valsize == sizeof(struct ip_mreqn)) {
+				struct ip_mreqn *mreqn =
+				    (struct ip_mreqn *)&mreqs;
+
+				error = sooptcopyin(sopt, mreqn,
+				    sizeof(struct ip_mreqn),
+				    sizeof(struct ip_mreqn));
+				if (error)
+					return (error);
+
+				if (mreqn->imr_ifindex < 0 ||
+				    if_index < mreqn->imr_ifindex)
+					return (EINVAL);
+				if (mreqn->imr_ifindex == 0) {
+					ifp = NULL;
+				} else {
+					ifp = ifnet_byindex(mreqn->imr_ifindex);
+					if (ifp == NULL)
+						return (EADDRNOTAVAIL);
+				}
+			} else {
+				error = sooptcopyin(sopt, &mreqs,
+				    sizeof(struct ip_mreq),
+				    sizeof(struct ip_mreq));
+			}
+			/*
+			 * Do argument switcharoo from ip_mreq[n] into
 			 * ip_mreq_source to avoid using two instances.
 			 */
 			mreqs.imr_interface = mreqs.imr_sourceaddr;
@@ -995,29 +1024,48 @@ inp_join_group(struct inpcb *inp, struct
 		}
 
 		/*
-		 * Obtain ifp. If no interface address was provided,
-		 * use the interface of the route to the given multicast
-		 * address (usually this is the default route).
+		 * If ifp was not already overridden, obtain it.
+		 *
+		 * If no interface address was provided, use the interface
+		 * of the route in the unicast FIB for the given multicast
+		 * destination; usually, this is the default route.
+		 *
+		 * If this lookup fails, attempt to use the first non-loopback
+		 * interface with multicast capability in the system as a
+		 * last resort. The legacy IPv4 ASM API requires that we do
+		 * this in order to allow groups to be joined when the routing
+		 * table has not yet been populated during boot.
+		 *
+		 * If all of these conditions fail, return EADDRNOTAVAIL, and
+		 * reject the IPv4 multicast join.
 		 */
-		if (mreqs.imr_interface.s_addr != INADDR_ANY) {
+		if (ifp == NULL && mreqs.imr_interface.s_addr != INADDR_ANY) {
 			INADDR_TO_IFP(mreqs.imr_interface, ifp);
-		} else {
+		} else if (ifp == NULL) {
 			struct route ro;
 
 			ro.ro_rt = NULL;
 			*(struct sockaddr_in *)&ro.ro_dst = gsa->sin;
 			rtalloc_ign(&ro, RTF_CLONING);
-			if (ro.ro_rt == NULL) {
-#ifdef DIAGNOSTIC
-				printf("%s: no route to %s\n", __func__,
-				    inet_ntoa(gsa->sin.sin_addr));
-#endif
-				return (EADDRNOTAVAIL);
+			if (ro.ro_rt != NULL) {
+				ifp = ro.ro_rt->rt_ifp;
+				KASSERT(ifp != NULL, ("%s: null ifp",
+				    __func__));
+				RTFREE(ro.ro_rt);
+			} else {
+				struct in_ifaddr *ia;
+				struct ifnet *mfp = NULL;
+				TAILQ_FOREACH(ia, &in_ifaddrhead, ia_link) {
+					mfp = ia->ia_ifp;
+					if (!(mfp->if_flags & IFF_LOOPBACK) &&
+					     (mfp->if_flags & IFF_MULTICAST)) {
+						ifp = mfp;
+						break;
+					}
+				}
 			}
-			ifp = ro.ro_rt->rt_ifp;
-			KASSERT(ifp != NULL, ("%s: null ifp", __func__));
-			RTFREE(ro.ro_rt);
 		}
+
 #ifdef DIAGNOSTIC
 		if (bootverbose) {
 			printf("%s: imr_interface = %s, ifp = %p\n",
@@ -1207,12 +1255,42 @@ inp_leave_group(struct inpcb *inp, struc
 	case IP_DROP_MEMBERSHIP:
 	case IP_DROP_SOURCE_MEMBERSHIP:
 		if (sopt->sopt_name == IP_DROP_MEMBERSHIP) {
-			error = sooptcopyin(sopt, &mreqs,
-			    sizeof(struct ip_mreq),
-			    sizeof(struct ip_mreq));
+			/*
+			 * Handle the case where a struct ip_mreqn
+			 * was passed to the IP_DROP_MEMBERSHIP ioctl to
+			 * specify an interface index. This is an
+			 * undocumented modification of the IPv4 ASM API
+			 * obtained from Linux.
+			 */
+			if (sopt->sopt_valsize == sizeof(struct ip_mreqn)) {
+				struct ip_mreqn *mreqn =
+				    (struct ip_mreqn *)&mreqs;
+
+				error = sooptcopyin(sopt, mreqn,
+				    sizeof(struct ip_mreqn),
+				    sizeof(struct ip_mreqn));
+				if (error)
+					return (error);
+
+				if (mreqn->imr_ifindex < 0 ||
+				    if_index < mreqn->imr_ifindex)
+					return (EINVAL);
+				if (mreqn->imr_ifindex == 0) {
+					ifp = NULL;
+				} else {
+					ifp = ifnet_byindex(mreqn->imr_ifindex);
+					if (ifp == NULL)
+						return (EADDRNOTAVAIL);
+				}
+			} else {
+				error = sooptcopyin(sopt, &mreqs,
+				    sizeof(struct ip_mreq),
+				    sizeof(struct ip_mreq));
+			}
+
 			/*
 			 * Swap interface and sourceaddr arguments,
-			 * as ip_mreq and ip_mreq_source are laid
+			 * as ip_mreq[n] and ip_mreq_source are laid
 			 * out differently.
 			 */
 			mreqs.imr_interface = mreqs.imr_sourceaddr;
@@ -1235,7 +1313,7 @@ inp_leave_group(struct inpcb *inp, struc
 			ssa->sin.sin_addr = mreqs.imr_sourceaddr;
 		}
 
-		if (gsa->sin.sin_addr.s_addr != INADDR_ANY)
+		if (ifp == NULL && gsa->sin.sin_addr.s_addr != INADDR_ANY)
 			INADDR_TO_IFP(mreqs.imr_interface, ifp);
 
 #ifdef DIAGNOSTIC

--------------060206070705040907050507--



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