Subject: Benjamin Kaduk's No Objection on draft-ietf-quic-recovery-33: (with COMMENT)

From: Benjamin Kaduk via Datatracker <[email-masked]>
Date: Wed, 06 Jan 2021 22:17:49 -0800

Benjamin Kaduk has entered the following ballot position for
draft-ietf-quic-recovery-33: No Objection

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-recovery/

----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

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?

---

From: Lucas Pardue <[email-masked]>
Date: Fri, 8 Jan 2021 02:26:25 +0000

Hi Ben,

Thanks for the review. I've created GitHub issues for each of these, see
the links inline.

On Thu, Jan 7, 2021 at 6:17 AM Benjamin Kaduk via Datatracker <
[email-masked]> wrote:

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

https://github.com/quicwg/base-drafts/issues/4708

> after an ack-eliciting packet.

https://github.com/quicwg/base-drafts/issues/4709

> are discarded" occur simultaneously, thus it is redundant to list both.

https://github.com/quicwg/base-drafts/issues/4710

> "authenticated" in some sense.

https://github.com/quicwg/base-drafts/issues/4711

> that there is no value in having both).

https://github.com/quicwg/base-drafts/issues/4712

> improved.  The best I can come up with is "bytes/time".

https://github.com/quicwg/base-drafts/issues/4713

> control of a (Dolev-Yao) attacker, in addition to the loss and ECN

https://github.com/quicwg/base-drafts/issues/4714

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

https://github.com/quicwg/base-drafts/issues/4715

Cheers,
Lucas
On behalf of QUIC WG Chairs