Subject: [tcpm] Review (Re: WGLC for draft-ietf-tcpm-rack-08)

From: Theresa Enghardt <[email-masked]>
Date: Sat, 04 Apr 2020 18:28:04 +0200

Dear TCPM,

I reviewed draft-ietf-tcpm-rack-08 focusing on whether it is clear and 
easy to follow when seeking to understand how one would implement this 
and/or how an existing implementation works. As a secondary concern, I 
noted some editorial comments and suggestions of how one could make the 
document more accessible.

Overall, I think the document explains the scope and idea of the 
algorithms really well and the algorithm details that are provided are 
already quite useful. So thank you for this draft.
However, I did find a couple of points where some more clarification 
would help, and a few consistency as well as editorial issues. I would 
appreciate it if these were fixed before this document is published as a 
Proposed Standard RFC.

Please find my review below:

Section 2, Introduction:

“In recent years we have been observing several increasingly common loss 
and reordering patterns in the Internet: …”
As far as I can tell, this list actually combines the description of the 
loss patterns with the limitations that existing loss recovery 
mechanisms have, rather than describe limitations that are inherent to 
the patterns or to TCP. So I think it makes sense to add something like 
the following to this introductory sentence: “Given these patterns, 
existing loss recovery mechanisms have the following limitations:” 
Alternatively, you could move the paragraph starting with “Despite TCP 
stacks …” above the list, so the list would already be in the context of 
limitations of existing loss recovery mechanisms.

Please consider adding a half-sentence here explaining what a tail drop 
is to make the document more accessible.

“Also, these algorithms, including RFCs, rarely address the interactions 
with other algorithms.”
I think it’s worth clarifying the scope of the problem: Is this about 
using multiple different algorithms at the same time on the same sender? 
Or is this about interaction between different senders using different 
algorithms?

“The goal of RACK is to solve all the problems above …”
Perhaps this paragraph should mention that RACK has limitations, too. 
For example, this paragraph could already touch upon the problem of 
reordering and spurious retransmissions, or it could briefly summarize 
some of the disadvantages discussed in Section 8.2.

The introduction, as well as the abstract, need to mention that this 
document actually introduces two algorithms, RACK and TLP, and what 
their relationship is. For example, while they can be implemented 
separately, are there any dependencies between them? Why is it 
beneficial to use them together?
Note that Section 8.5.1 has wording that suggests that using RACK 
implies TLP and Section 9 implies that it’s possible to use TLP without 
RACK. Please consider adding some of these points to the introduction 
and make sure they’re consistent in the document.
Moreover, Section 8.4 discusses the relationship between RACK and TLP at 
the very end. Please consider moving (some of) this discussion to 
Section 2 or include a reference to the discussion here.

Section 3, Overview:

"RACK can be applied to both fast recovery and timeout recovery ..."
Timeout recovery refers to recovery after an RTO, correct? Does RACK 
work the same way for both fast recovery and timeout recovery? This 
reads a bit confusing to me, because the whole point of RACK seems to be 
to avoid RTOs, and if an RTO happens, we already know that there has 
been a packet loss.

Section 4, Design Rationale for Reordering Tolerance:

"TCP congestion control implicitly assumes the feedback from ACKs are 
from the same bottleneck"
Please consider making the wording a bit clearer: ”... feedback from all 
ACKs in a connection are from the same bottleneck” - unless I’m missing 
something here.

"How long the sender waits for such potential reordering events to 
settle is determined by the current reordering window."
In this paragraph, it seems to me like the document jumps from general 
considerations on the impact of RACK on packet reordering to specific 
design considerations for RACK to mitigate these potential impacts. I 
think it’s a good idea to make this transition clearer, e.g., by adding 
text like: "RACK deals with reordering by estimating a reordering 
window. The way RACK deals with reordering disincentivizes excessive 
reordering because of the following constraints on the reordering 
window: ..."
If the reordering window is a new concept that RACK introduces, or a 
concept that was not widely known before, it might be worth explicitly 
introducing it as a concept.

“If no reordering has been observed, then RACK SHOULD honor the classic 
3-DUPACK rule for initiating fast recovery.  One simple way to implement 
this is to temporarily override the reorder window to 0.”
Just to make sure I understand correctly: Honoring the classic 3-DUPACK 
rule means detecting a packet lost after receiving 3 dup acks, in 
addition to detecting packets lost based on timestamps, basically 
whichever occurs first? It’s not obvious to me how this can be 
implemented by overriding the reorder window to 0. So perhaps I’m 
misunderstanding something here and some clarification would help.

"...to adaptively estimate the duration of reordering events." 
Perhaps it makes sense to add “and adjust the reordering window 
accordingly” to this sentence, as this part talks about mandates on the 
reordering window.

"As a flow starts, either condition 1 or condition 2 or both would 
trigger RACK to start the recovery process quickly." 
Prior to this sentence, I assumed that the above points were mandates, 
i.e., all of these criteria must be satisfied by the implementation. But 
now the text says they are in fact conditions when to declare a packet 
lost? Are 3 and 4 also conditions?

“It also relaxes ordering constraints to allow sending flights of TCP 
packets on different paths dynamically for better load-balancing (e.g. 
flowlets).”
Please consider adding a reference for “flowlets”.

