Hi Stewart, thank you for the review. I've opened a number of GitHub issues for the points you raise below, please feel free to track their resolution there. There is also a tracking milestone at https://github.com/quicwg/base-drafts/milestone/4. Thanks, Lars On 2020-10-28, at 14:44, Stewart Bryant via Datatracker <noreply@xxxxxxxx> wrote: > 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. https://github.com/quicwg/base-drafts/issues/4265 > 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. https://github.com/quicwg/base-drafts/issues/4266 > 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 https://github.com/quicwg/base-drafts/issues/4267 > 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. https://github.com/quicwg/base-drafts/issues/4268 > 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. https://github.com/quicwg/base-drafts/issues/4269 > 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. https://github.com/quicwg/base-drafts/issues/4270 > 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. https://github.com/quicwg/base-drafts/issues/4271 > 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? https://github.com/quicwg/base-drafts/issues/4272 > 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. https://github.com/quicwg/base-drafts/issues/4273 > 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. https://github.com/quicwg/base-drafts/issues/4274 > 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. https://github.com/quicwg/base-drafts/issues/4275 > 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? https://github.com/quicwg/base-drafts/issues/4276 > 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. https://github.com/quicwg/base-drafts/issues/4277 > 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? https://github.com/quicwg/base-drafts/issues/4278 > 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. https://github.com/quicwg/base-drafts/issues/4279 > 6. Version Negotiation > SB> I am surprised that there is no IANA version registry https://github.com/quicwg/base-drafts/issues/4280 > 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? https://github.com/quicwg/base-drafts/issues/4281 > 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? https://github.com/quicwg/base-drafts/issues/4282 > 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? https://github.com/quicwg/base-drafts/issues/4283 > 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? https://github.com/quicwg/base-drafts/issues/4284 > 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. https://github.com/quicwg/base-drafts/issues/4285 > 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. https://github.com/quicwg/base-drafts/issues/4286 > 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 https://github.com/quicwg/base-drafts/issues/4287 > 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. https://github.com/quicwg/base-drafts/issues/4288 > 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. https://github.com/quicwg/base-drafts/issues/4289 > 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. https://github.com/quicwg/base-drafts/issues/4290 > 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), > } https://github.com/quicwg/base-drafts/issues/4291 > 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. https://github.com/quicwg/base-drafts/issues/4292 > 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 https://github.com/quicwg/base-drafts/issues/4293 > 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. https://github.com/quicwg/base-drafts/issues/4294 > 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. https://github.com/quicwg/base-drafts/issues/4295 > 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. https://github.com/quicwg/base-drafts/issues/4296 > 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. https://github.com/quicwg/base-drafts/issues/4297 > 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 {}) https://github.com/quicwg/base-drafts/issues/4298 > 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. https://github.com/quicwg/base-drafts/issues/4299 > 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. https://github.com/quicwg/base-drafts/issues/4300 > 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. https://github.com/quicwg/base-drafts/issues/4301 > 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. https://github.com/quicwg/base-drafts/issues/4302 > The Transport Parameter Length field contains the length of the Transport > Parameter Value field. > > SB> Presumably the length in bytes. https://github.com/quicwg/base-drafts/issues/4303 > 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 https://github.com/quicwg/base-drafts/issues/4304
Attachment:
signature.asc
Description: Message signed with OpenPGP
-- last-call mailing list last-call@xxxxxxxx https://www.ietf.org/mailman/listinfo/last-call
