Subject: [quicwg/base-drafts] First iteration over transport draft (#1857)

From: janaiyengar <[email-masked]>
Date: Fri, 12 Oct 2018 23:10:43 -0700

Bringing draft up to proposed structure in #411.  We'll need to do more moving around of things and adding glue to various places to make sure the text flows freely and correctly, but I believe this PR moves major sections into the right slots (mostly).
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * Moved stream stuff up and added intro text to describe structure of doc
  * Moved major sections around.
  * created packets and frames section and moved things about

-- File Changes --

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

-- Patch Links --

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

---

From: Martin Thomson <[email-masked]>
Date: Mon, 15 Oct 2018 01:48:18 -0700

martinthomson commented on this pull request.

This is a great start.  Thanks for putting this together.

I took the opportunity to do a fairly thorough read-through (up until I got to the frame descriptions).  So there are a mix of comments throughout.  I probably strayed over the line on occasion and made some non-editorial comments as I went; as we discuss, we can open issues for those.

Overall, this is a big improvement, and I can see how it might be the basis for more improvements (some of which I noted).  We shouldn't worry about getting things perfect, but at the same time I don't want to lose these comments.  Most of them are fairly directly actionable.

No more time today, but I might be able to generate a PR for some of this as things stabilize more.

> @@ -134,18 +134,27 @@ encrypts most of the data it exchanges, including its signaling.  This allows
 the protocol to evolve without incurring a dependency on upgrades to
 middleboxes.
 
-This document describes the core QUIC protocol, including the conceptual design,
-wire format, and mechanisms of the QUIC protocol for connection establishment,
-stream multiplexing, stream and connection-level flow control, connection
-migration, and data reliability.
+This document describes the core QUIC protocol, and is structured as follows:

Does a two-level list make sense here?

```
* Streams are the basic service abstraction that QUIC provides:
** {{streams}} describes core concepts related to streams, 
** {{stream-states}} provides a reference model for stream states, and 
** {{flow-control}} outlines the operation of flow control.

* Connections are the context in which QUIC endpoints communicate.
** {{}} details the process for establishing connections,
** {{}} describes how endpoints migrate a connection to use a new network paths, and
** {{}} lists the options for terminating an open connection.

* Packets and frames are the basic unit used by QUIC to communicate.
** {{}} describes the format of packets and frames,
** {{}} defines a models for the transmission and retransmission of information,
** {{}} describes acknowledgments, the primary feedback mechanism in QUIC, and
** {{}} contains a rules for managing the size of packets.

* Details of encoding of QUIC protocol elements is described in:
** {{}} (Versions),
** {{}} (Packet Headers),
** {{}} (TP), 
** {{}} (Frames), and 
** {{}} (Errors).
```

If we do that, I would add a heading, like `## This Document` here.  That would also satisfy the itch that adding conventions and notations as 1.1 has, which says that having only one subsection is wrong.

-Version numbers used to identify IETF drafts are created by adding the draft
-number to 0xff000000.  For example, draft-ietf-quic-transport-13 would be
-identified as 0xff00000D.
+Streams are identified by an unsigned 62-bit integer, referred to as the Stream

This is the first mention of a 62-bit value, which I *guarantee* will raise eyebrows and generate annoying questions.  A forward reference to the varint encoding is probably necessary.  Maybe just move the last paragraph up here.

-Any QUIC packet has either a long or a short header, as indicated by the Header
-Form bit. Long headers are expected to be used early in the connection before
-version negotiation and establishment of 1-RTT keys.  Short headers are minimal
-version-specific headers, which are used after version negotiation and 1-RTT
-keys are established.
+The first bi-directional stream opened by the client is stream 0.

s/bi-directional/bidirectional

-: The most significant bit (0x80) of octet 0 (the first octet) is set to 1 for
-  long headers.
+An endpoint limits the number of concurrently active incoming streams by

Missing transitional glue: "QUIC allows for an arbitrary number of streams to operate concurrently."  Necessary if you take my proposed edit above.

-: The most significant bit (0x80) of octet 0 (the first octet) is set to 1 for
-  long headers.
+An endpoint limits the number of concurrently active incoming streams by
+adjusting the maximum stream ID.  An initial value is set in the transport

s/adjusting/limiting/

-Versions with the most significant 16 bits of the version number cleared are
-reserved for use in future IETF consensus documents.
+The creation and destruction of streams are expected to have minimal bandwidth
+and computational cost.  A single STREAM frame may create, carry data for, and

We don't really say here how streams are created.  It's something you need to infer.  Maybe move this paragraph above the previous one and make it:

> Streams can be created by sending data.  Other processes associated with stream management - ending, cancelling, and managing flow control - are all designed to impose minimal overheads.  For instance, a single STREAM frame ({{frame-stream}}) can open, carry data for, and close a stream.

> -: The octet following the version contains the lengths of the two connection ID
-  fields that follow it.  These lengths are encoded as two 4-bit unsigned
-  integers. The Destination Connection ID Length (DCIL) field occupies the 4
-  high bits of the octet and the Source Connection ID Length (SCIL) field
-  occupies the 4 low bits of the octet.  An encoded length of 0 indicates that
-  the connection ID is also 0 octets in length.  Non-zero encoded lengths are
-  increased by 3 to get the full length of the connection ID, producing a length
-  between 4 and 18 octets inclusive.  For example, an octet with the value 0x50
-  describes an 8-octet Destination Connection ID and a zero-length Source
-  Connection ID.
+Once a stream is created, endpoints may use the stream to send and receive data.
+Each endpoint may send a series of STREAM frames encapsulating data on a stream
+until the stream is terminated in that direction.  Streams are an ordered
+byte-stream abstraction, and they have no other structure within them.  STREAM
+frame boundaries are not expected to be preserved in retransmissions from the
+sender or during delivery to the application at the receiver.

Suggest:

> Endpoints uses streams to send and receive data.  Endpoints send STREAM frames, which encapsulate data for a stream.  STREAM frames carry a flag that can be used to signal the end of a stream.
>
> Streams are an ordered byte-stream abstraction with no other structure that is visible to QUIC.  STREAM frame boundaries are not expected to preserved when data is transmitted, including when data is retransmitted after packet loss.

-Length:
+Flow control is described in detail in {{flow-control}}, and congestion control
+is described in the companion document {{QUIC-RECOVERY}}.

This congestion control comment now seems entirely out of place.  I suggest merging the flow control note into the previous paragraph, and dropping the recovery reference entirely.

> @@ -208,1721 +217,1890 @@ x (*) ...
 : Indicates that x is variable-length

-# Versions {#versions}
+# Streams: QUIC's Data Structuring Abstraction {#streams}

Can we just call this "Streams"?  Please.  If only because we might grow new abstractions in future.

-The client responds to a Retry packet with an Initial packet that includes the
-provided Retry Token to continue connection establishment.
+It is necessary to limit the amount of data that a sender may have outstanding
+at any time, so as to prevent a fast sender from overwhelming a slow receiver,
+or to prevent a malicious sender from consuming significant resources at a
+receiver.  This section describes QUIC's flow-control mechanisms.

We can probably drop the last sentence.  It's self-evident.

-A client sets the Destination Connection ID field of this Initial packet to the
-value from the Source Connection ID in the Retry packet. Changing Destination
-Connection ID also results in a change to the keys used to protect the Initial
-packet. It also sets the Token field to the token provided in the Retry. The
-client MUST NOT change the Source Connection ID because the server could include
-the connection ID as part of its token validation logic (see {{tokens}}).
+QUIC employs a credit-based flow-control scheme similar to HTTP/2's flow control

Avoid possessive: "similar to that in HTTP/2 {{?HTTP2}}"

-A client sets the Destination Connection ID field of this Initial packet to the
-value from the Source Connection ID in the Retry packet. Changing Destination
-Connection ID also results in a change to the keys used to protect the Initial
-packet. It also sets the Token field to the token provided in the Retry. The
-client MUST NOT change the Source Connection ID because the server could include
-the connection ID as part of its token validation logic (see {{tokens}}).
+QUIC employs a credit-based flow-control scheme similar to HTTP/2's flow control
+{{?HTTP2}}.  A receiver advertises the number of octets it is prepared to
+receive on a given stream and for the entire connection.  This leads to two
+levels of flow control in QUIC: (i) Connection flow control, which prevents

Make a real bullet list, I think.  Also, we should try to be consistent about order:  I think that stream flow control first (MAX_STREAM_DATA) and connection second (MAX_DATA) makes sense based on the description in the next paragraph.  Of course, that's opposite to how we do things everywhere else...

-All these packets use the long header and contain the current QUIC version in
-the version field.
+Conversely if a sender believes it is blocked, while endpoint B expects more

Remove paragraph break before this.

-## Cryptographic Handshake Packets {#handshake-packets}
+## Edge Cases and Other Considerations

I don't like this section title, maybe "Consistency of Flow Control State" or "Handling of Stream Cancellation"  (see comment below)

+To avoid this de-synchronization, a RST_STREAM sender MUST include the final

"To ensure that endpoints maintain a consistent connection-level flow control state, the RST_STREAM frame {{frame-rst-stream}} includes the largest offset of data sent on the stream."

-The Initial packet (shown in {{initial-format}}) has two additional header
-fields that are added to the Long Header before the Length field.
+### Data Limit Increments {#fc-credit}

Make this heading 2 ("## Data Limit Increments").

> -+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
-|                            Token (*)                        ...
-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
-|                           Length (i)                        ...
-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
-|                     Packet Number (8/16/32)                   |
-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
-|                          Payload (*)                        ...
-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
-~~~
-{: #initial-format title="Initial Packet"}
+This document leaves when and how many bytes to advertise in a MAX_DATA or
+MAX_STREAM_DATA to implementations, but offers a few considerations.  These
+frames contribute to connection overhead.  Therefore frequently sending frames
+with small changes is undesirable.  At the same time, infrequent updates require
+larger increments to limits if blocking is to be avoided.  Thus, larger updates

"At the same time, larger increments to limits are necessary to avoid blocking if updates are less frequent."

-The client and server use the Initial packet type for any packet that contains
-an initial cryptographic handshake message. This includes all cases where a new
-packet containing the initial cryptographic message needs to be created, such as
-the packets sent after receiving a Version Negotiation ({{packet-version}}) or
-Retry packet ({{packet-retry}}).
+### Blocking on Flow Control {#blocking}

This belongs under the data limit increments section, not the stream limit increments.

-A server sends its first Initial packet in response to a client Initial.  A
-server may send multiple Initial packets.  The cryptographic key exchange could
-require multiple round trips or retransmissions of this data.
+If a sender does not receive a MAX_DATA or MAX_STREAM_DATA frame when it has run
+out of flow control credit, the sender will be blocked and SHOULD send a BLOCKED

"[...] the sender will be unable to send new data.  That is, the sender it blocked.  A blocked sender SHOULD send a STREAM_BLOCKED or BLOCKED frame."

-## Initial Packet {#packet-initial}
+### Response to a RST_STREAM

I think that we can drop this heading.  It fits well-enough into the preceding section.

-Token Length:
+## Stream Limit Increment

See comment above in stream concurrency section.

-: The most significant bit (0x80) of octet 0 (the first octet) is set to 1 for
-  long headers.
+An endpoint limits the number of concurrently active incoming streams by
+adjusting the maximum stream ID.  An initial value is set in the transport
+parameters (see {{transport-parameter-definitions}}) and is subsequently
+increased by MAX_STREAM_ID frames (see {{frame-max-stream-id}}).

I think that we need to forward reference the section on increasing stream concurrency instead of transport parameters and whatnot.  Move this last sentence to that section and replace with a forward reference here.

-The payload of an Initial packet includes a CRYPTO frame (or frames) containing
-a cryptographic handshake message, ACK frames, or both.  PADDING and
-CONNECTION_CLOSE frames are also permitted.  An endpoint that receives an
-Initial packet containing other frames can either discard the packet as spurious
-or treat it as a connection error.
+For smooth operation of the congestion controller, it is generally considered

There are many reasons we don't want a peer to become blocked.  Do we have to pick just one?

-When a server receives an Initial packet with an address validation token, it
-SHOULD attempt to validate it.  If the token is invalid then the server SHOULD
-proceed as if the client did not have a validated address, including potentially
-sending a Retry. If the validation succeeds, the server SHOULD then allow the
-handshake to proceed (see {{stateless-retry}}).
+Each connection possesses a set of identifiers, any of which could be used to
+distinguish it from other connections.  Connection IDs are selected
+independently in each direction.  Each Connection ID has an associated sequence
+number to assist in deduplicating messages.

Move this last sentence to the "issuing" section below.  It's not important enough to be right up here.

+The initial connection ID issued by an endpoint is the Sour

"[...] by an endpoint is sent in the Source Connection ID field of the long packet header ({{...}}) during the handshake."

+The initial connection ID issued by an endpoint is the Sour

We are missing a paragraph on failed packet protection removal here.  We probably want that.

+The initial connection ID issued by an endpoint is the Sour

Circular reference.  I think that we need to say

> [...] matched to a connection using the connection ID or - for packets with zero-length connection IDs - the address tuple.  If the packet doesn't match an existing connection, the server continues below.

+The initial connection ID issued by an endpoint is the Sour

We can remove this requirement now that we have Retry and the token stuff.

+The initial connection ID issued by an endpoint is the Sour

The first CRYPTO frame *from a client*

+The initial connection ID issued by an endpoint is the Sour

Are we at 1232, or 1200?

+The initial connection ID issued by an endpoint is the Sour

"The sequence numbers used by CRYPTO frames to ensure ordered delivery of cryptographic handshake data start from zero in each packet number space."

+The initial connection ID issued by an endpoint is the Sour

No EOED any more.

+The initial connection ID issued by an endpoint is the Sour

We need text leading into this diagram as well.  Something like "{{tls-0rtt-handshake}} shows the same handshake with 0-RTT data added."

+The initial connection ID issued by an endpoint is the Sour

0..2^16-1 now that we don't mandate use of any parameters.

+The initial connection ID issued by an endpoint is the Sour

s/encode the/describe the encoding of/

+The initial connection ID issued by an endpoint is the Sour

We should include original_connection_id here as well?

+The initial connection ID issued by an endpoint is the Sour

Merge max_*_streams into the other list.

+The initial connection ID issued by an endpoint is the Sour

"Address validation is used by QUIC to avoid being used for a traffic amplification attack." - and strike the first paragraph entirely.  Then make the change I suggest below.

+The initial connection ID issued by an endpoint is the Sour

> The primary defense against amplification attack is verifying that a client is able to receive packets at the transport address that it claims.  QUIC also requires that clients send UDP datagrams with at least 1200 octets of payload until the server has completed address validation.  A server can thereby send more data to an unproven addresses without increasing the amplification advantage gained by an attacker.

+The initial connection ID issued by an endpoint is the Sour

should this be "As long as it is not possible for an attacker to guess a valid token for their address ..." ?

+The initial connection ID issued by an endpoint is the Sour

Split paragraph before "Because".

+The initial connection ID issued by an endpoint is the Sour

Add: "The only requirement is that a valid token be difficult to guess for an attacker."

+The initial connection ID issued by an endpoint is the Sour

Strike this paragraph.  It's wrong now.

+The initial connection ID issued by an endpoint is the Sour

We should add: "A resumption token SHOULD be easily distinguishable from tokens that are sent in Retry packets as they are carried in the same field."

+The initial connection ID issued by an endpoint is the Sour

This belongs somewhere else now, but I don't know where just yet.

+The initial connection ID issued by an endpoint is the Sour

I think that we want this to be heading 1.  It's under the cryptographic handshake, but I think that it logically belongs before it.  That's unfortunate, because I'd really like to have an overview of the handshake up front.  Perhaps we need a new "Connection Lifecycle Overview" section ahead of all this stuff.

If we make this a top-level section, then you can cover the two aspects of address validation in the one section: this section covers the handshake, then you have the explicit validation stuff that migration relies on.  A brief intro can explain that there are two mechanisms and why.

+The initial connection ID issued by an endpoint is the Sour

s/For instance,/In particular,/ -- these are good practices we want to encourage.  Also, using "For instance" in this way might be read to imply that STREAM is verboten.  It's not, it's just that you don't really want to rely on it.

+The initial connection ID issued by an endpoint is the Sour

"Initiating Path Validation" is better - it's easy to lose necessary context when jumping around, and "Initiation" is very generic.

+The initial connection ID issued by an endpoint is the Sour

"[...] MAY send multiple PATH_CHALLENGE frames to guard against loss."

+The initial connection ID issued by an endpoint is the Sour

"Successful Path Validation"

+The initial connection ID issued by an endpoint is the Sour

"Path Validation Responses"

+The initial connection ID issued by an endpoint is the Sour

s/, with the following stipulation.  Since/. However, because/

+The initial connection ID issued by an endpoint is the Sour

s/MAY/MUST/g

+The initial connection ID issued by an endpoint is the Sour

s/: from/. That is, from/

+The initial connection ID issued by an endpoint is the Sour

"Failed Path Validation"

+The initial connection ID issued by an endpoint is the Sour

> Path validation only fails when the endpoint attempting to validate the path abandons its attempt to validate the path.
> 
> Endpoints SHOULD abandon path validation based on a timer.  When setting this timer [...]

+The initial connection ID issued by an endpoint is the Sour

> When an endpoint abandons path validation, it determines that the path is unusable.  This does not [...]

+The initial connection ID issued by an endpoint is the Sour

s/QUIC/The use of a connection ID/

+The initial connection ID issued by an endpoint is the Sour

NAT rebinding is not connection migration as defined in this section, though endpoints SHOULD perform path validation ({{migrate-validate}}) if they detect a change in a remote address.

+The initial connection ID issued by an endpoint is the Sour

Maybe we should reiterate that this won't include non-probing frames here.

+The initial connection ID issued by an endpoint is the Sour

s/frames other than probing/non-probing/

+The initial connection ID issued by an endpoint is the Sour

I think that we can put this under migration safely.

+The initial connection ID issued by an endpoint is the Sour

"at any time after it accepts a connection."  Handshake completion is not a prerequisite because the client will believe that the handshake is complete before the server does.

+The initial connection ID issued by an endpoint is the Sour

I don't think that we want special rules here, so we should use "probing"/"non-probing" terms.  This looks like it is creating special rules, where I don't think we need any.  It's only this sentence, the rest is good.

+The initial connection ID issued by an endpoint is the Sour

s/to//

+The initial connection ID issued by an endpoint is the Sour

Can we move this under the Acknowledgments section?  We really need to break this section up a little better, but I can't comment on it here, so I'll come back to this.

+In the closing state, only a packet containing a closing frame can be sent.  An

This first sentence is duplicative, maybe drop it and merge this paragraph with the previous.

-An endpoint also MUST NOT initiate connection migration if the peer sent the
-`disable_migration` transport parameter during the handshake.  An endpoint which
-has sent this transport parameter, but detects that a peer has nonetheless
-migrated to a different network MAY treat this as a connection error of type
-INVALID_MIGRATION.
+This design relies on the peer always sending a connection ID in its packets so
+that the endpoint can use the connection ID from a packet to reset the
+connection.  An endpoint that uses this design MUST either use the same
+connection ID length for all connections or encode the length of the connection
+ID such that it can be recovered without state.  In addition, it MUST NOT

s/MUST NOT/cannot/

-When migrating, the new path might not support the endpoint's current sending
-rate. Therefore, the endpoint resets its congestion controller, as described in
-{{migration-cc}}.
+Any QUIC packet, with the exception of the Version Negotiation packet, has

I think that we can make this multiple paragraphs.  It's also lacking a little bit of context.  How about:

> QUIC endpoints communicate by exchanging packets.  Packets are carried in UDP datagrams (see {{packet-coalesce}}) and have confidentiality and integrity protection (see {{packet-protected}}).
> 
> This version of QUIC uses the long packet header (see {{header-long}}) during connection establishment and the short header (see {{header-short}}) once 1-RTT keys have been established.
>
> Packets that carry the long header are Initial [...].
>
> Packets with the short header are designed for minimal overhead and are used after a connection is established.
>
> Version negotiation uses a packet with a special format (see {{packet-version}}).

+All QUIC packets use packet protection.  Packets that are protected with the

This bit could be a better, I think.

> All QUIC packets except Version Negotiation and Retry packets use authenticated encryption with additional data (AEAD) {{!RFC5119}} to provide confidentiality and integrity protection.  Details of packet protection are found in {{QUIC-TLS}}; this section includes an overview of the process.
>
> Initial packets are protected using keys that are statically derived.  This packet protection is not effective confidentiality protection, it only exists to ensure that the sender of the packet is on the network path.  Any entity that receives the Initial packet from a client can recover the keys necessary to remove packet protection or to generate packets that will be successfully authenticated.
>
> All other packets are protected with keys derived from the cryptographic handshake.  The type of the packet from the long header or key phase from the short header are used to identify which encryption level - and therefore the keys - that are used. Packets protected with 0-RTT and 1-RTT keys are expected to have confidentiality and data origin authentication; the cryptographic handshake ensures that only the communicating endpoints receive the corresponding keys.

That covers the first few paragraphs here.

-An endpoint MAY send data to an unvalidated peer address, but it MUST protect
-against potential attacks as described in {{address-spoofing}} and
-{{on-path-spoofing}}.  An endpoint MAY skip validation of a peer address if that
-address has been seen recently.
+The version field for protected packets is the current QUIC version.

I don't think that we need to say this.

-In response to such a packet, an endpoint MUST start sending subsequent packets
-to the new peer address and MUST initiate path validation ({{migrate-validate}})
-to verify the peer's ownership of the unvalidated address.
+Packets protected with 0-RTT keys use a type value of 0x7C.  The connection ID
+fields for a 0-RTT packet MUST match the values used in the Initial packet
+({{packet-initial}}).

We can drop this paragraph.

-After changing the address to which it sends non-probing packets, an endpoint
-could abandon any path validation for other addresses.
+The payload is protected using authenticated encryption.  {{QUIC-TLS}} describes
+packet protection in detail.  After decryption, the plaintext consists of a
+sequence of frames, as described in {{frames}}.

With the above edits, this paragraph is redundant too.

+A sender can coalesce multiple QUIC packets (typically a Cryptographic Handshake
+packet and a Protected packet) into one UDP datagram.  This can reduce the

"Cryptographic Handshake" and "Protected" aren't defined terms.  I would drop the parenthetical.

+A sender can coalesce multiple QUIC packets (typically a Cryptographic Handshake
+packet and a Protected packet) into one UDP datagram.  This can reduce the
+number of UDP datagrams needed to send application data during the handshake and
+immediately afterwards. It is not necessary for senders to coalesce

> [...] number of UDP datagrams needed to complete the cryptographic handshake and starting sending data.

+A sender can coalesce multiple QUIC packets (typically a Cryptographic Handshake
+packet and a Protected packet) into one UDP datagram.  This can reduce the
+number of UDP datagrams needed to send application data during the handshake and
+immediately afterwards. It is not necessary for senders to coalesce
+packets, though failing to do so will require sending a significantly
+larger number of datagrams during the handshake. Receivers MUST

Strike the "It is not necessary" sentence.  The benefits speak for themselves without making hard to test assertions ("significantly larger" doesn't really help, the actual amount is pretty simple to work out).

-It is possible that a peer is spoofing its source address to cause an endpoint
-to send excessive amounts of data to an unwilling host.  If the endpoint sends
-significantly more data than the spoofing peer, connection migration might be
-used to amplify the volume of data that an attacker can generate toward a
-victim.
+Senders MUST NOT coalesce QUIC packets with different Destination Connection
+IDs into a single UDP datagram. Receivers SHOULD ignore any subsequent packets

s/with different Destination Connection IDs/for different connections/

Yes, this introduces pretty strong linkability (maybe) for multiple different connection IDs, but we tripped up against this in NYC and the requirement is same-connection, not same-connection-id.

> -congestion window's worth of data per estimated round-trip time (kMinimumWindow,
-as defined in {{QUIC-RECOVERY}}).  In the absence of this limit, an endpoint
-risks being used for a denial of service attack against an unsuspecting victim.
-Note that since the endpoint will not have any round-trip time measurements to
-this address, the estimate SHOULD be the default initial value (see
-{{QUIC-RECOVERY}}).
+Every QUIC packet that is coalesced into a single UDP datagram is separate and
+complete.  Though the values of some fields in the packet header might be
+redundant, no fields are omitted.  The receiver of coalesced QUIC packets MUST
+individually process each QUIC packet and separately acknowledge them, as if
+they were received as the payload of different UDP datagrams.  If one or more
+packets in a datagram cannot be processed yet (because the keys are not yet
+available) or processing fails (decryption failure, unknown type, etc.), the
+receiver MUST still attempt to process the remaining packets.  The skipped
+packets MAY either be discarded or buffered for later processing, just as if the
+packets were received out-of-order in separate datagrams.

Note to transfer #1844 here once that resolves.

-If an endpoint skips validation of a peer address as described in
-{{migration-response}}, it does not need to limit its sending rate.
+Retry ({{packet-retry}}) and Version Negotiation ({{packet-version}}) packets
+cannot be coalesced.

Should we change this to say that Retry packets, VN packets, and packets with a short header cannot be followed by other packets in the same datagram?  Because you could technically put Retry after an Initial, as much as that doesn't make any sense.

-#### Handling Address Spoofing by an On-path Attacker {#on-path-spoofing}
+## Connection ID Encoding

I think that we can merge this section with the earlier section on connection IDs or maybe the handshaking section (because it deals with negotiating connection ID).  It's got some good stuff, but it doesn't really belong here.

Also, the title should be "Negotiating Connection IDs" or something like that.

-This eliminates the use of the connection ID for linking activity from
-the same connection on different networks.  Protection of packet numbers ensures
-that packet numbers cannot be used to correlate activity.  This does not prevent
-other properties of packets, such as timing and size, from being used to
-correlate activity.
+A Version Negotiation packet ({{packet-version}}) does not include a packet
+number.  The Retry packet ({{packet-retry}}) has special rules for populating
+the packet number field.

Bug: Retry doesn't have a packet number either.

-Using a stable connection ID on multiple network paths allows a passive observer
-to correlate activity between those paths.  An endpoint that moves between
-networks might not wish to have their activity correlated by any entity other
-than their peer, so different connection IDs are used when sending from
-different local addresses, as discussed in {{connection-id}}.  For this to be
-effective endpoints need to ensure that connections IDs they provide cannot be
-linked by any other entity.
+The packet number is an integer in the range 0 to 2^62-1, present in all long

Same comment as before regarding the 2^62 thing.  Also, packet numbers are not present in all packets.

-Migrating a connection to a new server address mid-connection is left for future
-work. If a client receives packets from a new server address not indicated by
-the preferred_address transport parameter, the client SHOULD discard these
-packets.
+In the QUIC long and short packet headers, the number of bits required to

I think that we need to move this paragraph and the table, maybe a little more to the "Packet Formats" section, with a forward reference from here.  This is very much bits and bytes.

I think that there are two sections: Packet Number Encoding, and Packet Number Recovery.

-Once the handshake is finished, the client SHOULD initiate path validation (see
-{{migrate-validate}}) of the server's preferred address using the connection ID
-provided in the preferred_address transport parameter.
+The encoded packet number is protected as described in Section 5.3

missing "of": Section 5.3 of {{QUIC-TLS}}

-Once the handshake is finished, the client SHOULD initiate path validation (see
-{{migrate-validate}}) of the server's preferred address using the connection ID
-provided in the preferred_address transport parameter.
+The encoded packet number is protected as described in Section 5.3

This starts the section of Packet Number Recovery, which belongs below, I think.

+The sender MUST use a packet number size able to represent more than twice as

Packet number encoding again, I think.

-The server SHOULD also initiate path validation of the client using its
-preferred address and the address from which it received the client probe.  This
-helps to guard against spurious migration initiated by an attacker.
+A receiver MUST discard a newly unprotected packet unless it is certain that it

We should keep this up-top.  It's important.

-### Interaction of Client Migration and Preferred Address
+The payload of all packets, after removing packet protection, consists of a

Not all packets have frames.  We need to finesse this a little.

-### Interaction of Client Migration and Preferred Address
+The payload of all packets, after removing packet protection, consists of a
+sequence of frames, as shown in {{packet-frames}}.  Version Negotiation and
+Stateless Reset do not contain frames.

Nor does Retry.

-Connections should remain open until they become idle for a pre-negotiated
-period of time.  A QUIC connection, once established, can be terminated in one
-of three ways:
+| Type Value  | Frame Type Name      | Definition                     |
+|:------------|:---------------------|:-------------------------------|
+| 0x00        | PADDING              | {{frame-padding}}              |

Let's reorder these too, but after this big shuffle.

-The draining state is entered once an endpoint receives a signal that its peer
-is closing or draining.  While otherwise identical to the closing state, an
-endpoint in the draining state MUST NOT send any packets.  Retaining packet
-protection keys is unnecessary once a connection is in the draining state.
+A sender SHOULD minimize per-packet bandwidth and computational costs by

This SHOULD isn't an interop requirement, I think that can s/SHOULD minimize/minimizes/

-Disposing of connection state prior to the end 

Do we need to say anything about partial processing.  And our expectations about what implementations do with packets that have errors after valid frames?

-Disposing of connection state prior to the end 

I think that we can strike this implicit bit now.

-Disposing of connection state prior to the end 

We should put the varint encoding right at the top of this section.  It already deals with network byte order, so the extra text on integer encoding naturally fits.

-Disposing of connection state prior to the end 

Do we use this convention?  I tend to call the MSB bit 0 myself...

-Disposing of connection state prior to the end 

We can probably drop this and say that "The Length field enables packet coalescing ({{packet-coalesce}})." in the preceding paragraph instead.

-Disposing of connection state prior to the end 

missing close paren

-Disposing of connection state prior to the end 

Maybe as a follow-up we can fix the description of the long header.  We have 3 formats and only 2 of the 4 types use the generic format we show here.  Not sure what to do about it exactly, but it's a little confusing to have such a high amount of variance from what we put forth as the baseline format.  The same applies when implementing this.

-Disposing of connection state prior to the end 

I think that we need to break this stuff up a little.  Maybe this is as simple as having "Sending Retry" and "Processing Retry" sections, but it's pretty hard to access right now.

-Disposing of connection state prior to the end 

I don't think that this sub-heading helps any.  Initial and Handshake don't share a format and don't really need to have a lead-in.  More critically, I think that Initial belongs before Retry (and Version Negotiation even).

The text that is here probably belongs with the connection establishment section.

-Disposing of connection state prior to the end 

This can go with Initial packets.

-Disposing of connection state prior to the end 

Remember that I mentioned a "Negotiating Connection IDs" section above.  Merge this with that section.  I don't think that it belongs in the nuts-and-bolts sections.

-Disposing of connection state prior to the end 

This can be moved to the address validation section.

-Disposing of connection state prior to the end 

We covered this in packet number spaces already.  Maybe it can go.

-Disposing of connection state prior to the end 

Move to section on packet number sizes.  (I think that it can be deleted now).

-Disposing of connection state prior to the end 

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.

-Disposing of connection state prior to the end 

This is old text, I think.  It doesn't even mention Initial, which we now need before any Handshake packets can be sent.

-Disposing of connection state prior to the end 

Move this and the next paragraph to address validation section.

We might also need a xref from the packet size section to that section as well.

-Disposing of connection state prior to the end 

We use this encoding in packet headers, so this really needs to be earlier, as I said.

> @@ -208,1721 +217,1890 @@ x (*) ...
 : Indicates that x is variable-length

This section on encoding and pictures could be moved much later in the document.  We don't really use this until we start describing the wire format, and if we had a section on notation conventions before we got into packet formats and the like, we could put this with the note on network byte order and varint encoding.

-Disposing of connection state prior to the end 

Error codes and versions.

-# Packetization and Reliability {#packetization}
+Transmission errors, timeouts and retransmissions might cause the same
+NEW_CONNECTION_ID frame to be received multiple times.  Receipt of the same
+frame multiple times MUST NOT be treated as a connection error.  A receiver can
+use the sequence number supplied in the NEW_CONNECTION_ID frame to identify new
+connection IDs from old ones.

I think that we can remove the first two sentences of this paragraph.  They are vestiges of the design prior to us putting sequence numbers back in.

> -signal that it wishes to abandon transmission of stream data.  Similarly, the
-endpoint might receive a STOP_SENDING frame from its peer.  In either case, the
-endpoint sends a RST_STREAM frame, which causes the stream to enter the "Reset
-Sent" state.
-
-An endpoint MAY send a RST_STREAM as the first frame on a send stream; this
-causes the send stream to open and then immediately transition to the "Reset
-Sent" state.
-
-Once a packet containing a RST_STREAM has been acknowledged, the send stream
-enters the "Reset Recvd" state, which is a terminal state.
-
-
-### Receive Stream States {#stream-recv-states}
-
-{{fig-s

We should move the remainder of this section to the acknowledgments section.

> -signal that it wishes to abandon transmission of stream data.  Similarly, the
-endpoint might receive a STOP_SENDING frame from its peer.  In either case, the
-endpoint sends a RST_STREAM frame, which causes the stream to enter the "Reset
-Sent" state.
-
-An endpoint MAY send a RST_STREAM as the first frame on a send stream; this
-causes the send stream to open and then immediately transition to the "Reset
-Sent" state.
-
-Once a packet containing a RST_STREAM has been acknowledged, the send stream
-enters the "Reset Recvd" state, which is a terminal state.
-
-
-### Receive Stream States {#stream-recv-states}
-
-{{fig-s

Same here.

> -signal that it wishes to abandon transmission of stream data.  Similarly, the
-endpoint might receive a STOP_SENDING frame from its peer.  In either case, the
-endpoint sends a RST_STREAM frame, which causes the stream to enter the "Reset
-Sent" state.
-
-An endpoint MAY send a RST_STREAM as the first frame on a send stream; this
-causes the send stream to open and then immediately transition to the "Reset
-Sent" state.
-
-Once a packet containing a RST_STREAM has been acknowledged, the send stream
-enters the "Reset Recvd" state, which is a terminal state.
-
-
-### Receive Stream States {#stream-recv-states}
-
-{{fig-s

This paragraph and the next probably belong in the packetization 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/1857#pullrequestreview-164539169

---

From: janaiyengar <[email-masked]>
Date: Mon, 15 Oct 2018 19:15:59 -0700

janaiyengar commented on this pull request.

I'm merging this subsection into the data limits increment 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/1857#discussion_r225375520

---

From: ianswett <[email-masked]>
Date: Tue, 16 Oct 2018 13:53:20 -0700

ianswett commented on this pull request.

I like two levels

-- 
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/1857#discussion_r225703729

---

From: martinduke <[email-masked]>
Date: Tue, 16 Oct 2018 14:10:53 -0700

If people are OK with the general layout, I suggest we land this PR quickly (to unblock other PR) and then wordsmith the nits in future PRs.

-- 
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/1857#issuecomment-430402253

---

From: janaiyengar <[email-masked]>
Date: Tue, 16 Oct 2018 15:10:47 -0700

@janaiyengar pushed 2 commits.

438b965  modified intro, made error codes first level section
53f0bad  Done through connection migration section

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1857/files/c4ce2c29be13de5146556c2a49ef3c8aa57cc317..53f0badcfab54967edd5f3c28b5ad43e2b03bb90

---

From: martinduke <[email-masked]>
Date: Wed, 17 Oct 2018 15:52:51 -0700

martinduke requested changes on this pull request.

Here is a review through and including the flow control section. Hopefully more to come,.

 Accompanying documents describe QUIC's loss detection and congestion control
 {{QUIC-RECOVERY}}, and the use of TLS 1.3 for key negotiation {{QUIC-TLS}}.
 
 QUIC version 1 conforms to the protocol invariants in {{QUIC-INVARIANTS}}.

-# Conventions and Definitions
+## Conventions and Definitions

The definition of "stream" says they're bi-directional. That's not accurate. Maybe "a logical channel of ordered bytes within a QUIC connection, that can be unidirectional or bidirectional"?

-Version:
+A receiver cannot renege on an advertisement; that is, once a receiver
+advertises a stream ID via a MAX_STREAM_ID frame, advertising a smaller maximum
+ID has no effect.  A sender MUST ignore any MAX_STREAM_ID frame that does not

s/sender/receiver

-Destination Connection ID:
+When new data is to be sent on a stream, a sender MUST set the encapsulating
+STREAM frame's offset field to the stream offset of the first byte of this new

s/byte/octet

-All subsequent Initial packets from the client MUST use the connection ID and
-token values from the Retry packet.  Aside from this, the Initial packet sent
-by the client is subject to the same restrictions as the first Initial packet.
-A client can either reuse the cryptographic handshake message or construct a
-new one at its discretion.
+A data receiver sends MAX_STREAM_DATA or MAX_DATA frames to the sender to

I think we should add a paragraph here that goes like this:

"Endpoints SHOULD communicate initial values for stream and connection flow control using transport parameters {#transport-params}. Data senders MUST use a value of zero for limits not advertised in the peer's transport parameters." 

-- 
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/1857#pullrequestreview-165371333

---

From: janaiyengar <[email-masked]>
Date: Wed, 17 Oct 2018 19:33:25 -0700

@janaiyengar pushed 1 commit.

9600d97  addresses mt's comments.

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1857/files/53f0badcfab54967edd5f3c28b5ad43e2b03bb90..9600d976e421bb846fdf85831e3864ca16300af8

---

From: janaiyengar <[email-masked]>
Date: Wed, 17 Oct 2018 20:01:54 -0700

209a269  detailed structure in intro

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1857/files/9600d976e421bb846fdf85831e3864ca16300af8..209a2699855b8f8bf3fce6691cd9a64ee661ccbf

---

From: janaiyengar <[email-masked]>
Date: Wed, 17 Oct 2018 20:06:43 -0700

d927b6a  most of martinduke comments

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1857/files/209a2699855b8f8bf3fce6691cd9a64ee661ccbf..d927b6af974a164f9fc14252f20997757fd4ef14

---

From: janaiyengar <[email-masked]>
Date: Wed, 17 Oct 2018 20:13:40 -0700

af51da7  added initial values in flow control

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

---

From: janaiyengar <[email-masked]>
Date: Wed, 17 Oct 2018 20:16:24 -0700

d5a1e49  lint

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

---

From: Martin Thomson <[email-masked]>
Date: Thu, 18 Oct 2018 02:01:25 -0700

martinthomson approved this pull request.

I've tried to attach suggestions to every comment here.  Let me know if that works out, it's a new GitHub feature, and a little rough.  I'd appreciate if you look at the suggestions before you do, but please merge this ASAP.

The paragraph above looks good enough to me.  I don't think we need to set defaults (if this were the only thing that is read, a reader would (mostly correctly) infer that the values are always set.

> +A receiver MAY advertise a larger offset at any point by sending MAX_STREAM_DATA
+or MAX_DATA frames.  A receiver cannot renege on an advertisement; that is, once
+a receiver advertises an offset, advertising a smaller offset has no effect.  A
+sender MUST therefore ignore any MAX_STREAM_DATA or MAX_DATA frames that do not
+increase flow control limits.
+
+A receiver MUST close the connection with a FLOW_CONTROL_ERROR error
+({{error-handling}}) if the peer violates the advertised connection or stream
+data limits.
+
+A sender SHOULD send STREAM_BLOCKED or BLOCKED frames to indicate it has data to
+write but is blocked by flow control limits.  These frames are expected to be
+sent infrequently in common cases, but they are considered useful for debugging
+and monitoring purposes.
+
+(TODO: Add something about max_stream_id)

```suggestion
A similar method is used to control the number of open streams
see {{stream-limit-increment}} for details.
```

> +willing to receive, the connection will be torn down when too much data arrives.
+Conversely if a sender believes it is blocked, while endpoint B expects more
+data can be received, then the connection can be in a deadlock, with the sender
+waiting for a MAX_STREAM_DATA or MAX_DATA frame which will never come.
+
+On receipt of a RST_STREAM frame, an endpoint will tear down state for the
+matching stream and ignore further data arriving on that stream.  This could
+result in the endpoints getting out of sync, since the RST_STREAM frame may have
+arrived out of order and there may be further bytes in flight.  The data sender
+would have counted the data against its connection level flow control budget,
+but a receiver that has not received these bytes would not know to include them
+as well.  The receiver must learn the number of bytes that were sent on the
+stream to make the same adjustment in its connection flow controller.
+
+To ensure that endpoints maintain a consistent connection-level flow control
+state, the RST_STREAM frame {{frame-rst-stream}} includes the largest offset of

```suggestion
state, the RST_STREAM frame ({{frame-rst-stream}}) includes the largest offset of
```

> +is not excessive buffering at multiple layers.
+
+
+## Stream Limit Increment {#stream-limit-increment}
+
+An endpoint limits the number of concurrently active incoming streams by
+limiting the maximum stream ID.  An initial value is set in the transport
+parameters (see {{transport-parameter-definitions}}) and is subsequently
+increased by MAX_STREAM_ID frames (see {{frame-max-stream-id}}).
+
+As with stream and connection flow control, this document leaves when and how
+many streams to make available to a peer via MAX_STREAM_ID to implementations,
+but offers a few considerations.  MAX_STREAM_ID frames constitute minimal
+overhead, while withholding MAX_STREAM_ID frames can prevent the peer from using
+the available parallelism.
+

```suggestion

The STREAM_ID_BLOCKED frame ({{frame-stream-id-blocked}}) can be
used to signal a shortage of available streams.
```

> +
+As with stream and connection flow control, this document leaves when and how
+many streams to make available to a peer via MAX_STREAM_ID to implementations,
+but offers a few considerations.  MAX_STREAM_ID frames constitute minimal
+overhead, while withholding MAX_STREAM_ID frames can prevent the peer from using
+the available parallelism.
+
+Implementations will likely want to increase the maximum stream ID as
+peer-initiated streams close.  A receiver MAY also advance the maximum stream ID
+based on current activity, system conditions, and other environmental factors.
+
+
+# Connections {#connections}
+
+A QUIC connection is a single conversation between two QUIC endpoints.  QUIC's
+connection establishment intertwines version negotiation with the cryptographic

```suggestion
connection establishment combines version negotiation with the cryptographic
```

> +but offers a few considerations.  MAX_STREAM_ID frames constitute minimal
+overhead, while withholding MAX_STREAM_ID frames can prevent the peer from using
+the available parallelism.
+
+Implementations will likely want to increase the maximum stream ID as
+peer-initiated streams close.  A receiver MAY also advance the maximum stream ID
+based on current activity, system conditions, and other environmental factors.
+
+
+# Connections {#connections}
+
+A QUIC connection is a single conversation between two QUIC endpoints.  QUIC's
+connection establishment intertwines version negotiation with the cryptographic
+and transport handshakes to reduce connection establishment latency, as
+described in {{handshake}}.  Once established, a connection may migrate to a
+different IP or port at either endpoint, due to NAT rebinding or mobility, as

```suggestion
different IP or port at either endpoint as
```

> +based on current activity, system conditions, and other environmental factors.
+
+
+# Connections {#connections}
+
+A QUIC connection is a single conversation between two QUIC endpoints.  QUIC's
+connection establishment intertwines version negotiation with the cryptographic
+and transport handshakes to reduce connection establishment latency, as
+described in {{handshake}}.  Once established, a connection may migrate to a
+different IP or port at either endpoint, due to NAT rebinding or mobility, as
+described in {{migration}}.  Finally, a connection may be terminated by either
+endpoint, as described in {{termination}}.
+
+## Connection ID {#connection-id}
+
+Each connection possesses a set of identifiers, any of which could be used to

```suggestion
Each connection possesses a set of connection identifiers, or connection IDs,
each of which can be identify the connection.  Connection IDs are independently
selected by endpoints; each endpoint selects the connection IDs that its peer
uses.
```

Multi-line suggestion, sorry, I don't know how to hit the entire paragraph.

> +
+
+# Connections {#connections}
+
+A QUIC connection is a single conversation between two QUIC endpoints.  QUIC's
+connection establishment intertwines version negotiation with the cryptographic
+and transport handshakes to reduce connection establishment latency, as
+described in {{handshake}}.  Once established, a connection may migrate to a
+different IP or port at either endpoint, due to NAT rebinding or mobility, as
+described in {{migration}}.  Finally, a connection may be terminated by either
+endpoint, as described in {{termination}}.
+
+## Connection ID {#connection-id}
+
+Each connection possesses a set of identifiers, any of which could be used to
+distinguish it from other connections.  Connection IDs are selected

```suggestion
```

> +
+# Connections {#connections}
+
+A QUIC connection is a single conversation between two QUIC endpoints.  QUIC's
+connection establishment intertwines version negotiation with the cryptographic
+and transport handshakes to reduce connection establishment latency, as
+described in {{handshake}}.  Once established, a connection may migrate to a
+different IP or port at either endpoint, due to NAT rebinding or mobility, as
+described in {{migration}}.  Finally, a connection may be terminated by either
+endpoint, as described in {{termination}}.
+
+## Connection ID {#connection-id}
+
+Each connection possesses a set of identifiers, any of which could be used to
+distinguish it from other connections.  Connection IDs are selected
+independently in each direction.

```suggestion
```

> +and MUST NOT send packets from any other local address.
+
+When an endpoint has requested a non-zero-length connection ID, it needs to
+ensure that the peer has a supply of connection IDs from which to choose for
+packets sent to the endpoint.  These connection IDs are supplied by the endpoint
+using the NEW_CONNECTION_ID frame ({{frame-new-connection-id}}).
+
+
+### Issuing Connection IDs
+
+Each Connection ID has an associated sequence number to assist in deduplicating
+messages.  The initial connection ID issued by an endpoint is sent in the Source
+Connection ID field of the long packet header ({{long-header}}) during the
+handshake.  The sequence number of the initial connection ID is 0.  If the
+preferred_address transport parameter is sent, the sequence number of the
+supplied connection ID is 1. Subsequent connection IDs are communicated to the

```suggestion
supplied connection ID is 1.

Additional connection IDs are communicated to the
```

> +
+When an endpoint has requested a non-zero-length connection ID, it needs to
+ensure that the peer has a supply of connection IDs from which to choose for
+packets sent to the endpoint.  These connection IDs are supplied by the endpoint
+using the NEW_CONNECTION_ID frame ({{frame-new-connection-id}}).
+
+
+### Issuing Connection IDs
+
+Each Connection ID has an associated sequence number to assist in deduplicating
+messages.  The initial connection ID issued by an endpoint is sent in the Source
+Connection ID field of the long packet header ({{long-header}}) during the
+handshake.  The sequence number of the initial connection ID is 0.  If the
+preferred_address transport parameter is sent, the sequence number of the
+supplied connection ID is 1. Subsequent connection IDs are communicated to the
+peer using NEW_CONNECTION_ID frames ({{frame-new-connection-id}}), and the

```suggestion
peer using NEW_CONNECTION_ID frames ({{frame-new-connection-id}}).  The
```

-The header form and connection ID field of a short header packet are
-version-independent.  The remaining fields are specific to the selected QUIC
-version.  See {{QUIC-INVARIANTS}} for details on how packets from different
-versions of QUIC are interpreted.
+<!-- TODO: We are missing a paragraph on failed packet protection removal

```suggestion
Packets that are matched to an existing connection, but for which the endpoint
cannot remove packet protection, are discarded.
```

-The header form and connection ID field of a short header packet are
-version-independent.  The remaining fields are specific to the selected QUIC
-version.  See {{QUIC-INVARIANTS}} for details on how packets from different
-versions of QUIC are interpreted.
+<!-- TODO: We are missing a paragraph on failed packet protection removal
+here. We probably want that.-->

```suggestion
```

-## Version Negotiation Packet {#packet-version}
+Valid packets sent to clients always include a Destination Connection ID that
+matches the value the client selects.  Clients that choose to receive

```suggestion
matches a value the client selects.  Clients that choose to receive
```

-The sender MUST use a packet number size able to represent more than twice as
-large a range than the difference between the largest acknowledged packet and
-packet number being sent.  A peer receiving the packet will then correctly
-decode the packet number, unless the packet is delayed in transit such that it
-arrives after many higher-numbered packets have been received.  An endpoint
-SHOULD use a large enough packet number encoding to allow the packet number to
-be recovered even if the packet arrives after packets that are sent afterwards.
+{{tls-0rtt-handshake}} shows an example of a connection with a 0-RTT handshake
+and a single packet of 0-RTT data. Note that as described in {{packet-numbers}},
+the server ACKs the 0-RTT data at the 1-RTT encryption level, and the client's

```suggestion
the server acknowledges 0-RTT data at the 1-RTT encryption level, and the
client sends 1-RTT packets in the same packet number space.
```

-The sender MUST use a packet number size able to represent more than twice as
-large a range than the difference between the largest acknowledged packet and
-packet number being sent.  A peer receiving the packet will then correctly
-decode the packet number, unless the packet is delayed in transit such that it
-arrives after many higher-numbered packets have been received.  An endpoint
-SHOULD use a large enough packet number encoding to allow the packet number to
-be recovered even if the packet arrives after packets that are sent afterwards.
+{{tls-0rtt-handshake}} shows an example of a connection with a 0-RTT handshake
+and a single packet of 0-RTT data. Note that as described in {{packet-numbers}},
+the server ACKs the 0-RTT data at the 1-RTT encryption level, and the client's
+sequence numbers at the 1-RTT encryption level continue to increment from its

```suggestion
```

-The sender MUST use a packet number size able to represent more than twice as
-large a range than the difference between the largest acknowledged packet and
-packet number being sent.  A peer receiving the packet will then correctly
-decode the packet number, unless the packet is delayed in transit such that it
-arrives after many higher-numbered packets have been received.  An endpoint
-SHOULD use a large enough packet number encoding to allow the packet number to
-be recovered even if the packet arrives after packets that are sent afterwards.
+{{tls-0rtt-handshake}} shows an example of a connection with a 0-RTT handshake
+and a single packet of 0-RTT data. Note that as described in {{packet-numbers}},
+the server ACKs the 0-RTT data at the 1-RTT encryption level, and the client's
+sequence numbers at the 1-RTT encryption level continue to increment from its
+0-RTT packets.

```suggestion
```

-After sending a closing frame, endpoints immediately enter the closing state.
-During the closing period, an endpoint that sends a closing frame SHOULD respond
-to any packet that it receives with another packet containing a closing frame.

```suggestion
  {{QUIC-RECOVERY}}, until either all data has been acknowledged.
```

-After sending a closing frame, endpoints immediately enter the closing state.
-During the closing period, an endpoint that sends a closing frame SHOULD respond
-to any packet that it receives with another packet containing a closing frame.

```suggestion
```

-After sending a closing frame, endpoints immediately enter the closing state.
-During the closing period, an endpoint that sends a closing frame SHOULD respond
-to any packet that it receives with another packet containing a closing frame.

```suggestion
```

I think that this is fine as it is.  If it were longer, maybe, but it's a nice size now and the two concepts are very closely related.

-After sending a closing frame, endpoints immediately enter the closing state.
-During the closing period, an endpoint that sends a closing frame SHOULD respond
-to any packet that it receives with another packet containing a closing frame.

```suggestion

<!-- TODO: delete this section after confirming that it is redundant -->
```

-After sending a closing frame, endpoints immediately enter the closing state.
-During the closing period, an endpoint that sends a closing frame SHOULD respond
-to any packet that it receives with another packet containing a closing frame.

```suggestion

<!-- TODO: split out definition of PreferredAddress -->

```

-After sending a closing frame, endpoints immediately enter the closing state.
-During the closing period, an endpoint that sends a closing frame SHOULD respond
-to any packet that it receives with another packet containing a closing frame.

I'm not sure that I like the fact that the specific transport parameters are described way back up top now.  I think that having the description of each down here is better.  There's a lot of encoding information in them, and we don't generally rely on the definitions of transport parameters for much.  Anything we do need can use a forward reference.

-After sending a closing frame, endpoints immediately enter the closing state.
-During the closing period, an endpoint that sends a closing frame SHOULD respond
-to any packet that it receives with another packet containing a closing frame.

```suggestion
```

-After sending a closing frame, endpoints immediately enter the closing state.
-During the closing period, an endpoint that sends a closing frame SHOULD respond
-to any packet that it receives with another packet containing a closing frame.

```suggestion
```

-After sending a closing frame, endpoints immediately enter the closing state.
-During the closing period, an endpoint that sends a closing frame SHOULD respond
-to any packet that it receives with another packet containing a closing frame.

```suggestion
```

-After sending a closing frame, endpoints immediately enter the closing state.
-During the closing period, an endpoint that sends a closing frame SHOULD respond
-to any packet that it receives with another packet containing a closing frame.

```suggestion
```

-After sending a closing frame, endpoints immediately enter the closing state.
-During the closing period, an endpoint that sends a closing frame SHOULD respond
-to any packet that it receives with another packet containing a closing frame.

```suggestion
An endpoint can use the sequence number supplied in the NEW_CONNECTION_ID frame
to identify connection IDs that it has already received.
```

-- 
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/1857#pullrequestreview-165972401

---

From: janaiyengar <[email-masked]>
Date: Thu, 18 Oct 2018 14:56:24 +0000 (UTC)

At least 1200 is what we have elsewhere. And we have at most 1280 for IP packets without PMTU, which leads to 1232 payload. We may want to reconsider the 1280 limit.

There is text above the diagram. Did you mean something else?

I like this. We already have a "Connections" section, and we can add a lifecycle subsection there that goes into the way the different subsequent sections are connected. We'll also have to slightly shift the focus of the validation section from address to address and path validation, which is fine I think. FWIW, for now, I'll move this to a top level section, add a connection lifecycle placeholder in the Connections section, and leave path validation where it is for now. We can see how well this works out in the next iteration.

SGTM. 

Good point; added.

agreed. added a TODO.

What won't include non-probing frames? Also, non-probing frames are defined below.

Removing "to" doesn't make sense. But I've re-phrased the sentence.

I don't think this belongs under the Acknowledgment section, but I'm not thrilled with this here either. I'm leaving a TODO, to come back to this.

I am taking this in. I don't think we want to be dealing with coalescing packets from multiple connections, certainly not in v1.

Added. I've already answered the question of coalescing multiple short header packets more than once. It'll be good to deal with it too.

Moved to handshake section for now, with a TODO to check if it's the right place.

I've moved a bunch to Packet Formats, under a new section called Packet Number Encoding and Decoding for now.

I don't think we can move this further up. We need to describe spaces first, and then there are only two rules -- sender MUST not send dups and receiver MUST not process dups.

What did you want to reorder here?

My brain is tired right now, but what do you want to change this to? If I send and receive 1-RTT packets, I can assume that all initial and handshake keyed packets do not need retransmission anymore, right?

Nope, I think this is legacy and incorrect

I think we need a different taxonomy. Perhaps "Special packets" are a different class of packet. I'll add a comment here, and let's revisit this.

I've moved the first para in the example hadnshake section, the last para into the Initial packet section, and removed this heading.

Yup, done.

Done. I've also reworked that section, and left a comment to take a second look at that text.

-- 
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/1857#discussion_r225382721

---

From: janaiyengar <[email-masked]>
Date: Thu, 18 Oct 2018 07:59:38 -0700

db2e9b2  max_stream_id

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

---

From: janaiyengar <[email-masked]>
Date: Thu, 18 Oct 2018 08:01:50 -0700

ca49ae4  parenthesize

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

---

From: janaiyengar <[email-masked]>
Date: Thu, 18 Oct 2018 08:02:08 -0700

0368ca9  parenthesize

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1857/files/ca49ae4cdf1b2470de3e0cfa3b1b0ab9e7b28a62..0368ca91704757d307072511cb38f0968b32f72a

---

From: janaiyengar <[email-masked]>
Date: Thu, 18 Oct 2018 08:02:58 -0700

1d4f7d3  stream-id-blocked frame

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1857/files/0368ca91704757d307072511cb38f0968b32f72a..1d4f7d395a4dc6877b6ed81a77fc7d9032680ff6

---

From: janaiyengar <[email-masked]>
Date: Thu, 18 Oct 2018 15:03:28 +0000 (UTC)

c3426ee  s/intertwines/combines

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1857/files/1d4f7d395a4dc6877b6ed81a77fc7d9032680ff6..c3426ee9042e67a9d8a4db4e34d22073e2c6cbdb

---

From: janaiyengar <[email-masked]>
Date: Thu, 18 Oct 2018 15:03:55 +0000 (UTC)

140a9c6  unnecessary text

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1857/files/c3426ee9042e67a9d8a4db4e34d22073e2c6cbdb..140a9c601c8a60967e3e06c6231231c5475f29b4

---

From: janaiyengar <[email-masked]>
Date: Thu, 18 Oct 2018 08:06:50 -0700

23d95c3  connection id para

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1857/files/140a9c601c8a60967e3e06c6231231c5475f29b4..23d95c3fc683976506d9465083038dbe32f994dc

---

From: janaiyengar <[email-masked]>
Date: Thu, 18 Oct 2018 08:07:01 -0700

d3cf19f  connection id contd

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1857/files/23d95c3fc683976506d9465083038dbe32f994dc..d3cf19fd880fc281e208e75899834ff241b5f7c6

---

From: janaiyengar <[email-masked]>
Date: Thu, 18 Oct 2018 08:07:08 -0700

4c9858f  connection id contd

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

---

From: janaiyengar <[email-masked]>
Date: Thu, 18 Oct 2018 08:07:34 -0700

0dfa885  s/subsequent/additional

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1857/files/4c9858f1a212fe7c7f871048389612a64dc2d922..0dfa88580bb9fc3aa32783624b60c4422d6b2b8f

---

From: janaiyengar <[email-masked]>
Date: Thu, 18 Oct 2018 15:07:50 +0000 (UTC)

a6262f9  shorter sentences

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1857/files/0dfa88580bb9fc3aa32783624b60c4422d6b2b8f..a6262f9116957886f682c5c3fb878afac512cbb8

---

From: janaiyengar <[email-masked]>
Date: Thu, 18 Oct 2018 08:08:49 -0700

b90815f  packet protection TODO

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

---

From: janaiyengar <[email-masked]>
Date: Thu, 18 Oct 2018 08:09:14 -0700

bbb241e  packet protection TODO contd

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

---

From: janaiyengar <[email-masked]>
Date: Thu, 18 Oct 2018 08:09:26 -0700

2a00e0b  s/the/a

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1857/files/bbb241e8e59cf45f6643f95aeee63dd1ad932718..2a00e0b7316c06bec7693abf1ad1408b3b6fa96b

---

From: janaiyengar <[email-masked]>
Date: Thu, 18 Oct 2018 13:24:48 -0700

2986d5b  s/acks/acknowledges

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1857/files/2a00e0b7316c06bec7693abf1ad1408b3b6fa96b..2986d5b90f1dd8317554d60e8b70c0e82871cd87

---

From: janaiyengar <[email-masked]>
Date: Thu, 18 Oct 2018 20:26:59 +0000 (UTC)

c631c31  unnecessary text

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1857/files/2986d5b90f1dd8317554d60e8b70c0e82871cd87..c631c3171ed7bead8240f83e3b48cd6eb52bf474

---

From: janaiyengar <[email-masked]>
Date: Thu, 18 Oct 2018 20:27:14 +0000 (UTC)

6a980bf  unnecessary text

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1857/files/c631c3171ed7bead8240f83e3b48cd6eb52bf474..6a980bfd5c8c1e8244045449ed7ed24ec7bf6301

---

From: janaiyengar <[email-masked]>
Date: Thu, 18 Oct 2018 14:10:28 -0700

d6c9267  last of mt's comments

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1857/files/6a980bfd5c8c1e8244045449ed7ed24ec7bf6301..d6c92675a9d3933838c0daa1b2dff1db17166012

---

From: janaiyengar <[email-masked]>
Date: Thu, 18 Oct 2018 14:13:06 -0700

e13b6b3  lint

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

---

From: janaiyengar <[email-masked]>
Date: Thu, 18 Oct 2018 14:16:07 -0700

There is one commit I'm not taking in, but everything else looks good.  I really wanted to try this new feature -- it is great, except that (i) every single comment is a separate commit, which doesn't scale well, and (ii) github UI pipeline is breaking somewhere, and shows incorrect text for suggested deletions at higher line numbers. The line numbers seem fine.

I added that paragraph based on martinduke's comment, and yes, I agree that we don't need to recommend setting defaults.

+
+

This has always been a limitation -- you can't comment on blocks. It seems to be leaking into this feature too.

I'm not taking this yet - I don't think this should be removed. Let's discuss this after this PR.

Something is broken here. This suggested patch doesn't make sense.

Again, this suggested patch does not make sense. I think you're talking about the TODO, which I agree, and I'll remove, but the suggested patch is incorrect. Looking closer, the line number is correct, but the text it shows is broken.

-- 
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/1857#pullrequestreview-166140557

---

From: janaiyengar <[email-masked]>
Date: Thu, 18 Oct 2018 21:21:14 +0000 (UTC)

dd74eda  lint

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

---

From: janaiyengar <[email-masked]>
Date: Thu, 18 Oct 2018 14:24:03 -0700

Merged #1857 into reorganize.

-- 
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/1857#event-1913388152