Subject: [quicwg/base-drafts] define compound packets (#1262)

From: Kazuho Oku <[email-masked]>
Date: Wed, 04 Apr 2018 11:45:25 +0000 (UTC)

In https://github.com/quicwg/base-drafts/issues/630#issuecomment-372985171 @janaiyengar wrote:
> Discussed at editors' meeting. We seemed to agree on something like option 4, with some specific rules: create "compound packet types" which have a length, and encode HS+0RTT and HS+1RTT long header packet types. To keep packet processing simple, the long header can be repeated so that a receiver can process parts of the packet under different keys in a straightforward manner.
> 
> This doesn't have to be the final resolution, but we should drive this issue to an end.
> 
> @kazuho : Would you be interested in driving this? Would you be able to write up a PR with a proposed design (use the one above if you like that design) and propose to the wg?

Thank you for your patience. This is the PR that does what is being suggested. Please take a look.

Some notes:

* a Compound-Initial packet is sent by client
* a Compound-Handshake packet is sent by both endpoints
  * a client can use it to send 0-RTT data along with packets ACKing the server's handshake packets prior to ServerFinished
  * a server can use it to send 0.5-RTT data along with ServerFinished
  * a client can use it to send 1-RTT data along with ClientFinished

* an endpoint is required to process the handshake portion of a compound packet even if the protected part is broken
  * the rationale behind is to not give an on-path attacker the chance of terminating the handshake simply by appending several bytes to the packet (currently, we require an on-path attacker to do AES based on the handshake protection key)

You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * define compound packets

-- File Changes --

    M draft-ietf-quic-transport.md (39)

-- Patch Links --

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

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 04 Apr 2018 04:48:30 -0700

@kazuho pushed 1 commit.

d9c7f0d  fold in 80 chars

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1262/files/87af6f9f337f87e4d9dcfa99fbe9a07d228c6cf3..d9c7f0dc9351febfdcfe1feaff1dd6d468aa395d

---

From: Mike Bishop <[email-masked]>
Date: Wed, 04 Apr 2018 16:20:41 +0000 (UTC)

MikeBishop commented on this pull request.

Good start -- a few comments, but I think this is the right general outline.

> @@ -364,6 +364,8 @@ keys are established.
 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
 |                       Packet Number (32)                      |
 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+|     [Payload Length (16)]     |
++-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
 |                          Payload (*)                        ...

Totally editorial:  You could put Payload in the second half of the previous line to avoid having a jagged illustration.  Variable-length fields like Payload don't have to have any particular width.

> @@ -678,6 +687,34 @@ packet protection in detail.  After decryption, the plaintext consists of a
 sequence of frames, as described in {{frames}}.

+## Compound Packets {#packet-compound}
+
+Compound packets allow the sender to assemble an Initial ({{packet-initial}})

I might use a different verb than "assemble"; perhaps "combine"?

> @@ -678,6 +687,34 @@ packet protection in detail.  After decryption, the plaintext consists of a
 sequence of frames, as described in {{frames}}.

+## Compound Packets {#packet-compound}
+
+Compound packets allow the sender to assemble an Initial ({{packet-initial}})
+or a Handshake ({{packet-handshake}}) packet and a Protected
+({{packet-protected}}) packet into one UDP packet, thereby reducing the number
+of packets required to be emitted when application data can be sent during the
+handshake.

Question, and important to discuss either way:  When sending Compound Initial + 0-RTT, does the minimum packet size apply to the entire compound and not just the Initial section?  If so, might be worth mentioning that one intended use-case of this is to meet the minimum with 0-RTT data instead of PADDING.

> +Compound packets allow the sender to assemble an Initial ({{packet-initial}})
+or a Handshake ({{packet-handshake}}) packet and a Protected
+({{packet-protected}}) packet into one UDP packet, thereby reducing the number
+of packets required to be emitted when application data can be sent during the
+handshake.
+
+A Compound-Initial packet is identical to an Initial packet with the exception
+being that it has the Payload Length field.  The field designates the length of
+the payload of the Initial packet.  The remainder of the UDP packet contains a
+0-RTT Protected packet.
+
+A Compound-Handshake packet is identical to a Handshake packet with the
+exception being that it has the Payload Length field.  The field designates the
+length of the payload of the Handshake packet.  The remainder of the UDP packet
+contains either a 0-RTT Protected packet or a short header packet
+({{short-header}}).

It might be worth explaining or giving an example that all the packet header fields are there for the subsequent packet.  If that includes the type, by the way, you probably don't actually have to specify what type will be there, since the packet will declare it.

> +the payload of the Initial packet.  The remainder of the UDP packet contains a
+0-RTT Protected packet.
+
+A Compound-Handshake packet is identical to a Handshake packet with the
+exception being that it has the Payload Length field.  The field designates the
+length of the payload of the Handshake packet.  The remainder of the UDP packet
+contains either a 0-RTT Protected packet or a short header packet
+({{short-header}}).
+
+The sender MUST NOT assemble QUIC packets belonging to different QUIC
+connections into a single Compound packet.
+
+The receiver of a Compound packet MUST individually process the Cryptographic
+Handshake packet and the Protected packet being contained.  It MUST process the
+Cryptographic Handshake packet of a Compound packet even when the Protected
+packet being contained is deemed invalid.

I think I'd simplify away these terms for the sections of the packet.  Simply say that the thing which follows is processed as a separate packet, which might be discarded, buffered, or processed depending on the receiver's current state.

-- 
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/1262#pullrequestreview-109397762

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 04 Apr 2018 16:37:58 +0000 (UTC)

@kazuho pushed 1 commit.

a52715a  Payload Length and Payload in one line

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

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 04 Apr 2018 16:38:49 +0000 (UTC)

kazuho commented on this pull request.

Thank you very much for the review. I've fixed this in a52715a. I will address other issues later (hopefully tomorrow).

-- 
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/1262#discussion_r179206902

---

From: Mike Bishop <[email-masked]>
Date: Wed, 04 Apr 2018 10:46:00 -0700

+0-RTT Protected packet.
+

...and contains its own packet number, and therefore gets acknowledged separately.

-- 
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/1262#discussion_r179226866

---

From: Martin Thomson <[email-masked]>
Date: Thu, 05 Apr 2018 02:39:19 +0000 (UTC)

martinthomson commented on this pull request.

I think that the name is wrong.  Compound implies that there are multiple packets *in* the one packet.  Which is true, but not from the perspective of the Initial or Handshake packet that is included, which is where the label is applied.

That is, rename the packet types to something like Initial-With-Length.  The section title is fine.

> @@ -678,6 +685,34 @@ packet protection in detail.  After decryption, the plaintext consists of a

What if the remainder of the packet doesn't contain a 0-RTT packet?

-- 
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/1262#pullrequestreview-109562792

---

From: Kazuho Oku <[email-masked]>
Date: Mon, 09 Apr 2018 04:58:01 +0000 (UTC)

@kazuho pushed 4 commits.

77f90aa  switch typenames to `*-Initial`
55b806d  remove redundancy
487d887  more clarification on how a succeeding QUIC packet looks like
4e01ff7  compound packet can be used to expand the Initial packet

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

---

From: Kazuho Oku <[email-masked]>
Date: Mon, 09 Apr 2018 05:01:08 +0000 (UTC)

@kazuho pushed 1 commit.

9e73eab  typos

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1262/files/4e01ff7e3e5cdb086c6a552f14e470db5309e2c7..9e73eabe261b7b24b9ebdfaa2a5459a53c8d155a

---

From: Kazuho Oku <[email-masked]>
Date: Mon, 09 Apr 2018 05:35:23 +0000 (UTC)

@MikeBishop @martinthomson Thank you for your comments. I think I have addressed all of them. Please take a look.

And I think that the text has become clearer by saying that a compound _UDP_ packet is something that consists of a *-with-Length packet and a protected packet. Thank you to @martinthomson for the suggestion.

-- 
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/1262#issuecomment-379637555

---

From: ianswett <[email-masked]>
Date: Mon, 09 Apr 2018 17:28:05 +0000 (UTC)

ianswett approved this pull request.

I'm happy with this, though it reminds me that 32 bit packet numbers in the long header is almost certainly overkill.

> @@ -364,7 +364,7 @@ keys are established.

This makes me think we should move to 2 byte packet numbers.

-- 
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/1262#pullrequestreview-110506912

---

From: ekr <[email-masked]>
Date: Mon, 09 Apr 2018 18:28:28 +0000 (UTC)

This seems OK, but why bother to have extra packet types. We've already expanded the long header quite a bit to make room for the two directional CIDs. Why not just add a length varint and have only one header form

-- 
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/1262#issuecomment-379848730

---

From: Mike Bishop <[email-masked]>
Date: Mon, 09 Apr 2018 19:47:20 +0000 (UTC)

MikeBishop approved this pull request.

Looks good to me.

-- 
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/1262#pullrequestreview-110591210

---

From: ianswett <[email-masked]>
Date: Mon, 09 Apr 2018 12:51:19 -0700

In regards to @ekr.  If we're going to make the length varint and always present(which I'm happy with), can we make the packet number varint(but not relative) too for consistency?

