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

From: Yuchung Cheng <[email-masked]>
Date: Mon, 13 Jul 2020 17:17:53 -0700

On Mon, Apr 6, 2020 at 5:41 AM Mirja Kuehlewind
<mirja.kuehlewind=[email-masked]> wrote:
>
> Hi all,
>
> I reviewed this document and I think content-wise this is ready to move on (just some small questions below).
Thank you for the review. Sorry it took so long to respond. We have
made lots of changes based on your and other comments (nearly a
rewrite).
https://tools.ietf.org/html/draft-ietf-tcpm-rack-09

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

Thank you very much. We rewrote the first four sections so that
RACK-TLP are an integrated design. We also add a protocol diagram to
illustrate.

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

We are not sure about the prospects of reordering consideration,
congestion control, and spurious retransmission in the context of
security. We do added this part
"   RACK-TLP algorithm behavior is based on information conveyed in SACK
   options, so it has security considerations similar to those described
   in the Security Considerations section of [RFC6675].
"

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

Correct. Zero RACK reordering window  essentially makes retransmit
immediate w/o any further reordering accommodation.
>
> 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...

To reduce confusion we move the RACK.reo_wnd computation inside
RACK_detect_loss. RACK.reo_wnd is adjusted every ACK. Does the new
pseudocode help?

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

We revised and replaced all RACK.dupthresh with RFC6657 ‘DupThresh’ for clarity.

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

Good question. We added the rationale in the draft.

If the reordering is temporary then a large adapted reordering window
would slow the loss recovery after the reordering disappears.
Therefore the inflated RACK.reo_wnd would persist for 16 loss
recoveries and after which it resets to its starting value, min_RTT /
4. The downside of resetting the reordering window is false loss
recovery if the reordering remains high. The rationale is to bound
such false recoveries up to once every 16 recoveries (less than 7%).

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

Good idea. Replaced/remove all “we” words

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

Good idea. We added algorithm diagram in the new ‘RACK-TLP high-level
design’ section
https://tools.ietf.org/html/draft-ietf-tcpm-rack-09#section-3.4

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

We rewrote section 4. Hopefully it’s more clear now.
https://tools.ietf.org/html/draft-ietf-tcpm-rack-09#section-4

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

changed to RACK.reo_wnd_incr

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

Sorry for the unclear text. We added a goal in the step and remove the
offload text. The tie breaker is not necessarily due to segmentation
offload.
https://tools.ietf.org/html/draft-ietf-tcpm-rack-09#section-6.2

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

Great suggestions. We have TLP in its whole section, and integrates
them from the beginning (starting at abstract)
https://tools.ietf.org/html/draft-ietf-tcpm-rack-09#section-3

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

Added SRTT in the https://tools.ietf.org/html/draft-ietf-tcpm-rack-09#section-5

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

Agreed. Now mentioned in
https://tools.ietf.org/html/draft-ietf-tcpm-rack-09#section-2.2

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

We decided to have a dedicated section about reordering window value
selection. It was scattered all over the document making readers hard
to puzzle up the policy. Hopefully this section
https://tools.ietf.org/html/draft-ietf-tcpm-rack-09#section-3.3.2
makes it clear how RACK reo_wnd can be used to support 3-dupack rule
easily.

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

We now provide the two more examples early on in the Design section.

>
> 12) Maybe rename section 8.3 to "Reasoning for basic reordering window size" (instead of " Adjusting the reordering window"...?
>
All reordering design rationale / policy is now in
https://tools.ietf.org/html/draft-ietf-tcpm-rack-09#section-3.3 to
avoid fragmented design reasonings.

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

agreed. removed

>
> 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.
removed this as RACK-TLP is described as one united algorithm

>
> 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?
>
Agreed. removed. There wasn't a research paper for RACK-TLP but we
probably should have published one :-)

> 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.
We are not sure if these are tied to security, or maybe there are
specific issues we are missing?

>
> 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
>
>
>
>
> _______________________________________________
> tcpm mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/tcpm

---

From: Yuchung Cheng <[email-masked]>
Date: Mon, 13 Jul 2020 15:06:21 -0700

On Sat, Apr 4, 2020 at 9:28 AM Theresa Enghardt <[email-masked]> wrote:

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

Thank you for such an excellent super detailed review. Sorry for taking

such a long time to respond. We have made significant edits to our draft

and published a newer version.

detailed responses inline.

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

We like your suggestion and want to make further changes to clarify. We
rewrote the first couple sections and split into existing mechanisms, their
shortcoming in modern networks, and the design rationale of RACK-TLP.

