Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Behavior around key availability delays during handshake #3874

Merged
merged 35 commits into from
Sep 1, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
f851c8c
allow endpoints to generate traffic keys asynchronously, write down t…
kazuho Jul 8, 2020
6e54f3e
QUIC does not generate keys, TLS provides them
kazuho Jul 8, 2020
964fd96
Rearrange the text, based on @marten-seeman's point that the intent o…
kazuho Jul 8, 2020
2e6bc76
both endpoints refrain from sending probes before obtaining the read …
kazuho Jul 8, 2020
784b062
explicitly state from which PN spaces to choose
kazuho Jul 9, 2020
8334a5e
MT's suggestion
kazuho Jul 9, 2020
a77e003
Update draft-ietf-quic-recovery.md
kazuho Jul 9, 2020
e024864
Jana's suggestions
kazuho Jul 14, 2020
07a668e
based on the established rationale, PTO should be disabled until the …
kazuho Jul 15, 2020
9a008e7
Apply suggestions from code review
janaiyengar Aug 4, 2020
5e25bcd
Resolution from #3821 (pseudocode missing)
janaiyengar Aug 4, 2020
7f22ad8
agreed changes from editors meeting
janaiyengar Aug 4, 2020
728dc37
mt's comments
janaiyengar Aug 5, 2020
1029078
kazuho corrections
janaiyengar Aug 5, 2020
9821f07
s/MAY/SHOULD, and some editorializing
janaiyengar Aug 5, 2020
1fe9727
undo SHOULD
janaiyengar Aug 5, 2020
0000ceb
Add para on local delays when ack is buffered
janaiyengar Aug 5, 2020
8083104
Apply suggestions from code review
janaiyengar Aug 5, 2020
3d26a53
more changes
janaiyengar Aug 5, 2020
e796999
fix ref
janaiyengar Aug 6, 2020
ef2a714
tightening
janaiyengar Aug 6, 2020
4c41f55
Apply suggestions from code review
janaiyengar Aug 6, 2020
7f87ff7
move local delay para up
janaiyengar Aug 6, 2020
6635adb
Ian's comments
janaiyengar Aug 6, 2020
ddb4b86
Update draft-ietf-quic-recovery.md
janaiyengar Aug 6, 2020
0b9b2e1
RTT
janaiyengar Aug 6, 2020
936c4a0
more comments
janaiyengar Aug 6, 2020
e7d423e
Ian's comments
janaiyengar Aug 18, 2020
721ee00
s/ack immediately/not delay intentionally
janaiyengar Aug 18, 2020
952952a
Martin's concern in #4035
janaiyengar Aug 20, 2020
99d976f
kazuho's comments
janaiyengar Aug 20, 2020
2a50ecd
kazuho's comments
janaiyengar Aug 20, 2020
95988d7
Update draft-ietf-quic-recovery.md
janaiyengar Aug 28, 2020
cae02dc
reference for why Initial packets won't be delayed
janaiyengar Aug 28, 2020
1a02636
Merge branch 'master' into kazuho/pto-vs-key-generation
janaiyengar Sep 1, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 19 additions & 26 deletions draft-ietf-quic-recovery.md
Original file line number Diff line number Diff line change
Expand Up @@ -340,19 +340,9 @@ The calculation of smoothed_rtt uses path latency after adjusting RTT samples
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}}.

A peer MUST immediately acknowledge all ack-eliciting Initial packets.

Prior to handshake confirmation, a peer might not have the packet protection
keys for Handshake, 0-RTT, or 1-RTT packets when they are received. It might
therefore buffer them and acknowledge them when the requisite keys become
available. When this is not the case, the peer MUST immediately acknowledge all
ack-eliciting Handshake packets, and MUST NOT delay acknowledgement of
ack-eliciting 0-RTT, or 1-RTT packets for any longer than the period that it
advertised in the max_ack_delay transport parameter (Section 18.2 of
{{QUIC-TRANSPORT}}).