-- 
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/1262#issuecomment-379873037

---

From: MikkelFJ <[email-masked]>
Date: Mon, 09 Apr 2018 20:02:22 +0000 (UTC)

Note that varint packet numbers has on effect on my proposed variation of #1079 PNE because it is immediately obvious where the PN begins if you encrypt and attach to the end. This is not an argument against, but a note to consider exposing length info elsewhere +/- privacy concerns.

-- 
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/1262#issuecomment-379876192

---

From: ianswett <[email-masked]>
Date: Mon, 09 Apr 2018 20:09:02 +0000 (UTC)

For the context of this PR, I'd slightly favor adding a varint length to all long headers, as EKR suggested.

We can discuss packet numbers elsewhere, because as you said @mikkelfj, it may be complicated.

-- 
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/1262#issuecomment-379877972

---

From: MikkelFJ <[email-masked]>
Date: Mon, 09 Apr 2018 20:33:28 +0000 (UTC)

If we assume packet numbers are vartiable length and the PNE scheme is modified to attach the PN at the end as I proposed earlier, then the following extension could handle the length encoding AND provide a fast sanity check of packet validity.

Given the cleartext PN in a 32-bit word (assuming we never encode more than that), the length is given as a 2 bit number. This length can be stored in the last octet of packet, after the encrypted PN. The remaining 6 bits are used as follows:

