Re: [quicwg/base-drafts] Rewrite text about Version Negotiation (#1039)

[Martin Thomson <[e-mail]>]

martinthomson commented on this pull request.

This is much better.  I think that we can address the stateless VN/Retry thing pretty easily by prefacing the server-specific packet handling section appropriately (you already have the text in the right order).

> +Due to packet reordering or loss, clients might receive packets for a
+connection encrypted with a key it has not yet computed. Clients MAY drop
+these packets, or MAY buffer them in anticipation of later packets that
+allow it to compute the key.
+
+
+### Server-Specific Behaviors {#server-specific-behaviors}
+
+If a server receives a packet that has an unknown connection ID, an
+unsupported version, and a sufficient length to be an Initial packet for
+some version supported by the server, it SHOULD send a Version Negotiation
+packet as described in {{send-vn}}.
+
+Servers MUST drop other packets that contain unsupported versions.
+
+If the packet is a supported version, and an Initial packet fully

I think that you can remove the if (supported(version)) check from these paragraphs.  You already did the early return on !supported

> +the tuple of a connection with no received packets, it is a reply to an
+Initial packet with a server-generated connection ID and will be processed
+accordingly. Clients SHOULD discard any packets with new connection IDs that
+do not meet these criteria.
+
+Note that a successfully associated packet may be a Version Negotiation
+packet, which is handled in accordance with {{handle-vn}}.
+
+Due to packet reordering or loss, clients might receive packets for a
+connection encrypted with a key it has not yet computed. Clients MAY drop
+these packets, or MAY buffer them in anticipation of later packets that
+allow it to compute the key.
+
+
+### Server-Specific Behaviors {#server-specific-behaviors}
+

This is probably where you need to finesse the stateless generation of VN and Retry packets.

> -used - and they MUST be padded to at least 1200 octets.
-
-The server receives this packet and determines whether it potentially creates a
-new connection (see {{packet-handling}}).  If the packet might generate a new
-connection, the server then checks whether it understands the version that the
-client has selected.
-
-If the packet contains a version that is acceptable to the server, the server
-proceeds with the handshake ({{handshake}}).  This commits the server to the
-version that the client selected.
+with an existing connection, or - for servers - potentially create a new
+connection.
+
+First, hosts try to associate the packet with an existing connection. If the
+packet has a connection ID corresponding to an existing connection, QUIC
+processes that packet accordingly. Note that a NEW_CONNECTION_ID frame

I would agree, if it weren't for the point about stateless handling.  I think that's together it's worth addressing.  I think that all you need to do is put in a "Before attempting to identify an existing connection a server might process Initial packets and generate Version Negotiation or Retry packets.  Efficient and stateless generation of these packets can be important in reducing server load."  Or some such blather.

> +some version supported by the server, it SHOULD send a Version Negotiation
+packet as described in {{send-vn}}.
+
+Servers MUST drop other packets that contain unsupported versions.
+
+If the packet is a supported version, and an Initial packet fully
+conforming with the specification, the server proceeds with the handshake
+({{handshake}}).  This commits the server to the version that the client
+selected.
+
+If the packet is a supported version and a 0-RTT packet, the server MAY
+buffer a limited number of these packets in anticipation of a late-arriving
+Initial Packet. Clients are forbidden from sending Handshake packets prior
+to receiving a server response, so servers SHOULD ignore any such packets.
+
+Servers MUST drop incoming packets under all other circumstances.

What about regular packets for a connection?   I know that this section only lists the exceptional cases, but because it doesn't mention packets that are matched to a connection, this could be read as requiring servers to discard all packets that belong to connections.

> -{{handshake}}, but any Initial packet sent from the client to the server MUST
-use the long header format - which includes the version of the protocol being
-used - and they MUST be padded to at least 1200 octets.
-
-The server receives this packet and determines whether it potentially creates a
-new connection (see {{packet-handling}}).  If the packet might generate a new
-connection, the server then checks whether it understands the version that the
-client has selected.
-
-If the packet contains a version that is acceptable to the server, the server
-proceeds with the handshake ({{handshake}}).  This commits the server to the
-version that the client selected.
+with an existing connection, or - for servers - potentially create a new
+connection.
+
+First, hosts try to associate the packet with an existing connection. If the

Remove the first here and forward-reference the server-specific section.

> +If a client receives a packet with an unknown connection ID, and it matches
+the tuple of a connection with no received packets, it is a reply to an
+Initial packet with a server-generated connection ID and will be processed
+accordingly. Clients SHOULD discard any packets with new connection IDs that
+do not meet these criteria.
+
+Note that a successfully associated packet may be a Version Negotiation
+packet, which is handled in accordance with {{handle-vn}}.
+
+Due to packet reordering or loss, clients might receive packets for a
+connection encrypted with a key it has not yet computed. Clients MAY drop
+these packets, or MAY buffer them in anticipation of later packets that
+allow it to compute the key.
+
+
+### Server-Specific Behaviors {#server-specific-behaviors}

s/Behaviors/Packet Handling/

A shorter anchor might be nice 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/1039#pullrequestreview-96033938