Re: [quicwg/base-drafts] Massive batch of probably-editorial nits (#3805)

Mike Bishop <notifications@github.com> Wed, 01 July 2020 00:07 UTC

Return-Path: <noreply@github.com>
X-Original-To: quic-issues@ietfa.amsl.com
Delivered-To: quic-issues@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 7EBFA3A07ED for <quic-issues@ietfa.amsl.com>; Tue, 30 Jun 2020 17:07:49 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: 1.9
X-Spam-Level: *
X-Spam-Status: No, score=1.9 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, GB_SUMOF=5, HTML_MESSAGE=0.001, MAILING_LIST_MULTI=-1, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, URIBL_BLOCKED=0.001] autolearn=no autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (1024-bit key) header.d=github.com
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id GgwXnT4TJsZo for <quic-issues@ietfa.amsl.com>; Tue, 30 Jun 2020 17:07:47 -0700 (PDT)
Received: from out-25.smtp.github.com (out-25.smtp.github.com [192.30.252.208]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id DC1613A07E0 for <quic-issues@ietf.org>; Tue, 30 Jun 2020 17:07:46 -0700 (PDT)
Received: from github-lowworker-d1d6e31.ash1-iad.github.net (github-lowworker-d1d6e31.ash1-iad.github.net [10.56.105.50]) by smtp.github.com (Postfix) with ESMTP id 29C70282864 for <quic-issues@ietf.org>; Tue, 30 Jun 2020 17:07:46 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=github.com; s=pf2014; t=1593562066; bh=UOYs6g3/Kx3HEzHETQ3PNHiZDPALM1evA9ZSwzWk+ls=; h=Date:From:Reply-To:To:Cc:In-Reply-To:References:Subject:List-ID: List-Archive:List-Post:List-Unsubscribe:From; b=PGxv74ceLddImf1EAhOpks/Dn6YCY3lU0l+G/JZfUr06I3StI2rfNQj/JKvy7Pyni N62XvQuKZQb7dNDre85w3Btn0SiOZic8hFZMXCbwPOpsR21zbVtU/VoTg8M/5Rm3Gm 9PH+E00WmkV0r3/z5S22wgdhSx/a04mVMXauEWfQ=
Date: Tue, 30 Jun 2020 17:07:46 -0700
From: Mike Bishop <notifications@github.com>
Reply-To: quicwg/base-drafts <reply+AFTOJK2WIL2LBV6UA4UAMSV5A62NFEVBNHHCNKDJHI@reply.github.com>
To: quicwg/base-drafts <base-drafts@noreply.github.com>
Cc: Subscribed <subscribed@noreply.github.com>
Message-ID: <quicwg/base-drafts/pull/3805/review/440460046@github.com>
In-Reply-To: <quicwg/base-drafts/pull/3805@github.com>
References: <quicwg/base-drafts/pull/3805@github.com>
Subject: Re: [quicwg/base-drafts] Massive batch of probably-editorial nits (#3805)
Mime-Version: 1.0
Content-Type: multipart/alternative; boundary="--==_mimepart_5efbd3d219cf4_28463fc524ccd96c1029b5"; charset="UTF-8"
Content-Transfer-Encoding: 7bit
Precedence: list
X-GitHub-Sender: MikeBishop
X-GitHub-Recipient: quic-issues
X-GitHub-Reason: subscribed
X-Auto-Response-Suppress: All
X-GitHub-Recipient-Address: quic-issues@ietf.org
Archived-At: <https://mailarchive.ietf.org/arch/msg/quic-issues/5UCf8Hf6X5-muqDnnsnczZU3cFc>
X-BeenThere: quic-issues@ietf.org
X-Mailman-Version: 2.1.29
List-Id: Notification list for GitHub issues related to the QUIC WG <quic-issues.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/quic-issues>, <mailto:quic-issues-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/quic-issues/>
List-Post: <mailto:quic-issues@ietf.org>
List-Help: <mailto:quic-issues-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/quic-issues>, <mailto:quic-issues-request@ietf.org?subject=subscribe>
X-List-Received-Date: Wed, 01 Jul 2020 00:07:50 -0000

@MikeBishop commented on this pull request.



>  
 An endpoint SHOULD copy the error code from the STOP_SENDING frame to the
 RESET_STREAM frame it sends, but MAY use any application error code.  The
 endpoint that sends a STOP_SENDING frame MAY ignore the error code carried in
 any RESET_STREAM frame it receives.
 
-If the STOP_SENDING frame is received on a stream that is already in the
-"Data Sent" state, an endpoint that wishes to cease retransmission of
-previously-sent STREAM frames on that stream MUST first send a RESET_STREAM
-frame.

This is duplicative of two paragraphs earlier, so I merged them.

> @@ -883,16 +878,7 @@ Endpoints need to eventually agree on the amount of flow control credit that has
 been consumed, to avoid either exceeding flow control limits or deadlocking.
 
 On receipt of a RESET_STREAM frame, an endpoint will tear down state for the
-matching stream and ignore further data arriving on that stream.  Without the
-offset included in RESET_STREAM, the two endpoints could disagree on
-the number of bytes that count towards connection flow control.
-
-To remedy this issue, a RESET_STREAM frame ({{frame-reset-stream}}) includes the
-final size of data sent on the stream.  On receiving a RESET_STREAM frame, a
-receiver definitively knows how many bytes were sent on that stream before the
-RESET_STREAM frame, and the receiver MUST use the final size of the stream to
-account for all bytes sent on the stream in its connection level flow
-controller.

This is duplicative of the next section, so I merged it down there.

>  
 
 # Connections {#connections}
 
 QUIC's connection establishment combines 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 as
-described in {{migration}}.  Finally, a connection may be terminated by either
-endpoint, as described in {{termination}}.
+latency, as described in {{handshake}}. During connection establishment, each
+side validates the peer's address, as described in {{address-validation}}.  Once

The old version gave you a forward-looking roadmap to Sections 7, 9, and 10; but not 8.  This adds the missing section.

> @@ -1014,8 +1003,7 @@ connection ID to vary in length and still be used by the load balancer.
 
 A Version Negotiation ({{packet-version}}) packet echoes the connection IDs
 selected by the client, both to ensure correct routing toward the client and to
-allow the client to validate that the packet is in response to an Initial
-packet.
+demonstrate that the packet is in response to a packet sent by the client.

We don't know it's an Initial packet, because we don't speak the version we received to know whether it has such things.

> @@ -1084,8 +1072,8 @@ IDs.
 
 An endpoint that initiates migration and requires non-zero-length connection IDs
 SHOULD ensure that the pool of connection IDs available to its peer allows the
-peer to use a new connection ID on migration, as the peer will close the
-connection if the pool is exhausted.
+peer to use a new connection ID on migration, as the peer will be unable to
+respond if the pool is exhausted.

The requirement to close the connection if the pool is exhausted doesn't seem to exist in the document, only the requirement that you not send.

> @@ -2018,13 +2004,6 @@ consider other properties of the connection that is being attempted, including
 the choice of possible application protocols, session tickets, or other
 connection properties.
 
-Attackers could replay tokens to use servers as amplifiers in DDoS attacks. To
-protect against such attacks, servers SHOULD ensure that tokens sent in Retry
-packets are only accepted for a short time. Tokens that are provided in
-NEW_TOKEN frames ({{frame-new-token}}) need to be valid for longer, but
-SHOULD NOT be accepted multiple times in a short period. Servers are encouraged
-to allow tokens to be used only once, if possible.

Duplicates paragraph in next section, so merged.

>  
 
 ## Path Validation {#migrate-validate}
 
-Path validation is used during connection migration (see {{migration}} and
-{{preferred-address}}) by the migrating endpoint to verify reachability of a

{{preferred-address}} is part of {{migration}}, so this reads oddly in the output.  There's no reason to call out that particular subsection as "especially," so I removed the sub-section reference.

> @@ -2304,8 +2286,8 @@ 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}}.
 