As you discuss design rationale for the reordering window in this 
section, perhaps it’s worth adding a reference to Section 8.3, which 
contains more discussion of the reordering window and its default 
values.

Section 6.1, Definitions of variables:

(Editorial) This appears to be the only subsection of Section 6. Can 
this subsection be collapsed into Section 6?

As you’re discussing packets and sequence numbers, I think it’s worth 
adding a few words on how to map TCP segments to packets, e.g., that an 
implementation might want to store the first and last sequence number of 
each TCP segment and consider this a packet. This might also be helpful 
to make the connection between definitions that are based on sequence 
numbers and others that are based on packets.

Please add a definition of Packet.end_seq here, as this variable is used 
in later sections, but not defined here.

For better overview, it might make sense to group variables, e.g., all 
RTTs, all variables directly related to reordering, all sequence 
numbers.

“The RACK.xmit_ts, RACK.end_seq, RACK.rtt, RACK.reo_wnd, and 
RACK.min_RTT variables are kept in the per-connection TCP control block. 
RACK.packet and RACK.ack_ts are used as local variables in the 
algorithm.”
Aren’t RACK.packet and RACK.ack_ts also per-connection? If so, shouldn’t 
the choice of whether to put these in the per-connection TCP control 
block or to use them as local variables up to the implementer? (Unless 
I’m missing something here, in which case please clarify what the 
difference is between these variables.)

Section 7.2, Upon receiving an ACK:

Step 1: 

"Use the RTT measurements obtained via [RFC6298] or [RFC7323] to update 
the estimated minimum RTT in RACK.min_RTT."
Does this document mandate how to collect individual RTT samples, in 
which case maybe it’s worth highlighting this with a MUST or SHOULD?
Especially as one of the following sentences says "This document does 
not specify an exact approach", but this might only refer to how to 
derive the minimum RTT, not the individual RTT samples. Please clarify.

Do you perform Step 1 even for packets that you later find to be a 
spurious retransmission, as this step occurs before the check for 
spurious retransmissions?
Text in later sections reads as if RACK.min_RTT is not updated for 
spurious retransmissions. In this case, it might be worth merging this 
step with Step 2 and updating all stats after the check for spurious 
retransmissions.

Step 2:

The document first says to record the most recent Packet.xmit_ts if it 
is ahead of RACK.xmit_ts. Again this is before the check for spurious 
retransmissions, at least in the text. The pseudocode performs the check 
first, which makes more sense to me. I think the text should be changed 
to reflect the pseudocode, if this is indeed the intended order of 
steps.

"... the sender would not be able to update its RACK.min_rtt using the 
(ambiguous) RTT samples from retransmissions"
Possibly this whole example is intended as an exercise for the reader, 
but just to be clear: Here the point is that the sender is unable to 
tell whether an ACK for M is from the original transmission or the 
retransmission, right? And therefore it can’t update the RTT? Please 
consider making this example a bit more explicit to make it easier to 
understand.

Step 3:

Maybe consider substituting “false retransmission” with “spurious 
retransmission”, as these terms seem to refer to the same concept.

Step 4:

“RACK starts initially with a conservative window of min_RTT/4"
This is RACK.min_RTT, right? Please use the variable name.

What is the design rationale for increasing reo_wnd by min_RTT/4, and 
not more or less, and for persisting for 16 loss recoveries, and not 
more or less?

SRTT refers to the connection's SRTT, defined similarly as in Section 
7.5 for TLP, right? Maybe it’s worth adding this as a definition in 
Section 6 already.

"Note that extensions that require additional TCP features (e.g. DSACK) 
would work if the feature functions simply return false."
I’m taking this to basically mean “If you don't implement DSACK or if 
you don't get DSACK, just return false from these functions and the 
algorithm below still works”, but I’m unsure if this is correct. Does 
“extensions that require additional TCP features” refer to the 
DSACK-based increases of the reordering window? Because “extensions” to 
me sounds like something that would be defined in another section or 
another RFC, and that would be explicitly marked as an extension.
If the meaning above is correct, please consider making this sentence 
specific to the example of the DSACK-based increase in reordering 
window.

The pseudocode says that RACK.min_RTT is updated here - but wasn't this 
step 1 already? Or is RACK.min_RTT updated twice?

"If SND.UNA < RACK.rtt_seq:
				 RACK.dsack = false /* React to DSACK once per round trip */"
It seems like step 4 is the only part that uses RACK.rtt_seq, but the 
definition in Section 6.1 says “SND.NXT when RACK.rtt is updated.” Isn’t 
RACK.rtt updated in Step 2 already? Is this definition (still) correct?

Another question about the above pseudocode: Why does this mean that the 
algorithm reacts once per round trip? What does the round trip have to 
do with SND.UNA moving above RACK.rtt_seq?

Why do you set RACK.reo_wnd_persist to -1 here, and not to 0?

Step 5:

"the sender MAY install a "reordering settling" timer set to fire at the 
earliest moment at which it is safe to conclude that some packet is 
lost"
This sounds like a really good idea, so why not make this a SHOULD?

"To be more robust to reordering, RACK uses a more conservative RTT 
value to decide if an unacknowledged packet should be considered lost, 
RACK.rtt"
Is it a SHOULD or a MUST to be this conservative? Or are implementations 
also free to use RACK.min_RTT?