C std lib has fast low quality PRNG:

int rand() {
  return (((holdrand = holdrand * 214013L + 2531011L) >> 16) & 0x7fff);
}

PN_checksum = (((PN * 214013L + 2531011L) >> 22) & 0xfc) | (PN_len - 1)
where PN_len is length in 1 to 4 octets.

The exact details may vary, but the point is a single fast 32-bit multiplication.

The length of the packet number can now be immediately decoded from the tail byte.
The packet number checksum can be verified immediately after decrypting the packet number.
If it fails there is roughly (and biased) a 1 / 64 chance that a packet with random content will have matching PN checksum. This can weed out trial decryption of the entire packet very fast. This in turn, can be used as a fast packet filter - even without having the full traffic key but just the less sensitive PN key.

Note that exposing the PN length can contribute to connection linkage since it says something about the connection data volume or age.

-- 
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/1262#issuecomment-379884980

---

From: Kazuho Oku <[email-masked]>
Date: Mon, 09 Apr 2018 22:15:01 +0000 (UTC)

@MikeBishop @ianswett @ekr @mikkelfj Thank you all.

> Why not just add a length varint and have only one header form?

I will do this.

OTOH, I agree with @ianswett that how PN of a long header is encoded should better be discussed separately. Reasons: i) they *can* be discussed separately ii) representation of a PN depends on #1079 iii) representation of a PN may depend on the outcome of the stream 0 redesign.

