Do not reset pto_count on Initial ACKs #3551
Conversation
In order to avoid sending anti-deadlock packets too quickly when the server doesn't yet have anything to send. Fixes #3546
When writing #3551, I found some of the text organization to be odd, so this moves it some.
ianswett
added
the
design
This should have been in the PR that created OnPacketNumberSpaceDiscarded().
Co-Authored-By: Martin Thomson <mt@lowentropy.net>
draft-ietf-quic-recovery.md
Outdated
| 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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Agreed, accepted with some small editorial changes.
draft-ietf-quic-recovery.md
Outdated
| value. This exponential reduction in the sender's rate is important because | ||
| value. The PTO period is reset based on the latest rtt information when | ||
| receiving an acknowledgement if the peer has completed address validation. | ||
| The PTO timer is not decreased while the peer is validating the |
There was a problem hiding this comment.
| The PTO timer is not decreased while the peer is validating the | |
| The PTO backoff is not decreased even on receiving an | |
| acknowledgement as long as the peer is validating this endpoint's |
| // Reset pto_count unless the server has not yet | ||
| // validated the client's address. | ||
| if (PeerCompletedAddressValidation()): | ||
| pto_count = 0 | ||
| SetLossDetectionTimer() |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
A new RTT measurement is available, so the alarm value should always be updated.
Co-Authored-By: Jana Iyengar <jri.ietf@gmail.com>
In order to avoid sending anti-deadlock packets too quickly when the server doesn't yet have anything to send.
Also sets pto_count to 0 in pseudocode when Initial or Handshake keys are dropped, lining it up with existing normative text.(this could be split into a separate non-design PR if people prefer)
Adds the term 'backoff' which is usually reset upon receiving an acknowledgement. The lines up more closely with 'pto_count' in the pseudocode.
Fixes #3546