-If an endpoint skips validation of a peer address as described in
-{{migration-response}}, it does not need to limit its sending rate.

This is a subsection of the referenced section; it reads oddly to self-reference.

> @@ -2496,9 +2478,8 @@ ensure connection stability. This section describes the protocol for migrating a
 connection to a preferred server address.
 
 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.
+work. If a client receives packets from a new server address when the client has
+not initiated a migration, the client SHOULD discard these packets.

We don't allow spontaneous server migration even after sending SPA.

> @@ -2749,11 +2730,11 @@ endpoints MAY send the exact same packet.
 
 Note:
 
-: Allowing retransmission of a closing packet contradicts other advice in this
-  document that recommends the creation of new packet numbers for every packet.
-  Sending new packet numbers is primarily of advantage to loss recovery and
-  congestion control, which are not expected to be relevant for a closed
-  connection.  Retransmitting the final packet requires less state.
+: Allowing retransmission of a closing packet is an exception to the requirement
+  for new packet numbers for every packet in {{packet-numbers}}. Sending new

That "other advice" is a MUST.

> @@ -3227,9 +3208,9 @@ Packet numbers are divided into 3 spaces in QUIC:
 
 - Initial space: All Initial packets ({{packet-initial}}) are in this space.
 - Handshake space: All Handshake packets ({{packet-handshake}}) are in this
