6783391 multirt loop in ip_newroute() may break prematurely

5 files changed, 90 insertions, 32 deletions

diff --git a/usr/src/uts/common/inet/ip/ip.c b/usr/src/uts/common/inet/ip/ip.c
index 43fb5548a0..b08f136399 100644
--- a/usr/src/uts/common/inet/ip/ip.c
+++ b/usr/src/uts/common/inet/ip/ip.c
@@ -7937,7 +7937,6 @@ ip_newroute(queue_t *q, mblk_t *mp, ipaddr_t dst, conn_t *connp,
 	boolean_t mctl_present;
 	ipsec_out_t *io;
 	mblk_t	*saved_mp;
-	ire_t	*first_sire = NULL;
 	mblk_t	*copy_mp = NULL;
 	mblk_t	*xmit_mp = NULL;
 	ipaddr_t save_dst;
@@ -7950,6 +7949,10 @@ ip_newroute(queue_t *q, mblk_t *mp, ipaddr_t dst, conn_t *connp,
 	tsol_ire_gw_secattr_t *attrp = NULL;
 	tsol_gcgrp_t *gcgrp = NULL;
 	tsol_gcgrp_addr_t ga;
+	int multirt_res_failures = 0;
+	int multirt_res_attempts = 0;
+	int multirt_already_resolved = 0;
+	boolean_t multirt_no_icmp_error = B_FALSE;
 
 	if (ip_debug > 2) {
 		/* ip1dbg */
@@ -8100,12 +8103,14 @@ ip_newroute(queue_t *q, mblk_t *mp, ipaddr_t dst, conn_t *connp,
 			ASSERT(sire != NULL);
 			multirt_is_resolvable =
 			    ire_multirt_lookup(&ire, &sire, multirt_flags,
-			    msg_getlabel(mp), ipst);
+			    &multirt_already_resolved, msg_getlabel(mp), ipst);
 
 			ip3dbg(("ip_newroute: multirt_is_resolvable %d, "
-			    "ire %p, sire %p\n",
-			    multirt_is_resolvable,
-			    (void *)ire, (void *)sire));
+			    "multirt_already_resolved %d, "
+			    "multirt_res_attempts %d, multirt_res_failures %d, "
+			    "ire %p, sire %p\n", multirt_is_resolvable,
+			    multirt_already_resolved, multirt_res_attempts,
+			    multirt_res_failures, (void *)ire, (void *)sire));
 
 			if (!multirt_is_resolvable) {
 				/*
@@ -8117,23 +8122,37 @@ ip_newroute(queue_t *q, mblk_t *mp, ipaddr_t dst, conn_t *connp,
 					ire_refrele(ire);
 					ire = NULL;
 				}
+				/*
+				 * Generate ICMP error only if all attempts to
+				 * resolve multirt route failed and there is no
+				 * already resolved one.  Don't generate ICMP
+				 * error when:
+				 *
+				 *  1) there was no attempt to resolve
+				 *  2) at least one attempt passed
+				 *  3) a multirt route is already resolved
+				 *
+				 *  Case 1) may occur due to multiple
+				 *    resolution attempts during single
+				 *    ip_multirt_resolution_interval.
+				 *
+				 *  Case 2-3) means that CGTP destination is
+				 *    reachable via one link so we don't want to
+				 *    generate ICMP host unreachable error.
+				 */
+				if (multirt_res_attempts == 0 ||
+				    multirt_res_failures <
+				    multirt_res_attempts ||
+				    multirt_already_resolved > 0)
+					multirt_no_icmp_error = B_TRUE;
 			} else {
 				ASSERT(sire != NULL);
 				ASSERT(ire != NULL);
-				/*
-				 * We simply use first_sire as a flag that
-				 * indicates if a resolvable multirt route
-				 * has already been found.
-				 * If it is not the case, we may have to send
-				 * an ICMP error to report that the
-				 * destination is unreachable.
-				 * We do not IRE_REFHOLD first_sire.
-				 */
-				if (first_sire == NULL) {
-					first_sire = sire;
-				}
+
+				multirt_res_attempts++;
 			}
 		}