s/RACK.time_ts/RACK.xmit_ts/

For the implementation optimization, maybe just briefly say explicitly 
that the outcome of your optimization is that you only check packets 
that have been sent after RACK.xmit_ts, so the advantage is immediately 
obvious to the reader.

Section 7.3, Tail Loss Probe

(editorial) Should this be its own section? After all, it is its own 
separate algorithm?

Please consider substituting  "Common causes of RTOs 
include:" with "Common causes of RTOs, which TLP is intended to 
mitigate, include:" to make the purpose of this section more clear.

Section 7.4, Tail Loss Probe: An Example

Please consider substituting "Following is an example of 
TLP" with "Following is an example of TLP used with RACK".

Section 7.5, Tail Loss Probe Algorithm Details

TLPRxtOut, TLPHighRxt and WCDelAckT seem very different from the 
variable names used otherwise the document, i.e., they are CamelCase, 
while underscores are used in most other variables of the document. Also 
these variable names are really hard to read and pronounce. Unless 
there’s a good reason to call them exactly that (have they been defined 
and used in other documents, in which case maybe it’s worth adding a 
reference?), maybe consider making these variable names more similar.

(editorial) The PTO definition should go on a new line, otherwise it's 
easy to overlook.

Section 7.5.1, Phase 1: Scheduling a loss probe:

(editorial) There is a lot of nesting here, i.e., “phase 1”, “step 1”, 
point 1., 2., 3.. Maybe consider restructuring this part, e.g., let 
phases be different subsections, not subsubsections.

(editorial) In the RACK algorithm, you put the description first and 
then the pseudocode. Here, it's reversed. Maybe it's worth unifying your 
approach? FWIW, I prefer putting the description first because it makes 
the pseudocode easier to follow.

"A sender should schedule a PTO only if all of the following conditions 
are met"
Is this a “SHOULD NOT schedule a PTO unless all of the following 
conditions are met”?

"2. The connection has no SACKed sequences in the SACK scoreboard"
What is the reason for this constraint?

"The RECOMMENDED value for WCDelAckT is 200ms."
Why this value and not another?

s/If FlightSize = 1/If FlightSize == 1/

Section 7.5.2, Phase 2: Sending a loss probe:

SMSS is used here, but not defined. Please expand and add a definition.

Please clarify here already that while you allow only one TLP 
retransmission to be in flight at a time, you allow multiple TLP new 
data to be in flight. This is said later, but it is already relevant for 
understanding this part.

As you don't allow multiple TLP retransmits to be in flight: Does this 
mean the PTO could fire but then you do not send any data, because there 
is no new data available but there is already a TLP retransmit in 
flight?

"the sender MUST arm the RTO timer, not the PTO timer, at the end of 
TLP_send_probe() if FlightSize is not zero."
Please add this to the pseudocode.

"Checking TLPRxtOut prior to sending the loss probe is also critical to 
avoid TLP loops if an application writes periodically at an interval 
less than PTO."
What is a TLP loop? And why would it occur if an application writes 
periodically at an interval less than PTO? Please consider adding a 
definition.

Section 7.5.3: Phase 3: ACK processing

Isn’t this phase a special case of phase 1? “Phase” implies that these 
occur one after the other.

Section 7.6: TLP recovery detection

Is this still part of the TLP algorithm? If so, why is it a new 
subsection and not in the “TLP algorithm details” section, and not 
referenced from it? Or is this an optional addition/extension?

To make the purpose of this algorithm clearer early on, please consider 
briefly stating in the first paragraph that this introduces an algorithm 
to detect whether a packet loss occurred or not, i.e., whether both the 
original packet and the TLP arrived, or whether one of them got lost, 
therefore, it's a loss, therefore, you have to reduce cwnd.

Section 7.6.3, Detecting recoveries accomplished by loss probes:

What is the relationship between this Step 1, 2, 3, and Phase 1, 2, 3 
above? Consider rethinking your structure to make clear what happens 
when. If “Step” and “Phase” refer to the same concept, please consider 
unifying your terminology.

For condition 1: Why does this include "the segment contains no 
data" and "the segment is not a window update"? (Why) Is it not possible 
to detect recovery if either of these is true?

Condition 1 refers to a dup ack for the TLP retransmission, right? And 
receiving this dup ack implies the TLP retransmission was spurious? (I’m 
just trying to make sure I understand correctly, but maybe it's worth 
adding this to the draft to make it easier to follow.)

Condition 2 uses DSACK, but you previously say "Since a significant 
fraction of the hosts that support SACK do not support duplicate 
selective acknowledgments (D-SACKs) [RFC2883] the TLP algorithm for 
detecting such lost segments relies only on basic SACK support 
[RFC2018]."
This seems to be at odds. Isn't it rather "with DSACK you can make this 
detection more accurate, if you don't have it you have to rely on 
condition 1 only"? Please consider rephrasing the sentence starting with 
“Since a significant fraction…”

The last sentence of step 1 says:  "... so the sender considers the TLP 
episode to be done, and records that fact by setting TLPRxtOut to 
false." 
Isn't this already part of step 2, "Mark the end of a TLP retransmission 
episode and detect losses"? Should this part be moved to step 2?

