The RACK-TLP Loss Detection Algorithm for TCP
draft-ietf-tcpm-rack-15

[[ nits ]]

[ section 3.1 ]

* "Conceptually, RACK puts a virtual timer for"

  Instead of "puts" perhaps "uses", "keeps", "imagines", or something?

[ section 4 ]

* "a timestamp whose granularity that is finer" ->
  "a timestamp with a granularity that is finer",  or
  "a timestamp whose granularity is finer", perhaps

[ section 9.4 ]

* "sender takes longer time" -> "sender takes a longer time"?

Thanks for this work.  It's interesting stuff!

Please expand DUPACK on first use.

Section 2 is curious.  As I read it, it establishes a normative recommendation to use it, but doesn't update either of its antecedents (RFC 5681 or RFC 6675) such that someone reading those might be referred to this.

Thank you to Paul Wouters for the SECDIR review.

Just a very small comment:

— Section 1 —

   In this document, these words will appear
   with that interpretation only when in UPPER CASE.  Lower case uses of
   these words are not to be interpreted as carrying [RFC2119]
   significance.

I don’t particularly object to that quoted text, but it’s redundant: the (correct) BCP 14 boilerplate before it already says that.  What’s the purpose of adding this text?  (I’m guessing it’s a remnant: it was there with the old BCP 14 boilerplate frim 2119, and when you switched to 8174 you didn’t remove this.)

Thanks for presenting this complicated topic in a very easy-to-read
manner!

Section 3.3.2

   1.  The reordering window SHOULD be set to zero if no reordering has
       been observed on the connection so far, and either (a) three
       segments have been delivered out of order since the last recovery

I assume there's some subtle technical difference between "reordering"
and "segments delivered out of order" that makes this a reasonable thing
to say ... but it would be nice if the distincion was made more clear
(whether here or in earlier text).

Section 6.2

   Among all the segments newly ACKed or SACKed by this ACK that pass
   the checks above, update the RACK.rtt to be the RTT sample calculated
   using this ACK.  Furthermore, record the most recent Segment.xmit_ts
   in RACK.xmit_ts if it is ahead of RACK.xmit_ts.  If Segment.xmit_ts
   equals RACK.xmit_ts (e.g. due to clock granularity limits) then
   compare Segment.end_seq and RACK.end_seq to break the tie.

Perhaps we should state what the result of breaking the tie is used for
(i.e., updating RACK.segment & co.)?

   To avoid the issue above, RACK dynamically adapts to higher degrees
   of reordering using DSACK options from the receiver.  Receiving an
   ACK with a DSACK option indicates a possible spurious retransmission,
   suggesting that RACK.reo_wnd may be too small.  The RACK.reo_wnd
   increases linearly for every round trip in which the sender receives
   some DSACK option, so that after N distinct round trips in which a
   DSACK is received, the RACK.reo_wnd becomes (N+1) * min_RTT / 4, with
   an upper-bound of SRTT.

What constitutes a "distinct round trip"?

       Return min(RACK.min_RTT / 4 * RACK.reo_wnd_mult, SRTT)

I suggest reordering the expression to be min(RACK.reo_wnd_mult *
RACK.min_RTT / 4, SRTT), to avoid needing to consider the order of
operations and operator precedence in the pseudocode.  (soapbox: in
formal mathematics, there is no "division" operation, just
multiplication by the multiplicative inverse, in part because it makes
dealing with associativity and commutativity of operations harder to
reason about.)

Section 8

   sender SHOULD cancel any other pending timer(s).  An implementation
   is to have one timer with an additional state variable indicating the
   type of the timer.

nit: maybe "is expected to have one timer", to avoid any risk of being
interpreted as over-specifying implementation behavior?

Is there anything to say about increasing the usage of fast recovery
(vs RTO) potentially having an aggregate effect globally on how much
data is in flight and increasing the overall risk of congestion?  (I
mostly assume not, but it's kind of my job to ask.)

Section 13.1

In terms of how we reference it, RFC 3042 seems like it could be
informative.

Hi,

I didn't manage to review the algorithm in detail (way outside my area), but surrounding text was clear, understandable and interesting for someone without TCP expertise, so thank you for that.

One minor NIT would be the change the footer from "RACK" to "RACK-TLP".

Section 9.5 talks about using RACK for other protocols and I wanted to confirm that it does mean only RACK and not RACK-TLP for this case.

Regards,
Rob