QUIC Loss Detection and Congestion Control
draft-ietf-quic-recovery-34

Thanks to Derrell Piper for the SECDIR review.

** Section 3.  Per “The encryption level indicates the packet number space, as described in [QUIC-TRANSPORT ]”, I think that reference here is Section 4.* from [QUIC-TLS] as I can’t find a mapping between PNS and encryption levels as discussed here in [QUIC-TRANSPORT].

Thank you for the work put into this document. Even for a non transport person like myself, I find the text easy to read.

Please find below some non-blocking COMMENT points (but replies would be appreciated), and some nits.

I hope that this helps to improve the document,

Regards,

-éric

== COMMENTS ==

Does the QUIC WG intend to have a BCP-like document on how network devices on the path should handle buffers? E.g., QUIC ressembles a lot to TCP so nodes can apply back pressure with mechanisms similar to RED (but, AFAIK, RED is mainly applied to TCP traffic so not to QUIC/UDP).

-- Abstract --
Should the QUIC version be specified ?

-- Section 1 --
Suggest to specify version 1 for the 2nd sentence.

This (too?) short introduction would benefit by mentioning UDP transport.

== NITS ==

-- Section 7.2 --
  "Endpoints SHOULD use an initial congestion
   window of 10 times the maximum datagram size (max_datagram_size),
   limited to the larger of 14720 bytes or twice the maximum datagram
   size. "

The above text looks ambiguous to me especially with the ',' before 'limited'.

Also, s/8 byte overhead/8-byte overhead/ ?

Thanks for an exceptionally well written and easy-to-read document.

Another well-written QUIC document; thank you!
(The last version of -recovery I read was quite some time ago, perhaps the
-09, and left me scratching my head quite often.  This one is quite clear.)

I put my usual editorial suggestions up at https://github.com/quicwg/base-drafts/pull/4685

Section 5.2

> Endpoints SHOULD set the min_rtt to the newest RTT sample after
> persistent congestion is established.  [...]

Earlier we said that min_rtt was the minimum value observed "over the
lifetime of the path", which seems in conflict with this behavior unless
we somehow declare that the path is re-established after a persistent
congestion event.  Perhaps we could finesse the wording a bit in the
former location (given that we go on to allow re-establishing (or
refreshing) the min_rtt at other times as well)?  (Appendix A.3 might be
affected as well, if changes are made.)

Section 5.3

> After the handshake is confirmed, any acknowledgement delays reported
> by the peer that are greater than the peer's max_ack_delay are
> attributed to unintentional but potentially repeating delays, such as
> scheduler latency at the peer or loss of previous acknowledgements.

I don't think I understand how loss of previous acknowledgements could
induce a peer to delay sending an ack longer than its max_ack_delay
after an ack-eliciting packet.

Section 6.2.1

> A sender SHOULD restart its PTO timer every time an ack-eliciting
> packet is sent or acknowledged, when the handshake is confirmed
> (Section 4.1.2 of [QUIC-TLS]), or when Initial or Handshake keys are
> discarded (Section 4.9 of [QUIC-TLS]).  [...]

If I understand correctly, "handshake is confirmed" and "handshake keys
are discarded" occur simultaneously, thus it is redundant to list both.

Section 6.2.3

> Endpoints
> that do not cease retransmitting packets in response to
> unauthenticated data risk creating an infinite exchange of packets.

We may want to be more precise about what "unauthenticated data" means
here, since in -transport we say that even Initial packets are
"authenticated" in some sense.

Section 6.2.4


> When a PTO timer expires, a sender MUST send at least one ack-
> eliciting packet in the packet number space as a probe.  [...]
> [...]
> When the PTO timer expires, an ack-eliciting packet MUST be sent.  [...]

These look redundant to me (which, of course, does not necessarily mean
that there is no value in having both).

Section 7.7

> Endpoints can implement pacing as they choose.  A perfectly paced
> sender spreads packets exactly evenly over time.  For a window-based
> congestion controller, such as the one in this document, that rate
> can be computed by averaging the congestion window over the round-
> trip time.  Expressed as a rate in bytes:

I cringed a little when I saw a rate being expressed as a unit
expression with no time denominator, but am not sure how it might be
improved.  The best I can come up with is "bytes/time".

Section 8.1

I guess in some sense the observed RTT is also a signal under the
control of a (Dolev-Yao) attacker, in addition to the loss and ECN
signals.

Appendix A

Should we say explicitly that '^' in the pseudocode is used to indicate
exponentiation?

Another well written QUIC document, thank you.