Subject: [quicwg/base-drafts] PADDING counts towards byte in flight (#1540)

From: ianswett <[email-masked]>
Date: Mon, 09 Jul 2018 06:32:35 -0700

Clarifies the recovery draft in light of #1517
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * PADDING counts towards byte in flight

-- File Changes --

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

-- Patch Links --

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

---

From: MikkelFJ <[email-masked]>
Date: Mon, 09 Jul 2018 06:38:54 -0700

mikkelfj commented on this pull request.

> @@ -1018,7 +1025,7 @@ ecn_ce_counter:
 
 bytes_in_flight:
 : The sum of the size in bytes of all sent packets that contain at least
-  one retransmittable frame, and have not been acked or declared
+  one retransmittable or PADDING frame, and have not been acked or declared
   lost. The size does not include IP or UDP overhead.

But presumably it does include AEAD tags?

-- 
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/1540#pullrequestreview-135420006

---

From: ianswett <[email-masked]>
Date: Mon, 09 Jul 2018 13:42:36 +0000 (UTC)

@ianswett pushed 1 commit.

1e34037  Clarify byte_in_flight

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1540/files/96c1d03def881f497f037f986297ec3e74d43c76..1e340379d4ca016ede8075021741d7a34fabafe6

---

From: Mike Bishop <[email-masked]>
Date: Mon, 09 Jul 2018 20:15:25 +0000 (UTC)

MikeBishop commented on this pull request.

I believe "in-flight" should be hyphenated when used as an adjective before the word.

> @@ -110,6 +110,13 @@ important to the loss detection and congestion control machinery below.
 * Retransmittable packets are those that contain at least one retransmittable
   frame.
 
+* PADDING frames do cause packets containing them to count towards bytes in
+  flight, but do not instigate acknowledgement.

Instigate feels like an unusual word choice here.  Not necessarily incorrect, but unusual.

-- 
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/1540#pullrequestreview-135573985

---

From: Martin Thomson <[email-masked]>
Date: Mon, 09 Jul 2018 16:13:07 -0700

martinthomson commented on this pull request.

What Mike said, but I think that it would be better to concentrate on the more important distinctions and perhaps rewrite this bit.  Your bullet list is now 6 items, which is too long anyway, so perhaps this could be reduced:

QUIC packets can contain multiple frames of different types.  The recovery mechanisms ensure that frames that need reliable delivery are acknowledged and sent in new packets as necessary.  The types of frames contained in a packet can affect recovery and congestion control logic:

* All packets are acknowledged, though packets that contain only ACK, ACK_ECN, and PADDING frames are not acknowledged immediately.

* Packets that contain CRYPTO frames are critical to the performance of the QUIC handshake and use shorter timers for acknowledgement and retransmission.

* Packets that contain only ACK and ACK_ECN frames do not count toward congestion control limits (see {{blah}}).  That is, they do not contribute toward bytes in flight.  Note that this means that PADDING frames cause packets to contribute toward bytes in flight without directly causing an acknowledgment to be sent. 

(This last point might be accomplished better with a forward reference, but I didn't see a nice place to put that, the definition of bytes in flight is buried a little.)

-- 
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/1540#discussion_r201175532

---

From: ianswett <[email-masked]>
Date: Tue, 10 Jul 2018 19:19:14 -0700

aa00f6b  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/1540/files/1e340379d4ca016ede8075021741d7a34fabafe6..aa00f6b56fd82e78586f1344c6d548187b3f3687

---

From: ianswett <[email-masked]>
Date: Tue, 10 Jul 2018 19:20:41 -0700

4be49f9  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/1540/files/aa00f6b56fd82e78586f1344c6d548187b3f3687..4be49f98e0b368c740f0e4f4da8585b4a4f2f713

---

From: Martin Thomson <[email-masked]>
Date: Tue, 10 Jul 2018 21:46:28 -0700

Bump for this comment.  I think that it might be a better framing still, after some adjustments.

-- 
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/1540#discussion_r201563766

---

From: ianswett <[email-masked]>
Date: Wed, 11 Jul 2018 13:43:44 -0700

bbfcde0  in-flight

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1540/files/4be49f98e0b368c740f0e4f4da8585b4a4f2f713..bbfcde0d7f5fdd69720489c3280a178517d5ed1d

---

From: ianswett <[email-masked]>
Date: Wed, 11 Jul 2018 14:15:35 -0700

7a35971  Integrate Martin's suggestion

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1540/files/bbfcde0d7f5fdd69720489c3280a178517d5ed1d..7a35971cba2219624e0b5c7eec2522700051f80e

---

From: ianswett <[email-masked]>
Date: Wed, 11 Jul 2018 14:16:43 -0700

I think there are more cleanups to be done, possibly by adding a terminology section for bytes_in_flight, etc, but I'd like to do that in a separate PR if it's a good idea.

-- 
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/1540#issuecomment-404312165

---

From: Mike Bishop <[email-masked]>
Date: Wed, 11 Jul 2018 14:58:23 -0700

> -* ACK and ACK_ECN frames contain acknowledgment information. ACK_ECN frames
-  additionally contain information about ECN codepoints seen by the peer.  (The
-  rest of this document uses ACK frames to refer to both ACK and ACK_ECN
-  frames.)
+QUIC packets can contain multiple frames of different types. The recovery
+mechanisms ensure that data and frames that need reliable delivery are
+acknowledged or declared lost and sent in new packets as necessary. The types
+of frames contained in a packet affect recovery and congestion control logic:
+
+* All packets are acknowledged, though packets that contain only ACK,
+  ACK_ECN, and PADDING frames are not acknowledged immediately.  Packets
+  containing at least one frame besides ACK, ACK_ECN, and PADDING are referred
+  to as retransmittable below.
+
+* Packets that contain CRYPTO frames are critical to the performance of the
+  QUIC handshake and use shorter timers for acknowledgement and retransmission.

Technically, this isn't (or need not be) true once you've reached 1-RTT.  Are NewSessionTicket messages treated with equally high priority?

-- 
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/1540#pullrequestreview-136438063

---

From: ianswett <[email-masked]>
Date: Wed, 11 Jul 2018 15:01:38 -0700

02c636d  Long header CRYPTO packets

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1540/files/7a35971cba2219624e0b5c7eec2522700051f80e..02c636d816e06c20546b9e78f568204db69802e3

---

From: Mike Bishop <[email-masked]>
Date: Wed, 11 Jul 2018 15:04:46 -0700

> @@ -110,8 +110,9 @@ of frames contained in a packet affect recovery and congestion control logic:
   containing at least one frame besides ACK, ACK_ECN, and PADDING are referred
   to as retransmittable below.
 
-* Packets that contain CRYPTO frames are critical to the performance of the
-  QUIC handshake and use shorter timers for acknowledgement and retransmission.
+* Long header ackets that contain CRYPTO frames are critical to the performance

**p**ackets

-- 
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/1540#pullrequestreview-136439707

---

From: ianswett <[email-masked]>
Date: Wed, 11 Jul 2018 22:09:09 +0000 (UTC)

a72ec02  packets

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1540/files/02c636d816e06c20546b9e78f568204db69802e3..a72ec021c4ab3889024363f21daf6907ae47e9ad

---

From: Martin Thomson <[email-masked]>
Date: Wed, 11 Jul 2018 17:13:57 -0700

martinthomson 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/1540#pullrequestreview-136465231

---

From: janaiyengar <[email-masked]>
Date: Wed, 11 Jul 2018 20:16:44 -0700

janaiyengar commented on this pull request.

a few nits

-* Cryptographic handshake data is sent in CRYPTO frames, and uses the
-  reliability machinery of QUIC underneath.
-
-* ACK and ACK_ECN frames contain acknowledgment information. ACK_ECN frames
-  additionally contain information about ECN codepoints seen by the peer.  (The

This sentence in the parenthesis is necessary for subsequent uses of the term "ACK frames"

-* Retransmittable frames are those that count towards bytes in flight and need
-  acknowledgement.  The most common are STREAM frames, which typically contain
-  application data.
+* All packets are acknowledged, though packets that contain only ACK,
+  ACK_ECN, and PADDING frames are not acknowledged immediately.  Packets
+  containing at least one frame besides ACK, ACK_ECN, and PADDING are referred
+  to as retransmittable below.

put "retransmittable" in quotes, for clarity

> @@ -582,7 +583,8 @@ sent_packets:
 
 : An association of packet numbers to information about them, including a number
   field indicating the packet number, a time field indicating the time a packet
-  was sent, a boolean indicating whether the packet is ack only, and a bytes
+  was sent, a boolean indicating whether the packet is ack only, a boolen

boolean

> @@ -623,9 +625,12 @@ are as follows:
 
 * packet_number: The packet number of the sent packet.
 
-* is_ack_only: A boolean that indicates whether a packet only contains an
-  ACK frame.  If true, it is still expected an ack will be received for
-  this packet, but it is not retransmittable.
+* ack_only: A boolean that indicates whether a packet only contains an

s/only contains an/contains only/

-- 
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/1540#pullrequestreview-136489398

---

From: ianswett <[email-masked]>
Date: Thu, 12 Jul 2018 07:35:18 -0700

c32e591  Jana's comments

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

---

From: janaiyengar <[email-masked]>
Date: Sat, 14 Jul 2018 07:10:56 -0700

@janaiyengar pushed 1 commit.

be54deb  65 chars

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

---

From: janaiyengar <[email-masked]>
Date: Sat, 14 Jul 2018 07:11:53 -0700

Merged #1540.

-- 
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/1540#event-1733882735