Subject: Re: [quicwg/base-drafts] Rearrange Packet Types section and references (#2203)

From: David Schinazi <[email-masked]>
Date: Mon, 07 Jan 2019 15:20:05 -0800

DavidSchinazi commented on this pull request.

> @@ -2609,15 +2605,22 @@ details.
 
 ## Coalescing Packets {#packet-coalesce}
 
-A sender can coalesce multiple QUIC packets into one UDP datagram.  This can
-reduce the number of UDP datagrams needed to complete the cryptographic
-handshake and starting sending data.  Receivers MUST be able to process
-coalesced packets.
+Initial ({{packet-initial}}), 0-RTT ({{packet-0rtt}}), and Handshake
+({{packet-handshake}}) packets contain a Length field, which determines the end

I think we might want to either rename the Length field, or at least clarify that it's in a different place based on the Long Header Type.

-- 
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/2203#pullrequestreview-190036357

---

From: David Schinazi <[email-masked]>
Date: Mon, 07 Jan 2019 15:23:15 -0800

The comments in this PR discuss an "Extended Long Header" but I don't see it in the actual diff. Was it removed? I tried to address this in #2295 but I'm happy to have it be fixed here with an explicit extended long header.

-- 
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/2203#issuecomment-452118276

---

From: Mike Bishop <[email-masked]>
Date: Tue, 08 Jan 2019 14:00:28 -0800

Yes -- because it's subtly different in the different packets, I changed it to an overarching description of fields which occur in multiple packet types, but indicate which fields are present in each packet type's section.

-- 
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/2203#issuecomment-452467494

---

From: Mike Bishop <[email-masked]>
Date: Tue, 08 Jan 2019 22:16:00 +0000 (UTC)

@MikeBishop pushed 2 commits.

9715d3579a713c54345bdf0a79a30a757f834e95  Merge remote-tracking branch 'origin/master' into transport/packet-types
7ad6580092f5b531268bdb74db6181ce985629d0  Lower nibble as well

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2203/files/03f8fb768bc74c5c869807c936750c2df9fe678c..7ad6580092f5b531268bdb74db6181ce985629d0

---

From: Martin Thomson <[email-masked]>
Date: Tue, 08 Jan 2019 22:36:46 +0000 (UTC)

martinthomson approved this pull request.

This is good.  Thanks for doing the hard work here.

> -Header Form:
-
-: The most significant bit (0x80) of byte 0 is set to 0 for the short header.
-
-Fixed Bit:
-
-: The next bit (0x40) of byte 0 is set to 1.  Packets containing a zero value
-  for this bit are not valid packets in this version and MUST be discarded.
-
-Spin Bit (S):
-
-: The sixth bit (0x20) of byte 0 is the Latency Spin Bit, set as described in
-  {{!SPIN=I-D.ietf-quic-spin-exp}}.
+packet type.  While type-specific semantics for this version are described in
+the following sections, several long-header packets in this version of QUIC
+contain these additional fields:

Is there a picture to accompany this, or are you relying on the Initial/Handshake/0-RTT pictures?

-### 0-RTT Packet Numbers {#retry-0rtt-pn}
-
-<!-- TODO: Thus far, we have not really addressed 0-RTT at all. We don't even
-really say much about what it is for. This really belongs in a section for that,
-probably right after the example handshake exchanges. -->
+See Sections 2.3 of {{!TLS13}} for a discussion of 0-RTT data and its

```suggestion
See Section 2.3 of {{!TLS13}} for a discussion of 0-RTT data and its
```

+~~~
++-+-+-+-+-+-+-+-+
+|1|1| 0 |R R|P P|

```suggestion
|1|1| 1 |R R|P P|
```

-A Handshake packet uses long headers with a type value of 0x2.  It is
-used to carry acknowledgments and cryptographic handshake messages from the
-server and client.
+A Handshake packet uses long headers with a type value of 0x2, followed by the
+Length and Packet Number fields.  The first byte contains the Reserved and
+Packet Number Length bits.  It is used to carry acknowledgments and
+cryptographic handshake messages from the server and client.
+
+~~~
++-+-+-+-+-+-+-+-+
+|1|1| 0 |R R|P P|

```suggestion
|1|1| 2 |R R|P P|
```

-- 
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/2203#pullrequestreview-190489740

---

From: janaiyengar <[email-masked]>
Date: Tue, 08 Jan 2019 15:55:36 -0800

janaiyengar approved this pull request.

Thanks for doing this!

> @@ -2627,7 +2632,7 @@ packet for later processing and MUST attempt to process the remaining packets.
 
 Retry packets ({{packet-retry}}), Version Negotiation packets
 ({{packet-version}}), and packets with a short header cannot be followed by
-other packets in the same UDP datagram.
+other packets in the same UDP datagram, as they do not contain a Length field.

Minor rearrangement to avoid ambiguity about "they":
"... and packets with a short header do not contain a Length field and cannot be followed by other packets ..."

-- 
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/2203#pullrequestreview-186302622

---

From: Mike Bishop <[email-masked]>
Date: Tue, 08 Jan 2019 15:56:00 -0800

MikeBishop commented on this pull request.

-

Replying on the other pictures.

-- 
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/2203#discussion_r246210838

---

From: Mike Bishop <[email-masked]>
Date: Tue, 08 Jan 2019 15:56:33 -0800

@MikeBishop pushed 1 commit.

40818cd06501d45b9e39ba52bbc3d6acd3053cdf  Apply suggestions from code review

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2203/files/7ad6580092f5b531268bdb74db6181ce985629d0..40818cd06501d45b9e39ba52bbc3d6acd3053cdf

---

From: Mike Bishop <[email-masked]>
Date: Tue, 08 Jan 2019 16:03:42 -0800

b01d230f0cdf24ff458dee88f996cf57df51466d  Jana's suggestion; remove "they"

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2203/files/40818cd06501d45b9e39ba52bbc3d6acd3053cdf..b01d230f0cdf24ff458dee88f996cf57df51466d

---

From: Martin Thomson <[email-masked]>
Date: Tue, 08 Jan 2019 16:06:26 -0800

Merged #2203 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/2203#event-2061692063