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

Split PTO calculation into a dedicated method #3681

Merged
merged 12 commits into from
May 31, 2020
75 changes: 41 additions & 34 deletions draft-ietf-quic-recovery.md
Original file line number Diff line number Diff line change
Expand Up @@ -1110,7 +1110,7 @@ loss_detection_timer:
pto_count:
: The number of times a PTO has been sent without receiving an ack.

time_of_last_sent_ack_eliciting_packet\[kPacketNumberSpace]:
time_of_last_ack_eliciting_packet\[kPacketNumberSpace]:
: The time the most recent ack-eliciting packet was sent.

largest_acked_packet\[kPacketNumberSpace]:
Expand Down Expand Up @@ -1140,7 +1140,7 @@ follows:
max_ack_delay = 0
for pn_space in [ Initial, Handshake, ApplicationData ]:
largest_acked_packet[pn_space] = infinite
time_of_last_sent_ack_eliciting_packet[pn_space] = 0
time_of_last_ack_eliciting_packet[pn_space] = 0
loss_time[pn_space] = 0
~~~

Expand All @@ -1163,7 +1163,7 @@ Pseudocode for OnPacketSent follows:
sent_packets[pn_space][packet_number].in_flight = in_flight
if (in_flight):
if (ack_eliciting):
time_of_last_sent_ack_eliciting_packet[pn_space] = now()
time_of_last_ack_eliciting_packet[pn_space] = now()
OnPacketSentCC(sent_bytes)
sent_packets[pn_space][packet_number].size = sent_bytes
SetLossDetectionTimer()
Expand Down Expand Up @@ -1269,19 +1269,43 @@ timers wake up late. Timers set in the past fire immediately.
Pseudocode for SetLossDetectionTimer follows:

~~~
GetEarliestTimeAndSpace(times):
time = times[Initial]
GetLossTimeAndSpace():
time = loss_time[Initial]
space = Initial
for pn_space in [ Handshake, ApplicationData ]:
if (times[pn_space] != 0 &&
(time == 0 || times[pn_space] < time) &&
# Skip ApplicationData until handshake completion.
(pn_space != ApplicationData ||
IsHandshakeComplete()):
time = times[pn_space];
if (time == 0 || loss_time[pn_space] < time):
time = loss_time[pn_space];
space = pn_space
return time, space

GetPtoTimeAndSpace():
duration = (smoothed_rtt + max(4 * rttvar, kGranularity))
* (2 ^ pto_count)
// Arm PTO from now when there are no inflight packets.
if (no in-flight packets):
Copy link
Contributor

Choose a reason for hiding this comment

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

Assuming this is for the amplification protection, something's missing here. Perhaps this should be
if (is_client && no packets in flight && !PeerCompletedAddressValidation()).
Otherwise, you never get into the case below on line 1293.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added an assert that for !PeerCompletedAddressValidation().

I think the condition you're mentioning is:
"if (no in-flight packets in space):
continue;"

That's per PN-space, not for the connection as a whole.

Copy link
Contributor

Choose a reason for hiding this comment

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

never mind, I thought the code below to do the early return when no packets are in flight after address validation was removed. It's still there, so all good.

assert(!PeerCompletedAddressValidation())
if (has handshake keys):
return (now() + duration), Handshake
else:
return (now() + duration), Initial
pto_timeout = infinite
pto_space = Initial
for space in [ Initial, Handshake, ApplicationData ]:
if (no in-flight packets in space):
continue;
if (space == ApplicationData):
// Skip ApplicationData until handshake complete.
if (handshake is not complete):
return pto_timeout, pto_space
// Include max_ack_delay and backoff for ApplicationData.
duration += max_ack_delay * (2 ^ pto_count)

t = time_of_last_ack_eliciting_packet[space] + duration
if (t < pto_timeout):
pto_timeout = t
pto_space = space
return pto_timeout, pto_space

PeerCompletedAddressValidation():
# Assume clients validate the server's address implicitly.
if (endpoint is server):
Expand All @@ -1293,7 +1317,7 @@ PeerCompletedAddressValidation():
has received HANDSHAKE_DONE

SetLossDetectionTimer():
earliest_loss_time, _ = GetEarliestTimeAndSpace(loss_time)
earliest_loss_time, _ = GetLossTimeAndSpace()
if (earliest_loss_time != 0):
// Time threshold loss detection.
loss_detection_timer.update(earliest_loss_time)
Expand All @@ -1313,23 +1337,8 @@ SetLossDetectionTimer():
return

// Determine which PN space to arm PTO for.
sent_time, pn_space = GetEarliestTimeAndSpace(
time_of_last_sent_ack_eliciting_packet)
// Don't arm PTO for ApplicationData until handshake complete.
if (pn_space == ApplicationData &&
handshake is not confirmed):
loss_detection_timer.cancel()
return
if (sent_time == 0):
assert(!PeerCompletedAddressValidation())
sent_time = now()

// Calculate PTO duration
timeout = smoothed_rtt + max(4 * rttvar, kGranularity) +
max_ack_delay
timeout = timeout * (2 ^ pto_count)

loss_detection_timer.update(sent_time + timeout)
timeout, _ = GetPtoTimeAndSpace()
loss_detection_timer.update(timeout)
~~~


Expand All @@ -1342,8 +1351,7 @@ Pseudocode for OnLossDetectionTimeout follows:

~~~
OnLossDetectionTimeout():
earliest_loss_time, pn_space =
GetEarliestTimeAndSpace(loss_time)
earliest_loss_time, pn_space = GetLossTimeAndSpace()
if (earliest_loss_time != 0):
// Time threshold loss Detection
lost_packets = DetectLostPackets(pn_space)
Expand All @@ -1355,8 +1363,7 @@ OnLossDetectionTimeout():
if (bytes_in_flight > 0):
// PTO. Send new data if available, else retransmit old data.
// If neither is available, send a single PING frame.
_, pn_space = GetEarliestTimeAndSpace(
time_of_last_sent_ack_eliciting_packet)
_, pn_space = GetPtoTimeAndSpace()
SendOneOrTwoAckElicitingPackets(pn_space)
else:
assert(endpoint is client without 1-RTT keys)
Expand Down Expand Up @@ -1612,7 +1619,7 @@ OnPacketNumberSpaceDiscarded(pn_space):
bytes_in_flight -= size
sent_packets[pn_space].clear()
// Reset the loss detection and PTO timer
time_of_last_sent_ack_eliciting_packet[pn_space] = 0
time_of_last_ack_eliciting_packet[pn_space] = 0
loss_time[pn_space] = 0
pto_count = 0
SetLossDetectionTimer()
Expand Down