"...it SHOULD invoke a congestion control response equivalent to fast 
recovery."
Is this really a SHOULD or is this a MUST? Because the document also 
says, "If the TLP sender does not receive such an indication before the 
end of the TLP retransmission episode, then it MUST estimate that either 
the original data segment or the TLP retransmission were lost, and 
congestion control MUST react appropriately to that loss as it would any 
other loss."

Section 8.1, Advantages:

"Suppose the transmission of each packet is at least RACK.reo_wnd (1 
millisecond by default)"
Isn't the window RTT_min/4 by default? I think this is what it says 
above. I am not seeing the 1 millisecond mentioned in any other part of 
the document.

"This example illustrates how RACK is able to repair certain drops at 
the tail of a transaction without any timer."
What do you mean by “without any timer”? RACK does potentially use 
timers, doesn't it? Did you mean a specific timer here, like "without 
PTO" or "without RTO"?

Section 8.3, Adjusting the reordering window:

"... the Performance evaluation section ..."
Maybe consider adding a reference to this section.

Section 8.4, Relationships with other loss recovery algorithms

At the very end, this section discusses the relationship between RACK 
and TLP. As this document introduces both algorithms, consider moving 
this discussion somewhere further up in the document. Alternatively, 
please at least mention the existence of this discussion earlier in the 
document and add a reference to the section of this discussion.

Section 8.5, Interaction with congestion control

"RACK may detect losses faster or slower than the conventional duplicate 
ACK threshold approach does.  RACK can detect losses faster by not 
requiring three DUPACKs, so congestion control may reduce the congestion 
window earlier."
Does this text imply that RACK can be used without the three dup ack 
threshold? This seems to be at odds with other parts of the document, 
which clearly state that RACK implies reacting to three dup acks.
If RACK indeed implies using the three dup ack threshold, is this 
paragraph still true?

Similar to my comment on Section 2, please consider briefly clarifying 
whether “interaction” means using two algorithms within the same sender, 
or whether this is about coexistence of different senders.

Section 8.5.1, Example: interactions with congestion control

"With RACK, a sender would send the TLP after 2*RTT"
This reads as if RACK does imply TLP?

Section 8.7, RACK for other transport protocols

"The algorithm can be simplified by skipping step 3 "
Please consider just briefly clarifying if you mean step 3 of RACK, 
"Detect packet reordering". In fact, why does it mean that? Even if you 
have unique transmission or packet identifiers, reordering might still 
occur, right? So don’t we still have to detect packet reordering?

Section 9, Experiments and Performance Evaluations

“We plan to expand our experiments to regions with worse connectivity, 
in particular on networks with strong traffic policing." 
Just curious: Have these experiments actually taken place?

Section 10, Security Considerations

"An interesting scenario is ACK-splitting attacks"
Maybe start the paragraph with something saying "In fact, RACK lowers 
the risk profile because it prevents the following attack"? But then it 
does change the risk profile, which seems to be at odds with the first 
sentence of this section.

This concludes my review. Again, thank you for this draft.

Best,
Theresa Enghardt

---

From: Mirja Kuehlewind <[email-masked]>
Date: Mon, 6 Apr 2020 12:40:44 +0000

Hi all,

I reviewed this document and I think content-wise this is ready to move on (just some small questions below).

I would like to see another editorial pass on the document. There are more detail comments below but I think the main points are a) provide a more detailed algorithm overview section (either in section 3 or beginning of section 7 to make it easier to understand the steps in the subsections in 7 and b) have an own main section for TLP and introduce parameters used for TLP also in section 6 and mention TLP in the intro already. It's still obvious that TLP was squeezed in later on....

Further I think the security considerations could be more extensive: it could a) mention again some of the reordering consideration in section 4 (or maybe some of the text can be moved there entirely to not break the reading flow so much at the beginning of the doc) and b) say more about the impact of this algorithm on spurious retransmission (I guess it can both increase or decrease the number of spurious retransmissions in certain scenarios) and c) mention again the impact on congestion control as described in section 8.5 that a window reduction can happen both earlier or later in different scenarios.

Here are my technical questions/comments on section 7.2:

For this sentence:
"If no reordering has been observed, RACK uses
   RACK.reo_wnd of 0 during loss recovery, in order to retransmit
   quickly, or when the number of DUPACKs exceeds the classic DUPACK
   threshold."

And the respective pseudo code here:
       If RACK.reord is FALSE:
           If in loss recovery:  /* If in fast or timeout recovery */
               RACK.reo_wnd = 0
               Return
           Else if RACK.pkts_sacked >= RACK.dupthresh:
               RACK.reo_wnd = 0
               Return

My understanding here is that the RACK-based time threshold should not be used if in recovery or if no reordering is detected, right?

Why is that realized by setting the RACK.reo_wnd to zero rather than implementing this logic in RACK_detect_loss()? I think the difference would be that you don't reset RACK.reo_wnd but keep the previous state about adjustments. But wouldn't that be good/better?

My point it that the way it is described now confused me a bit and I'm looking for a way to better describe that part...

