Subject: Re: [quicwg/base-drafts] Fix Recovery Pseudocode (#2907)

From: ianswett <[email-masked]>
Date: Fri, 23 Aug 2019 07:14:53 -0700

ianswett commented on this pull request.

> @@ -581,13 +581,13 @@ data.  If no data can be sent, then the PTO alarm MUST NOT be armed until
 data has been received from the client.
 
 Because the server could be blocked until more packets are received, the client
-MUST ensure that the retransmission timer is set if the client does not yet
-have 1-RTT keys.  If the probe timer expires before the client has 1-RTT keys,
-it is possible that the client may not have any crypto data to retransmit.
-However, the client MUST send a new packet, containing only PADDING frames if
-necessary, to allow the server to continue sending data. If Handshake keys
-are available to the client, it MUST send a Handshake packet, and otherwise
-it MUST send an Initial packet in a UDP datagram of at least 1200 bytes.
+MUST ensure that the retransmission timer is set if the client has not received
+an ACK in a Handshake packet or does not yet have 1-RTT keys.  If the probe

I think Martin's right, this should be 'and', since if the client has 1-RTT keys and has nothing to send at lower encryption levels, 

But now that I write that out, would it be better to say "received an ACK in a Handshake or 1-RTT packet."?

-- 
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/2907#discussion_r317152240

---

From: ianswett <[email-masked]>
Date: Fri, 23 Aug 2019 07:31:48 -0700

@ianswett pushed 1 commit.

fef247f1c6f052f0eb439b816018c42879769f9e  Martin's suggestion

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2907/files/59c3af3f32313b54f31eacd6d64df1ad19c1210c..fef247f1c6f052f0eb439b816018c42879769f9e

---

From: Jana Iyengar <[email-masked]>
Date: Fri, 23 Aug 2019 12:06:05 -0700

janaiyengar commented on this pull request.

> @@ -581,17 +581,17 @@ data.  If no data can be sent, then the PTO alarm MUST NOT be armed until
 
 Because the server could be blocked until more packets are received, the client
-MUST ensure that the retransmission timer is set if the client does not yet
-have 1-RTT keys.  If the probe timer expires before the client has 1-RTT keys,
-it is possible that the client may not have any crypto data to retransmit.
-However, the client MUST send a new packet to allow the server to continue
-sending data.  At this stage in the connection, when few to none RTT samples
-have been generated, it is possible that the probe timer expiration is due to
-an incorrect RTT estimate at the client. To allow the client to improve its RTT
-estimate, the new packet that it sends MUST be ack-eliciting.  If Handshake
-keys are available to the client, it MUST send a Handshake packet, and
-otherwise it MUST send an Initial packet in a UDP datagram of at least 1200
-bytes.
+MUST ensure that the retransmission timer is set if the client has not received
+an ACK in a Handshake packet and does not yet have 1-RTT keys.  If the probe

This still doesn't work. I think spell it out a bit more might help clarify... how about replacing the sentence with:

"Since the server could be blocked until more packets are received from the client, it is the client's responsibility to send data to unblock the server until it is certain that the server has finished its validation. That is, the client MUST set the retransmission timer if one of the following conditions is true: the client does not yet have 1-RTT keys, or the client has not received an ACK for one of its Handshake packets."

-- 
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/2907#pullrequestreview-279174450

---

From: ianswett <[email-masked]>
Date: Sat, 24 Aug 2019 16:18:00 -0700

e97e175e7c82e6db7d5478c3d29be2e169773d85  Jana's suggestion

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2907/files/fef247f1c6f052f0eb439b816018c42879769f9e..e97e175e7c82e6db7d5478c3d29be2e169773d85

---

From: ianswett <[email-masked]>
Date: Sat, 24 Aug 2019 16:18:41 -0700

Suggestion accepted with some minor tweaks, PTAL.

-- 
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/2907#discussion_r317377161

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 27 Aug 2019 15:19:45 -0700

@janaiyengar pushed 1 commit.

b47dbbc79b26d958d54146d09c0c172cd28db5d4  nit

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2907/files/e97e175e7c82e6db7d5478c3d29be2e169773d85..b47dbbc79b26d958d54146d09c0c172cd28db5d4

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 27 Aug 2019 15:34:51 -0700

janaiyengar requested changes on this pull request.

> @@ -580,14 +580,17 @@ data at Handshake encryption MUST be retransmitted before any ApplicationData
 data.  If no data can be sent, then the PTO alarm MUST NOT be armed until
 data has been received from the client.
 
-Because the server could be blocked until more packets are received, the client
-MUST ensure that the retransmission timer is set if the client does not yet
-have 1-RTT keys.  If the probe timer expires before the client has 1-RTT keys,
-it is possible that the client may not have any crypto data to retransmit.
-However, the client MUST send a new packet to allow the server to continue
-sending data.  At this stage in the connection, when few to none RTT samples
-have been generated, it is possible that the probe timer expiration is due to
-an incorrect RTT estimate at the client. To allow the client to improve its RTT
+Since the server could be blocked until more packets are received from the
+client, it is the client's responsibility to send packets to unblock the server
+until it is certain that the server has finished its address validation
+(see Section 8 of {{QUIC-TRANSPORT}}).  That is, the client MUST set the
+probe timer if one of the following conditions is true: the client
+does not yet have 1-RTT keys, or the client has not received an acknowledgement

```suggestion
does not yet have handshake confirmation, or the client has not received an acknowledgement
```

> @@ -1190,9 +1193,13 @@ SetLossDetectionTimer():
     return
 
   // Don't arm timer if there are no ack-eliciting packets
-  // in flight and the handshake is complete.
-  if (endpoint is client with 1-RTT keys &&
-      no ack-eliciting packets in flight):
+  // in flight and the endpoint is a server. Arm the
+  // timer until the client has received a Handshake ACK
+  // or has completed the handshake.
+  if (no ack-eliciting packets in flight &&
+      (endpoint is server ||
+       has 1-RTT keys ||
+       has received Handshake ACK)):

Chatting about this at the editors meeting, @martinthomson pointed out that this isn't quite right. Here's my reformulation that I believe is the right condition:
"endpoint is server || until the handshake is confirmed || has received Handshake ACK"

-- 
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/2907#pullrequestreview-280506163

---

From: Martin Thomson <[email-masked]>
Date: Tue, 27 Aug 2019 15:35:21 -0700

martinthomson commented on this pull request.

     return
 
+  // in flight and the endpoint is a server.  Do arm the

The prose I would use here is:

An endpoint always keeps the timer running if it has an ack-eliciting packet that has not been acknowledged.

A client keeps the timer running during the handshake until it has confirmed that the server is no longer limited in how much it can send.  The server needs to acknowledge a Handshake packet or a 1-RTT packet from the client to demonstrate this.

-- 
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/2907#pullrequestreview-280507972

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 27 Aug 2019 15:38:03 -0700

return

```suggestion
  // Cancel the timer if there are no ack-eliciting packets
```

     return

```suggestion
  // in flight and the endpoint is a server. Do not cancel the
```

-- 
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/2907#pullrequestreview-280508157