[tcpm] Benjamin Kaduk's No Objection on draft-ietf-tcpm-rack-14: (with COMMENT)

[Benjamin Kaduk via Datatracker <[e-mail]>]

Benjamin Kaduk has entered the following ballot position for
draft-ietf-tcpm-rack-14: No Objection

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)


Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.


The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-tcpm-rack/



----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

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.