Also why is RACK.dupthresh a separate parameter in the RACK name space instead of using the "existing" one (as you do with Packet. xmit_ts)? Further this is actually not a parameter that is adjusted by the algorithm but a configured constant. It be helpful for the reader to indicate this in the naming, e.g. call it DUPTHRESH instead. In general I'm actually not sure that using the RACK name space in this document is really helpful.

And another unrelated question but on the same part of the algorithm: where does the number 16 come from for the number of recovery cycles to keep any adaptation?

Then my more detailed (small and larger) editorial comments:

1) In the intro you use  a couple of times "we" which is maybe less common in RFCs as an RFC is a product of a wg not only of the author group. Maybe you can rephrase this.

2) also in the intro:
"The goal of RACK is to solve all the problems..."
Maybe
"The goal of RACK is to address all the problems..."

3) Section 3 provides rather a high level overview. It would be nice to get more of an overview about the algorithm itself, e.g. explaining the different step described in section 7 on a high level. Or maybe you could add alternatively more text at the beginning of section 7 explaining how the different subsections "fit together". Actually doing both would also not hurt as some redundancy where you go step by step in more details is often really helpful for the reader.

4) Section 4 I first thought that some of this text could actually go into the security considerations, however, I guess there are also some requirement here, so not sure if that could be splitted up somehow. I think the problem I have here is that is breaks quite a bit the reading flow between the overview section and the actual algorithm part. 

5) I find the naming of both RACK.reo_wnd and RACK.reo_wnd_incr a bit confusing, as RACK.reo_wnd is a time  frame (and I would expect a packet window based on the naming, given we have the congestion window or the received window) and RACK.reo_wnd_incr is a multiplier (and I would expect some value that would be added when "incr" is used)

6) 7.2:
"   Sometimes the timestamps of RACK.Packet and Packet could carry the
   same transmit timestamps due to clock granularity or segmentation
   offloading (i.e. the two packets were handed to the NIC as a single
   unit).  In that case the sequence numbers of RACK.end_seq and
   Packet.end_seq are compared to break the tie."
The textual description is not fully clear to me (pseudo code is fine). I guess this text is in the context of update RACK.Packet itself, right? Maybe that the missing part that should be spelled out.

7) I would recommend to have section 7.3. to 7.6 in their own new main section as it is an supplemental but separate algorithm. Further I would also recommend to mention Tail Loss Probe also in the abstract and intro.

8) Section 7.5: At least SRTT was already used earlier on in the doc, so maybe just add all these definitions to section 6.1?

9) Section  8.1:
" The examples above show that RACK is particularly useful when the
   sender is limited by the application, which is common for
   interactive, request/response traffic."
I think that is actually an important point to make in the introduction of the document.

10) Also section 8.2:
"   RACK can easily and optionally support the conventional approach in
   [RFC6675][RFC5681] by resetting the reordering window to zero when
   the threshold is met.  Note that this approach differs slightly from
   [RFC6675] which considers a packet lost when at least DupThresh
   higher-sequence packets are SACKed. ..."
Also this information would be really helpful in the overview section already.

11) Also on both sections 8.1. and 8.2, I find the section titles not very well suitable. I would maybe suggest to rearrange the content a bit and have a section "Examples where RACK (and TLP) is beneficial" and another section on "Implementation considerations". At least for  the examples section I would recommend to use an own subsection.

12) Maybe rename section 8.3 to "Reasoning for basic reordering window size" (instead of " Adjusting the reordering window"...?

13) Section 8.4:
" Furthermore, RACK naturally works well with Tail Loss Probe [TLP]"
This reference should be removed as content was integrated into this document. Maybe the whole paragraph should be rephrased, removed entirely, or moved into the TLP section.

14) Section 8.5.1:
" Without RACK, the sender would time out, ..."
I suggest to say "Without RACK and TLP,..." as TLP is described as a separate optional algorithm but TLP is the important bit here. Also If you decide to have a separate example section, I would suggest to move this example there as well.

15) Should section 9 stay in the final RFC? If so, I recommend to move it into the appendix. However, it would also be nice to also provide results on the number of retransmissions (have they increased with RACK and TLP?). Alternatively I suggest removing this section and point to some paper maybe?

16) I think the security considerations could be more extensive, covering impact on reordering, spurious retransmission, and congestion control, as I said at the top of this mail.

Thanks!
Mirja

On 31.03.20, 00:28, "tcpm on behalf of Michael Tuexen" <[email-masked] on behalf of [email-masked]> wrote:

    Dear all,
    
    just a quick reminder:
    
    We are currently running a WGLC for TCP RACK until Tuesday, April 7th 2020.
    
    Please send any comments, including indications to support this document,
    to the TCMP mailing list by then.
    
    The ID is available at
    https://tools.ietf.org/html/draft-ietf-tcpm-rack-08
    
    Best regards
    Michael

---

From: Gorry Fairhurst <[email-masked]>
Date: Mon, 6 Apr 2020 16:22:32 +0100

I reviewed the latest version of RACK as a part of the TCPM WGLC.

I do not think this is ready. I think the specification is important, 
but the document has many editorial issues.

These issues seem to be very important and I do not think the current 
document is ready to proceed without a revision being prepared that can 
be properly reviewed. Overall I am concerned that this draft provides 
little evidence or justification for the proposed change to an existing 
PS. I do not argue against the proposal, but  I do suggest the 
justification and trade-offs involved needs to be clear before the IETF 
should publish such a specification.

