Reviewer: Stewart Bryant Review result: Ready with Issues I am the assigned Gen-ART reviewer for this draft. The General Area Review Team (Gen-ART) reviews all IETF documents being processed by the IESG for the IETF Chair. Please treat these comments just like any other last call comments. For more information, please see the FAQ at <https://trac.ietf.org/trac/gen/wiki/GenArtfaq>. Document: draft-ietf-quic-transport-32 Reviewer: Stewart Bryant Review Date: 2020-10-28 IETF LC End Date: 2020-11-16 IESG Telechat date: Not scheduled for a telechat Summary: I would like to remind the reader that the Genart reviewer is someone was not involved with the design and is ideally reading the text for the first time rather than an expert. In this regard I hope that I am accuracy modelling the reaction of such a reader in the future. This is a very clever, well written and innovative body of work, and could be published as is. However, I am not sure it services the needs of all of the communities that will need to read the document and as such ought to have additional text, or point to another supplementary document. Specifically as the base document for the protocol it is the one that people will naturally start with, and I found that it was not until I got to past page 100 that I started to appreciate how it worked. Not everyone needs to know the whole protocol in detail, and some will just need to dip into important sections. A tutorial/introduction explaining the basics of operation to provide a framework for the rest of the document would help a lot of readers. The text uses a new notation which is clever and succinct for representing the detail of the transmitted protocol, but I suspect that the use of conventional diagrams or at least a tutorial appendix that includes them would be appreciated by other new readers looking at this for the first time. I amplify this in the comments below. Major issues: Section 1 provides an ultrafast introduction to the problem, and very little on the fundamentals of how the protocol works. It would be useful if there a process by which the reader is gradually immersed in the detail. ======== Section 1 is the first place that you introduce the term "Frame". I got very confused about frames and packets, and I thing the taxonomy needs to be clearly explained. You have UDP/IP packets that carry one or more QUIC PDUs, but the frame/packet notation made it unclear what was being discussed and whether the terms were being used consistently. Some clarification would be useful, or if it is in the text then making it more obvious would help. ======= Packet and frame diagrams in this document use a custom format. The purpose of this format is to summarize, not define, protocol elements. SB> I found this confusing, because in a ST protocol specification I was looking for a protocol definition ======= Streams are identified within a connection by a numeric value, referred to as the stream ID. A stream ID is a 62-bit integer (0 to 2^62-1) that is unique for all streams on a connection. SB> I found the 62 bits pulled out of a hat very confusing. Eventually the reader realises that this is 64 bits with the two lowest bits signifying the parameters below. Some explanatory text would help. ====== Stream IDs are encoded as variable-length integers; see Section 16. A QUIC endpoint MUST NOT reuse a stream ID within a connection. The least significant bit (0x1) of the stream ID identifies the SB> This was a common problem throughout the text. I found the use of (0x1) and similar confusing in the text. Where you use this notation you mean the bit manipulated by the mask 0x1 (and similar). It would be helpful to the reader to be a little more pedantic in the use of the term in each case. We will get to it later, in the comments but a lot of readers would be helped by the use of the classic ASCII art style representation of bit fields. ========== =========== SB> When I read the text around table 1, I found this to be confusing and was not clear whether the object is a 64 bit object with a 62 bit instance ID or a 62 bit object with a 60 bit instance ID. If the former I am not sure what the name of the whole is vs the name of the 62 bit instance ID is. This could usefully be clarified. =========== QUIC depends upon a minimum IP packet size of at least 1280 bytes. This is the IPv6 minimum size ([IPv6]) and is also supported by most modern IPv4 networks. Assuming the minimum IP header size of 40 bytes for IPv6 and 20 bytes for IPv4 and a UDP header size of 8 bytes, this results in a maximum datagram size of 1232 bytes for IPv6 and 1252 bytes for IPv4. The maximum datagram size MUST be at least 1200 bytes. SB> The previous two paras are quite confusing. I think the first para refers to the max packet size in the network layer, but the second para is not clear, nor is the next para. Any maximum datagram size larger than 1200 bytes can be discovered using Path Maximum Transmission Unit Discovery (PMTUD; see Section 14.2.1) or Datagram Packetization Layer PMTU Discovery (DPLPMTUD; see Section 14.3). Enforcement of the max_udp_payload_size transport parameter (Section 18.2) might act as an additional limit on the maximum datagram size. A sender can avoid exceeding this limit, once the value is known. However, prior to learning the value of the transport parameter, endpoints risk datagrams being lost if they send datagrams larger than the smallest allowed maximum datagram size of 1200 bytes. SB> Again unclear. Are you saying that the network must support a network layer packet of at least 1200B which is used to carry Ip + UDP + UDP payload? UDP datagrams MUST NOT be fragmented at the IP layer. In IPv4 ([IPv4]), the DF bit MUST be set if possible, to prevent fragmentation on the path. Datagrams are required to be of a minimum size under some conditions. However, the size of the datagram is not authenticated. Therefore, an endpoint MUST NOT close a connection when it receives a datagram that does not meet size constraints, though the endpoint MAY discard such datagrams. SB> I really think this section needs re-writing for clarity. ============ 14.2.1. Handling of ICMP Messages by PMTUD SB> I am worried about the text in this section. ICMP and UDP paths may not be congruent as a result of different RTG ECMP treatment. The only packet you can trust to explore the path is a QUIC packet. ============ 22.1.1. Provisional Registrations Provisional registration of codepoints are intended to allow for private use and experimentation with extensions to QUIC. Provisional registrations only require the inclusion of the codepoint value and contact information. However, provisional registrations could be reclaimed and reassigned for another purpose. SB> Experience in other WGs suggests that reclaiming is very hard. We really needed to do this in MPLS where we had a 16 value field and found it impossible. ============ Additionally, each value of the format "31 * N + 27" for integer values of N (that is, 27, 58, 89, ...) are reserved and MUST NOT be assigned by IANA. SB> We do not normally ask IANA to run an algorithm to check a value. Are we sure that this will work? ============= Minor issues: SB> In looking at "Figure 2: States for Sending Parts of Streams" it occurred to me that this was the major states but there were possibly a number of error states missing. It would be useful to clarify. ========= It is necessary to limit the amount of data that a receiver could buffer, to prevent a fast sender from overwhelming a slow receiver, or to prevent a malicious sender from consuming a large amount of memory at a receiver. SB> Surely it is also necessary to do this to prevent the network from being overwhelmed? ======= If a max_streams transport parameter or a MAX_STREAMS frame is received with a value greater than 2^60, SB> This pulling of 2^60 out of a hat is too cryptic. As I understand it this is because you need 2 bits to express the size of the object that holds to identifier and two bits to hold the control parameters that go in the LSBs. It would be helpful to the new reader to call all this out earlier rather than making them figure it all out. ======= 6. Version Negotiation SB> I am surprised that there is no IANA version registry ======== For a server to use a new version in the future, clients need to correctly handle unsupported versions. Some version numbers (0x?a?a?a?a as defined in Section 15) are reserved for inclusion in fields that contain version numbers. SB> The value 0x?a?a?a?a is somewhat pulled out of a hat, it would be useful to explain the rational. SB> and then in Section 15 I wondered why this pattern which leaves all sorts of holes in the version space? ======= Implementations need to maintain a buffer of CRYPTO data received out of order. Because there is no flow control of CRYPTO frames, an endpoint could potentially force its peer to buffer an unbounded amount of data. SB> What are the congestion consequences of this? ========== validation_timeout = max(3*PTO, 6*kInitialRtt) SB>A quick look in the reference did not show what units of time you are using in the protocol - maybe I missed it? ======= Stateless Reset { Fixed Bits (2) = 1, Unpredictable Bits (38..), Stateless Reset Token (128), } Figure 10: Stateless Reset Packet SB> When I first met this my reaction was that I was sure it sends a lot more bits on the wire than this, but so far I have not see a packet definition nor a pointer to a packet definition. Please could you clarify what is happening? ===== 10.3.1. Detecting a Stateless Reset An endpoint detects a potential stateless reset using the trailing 16 bytes of the UDP datagram. SB> I find this rather opaque given that so far we do not know what a packet looks like, and I cannot see what the encoding of this is. SB> To add to the confusion over packets and frames we now have datagrams. ======= SB> Having read to the end of section 10 I made a note that I was still trying to figure out what a reset token actually looked like. ===== 12.3. Packet Numbers The packet number is an integer in the range 0 to 2^62-1. SB> It would save a bit of wondering until later if this was described as a non-reusable number space ======= A QUIC endpoint MUST NOT reuse a packet number within the same packet number space in one connection. If the packet number for sending reaches 2^62 - 1, the sender MUST close the connection without sending a CONNECTION_CLOSE frame or any further packets; an endpoint MAY send a Stateless Reset (Section 10.3) in response to further packets that it receives. SB> I know that this is a lot of space, although you subdivide it so its smaller, but I would be useful is the fact that there was a maximum data size was stated up from and it was specified. There are some applications that expect to send continuously without disruption and would need to take action prior to termination of the transfer. ===== 12.4. Frames and Frame Types The payload of QUIC packets, after removing packet protection, Packet Payload { Frame (8..) ..., } consists of a sequence of complete frames, as shown in Figure 11. Version Negotiation, Stateless Reset, and Retry packets do not contain frames. SB> At this point (page 82) I am still not sure what a packet or a frame is and have no idea what is going on on the wire. ======= 14. Datagram Size A UDP datagram can include one or more QUIC packets. SB> There is some confusion in the text between packets, frames and datagrams. That could be helped by earlier clarification. ====== 17.2. Long Header Packets SB> How is the following encoded? It is not clear how long the fields are. SB> If the notation is that (x) means x bits it would be good to remind the reader. It may have been called up earlier but that was 100 pages ago with no back pointer. This would be so much clearer to most readers with a conventional packet diagram Long Header Packet { Header Form (1) = 1, Fixed Bit (1) = 1, Long Packet Type (2), Type-Specific Bits (4), Version (32), Destination Connection ID Length (8), Destination Connection ID (0..160), Source Connection ID Length (8), Source Connection ID (0..160), } ======= The layout of a Version Negotiation packet is: Version Negotiation Packet { Header Form (1) = 1, Unused (7), Version (32) = 0, Destination Connection ID Length (8), Destination Connection ID (0..2040), Source Connection ID Length (8), Source Connection ID (0..2040), Supported Version (32) ..., } Figure 14: Version Negotiation Packet The value in the Unused field is selected randomly by the server. SB> When I first met this text the following questions arose. SB> What does the (7) in unused mean? SB> What length is Destination/Source Connection ID/What does (0..2040) mean? SB> Now I have figured this out, but I really worry about how difficult this is for the new/casual reader of the text. ======== The server includes a connection ID of its choice in the Source Connection ID field. This value MUST NOT be equal to the Destination Connection ID field of the packet sent by the client. SB> It would be helpful to explain why ======== 18.1. Reserved Transport Parameters Transport parameters with an identifier of the form "31 * N + 27" for integer values of N are reserved to exercise the requirement that unknown transport parameters be ignored. These transport parameters have no semantics, and may carry arbitrary values. SB> Which hat did that come out of? It would be helpful to the reader to explain the rational. =========== 19.1. PADDING Frames SB> The choice of padding values is itself an interesting problem, but as far as I can see the text is silent on the subject. ========== 22.1.2. Selecting Codepoints New uses of codepoints from QUIC registries SHOULD use a randomly selected codepoint that excludes both existing allocations and the first unallocated codepoint in the selected space. Requests for multiple codepoints MAY use a contiguous range. SB> Doesn’t this lead to fragment code-spaces where it becomes progressively harder to get blocks. Why this policy? SB> Related to which I am not sure the process in para 2 of 22.1.3 is realistic and wonder of it is really needed. ========= Appendix A. Sample Packet Number Decoding Algorithm The pseudo-code in Figure 45 shows how an implementation can decode packet numbers after header protection has been removed. SB> The variable in the code need to be defined. SB> There should also be some commentary to make it understandable to the new reader. SB> It would be useful for this to have a back reference to the body of the document. ========== Nits/editorial comments: SB> "Example Structure" SB> The use of white space coupled name components was strange to me and it too a while and a look late in the text to realise that Structure was part the name and not part of the syntax (i.e. that a structure was not Structure {}) ======= Table 3: Frame Types SB> The notation used in the table took a while to figure out. A little more explanation, and possibly before the table itself might make it clearer. ========= Table 4: Summary of Integer Encodings For example, the eight byte sequence c2 19 7c 5e ff 14 e8 8c (in hexadecimal) 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). SB> The SB> Some hex & decimal indication would help. SB> A picture is worth a thousand words and save the reader drawing this out themselves to understand it and getting confused. SB> The conversion pseudo code would be helpful, possibly in an appendix. ============== As a result, the size of the packet number encoding is at least one bit more than the base-2 logarithm of the number of contiguous unacknowledged packet numbers, including the new packet. SB> That is really hard to parse. For example, if an endpoint has received an acknowledgment for packet 0xabe8bc, sending a packet with a number of 0xac5c02 requires a packet number encoding with 16 bits or more; whereas the 24-bit packet number encoding is needed to send a packet with a number of 0xace8fe. SB> and so is that. SB> Indeed I found the remaining paras of the section difficult to follow. ========= With this mechanism, the server reflects the spin value received, while the client 'spins' it after one RTT. On-path observers can measure the time between two spin bit toggle events to estimate the end-to-end RTT of a connection. SB> It would be clearer to the reader if the explanation of operation proceeded the detail. ========= The Transport Parameter Length field contains the length of the Transport Parameter Value field. SB> Presumably the length in bytes. ======== 21. Security Considerations SB> An introduction to this long section would be useful ========= 21.13. Overview of Security Properties SB> I find it strange that having read 12 previous security analysis subsections I arrive at this section. I am surprised that the overview is not section 21.1 ============ -- last-call mailing list last-call@xxxxxxxx https://www.ietf.org/mailman/listinfo/last-call