-space.
-- Application data space: All 0-RTT and 1-RTT encrypted packets
-  ({{packet-protected}}) are in this space.

packet-protected is in a different section from packet-initial and packet-handshake, so that reference looks odd in the output.

>  such as once per round trip, to elicit an ACK from the peer.
 
-A receiver MUST NOT bundle an ack-eliciting frame with packets that would
+A receiver MUST NOT send an ack-eliciting frame in all packets that would

The "all" was removed in a recent change (I didn't look to see which), but changes the meaning.  I think it's needed.

> @@ -3906,7 +3887,7 @@ The QUIC packet size includes the QUIC header and protected payload, but not the
 UDP or IP headers.
 
 QUIC depends upon a minimum IP packet size of at least 1280 bytes.  This is the
-IPv6 minimum size {{?RFC8200}} and is also supported by most modern IPv4
+IPv6 minimum size ({{?IPv6=RFC8200}}) and is also supported by most modern IPv4

For consistency with [IPv4] below.

> @@ -4169,7 +4151,7 @@ decodes to the decimal value 151288809941952652; the four byte sequence 9d 7f 3e
 7d decodes to 494878333; the two byte sequence 7b bd decodes to 15293; and the
 single byte 25 decodes to 37 (as does the two byte sequence 40 25).
 
-Error codes ({{error-codes}}) and versions ({{versions}}) are described using
+Versions ({{versions}}) and error codes ({{error-codes}}) are described using

This puts the references in the order in which the referenced sections appear.

> @@ -4285,7 +4267,7 @@ Destination Connection ID Length:
 Destination Connection ID:
 
 : The Destination Connection ID field follows the Destination Connection ID
-  Length field and is between 0 and 20 bytes in length.
+  Length field, which indicates the length of this field.

By not explicitly calling out the length, we don't have to repeat the discussion about v1 vs. invariant.

> @@ -5618,8 +5599,8 @@ All data sent in STREAM frames counts toward this limit.  The sum of the largest
 received offsets on all streams - including streams in terminal states - MUST
 NOT exceed the value advertised by a receiver.  An endpoint MUST terminate a
 connection with a FLOW_CONTROL_ERROR error if it receives more data than the
-maximum data value that it has sent, unless this is a result of a change in
-the initial limits; see {{zerortt-parameters}}.
+maximum data value that it has sent.  This includes violations of remembered
+limits in Early Data; see {{zerortt-parameters}}.

Initial limits can't be changed in a way that would produce a flow control violation.

>  
 All registrations made by Standards Track publications MUST be permanent.
 
 All registrations in this document are assigned a permanent status and list as
 contact both the IESG (ietf@ietf.org) and the QUIC working group
-([quic@ietf.org](mailto:quic@ietf.org)).

Hyperlinking results in the e-mail address being included twice in the text output.

-- 
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/3805#pullrequestreview-440460046