Some of the comments below could be fixed by editorial work, but as 
currently written this makes many bold statements about IETF consensus 
that I would not agree with. Many of these could be easily rephrased too 
focus only on the topics being standardised, or by providing appropriate 
evidence for the claims.

Specific comments follow - some issues I will highlight in a separate mail.

Gorry

Section 2:

—
I do not see a reference that offers data for this:
In recent years we have been observing several increasingly common loss 
and reordering patterns in the Internet.
- Can you explain who “we” are and where this has been published or 
presented? I’d be quite comfortable if this said “in recent tears there 
is an increasing concern about the implications of loss or reordering” - 
but the present set asserts this is common, so I ask for evidence please?
—
This is not explained:
“ Structured request-response traffic turns more losses into tail drops.”
- I don’t think the losses are turned into tail drops, maybe the losses 
are observed at the tails of transmission?
—
This is not justified:
“ Despite TCP stacks (e.g.  Linux) that implement many of the standard
    and proposed loss detection algorithms 
[RFC4653][RFC5827][RFC5681][RFC6675][RFC7765][FACK][THIN-STREAM], we've 
found that together they do not perform well. “
- Can you explain who “we” are and where this has been published/presented?
—
This sentence doesn’t read well: “They can either detect loss
    quickly or accurately, but not both, especially when the sender is
    application-limited or under reordering that is unpredictable. “
- /They/A sender/ …. /or under reordering that is unpredictable/or when 
the reordering is unpredictable/.
- What does unpredictable mean in this sentence? Is it about the pattern 
or reordering, the variation in the pattern, the presence or not within 
an RTT or something else?
—
This is not explained or justified:
“And
    under these conditions none of them can detect lost retransmissions
    well.”
- None of them can doe well? Can you explain this better, saying this is 
not “well” seems a very poor justification for changing a standard
—
This is confusing:
“Also, these algorithms, including RFCs, rarely address the
    interactions with other algorithms.  “
- Which? and why this this important?
—
Since section 2 seems mainly set out to provide the reason for the 
change, would it not be more normal to precede the RFC2119 terminology 
declaration?

This could also apply to section 3, which introduces the method, but 
does not appear normative.
——
Section 3.

“The main idea behind RACK is that if a packet has been delivered out
    of order, then the packets sent chronologically before that were
    either lost or reordered.”
- This is put forward as a TCP specification, and yet it describes 
things in network-layer “packets”. If the IP layer were to fragment, 
does this still work on IP packets?  I think this should be in terms of 
transport segments, or the relationship between packets and segments 
explained.
—
“Using a threshold for counting duplicate acknowledgments (i.e.,
    DupThresh) alone is no longer reliable because of today's prevalent
    reordering patterns. “
- To me this is quite an annoying statement to make without reference to 
a source of this consensus. Saying the method is robust to reodering 
seems good, saying the current Internet has "prevalent reordering 
patterns" makes me ask where? what evidence? how important? etc. I 
suggest this document should not be making these claims.
——
“A common type of reordering is that the last
    "runt" packet of a window's worth of packet bursts gets delivered
    first, then the rest arrive shortly after in order. “
- Cite data please and explain the cases, or please do not claim this is 
“common”!
—
“Today's prevalent lost retransmissions also cause problems with
    packet-counting approaches “
- Cite data please, or please do not claim this is “prevalent”.
——
“are often unable to infer and quickly repair losses”
- “often” is being used here in a way that implies this is frequent? Is 
this strictly necessary to define the algorithm. I think it would be 
enough to explain these events can happen, and that the algorithm can 
detect these cases and appropriately respond. I do not see the need to 
make a statement of how common this is.
—
This isn’t clear text, I had to read several times to be sure:
“On each ACK, RACK marks any already-expired packets lost,
    and for any packets that have not yet expired it waits until the
    reordering window passes and then marks those lost as well”
- I think this could be explained more simply.
—
“hurts” performance. Could be better phrased.
—
  “TCP congestion control implicitly assumes the
        feedback from ACKs are from the same bottleneck. “
- I don’t think this is true. There is no assumption. The design was 
based on a single bottleneck at any one time along the path. However 
that does not imply that more than one bottleneck can not be present.
—
“Therefore it
        cannot handle well scenarios where packets are traversing largely
        disjoint paths.”
- I don’t know what “handle well” means, not do I understand “largely 
disjoint” really means in this context. Can this be explained?
—
“Having an excessively large reordering window to
        accommodate widely different latencies from different paths would
        increase the latency of loss recovery.”
- This needs better phrasing. Why does this have to be “excessively” 
large and why does a TCP transport care about “different paths” 
irrespective of being “widely different” - I could not understand what 
was intended at all.
- —
“An end-to-end transport protocol cannot tell immediately whether a
    hole is reordering or loss. “
- Please explain hole before using.
—
“How long the sender waits for such
    potential reordering events to settle is determined by the current
    reordering window.”
- NiT; except one is measured in packets, and “how long” is in units of 
seconds.
—
“The initial RACK reordering window SHOULD be set to a small
        fraction of the round-trip time.”
