Subject: [quicwg/base-drafts] Ack-eliciting, not inflight (#2117)

From: janaiyengar <[email-masked]>
Date: Tue, 11 Dec 2018 16:40:33 -0800

Closes #2093 
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * ack-eliciting, not inflight

-- File Changes --

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

-- Patch Links --

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

---

From: ianswett <[email-masked]>
Date: Tue, 11 Dec 2018 18:22:48 -0800

ianswett commented on this pull request.

> @@ -792,7 +792,7 @@ Pseudocode for SetLossDetectionTimer follows:
  SetLossDetectionTimer():
     // Don't arm timer if there are no ack-eliciting packets
     // in flight.
-    if (bytes_in_flight == 0):
+    if (no ack-eliciting packets oustanding):

We never define what outstanding means, from what I can find, though we do use in a few spots.  In this case, I believe it should be "no ack-eliciting packets In-flight"?

-- 
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/2117#pullrequestreview-183989952

---

From: janaiyengar <[email-masked]>
Date: Tue, 11 Dec 2018 19:47:48 -0800

janaiyengar commented on this pull request.

  SetLossDetectionTimer():

we use it below, which is why I used it here. I could change them both to "in flight" or leave them at outstanding

-- 
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/2117#discussion_r240874379

---

From: Martin Thomson <[email-masked]>
Date: Tue, 11 Dec 2018 19:48:43 -0800

martinthomson commented on this pull request.

  SetLossDetectionTimer():

```suggestion
    if (no ack-eliciting packets outstanding):
```

This is a mistake I often make too.

-- 
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/2117#pullrequestreview-184003599

---

From: janaiyengar <[email-masked]>
Date: Tue, 11 Dec 2018 19:49:29 -0800

@janaiyengar pushed 1 commit.

d5ddc522151f6d84eb9eabfde2da206ed7627c3b  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/2117/files/5cac86d2caa5620073596ae71adcaf3e268a1a1d..d5ddc522151f6d84eb9eabfde2da206ed7627c3b

---

From: ianswett <[email-masked]>
Date: Wed, 12 Dec 2018 02:31:33 -0800

SetLossDetectionTimer():

I sent #2120 to remove existing uses of outstanding.

-- 
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/2117#discussion_r240959021

---

From: ianswett <[email-masked]>
Date: Mon, 17 Dec 2018 21:11:18 +0000 (UTC)

SetLossDetectionTimer():
     // Don't arm timer if there are no ack-eliciting packets
     // in flight.
-    if (bytes_in_flight == 0):
+    if (no ack-eliciting packets outstanding):

```suggestion
     if (no ack-eliciting packets in flight):
```

-- 
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/2117#pullrequestreview-185799953

---

From: ianswett <[email-masked]>
Date: Mon, 17 Dec 2018 18:46:49 -0800

ianswett 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/2117#pullrequestreview-185887932

---

From: ianswett <[email-masked]>
Date: Mon, 17 Dec 2018 18:46:57 -0800

Merged #2117 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/2117#event-2031894138