Date: Mon, 17 Oct 2011 18:09:49 +0200 From: Andre Oppermann <andre@freebsd.org> To: Ben Hutchings <bhutchings@solarflare.com> Cc: freebsd-net@freebsd.org Subject: Re: TSO broken with jumbo MTU Message-ID: <4E9C534D.4090405@freebsd.org> In-Reply-To: <1318865394.2784.4.camel@bwh-desktop> References: <1317309906.2743.9.camel@bwh-desktop> <1318865394.2784.4.camel@bwh-desktop>
next in thread | previous in thread | raw e-mail | index | archive | help
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.
>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4E9C534D.4090405>