Date: Fri, 03 Sep 2010 10:10:33 +0200 From: Andre Oppermann <andre@freebsd.org> To: ben wilber <ben@desync.com> Cc: FreeBSD Current <freebsd-current@freebsd.org> Subject: Re: TSO panic Message-ID: <4C80AD79.8070307@freebsd.org> In-Reply-To: <137CFFE2-6783-449E-A2EC-8415F9287D97@desync.com> References: <20100831231314.GA9637@exodus.desync.com> <4C7E4DC0.5040608@freebsd.org> <137CFFE2-6783-449E-A2EC-8415F9287D97@desync.com>
next in thread | previous in thread | raw e-mail | index | archive | help
This is a multi-part message in MIME format.
--------------030903040105020300020909
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
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
--------------030903040105020300020909
Content-Type: text/plain;
name="tcp_tso-fix.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename="tcp_tso-fix.diff"
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__));
--------------030903040105020300020909--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4C80AD79.8070307>