TSO broken with jumbo MTU

[Andre Oppermann]

On 17.10.2011 17:29, Ben Hutchings wrote:
> This is the fix/workaround I used:

Thanks for the fix.  I'll review it and put it into FreeBSD maybe in
a slightly different form.

-- 
Andre

> --- a/netinet/tcp_output.c
> +++ b/netinet/tcp_output.c
> @@ -1062,9 +1062,7 @@
>   	 * The TCP pseudo header checksum is always provided.
>   	 * XXX: Fixme: This is currently not the case for IPv6.
>   	 */
> -	if (tso) {
> -		KASSERT(len>  tp->t_maxopd - optlen,
> -		    ("%s: len<= tso_segsz", __func__));
> +	if (tso&&  len>  tp->t_maxopd - optlen) {
>   		m->m_pkthdr.csum_flags |= CSUM_TSO;
>   		m->m_pkthdr.tso_segsz = tp->t_maxopd - optlen;
>   	}
> --- END ---
>
> But the correct thing to do may be to change the calculation of t_maxopd
> (untested):
>
> --- a/netinet/tcp_input.c
> +++ b/netinet/tcp_input.c
> @@ -3087,7 +3087,7 @@
>   tcp_mss_update(struct tcpcb *tp, int offer,
>       struct hc_metrics_lite *metricptr, int *mtuflags)
>   {
> -	int mss;
> +	int mss, ts_len;
>   	u_long maxmtu;
>   	struct inpcb *inp = tp->t_inpcb;
>   	struct hc_metrics_lite metrics;
> @@ -3212,22 +3212,17 @@
>   	mss = max(mss, 64);
>
>   	/*
> -	 * maxopd stores the maximum length of data AND options
> -	 * in a segment; maxseg is the amount of data in a normal
> -	 * segment.  We need to store this value (maxopd) apart
> -	 * from maxseg, because now every segment carries options
> -	 * and thus we normally have somewhat less data in segments.
> -	 */
> -	tp->t_maxopd = mss;
> -
> -	/*
>   	 * origoffer==-1 indicates that no segments were received yet.
>   	 * In this case we just guess.
>   	 */
>   	if ((tp->t_flags&  (TF_REQ_TSTMP|TF_NOOPT)) == TF_REQ_TSTMP&&
>   	(origoffer == -1 ||
>   	     (tp->t_flags&  TF_RCVD_TSTMP) == TF_RCVD_TSTMP))
> -		mss -= TCPOLEN_TSTAMP_APPA;
> +		ts_len = TCPOLEN_TSTAMP_APPA;
> +	else
> +		ts_len = 0;
> +
> +	mss -= ts_len;
>
>   #if	(MCLBYTES&  (MCLBYTES - 1)) == 0
>   	if (mss>  MCLBYTES)
> @@ -3237,6 +3232,15 @@
>   		mss = mss / MCLBYTES * MCLBYTES;
>   #endif
>   	tp->t_maxseg = mss;
> +
> +	/*
> +	 * maxopd stores the maximum length of data AND options
> +	 * in a segment; maxseg is the amount of data in a normal
> +	 * segment.  We need to store this value (maxopd) apart
> +	 * from maxseg, because now every segment carries options
> +	 * and thus we normally have somewhat less data in segments.
> +	 */
> +	tp->t_maxopd = mss + ts_len;
>   }
>
>   void
> --- END ---
>
> Ben.
>