Subject: [quicwg/base-drafts] Be clearer about persistent congestion (#4429)

From: Jana Iyengar <[email-masked]>
Date: Tue, 08 Dec 2020 14:03:05 -0800

@msvoelker rightly noted lack of clarity ([here](https://github.com/quicwg/base-drafts/pull/4421#discussion_r538073397)), which this PR attempts to fix.

There&#39;s a new MUST here, but I think it was again implicit.
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * more text-fixing for persistent congestion

-- File Changes --

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

-- Patch Links --

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

---

From: Martin Thomson <[email-masked]>
Date: Tue, 08 Dec 2020 14:49:59 -0800

@martinthomson commented on this pull request.

The MUST is weird.  The other change is fine.

> @@ -994,11 +994,11 @@ congestion without depending on PTO expiration.
 
 ### Establishing Persistent Congestion
 
-A sender establishes persistent congestion after the receipt of an
-acknowledgement if at least two ack-eliciting packets are declared lost, and:
+A sender MUST establish persistent congestion after the receipt of an

MUST isn't necessary here.

```suggestion
A sender establishes persistent congestion after the receipt of an
```

-- 
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/4429#pullrequestreview-547698024

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 08 Dec 2020 15:02:22 -0800

@janaiyengar commented on this pull request.

 

@martinthomson : Can you clarify why you think so? If you are thinking about enforceability, this document has several other unenforceable MUSTs, especially in the congestion controller.

-- 
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/4429#discussion_r538873689

---

From: ianswett <[email-masked]>
Date: Tue, 08 Dec 2020 15:42:51 -0800

@ianswett commented on this pull request.

 * all packets, across all packet number spaces, sent between the send times of
-  two ack-eliciting packets are declared lost;
+  these two packets are declared lost;

I know we keep disagreeing about this, but I think paralellism is important.  Above, they're called "two ack-eliciting packets" and below we drop 'ack-eliciting', which I find confusing.

If you don't want to restate ack-eliciting, then I'd suggest "if two packets that are ack-eliciting are declared lost,"...

 
-A sender establishes persistent congestion after the receipt of an
-acknowledgement if at least two ack-eliciting packets are declared lost, and:
+A sender MUST establish persistent congestion after the receipt of an
+acknowledgement if two ack-eliciting packets are declared lost, and:

acknowledgement if two packets which are ack-eliciting are declared lost, and:

-- 
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/4429#pullrequestreview-547714658

---

From: ianswett <[email-masked]>
Date: Tue, 08 Dec 2020 15:43:59 -0800

```suggestion

-- 
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/4429#pullrequestreview-547726659

---

From: Martin Thomson <[email-masked]>
Date: Tue, 08 Dec 2020 15:44:00 -0800

* the duration between the send times of these two packets exceeds the
   persistent congestion duration ({{pc-duration}}); and

down in the third bullet, maybe s/both packets/these two packets/

-- 
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/4429#pullrequestreview-547726645

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 08 Dec 2020 15:46:35 -0800

@janaiyengar pushed 1 commit.

644335442f818d1310e20aaf2dbe2aad1be18671  ian's suggestions

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/4429/files/4a4f6d8ae036c65fcc6d963b516fd42d27f04f4e..644335442f818d1310e20aaf2dbe2aad1be18671

---

From: Martin Thomson <[email-masked]>
Date: Tue, 08 Dec 2020 15:46:47 -0800

@martinthomson 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/4429#pullrequestreview-547727833

---

From: ianswett <[email-masked]>
Date: Tue, 08 Dec 2020 15:46:50 -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/4429#pullrequestreview-547727855

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 08 Dec 2020 15:48:06 -0800

Merged #4429 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/4429#event-4087267899