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

Do not reset pto_count on Initial ACKs #3551

Merged
merged 23 commits into from
Apr 19, 2020
Merged
Changes from 9 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
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
15 changes: 12 additions & 3 deletions draft-ietf-quic-recovery.md
Original file line number Diff line number Diff line change
Expand Up @@ -501,7 +501,13 @@ and prevents a server from sending a 1-RTT packet on a PTO expiration before it
has the keys to process an acknowledgement.

When a PTO timer expires, the PTO period MUST be set to twice its current
value. This exponential reduction in the sender's rate is important because
value. The PTO period is set back to the original value upon receiving an
acknowledgement for a non-Initial packet. The PTO timer is not decreased when
the client receives an Initial ACK to ensure the client's anti-deadlock timer
Copy link
Member

Choose a reason for hiding this comment

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

How about changing the text to something like: The PTO period is set back to the original value when receiving an acknowledgement, if the endpoint knows that the peer has completed address validation. The PTO timer is not decreased while the peer is validating the address, to ensure the endpoint's anti-deadlock timer does not fire...

I think that is our intent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, accepted with some small editorial changes.

does not fire too aggressively when the server does not yet have handshake data
to send.

This exponential reduction in the sender's rate is important because
consecutive PTOs might be caused by loss of packets or acknowledgements due to
severe congestion. Even when there are ack-eliciting packets in-flight in
multiple packet number spaces, the exponential increase in probe timeout
Expand Down Expand Up @@ -1139,8 +1145,10 @@ OnAckReceived(ack, pn_space):
OnPacketAcked(acked_packet.packet_number, pn_space)

DetectLostPackets(pn_space)

pto_count = 0
// Reset pto_count unless the server has not yet
// validated the client's address.
if (PeerCompletedAddressValidation()):
pto_count = 0

SetLossDetectionTimer()
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this also be done only when pto_count is set to 0? Why restart the timer here if the pto_count hasn't changed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A new RTT measurement is available, so the alarm value should always be updated.


Expand Down Expand Up @@ -1541,6 +1549,7 @@ OnPacketNumberSpaceDiscarded(pn_space):
// Reset the loss detection and PTO timer
time_of_last_sent_ack_eliciting_packet[kPacketNumberSpace] = 0
loss_time[pn_space] = 0
pto_count = 0
SetLossDetectionTimer()
~~~

Expand Down