Subject: [quicwg/base-drafts] Move key discard to loss recovery section (#4173)

From: Martin Thomson <[email-masked]>
Date: Wed, 30 Sep 2020 03:33:29 -0700

None of the variables here are congestion control variables, so this is
clearly loss recovery logic that is just misplaced.
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * Move key discard to loss recovery section

-- File Changes --

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

-- Patch Links --

https://github.com/quicwg/base-drafts/pull/4173.patch
https://github.com/quicwg/base-drafts/pull/4173.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/4173

---

From: ianswett <[email-masked]>
Date: Wed, 30 Sep 2020 12:36:02 -0700

@ianswett commented on this pull request.

> @@ -1574,6 +1574,29 @@ DetectAndRemoveLostPackets(pn_space):
 ~~~

+## 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 != ApplicationData)
+  // Remove any unacknowledged packets from flight.
+  foreach packet in sent_packets[pn_space]:
+    if packet.in_flight
+      bytes_in_flight -= size

This was placed below because it modifies bytes_in_flight, but it also modifies a bunch of loss detection state, so I don't have a strong opinion on where it is.

-- 
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/4173#pullrequestreview-499750166

---

From: Martin Thomson <[email-masked]>
Date: Wed, 30 Sep 2020 16:53:34 -0700

@martinthomson commented on this pull request.

 ~~~

Hmm, I missed that.  This argues for dual sections.  I'll work on it.

-- 
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/4173#discussion_r497862743

---

From: Martin Thomson <[email-masked]>
Date: Wed, 30 Sep 2020 16:58:50 -0700

@martinthomson pushed 1 commit.

4d49737dae5663562f59138ab0942d60b6930cf8  Split between sections

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/4173/files/51204d8032f1a49a8498962fb25c4056740ee466..4d49737dae5663562f59138ab0942d60b6930cf8

---

From: ianswett <[email-masked]>
Date: Wed, 30 Sep 2020 17:30:40 -0700

@ianswett approved this pull request.

> +RemoveFromBytesInFlight(discarded):
+  // Remove any unacknowledged packets from flight.
+  foreach packet in discarded:

```suggestion
RemoveFromBytesInFlight(discarded_pn_space):
  // Remove any unacknowledged packets from flight.
  foreach packet in discarded_pn_space:
```

-- 
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/4173#pullrequestreview-499940089

---

From: Martin Thomson <[email-masked]>
Date: Wed, 30 Sep 2020 17:46:59 -0700

Would "discarded_packets" be better?  It's really just "discarded packets" which only happen to be from a given packet number space.

-- 
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/4173#discussion_r497888620

---

From: ianswett <[email-masked]>
Date: Wed, 30 Sep 2020 17:56:14 -0700

Yes, after re-reading the text, discarded_packets seems correct.

-- 
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/4173#discussion_r497894581

---

From: Martin Thomson <[email-masked]>
Date: Wed, 30 Sep 2020 18:07:30 -0700

46f4f1eee24cf4adac511cacad735d139e9ecaeb  Discarded packets, not just discarded

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/4173/files/4d49737dae5663562f59138ab0942d60b6930cf8..46f4f1eee24cf4adac511cacad735d139e9ecaeb

---

From: Jana Iyengar <[email-masked]>
Date: Wed, 30 Sep 2020 23:05:27 -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/4173#pullrequestreview-500029972

---

From: Jana Iyengar <[email-masked]>
Date: Wed, 30 Sep 2020 23:05:33 -0700

Merged #4173 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/4173#event-3827658054