Subject: [quicwg/base-drafts] Editorial cleanup of Recovery (#1957)

From: ianswett <[email-masked]>
Date: Sat, 03 Nov 2018 04:36:18 -0700

Some small changes I noticed when re-reading the entire doc.
You can view, comment on, or merge this pull request online at:

  https://github.com/quicwg/base-drafts/pull/1957

-- Commit Summary --

  * Editorial cleanup of Recovery

-- File Changes --

    M draft-ietf-quic-recovery.md (120)

-- Patch Links --

https://github.com/quicwg/base-drafts/pull/1957.patch
https://github.com/quicwg/base-drafts/pull/1957.diff

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/1957

---

From: ianswett <[email-masked]>
Date: Sat, 03 Nov 2018 04:52:38 -0700

@ianswett pushed 1 commit.

a0eb5f6  Update draft-ietf-quic-recovery.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1957/files/135b73759c12dc38435e50d25c14f9ddf9f773fb..a0eb5f67562010f93249c252d6039c62b0be6baa

---

From: MikkelFJ <[email-masked]>
Date: Sat, 03 Nov 2018 04:59:47 -0700

mikkelfj commented on this pull request.

> @@ -163,17 +163,18 @@ across packet number spaces.
 
 ### Monotonically Increasing Packet Numbers
 
-TCP conflates transmission sequence number at the sender with delivery sequence
-number at the receiver, which results in retransmissions of the same data
-carrying the same sequence number, and consequently to problems caused by
-"retransmission ambiguity".  QUIC separates the two: QUIC uses a packet number
-for transmissions, and any application data is sent in one or more streams,
-with delivery order determined by stream offsets encoded within STREAM frames.
-
-QUIC's packet number is strictly increasing, and directly encodes transmission
-order.  A higher QUIC packet number signifies that the packet was sent later,
-and a lower QUIC packet number signifies that the packet was sent earlier.  When
-a packet containing frames is deemed lost, QUIC rebundles necessary frames in a
+TCP conflates transmission order at the sender with delivery order at the
+receiver, which results in retransmissions of the same data carrying the same
+sequence number, and consequently to "retransmission ambiguity".  QUIC

```suggestion
sequence number, and consequently leads to "retransmission ambiguity".  QUIC
```

> @@ -236,7 +237,9 @@ underestimation of min RTT, which in turn prevents underestimating smoothed RTT.
 Ack-based loss detection implements the spirit of TCP's Fast Retransmit
 {{?RFC5681}}, Early Retransmit {{?RFC5827}}, FACK, and SACK loss recovery
 {{?RFC6675}}. This section provides an overview of how these algorithms are
-implemented in QUIC.
+implemented in QUIC.  Though both time-based loss detection and early retransmit
+use a timer, they're part of Ack-based detection because they do not use a timer

```suggestion
use a timer, they are part of Ack-based detection because they do not use a timer
```

> @@ -450,11 +458,10 @@ this packet adds network load without establishing packet loss.
 ## Generating Acknowledgements
 
 QUIC SHOULD delay sending acknowledgements in response to packets, but MUST NOT
-excessively delay acknowledgements of packets containing frames other than ACK.
-Specifically, implementations MUST attempt to enforce a maximum
-ack delay to avoid causing the peer spurious timeouts.  The maximum ack delay
-is communicated in the `max_ack_delay` transport parameter and the default
-value is 25ms.
+excessively delay acknowledgements of retransmittable packets. Specifically,
+implementations MUST attempt to enforce a maximum ack delay to avoid causing
+the peer spurious timeouts.  The maximum ack delay is communicated in the
+`max_ack_delay` transport parameter and the default value is 25ms.

Just a stray thought: would some kind of hysteresis be of relevance? E.g. receiver should allow for 10% overrun of ack delay? Or perhaps this is already taken into account.

> @@ -501,13 +508,13 @@ frame may be saved.  When a packet containing an ACK frame is acknowledged, the
 receiver can stop acknowledging packets less than or equal to the largest
 acknowledged in the sent ACK frame.
 
-In cases without ACK frame loss, this algorithm allows for a minimum of 1 RTT of
-reordering. In cases with ACK frame loss, this approach does not guarantee that
-every acknowledgement is seen by the sender before it is no longer included in
-the ACK frame. Packets could be received out of order and all subsequent ACK
-frames containing them could be lost. In this case, the loss recovery algorithm
-may cause spurious retransmits, but the sender will continue making forward
-progress.
+In cases without ACK frame loss, this algorithm allows for a minimum of 1 RTT
+of reordering. In cases with ACK frame loss and reordering, this approach does
+not guarantee that every acknowledgement is seen by the sender before it is no
+longer included in the ACK frame. Packets could be received out of order and
+all subsequent ACK frames containing them could be lost. In this case, the
+loss recovery algorithm may cause spurious retransmits, but the sender will
+continue making forward progress.

```suggestion
continue to make forward progress.
```

> @@ -878,13 +881,9 @@ Pseudocode for OnLossDetectionTimeout follows:
 
 ### Detecting Lost Packets
 
-Packets in QUIC are only considered lost once a larger packet number in
-the same packet number space is acknowledged.  DetectLostPackets is called
-every time an ack is received and operates on the sent_packets for that
-packet number space.  If the loss detection timer expires and the loss_time
-is set, the previous largest acked packet is supplied.
-
-#### Pseudocode
+DetectLostPackets is called every time an ack is received and operates on

```suggestion
DetectLostPackets is called every time an ACK is received and operates on
```

> @@ -878,13 +881,9 @@ Pseudocode for OnLossDetectionTimeout follows:
 
 ### Detecting Lost Packets
 
-Packets in QUIC are only considered lost once a larger packet number in
-the same packet number space is acknowledged.  DetectLostPackets is called
-every time an ack is received and operates on the sent_packets for that
-packet number space.  If the loss detection timer expires and the loss_time
-is set, the previous largest acked packet is supplied.
-
-#### Pseudocode
+DetectLostPackets is called every time an ack is received and operates on
+the sent_packets for that packet number space. If the loss detection timer
+expires and the loss_time is set, the previous largest acked packet is supplied.

```suggestion
expires and the loss_time is set, the previous largest acknowledged packet is supplied.
```

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/1957#pullrequestreview-171325030

---

From: MikkelFJ <[email-masked]>
Date: Sat, 03 Nov 2018 05:00:21 -0700

or ACK-based ?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/1957#discussion_r230553023

---

From: ianswett <[email-masked]>
Date: Sat, 03 Nov 2018 05:03:45 -0700

655e859  Update draft-ietf-quic-recovery.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1957/files/a0eb5f67562010f93249c252d6039c62b0be6baa..655e859a04fe66eabbe7d5a08ac57d26cc9b3d8f

---

From: ianswett <[email-masked]>
Date: Sat, 03 Nov 2018 05:06:31 -0700

ianswett commented on this pull request.

The current model is the receiver communicates it's max ack delay.  The receiver is in the best place to determine how late alarms are likely to fire, so I think they should include that in the value they send.  That being said, it's probably worth stating that specifically.  Possibly in the transport draft?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/1957#discussion_r230553172

---

From: ianswett <[email-masked]>
Date: Sat, 03 Nov 2018 05:07:41 -0700

982bb91  Update draft-ietf-quic-recovery.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1957/files/655e859a04fe66eabbe7d5a08ac57d26cc9b3d8f..982bb918174feecec1d254e43f83ddeec0459ec7

---

From: ianswett <[email-masked]>
Date: Sat, 03 Nov 2018 05:07:45 -0700

f61763e  Update draft-ietf-quic-recovery.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1957/files/982bb918174feecec1d254e43f83ddeec0459ec7..f61763e88b21a02f57273382495e539653e2969d

---

From: ianswett <[email-masked]>
Date: Sat, 03 Nov 2018 05:08:05 -0700

4118fb8  Update draft-ietf-quic-recovery.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1957/files/f61763e88b21a02f57273382495e539653e2969d..4118fb885ef9a9cb612b8535894f446be5c06d0c

---

From: ianswett <[email-masked]>
Date: Sat, 03 Nov 2018 05:15:20 -0700

e8bd1b8  Update draft-ietf-quic-recovery.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1957/files/4118fb885ef9a9cb612b8535894f446be5c06d0c..e8bd1b8442bddc084e4b172bab4c2f65d86f7f1d

---

From: MikkelFJ <[email-masked]>
Date: Sat, 03 Nov 2018 05:16:00 -0700

Makes sense

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/1957#discussion_r230553400

---

From: ianswett <[email-masked]>
Date: Sat, 03 Nov 2018 05:25:52 -0700

661ee92  Update draft-ietf-quic-recovery.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1957/files/e8bd1b8442bddc084e4b172bab4c2f65d86f7f1d..661ee92884bb3fdff7bbd7a2b129b8b14656967e

---

From: MikkelFJ <[email-masked]>
Date: Sat, 03 Nov 2018 05:27:55 -0700

> @@ -777,16 +790,13 @@ Pseudocode for OnAckReceived and UpdateRtt follow:
 
 ### On Packet Acknowledgment
 
-When a packet is acked for the first time, the following OnPacketAcked function
-is called.  Note that a single ACK frame may newly acknowledge several packets.
-OnPacketAcked must be called once for each of these newly acked packets.
+When a packet is acknowledged for the first time, the following OnPacketAcked
+function is called.  Note that a single ACK frame may newly acknowledge several
+packets. OnPacketAcked must be called once for each of these newly acked

```suggestion
packets. OnPacketAcked must be called once for each of these newly acknowledged
```

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/1957#pullrequestreview-171325993

---

From: ianswett <[email-masked]>
Date: Sat, 03 Nov 2018 13:29:43 -0700

5d965ca  Update draft-ietf-quic-recovery.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1957/files/661ee92884bb3fdff7bbd7a2b129b8b14656967e..5d965ca87cb8d6f20f550068c50acd7443ea3f44

---

From: ianswett <[email-masked]>
Date: Sat, 03 Nov 2018 13:30:11 -0700

I think I prefere making, since it's more active?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/1957#discussion_r230567039

---

From: ianswett <[email-masked]>
Date: Sat, 03 Nov 2018 13:30:43 -0700

PR #1960 merged already fyi.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/1957#discussion_r230567059

---

From: MikkelFJ <[email-masked]>
Date: Sat, 03 Nov 2018 13:36:09 -0700

To me it seems more like spoken language, but either will do. 

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/1957#discussion_r230567224

---

From: MikkelFJ <[email-masked]>
Date: Sat, 03 Nov 2018 13:38:03 -0700

Yes I noticed, looks good, thanks.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/1957#discussion_r230567271

---

From: ianswett <[email-masked]>
Date: Sat, 03 Nov 2018 13:50:28 -0700

810bedd  Update draft-ietf-quic-recovery.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1957/files/5d965ca87cb8d6f20f550068c50acd7443ea3f44..810bedd68d2ce743c14fe22868321f6b3af769f0

---

From: ianswett <[email-masked]>
Date: Sat, 03 Nov 2018 15:33:41 -0700

49aedc4  Update draft-ietf-quic-recovery.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1957/files/810bedd68d2ce743c14fe22868321f6b3af769f0..49aedc49577402bcdab0ae6952a8b178bb069cce

---

From: janaiyengar <[email-masked]>
Date: Sat, 03 Nov 2018 22:40:26 -0700

janaiyengar commented on this pull request.

-with delivery order determined by stream offsets encoded within STREAM frames.
-
-QUIC's packet number is strictly increasing, and directly encodes transmission
-order.  A higher QUIC packet number signifies that the packet was sent later,
-and a lower QUIC packet number signifies that the packet was sent earlier.  When
-a packet containing frames is deemed lost, QUIC rebundles necessary frames in a
+TCP conflates transmission order at the sender with delivery order at the
+receiver, which results in retransmissions of the same data carrying the same
+sequence number, and consequently leads to "retransmission ambiguity".  QUIC
+separates the two: QUIC uses a packet number to indicate transmission order,
+and any application data is sent in one or more streams, with delivery order
+determined by stream offsets encoded within STREAM frames.
+
+QUIC's packet number is strictly increasing within a packet number space,
+and directly encodes transmission order.  A higher QUIC packet number

```suggestion
and directly encodes transmission order.  A higher packet number
```

-
-QUIC's packet number is strictly increasing, and directly encodes transmission
-order.  A higher QUIC packet number signifies that the packet was sent later,
-and a lower QUIC packet number signifies that the packet was sent earlier.  When
-a packet containing frames is deemed lost, QUIC rebundles necessary frames in a
+TCP conflates transmission order at the sender with delivery order at the
+receiver, which results in retransmissions of the same data carrying the same
+sequence number, and consequently leads to "retransmission ambiguity".  QUIC
+separates the two: QUIC uses a packet number to indicate transmission order,
+and any application data is sent in one or more streams, with delivery order
+determined by stream offsets encoded within STREAM frames.
+
+QUIC's packet number is strictly increasing within a packet number space,
+and directly encodes transmission order.  A higher QUIC packet number
+signifies that the packet was sent later, and a lower QUIC packet number

```suggestion
signifies that the packet was sent later, and a lower packet number
```

-order.  A higher QUIC packet number signifies that the packet was sent later,
-and a lower QUIC packet number signifies that the packet was sent earlier.  When
-a packet containing frames is deemed lost, QUIC rebundles necessary frames in a
+TCP conflates transmission order at the sender with delivery order at the
+receiver, which results in retransmissions of the same data carrying the same
+sequence number, and consequently leads to "retransmission ambiguity".  QUIC
+separates the two: QUIC uses a packet number to indicate transmission order,
+and any application data is sent in one or more streams, with delivery order
+determined by stream offsets encoded within STREAM frames.
+
+QUIC's packet number is strictly increasing within a packet number space,
+and directly encodes transmission order.  A higher QUIC packet number
+signifies that the packet was sent later, and a lower QUIC packet number
+signifies that the packet was sent earlier.  When a packet containing
+retransmittable frames is deemed lost, QUIC rebundles necessary frames in a

```suggestion
retransmittable frames is detected lost, QUIC rebundles necessary frames in a
```

+use a timer, they are part of Ack-based detection because they do not use a

```suggestion
use a timer, they are part of ack-based detection because they do not use a
```

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/1957#pullrequestreview-171350160

---

From: Martin Thomson <[email-masked]>
Date: Sat, 03 Nov 2018 23:03:35 -0700

martinthomson commented on this pull request.

the word is "acknowledgment", which I appreciate is long and hard to say and type, but we should use the whole word unless we're talking about the frame.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/1957#discussion_r230576374

---

From: Martin Thomson <[email-masked]>
Date: Sun, 04 Nov 2018 00:55:28 -0700

@martinthomson pushed 1 commit.

14be70c  Reflow

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1957/files/49aedc49577402bcdab0ae6952a8b178bb069cce..14be70c550c13c3663dca4642b9835af2008e8c6

---

From: ianswett <[email-masked]>
Date: Sun, 04 Nov 2018 02:27:20 -0800

b3c8d5d  Update draft-ietf-quic-recovery.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1957/files/14be70c550c13c3663dca4642b9835af2008e8c6..b3c8d5dd19776663b309da4756afbcc9b5db20f2

---

From: ianswett <[email-masked]>
Date: Sun, 04 Nov 2018 02:27:52 -0800

38f4988  Update draft-ietf-quic-recovery.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1957/files/b3c8d5dd19776663b309da4756afbcc9b5db20f2..38f4988d2bf2c27148cf00b500062a6dcf132fc9

---

From: ianswett <[email-masked]>
Date: Sun, 04 Nov 2018 02:28:06 -0800

9d5a151  Update draft-ietf-quic-recovery.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1957/files/38f4988d2bf2c27148cf00b500062a6dcf132fc9..9d5a15192cb7ddd5cc20dda2904565cb83306548

---

From: ianswett <[email-masked]>
Date: Sun, 04 Nov 2018 02:30:34 -0800

Hmm, the naming of this is a good point, but I think that's best saved for a follow up change, since it's used in a good number of spots.  Also, I was thinking it's ACK like the frame, but I'm not sure we want to go to ACK-based loss detection everywhere?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/1957#discussion_r230583176

---

From: ianswett <[email-masked]>
Date: Sun, 04 Nov 2018 02:30:40 -0800

7adf53b  Update draft-ietf-quic-recovery.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1957/files/9d5a15192cb7ddd5cc20dda2904565cb83306548..7adf53b300a4992bdb6ed837dfc726704e47dfc0

---

From: ianswett <[email-masked]>
Date: Sun, 04 Nov 2018 15:37:06 -0800

0210603  Update draft-ietf-quic-recovery.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1957/files/7adf53b300a4992bdb6ed837dfc726704e47dfc0..021060398f8b2565cbfc45f6aef7158ccd94cebf

---

From: ianswett <[email-masked]>
Date: Sun, 04 Nov 2018 15:46:09 -0800

Merged #1957 into master.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/1957#event-1945085193