- Sounds good, is this after connection setup, or before? How does the 
sender know the RTT?
—
“ To accomplish this RACK places the
    following mandates on the reordering window:”
- The discussion ‘mandates’ appears to be different to the discussion of 
requirements. I don’t see the difference explained. I wonder whether the 
document should place all requirements in section 5, but if not, then 
the purpose should be better explained.
—
“   RACK does not need any change on the receiver.”
- OK. However, there is a requirement that the receiver reports loss 
using SACK.
—
“ RACK.dsack" indicates if a DSACK option .”
- DSACK not mentioned until here. Should you cite DSACK [RFC3708] ?

---

“The RECOMMENDED value for  WCDelAckT is 200ms.”
-Why this value? is this linked to TCP Delayed ACK value?
—
“We have evaluated using the smoothed RTT”
…. where is the data?
—
“They do not make any significant difference in terms of total recovery 
latency.”
- Is this true of **ALL** cases? I strongly dislike such conclusions in 
RFCs. The document can only make conclusions based on available data, 
and claiming this for arbitrary paths is dangerous and unnecessary.
-
“While RACK can be a supplemental loss
    detection mechanism on top of these algorithms, this is not
    necessary, because RACK implicitly subsumes most of them.”
- what is not necessary RACK? or one of them? why does it say “most”, 
please be specific.
-
I think sections 8 and 9 are both informative. Is this the case? I like 
this information, but I would prefer the sections to explain that they 
are informative, if that is the case

---
“"Common causes of RTOs include:"
- This seems unnecessary, can’t you scope this to the ones that you 
think TLP will address, instead of making this very general statement. 
The current text seems to need a reference, whereas saying this is what 
TLP addresses would not.
—
“"A sender should schedule a PTO only if all of the following conditions 
are met"
- what is the actual requirement here? I’m searching to understand what 
is actually needed or recommended.
---

NiTs:

“eg.” should be “e.g.,”

“is the easiest way to get a network to go faster.”
- This needs to be explained better.
---
“Therefore their main
    constraint on speed is reordering, and there is pressure to relax
    that constraint.  “
- I object to this statement within a TCPM document. This is not related 
to the TCPM Charter which is about maintaining transport protocols and 
not changing network-layer forwarding behaviour.
I similarly think  the following sentence is entirely inappropriate:
“If RACK becomes widely deployed, the underlying
    networks may introduce more reordering for higher throughput. “
- albeit with a lower case “may”, this is still something that tsvwg has 
spent many meetings discussing and one that has significant pushback and 
therefore a need for careful choice of words!

---

From: Gorry Fairhurst <[email-masked]>
Date: Mon, 6 Apr 2020 16:46:09 +0100

I reviewed the latest version of RACK as a part of the TCPM WGLC and 
this email contains 3 points I think are more important for the WG to 
consider.

(1) Why re-specify the DupACK Thresold?

I have a question why does RACK need to say this, effectively 
over-riding any future change to the DupACK threshold:
“then RACK SHOULD honor the classic 3-DUPACK rule for initiating fast 
recovery.”.
I suggest: “then RACK SHOULD honor the
        classic DUPACK rule for fast recovery as specified in [RFC5681].”
and later:
“and use of the 3-DUPACK rule” rather than “DUPACK Rule [RFC5681]”.
- If that spec changes, I think RACK should also. I do not see the 
reason to restrict this to the constant three for the purpose of RACK.

—
(2) Can the document be clearer about how RTTs are used and what happens 
on paths where the RTT varies significantly?

“The RACK reordering window MUST be bounded and this bound SHOULD
        be one round trip.”
- Which round trip estimate? Is the smoothed RTT, the RTO? the last 
measured RTT? I think this needs to be explained, because it also 
appears later

“  Since an ACK can also acknowledge retransmitted data packets, and
    retransmissions can be spurious, the sender must take care to avoid
    spurious inferences.  For example, if the sender were to use timing
    information from a spurious retransmission, the RACK.rtt could be
    vastly underestimated.”
- ‘must take care’? is that “needs” or if not, what does “take care” 
really mean?
- Why ‘vastly’ - I would have thought that was possible,  but doesn’t 
the problem emerge when any underestimate is present. Please clarify by 
removing “vastly” or explaining this.
—
    “Use the RTT measurements obtained via [RFC6298] or [RFC7323] to
    update the estimated minimum RTT in RACK.min_RTT.”