> Please consider adding a half-sentence here explaining what a tail drop

> is to make the document more accessible.

We avoid that confusing word 'tail drop' altogether now b/c it may mean
different things in different context (router queues vs application data
frame). instead we describe the drop behavior directly "Packet drops at the
end of an application data flight. "

> “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?

As other reviewers have also reflected this is unclear and not
informational. We remove it.

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

Good suggestion. We now have a whole new subsection 'Reordering resilience
with a time threshold' detailing the pros and cons of RACK design.

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

Other reviewers have expressed similar comments. We now mention them
together in abstract and the high level design section together, along with
their relationship in the Requirement section

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

This gets cleaned up now by removing section 9 entirely and rewriting the
design section. Please take a look.

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

To help explain, we added a new subsection 'An Example of RACK-TLP in
Action: RTO' in the design, along with a subsection 'Upon RTO expiration'
in the RACK detailed algorithm section. Let us know if that helps.

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

We removed this unclear sentence since it's really not critical to explain
RACK reordering. We replace this with a new subsection of 'RACK reordering
window adaptation'

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

Sorry for the messy text. We split RACK into the general design plus a
specific section on reordering. We do not claim the reordering window is a
new concept as many people have proposed similar idea. Rather we explain
the new part is to combine the per-segment timestamping and reordering
window to achieve better loss / reordering handling. This is detailed in
the beginning of the new 'RACK-TLP high-level design' section

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

Agreed the text is confusing without reading the code. The new subsection
'RACK reordering window adaptation explicitly lists the rules of 3-dupacks
and reordering window.

When the reordering window is zero, RACK will mark the first unacked packet
lost on the first DUPACK. The rule is to hold up until the third DUPACK (or
3 MSS worth of bytes been sacked).

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

We removed this sentence. What it meant originally was that at startup,
RACK tends to error on the side of spurious retransmission with a lower
initial reordering window. Since the majority of flows are short, the
spurious rtx isn't a big concern if their paths have high reordering. But
we realize the following sentences 'For short flows, the low initial ...'
delivers that message already.

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

After reviewing it again, we realize it does not help as there are many
ways to achieve load-balancing which is out of scope of this document. So
we remove this sentence.

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

Added

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

We replaced all references of a packet carrying sequence numbers to
'segment', as Gorry suggested.

We added Segment.end_seq definition. Thanks for catching that

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

Sure we now split into per-connection variables and per segment variables
and grouped related states together.

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

We added a SHOULD: the sender SHOULD track a simple ...

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

For spuriously retransmitted packet, the ACK with TCP timestamp can still
generate a valid RTT measurement, hence it should be incorporated in the
min RTT update in Step 1.

note that in Step 2, RACK.min_RTT is not updated as it is updated in Step
1. The RTT that's not updated is the RACK.rtt, which keeps the most recent
RTT measurement.

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

Sure, we've reversed the text order to match the pseudo code better

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

Sorry about the verbose confusing text. We replaced it with "Note that the
second check is a heuristic when Timestamp option is either not available
or when the round trip time is less than the timestamp clock granularity.

> Step 3:

> Maybe consider substituting “false retransmission” with “spurious

> retransmission”, as these terms seem to refer to the same concept.

done. replaced all such words throughput

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

SRTT is now referenced at the Definitions section

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

We now clarified that in the subsection of the design rationale section

'RACK reordering window adaptation'

'If the connection supports Duplicate Selective Acknowledgement (DSACK)
information [RFC2883], the RACK reordering window SHOULD leverage that to
adaptively estimate the duration of reordering events.'

> The pseudocode says that RACK.min_RTT is updated here - but wasn't this

> step 1 already? Or is RACK.min_RTT updated twice?

Good catch. We removed the redundant pseudo code line.

> "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?

Changed to RECOMMENDED now. A few reviewers made the same comment too.

"For timely loss detection, the sender is RECOMMENDED to install a
reordering settling timer. This timer expires at the earliest moment when
RACK would conclude that all the unacknowledged segments within the
reordering window were lost."

> "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?

We removed this sentence entirely since it is now obsolete. The reordering
window setting is now discussed in the new design rationale section.

> s/RACK.time_ts/RACK.xmit_ts/

Nice catch, revised.

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

Great idea. Added "checking only segments that have been sent after
RACK.xmit_ts is more efficient than scanning the entire SACK scoreboard,
especially when the inflight is large. " at the beginning of the paragraph
now.

