On Fri, Jan 28, 2022 at 3:09 PM Benjamin Kaduk <[email protected]> wrote:
> On Fri, Jan 28, 2022 at 02:55:57PM -0800, David Schinazi wrote: > > Thank you for your review Ben. Responses inline. > > > > David > > > > On Fri, Jan 28, 2022 at 2:23 PM Benjamin Kaduk via Datatracker < > > [email protected]> wrote: > > > > > ---------------------------------------------------------------------- > > > DISCUSS: > > > ---------------------------------------------------------------------- > > > > > > Section 5 refers to a "max_packet_size" transport parameter but I do > not > > > see that parameter defined in the registry or RFC 9000. > > > It seems that a transport parameter of that name was present in earlier > > > versions of draft-ietf-quic-transport, but got renamed to > > > max_udp_payload_size in the -28, so hopefully this is just a trivial > > > rename. > > > > > > > You're absolutely right, we missed the rename and will fix this by > > landing your PR #76. > > Excellent, I'm glad this was the easy option, and as you note in the > follow-up, it's fixed in the editor's copy already, so I'll go clear now. > > > ---------------------------------------------------------------------- > > > COMMENT: > > > ---------------------------------------------------------------------- > > > > > > I put some editorial suggestions (including the presumed resolution of > the > > > DISCUSS) on github at https://github.com/quicwg/datagram/pull/76 . > > > > > > > Thanks, let's discuss those on the PR. > > > > Section 2 > > > > > > * QUIC uses a more nuanced loss recovery mechanism than the DTLS > > > handshake, which has a basic packet loss retransmission timer. > > > > > > This is true of DTLS 1.2 and prior versions, which technically is right > > > now the current version of DTLS. However, it's not quite true of DTLS > > > 1.3, which includes an explicit ACK message to supplement the > > > retransmission timer. DTLS 1.3 stands a pretty decent chance of being > > > published as an RFC prior to this document (per ekr, it should have the > > > last technical changes from the WG finalized this weekend and then go > into > > > the "real" AUTH48 state), so I think we ought to speak to the > mechanisms > > > of DTLS 1.3 here. > > > > > > > How about we just remove "which has a basic packet loss retransmission > > timer"? > > I think it's safe to say that QUIC congestion control is more nuanced > than > > that of > > any version of DTLS, including DTLS 1.3. > > That seems reasonable. I agree that QUIC CC is clearly more nuanced than > DTLS of any version. And the value of going into any level of detail about > what DTLS actually does provide seems pretty minimal. > Thanks. I've now made that change to the editor's copy. David > Section 3 > > > > > > For most uses of DATAGRAM frames, it is RECOMMENDED to send a value > > > of 65535 in the max_datagram_frame_size transport parameter to > > > indicate that this endpoint will accept any DATAGRAM frame that fits > > > inside a QUIC packet. > > > > > > It's interesting to compare this to the RFC 9000 max_udp_payload_size > > > default of 65527, the maximum permitted UDP payload. Indeed, the QUIC > > > 1-RTT packet header does not even contain a length field that would > limit > > > the frame size. So I'm not entirely sure what motivates the 65535 > value > > > specifically. (I do see the subsequent discussion about how there are > > > other factors, including max_packet_size/max_udp_payload_size, that can > > > further limit what is usable.) > > > > > > > We picked that value because it means "don't limit the length" and > doesn't > > involve thinking about header sizes or doing math. > > Okay. I can't fault that, even if it seems that since we're bounded by > max_udp_payload_size we could just as easily reuse the 65527 value. > > Thanks for the quic(k) turnaround! > > -Ben >
