Re: [tcpm] I-D Action: draft-ietf-tcpm-prr-rfc6937bis-02.txt

[Yuchung Cheng <[e-mail]>]

Looks great to me. Thanks for the suggestions

On Mon, Nov 14, 2022 at 1:42 PM Neal Cardwell <[e-mail]> wrote:

> On Mon, Nov 14, 2022 at 3:43 PM Yuchung Cheng <ycheng=
> [e-mail]> wrote:
>
>> On Wed, Nov 9, 2022 at 3:13 PM Scheffenegger, Richard <[e-mail]>
>> wrote:
>> >
>> > Chairs,
>> >
>> > Matt, Yuchung,
>> >
>> > This is a summary of my Read of 6937bis-02.
>> >
>> > I found mostly editorial only nit-picks, and a few clarification
>> > questions - if my read of the heuristic is correct now.
>> >
>> > In short, I'm happy with the document, there are few editorial nits; I
>> > would like a very concrete example of a SafeACK, even though the last
>> > and final description appears to confirm what I assumed a SafeACK would
>> > be. But then again, still not 100% sure.
>> >
>> > Reason for my question: "does not indicate prior fast retransmit has
>> > been lost" may indicate only partial ACKs which do *not* shrink a SACK
>> > hole *from the right*, or create a new SACK hole (e.g. splitting an old
>> > one) are elegible... But partial ACKs on non-SACK sessions would always
>> > be elegible?
>>
>> see my reply below
>>
>> >
>> > The forced fast retransmit may not be reflected in the PRR packet
>> > diagram - unless I didn't quite understand the values there...
>>
>> The examples do not include force retransmit examples. We'd clarify
>> that in the description of the examples to clarify.
>>
>> >
>> > Thanks for doing this important work!
>> Thanks for the detailed review!
>>
>>
>> >
>> > Other than the above (SafeAck), all my comments from 8.Feb 2021 and
>> > 23.Feb.2021 appear to have been addressed.
>> >
>> > Best regards,
>> >     Richard
>> >
>> >
>> >
>> > Abstract:
>> >
>> >  > PRR potentially replaces the Fast Recovery to regulate
>> >
>> > missing noun "Fast Recovery algorithm" (excessive deletion btwn -01/-02.
>> >
>> > Also, the abbreviations PRR, ssthresh are mentioned again in
>> > Introduction, so maybe leave the bracketed acronym out in the abstract.
>> >
>> > Sec 1 - maybe reference to RACK-TLP with LRD when LRD is mentioned the
>> > first time; the reference is further down below currently. There are
>> > other ways to do LRD (with SACK), but not in the RFC series.
>> >
>> > Sec 2 - [6675] is not a link.
>> >
>> > Sec 5 - update rfc793 to 9293?
>>
>> will address the nits above in the next rev. thanks
>>
>> >
>> > SafeACK - missing what "and the ACK does not indicate
>> >     further losses." actually means. No new SACK block ? (new sack
>> > scoreboard hole)?
>> >
>> > Sec 6 - SafeACK again: "and the ACK does not indicate
>> >     further losses." (concrete example?)
>> >
>> >
>> > Sec 7, PRR example... You describe the forced fast retransmit on the
>> > first transmission oppty. Shouldn't that shift the "N" transmissions
>> > here one notch to the left? (The ACK for "4").
>> >
>> > PRR
>> >     ack#   X  1  2  3  4  5  6  7  8  9 10 11 12 13 14 15 16 17 18 19
>> >     cwnd:    20 20 19 19 18 17 17 16 16 15 15 14 14 13 13 12 12 11 10
>> >     pipe:    19 19 18 18 17 17 16 16 15 15 14 14 13 13 12 12 11 11 10
>> >     sent:     N  N  R  N        N     N     N     N     N     N     N
>> no because forced retransmit only applies when the fast recovery
>> starts (i.e. prr_out = 0). by ack #4, prr_out is already 1 due to the
>> first "R".
>>
>> >
>> > Sec 9 - SafeAck seems to indicate, when SACK is in use, that no new SACK
>> > range is included (adding a new hole to the scoreboard). However, in a
>> > non-SACK case, due to the lack of this knowledge, wouldn't be all
>> > partial ACKs be considered to be SafeACKs?
>>
>> Yes when SACK is not used SafeACK will be true for all partial ACKs.
>> To clarify better, we'd add LRD requirement on SACK and non-SACK sup
>>
>> The current wording in the section "Changes from RFC 6937 is less
>> clear. We'd revise from
>> "For TCP, when the latest ACK advances snd.una and does not indicate
>> prior fast retransmit has been lost"
>> to
>> "For TCP, when the latest ACK advances snd.una and does not indicate
>> further new packet losses on both SACK and NonSACK"
>>
>
> FWIW, for this sentence I find the "on both SACK and NonSACK" phrasing a
> bit unclear. WDYT about something like:
>
> Current (prr-rfc6937bis-02):
>
>    The largest change since [RFC6937 <https://datatracker.ietf.org/doc/html/rfc6937>] is the introduction of a new
>    heuristic that uses good recovery progress (For TCP, when the latest
>    ACK advances snd.una and does not indicate prior fast retransmit has
>    been lost) to select which Reduction Bound.
>
>
> Suggested:
>
>    The largest change since [RFC6937 <https://datatracker.ietf.org/doc/html/rfc6937>] is the introduction of a new
>    "safeACK" heuristic that assesses recovery progress to select the
>
>    Reduction Bound. For TCP, an ACK is deemed a "safeACK" when
>
>    processing the ACK advances SND.UNA without the loss detection
>
>    algorithm detecting any new packet losses (this heuristic
>
>    applies to both SACK and non-SACK connections).
>
>
> neal
>
>
>