changeset 12570:4be1b7cb5db7

6952458 SIOCSXARP erroneously requires IP interface to be NUL-terminated 6955756 ifconfig <intf> down can hang due to pending ill_ire_cnt
author Sowmini Varadhan <Sowmini.Varadhan@Sun.COM>
date Mon, 07 Jun 2010 10:10:19 -0400
parents 06d420824c4b
children 05943d9c379f
files usr/src/uts/common/inet/ip.h usr/src/uts/common/inet/ip/ip_if.c usr/src/uts/common/inet/ip/ip_ire.c
diffstat 3 files changed, 49 insertions(+), 26 deletions(-) [+]
line wrap: on
line diff
--- a/usr/src/uts/common/inet/ip.h	Mon Jun 07 13:16:48 2010 +0200
+++ b/usr/src/uts/common/inet/ip.h	Mon Jun 07 10:10:19 2010 -0400
@@ -1429,10 +1429,13 @@
 #define	ILL_LL_SUBNET_PENDING	0x01	/* Waiting for DL_INFO_ACK from drv */
 #define	ILL_CONDEMNED		0x02	/* No more new ref's to the ILL */
 #define	ILL_DL_UNBIND_IN_PROGRESS	0x04	/* UNBIND_REQ is sent */
-#define	ILL_DOWN_IN_PROGRESS	0x08	/* ILL is going down - no new nce's */
-#define	ILL_LL_BIND_PENDING	0x0020	/* XXX Reuse ILL_LL_SUBNET_PENDING ? */
-#define	ILL_LL_UP		0x0040
-#define	ILL_LL_DOWN		0x0080
+/*
+ * ILL_DOWN_IN_PROGRESS is set to ensure the following:
+ * - no packets are sent to the driver after the DL_UNBIND_REQ is sent,
+ * - no longstanding references will be acquired on objects that are being
+ *   brought down.
+ */
+#define	ILL_DOWN_IN_PROGRESS	0x08
 
 /* Is this an ILL whose source address is used by other ILL's ? */
 #define	IS_USESRC_ILL(ill)			\
--- a/usr/src/uts/common/inet/ip/ip_if.c	Mon Jun 07 13:16:48 2010 +0200
+++ b/usr/src/uts/common/inet/ip/ip_if.c	Mon Jun 07 10:10:19 2010 -0400
@@ -1117,11 +1117,6 @@
 	ipif_t	*ipif;
 
 	ASSERT(IAM_WRITER_ILL(ill));
-	mutex_enter(&ill->ill_lock);
-	ill->ill_state_flags |= ILL_DOWN_IN_PROGRESS;
-	/* no more nce addition allowed */
-	mutex_exit(&ill->ill_lock);
-
 	/*
 	 * It is possible that some ioctl is already in progress while we
 	 * received the M_ERROR / M_HANGUP in which case, we need to abort
@@ -13295,6 +13290,22 @@
 	}
 
 	/*
+	 * Removal of the last ipif from an ill may result in a DL_UNBIND
+	 * being sent to the driver, and we must not send any data packets to
+	 * the driver after the DL_UNBIND_REQ. To ensure this, all the
+	 * ire and nce entries used in the data path will be cleaned
+	 * up, and we also set  the ILL_DOWN_IN_PROGRESS bit to make
+	 * sure on new entries will be added until the ill is bound
+	 * again. The ILL_DOWN_IN_PROGRESS bit is turned off upon
+	 * receipt of a DL_BIND_ACK.
+	 */
+	if (ill->ill_wq != NULL && !ill->ill_logical_down &&
+	    ill->ill_ipif_up_count == 0 && ill->ill_ipif_dup_count == 0 &&
+	    ill->ill_dl_up) {
+		ill->ill_state_flags |= ILL_DOWN_IN_PROGRESS;
+	}
+
+	/*
 	 * Blow away memberships we established in ipif_multicast_up().
 	 */
 	ipif_multicast_down(ipif);
@@ -13599,6 +13610,7 @@
 	ipif_t	*ipif;
 	uint_t	ire_type;
 	boolean_t did_alloc = B_FALSE;
