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

[Lucas Pardue <[e-mail]>]

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 <
[e-mail]> wrote:

> 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.)
>

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


> 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.
>

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


> 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.
>

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


> 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.
>

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


> 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).
>

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


> 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".
>

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


> 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.
>

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


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

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

Cheers,
Lucas
On behalf of QUIC WG Chairs