-- 
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/1262#issuecomment-379910581

---

From: MikkelFJ <[email-masked]>
Date: Mon, 09 Apr 2018 22:54:49 +0000 (UTC)

Oh sorry, I meant to post this PN scheme elsewhere - completely ignoring Ians request by mistake.

-- 
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/1262#issuecomment-379918902

---

From: Martin Thomson <[email-masked]>
Date: Tue, 10 Apr 2018 00:47:22 +0000 (UTC)

Yes, let's solve one problem at a time.  The idea of a varint packet number is pretty appealing to me, and I don't *think* that the stream 0 changes will necessarily change it, but we should probably wait and see.

-- 
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/1262#issuecomment-379938292

---

From: Kazuho Oku <[email-masked]>
Date: Mon, 09 Apr 2018 21:36:26 -0700

@kazuho pushed 2 commits.

9b47a09  `Payload Length` on all long header packets
8aae10e  better clarify that every QUIC packet in a compound UDP packet are equal

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1262/files/9e73eabe261b7b24b9ebdfaa2a5459a53c8d155a..8aae10ef5f4247c6ae7bb1a8ebe1ec40e38b823a

---

From: Kazuho Oku <[email-masked]>
Date: Mon, 09 Apr 2018 21:43:20 -0700

@kazuho pushed 1 commit.

ff01058  decrement maximum CHLO length due to Payload Length becoming non-optional

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1262/files/8aae10ef5f4247c6ae7bb1a8ebe1ec40e38b823a..ff01058624e89f2326368e3e4e77e427b271282c

---

From: Martin Thomson <[email-masked]>
Date: Tue, 10 Apr 2018 04:56:46 +0000 (UTC)

This seems like a good change.  Nits only, the structure and content is basically fine.

> @@ -552,7 +552,7 @@ older than 1.3 is negotiated.
 
 QUIC requires that the initial handshake packet from a client fit within the
 payload of a single packet.  The size limits on QUIC packets mean that a record
-containing a ClientHello needs to fit within 1171 octets.
+containing a ClientHello needs to fit within 1170 octets.

An 1170 octet ClientHello will have a two octet length, so this needs to be 1169.

> @@ -403,10 +405,13 @@ Packet Number:
 : Octets 13 to 16 contain the packet number.  {{packet-numbers}} describes the
   use of packet numbers.
 
+Payload Length:
+
+: The length of the Payload field in octets.

...encoded as a variable-length integer.

> @@ -678,6 +691,22 @@ packet protection in detail.  After decryption, the plaintext consists of a

+## Compound Packets {#packet-compound}
+
+A sender can combine multiple QUIC packets (typically a Cryptographic Handshake
+packet and a Protected packet) into one UDP packet, thereby reducing the number

s/UDP packet/UDP datagram/

> @@ -678,6 +691,22 @@ packet protection in detail.  After decryption, the plaintext consists of a

+## Compound Packets {#packet-compound}
+
+A sender can combine multiple QUIC packets (typically a Cryptographic Handshake
+packet and a Protected packet) into one UDP packet, thereby reducing the number
+of UDP packets required to be emitted when application data can be sent during

End the sentence at the comma.  Then "This can reduce the number of UDP datagrams that are needed to complete the handshake when multiple types of packets are sent at the same time."

> @@ -678,6 +691,22 @@ packet protection in detail.  After decryption, the plaintext consists of a

+## Compound Packets {#packet-compound}
+
+A sender can combine multiple QUIC packets (typically a Cryptographic Handshake
+packet and a Protected packet) into one UDP packet, thereby reducing the number
+of UDP packets required to be emitted when application data can be sent during
+the handshake.