> Section 7.3, Tail Loss Probe

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

> separate algorithm?

We like that suggestion. So TLP is now a whole separate section

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

This section should be about TLP details. So we've integrated the RTO
causes into the new Motivation subsection upfront.

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

We've changed and moved the example to a RACK-TLP at the "An Example of
RACK-TLP in Action: Fast Recovery" subsection

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

We renamed these variables to follow the styles of RACK variable names.

> (editorial) The PTO definition should go on a new line, otherwise it's

> easy to overlook.

Fixed

> 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”?

yes

> "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?

Added an explanation. It’s a value used by Linux for many years, likely
inherited from BSD (but not certain).

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

Added

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

Thanks for catching this -- we decided that there should be one outstanding
TLP regardless of new or retransmission. This is safer from congestion
control point of view. As a result we’ve modified this part of the
algorithm. Therefore PTO can fire but not send any new (or old) data, by
the guard of TLP.end_seq.

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

The pseudo code TLP_send_probe has that (in the bottom)

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

This text is removed since TLP is now restricted to one outstanding probe
at most.

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

We removed all the phases words for clarity

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

It was. Now we have folded this part into the main TLP algorithm / Section.

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

Your comments suggest the TLP algorithm part is poorly written. We’ve
completely rewritten it so hopefully it’s easier to read. The part that was
confusing is the TLP recovery is focused on a special case when only one
(last) packet was lost and repaired by TLP. This triggers particular type
of ACKs depending on DSACK support or not. Let us know if the new section
helps.

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

We have revised and changed to SHOULD

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

1msec was used in the older drafts that we've removed since. We've removed
all mentions of it.

> "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"?

again old text while RACK timer was not (yet) introduced. s/any timer/RTO
recovery/

> Section 8.3, Adjusting the reordering window:

> "... the Performance evaluation section ..."

> Maybe consider adding a reference to this section.

This subsection is now folded into the detailed RACK algorithm to give the
rationale behind RACK.min_RTT/4 to avoid forward reference.

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

We've moved this part and folded into the new 'High level design' section
of RACK-TLP

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

Sorry about the confusion. This text was written before we baked the
3-dupack rule into the RACK reordering window without a proper update here.
We've revised to

"If the packet losses happen after the reordering window has been increased
by DSACK, RACK-TLP may take longer to detect losses than the pure
DUPACK-counting approach. In this case TCP with pure loss-based congestion
control may continue to increase the congestion window upon receiving ACKs
during this time, making the sender more aggressive."

To further clarify: RACK-TLP in theory does NOT need to respect the
3-DUPACK rule. The 3-DUPACK rule is added simply for better compatibility.

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

yes we make this clear in the Requirement section now.

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

Sorry about the confusion. Again the step 3 was mis-cited from a very old
version of the draft. We've cleaned this part up.

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

We have expanded the experiment and eventually rolled out RACK-TLP to the
entire Google (inside and Internet-facing) a few years ago. The results
were fairly similar to the initial smaller-scale experiment. The values
used in this draft is based on the result.

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

Good point. Revised.

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

We (all the authors) sincerely appreciate your superb thorough and detailed
review!

---

From: Yuchung Cheng <[email-masked]>
Date: Tue, 14 Jul 2020 17:35:34 -0700

On Mon, Apr 6, 2020 at 8:24 AM Gorry Fairhurst <[email-masked]> wrote:
>
> 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.
Thanks for your review and comments. Sorry about the late response.
We've revised the documents to remove all wordings about the frequency or
population of issues we've observed, such as "prevalent reordering ...".
https://tools.ietf.org/html/draft-ietf-tcpm-rack-09

Instead we put an explicit high-level design section to focus on the actual
problems this draft aims to improve / fix, without claiming these problems
are common or not, because they do vary across networks.

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

We rewrote sections 1-3 to clarify the motivation, the design goals,
and the rationale for these changes, plus a section to
illustrate/overview the protocol.
Mostly we removed “we” and other claims based on unpublished data or
anecdotal evidence.

> —
> 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?
We removed the confusing word "tail" drops. instead a new subsection to
explain the problems more precisely
https://tools.ietf.org/html/draft-ietf-tcpm-rack-09#section-2.2

> —
> 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?
removed all wordings of "we", but it means the authors. This sentence
is removed in the new revision entirely.

> —
> 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?
the pattern of reordering. we put down two specific cases to explain:
https://tools.ietf.org/html/draft-ietf-tcpm-rack-09#section-3.3.1