- I don’t see either of these specifications define a minimum RTT. I 
understand that QUIC doesn’t have a way to maintain an accurate minimum 
RTT,  how does a sender determine the minimum RTT?
- Considering robustness to path changes, how does the minimum RTT take 
into account the variation of the path RTT?
- How does this relate to RTO Consider 
(https://tools.ietf.org/html/draft-ietf-tcpm-rto-consider-10), which 
appears to place strong demands on timers tracking variance and path 
changes?
—
** “In such cases RACK may not detect losses from
    ACK events and the recovery would then resort to the (slower) TLP or
    RTO timer-based recovery.  However, such events should be rare and
    the connection would pick up the new minimum RTT when the recovery
    ends, so the sender can avoid repeated similar failures.”
- I’d strongly suggest changing that “may” to “could” to be really clear 
this is not permitting something.
- I’m also not sure how a sender would know that “such events should be 
rare” - isn’t that dependent on path characteristics?
—
“"To be more robust to reordering, RACK uses a more conservative RTT 
value to decide if an unacknowledged packet should be considered lost, 
RACK.rtt"
-
- Why is this not normative? I think this is either a SHOULD or a MUST?

There's suggestions that much of this may be covered in various places, 
and that could just need editorial work? As I noted before in the 
working group, I do like the idea of RACK, but I do have concerns about 
what happens on paths where there is a large variation in the RTT. With 
a DUPACK-based recovery the variability extends the recovery time, and 
RTO inflates - both are safe. I do not have any feeling that a sender 
would not be overly aggressive when used over a widely varying path RTT.

This seems like it is not safe unless you include a variance term in the 
RTT, or as an enabling function to using the method.

I really do think this has to be explained and addressed.

—
(3)

There are places where the text explcitly motivates a change to the 
network forwarding behaviour, saying almost that this is an allowed 
outcome of deploying RACK for TCP.

Section 4 in particular is a concern:

" The reordering behavior of networks can evolve (over years) in

    response to the behavior of transport protocols and applications, as
    well as the needs of network designers and operators.  From a network
    or link designer's viewpoint, parallelization (eg. link bonding) is
    the easiest way to get a network to go faster.  Therefore their main
    constraint on speed is reordering, and there is pressure to relax
    that constraint.  If RACK becomes widely deployed, the underlying
    networks may introduce more reordering for higher throughput. "
and later:
"  This handles reordering caused by path divergence in small time
    scales (reordering within the round-trip time of the shortest path),
    which should tolerate much of the reordering from link bonding,
    multipath routing, or link-layer out-of-order delivery."

As explained in TSVWG (within the context of L4S), my own opinion is 
that it would be good to have greater tolerance to reordering and that 
the presence of paths with significant reordering is a motivation for 
that work.

Beyond that, I would urge much caution: Writing anything that could 
suggest RACK enables greater reordering has implications on a wide range 
of IETF protocols and would represent a very significant change in 
advice for the network. I would even suggest that such advice is wrong, 
until methods such as RACK have been deployed in transports as a whole, 
not just TCP. I also think this sort of advice and reasoning does not 
belong within the specification of a protocol mechanism.

Gorry

---

From: Praveen Balasubramanian <[email-masked]>
Date: Mon, 6 Apr 2020 22:54:44 +0000

We reviewed the latest version of RACK as a part of the TCPM WGLC and this is some feedback: 

Section 5 Requirements
>>  For each packet sent, the sender MUST store its most recent transmission time 
It is important to call out that in practice for sends that are batched together in time, an implementation may choose to keep per-send transmission timestamps instead of per-packet. This improves memory efficiency. The way the RFC requirement is worded it seems to exclude such an optimization. 

Section 4 Design Rationale for Reordering Tolerance
>>If RACK becomes widely deployed, the underlying networks may introduce more reordering for higher throughput.
A better word here would be universally deployed because otherwise networks would be unfairly harming TCP stacks that have not deployed RACK. Plus as pointed out, reordering puts other strains on the network including ECMP, RSS, hardware offloads etc. Consider removing any recommendations for networks from this draft to prevent any unintentional relaxing of requirements on networks. This section should ideally only focus on why RACK chooses to relax reordering requirements as they pertain to loss detection and recovery. 

>>temporarily override the reorder window
Nit: Let's consistently call this reordering window throughout the draft.

Section 7.2 Upon receiving an ACK   Step 5.
>>For timely loss detection, the sender MAY install a "reordering settling" timer set to fire at the earliest moment at which it is safe to conclude that some packet is lost.
At least in lab tests this timer seems to make a big difference. Recommend changing this to a SHOULD instead of MAY.

>>This can be implemented by using a seperate doubly-linked list sorted in time order.
Typo separate. We shouldn't recommend a specific data structure. 

Some questions that need more explanation: 
1. What about TLP and its relationship to Limited Transmit? Both of them are techniques trying to solve the same problem. Its important to clarify whether Limited Transmit is left unchanged from RFC 6675. It is also unclear whether Limited Transmit should be upgraded from a MAY to a SHOULD. The non-SACK case might benefit from Limited Transmit? 
2. Why does TLP require SACK? In the case SACK negotiation fails, or SACK blocks are stripped in the network, TLP may still recover faster than an RTO when FlightSize is 1.

-----Original Message-----
From: tcpm <[email-masked]> On Behalf Of Michael Tuexen
Sent: Monday, March 30, 2020 3:28 PM
To: tcpm IETF list <[email-masked]>
Subject: [EXTERNAL] [tcpm] WGLC for draft-ietf-tcpm-rack-08

Dear all,

just a quick reminder:

Please send any comments, including indications to support this document, to the TCMP mailing list by then.

The ID is available at
https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftools.ietf.org%2Fhtml%2Fdraft-ietf-tcpm-rack-08&amp;data=02%7C01%7Cpravb%40microsoft.com%7C0f4389073900466c89c408d7d4f9b460%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637212041259507026&amp;sdata=nQghPe61ph4EBThh4xRYp9LTmfisCp6LZAj96leSpOI%3D&amp;reserved=0

Best regards
Michael