Add: "A packet with a short header doesn't include a length, so it has to be the last packet included in a UDP datagram."

> @@ -586,12 +596,15 @@ The first Initial packet that is sent by a client contains a randomized packet
 number.  All subsequent packets contain a packet number that is incremented by
 one, see ({{packet-numbers}}).
 
-The payload of a Initial packet consists of a STREAM frame (or frames)
-for stream 0 containing a cryptographic handshake message, with enough PADDING
-frames that the packet is at least 1200 octets (see {{packetization}}).  The
-stream in this packet always starts at an offset of 0 (see {{stateless-retry}})
-and the complete cryptographic handshake message MUST fit in a single packet
-(see {{handshake}}).
+The payload of an Initial packet conveys a STREAM frame (or frames) for stream
+0 containing a cryptographic handshake message.  The stream in this packet
+always starts at an offset of 0 (see {{stateless-retry}}) and the complete
+cryptographic handshake message MUST fit in a single packet (see {{handshake}}).
+
+The payload of a UDP packet carrying the Initial packet MUST be expanded to at

UDP datagram.

> @@ -586,12 +596,15 @@ The first Initial packet that is sent by a client contains a randomized packet
 number.  All subsequent packets contain a packet number that is incremented by
 one, see ({{packet-numbers}}).
 
-The payload of a Initial packet consists of a STREAM frame (or frames)
-for stream 0 containing a cryptographic handshake message, with enough PADDING
-frames that the packet is at least 1200 octets (see {{packetization}}).  The
-stream in this packet always starts at an offset of 0 (see {{stateless-retry}})
-and the complete cryptographic handshake message MUST fit in a single packet
-(see {{handshake}}).
+The payload of an Initial packet conveys a STREAM frame (or frames) for stream
+0 containing a cryptographic handshake message.  The stream in this packet
+always starts at an offset of 0 (see {{stateless-retry}}) and the complete
+cryptographic handshake message MUST fit in a single packet (see {{handshake}}).
+
+The payload of a UDP packet carrying the Initial packet MUST be expanded to at
+least 1200 octets (see {{packetization}}), by adding PADDING frames to the
+Initial packet and/or by combining the Initial packet with a Protected packet
+(see {{packet-compound}}).

s/Protected packet/0-RTT packet/

No point in being any less precise than needed.

> @@ -678,6 +691,22 @@ packet protection in detail.  After decryption, the plaintext consists of a

+## Compound Packets {#packet-compound}
+
+A sender can combine multiple QUIC packets (typically a Cryptographic Handshake
+packet and a Protected packet) into one UDP packet, thereby reducing the number
+of UDP packets required to be emitted when application data can be sent during
+the handshake.
+
+The sender MUST NOT combine QUIC packets belonging to different QUIC
+connections into a single UDP packet.

datagram

> @@ -678,6 +691,22 @@ packet protection in detail.  After decryption, the plaintext consists of a

+## Compound Packets {#packet-compound}
+
+A sender can combine multiple QUIC packets (typically a Cryptographic Handshake
+packet and a Protected packet) into one UDP packet, thereby reducing the number
+of UDP packets required to be emitted when application data can be sent during
+the handshake.
+
+The sender MUST NOT combine QUIC packets belonging to different QUIC
+connections into a single UDP packet.
+
+Every QUIC packet that is being conveyed in a compound UDP packet is a complete
+QUIC packet.  No fields in the packet header are omitted.  The receiver of a
+compound UDP packet MUST individually process each QUIC packet and separately
+acknowledge them, as if they were received as distinct UDP packets.

s/as distinct UDP packets/as the payload of different UDP datagrams/

-- 
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/1262#pullrequestreview-110691511

---

From: Kazuho Oku <[email-masked]>
Date: Tue, 10 Apr 2018 05:10:26 +0000 (UTC)

