Subject: [quicwg/base-drafts] Reset PTO and loss detection timer when discarding keys (#3415)

From: ianswett <[email-masked]>
Date: Tue, 04 Feb 2020 15:48:59 -0800

Fixes #3272 

This doesn&#39;t add normative statements, but it could if people would like.
You can view, comment on, or merge this pull request online at:

  https://github.com/quicwg/base-drafts/pull/3415

-- Commit Summary --

  * Reset PTO and loss detection timer when discarding keys

-- File Changes --

    M draft-ietf-quic-recovery.md (3)

-- Patch Links --

https://github.com/quicwg/base-drafts/pull/3415.patch
https://github.com/quicwg/base-drafts/pull/3415.diff

-- 
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/3415

---

From: Marten Seemann <[email-masked]>
Date: Wed, 05 Feb 2020 06:06:18 -0800

marten-seemann 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/3415#pullrequestreview-353739139

---

From: ianswett <[email-masked]>
Date: Fri, 07 Feb 2020 15:04:20 -0800

@ianswett pushed 1 commit.

6b01be6c4d9caa77f6bcefa7d42bcdbfe9f1deec  Add pseudocode

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3415/files/5108eb30b428dec3a39138bbcadfa70dfdd5d05e..6b01be6c4d9caa77f6bcefa7d42bcdbfe9f1deec

---

From: ianswett <[email-masked]>
Date: Fri, 07 Feb 2020 15:07:00 -0800

db231e9d6c2c991c3a9360d3f4832ce32d9a6427  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/3415/files/6b01be6c4d9caa77f6bcefa7d42bcdbfe9f1deec..db231e9d6c2c991c3a9360d3f4832ce32d9a6427

---

From: ianswett <[email-masked]>
Date: Fri, 07 Feb 2020 15:13:13 -0800

Pseudocode added, 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/3415#issuecomment-583655855

---

From: Martin Thomson <[email-masked]>
Date: Sun, 09 Feb 2020 14:04:21 -0800

martinthomson commented on this pull request.

> @@ -1432,6 +1434,26 @@ Invoked from DetectLostPackets when packets are deemed lost.
        congestion_window = kMinimumWindow
 ~~~
 
+## Upon dropping Initial or Handshake keys
+
+When Initial or Handshake keys are discarded, packets from the space
+are discarded and loss detection state is updated.
+
+Pseudocode for OnPacketNumberSpaceDiscarded follows:
+
+~~~
+OnPacketNumberSpaceDiscarded(pn_space):
+  assert(pn_space != 1RTT)
+  // Remove any unacknowledged packets from flight.
+  foreach packet in sent_packets[pn_space]:
+    if packet.in_flight
+      bytes_in_flight -= size
+      packet.in_flight = false

You don't really need to do anything for objects that are being disposed of.  But I guess that this is equivalent to `sent_packets[pn_space].clear()`.

> @@ -1432,6 +1434,26 @@ Invoked from DetectLostPackets when packets are deemed lost.
        congestion_window = kMinimumWindow
 ~~~
 
+## Upon dropping Initial or Handshake keys
+
+When Initial or Handshake keys are discarded, packets from the space
+are discarded and loss detection state is updated.
+
+Pseudocode for OnPacketNumberSpaceDiscarded follows:
+
+~~~
+OnPacketNumberSpaceDiscarded(pn_space):
+  assert(pn_space != 1RTT)

Don't you use ApplicationData?

-- 
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/3415#pullrequestreview-355636875

---

From: ianswett <[email-masked]>
Date: Mon, 10 Feb 2020 06:54:39 -0800

578d2b79af642126ee9be062851b621c8e0751ae  Martin's suggestions

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3415/files/db231e9d6c2c991c3a9360d3f4832ce32d9a6427..578d2b79af642126ee9be062851b621c8e0751ae

---

From: ianswett <[email-masked]>
Date: Mon, 10 Feb 2020 06:55:06 -0800

ianswett commented on this pull request.

Of course, fixed.

-- 
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/3415#discussion_r377111272

---

From: ianswett <[email-masked]>
Date: Mon, 10 Feb 2020 06:53:40 -0800

Good point, I changed it to clear().

-- 
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/3415#discussion_r377110334

---

From: Jana Iyengar <[email-masked]>
Date: Thu, 27 Feb 2020 16:33:20 -0800

janaiyengar commented on this pull request.

Looks fine, but is there a reason to not require this?

> @@ -531,6 +531,9 @@ bytes.
 
 Initial packets and Handshake packets could be never acknowledged, but they are
 removed from bytes in flight when the Initial and Handshake keys are discarded.
+When Initial or Handshake keys are discarded, the PTO and loss detection timers
+are reset, because discarding keys indicates forward progress and the loss

```suggestion
MUST be reset, because discarding keys indicates forward progress and the loss
```

-- 
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/3415#pullrequestreview-366093507

---

From: ianswett <[email-masked]>
Date: Thu, 27 Feb 2020 17:48:19 -0800

36da8076286a15451100e0de314338cee9b6c005  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/3415/files/578d2b79af642126ee9be062851b621c8e0751ae..36da8076286a15451100e0de314338cee9b6c005

---

From: ianswett <[email-masked]>
Date: Thu, 27 Feb 2020 17:48:44 -0800

I'm fine with requiring it, given it's already a design issue.

-- 
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/3415#issuecomment-592270264

---

From: Jana Iyengar <[email-masked]>
Date: Thu, 27 Feb 2020 19:53:35 -0800

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/3415#pullrequestreview-366147685