> —
> 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
removed this sentence. instead we compared their pros & cons in the
https://tools.ietf.org/html/draft-ietf-tcpm-rack-09#section-8.1
https://tools.ietf.org/html/draft-ietf-tcpm-rack-09#section-8.2

> —
> This is confusing:
> “Also, these algorithms, including RFCs, rarely address the
>     interactions with other algorithms.  “
> - Which? and why this this important?

For all comments above. We agree this section was not clear and some
statements lack data citations.
As mentioned earlier we divided into a motivation section and a design
section to avoid conflating the two.
https://tools.ietf.org/html/draft-ietf-tcpm-rack-09#section-2.2

> —
> 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?

Sure we moved the terminology section to the beginning.

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

all packets are replaced with segments (if appropriate).

> —
> “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”.
re:annoying claims -- explained earlier. We try to be as thorough as
possible to remove all the adjectives in the new revision.

> ——
> “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.
https://tools.ietf.org/html/draft-ietf-tcpm-rack-09#section-3.1
is this better?
"...  For each ACK received, the sender
   calculates the latest RTT measurement (if eligible) and adjusts the
   expiration time of every segment sent but not yet delivered.  If a
   segment has expired, RACK marks it lost."

> —
> “hurts” performance. Could be better phrased.
Section motivation tries to be more precise on the latency difference
with / without RACK-TLP

> —
>   “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.
Fair enough. We removed this since it's more or less out of scope of RACK-TLP

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

In order to be more clear about the reordering design of RACK-TLP, we made
an explicit new section
https://tools.ietf.org/html/draft-ietf-tcpm-rack-09#section-3.3.1
please let us know if that looks more clear

> - —
> “An end-to-end transport protocol cannot tell immediately whether a
>     hole is reordering or loss. “
> - Please explain hole before using.
removed the colloquial word "hole". new text

"Upon receiving an ACK indicating an out-of-order data delivery, a
   sender cannot tell immediately whether that out-of-order delivery was
   a result of reordering or loss. "

> —
> “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?
Nice catch. The sender is assumed to implement RFC6298 and estimate RTT.
But in case the sender does not know (e.g. syn retransmits w/o TS),
we advise to keep reo_wnd 0 w/o further better choice so it mimics old 3-dupack
https://tools.ietf.org/html/draft-ietf-tcpm-rack-09#section-3.3.2

"1. If the sender has observed some reordering since the connection
       was established, then the RACK reordering window SHOULD be set to
       a small fraction of the round-trip time, or zero if no round-trip
       time estimate is available."

> —
> “ 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.
That's a good idea. We consolidated all reordering window requirements into
https://tools.ietf.org/html/draft-ietf-tcpm-rack-09#section-3.3.2

> —
> “   RACK does not need any change on the receiver.”
> - OK. However, there is a requirement that the receiver reports loss
> using SACK.
added that to be clear

> —
> “ RACK.dsack" indicates if a DSACK option .”
> - DSACK not mentioned until here. Should you cite DSACK [RFC3708] ?
cited now in -09

>
> ---
>
> “The RECOMMENDED value for  WCDelAckT is 200ms.”
> -Why this value? is this linked to TCP Delayed ACK value?
yes it's now named TLP.max_ack_delay to be explicit.
200ms originated from Linux which we believe (but not are not certain)
came from old BSD.

but this value is fairly inadequate in modern networking as some
recent discussions reveal.
so we changed to to

Third, when FlightSize is one segment, the sender MAY inflate PTO by
   TLP.max_ack_delay to accommodate a potential delayed acknowledgment
   and reduce the risk of spurious retransmissions.  The actual value of
   TLP.max_ack_delay is implementation-specific.

to avoid hard-coding yet another number that'd become obsolete soon

> —
> “We have evaluated using the smoothed RTT”
> …. where is the data?
Not published

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

We removed this. In the abstract we make it clear what RACK is intended to
be (an alternative to conventional DUPACK-counting approach).

> -
> 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
I am not sure what to add to the section. "This is an informative section, ..."
>
> ---
> “"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.
sure removed

> —
> “"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.
The requirement is clear to me. but here is our newer text to hopefully
make the reason clear to others:
"After attempting to send a loss probe, regardless of whether a loss
   probe was sent, the sender MUST re-arm the RTO timer, not the PTO
   timer, if FlightSize is not zero.  This ensures RTO recovery remains
   the last resort if TLP fails."