@kazuho pushed 1 commit.

f0575f6  Merge branch 'master' into kazuho/compound-packet

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

---

From: Kazuho Oku <[email-masked]>
Date: Tue, 10 Apr 2018 05:12:48 +0000 (UTC)

At the moment the PR states that Payload Length field is a varint, and therefore the minimum overhead is 1 octet. Do you think that we should change it to a fix length of 2 octets?

-- 
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/1262#discussion_r180298854

---

From: Kazuho Oku <[email-masked]>
Date: Tue, 10 Apr 2018 05:23:48 +0000 (UTC)

@kazuho pushed 6 commits.

4504fdc  s/UDP packet/UDP datagram/g
b4dfcbb  clarify that Payload Length is varint
5ed67ea  split the sentence
9dd629e  short header packet needs to be the last one
d5b7312  be more precise
49d6f96  as "the payload of"

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1262/files/f0575f6cafd7367a20c5f417cc40c32fe5b3c959..49d6f961d05ec0f0156417a7bf6370d57a621f59

---

From: Kazuho Oku <[email-masked]>
Date: Mon, 09 Apr 2018 22:24:52 -0700

Done in 4504fdc.

-- 
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/1262#discussion_r180300138

---

From: Kazuho Oku <[email-masked]>
Date: Tue, 10 Apr 2018 05:25:12 +0000 (UTC)

Done in 5ed67ea.

-- 
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/1262#discussion_r180300157

---

From: Kazuho Oku <[email-masked]>
Date: Mon, 09 Apr 2018 22:25:27 -0700

Done in 9dd629e.

-- 
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/1262#discussion_r180300190

---

From: Kazuho Oku <[email-masked]>
Date: Tue, 10 Apr 2018 05:25:46 +0000 (UTC)

Done in 4504fdc.

-- 
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/1262#discussion_r180300237

---

From: Kazuho Oku <[email-masked]>
Date: Tue, 10 Apr 2018 05:26:10 +0000 (UTC)

Done in d5b7312.

-- 
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/1262#discussion_r180300293

---

From: Kazuho Oku <[email-masked]>
Date: Tue, 10 Apr 2018 05:26:30 +0000 (UTC)

Done in 4504fdc.

-- 
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/1262#discussion_r180300349

---

From: Kazuho Oku <[email-masked]>
Date: Mon, 09 Apr 2018 22:26:42 -0700

Done in 49d6f96.

-- 
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/1262#discussion_r180300375

---

From: Kazuho Oku <[email-masked]>
Date: Tue, 10 Apr 2018 05:28:28 +0000 (UTC)

@martinthomson Thank you for the super quick feedback. I think that I have addressed all your comments (as well as merging master) with the exception of https://github.com/quicwg/base-drafts/pull/1262#discussion_r180298854. Please take a look.

-- 
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/1262#issuecomment-379978391

---

From: Martin Thomson <[email-masked]>
Date: Mon, 09 Apr 2018 22:49:49 -0700

If the packet is this big, then the length will be 2 octets.  You can't have encode 1170 (or 1169) in one octet.  Obviously, if the ClientHello is smaller, then the length might only be 1 octet, but I think that one octet is impossible.  ClientHello.random is 32 octets and either a shares is at least 32 octets or a PSK binder is that big too.

-- 
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/1262#discussion_r180303098

---

From: Kazuho Oku <[email-masked]>
Date: Tue, 10 Apr 2018 05:50:46 +0000 (UTC)

Ah! Thank you for very much for the clarification.

-- 
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/1262#discussion_r180303198

---

From: Kazuho Oku <[email-masked]>
Date: Mon, 09 Apr 2018 22:52:25 -0700

@kazuho pushed 1 commit.

93fe0be  decrement by two, since at this size `Payload Length` would be a 2-byte field

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1262/files/49d6f961d05ec0f0156417a7bf6370d57a621f59..93fe0be928a450eb775d22bfe98e7ba93cd33683

