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

[Yuchung Cheng <[e-mail]>]

On Thu, Dec 17, 2020 at 9:55 AM Benjamin Kaduk via Datatracker <
[e-mail]> wrote:

> 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!
>
Thank you for your review.


>
> 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).
>
Great idea to define reordering more precisely. In this document,
reordering refers to when two TCP segments are delivered at the TCP
receiver in a different order from they were sent at the TCP sender for a
given TCP flow. Notice the ordering is chronological instead of sequential
on TCP sequences.

We'll add that definition on the first appearance of the word 'reordering'
to clarify.


>
> 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.)?
>

sure: we'll add "breaking the tie to update RACK.segment's associated
states'.


>
>    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"?
>
The distinct is redundant. It really means N round trips (which obviously
are distinct and different from the other N-1). We'll remove 'distinct'
word.


>
>        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.)
>
Sure we'll change to the suggested more clear format.


>
> 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?
>
good idea. will add "expected".


>
> 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.)
>
Great question. We agree with you: our conjecture is that RACK-TLP reduces
(loss-based) C.C. over-correction and BW under-utilization than C.C.
under-correction, particularly for app-limited short flows. For bulk long
flows that comprise the majority of the Internet traffic volume, most
losses are well detected by the existing 3-dupack-rule since the flows'
congestion windows are large enough to trigger 3 dupacks. RACK-TLP actually
helps reduce spurious fast recoveries on moderately reordered paths for
these long flows. But overall in our experience, the state of congestion is
much dominated by the actual C.C. algorithms (e.g. slow-start, increase and
decrease functions) of these long flows and less about #fast-recoveries.


>
> Section 13.1
>
> In terms of how we reference it, RFC 3042 seems like it could be
> informative.
>
Good catch. will move to normative