Date: Sat, 20 Mar 2010 11:14:07 +0000 From: Rui Paulo <rpaulo@freebsd.org> To: Chris Harrer <cjharrer@comcast.net> Cc: freebsd-net@freebsd.org Subject: Re: Bug in tcp_output? Message-ID: <2FEF7D36-31B7-4FD7-9350-EECA7B38D519@freebsd.org> In-Reply-To: <006f01cac6d8$5fc03cb0$1f40b610$@net> References: <006f01cac6d8$5fc03cb0$1f40b610$@net>
next in thread | previous in thread | raw e-mail | index | archive | help
On 18 Mar 2010, at 20:19, Chris Harrer wrote:
> Hi All,
>=20
>=20
>=20
> In the following block of code, running on a x86_64 platform, I =
believe that
> cwin should be declared as an int:
>=20
> /*
>=20
> * If snd_nxt =3D=3D snd_max and we have transmitted a FIN, the
>=20
> * offset will be > 0 even if so_snd.sb_cc is 0, resulting in
>=20
> * a negative length. This can also occur when TCP opens up
>=20
> * its congestion window while receiving additional duplicate
>=20
> * acks after fast-retransmit because TCP will reset snd_nxt
>=20
> * to snd_max after the fast-retransmit.
>=20
> *
>=20
> * In the normal retransmit-FIN-only case, however, snd_nxt =
will
>=20
> * be set to snd_una, the offset will be 0, and the length may
>=20
> * wind up 0.
>=20
> *
>=20
> * If sack_rxmit is true we are retransmitting from the =
scoreboard
>=20
> * in which case len is already set.
>=20
> */
>=20
> if (sack_rxmit =3D=3D 0) {
>=20
> if (sack_bytes_rxmt =3D=3D 0)
>=20
> len =3D ((long)ulmin(so->so_snd.sb_cc, sendwin) =
- off);
>=20
> else {
>=20
> long cwin; =DF-- Should be an int
>=20
>=20
>=20
> /*
>=20
> * We are inside of a SACK recovery episode and =
are
>=20
> * sending new data, having retransmitted all =
the
>=20
> * data possible in the scoreboard.
>=20
> */
>=20
> len =3D ((long)ulmin(so->so_snd.sb_cc, =
tp->snd_wnd)=20
>=20
> - off);
>=20
> /*
>=20
> * Don't remove this (len > 0) check !
>=20
> * We explicitly check for len > 0 here =
(although it=20
>=20
> * isn't really necessary), to work around a gcc=20=
>=20
> * optimization issue - to force gcc to compute
>=20
> * len above. Without this check, the =
computation
>=20
> * of len is bungled by the optimizer.
>=20
> */
>=20
> if (len > 0) {
>=20
> cwin =3D tp->snd_cwnd -=20
>=20
> (tp->snd_nxt - tp->sack_newdata) =
-
>=20
> sack_bytes_rxmt;
>=20
> if (cwin < 0)
>=20
> cwin =3D 0;
>=20
> len =3D lmin(len, cwin);
>=20
> }
>=20
> }
>=20
> }
>=20
>=20
>=20
> Consider the case where:
>=20
> sack_rxmit =3D 0
>=20
> sack_bytes_rxmt =3D 0x2238
>=20
> off =3D 0
>=20
> len =3D0xa19c
>=20
> tp->snd_cwnd =3D 0x2238
>=20
> tp->snd_nxt =3D 0xdd6d7974
>=20
> tp->sack_newdata =3D 0xdd6d6858
>=20
> In this case cwin evaluates to 0x00000000ffffe37c, which is not <0, =
but
> instead huge. This causes the remaining data on the socket=92s =
so->so_snd
> buffer to be sent to the network causing more problems at the receiver =
which
> is already dropping frames.
I see. This is most likely a bug. Can you send-pr so this doesn't get =
lost?
Regards,
--
Rui Paulo
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?2FEF7D36-31B7-4FD7-9350-EECA7B38D519>