> ---
> NiTs:
> “eg.” should be “e.g.,”
fixed

>
> “is the easiest way to get a network to go faster.”
> - This needs to be explained better.
This is over-simplified. so we remove this.

> ---
> “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!
Fine. the may is not intended to be the "MAY". We *do not* want to
encourage reordering.

Here is our revised text on excessive reordering:
"However, the fact that the initial reordering window is low, and the
   reordering window's adaptive growth is bounded, means that there will
   continue to be a cost to reordering to disincentivize excessive
   network reordering over highly disjoint paths.  For such networks
   there are good alternative solutions, such as MPTCP."
https://tools.ietf.org/html/draft-ietf-tcpm-rack-09#section-3.3.2

---

From: Yuchung Cheng <[email-masked]>
Date: Wed, 15 Jul 2020 12:29:22 -0700

On Mon, Apr 6, 2020 at 3:54 PM Praveen Balasubramanian
<pravb=[email-masked]> wrote:
>
> 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.

Good idea. We have added
"   2.  For each data segment sent, the sender MUST store its most recent
       transmission time with a timestamp whose granularity that is
       finer than 1/4 of the minimum RTT of the connection.  At the time
       of writing, microsecond resolution is suitable for intra-
       datacenter traffic and millisecond granularity or finer is
       suitable for the Internet.  Note that RACK-TLP can be implemented
       with TSO (TCP Segmentation Offload) support by having multiple
       segments in a TSO aggregate share the same timestamp."
https://tools.ietf.org/html/draft-ietf-tcpm-rack-09#section-4

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

yes the old text was being mis-intepreted as RACK encourages
reordering so we tried our best to make that clear the opposite is
true, toward the end of a new "reordering design" sub-section
https://tools.ietf.org/html/draft-ietf-tcpm-rack-09#section-3.3

>
> >>temporarily override the reorder window
> Nit: Let's consistently call this reordering window throughout the draft.
great idea. we have now renamed all different names to "RACK
reordering window, RACK.reo_wnd" to be specific throughout, instead of
"reordering settle time", "reordering time window", etc

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

s.g. s/MAY/RECOMMENDED in step 5 of
https://tools.ietf.org/html/draft-ietf-tcpm-rack-09#section-6.2

For timely loss detection, the sender is RECOMMENDED to install a
   reordering timer.  This timer expires at the earliest moment when
   RACK would conclude that all the unacknowledged segments within the
   reordering window were lost.

>
> >>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.
fixed typo and removed the RECOMMENDED

>
> 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?
TLP is 100% compatible with LT. We have some text on this now

https://tools.ietf.org/html/draft-ietf-tcpm-rack-09#section-7.2

7.2.  Scheduling a loss probe

   The sender schedules a loss probe timeout (PTO) to transmit a segment
   during the normal transmission process.  The sender SHOULD start or
   restart a loss probe PTO timer after transmitting new data (that was
   not itself a loss probe) or upon receiving an ACK that cumulatively
   acknowledges new data, unless it is already in fast recovery, RTO
   recovery, or the sender has segments delivered out-of-order (i.e.
   RACK.segs_sacked is not zero).  These conditions are excluded because
   they are addressed by similar mechanisms, like Limited Transmit
   [RFC3042], the RACK reordering timer, and F-RTO [RFC5682].

> 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.
     TLP requires RACK which requires SACK. TLP on non-sack is in
theory possible but has very limited benefit. It can repair the single
loss well, but consider this TLP w/ neither RACK & SACK:
    t0: send 10 packets (P1...P10, lost all except number 8)
    t1: dupack by P8
    t2: send TLP of P10
    t3: dupack by P10
    --> ? still RTO given that only two dupacks are received.

We decided early on it's no point to complicate TLP further to support
non-sack, given sack is widely deployed. the non-sack deserves to stay
unoptimized (their choice).

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

---

From: Theresa Enghardt <[email-masked]>
Date: Fri, 24 Jul 2020 13:16:05 -0700

Dear Yuchung,

Thank you for the reply and for posting a new draft.

I read the current version -09 and find that it has greatly improved 
clarity and readability.

This resolves most of my comments.

Just one reply and a few additional minor comments:

On 2020-07-13 15:06, Yuchung Cheng wrote:

> […] The new
> subsection 'RACK reordering window adaptation explicitly lists the
> rules of 3-dupacks and reordering window.
> When the reordering window is zero, RACK will mark the first unacked
> packet lost on the first DUPACK. The rule is to hold up until the
> third DUPACK (or 3 MSS worth of bytes been sacked).

