Hi Rob, Thanks for the review. I've captured your comments as issues on the QUIC WG GItHub repository. Links to each are provided as in-line responses.
On Wed, Jan 6, 2021 at 5:43 PM Robert Wilton via Datatracker < [email protected]> wrote: > Robert Wilton has entered the following ballot position for > draft-ietf-quic-transport-33: Discuss > > When responding, please keep the subject line intact and reply to all > email addresses included in the To and CC lines. (Feel free to cut this > introductory paragraph, however.) > > > Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html > for more information about IESG DISCUSS and COMMENT positions. > > > The document, along with other ballot positions, can be found here: > https://datatracker.ietf.org/doc/draft-ietf-quic-transport/ > > > > ---------------------------------------------------------------------- > DISCUSS: > ---------------------------------------------------------------------- > > With so many "Yes" votes from other ADs, I feel like I'm swimming against > the > flow by raising a discuss ... > > Firstly, I would like the thank the authors and WG on such a well written > document. I am supportive of this protocol and hope that it will be good > for > the Internet. > > However, I do have some discuss questions relating to the Spin Bit and the > ability to manage and monitor networks. I appreciate that there has > already > been a lot of (presumably heated) discussion on the spin bit, which I've > not > read or participated in, but I am concerned about the operational > manageability > aspect of QUIC. > > Firstly, I have two comments on clarifying the spin bit > behaviour/specification: > > 1) It would be helpful to clarify what the expected behaviour is for an > implementation that chooses not to support the spin-bit. Does it just > leave > the bit set as 0, or is it meant to follow the same behaviour as if > spin-bit is > supported but disabled? > https://github.com/quicwg/base-drafts/issues/4668 > > 2) This may not be discuss worthy, but some of the spin bit behaviour is > inconsistently defined between the quic transport and quic manageability > drafts. Specifically: > - The transport draft states that at least 1 in 16 connections "MUST" > disable > spinning, whereas the manageability draft states this as "recommended". > - In > the case that the spin bit is disabled, the transport draft uses > "RECOMMENDED" to use a random value for each packet, or chosen > independently > for each connection. Whereas the manageability draft uses "can" and > lists > the two options in the opposite order. > > For this review, since it is in IESG review, I've presumed that the > transport > draft has the definitive definitions and the manageability draft is > lagging. > https://github.com/quicwg/base-drafts/issues/4669 > But my two main discuss questions/comments relate to whether the spin-bit, > as > specified in quic transport, achieves its goal. I appreciate that there > are > individuals who don't think that it is required at all, conversely some > network > operators believe that they will lose vital information needed to help > manage > their networks, and presumably we are trying to find a pragmatic compromise > between these two positions. > > 1) I find it hard to understand why a server is allowed to independently > decide > whether or not to support the spin bit on a connection? Shouldn't the > client > (or administrator of the client system) that opened the connection be able > to > choose whether they want the RTT to be monitorable via the spin bit? What > is > the reasoning for allowing the server (or server administrator) to be able > to > independently be able to decide what is best for the client? > https://github.com/quicwg/base-drafts/issues/4666 > 2) In the case that the spin-bit is disabled, I don't understand the > benefit of > injecting a random spin bit value in each packet rather than always > setting it > to a per connection random value. It seems that whether or not the > randomness > is injected, it is expected to be feasible to extract the RTT for those > connections that are genuinely spinning the bit (or otherwise the spin bit > is > entirely pointless), but it just seems to make it computationally harder to > extract the signal from the noise. Perhaps the goal here is reduce the > ability > for pervasive monitoring to occur, but that feels a bit like security > through > obscurity. > https://github.com/quicwg/base-drafts/issues/4667 > Some enlightenment for these questions would be appreciated. > > Regards, > Rob > > > ---------------------------------------------------------------------- > COMMENT: > ---------------------------------------------------------------------- > > Comments: > > 1.2. Terms and Definitions > > Frame: A unit of structured protocol information. There are multiple > frame > types, each of which carries different information. Frames are > contained in > QUIC packets. > > Perhaps indicate that a quic packet can contain multiple frames? The > terminology states that a datagram contains multiple quick packets, but it > wasn't obvious to me that a quick packet can contain multple frames. > https://github.com/quicwg/base-drafts/issues/4670 > This document uses the terms "QUIC packets", "UDP datagrams", and "IP > packets" to refer to the units of the respective protocols. That is, > one or > more QUIC packets can be encapsulated in a UDP datagram, which is in > turn > encapsulated in an IP packet. > > Frame is a widely used term in L2. Hence, it might be helpful if this > description also covered "Frames". Perhaps this would also be a helpful > place > (or an alternative place) to mention that a QUIC packet contains one or > more > QUIC frames. > https://github.com/quicwg/base-drafts/issues/4671 > 3. Stream States > > Bidirectional streams use both state machines > > Maybe clarify this to indicate that it means that both state machines are > used > at each endpoint. Otherwise, even unidirection steams use both state > machines, > one at each endpoint. > https://github.com/quicwg/base-drafts/issues/4672 > 3.2. Receiving Stream States > > Figure 3: States for Receiving Parts of Streams > > Perhaps expand "App Read RST" to "App Read RESET", since there doesn't > seem a > great reason to abbreviate it. > https://github.com/quicwg/base-drafts/issues/4673 > 3.2. Receiving Stream States > > The receiving part of a stream enters the "Recv" state when the > sending part of a bidirectional stream initiated by the endpoint > (type 0 for a client, type 1 for a server) enters the "Ready" state. > > This might be slightly clearer if the conditional predicate was moved to > the > beginning of the sentence. E.g., For bidirectional streams, ... > > https://github.com/quicwg/base-drafts/issues/4674 > 4.1. Data Flow Control > > * Stream flow control, which prevents a single stream from consuming > the > entire receive buffer for a connection by limiting the amount of data > that > can be sent on any stream > > Perhaps "on a stream" would be better than "on any stream". > https://github.com/quicwg/base-drafts/issues/4675 > 4.6. Controlling Concurrency > > An endpoint that is unable to open a new stream due to the peer's > limits > SHOULD send a STREAMS_BLOCKED frame (Section 19.14). This signal is > considered useful for debugging. An endpoint MUST NOT wait to receive > this > signal before advertising additional credit, since Iyengar & Thomson > Expires 16 June 2021 [Page 29] Internet-Draft QUIC Transport Protocol > December 2020 doing so will mean that the peer will be blocked for at > least > an entire round trip, and potentially indefinitely if the peer chooses > not > to send STREAMS_BLOCKED frames. > > By additional credit, does it sending MAX_STREAMS? If so, it might be > helpful > to clarify that. > https://github.com/quicwg/base-drafts/issues/4676 > > 5.1. Connection ID > > 5.1.1. Issuing Connection IDs > > Additional connection IDs are communicated to the peer using > NEW_CONNECTION_ID frames (Section 19.15). The sequence number on > each newly issued connection ID MUST increase by 1. The connection > ID randomly selected by the client in the Initial packet and any > connection ID provided by a Retry packet are not assigned sequence > numbers unless a server opts to retain them as its initial > connection > ID. > > I was slightly confused by the "The connection ID randomly selected by the > client". Elsewhere in section 5.1 it says that connection id allocation > are > implementation specific. Are there any constraints on how connection ids > can > be allocated (other than not repeating as already stated). E.g., could an > implementation just allocate them as integers starting at 1? Or can all > connection ids (including NEW_CONNECTION_IDs) be randomly allocated? > https://github.com/quicwg/base-drafts/issues/4677 > > 16. Variable-Length Integer Encoding > > No requirement to send integers in smallest encoding possible? E.g. > okay > to send the value 3 in an 8 byte field? > > This section doesn't indicate whether a sender is allowed to send integers > not > using the smallest possible encoding, and doesn't indicate whether a > receive is > expected to process non optimal encodings. It might be helpful to be > explicit. > https://github.com/quicwg/base-drafts/issues/4678 > 17. Packet Formats > > Version: The QUIC Version is a 32-bit field that follows the first > byte. > This field indicates the version of QUIC that is in use and determines > how > the rest of the protocol fields are interpreted. > > By "rest of the protocol fields", I had incorrectly interpreted this as > meaning > all subsequent fields described after the version field are determined > Perhaps worth clarifying - although I recall that it does become clear > elsewhere. > https://github.com/quicwg/base-drafts/issues/4679 > 17. Packet Formats > > Type-Specific Bits: The lower four bits (those with a mask of 0x0f) of > byte > 0 are type-specific > > Perhaps "packet type specific" rather than just "type-specific"? > https://github.com/quicwg/base-drafts/issues/4680 Cheers, Lucas On behalf of QUIC WG Chairs
