[quicwg/base-drafts] QUIC transport Reading NiTs part 2 of 2 (#3735)

[Gorry Fairhurst <[e-mail]>]

I think there are NiTs in the second half the ID also, and while I suspectthese don't change the intended meaning, they nevertheless should be examined by the editors:

(1) Concerning ACKS:  I thought the following text was heading in the correct direction, but I remain concerned that this could be read to suggest that performance of the flow should drive the changes. 
/However, an implementer should only deviate from these requirements after careful consideration of the performance implications of doing so./
- I’d love this to actually say something about congestion. Decisions made in the generation of ACKs can impact the congestion to the network and therefore also the performance of other flows that share a path. I really think we should say this.

(2) And collateral impact on other traffic:
/It can also improve connection throughput on severely asymmetric links; see Section 3 of {{?RFC3449}}./
- It also can have important congestion impacts on other traffic that shares an asymmetric bottleneck, besides the self-harm of reducing the application’s own throughput.

(3) Justify? This text seems a good recommendation, but there isn’t a hint at why this is a strong requirement, could the editors add one line of text explaining?
/Senders MUST NOT coalesce QUIC packets for different connections into a single UDP datagram./

(4) The size and frequency of ACKs can impact the performance of QUIC, so there are also congestion control reasons to not overly report ACK ranges. 
/This is necessary if an ACK frame would be too large to fit in a packet, however receivers MAY also limit ACK frame size further to preserve space for other frames./

(5) This seems an awkward sentence and I could not understand what was intended:
/A receiver MUST NOT bundle an ack-eliciting frame with all packets that would otherwise be non-ack-eliciting, to avoid an infinite feedback loop of acknowledgements./
- Perhaps /all/ in this is part of my confusion?

(6) In Section: Versions (missing words)
/Implementors are encouraged to register version numbers of QUIC that they are using for private experimentation on the GitHub wiki at ./
- either way, it may be better to complete the sentence?

(7) In Section:  PING Frame: (NiT)
- This seems good advice, but I’d love the sentence to include “to keep a connection alive”, since at the moment reading this alone, could be seen as rather general guidance. Whereas there are other uses of PINGs, such as PLPMTUD, and for this use can generate pings by clients and servers:
/An application protocol SHOULD provide guidance about the conditions under which generating a PING is recommended. / e.g. /An application protocol SHOULD provide guidance about the conditions under which it recommends generating a PING to keep a connection alive./

(8) In ACKs: (NiT)
/This is unlike TCP SACKs ({{?RFC2018}})./This is unlike reneging for TCP SACKs ({{?RFC2018}})./
- can we be slightly more explicit about the important difference by adding a couple of words?

(9) In Section: STREAM Frames (line break:)
/Offset:/
- The first sentence in /Offset/ appears to have an odd line-break mid sentence.

(10) In Section: Extension Frames
- /which/that/?
/other extensions which modify or replace the same functionality unless the behavior of the combination is explicitly defined./

(11) In Section:  Transport Error Codes (line break:)
INVALID_TOKEN (0xB): :/
- The first sentence needs a line-break mid sentence.

(12) In Section: Version Negotiation 
Is this only a recommendation?
“Clients that support multiple QUIC versions SHOULD pad the first packet they send to the largest of the minimum packet sizes across all versions they support.”
Whereas in Section: Packet Size
/A client MUST expand the payload of all UDP datagrams carrying Initial packets to at least 1200 bytes, by adding PADDING frames to the Initial packet or by coalescing the Initial packet; see {{packet-coalesce}}./
- I think the latter is true, and suspect that version negotiation section shouldn’t include this text, but instead refer to the text norm initial packet size.

(13) In Section: Cryptographic and Transport Handshake
- Is this intended as optional, I think it is required when ECN is to be used?
“An endpoint can verify support for Explicit Congestion Notification (ECN) in the first packets it sends, as described in {{ecn-validation}}.”
- seems to suggest this is optional, whereas, I think actually this is describing a needed mechanism, so is it better to say “An endpoint verifies” as is done in the section: Initiating Connection Migration. which says: “The new path might not have the same ECN capability. Therefore, the endpoint verifies ECN capability as described in {{ecn}}.”  


-- 
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/issues/3735