+	char	last;
 
 	/*
 	 * If the caller wants to us to create the ipif, make sure we have a
@@ -13639,9 +13651,9 @@
 
 	if (cp <= name) {
 		cp = endp;
-	} else {
-		*cp = '\0';
-	}
+	}
+	last = *cp;
+	*cp = '\0';
 
 	/*
 	 * Look up the ILL, based on the portion of the name
@@ -13651,8 +13663,7 @@
 	 */
 	ill = ill_lookup_on_name(name, do_alloc, isv6,
 	    &did_alloc, ipst);
-	if (cp != endp)
-		*cp = IPIF_SEPARATOR_CHAR;
+	*cp = last;
 	if (ill == NULL)
 		return (NULL);
 
@@ -17733,9 +17744,16 @@
 	}
 
 	ipsq_current_start(ipsq, ill->ill_ipif, 0);
+
+	/*
+	 * Since we'll only do a logical down, we can't rely on ipif_down
+	 * to turn on ILL_DOWN_IN_PROGRESS, or for the DL_BIND_ACK to reset
+	 * ILL_DOWN_IN_PROGRESS. We instead manage this separately for this
+	 * case, to quiesce ire's and nce's for ill_is_quiescent.
+	 */
 	mutex_enter(&ill->ill_lock);
 	ill->ill_state_flags |= ILL_DOWN_IN_PROGRESS;
-	/* no more nce addition allowed */
+	/* no more ire/nce addition allowed */
 	mutex_exit(&ill->ill_lock);
 
 	/*
@@ -17817,16 +17835,19 @@
 	}
 
 	/*
+	 * reset ILL_DOWN_IN_PROGRESS so that we can successfully add ires
+	 * as we bring the ipifs up again.
+	 */
+	mutex_enter(&ill->ill_lock);
+	ill->ill_state_flags &= ~ILL_DOWN_IN_PROGRESS;
+	mutex_exit(&ill->ill_lock);
+	/*
 	 * If there are ipifs to bring up, ill_up_ipifs() will return
 	 * EINPROGRESS, and ipsq_current_finish() will be called by
 	 * ip_rput_dlpi_writer() or arp_bringup_done() when the last ipif is
 	 * brought up.
 	 */
 	status = ill_up_ipifs(ill, q, addrmp);
-	mutex_enter(&ill->ill_lock);
-	if (ill->ill_dl_up)
-		ill->ill_state_flags &= ~ILL_DOWN_IN_PROGRESS;
-	mutex_exit(&ill->ill_lock);
 	if (status != EINPROGRESS)
 		ipsq_current_finish(ipsq);
 }
@@ -17855,11 +17876,6 @@
 
 	ipsq_current_start(ipsq, ill->ill_ipif, 0);
 
-	mutex_enter(&ill->ill_lock);
-	ill->ill_state_flags |= ILL_DOWN_IN_PROGRESS;
-	/* no more nce addition allowed */
-	mutex_exit(&ill->ill_lock);
-
 	/*
 	 * If we can quiesce the ill, then continue.  If not, then
 	 * ill_replumb_tail() will be called from ipif_ill_refrele_tail().
--- a/usr/src/uts/common/inet/ip/ip_ire.c	Mon Jun 07 13:16:48 2010 +0200
+++ b/usr/src/uts/common/inet/ip/ip_ire.c	Mon Jun 07 10:10:19 2010 -0400
@@ -1117,10 +1117,14 @@
 		mutex_enter(&ill->ill_lock);
 
 		/*
-		 * Don't allow IRE's to be created on dying ills.
+		 * Don't allow IRE's to be created on dying ills, or on
+		 * ill's for which the last ipif is going down, or ones which
+		 * don't have even a single UP interface
 		 */
-		if (ill->ill_state_flags & ILL_CONDEMNED) {
+		if ((ill->ill_state_flags &
+		    (ILL_CONDEMNED|ILL_DOWN_IN_PROGRESS)) != 0) {
 			ire_atomic_end(irb_ptr, ire);
+			DTRACE_PROBE1(ire__add__on__dying__ill, ire_t *, ire);
 			return (ENXIO);
 		}