Re: [quicwg/base-drafts] Do not reset pto_count on Initial ACKs (#3551)

[Jana Iyengar <[e-mail]>]

@janaiyengar commented on this pull request.

Thanks, Ian, a couple of comments.

> @@ -501,7 +501,13 @@ packet on a PTO expiration before confirming that the server is able to decrypt
 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 reset based on the latest rtt information when

```suggestion
value.  The PTO period is set based on the latest RTT information when
```

> @@ -501,7 +501,13 @@ packet on a PTO expiration before confirming that the server is able to decrypt
 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 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

```suggestion
The PTO backoff is not decreased even on receiving an
acknowledgement as long as the peer is validating this endpoint's
```

> +address, to ensure the endpoint's anti-deadlock timer does not fire too
+aggressively when the server does not yet have handshake data to send.

```suggestion
address. Doing so ensures that the endpoint's anti-deadlock timer is not 
set too aggressively when the server seems to be slow in responding with
handshake data.
```

> @@ -1147,7 +1153,10 @@ OnAckReceived(ack, pn_space):
     OnPacketsLost(lost_packets)
   OnPacketsAcked(newly_acked_packets)
 
-  pto_count = 0
+  // Reset pto_count unless the server has not yet
+  // validated the client's address.
+  if (PeerCompletedAddressValidation()):
+    pto_count = 0
   SetLossDetectionTimer()

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?

-- 
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/3551#pullrequestreview-390450027