Subject: [quicwg/base-drafts] Rework section on persistent congestion (#3961)

From: Jana Iyengar <[email-masked]>
Date: Mon, 27 Jul 2020 20:36:52 -0700

I started with trying to do small fixes, but quickly found that restructuring a bit and clarifying the details of what the sender was precisely supposed to do was the better than trying to be terse. 

This adds a couple of normatives, which may be unnecessary, and I&#39;m happy to take suggestions on those.

Closes #3939.
Closes #3941.
Closes #3947.
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * sync
  * Rework section on persistent congestion

-- File Changes --

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

-- Patch Links --

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

---

From: Martin Thomson <[email-masked]>
Date: Mon, 27 Jul 2020 20:49:39 -0700

@martinthomson commented on this pull request.

This is much clearer, thanks.

> -sent over a long enough period of time, the network is considered to be
-experiencing persistent congestion.  Commonly, this can be established by
-consecutive PTOs, but since the PTO timer is reset when a new ack-eliciting
-packet is sent, an explicit duration must be used to account for those cases
-where PTOs do not occur or are substantially delayed. The rationale for this
-threshold is to enable a sender to use initial PTOs for aggressive probing, as
-TCP does with Tail Loss Probe (TLP; see {{RACK}}), before establishing
-persistent congestion, as TCP does with a Retransmission Timeout (RTO; see
-{{?RFC5681}}). The RECOMMENDED value for kPersistentCongestionThreshold is 3,
-which is approximately equivalent to two TLPs before an RTO in TCP.
-
-This duration is computed as follows:
+When a sender establishes loss of all in-flight packets sent over a long enough
+duration, the network is considered to be experiencing persistent congestion.
+
+### Duration {#duration}

```suggestion
### Duration {#pc-duration}
```

>  max_ack_delay irrespective of the packet number spaces in which losses are
 established.
 
+This duration is intended to allow a sender to use initial PTOs for aggressive

What is an "initial PTO"?  I think that you want to rephrase this.

Is the intent not to give the sender an opportunity to send multiple times before declaring PC?  And that we want to allow for more than one probe packet to be sent so that transient loss does not cause PC unnecessarily.

> +This design uses an explicit duration instead of consecutive PTO events since
+the PTO timer is restarted every time an ack-eliciting packet is sent. An
+application that trickles data restarts the PTO timer repeatedly, preventing the
+PTO timer from expiring for a potentially long period of time. A consequence of
+this design is that persistent congestion can be established without the
+occurrence of any PTOs.
+
+### Declaring Persistent Congestion
+
+A sender declares persistent congestion on receiving an acknowledgement, if the
+following conditions are true:
+
+* there are at least two ack-eliciting packets that are declared lost;
+
+* the duration between the send times of these two packets exceeds the
+  persistent congestion duration ({{duration}}); and

```suggestion
  persistent congestion duration ({{pc-duration}}); and
```

> +* the duration between the send times of these two packets exceeds the
+  persistent congestion duration ({{duration}}); and
+
+* all packets sent between those times are declared lost.
+
+Since network congestion is not affected by packet number spaces, persistent
+congestion SHOULD be established across packet number spaces. A sender that does
+not have state for all packet number spaces or an implementation that cannot
+compare send times across packet number spaces MAY use state for just the packet
+number space that was acknowledged.
+
+When persistent congestion is declared, the sender's congestion window MUST be
+reduced to the minimum congestion window (kMinimumWindow).  This response of
+collapsing the congestion window on persistent congestion is functionally
+similar to a sender's response on a Retransmission Timeout (RTO) in TCP
+({{RFC5681}}) after Tail Loss Probes (TLP; see {{RACK}}).

The repetition of the RTO and TLP citations seems a little duplicative here.

```suggestion
({{RFC5681}}) after Tail Loss Probes ({{RACK}}).
```

-- 
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/3961#pullrequestreview-456280522

---

From: Kazuho Oku <[email-masked]>
Date: Mon, 27 Jul 2020 21:46:16 -0700

@kazuho commented on this pull request.

Thank you for working on this. I think it is much better.

Q. Do you plan to have a separate PR for requiring RTT to be established on the far edge of the persistent congestion period?

```suggestion
* the duration between the send times of these packets exceeds the
```

"two" was a bit confusing for me, because the previous sentence talks about "at least two."

-- 
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/3961#pullrequestreview-456297996

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 28 Jul 2020 11:02:29 -0700

@janaiyengar pushed 1 commit.

f9ef6139ade822ea14f01cbaa4ab415b1dfdbdfd  Apply suggestions from code review

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3961/files/627bdd717e574dad4d4890735c93450edb019ffb..f9ef6139ade822ea14f01cbaa4ab415b1dfdbdfd

---

From: ianswett <[email-masked]>
Date: Tue, 28 Jul 2020 11:28:25 -0700

@ianswett approved this pull request.

> +This duration is intended to allow a sender to use initial PTOs for aggressive
+probing, as TCP does with Tail Loss Probe (TLP; see {{RACK}}), before

```suggestion
This duration is intended to allow a sender to use one or more PTOs for
probing, as TCP does with Tail Loss Probe (TLP; see {{RACK}}), before
```

 established.
 
+This duration is intended to allow a sender to use initial PTOs for aggressive
+probing, as TCP does with Tail Loss Probe (TLP; see {{RACK}}), before
+establishing persistent congestion, as TCP does with a Retransmission Timeout
+(RTO; see {{?RFC5681}}).
+
+The RECOMMENDED value for kPersistentCongestionThreshold is 3, which is
+approximately equivalent to two TLPs before an RTO in TCP.
+
+This design uses an explicit duration instead of consecutive PTO events since
+the PTO timer is restarted every time an ack-eliciting packet is sent. An
+application that trickles data restarts the PTO timer repeatedly, preventing the

nit: Is everyone going to understand trickle in this context?

> +The RECOMMENDED value for kPersistentCongestionThreshold is 3, which is
+approximately equivalent to two TLPs before an RTO in TCP.
+

How could this not be true given other requirements?
Also should this be kPersisentCongestion - 1?

-- 
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/3961#pullrequestreview-456878848

---

From: mjoras <[email-masked]>
Date: Tue, 28 Jul 2020 17:59:21 -0700

@mjoras commented on this pull request.

 established.

Yeah I think this paragraph is a little confusing. I think the intent is basically suggesting that an application which itself is steadily writing relatively small amounts of data into the transport will perpetuate the PTO timer, and hence detecting persistent congestion should not depend on this. However the use of "application" and "trickle" together is probably going to be confusing to some.

Additionally it seems like the first sentence and the last sort of overlap, which makes the paragraph read a little strangely.

-- 
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/3961#discussion_r461976858

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 28 Jul 2020 18:21:38 -0700

@janaiyengar commented on this pull request.

This could be any number of packets, but the two are the edges -- the earliest and the latest that we consider for the interval.

-- 
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/3961#discussion_r461982744

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 28 Jul 2020 18:22:49 -0700

@janaiyengar pushed 2 commits.

322c5ad18f6b16525d1303893d0c39c65fe1ac8a  martin and ian comments
ebbeadbc6d9c2883a267db2f7e97da986f0a556e  Merge branch 'jri/persistent' of https://github.com/quicwg/base-drafts into jri/persistent

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3961/files/f9ef6139ade822ea14f01cbaa4ab415b1dfdbdfd..ebbeadbc6d9c2883a267db2f7e97da986f0a556e

---

From: Kazuho Oku <[email-masked]>
Date: Tue, 28 Jul 2020 18:34:56 -0700

+approximately equivalent to two TLPs before an RTO in TCP.
+
+This design uses an explicit duration instead of consecutive PTO events since
+the PTO timer is restarted every time an ack-eliciting packet is sent. An
+application that sends data with silence periods can restart the PTO timer every
+time it sends, potentially preventing the PTO timer from expiring for a long
+period of time. A consequence of this design is that persistent congestion can
+be established without the occurrence of any PTOs.
+
+### Declaring Persistent Congestion
+
+A sender declares persistent congestion on receiving an acknowledgement, if the
+following conditions are true:
+
+* there are at least two ack-eliciting packets that are declared lost;

```suggestion
* there are at least two ack-eliciting packets that are declared lost, after at
  least one RTT sample was obtained;
```

-- 
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/3961#pullrequestreview-457136241

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 28 Jul 2020 18:37:13 -0700

328bb01d5678dfdc187eda7b070c7217262d2aed  mjoras comment

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3961/files/ebbeadbc6d9c2883a267db2f7e97da986f0a556e..328bb01d5678dfdc187eda7b070c7217262d2aed

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 28 Jul 2020 18:37:34 -0700

established.

I've reworded the para, 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/3961#discussion_r461987064

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 28 Jul 2020 18:39:06 -0700

We could make it a separate condition

-- 
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/3961#discussion_r461987465

---

From: Kazuho Oku <[email-masked]>
Date: Tue, 28 Jul 2020 18:47:05 -0700

That works. I use the form of review comment just to make sure that this requirement would be incorporated.

-- 
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/3961#discussion_r461989582

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 28 Jul 2020 19:07:05 -0700

2891f492f453906c695cf8ae8620933f9cc2b917  kazuho comment

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3961/files/328bb01d5678dfdc187eda7b070c7217262d2aed..2891f492f453906c695cf8ae8620933f9cc2b917

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 28 Jul 2020 19:07:14 -0700

added, and also added a rationale

-- 
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/3961#discussion_r461995031

---

From: Kazuho Oku <[email-masked]>
Date: Tue, 28 Jul 2020 19:13:17 -0700

@kazuho 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/3961#pullrequestreview-457147762

---

From: Marten Seemann <[email-masked]>
Date: Tue, 28 Jul 2020 19:55:59 -0700

@marten-seemann commented on this pull request.

+This design does not use consecutive PTO events to establish persistent
+congestion, since a PTO expiration is controlled by application patterns in
+addition to network activity. For example, a sender that sends small amounts of
+data with silence periods between them restarts the PTO timer every time it
+sends, potentially preventing the PTO timer from expiring for a long period of
+time, even when no acknowledgments are being received. The use of a duration
+enables a sender to establish persistent congestion without depending on the
+occurrence of PTOs.
+
+### Establishing Persistent Congestion
+
+A sender establishes persistent congestion on receiving an acknowledgement, if
+the following conditions are true:
+
+* a prior RTT sample exists;

More specific: a RTT sample existed at the time the first packet of the range was sent

-- 
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/3961#pullrequestreview-457159767

---

From: Jana Iyengar <[email-masked]>
Date: Wed, 29 Jul 2020 06:54:19 -0700

This reflects the resolution in #3889. I think the resolution there is adequate. Doing this as an RTT sample at the time the first packet in the range is sent, requires more state, IMO.

-- 
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/3961#discussion_r462316237

---

From: Marten Seemann <[email-masked]>
Date: Wed, 29 Jul 2020 07:12:35 -0700

#3889 says exactly that:
> The persistent congestion period SHOULD NOT start until there is at least one RTT sample.

... which means that you need to have had a RTT sample when you sent the first packet, right?

-- 
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/3961#discussion_r462330089

---

From: Jana Iyengar <[email-masked]>
Date: Wed, 29 Jul 2020 08:45:10 -0700

Gah! #3889 was a bit problematic. Moving this discussion to the 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/3961#discussion_r462400004

---

From: Jana Iyengar <[email-masked]>
Date: Wed, 29 Jul 2020 19:26:36 +0000 (UTC)

You're right -- but this was not resolved correctly. I've changed the language now to match what's in the draft as you proposed, and I've filed #3972.

-- 
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/3961#discussion_r462534577

---

From: Jana Iyengar <[email-masked]>
Date: Wed, 29 Jul 2020 12:28:01 -0700

b34b22abc2845e96bd0d84f68641360bfa664d09  editorial

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3961/files/f6d8ed87035dc79d8befabfe6a1e4c6dd4a3d238..b34b22abc2845e96bd0d84f68641360bfa664d09

---

From: ianswett <[email-masked]>
Date: Thu, 30 Jul 2020 13:18:52 -0700

@ianswett commented on this pull request.

Getting close, a few more suggestions

 ~~~
 (smoothed_rtt + max(4*rttvar, kGranularity) + max_ack_delay) *
     kPersistentCongestionThreshold
 ~~~
 
-Unlike the PTO computation in {{pto}}, persistent congestion includes the
-max_ack_delay irrespective of the packet number spaces in which losses are
-established.
+Unlike the PTO computation in {{pto}}, this duration includes the max_ack_delay
+irrespective of the packet number spaces in which losses are established.
+
+This duration allows a sender to send enough packets, including some in response

```suggestion
This duration allows a sender to send as many packets, including some in response
```

> -Unlike the PTO computation in {{pto}}, persistent congestion includes the
-max_ack_delay irrespective of the packet number spaces in which losses are
-established.
+Unlike the PTO computation in {{pto}}, this duration includes the max_ack_delay
+irrespective of the packet number spaces in which losses are established.
+
+This duration allows a sender to send enough packets, including some in response
+to PTO expiration, as TCP does with Tail Loss Probe ({{RACK}}), before
+establishing persistent congestion, as TCP does with a Retransmission Timeout
+({{?RFC5681}}).
+
+The RECOMMENDED value for kPersistentCongestionThreshold is 3, which is
+approximately equivalent to two TLPs before an RTO in TCP.
+
+This design does not use consecutive PTO events to establish persistent
+congestion, since a PTO expiration is controlled by application patterns in

```suggestion
congestion, since a PTO expiration is influenced by application patterns in
```

> -Unlike the PTO computation in {{pto}}, persistent congestion includes the
-max_ack_delay irrespective of the packet number spaces in which losses are
-established.
+Unlike the PTO computation in {{pto}}, this duration includes the max_ack_delay
+irrespective of the packet number spaces in which losses are established.
+
+This duration allows a sender to send enough packets, including some in response
+to PTO expiration, as TCP does with Tail Loss Probe ({{RACK}}), before
+establishing persistent congestion, as TCP does with a Retransmission Timeout
+({{?RFC5681}}).
+
+The RECOMMENDED value for kPersistentCongestionThreshold is 3, which is
+approximately equivalent to two TLPs before an RTO in TCP.
+
+This design does not use consecutive PTO events to establish persistent
+congestion, since a PTO expiration is controlled by application patterns in

Or maybe something else.  Controlled seems odd to me.

+sends, potentially preventing the PTO timer from expiring for a long period of
+time, even when no acknowledgments are being received. The use of a duration
+enables a sender to establish persistent congestion without depending on the
+occurrence of PTOs.
+
+### Establishing Persistent Congestion
+
+A sender establishes persistent congestion on receiving an acknowledgement, if
+the following conditions are true:
+
+* there are at least two ack-eliciting packets that are declared lost;
+
+* a prior RTT sample existed at the time these packets were sent;
+
+* the duration between the send times of these packets exceeds the

```suggestion
* the duration between the send times of the first and last lost packets exceeds the
```

+collapsing the congestion window on persistent congestion is functionally
+similar to a sender's response on a Retransmission Timeout (RTO) in TCP
+({{RFC5681}}) after Tail Loss Probes ({{RACK}}).

This sentence repeats lines 844 to 847, so I'd be inclined to remove it.

```suggestion
reduced to the minimum congestion window (kMinimumWindow).
```

-- 
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/3961#pullrequestreview-458719452

---

From: Jana Iyengar <[email-masked]>
Date: Fri, 31 Jul 2020 19:20:15 -0700

150ffe68462b8a694ce9326e0f4514a4c1b690ce  ian comments

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3961/files/b34b22abc2845e96bd0d84f68641360bfa664d09..150ffe68462b8a694ce9326e0f4514a4c1b690ce

---

From: Jana Iyengar <[email-masked]>
Date: Fri, 31 Jul 2020 19:22:52 -0700

Thanks, Ian, I've incorporated most of your comments.

This isn't correct. @marten-seemann had an example where:
- 1 - 10 are sent
- on ack, 1-4 and 6-9 are marked lost
- 1-4 have a period that exceeds the PC threshold. 

The text here allows for this to result in persistent congestion. I've changed the text here to be clearer.

Trimmed and rephrased.

-- 
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/3961#pullrequestreview-459542454