+
 		if (ire == NULL) {
 			if (ip_debug > 3) {
 				/* ip2dbg */
@@ -8141,21 +8160,17 @@ ip_newroute(queue_t *q, mblk_t *mp, ipaddr_t dst, conn_t *connp,
 				    "can't resolve %s\n", AF_INET, &dst);
 			}
 			ip3dbg(("ip_newroute: "
-			    "ire %p, sire %p, first_sire %p\n",
-			    (void *)ire, (void *)sire, (void *)first_sire));
+			    "ire %p, sire %p, multirt_no_icmp_error %d\n",
+			    (void *)ire, (void *)sire,
+			    (int)multirt_no_icmp_error));
 
 			if (sire != NULL) {
 				ire_refrele(sire);
 				sire = NULL;
 			}
 
-			if (first_sire != NULL) {
-				/*
-				 * At least one multirt route has been found
-				 * in the same call to ip_newroute();
-				 * there is no need to report an ICMP error.
-				 * first_sire was not IRE_REFHOLDed.
-				 */
+			if (multirt_no_icmp_error) {
+				/* There is no need to report an ICMP error. */
 				MULTIRT_DEBUG_UNTAG(first_mp);
 				freemsg(first_mp);
 				return;
@@ -8285,6 +8300,23 @@ ip_newroute(queue_t *q, mblk_t *mp, ipaddr_t dst, conn_t *connp,
 				src_ipif = ipif_select_source(dst_ill, saddr,
 				    zoneid);
 				if (src_ipif == NULL) {
+					/*
+					 * In the case of multirouting, it may
+					 * happen that ipif_select_source fails
+					 * as DAD may disallow use of the
+					 * particular source interface.  Anyway,
+					 * we need to continue and attempt to
+					 * resolve other multirt routes.
+					 */
+					if ((sire != NULL) &&
+					    (sire->ire_flags & RTF_MULTIRT)) {
+						ire_refrele(ire);
+						ire = NULL;
+						multirt_resolve_next = B_TRUE;
+						multirt_res_failures++;
+						continue;
+					}
+
 					if (ip_debug > 2) {
 						pr_addr_dbg("ip_newroute: "
 						    "no src for dst %s ",
@@ -8731,7 +8763,25 @@ ip_newroute(queue_t *q, mblk_t *mp, ipaddr_t dst, conn_t *connp,
 					ipif_refrele(src_ipif);
 					src_ipif = ipif_select_source(dst_ill,
 					    gw, zoneid);
+					/*
+					 * In the case of multirouting, it may
+					 * happen that ipif_select_source fails
+					 * as DAD may disallow use of the
+					 * particular source interface.  Anyway,
+					 * we need to continue and attempt to
+					 * resolve other multirt routes.
+					 */
 					if (src_ipif == NULL) {
+						if (sire != NULL &&
+						    (sire->ire_flags &
+						    RTF_MULTIRT)) {
+							ire_refrele(ire);
+							ire = NULL;
+							multirt_resolve_next =
+							    B_TRUE;
+							multirt_res_failures++;
+							continue;
+						}
 						if (ip_debug > 2) {
 							pr_addr_dbg(
 							    "ip_newroute: no "
diff --git a/usr/src/uts/common/inet/ip/ip6_ire.c b/usr/src/uts/common/inet/ip/ip6_ire.c
index 0d0f3621f5..c13a66fcc2 100644
--- a/usr/src/uts/common/inet/ip/ip6_ire.c
+++ b/usr/src/uts/common/inet/ip/ip6_ire.c
@@ -246,7 +246,7 @@ ire_lookup_multi_v6(const in6_addr_t *group, zoneid_t zoneid, ip_stack_t *ipst)
 		ire_t *cire = NULL;
 		/*
 		 * If the route is not resolvable, the looked up ire
-		 * may be changed here. In that case, ire_multirt_lookup()
+		 * may be changed here. In that case, ire_multirt_lookup_v6()
 		 * IRE_REFRELE the original ire and change it.
 		 */
 		(void) ire_multirt_lookup_v6(&cire, &ire, MULTIRT_CACHEGW,
diff --git a/usr/src/uts/common/inet/ip/ip_ftable.c b/usr/src/uts/common/inet/ip/ip_ftable.c
index 77da02268d..756368518b 100644
--- a/usr/src/uts/common/inet/ip/ip_ftable.c
+++ b/usr/src/uts/common/inet/ip/ip_ftable.c
@@ -652,7 +652,7 @@ ire_lookup_multi(ipaddr_t group, zoneid_t zoneid, ip_stack_t *ipst)
 		 * may be changed here. In that case, ire_multirt_lookup()
 		 * IRE_REFRELE the original ire and change it.
 		 */
-		(void) ire_multirt_lookup(&cire, &ire, MULTIRT_CACHEGW,
+		(void) ire_multirt_lookup(&cire, &ire, MULTIRT_CACHEGW, NULL,
 		    NULL, ipst);
 		if (cire != NULL)
 			ire_refrele(cire);
diff --git a/usr/src/uts/common/inet/ip/ip_ire.c b/usr/src/uts/common/inet/ip/ip_ire.c
index 4610d3015c..ba1c2015f3 100644
--- a/usr/src/uts/common/inet/ip/ip_ire.c
+++ b/usr/src/uts/common/inet/ip/ip_ire.c
@@ -4731,7 +4731,7 @@ ire_multirt_need_resolve(ipaddr_t dst, const ts_label_t *tsl, ip_stack_t *ipst)
 	/* At least one route is unresolved; search for a resolvable route. */
 	if (unres_cnt > 0)
 		resolvable = ire_multirt_lookup(&first_cire, &first_fire,
-		    MULTIRT_USESTAMP | MULTIRT_CACHEGW, tsl, ipst);
+		    MULTIRT_USESTAMP | MULTIRT_CACHEGW, NULL, tsl, ipst);
 
 	if (first_fire != NULL)
 		ire_refrele(first_fire);
@@ -4782,7 +4782,7 @@ ire_multirt_need_resolve(ipaddr_t dst, const ts_label_t *tsl, ip_stack_t *ipst)
  * - if MULTIRT_USESTAMP is not specified in flags, the first
  *   unresolved but resolvable route is selected.
  *
- * - Otherwise, there is no resolvalble route, and
+ * - Otherwise, there is no resolvable route, and
  *   B_FALSE is returned.
  *
  * At last, MULTIRT_SETSTAMP can be specified in flags to
@@ -4790,11 +4790,14 @@ ire_multirt_need_resolve(ipaddr_t dst, const ts_label_t *tsl, ip_stack_t *ipst)
  * be refreshed. This prevents the useless exploration
  * of those routes for a while, when MULTIRT_USESTAMP is used.
  *
+ * The argument already_resolved_count is an output variable to track number
+ * of already resolved multirt routes.
+ *
  * This only works in the global zone.
  */
 boolean_t
 ire_multirt_lookup(ire_t **ire_arg, ire_t **fire_arg, uint32_t flags,
-    const ts_label_t *tsl, ip_stack_t *ipst)
+    int *already_resolved_count, const ts_label_t *tsl, ip_stack_t *ipst)
 {
 	clock_t	delta;
 	ire_t	*best_fire = NULL;
@@ -4921,6 +4924,9 @@ ire_multirt_lookup(ire_t **ire_arg, ire_t **fire_arg, uint32_t flags,
 			if (already_resolved) {
 				ip2dbg(("ire_multirt_lookup: found cire %p, "
 				    "already resolved\n", (void *)cire));
+
+				if (already_resolved_count != NULL)
+					(*already_resolved_count)++;
 				continue;
 			}
 
@@ -5128,6 +5134,8 @@ ire_multirt_lookup(ire_t **ire_arg, ire_t **fire_arg, uint32_t flags,
 			 */
 			if (already_resolved) {
 				ire_refrele(gw_ire);
+				if (already_resolved_count != NULL)
+					(*already_resolved_count)++;
 				continue;
 			}
 
diff --git a/usr/src/uts/common/inet/ip_ire.h b/usr/src/uts/common/inet/ip_ire.h
index 0a9f8add85..f4882f7640 100644
--- a/usr/src/uts/common/inet/ip_ire.h
+++ b/usr/src/uts/common/inet/ip_ire.h
@@ -325,7 +325,7 @@ extern  void	ire_walk_ill_tables(uint_t match_flags, uint_t ire_type,
     zoneid_t zoneid, ip_stack_t *);
 extern	void	ire_walk_v6(pfv_t, void *, zoneid_t, ip_stack_t *);
 
-extern boolean_t	ire_multirt_lookup(ire_t **, ire_t **, uint32_t,
+extern boolean_t	ire_multirt_lookup(ire_t **, ire_t **, uint32_t, int *,
     const struct ts_label_s *, ip_stack_t *);
 extern boolean_t	ire_multirt_need_resolve(ipaddr_t,
     const struct ts_label_s *, ip_stack_t *);