Subject: [quicwg/base-drafts] No more Retransmittable (#1211)

From: ianswett <[email-masked]>
Date: Thu, 15 Mar 2018 16:28:51 +0000 (UTC)

Fixes Issue #591
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * No more Retransmittable

-- File Changes --

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

-- Patch Links --

https://github.com/quicwg/base-drafts/pull/1211.patch
https://github.com/quicwg/base-drafts/pull/1211.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/1211

---

From: Sean Turner <[email-masked]>
Date: Thu, 15 Mar 2018 09:32:15 -0700

seanturner approved this pull request.

-- 
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/1211#pullrequestreview-104280647

---

From: Marten Seemann <[email-masked]>
Date: Thu, 15 Mar 2018 16:34:15 +0000 (UTC)

marten-seemann commented on this pull request.

 * Crypto handshake data is sent on stream 0, and uses the reliability
   machinery of QUIC underneath.
 
 * ACK frames contain acknowledgment information.  ACK frames contain one or more
-  ranges of acknowledged packets.
+  ranges of acknowledged packets and are not congestion controlled.

This now sounds like ACKs never count towards the bytes_in_flight. As far as I understand the rest of this PR, this only applies for ACK-only packets though, right?

-- 
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/1211#pullrequestreview-104280692

---

From: ianswett <[email-masked]>
Date: Thu, 15 Mar 2018 16:35:13 +0000 (UTC)

@ianswett pushed 1 commit.

5873e94  Marten's comment

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1211/files/0e51defddb62fc680d6f07dec7999b8354259e84..5873e947a2fc05d88b1670335f7dd0c7b62137f1

---

From: ianswett <[email-masked]>
Date: Thu, 15 Mar 2018 09:35:38 -0700

Thanks Marten, reverted to what it was before.

-- 
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/1211#issuecomment-373440676

---

From: Nick Banks <[email-masked]>
Date: Thu, 15 Mar 2018 09:41:30 -0700

nibanks commented on this pull request.

> @@ -538,9 +539,9 @@ max_ack_delay:
   RTT sample less than min_rtt.
 
 reordering_threshold:
-: The largest delta between the largest acked
-  retransmittable packet and a packet containing retransmittable frames before
-  it's declared lost.
+: The largest packet number gap between the largest acked
+  congestion controlled packet and smaller unacknowledged congestion

This is the first reference you have to `congestion controlled packet`. You haven't explicitly defined what this means. I assume it is just a packet that contains at least 1 congestion controlled frame, but I think it would be worth explicitly stating that above.

> @@ -538,9 +539,9 @@ max_ack_delay:
   RTT sample less than min_rtt.
 
 reordering_threshold:
-: The largest delta between the largest acked
-  retransmittable packet and a packet containing retransmittable frames before
-  it's declared lost.
+: The largest packet number gap between the largest acked
+  congestion controlled packet and smaller unacknowledged congestion
+  controlled packet before it is declared lost.

Why does `reordering_threshold` care about congestion-controlled or not? If you have the following outstanding packets:

1. ACK only
2. ACK only
3. ACK only
4. MAX_DATA only

And then you get an ACK for packet 4, should you not consider any of the previous packets as lost? I understand that marking a non-congestion controlled packet as lost might have no effect, but it is lost nonetheless, right?

-- 
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/1211#pullrequestreview-104282738

---

From: Marten Seemann <[email-masked]>
Date: Thu, 15 Mar 2018 16:52:42 +0000 (UTC)

reordering_threshold:

@nibanks You don't even save packets 1 to 3 (in the sent_packets), so when receiving the ACK for 4, you wouldn't even remember that you ever sent them.

-- 
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/1211#discussion_r174854612

---

From: janaiyengar <[email-masked]>
Date: Thu, 15 Mar 2018 16:53:56 +0000 (UTC)

janaiyengar commented on this pull request.

> @@ -100,8 +100,9 @@ interpretation of TCP loss detection mechanisms.
 Every packet may contain several frames.  We outline the frames that are
 important to the loss detection and congestion control machinery below.
 
-* Retransmittable frames are frames requiring reliable delivery.  The most
-  common are STREAM frames, which typically contain application data.
+* Congestion controlled frames are frames that count towards bytes in
+  flight and need acknowledgement.  The most common are STREAM frames,

hyphenate bytes-in-flight

> @@ -100,8 +100,9 @@ interpretation of TCP loss detection mechanisms.
 Every packet may contain several frames.  We outline the frames that are
 important to the loss detection and congestion control machinery below.
 
-* Retransmittable frames are frames requiring reliable delivery.  The most
-  common are STREAM frames, which typically contain application data.
+* Congestion controlled frames are frames that count towards bytes in

"frames are those"

> @@ -100,8 +100,9 @@ interpretation of TCP loss detection mechanisms.
 Every packet may contain several frames.  We outline the frames that are
 important to the loss detection and congestion control machinery below.
 
-* Retransmittable frames are frames requiring reliable delivery.  The most
-  common are STREAM frames, which typically contain application data.
+* Congestion controlled frames are frames that count towards bytes in

"congestion-controlled"

 reordering_threshold:

"unacknowledged congestion-controlled packet with a lower packet number"

-- 
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/1211#pullrequestreview-104287885

---

From: Nick Banks <[email-masked]>
Date: Thu, 15 Mar 2018 16:55:09 +0000 (UTC)

reordering_threshold:

Yes, I agree you don't *need* to save them, but if you want to, for purely a statistical measurement of loss, couldn't you remember them?

-- 
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/1211#discussion_r174855401

---

From: Marten Seemann <[email-masked]>
Date: Thu, 15 Mar 2018 10:00:34 -0700

reordering_threshold:

You *could* do that, but you'll probably have to be very careful about loss at the tails. I'm not sure if this would be an improvement over a measurement that just uses retransmittable packets though.

-- 
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/1211#discussion_r174857221

---

From: MikkelFJ <[email-masked]>
Date: Thu, 15 Mar 2018 17:26:34 +0000 (UTC)

mikkelfj commented on this pull request.

 reordering_threshold:

How would you deal with optimistic ACK attack if you forget? It is valid to ACK an unackable packet if it is together with other packets, right?

-- 
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/1211#discussion_r174865496

---

From: Mike Bishop <[email-masked]>
Date: Thu, 15 Mar 2018 10:37:48 -0700

MikeBishop commented on this pull request.

 reordering_threshold:

All packets except Reset are ackable, and so they are included in the ACK when it's sent.  ACK-only packets don't provoke an ACK in response, however.

> @@ -970,9 +971,9 @@ are described in this section.
 
 bytes_in_flight:
 : The sum of the size in bytes of all sent packets that contain at least
-  one retransmittable or PADDING frame, and have not been acked or
-  declared lost. The size does not include IP or UDP overhead.
-  Packets only containing ACK frames do not count towards byte_in_flight
+  one congestion controlled frame, and have not been acked or declared

hyphenate

> @@ -733,8 +734,8 @@ Pseudocode for SetLossDetectionAlarm follows:
 ~~~
  SetLossDetectionAlarm():
     // Don't arm the alarm if there are no packets with
-    // retransmittable data in flight.
-    if (num_retransmittable_packets_outstanding == 0):
+    // congestion controlled data in flight.

hyphenate

> @@ -711,10 +712,10 @@ response to 0RTT packets.
 
 #### Tail Loss Probe and Retransmission Alarm
 
-Tail loss probes {{?TLP}} and
-retransmission timeouts {{?RFC6298}} are an alarm based mechanism to recover
-from cases when there are outstanding retransmittable packets, but an
-acknowledgement has not been received in a timely manner.
+Tail loss probes {{?TLP}} and retransmission timeouts {{?RFC6298}}
+are an alarm based mechanism to recover from cases when there are
+outstanding congestion controlled packets, but an acknowledgement has

hyphenate

-- 
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/1211#pullrequestreview-104305758

---

From: ianswett <[email-masked]>
Date: Sat, 17 Mar 2018 13:52:05 +0000 (UTC)

ianswett commented on this pull request.

 reordering_threshold:

Thanks, done above.

-- 
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/1211#discussion_r175258719

---

From: ianswett <[email-masked]>
Date: Sat, 17 Mar 2018 13:56:57 +0000 (UTC)

7e50460  Hyphenation and suggestions

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1211/files/5873e947a2fc05d88b1670335f7dd0c7b62137f1..7e50460a0641cd48cf71461d2744bd2ad68379b7

---

From: ianswett <[email-masked]>
Date: Sat, 17 Mar 2018 06:58:28 -0700

5f9b1db  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/1211/files/7e50460a0641cd48cf71461d2744bd2ad68379b7..5f9b1db19a0097016d03b1db8e48b6688723248b

---

From: ianswett <[email-masked]>
Date: Sat, 17 Mar 2018 07:01:10 -0700

4ec6045  Congestion-controlled

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1211/files/5f9b1db19a0097016d03b1db8e48b6688723248b..4ec60455fbf66a6b13b6435f4141a1b91a3469ee

---

From: ianswett <[email-masked]>
Date: Sat, 17 Mar 2018 14:03:03 +0000 (UTC)

284b408  One more

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1211/files/4ec60455fbf66a6b13b6435f4141a1b91a3469ee..284b40808adb18de5f2e7933de1152bda542561b

---

From: ianswett <[email-masked]>
Date: Sat, 17 Mar 2018 14:09:38 +0000 (UTC)

e3f7edd  No more ackable

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1211/files/284b40808adb18de5f2e7933de1152bda542561b..e3f7edd407545d602e15f52ab8e1115e67eaab28

---

From: ianswett <[email-masked]>
Date: Sat, 17 Mar 2018 14:16:34 +0000 (UTC)

64a5920  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/1211/files/e3f7edd407545d602e15f52ab8e1115e67eaab28..64a59208ce13b60b3513696b3e0e37d388ac09d5

---

From: ianswett <[email-masked]>
Date: Sat, 17 Mar 2018 07:17:13 -0700

4879ee6  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/1211/files/64a59208ce13b60b3513696b3e0e37d388ac09d5..4879ee68cfc10d9fe3090483ff86f9a80458c567

---

From: ianswett <[email-masked]>
Date: Sat, 17 Mar 2018 14:24:19 +0000 (UTC)

a3cd1c5  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/1211/files/4879ee68cfc10d9fe3090483ff86f9a80458c567..a3cd1c5531b4336d49525eb66ee8318545bfb3cd

---

From: ianswett <[email-masked]>
Date: Sat, 17 Mar 2018 14:27:11 +0000 (UTC)

f1dcd18  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/1211/files/a3cd1c5531b4336d49525eb66ee8318545bfb3cd..f1dcd18dc71c794874eb83acf6dd73c0ba882acd

---

From: Nick Banks <[email-masked]>
Date: Sat, 17 Mar 2018 14:50:16 +0000 (UTC)

reordering_threshold:

@ianswett What are your thoughts here? Shouldn't all packets, congestion controlled or not, be capable of being lost and therefore be able to be detected as lost?

-- 
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/1211#discussion_r175260218

---

From: MikkelFJ <[email-masked]>
Date: Sat, 17 Mar 2018 15:01:23 +0000 (UTC)

reordering_threshold:

This is one of the great unsolved mysteries of the universe - is information capable of getting lost.

-- 
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/1211#discussion_r175260471

---

From: ianswett <[email-masked]>
Date: Sat, 17 Mar 2018 15:14:58 +0000 (UTC)

reordering_threshold:

In my current implementation, I declare packets lost based only on packet number distance.  I believe it's slightly more correct to only count non-ACK packets, but it's also slightly more complex.  I don't have any data to show whether it matters in practice or not.

Our implementation saves some info about acks when they're sent, but does not mark them as in_flight.

-- 
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/1211#discussion_r175260817

---

From: ianswett <[email-masked]>
Date: Sat, 17 Mar 2018 08:28:54 -0700

I seem to be having problems getting any consensus on a replacement for retransmittable, so I'm going to keep it as retransmittable and we can continue to discuss a replacement name.
Leading suggestions:
1) Congestion-controlled (as in this PR now)
2) In-flight (because it's included in bytes_in_flight)

-- 
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/1211#issuecomment-373928669

---

From: ianswett <[email-masked]>
Date: Sat, 17 Mar 2018 08:32:39 -0700

ac1c35b  Back to retransmittable

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1211/files/f1dcd18dc71c794874eb83acf6dd73c0ba882acd..ac1c35b02961abbcfb6420d1bf74ec4ab70b1437

---

From: ianswett <[email-masked]>
Date: Sat, 17 Mar 2018 08:38:42 -0700

0759163  retrasnmittable

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1211/files/ac1c35b02961abbcfb6420d1bf74ec4ab70b1437..075916374da3fb479b012b7166564a92bebeec81

---

From: Marten Seemann <[email-masked]>
Date: Sat, 17 Mar 2018 15:44:02 +0000 (UTC)

marten-seemann approved this pull request.

-- 
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/1211#pullrequestreview-104769814

---

From: ianswett <[email-masked]>
Date: Sat, 17 Mar 2018 10:34:34 -0700

Merged #1211.

-- 
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/1211#event-1526926397

---

From: ianswett <[email-masked]>
Date: Sat, 17 Mar 2018 10:35:35 -0700

If people would like to continue discussing naming, please open a separate issue.  I think this PR clarifies the original Issue enough to consider it fixed.

-- 
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/1211#issuecomment-373938329

---

From: Martin Thomson <[email-masked]>
Date: Sat, 17 Mar 2018 16:21:24 -0700

@ianswett, remember that you are the editor, and questions of choice of words do not generally require consensus (not that that helps much, of course).

-- 
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/1211#issuecomment-373960179