Subject: [quicwg/base-drafts] Replace recovery epoch with congestion event (#2585)

From: ianswett <[email-masked]>
Date: Tue, 02 Apr 2019 13:33:28 -0700

Recovery epoch was undefined and congestion event makes more sense for QUIC.

Fixes #2566 
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * Replace recovery epoch with congestion event

-- File Changes --

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

-- Patch Links --

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

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 02 Apr 2019 15:14:15 -0700

janaiyengar commented on this pull request.

> @@ -1319,7 +1319,7 @@ window.
 ~~~
    CongestionEvent(sent_time):
      // Start a new congestion event if the sent time is larger
-     // than the start time of the previous recovery epoch.
+     // than the start time of the previous congestion event.

Suggestion: "... if the packet was sent before the last congestion event started."

-- 
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/2585#pullrequestreview-221928247

---

From: ianswett <[email-masked]>
Date: Tue, 02 Apr 2019 15:17:35 -0700

@ianswett pushed 1 commit.

231bc238704ae7d477ca1e9c41f5eb9984a11009  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/2585/files/80df4b0edf388d71ef5481406d169d64eb32f374..231bc238704ae7d477ca1e9c41f5eb9984a11009

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 02 Apr 2019 15:19:04 -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/2585#pullrequestreview-221929836

---

From: ianswett <[email-masked]>
Date: Tue, 02 Apr 2019 15:24:58 -0700

Merged #2585 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/2585#event-2248093438

---

From: Marten Seemann <[email-masked]>
Date: Tue, 02 Apr 2019 19:49:49 -0700

marten-seemann commented on this pull request.

> @@ -1318,8 +1318,8 @@ window.
 
 ~~~
    CongestionEvent(sent_time):
-     // Start a new congestion event if the sent time is larger
-     // than the start time of the previous recovery epoch.
+     // Start a new congestion event if packet was sent after the
+     // start of the previous congestion event.

I understand what the start and the end of an epoch are, but what is the start and end of a congestion event.
In my understanding, a congestion event is when a packet is acknowledged or when a packet is declared lost, and there’s no start and end to 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/2585#pullrequestreview-221985395

---

From: MikkelFJ <[email-masked]>
Date: Tue, 02 Apr 2019 20:13:31 -0700

Events tend to be points in time, I agree.

-- 
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/2585#issuecomment-479316559

---

From: ianswett <[email-masked]>
Date: Wed, 03 Apr 2019 06:04:20 -0700

ianswett commented on this pull request.

 ~~~

Good point.  We don't use epoch anywhere else in this doc except this pseudocode, and it's different from the definition in security contexts, so I'd like to avoid the term.

Do you have any other suggestions?  Congestion Period, etc?  I'm not even that happy about using InRecovery here, since in TCP that has a distinct meaning that doesn't map 1:1 to QUIC.

-- 
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/2585#discussion_r271731184

---

From: MikkelFJ <[email-masked]>
Date: Wed, 03 Apr 2019 06:13:25 -0700

mikkelfj commented on this pull request.

 ~~~

Maybe keep congestion event as the trigger. Isn't the period referred to as the recovery period, if entered.

-- 
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/2585#discussion_r271735047

---

From: MikkelFJ <[email-masked]>
Date: Wed, 03 Apr 2019 06:14:55 -0700

~~~

Well, seems like you just made a commit doing exactly that ...

-- 
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/2585#discussion_r271735746

---

From: ianswett <[email-masked]>
Date: Wed, 03 Apr 2019 06:15:56 -0700

~~~

PTAL at https://github.com/quicwg/base-drafts/pull/2588

-- 
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/2585#discussion_r271736247

---

From: MikkelFJ <[email-masked]>
Date: Wed, 03 Apr 2019 13:17:23 +0000 (UTC)

~~~

I did, we keep postposting.

-- 
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/2585#discussion_r271736845