TSO panic

[Andre Oppermann]

On 02.09.2010 00:11, ben wilber wrote:
> On Sep 1, 2010, at 8:57 AM, Andre Oppermann wrote:
>
>> On 01.09.2010 01:13, ben wilber wrote:
>>> Hi,
>>>
>>> I just upgraded from r210042 to r212073 and keep getting the panic
>>> introduced in r211317:
>>>
>>> panic: tcp_output: len<= tso_segsz
>>
>> Please try the attached patch and report back whether it
>> fixes the issue.
>
> The system ran for 8 hours or so before I received the same panic.  Previously, it would panic within 20 minutes.

Attached is an updated patch that should fix the panic.
Please try.

-- 
Andre
-------------- next part --------------
Index: netinet/tcp_output.c
===================================================================
--- netinet/tcp_output.c	(revision 212160)
+++ netinet/tcp_output.c	(working copy)
@@ -466,9 +466,8 @@
 	}
 
 	/*
-	 * Truncate to the maximum segment length or enable TCP Segmentation
-	 * Offloading (if supported by hardware) and ensure that FIN is removed
-	 * if the length no longer contains the last data byte.
+	 * Decide if we can use TCP Segmentation Offloading (if supported by
+	 * hardware).
 	 *
 	 * TSO may only be used if we are in a pure bulk sending state.  The
 	 * presence of TCP-MD5, SACK retransmits, SACK advertizements and
@@ -476,10 +475,6 @@
 	 * (except for the sequence number) for all generated packets.  This
 	 * makes it impossible to transmit any options which vary per generated
 	 * segment or packet.
-	 *
-	 * The length of TSO bursts is limited to TCP_MAXWIN.  That limit and
-	 * removal of FIN (if not already catched here) are handled later after
-	 * the exact length of the TCP options are known.
 	 */
 #ifdef IPSEC
 	/*
@@ -488,22 +483,15 @@
 	 */
 	ipsec_optlen = ipsec_hdrsiz_tcp(tp);
 #endif
-	if (len > tp->t_maxseg) {
-		if ((tp->t_flags & TF_TSO) && V_tcp_do_tso &&
-		    ((tp->t_flags & TF_SIGNATURE) == 0) &&
-		    tp->rcv_numsacks == 0 && sack_rxmit == 0 &&
-		    tp->t_inpcb->inp_options == NULL &&
-		    tp->t_inpcb->in6p_options == NULL
+	if ((tp->t_flags & TF_TSO) && V_tcp_do_tso && len > tp->t_maxseg &&
+	    ((tp->t_flags & TF_SIGNATURE) == 0) &&
+	    tp->rcv_numsacks == 0 && sack_rxmit == 0 &&
 #ifdef IPSEC
-		    && ipsec_optlen == 0
+	    ipsec_optlen == 0 &&
 #endif
-		    ) {
-			tso = 1;
-		} else {
-			len = tp->t_maxseg;
-			sendalot = 1;
-		}
-	}
+	    tp->t_inpcb->inp_options == NULL &&
+	    tp->t_inpcb->in6p_options == NULL)
+		tso = 1;
 
 	if (sack_rxmit) {
 		if (SEQ_LT(p->rxmit + len, tp->snd_una + so->so_snd.sb_cc))
@@ -733,38 +747,63 @@
 	 * bump the packet length beyond the t_maxopd length.
 	 * Clear the FIN bit because we cut off the tail of
 	 * the segment.
-	 *
-	 * When doing TSO limit a burst to TCP_MAXWIN minus the
-	 * IP, TCP and Options length to keep ip->ip_len from
-	 * overflowing.  Prevent the last segment from being
-	 * fractional thus making them all equal sized and set
-	 * the flag to continue sending.  TSO is disabled when
-	 * IP options or IPSEC are present.
 	 */
 	if (len + optlen + ipoptlen > tp->t_maxopd) {
 		flags &= ~TH_FIN;
+
+		/*
+		 * TSO is disabled when IP options or IPSEC are present.
+		 */
 		if (tso) {
-			if (len > TCP_MAXWIN - hdrlen - optlen) {
-				len = TCP_MAXWIN - hdrlen - optlen;
-				len = len - (len % (tp->t_maxopd - optlen));
+			KASSERT(ipoptlen == 0,
+			    ("%s: TSO can't do IP options", __func__));
+
+			/*
+			 * When doing TSO limit a burst to IP_MAXPACKET
+			 * IP, TCP and Options length to keep ip->ip_len
+			 * from overflowing.
+			 */
+			if (len > IP_MAXPACKET - hdrlen) {
+				len = IP_MAXPACKET - hdrlen;
 				sendalot = 1;
-			} else if (tp->t_flags & TF_NEEDFIN)
+			}
+
+			/*
+			 * Prevent the last segment from being
+			 * fractional unless there is no further
+			 * data and the send sockbuf can be emptied.
+			 */
+			if (sendalot && off + len < so->so_snd.sb_cc) {
+				len -= len % (tp->t_maxopd - optlen);
 				sendalot = 1;
+			}
+
+			/*
+			 * Send the FIN in a separate segment
+			 * after the bulk sending is done.
+			 * We don't trust the TSO implementations
+			 * to clear the FIN flag on all but the
+			 * last segment.
+			 */
+			if (tp->t_flags & TF_NEEDFIN)
+				sendalot = 1;
+
 		} else {
 			len = tp->t_maxopd - optlen - ipoptlen;
 			sendalot = 1;
 		}
-	}
+	} else
+		tso = 0;
 
-/*#ifdef DIAGNOSTIC*/
+	KASSERT(len + hdrlen + ipoptlen <= IP_MAXPACKET,
+	    ("%s: len too big", __func__));
 #ifdef INET6
-	if (max_linkhdr + hdrlen > MCLBYTES)
+	KASSERT(max_linkhdr + hdrlen < MCLBYTES,
+	    ("%s: ", __func__));
 #else
-	if (max_linkhdr + hdrlen > MHLEN)
+	KASSERT(max_linkhdr + hdrlen < MHLEN,
+	    ("%s: ", __func__))
 #endif
-		panic("tcphdr too big");
-/*#endif*/
-
 	/*
 	 * This KASSERT is here to catch edge cases at a well defined place.
 	 * Before, those had triggered (random) panic conditions further down.
@@ -1062,6 +1106,9 @@
 	 * The TCP pseudo header checksum is always provided.
 	 * XXX: Fixme: This is currently not the case for IPv6.
 	 */
+	KASSERT(len + hdrlen + ipoptlen == m_length(m, NULL),
+	    ("%s: mbuf chain shorter than expected", __func__));
+
 	if (tso) {
 		KASSERT(len > tp->t_maxopd - optlen,
 		    ("%s: len <= tso_segsz", __func__));