Subject: [quicwg/base-drafts] Persistent congestion period: text and pseudo-code don't agree (#3972)

From: Jana Iyengar <[email-masked]>
Date: Wed, 29 Jul 2020 09:15:39 -0700

We resolved #3875 with #3889, but I realized that it has a problem. The text and the pseudo-code don't agree.

The text says:
```
The persistent congestion period SHOULD NOT start until there is at
least one RTT sample.
```
Since the `persistent congestion period` starts with a packet that was sent in the past, this statement means that persistent congestion only includes packets that were _sent_ after an RTT sample was taken.

The pseudo code says:
```
   InPersistentCongestion(largest_lost):
     // Persistent congestion cannot be declared on the
     // first RTT sample.
     if (is first RTT sample):
       return false
```
This says that the persistent congestion can only be _established_ on the second (or later) ack that yields an RTT sample. This allows for packets that were sent before the first RTT sample was taken to be included in the persistent congestion period.

I had convinced myself that the pseudo-code was adequate, but I missed the fact that the text was not aligned.

-- 
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/issues/3972

---

From: Jana Iyengar <[email-masked]>
Date: Wed, 29 Jul 2020 09:18:20 -0700

I see two options.

Option 1: Implementing what the text says will require a new variable that records the next PN when the first RTT sample is taken.

Option 2: Changing the text to match the pseudo-code will look something like this:
```
Persistent congestion period SHOULD NOT be established until there is at least
one RTT sample.
```

-- 
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/issues/3972#issuecomment-665761529

---

From: Jana Iyengar <[email-masked]>
Date: Wed, 29 Jul 2020 09:33:00 -0700

An example might help:
1: Pkt 10, 11, 12 sent
2: PTO, Pkt 13, 14 sent
3: PTO, Pkt 15, 16 sent
4: Ack 10 received.
5: Ack 16 received.

At 5, the pseudo-code would declare persistent congestion if `(t3 - t1) > persistent congestion duration`.

The text however wouldn't, since there was no RTT sample during the persistent congestion period, t1 to t3.

-- 
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/issues/3972#issuecomment-665769626

---

From: Jana Iyengar <[email-masked]>
Date: Wed, 29 Jul 2020 09:48:35 -0700

Option 1 and 2 have a behavior difference with the following implication:

With option 2, a sender might mark persistent congestion with fewer PTOs than if the sender had a more accurate RTT estimate. This is arguably not a significant concern, since:
- the sender will still have sent at least two packets in that duration and marked them lost, since you need two ack-eliciting packets to be lost to declare persistent congestion; and
- the number of packets sent is also sensitive to how the application sends, and a sender cannot rely on that anyway.

Option 2 has the benefit that it is simpler to implement.

On balance, I think Option 2 is better, though it does make this a design change.

-- 
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/issues/3972#issuecomment-665777673

---

From: ianswett <[email-masked]>
Date: Wed, 29 Jul 2020 12:36:20 -0700

Thanks for the example, I was unclear on the issue up until then, though I think a slightly stronger example might be:
 5. Ack 14 received, since that's only a single PTO.

I think 1 aligns more with fully fixing the original issue, since it seems like your example is a case we agreed we would not want to declare as persistent congestion in #3875.  That being said, it makes the pseudocode more complex.

Persistent congestion is a new concept, so I suspect implementations will vary anyway.

-- 
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/issues/3972#issuecomment-665861076

---

From: Marten Seemann <[email-masked]>
Date: Wed, 29 Jul 2020 19:13:22 -0700

I agree that making the change to the pseudo-code seems to be the more correct solution.

That being said, I'm not planning to implement persistent congestion detection during the early phases of the handshake (but only after confirmation of the handshake), since I don't see clear benefits of the added complexity of cross-packet-number-space persistent congestion detection.

-- 
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/issues/3972#issuecomment-666040269

---

From: ianswett <[email-masked]>
Date: Thu, 30 Jul 2020 07:26:54 -0700

@marten-seemann I personally think that's fine, though I'll note that TCP is less aggressive than that, which is why I think we should describe this as something that covers the handshake as well.

-- 
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/issues/3972#issuecomment-666398571