Re: [quicwg/base-drafts] Allow endpoints to generate traffic keys asynchronously (#3874)

[ianswett <[e-mail]>]

@ianswett commented on this pull request.



> +As the peer might report acknowledgement delays that are larger than the peer's
+max_ack_delay during the handshake (Section 13.2.1 of {{QUIC-TRANSPORT}}), the

```suggestion
The peer might report acknowledgement delays that are larger than the peer's
max_ack_delay during the handshake (Section 13.2.1 of {{QUIC-TRANSPORT}}), so the
```

> -of the ACK frame as described in Section 19.3 of {{QUIC-TRANSPORT}}. For packets
-sent in the Application Data packet number space, a peer limits any delay in
-sending an acknowledgement for an ack-eliciting packet to no greater than the
-value it advertised in the max_ack_delay transport parameter. Consequently, when
-a peer reports an acknowledgment delay that is greater than its max_ack_delay,
-the delay is attributed to reasons out of the peer's control, such as scheduler
-latency at the peer or loss of previous ACK frames.  Any delays beyond the
-peer's max_ack_delay are therefore considered effectively part of path delay and
-incorporated into the smoothed_rtt estimate.
+for acknowledgement delays. These delays are computed using the ACK Delay
+field of the ACK frame as described in Section 19.3 of {{QUIC-TRANSPORT}}.
+
+As the peer might report acknowledgement delays that are larger than the peer's
+max_ack_delay during the handshake (Section 13.2.1 of {{QUIC-TRANSPORT}}), the
+endpoint SHOULD ignore max_ack_delay until the handshake is confirmed (Section
+4.1.2 of {{QUIC-TLS}}). Since these large acknowledgement delays, when they

```suggestion
4.1.2 of {{QUIC-TLS}}). Since these acknowledgement delays, when they
```

> -sending an acknowledgement for an ack-eliciting packet to no greater than the
-value it advertised in the max_ack_delay transport parameter. Consequently, when
-a peer reports an acknowledgment delay that is greater than its max_ack_delay,
-the delay is attributed to reasons out of the peer's control, such as scheduler
-latency at the peer or loss of previous ACK frames.  Any delays beyond the
-peer's max_ack_delay are therefore considered effectively part of path delay and
-incorporated into the smoothed_rtt estimate.
+for acknowledgement delays. These delays are computed using the ACK Delay
+field of the ACK frame as described in Section 19.3 of {{QUIC-TRANSPORT}}.
+
+As the peer might report acknowledgement delays that are larger than the peer's
+max_ack_delay during the handshake (Section 13.2.1 of {{QUIC-TRANSPORT}}), the
+endpoint SHOULD ignore max_ack_delay until the handshake is confirmed (Section
+4.1.2 of {{QUIC-TLS}}). Since these large acknowledgement delays, when they
+occur, are likely to be non-repeating and limited to the handshake, the endpoint
+can use them without limiting them to the max_ack_delay and avoid unnecessarily

```suggestion
can use them without limiting them to the max_ack_delay to avoid unnecessarily
```

>  
-- MUST NOT apply the adjustment if the resulting RTT sample is smaller than the
-  min_rtt.  This limits the underestimation that a misreporting peer can cause
-  to the smoothed_rtt.
+- MUST use the lesser of the acknowledgement delay and the peer's max_ack_delay
+  after the handshake is confirmed,
+
+- MUST NOT apply the adjustments above if the resulting RTT sample is smaller

```suggestion
- MUST NOT subtract the acknowledgement delay if the resulting RTT sample is smaller
```

> +Additionaly, an endpoint might postpone the processing of acknowledgements when
+the corresponding decryption keys are not immediately available. For example, a

```suggestion
Additionaly, an endpoint might be unable to process acknowledgements when
the corresponding decryption keys are not immediately available. For example, a
```

>  
-- MUST NOT apply the adjustment if the resulting RTT sample is smaller than the
-  min_rtt.  This limits the underestimation that a misreporting peer can cause
-  to the smoothed_rtt.
+- MUST use the lesser of the acknowledgement delay and the peer's max_ack_delay
+  after the handshake is confirmed,
+
+- MUST NOT apply the adjustments above if the resulting RTT sample is smaller
+  than the min_rtt.  This limits the underestimation of the smoothed_rtt because
+  of a misreporting peer.
+
+Additionaly, an endpoint might postpone the processing of acknowledgements when
+the corresponding decryption keys are not immediately available. For example, a
+client might receive an acknowledgement for a 0-RTT packet that it cannot
+decrypt because 1-RTT packet protection keys are not yet available to it. In
+such cases, an endpoint SHOULD ignore such local delays in its RTT sample until

```suggestion
such cases, an endpoint SHOULD subtract such local delays from its RTT sample until
```

>  
-The PTO value MUST be set to at least kGranularity, to avoid the timer expiring
+When the PTO is armed for Initial or Handshake packet number spaces, the
+max_ack_delay in the PTO period computation is set to 0, since the peer is

This seems potentially at odds with the text above about ignoring max_ack_delay until the handshake is confirmed?

> +field of the ACK frame as described in Section 19.3 of {{QUIC-TRANSPORT}}.
+
+As the peer might report acknowledgement delays that are larger than the peer's
+max_ack_delay during the handshake (Section 13.2.1 of {{QUIC-TRANSPORT}}), the
+endpoint SHOULD ignore max_ack_delay until the handshake is confirmed (Section
+4.1.2 of {{QUIC-TLS}}). Since these large acknowledgement delays, when they
+occur, are likely to be non-repeating and limited to the handshake, the endpoint
+can use them without limiting them to the max_ack_delay and avoid unnecessarily
+inflating the smoothed_rtt estimate.
+
+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. Therefore, these extra delays are
+considered effectively part of path delay and incorporated into the smoothed_rtt
+estimate.

I agree with @nibanks that I thought we already had text for that, though it'd be in transport.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3874#pullrequestreview-465443983