Subject: Re: some sack fixes
To: Kentaro A. Kurahone <kurahone@sigusr1.org>
From: Jonathan Stone <jonathan@dsg.stanford.edu>
List: tech-net
Date: 03/15/2005 13:12:22
[... [discussion of SACK blocks for peers that do SACK but not
rfc-1323 timestamps]]

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.

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.

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:

  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.

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?


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?

Hmm. Is FreeBSD's tcp_sack.c rev 1.6->1.7 the bugfix you alreadcy
integrated before the NetBSD commit?