This helps, thanks.

Reading the new Section 3.3.2, I'm wondering about the following though:

"If the sender has not observed any reordering since the
        connection was established, then the RACK reordering window
        SHOULD be zero in either of the following cases: […]

        1.  After learning that three segments have been delivered out of
            order (e.g. receiving 3 DUPACKs per [RFC5681]); in turn, this
            will cause the RACK loss detection logic to trigger fast
            recovery.

        2.  During fast recovery or RTO recovery."

Does this SHOULD mean that the implementation should set the reordering 
window to 0 under either of the listed conditions?
Because I fail to see how the two listed conditions mean that that "the 
sender has not observed any reordering".
Or SHOULD the sender set the reordering window to 0 if either it hasn't 
observed any reordering, or one of the listed conditions are true.

An alternative (and probably false) way of reading this would be that 
the SHOULD just means that the sender should expect the reordering 
window to be 0.
But it doesn't seem logical to me to use SHOULD in this way.

(This is probably clarified later in the detailed algorithm, but it 
would be nice to have this phrased clearly in this Section as well.)

In Section 3.3.2, I fail to understand or parse the following sentence:
             The RACK reordering window SHOULD leverage that to 
adaptively
             estimate the duration of reordering events, if the receiver 
uses
             Duplicate Selective Acknowledgement (DSACK) [RFC2883].

The RACK reordering window SHOULD leverage what? (If "that" refers to 
the sentence before, SRTT, then the connection to DSACK is not obvious 
to me. So I think it's good to substitute "that" here.)

Nits / Editorial comments:

In Section 2.2 and Section 3.3.2., the numbered list goes "1.", "1.", 
"2.", but should be "1.", "2.", "3.".

In Section 4, list item 2. might have an unnecessary "that":
"[…] timestamp whose granularity that is finer than 1/4 of the minimum 
RTT of the connection."

Best,
Theresa

---

From: Yuchung Cheng <[email-masked]>
Date: Fri, 24 Jul 2020 16:45:53 -0700

> I read the current version -09 and find that it has greatly improved
thanks for taking another look!

yes

> observed any reordering, or one of the listed conditions are true.

agree this can be better clarified. do you think the following is more clear:

1. The reordering window SHOULD be zero if no reordering has been
    observed on the connection so far, plus 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.
   Otherwise the reordering window SHOULD start from a small fraction of the
    round trip time, or zero if no round trip time estimate is available.

2.  The RACK reordering window SHOULD adaptively increase if the
     sender receives the Duplicate Selective Acknowledgement (DSACK) [RFC2883],
     suggesting the window is too small which causes spurious retransmission.

3.  The RACK reordering window MUST be bounded and this bound SHOULD
     be SRTT.

(The text after this needs to be adjusted for the new bullet numbers).

> to me. So I think it's good to substitute "that" here.)
sorry that=DSACK. sentence got cut out accidentally. Does the proposed
new text in my earlier comment look better?

> "2.", but should be "1.", "2.", "3.".
will fix
> "[…] timestamp whose granularity that is finer than 1/4 of the minimum
will fix.

---

From: Theresa Enghardt <[email-masked]>
Date: Fri, 24 Jul 2020 17:10:04 -0700

Hi Yuchung,

See inline:

On 2020-07-24 16:45, Yuchung Cheng wrote:

>> […]

This is much clearer, thanks!

Additionally, I suggest to change "The reordering window SHOULD be zero" 
to "The reordering window SHOULD be set to zero", or to prefix the 
entire list with a sentence like "More specifically, the sender sets the 
reordering window as follows:".

> new text in my earlier comment look better?

Yes, this is clearer to me now.

I would suggest the following slight changes to this sentence though:

OLD:
"The RACK reordering window SHOULD adaptively increase if the sender 
receives the Duplicate Selective Acknowledgement (DSACK) [RFC2883], 
suggesting the window is too small which causes spurious 
retransmission."

NEW:
"The RACK reordering window SHOULD adaptively increase if the sender 
receives a Duplicate Selective Acknowledgement (DSACK) [RFC2883], 
suggesting the window is too small, which causes spurious 
retransmission."

Best,
Theresa

---

From: Yuchung Cheng <[email-masked]>
Date: Sun, 26 Jul 2020 10:11:53 -0700

On Fri, Jul 24, 2020 at 5:10 PM Theresa Enghardt <[email-masked]> wrote:
> > retransmission.
Sure we'll revise accordingly.