As the peer might report large acknowledgement delays during the handshake,
the endpoint MAY ignore max_ack_delay until the handshake is confirmed (Section
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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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
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

endpoint MAY ignore max_ack_delay until the handshake is confirmed (Section
4.1.2 of {{QUIC-TLS}}). Since these large acknowledgement delays, when they
janaiyengar marked this conversation as resolved.
Show resolved Hide resolved
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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
can use them without limiting them to the max_ack_delay and avoid unnecessarily
can use them without limiting them to the max_ack_delay to avoid unnecessarily

Expand All @@ -370,7 +360,7 @@ endpoint:

- MAY ignore the acknowledgement delay for Initial packets,
ianswett marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand correctly, this leads to a higher current_rtt measurement. It would be nice if the paragraph above included a motivation for that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't because the peer is expected to ack these immediately. Added text and a reference.


- MAY ignore the peer's max_ack_delay until the handshake is confirmed,
- SHOULD ignore the peer's max_ack_delay until the handshake is confirmed,
janaiyengar marked this conversation as resolved.
Show resolved Hide resolved

- MUST use the lesser of the acknowledgement delay and the peer's max_ack_delay
after the handshake is confirmed,
Expand All @@ -397,8 +387,9 @@ default values.
On subsequent RTT samples, smoothed_rtt and rttvar evolve as follows:

~~~
ack_delay = acknowledgement delay from ACK frame
ack_delay = min(ack_delay, max_ack_delay)
ack_delay = decoded acknowledgement delay from ACK frame
janaiyengar marked this conversation as resolved.
Show resolved Hide resolved
if (handshake confirmed):
ack_delay = min(ack_delay, max_ack_delay)
janaiyengar marked this conversation as resolved.
Show resolved Hide resolved
adjusted_rtt = latest_rtt
if (min_rtt + ack_delay < latest_rtt):
adjusted_rtt = latest_rtt - ack_delay
Expand Down Expand Up @@ -541,8 +532,9 @@ and max_ack_delay, to account for the maximum time by which a receiver might
delay sending an acknowledgement.

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, as specified in 13.2.1
of {{QUIC-TRANSPORT}}.
max_ack_delay in the PTO period computation is set to 0, since the peer is
janaiyengar marked this conversation as resolved.
Show resolved Hide resolved
expected to acknowledge these packets immediately; see 13.2.1 of
{{QUIC-TRANSPORT}}.

The PTO period MUST be at least kGranularity, to avoid the timer expiring
immediately.
Expand All @@ -551,10 +543,10 @@ When ack-eliciting packets in multiple packet number spaces are in flight, the
timer MUST be set to the earlier value of the Initial and Handshake packet
number spaces.
janaiyengar marked this conversation as resolved.
Show resolved Hide resolved

An endpoint MUST NOT set its PTO timer for 0-RTT and 1-RTT packets until the
handshake is confirmed. Doing so prevents the endpoint from retransmitting
information in packets when either the peer does not yet have the keys to
process them or the endpoint does not yet have the keys to process their
An endpoint MUST NOT set its PTO timer for the application data packet number
space until the handshake is confirmed. Doing so prevents the endpoint from
retransmitting information in packets when either the peer does not yet have the
keys to process them or the endpoint does not yet have the keys to process their
acknowledgements. For example, this can happen when a client sends 0-RTT packets
to the server; it does so without knowing whether the server will be able to
decrypt them. Similarly, this can happen when a server sends 1-RTT packets
Expand Down Expand Up @@ -1292,8 +1284,9 @@ UpdateRtt(ack_delay):

// min_rtt ignores acknowledgment delay.
min_rtt = min(min_rtt, latest_rtt)
// Limit ack_delay by max_ack_delay
ack_delay = min(ack_delay, max_ack_delay)
// Limit ack_delay to max_ack_delay after handshake confirmation.
janaiyengar marked this conversation as resolved.
Show resolved Hide resolved
if (handshake confirmed):
ack_delay = min(ack_delay, max_ack_delay)
// Adjust for acknowledgment delay if plausible.
adjusted_rtt = latest_rtt
if (latest_rtt > min_rtt + ack_delay):
Expand Down Expand Up @@ -1341,8 +1334,8 @@ GetPtoTimeAndSpace():
if (no in-flight packets in space):
continue;
if (space == ApplicationData):
// Skip Application Data until handshake complete.
if (handshake is not complete):
// Skip Application Data until handshake confirmed.
if (handshake is not confirmed):
return pto_timeout, pto_space
// Include max_ack_delay and backoff for Application Data.
duration += max_ack_delay * (2 ^ pto_count)
Expand Down
17 changes: 15 additions & 2 deletions draft-ietf-quic-transport.md
Original file line number Diff line number Diff line change
Expand Up @@ -3530,11 +3530,19 @@ communicated using the max_ack_delay transport parameter; see
contract: an endpoint promises to never intentionally delay acknowledgments of
an ack-eliciting packet by more than the indicated value. If it does, any excess
accrues to the RTT estimate and could result in spurious or delayed
retransmissions from the peer. For Initial and Handshake packets, a
max_ack_delay of 0 is used. The sender uses the receiver's max_ack_delay value
retransmissions from the peer. A sender uses the receiver's max_ack_delay value
in determining timeouts for timer-based retransmission, as detailed in Section
6.2 of {{QUIC-RECOVERY}}.

An endpoint MUST immediately acknowledge all ack-eliciting Initial and Handshake
packets and MUST NOT delay acknowledgement of ack-eliciting 0-RTT, or 1-RTT
packets for any longer than the period that it advertised in the max_ack_delay
transport parameter (Section 18.2 of {{QUIC-TRANSPORT}}), with the following
exception. Prior to handshake confirmation, an endpoint might not have packet
protection keys for decrypting Handshake, 0-RTT, or 1-RTT packets when they are
received. It might therefore buffer them and acknowledge them when the requisite
keys become available.

Since packets containing only ACK frames are not congestion controlled, an
endpoint MUST NOT send more than one such packet in response to receiving an
ack-eliciting packet.
Expand Down Expand Up @@ -3691,6 +3699,11 @@ endpoint MUST NOT include delays that it does not control when populating the
ACK Delay field in an ACK frame, but endpoints SHOULD include buffering delays
caused by unavailability of decryption keys.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to explain why this is a SHOULD? We could say that if a packet containing CRYPTO data from the previous PN was lost, the buffering delay will be > 1 RTT. That should be a very good motivation to follow this SHOULD.


When the measured acknowledgement delay is larger than its max_ack_delay, an
endpoint SHOULD report the measured delay. This information is especially useful
during the handshake when delays might be large; see
{{sending-acknowledgements}}.

### ACK Frames and Packet Protection

ACK frames MUST only be carried in a packet that has the same packet number
Expand Down