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

From: Benjamin Kaduk via Datatracker <[email-masked]>
Date: Thu, 17 Dec 2020 09:55:01 -0800

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.

---

From: Yuchung Cheng <[email-masked]>
Date: Fri, 18 Dec 2020 17:59:20 -0800

> https://datatracker.ietf.org/doc/draft-ietf-tcpm-rack/
Thank you for your review.

> (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.

> (i.e., updating RACK.segment & co.)?

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

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

> dealing with associativity and commutativity of operations harder to
Sure we'll change to the suggested more clear format.

> interpreted as over-specifying implementation behavior?
good idea. will add "expected".

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

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

---

From: Benjamin Kaduk <[email-masked]>
Date: Tue, 29 Dec 2020 12:06:56 -0800

Hi Yuchung,

> We'll add that definition on the first appearance of the word 'reordering'

I am quite confident that you know how this works, though I am not sure
that I understand properly yet -- is the key distinction that "reordering
has been observed" and "segments have been delivered out of order" refer to
time vs TCP sequence number?  (There's not actually a need to respond; I am
sure the document will be correct.  I am just curious, is all.)

> decrease functions) of these long flows and less about #fast-recoveries.

Okay.  Thank you for thinking about it, and thanks for all the updates.

-Ben

---

From: Neal Cardwell <[email-masked]>
Date: Tue, 29 Dec 2020 23:04:41 -0500

> sure the document will be correct.  I am just curious, is all.)

Benjamin, I think I did not previously understand your earlier remark: '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 now I think perhaps I understand what you were getting at: were you
remarking that there is an apparent contradiction in the text between "no
reordering has been observed on the connection so far" and "three segments
have been delivered out of order", so that it seems impossible for the
connection to meet the conditions for both "no reordering has been observed
on the connection so far" and "(a) three segments have been delivered out
of order since the last recovery"?

I agree that the text of the draft as currently written seems contradictory
or at least unclear. :-)

The "three segments have been delivered out of order" phrase in the draft
was meant to refer to three segments being SACKed. But now that you raise
this point, it seems to me it would be more correct and clear to say
something like "three segments have been SACKed", rather than "three
segments have been delivered out of order".

>From my perspective, the SACKed segments are not necessarily "delivered out
of order", they merely have at least one hole in the sequence space. For
example, in the sequence [1, 3, 4, 5], the [3, 4, 5] are not "out of
order", and were they TCP segments we would not say that reordering has
been observed; it's just that 2 is not (yet?) present.

I also like the "three segments have been SACKed" phrase since the "have
been SACKed" phrase is used in RFC6675 for a similar purpose in the
description of "IsLost (SeqNum):"

If it's not too late for edits, I would propose that the section you
mentioned ("3.3.2
<https://tools.ietf.org/html/draft-ietf-tcpm-rack-15#section-3.3.2>.
Reordering window adaptation") that says:

   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
       or (b) the sender is already in fast or RTO recovery.

would be edited to:

   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 SACKed* since the last recovery
       or (b) the sender is already in fast or RTO recovery.

Benjamin, does that help address what you were talking about?

best,
neal

---

From: Benjamin Kaduk <[email-masked]>
Date: Tue, 29 Dec 2020 21:42:16 -0800

Hi Neal,

> of order since the last recovery"?

Exactly so!  Sorry for not having been more clear originally.

> I agree that the text of the draft as currently written seems contradictory

Well, I guess I can feel better about having been confused by it, now :)

> been observed; it's just that 2 is not (yet?) present.

Indeed, there is some subjectivity in the definition of "out of order".
(My initial draft comments on this document had included a request for more
clarity on how to tell which segments are "out of order" as compared to
some baseline, but I deleted it once I got to, IIRC, discussion in Section
6.2 about segments below RACK.fack getting acknowledged as being ones
delivered out of order, which I took to be effectively a definition, and
seemed well-formed at the time.  Though perhaps that formulation is not
well-defined for what it means for a retransmitted segment to be
out-of-order...)  If we apply the definition of "a segment is delivered out
of order if, when it is ACKd, RACK.fack is greater than its sequence
number" to your example above, it seems like we would conclude that 2 is out
of order, but 3, 4, and 5 are in-order.  (Right?)

> description of "IsLost (SeqNum):"

Also a pretty compelling argument.

> Benjamin, does that help address what you were talking about?

Yes.  From my point of view, that proposal is a big improvement.

Thanks,

Ben

---

From: Neal Cardwell <[email-masked]>
Date: Wed, 30 Dec 2020 08:55:25 -0500

On Wed, Dec 30, 2020 at 12:42 AM Benjamin Kaduk <[email-masked]> wrote:

> to

Great. Thanks for raising this issue!

> of order, but 3, 4, and 5 are in-order.  (Right?)

Right, exactly. If there are no retransmissions and the sequence of packet
deliveries is [1, 3, 4, 5, 2] then my intuition and
RACK_detect_reordering() would not conclude that 3, 4, 5 are "reordering",
but upon seeing that 2 is delivered both my intuition
an  RACK_detect_reordering() would declare that reordering has been seen.

> Yes.  From my point of view, that proposal is a big improvement.

Great. Glad to hear it. We will talk it over among the draft co-authors and
see if we can make this more clear.

Thanks!

neal

---

From: Yuchung Cheng <[email-masked]>
Date: Wed, 30 Dec 2020 09:11:35 -0800

Thank you Ben for your review. Here is the latest proposed diff. Let us
know what you think and we'll work w/ RFC editors to incorporate this minor
change.

On Wed, Dec 30, 2020 at 5:55 AM Neal Cardwell <[email-masked]> wrote:

> RACK_detect_reordering() would not conclude that 3, 4, 5 are
> "reordering", but upon seeing that 2 is delivered both my intuition
> Great. Glad to hear it. We will talk it over among the draft co-authors
> and see if we can make this more clear.
>
> Thanks!
>
> neal
>
>
>