Small bug with TCP zero windows

[John Baldwin]

Several years ago Dillon added a feature to TCP that casued soreceive() to
send an ACK right away if data was drained from a TCP socket that had
previously advertised a zero-sized window.  The current code requires the
receive window to be exactly zero for this to kick in.  If window scaling is  
enabled and the window is smaller than the scale, then the effective window
that is advertised is zero.  However, in that case the zero-sized window
handling is not enabled because the window is not exactly zero.  The patch
below changes the code to check the raw window value against zero.  Arguably
it could check 'th_win' directly instead if folks would prefer that.

Index: tcp_output.c
===================================================================
--- tcp_output.c (revision 198794)
+++ tcp_output.c (working copy)
@@ -992,7 +992,7 @@
  * to read more data than can be buffered prior to transmitting on
  * the connection.
  */
- if (recwin == 0)
+ if (recwin >> tp->rcv_scale == 0)
  tp->t_flags |= TF_RXWIN0SENT;
  else
  tp->t_flags &= ~TF_RXWIN0SENT;

--
John Baldwin
_______________________________________________
freebsd-net@... mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscribe@..."

[Bjoern A. Zeeb]

On Tue, 3 Nov 2009, John Baldwin wrote:

> Several years ago Dillon added a feature to TCP that casued soreceive() to
> send an ACK right away if data was drained from a TCP socket that had
> previously advertised a zero-sized window.  The current code requires the
> receive window to be exactly zero for this to kick in.  If window scaling is
> enabled and the window is smaller than the scale, then the effective window
> that is advertised is zero.  However, in that case the zero-sized window
> handling is not enabled because the window is not exactly zero.  The patch
> below changes the code to check the raw window value against zero.  Arguably
> it could check 'th_win' directly instead if folks would prefer that.

hmm, looking a few lines up, there is a htons() there as well;
obviously doesn't matter for 0.  th_win is set to something different
for SYNs.  I guess what you are doing is ok, and even though it is not
needed, I feel that it would be easier to read it with an extra pair
of ().

> Index: tcp_output.c
> ===================================================================
> --- tcp_output.c (revision 198794)
> +++ tcp_output.c (working copy)
> @@ -992,7 +992,7 @@
> * to read more data than can be buffered prior to transmitting on
> * the connection.
> */
> - if (recwin == 0)
> + if (recwin >> tp->rcv_scale == 0)
> tp->t_flags |= TF_RXWIN0SENT;
> else
> tp->t_flags &= ~TF_RXWIN0SENT;
>
>

--
Bjoern A. Zeeb         It will not break if you know what you are doing.
_______________________________________________
freebsd-net@... mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscribe@..."

[John Baldwin]

On Tuesday 03 November 2009 4:26:48 pm Bjoern A. Zeeb wrote:

> On Tue, 3 Nov 2009, John Baldwin wrote:
>
> > Several years ago Dillon added a feature to TCP that casued soreceive() to
> > send an ACK right away if data was drained from a TCP socket that had
> > previously advertised a zero-sized window.  The current code requires the
> > receive window to be exactly zero for this to kick in.  If window scaling is
> > enabled and the window is smaller than the scale, then the effective window
> > that is advertised is zero.  However, in that case the zero-sized window
> > handling is not enabled because the window is not exactly zero.  The patch
> > below changes the code to check the raw window value against zero.  Arguably
> > it could check 'th_win' directly instead if folks would prefer that.
>
> hmm, looking a few lines up, there is a htons() there as well;
> obviously doesn't matter for 0.  th_win is set to something different
> for SYNs.  I guess what you are doing is ok, and even though it is not
> needed, I feel that it would be easier to read it with an extra pair
> of ().

How about using 'if (th->th_win == htons(0))' for the condition?

> > Index: tcp_output.c
> > ===================================================================
> > --- tcp_output.c (revision 198794)
> > +++ tcp_output.c (working copy)
> > @@ -992,7 +992,7 @@
> > * to read more data than can be buffered prior to transmitting on
> > * the connection.
> > */
> > - if (recwin == 0)
> > + if (recwin >> tp->rcv_scale == 0)
> > tp->t_flags |= TF_RXWIN0SENT;
> > else
> > tp->t_flags &= ~TF_RXWIN0SENT;
> >
> >
>
> --
> Bjoern A. Zeeb         It will not break if you know what you are doing.
>

--
John Baldwin
_______________________________________________
freebsd-net@... mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscribe@..."