Subject: Re: [quicwg/base-drafts] Execute the PTO alarm immediately if in the past (#4831)

From: ianswett <[email-masked]>
Date: Thu, 04 Mar 2021 07:00:25 -0800

@ianswett pushed 1 commit.

6bde79401d0982175dc98eef55ab77219caf544a  Some of MT's suggestions

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/4831/files/042a58b716d68b69e0f462cb7fa92be9fe8fedc4..6bde79401d0982175dc98eef55ab77219caf544a

---

From: ianswett <[email-masked]>
Date: Thu, 04 Mar 2021 07:02:24 -0800

@ianswett commented on this pull request.

Thanks Martin, PTAL

> @@ -664,8 +664,11 @@ data it can send is limited to three times the amount of data received,
 as specified in {{Section 8.1 of QUIC-TRANSPORT}}. If no additional data can be
 sent, the server's PTO timer MUST NOT be armed until datagrams have been
 received from the client, because packets sent on PTO count against the
-anti-amplification limit. Note that the server could fail to validate the
-client's address even if 0-RTT is accepted.
+anti-amplification limit.  The server's PTO timer could be re-armed to a time
+in the past after being amplification limited, in which case it is executed
+immediately. Doing so avoids sending new 1-RTT packets prior to packets
+critical to the completion of the handshake.  Note that the server could fail
+to validate the client's address even if 0-RTT is accepted.

I reworked it to connect it back, which I think makes sense given this mostly happens when 0-RTT is accepted, but address validation fails.

> +      // PTO would have already expired, execute it
+      // immediately.

Suggestion taken with a small change.

> +anti-amplification limit.  The server's PTO timer could be re-armed to a time
+in the past after being amplification limited, in which case it is executed
+immediately. Doing so avoids sending new 1-RTT packets prior to packets
+critical to the completion of the handshake.  Note that the server could fail

I created a new paragraph and worked in the beginning of your suggestion, but I was having a harder time with the end.

-- 
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/4831#pullrequestreview-604177915

---

From: ianswett <[email-masked]>
Date: Thu, 04 Mar 2021 07:03:23 -0800

59dcd7eb518e02fcf81a56268db4e899d4e05f67  Update draft-ietf-quic-recovery.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/4831/files/6bde79401d0982175dc98eef55ab77219caf544a..59dcd7eb518e02fcf81a56268db4e899d4e05f67

---

From: ianswett <[email-masked]>
Date: Thu, 04 Mar 2021 07:03:21 -0800

> @@ -1375,6 +1381,10 @@ OnDatagramReceived(datagram):
   // PTO timer to avoid deadlock.
   if (server was at anti-amplification limit):
     SetLossDetectionTimer()
+    if loss_detection_timer.timeout < now():
+      // Execute PTO if it would have expired
+	    // while the amplification limit applied.

```suggestion
	     // while the amplification limit applied.
```

-- 
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/4831#pullrequestreview-604198584

---

From: ianswett <[email-masked]>
Date: Thu, 04 Mar 2021 07:04:06 -0800

e1681858ee08b703a8b1955038fce13c994807d2  Update draft-ietf-quic-recovery.md

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

---

From: Martin Thomson <[email-masked]>
Date: Thu, 04 Mar 2021 15:12:13 -0800

@martinthomson approved this pull request.

Thanks Ian. 

-- 
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/4831#pullrequestreview-604652365

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 23 Mar 2021 15:36:12 -0700

@janaiyengar commented on this pull request.

> @@ -664,8 +664,14 @@ data it can send is limited to three times the amount of data received,
 as specified in {{Section 8.1 of QUIC-TRANSPORT}}. If no additional data can be
 sent, the server's PTO timer MUST NOT be armed until datagrams have been
 received from the client, because packets sent on PTO count against the
-anti-amplification limit. Note that the server could fail to validate the
-client's address even if 0-RTT is accepted.
+anti-amplification limit.
+
+When the server receives a datagram from the client, the amplification limit is
+increased and the server resets the PTO timer.  If the PTO timer is then set to
+a time in the past, it is executed immediately. Doing so avoids sending new
+1-RTT packets prior to packets critical to the completion of the handshake.
+This could be common when 0-RTT is accepted, but the server fails to validate

```suggestion
This can happen when 0-RTT is accepted but the server fails to validate
```

-- 
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/4831#pullrequestreview-619142843

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 23 Mar 2021 15:36:56 -0700

@janaiyengar approved this pull request.

-- 
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/4831#pullrequestreview-619143215

---

From: ianswett <[email-masked]>
Date: Tue, 23 Mar 2021 15:40:23 -0700

It can happen without 0-RTT, it's just a lot more likely with 0-RTT.

-- 
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/4831#discussion_r600006158

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 23 Mar 2021 15:45:28 -0700

> +This could be common when 0-RTT is accepted, but the server fails to validate
+the client's address.

```suggestion
In particular, this can happen when 0-RTT is accepted but the server fails to
validate the client's address.
```

-- 
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/4831#pullrequestreview-619147490

---

From: ianswett <[email-masked]>
Date: Tue, 23 Mar 2021 15:46:14 -0700

78e6a89d0dc165c602f99b9f150cb968e23f1683  Update draft-ietf-quic-recovery.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/4831/files/e1681858ee08b703a8b1955038fce13c994807d2..78e6a89d0dc165c602f99b9f150cb968e23f1683

---

From: ianswett <[email-masked]>
Date: Tue, 23 Mar 2021 15:48:23 -0700

Merged #4831 into master.

-- 
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/4831#event-4498893625