---

From: Kazuho Oku <[email-masked]>
Date: Mon, 09 Apr 2018 23:33:14 -0700

I believe that all known issues have been fixed in commits up to 93fe0be.

-- 
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/1262#issuecomment-379989579

---

From: ianswett <[email-masked]>
Date: Tue, 10 Apr 2018 13:23:16 +0000 (UTC)

Two small editorial suggestions, but looks great, thanks for the fast updates.

> @@ -674,6 +689,24 @@ packet protection in detail.  After decryption, the plaintext consists of a

+## Compound Packets {#packet-compound}
+
+A sender can combine multiple QUIC packets (typically a Cryptographic Handshake
+packet and a Protected packet) into one UDP datagram.  This can reduce the
+number of UDP datagrams required to be emitted when application data can be sent
+during the handshake.  A packet with a short header does not include a length,
+so it has to be the last packet included in a UDP datagram.
+
+The sender MUST NOT combine QUIC packets belonging to different QUIC
+connections into a single UDP datagram.
+
+Every QUIC packet that is being conveyed in a compound UDP datagram is a

nit: I'd drop "that is being"

> @@ -674,6 +689,24 @@ packet protection in detail.  After decryption, the plaintext consists of a

+## Compound Packets {#packet-compound}
+
+A sender can combine multiple QUIC packets (typically a Cryptographic Handshake
+packet and a Protected packet) into one UDP datagram.  This can reduce the
+number of UDP datagrams required to be emitted when application data can be sent

For clarity, how about "number of UDP datagrams sent when application data is sent during the handshake."?

-- 
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/1262#pullrequestreview-110824680

---

From: Nick Banks <[email-masked]>
Date: Tue, 10 Apr 2018 13:53:13 +0000 (UTC)

nibanks approved this pull request.

I really like how clean this design is. Good improvement!

-- 
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/1262#pullrequestreview-110845200

---

From: MikkelFJ <[email-masked]>
Date: Tue, 10 Apr 2018 08:41:20 -0700

mikkelfj commented on this pull request.

What about packets in the gray zone such as version negotiation and resets?

> @@ -338,9 +340,14 @@ Packet Number:
 : The Packet Number is a 32-bit field that follows the two connection IDs.
   {{packet-numbers}} describes the use of packet numbers.
 
+Payload Length:
+
+: The length of the Payload field in octets, encoded as a variable-length
+  integer ({{integer-encoding}}).
+

I came to think a bit about the variable payload length:

It makes it difficult to build a packet when you don't know the length in advance. Of course you can always just start with enough space. The intial packet starts with 1200 bytes (not sure about 0-RTT), and I don't think it can be more than 64K? So wouldn't a fixed 2 byte length field make more sense here?

-- 
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/1262#pullrequestreview-110894387

---

From: Martin Thomson <[email-masked]>
Date: Tue, 10 Apr 2018 17:05:52 -0700

+Payload Length:
+

The variable length is primarily for consistency.  There are a few cases where packets will be small enough to benefit from the 6-bit length (Finished primarily), but the main benefit is that this is the same code throughout.  Better if we can manage to make the packet number encoding variable length as well (but that interacts in interesting ways with #1079, so that's not as clear a win as this).

-- 
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/1262#discussion_r180602396

---

From: Martin Thomson <[email-masked]>
Date: Tue, 10 Apr 2018 17:06:06 -0700

Yes, I will address those in a follow-up.

-- 
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/1262#discussion_r180602439

---

From: Martin Thomson <[email-masked]>
Date: Tue, 10 Apr 2018 17:11:42 -0700

Merged #1262.

-- 
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/1262#event-1567428107

---

From: janaiyengar <[email-masked]>
Date: Thu, 12 Apr 2018 14:15:59 -0700

Coming in late, but I like the final shape of this. Thanks, @kazuho, for driving this quickly!

-- 
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/1262#issuecomment-380946919