Subject: Re: some sack fixes
To: Jonathan Stone <jonathan@dsg.stanford.edu>
From: Kentaro A. Kurahone <kurahone@sigusr1.org>
List: tech-net
Date: 03/15/2005 21:45:21
--h31gzZEtNLTqOjlF
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On Tue, Mar 15, 2005 at 01:12:22PM -0800, Jonathan Stone wrote:
>=20
> [... [discussion of SACK blocks for peers that do SACK but not
> rfc-1323 timestamps]]
>=20
> For now, I beleive NetBSD requires a conservative strategy: one
> where we have a SACK that is correct in all cases. I'd even give
> up DSACK to be sure that the SACK-only functionality is
> fully RFC-conformant and correct.
Agreed. I'm not sure how many TCP stacks take advantage of D-SACK
(And for detecting spurious retransmits, RFC3522 feels like a better
solution[0]). Removing D-SACK would simplify the code a bit.
> Sending one less SACK block than the absolute maximum, in the (rare?)
> case of a peer that will do SACK, but not timestamps, fits my definition
> of "conservative", as it eliminates some corner-case logic.
>=20
> On that note: just by quickly skimming the code, I can no longer
> quickly convince myself that tcp_update_sack_list() is fully
> conformant with RFC-2018 . The original FreeBSDBSD code comments that:
>=20
> first reported SACK block MUST be the most recent one [[most recently
> received? -- jrs]], and the subsequent blocks SHOULD be in the order
> in which they arrived at the receiver. These two conditions make the
> implementation fully compliant with RFC-2018.
>=20
> Is the SHOULD implicitly guaranteed by the tailq, or by something
> else, or do we not guarantee it? Can we easily add kernel ASSERT()s to
> check the `MUST', and a DIAGNOSTIC check for the SHOULD?
The tailq should implicity guarantee this.
=20
> Also: it looks to me like FreeBSD sys/netinet/tcp_sack.c revision
> 1.8->1.9 (check for receiver sending a SACK block outside the window)
> would be useful. The limits (both per-connection and global) in
> FreeBSD's tcp_sack.c rev 1.9->1.10 sound like they're worth having.
> I also wonder about 1.7->1.8, although (see my first question!)
> maybe our tailq means you already eliminated those usages?
IIRC the reassembly code doen't get called for data received out of
window, so we should not need 1.7->1.8.
The other changes look like they are worth having.
> Hmm. Is FreeBSD's tcp_sack.c rev 1.6->1.7 the bugfix you alreadcy
> integrated before the NetBSD commit?
Yes, I pulled this in right before the commit.
--=20
Kentaro A. Kurahone
SIGUSR1 Research and Development
[0]: Except when the peer is WinXP I suppose.
--h31gzZEtNLTqOjlF
Content-Type: application/pgp-signature
Content-Disposition: inline
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.0 (FreeBSD)
iQIVAwUBQjdXb2fp+SLSG+tuAQKK4xAAhZ9jUAB0iOxNxqxyooLsV44LP1wrV5WS
fsu7N0R6h+BO2pmk51OrgQx9gfZYK/1cAU3bfOixgt30i7hHEPGzVt2Y8ggBxxud
yL79Qds+7TAJSyMoiz9kNkdVGB/+d6t0xUp3UBepmPbCxHpyT7ZuhIkDtjA3gdnf
0E0r33HMdPBbp0IkQaDqF13Zl6UMJ6MCTisJidRb9KUHmcuVmpRlxE7Hk1uL4U6s
VjIkA00/4vSy++PKZtx54vthTMFxH+lIZizGPcJQAWmmKo6nOlv7Q+I3gJBsyMEj
3ln8Roa7OlYxgoZMZLVHREjxH0jcf+NC4o9cp4okoOuwHiCCXRWJfvsGFrq+PvVu
6QjjXCXW3JLk7PjlnCatbU5oWx1V0CbH2bDIgqLiw5A+MxRXlvCBy2tWCy4r5Ui7
pnnaDPr69E6xWO0X5AV6Jll0l/8VEE3nqF4iEO5j7V1uCirSlyrW0tXXl1wPpSYQ
aO5Qm53neRUFxsjqBEXt84Adybm8FV0R/f0rJnEJj6HovpZBZSiPQsn54mCHwwRl
4Rwa16Dirx3K52dJgzwPitR4Pu5zVyiZcbP0jWUPMVGZrjhdBHuxWBto8Pokv4H4
enowbZ93iQi2r59JRcctLTXr79KXrpXp7iGayo7tsXkwXZLzHuqD6Bw9khAtlXMe
KXn63dmRJyQ=
=Pzo1
-----END PGP SIGNATURE-----
--h31gzZEtNLTqOjlF--