Subject: [quicwg/base-drafts] Add some MUSTs (#3978)

From: ianswett <[email-masked]>
Date: Sat, 01 Aug 2020 19:17:44 -0700

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

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

-- Commit Summary --

  * Add some MUSTs

-- File Changes --

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

-- Patch Links --

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

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 04 Aug 2020 15:47:01 -0700

@janaiyengar approved this pull request.

one nit, but LGTM

> +Congestion avoidance uses an Additive Increase Multiplicative Decrease (AIMD)
+approach that MUST increase the congestion window by one maximum packet size
+per congestion window acknowledged.  When a loss or ECN-CE marking is

```suggestion
Congestion avoidance uses an Additive Increase Multiplicative Decrease (AIMD)
approach and MUST increase the congestion window by one maximum packet size
per congestion window acknowledged.  When a loss or ECN-CE marking is
```

-- 
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/3978#pullrequestreview-461227149

---

From: Martin Thomson <[email-masked]>
Date: Tue, 04 Aug 2020 18:12:45 -0700

@martinthomson commented on this pull request.

This is a weird MUST to add.  It just does this.

-- 
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/3978#discussion_r465414676

---

From: ianswett <[email-masked]>
Date: Tue, 04 Aug 2020 19:20:14 -0700

@ianswett commented on this pull request.

Agreed, we can just drop this part of the 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/pull/3978#discussion_r465432895

---

From: Jana Iyengar <[email-masked]>
Date: Wed, 05 Aug 2020 22:26:58 -0700

@janaiyengar commented on this pull request.

What are we dropping? My suggestion or the MUST increase entirely? Dropping my suggestion is fine, but I would keep the MUST increase -- that's just the definition of Reno.

-- 
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/3978#discussion_r466153969

---

From: Martin Thomson <[email-masked]>
Date: Wed, 05 Aug 2020 23:26:04 -0700

The removal of the first sentence is fine.  (Though we need a definition of congestion avoidance, this isn't it.)

The original text here is fine without a MUST: "Congestion avoidance uses an Additive Increase Multiplicative Decrease (AIMD) approach that increases the congestion window by one maximum packet size per congestion window".  You don't need normative language, because it is definitional.  If you use "MUST" you do so by identifying an actor and an action.  This is saying what "congestion avoidance" is.

-- 
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/3978#discussion_r466172580

---

From: Jana Iyengar <[email-masked]>
Date: Wed, 05 Aug 2020 23:45:12 -0700

What I meant to say is that this is the definition of it. Congestion avoidance is simply the state of the controller where `cwnd > ssthresh`, this does not imply that the controller needs to increase by 1 packet size per round trip time. Reno isn't defined anywhere formally as a term, but it is an AIMD controller that "increases the congestion window by 1 packet size per round-trip time" and "decreases it by 0.5 when loss is encountered." My point was that to make our controller Reno, these two behaviors have to be MUSTs. Everything else is window dressing.

-- 
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/3978#discussion_r466179959

---

From: Jana Iyengar <[email-masked]>
Date: Wed, 05 Aug 2020 23:48:50 -0700

Now that I look at RFC 5681 a bit more carefully, we probably want to use language similar to what's used there:
```
   During congestion avoidance, cwnd is incremented by roughly 1 full-
   sized segment per round-trip time (RTT).  Congestion avoidance
   continues until congestion is detected.  The basic guidelines for
   incrementing cwnd during congestion avoidance are:

      * MAY increment cwnd by SMSS bytes

      * SHOULD increment cwnd per equation (2) once per RTT

      * MUST NOT increment cwnd by more than SMSS bytes
```

-- 
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/3978#discussion_r466181466

---

From: Jana Iyengar <[email-masked]>
Date: Wed, 05 Aug 2020 23:50:41 -0700

The important bit there is the MUST NOT. I don't think the MAY is useful, but the second and third statements are probably adequate.

-- 
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/3978#discussion_r466182249

---

From: Martin Thomson <[email-masked]>
Date: Thu, 06 Aug 2020 00:29:40 -0700

Yes, a MUST NOT would be useful.

-- 
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/3978#discussion_r466200995

---

From: mirjak <[email-masked]>
Date: Thu, 06 Aug 2020 03:46:18 -0700

@mirjak commented on this pull request.

I would prefer to refer to RFC5681 rather than respecifying these normative requirement here.

-- 
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/3978#discussion_r466326438

---

From: ianswett <[email-masked]>
Date: Mon, 10 Aug 2020 18:14:10 -0700

@ianswett pushed 1 commit.

02981cd8125623874c761b2a2689d2154ea96511  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/3978/files/8d117c6b3b5885b3c922be19a2cb8a9a6ff105bb..02981cd8125623874c761b2a2689d2154ea96511

---

From: ianswett <[email-masked]>
Date: Mon, 10 Aug 2020 18:19:09 -0700

4cd67d63b508b73a6625e5383e44228c8051a267  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/3978/files/02981cd8125623874c761b2a2689d2154ea96511..4cd67d63b508b73a6625e5383e44228c8051a267

---

From: ianswett <[email-masked]>
Date: Thu, 13 Aug 2020 14:26:18 -0700

7c8598786dd7a907818264a3a6b6837ce13f40d7  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/3978/files/4cd67d63b508b73a6625e5383e44228c8051a267..7c8598786dd7a907818264a3a6b6837ce13f40d7

---

From: ianswett <[email-masked]>
Date: Thu, 13 Aug 2020 14:32:25 -0700

6c0ad3c38ca6106f6b54d1de08fffa30d6d45b5a  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/3978/files/7c8598786dd7a907818264a3a6b6837ce13f40d7..6c0ad3c38ca6106f6b54d1de08fffa30d6d45b5a

---

From: Martin Thomson <[email-masked]>
Date: Mon, 17 Aug 2020 18:27:18 -0700

## Congestion Avoidance
 
-Slow start exits to congestion avoidance.  Congestion avoidance uses an
-Additive Increase Multiplicative Decrease (AIMD) approach that increases
-the congestion window by one maximum packet size per congestion window
-acknowledged.  When a loss or ECN-CE marking is detected, NewReno halves
-the congestion window, sets the slow start threshold to the new
-congestion window, and then enters the recovery period.
+Congestion avoidance uses an Additive Increase Multiplicative Decrease (AIMD)
+approach that typically increases the congestion window by one maximum packet

It's maximum datagram size, is it not?

-- 
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/3978#pullrequestreview-468930987

---

From: ianswett <[email-masked]>
Date: Tue, 18 Aug 2020 07:43:54 -0700

## Congestion Avoidance

Yup, fixed.

-- 
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/3978#discussion_r472251398

---

From: ianswett <[email-masked]>
Date: Tue, 18 Aug 2020 07:44:14 -0700

350846db14974fec5cb25810496aca4590df48ef  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/3978/files/6c0ad3c38ca6106f6b54d1de08fffa30d6d45b5a..350846db14974fec5cb25810496aca4590df48ef

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 18 Aug 2020 14:14:57 -0700

@mirjak -- this document does not make normative references to RFC 5681.

-- 
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/3978#discussion_r472497053

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 18 Aug 2020 14:17:37 -0700

## Congestion Avoidance
 
-Slow start exits to congestion avoidance.  Congestion avoidance uses an
-Additive Increase Multiplicative Decrease (AIMD) approach that increases
-the congestion window by one maximum packet size per congestion window
-acknowledged.  When a loss or ECN-CE marking is detected, NewReno halves
-the congestion window, sets the slow start threshold to the new
-congestion window, and then enters the recovery period.
+Congestion avoidance uses an Additive Increase Multiplicative Decrease (AIMD)
+approach that typically increases the congestion window by one maximum datagram
+size per congestion window acknowledged, and MUST NOT increase the congestion
+window faster.  When a loss or ECN-CE marking is detected, the sender MUST
+reduce the cwnd. NewReno halves the congestion window, sets the slow start

```suggestion
reduce the congestion window. NewReno halves the congestion window, sets the slow start
```

 ## Congestion Avoidance
 
-Slow start exits to congestion avoidance.  Congestion avoidance uses an
-Additive Increase Multiplicative Decrease (AIMD) approach that increases
-the congestion window by one maximum packet size per congestion window
-acknowledged.  When a loss or ECN-CE marking is detected, NewReno halves
-the congestion window, sets the slow start threshold to the new
-congestion window, and then enters the recovery period.
+Congestion avoidance uses an Additive Increase Multiplicative Decrease (AIMD)
+approach that typically increases the congestion window by one maximum datagram
+size per congestion window acknowledged, and MUST NOT increase the congestion
+window faster.  When a loss or ECN-CE marking is detected, the sender MUST
+reduce the cwnd. NewReno halves the congestion window, sets the slow start
+threshold to the new congestion window, and then enters the recovery period.
+[RFC8511] specifies an alternate cwnd reduction.

```suggestion
```

Why do we need to specify this? We're just specifying one algorithm and the implementation can choose to implement something else. The additional refs in the ECN text below seems adequate.

-- 
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/3978#pullrequestreview-469836202

---

From: ianswett <[email-masked]>
Date: Tue, 18 Aug 2020 15:22:45 -0700

3ff276ce608da8a9b1325d8a76397ae3cbe75e11  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/3978/files/350846db14974fec5cb25810496aca4590df48ef..3ff276ce608da8a9b1325d8a76397ae3cbe75e11

---

From: ianswett <[email-masked]>
Date: Tue, 18 Aug 2020 15:23:39 -0700

97b471268553d36987f495f540457820d8b39d38  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/3978/files/3ff276ce608da8a9b1325d8a76397ae3cbe75e11..97b471268553d36987f495f540457820d8b39d38

---

From: ianswett <[email-masked]>
Date: Tue, 18 Aug 2020 15:24:28 -0700

1548c57ebf2945eabb485a3e33fb0359fff5e730  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/3978/files/97b471268553d36987f495f540457820d8b39d38..1548c57ebf2945eabb485a3e33fb0359fff5e730

---

From: mirjak <[email-masked]>
Date: Thu, 20 Aug 2020 04:03:25 -0700

Why? The reason why we stick to Reno is because that's the only RFC we have. Rather than re-specifying everything here we should reply on what is specified there, so people understand what's congestion control specific (and could be changed by other congestion controllers) and what's a default requirement to implement 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/3978#discussion_r473886997

---

From: mirjak <[email-masked]>
Date: Thu, 20 Aug 2020 04:07:35 -0700

> @@ -996,11 +998,12 @@ Reporting additional ECN-CE markings will cause a sender to reduce their sending
 rate, which is similar in effect to advertising reduced connection flow control
 limits and so no advantage is gained by doing so.
 
-Endpoints choose the congestion controller that they use.  Though congestion
-controllers generally treat reports of ECN-CE markings as equivalent to loss
-({{?RFC8311}}), the exact response for each controller could be different.
-Failure to correctly respond to information about ECN markings is therefore
-difficult to detect.
+Endpoints choose the congestion controller that they use. Congestion controllers
+respond to reports of ECN-CE by reducing their rate, but the response may vary.
+Markings can be treated as equivalent to loss ({{?RFC3168}}), but other
+responses can be specified, such as ({{?RFC8511}}) or ({{?RFC8311}}). Failure to
+correctly respond to information about ECN markings is therefore difficult to
+detect.

I actually don't really understand the intention of the last sentence. This is also true for lost base congestion control or even controller which are not based on loss at all.

 ## Congestion Avoidance

This MUST NOT kind of already says that QUICv1 cannot implement Cubic. Is that what we want?

> @@ -767,20 +767,22 @@ While in slow start, QUIC increases the congestion window by the
 number of bytes acknowledged when each acknowledgment is processed, resulting
 in exponential growth of the congestion window.
 
-QUIC exits slow start upon loss or upon increase in the ECN-CE counter.
-When slow start is exited, the congestion window halves and the slow start
-threshold is set to the new congestion window.  QUIC re-enters slow start
-any time the congestion window is less than the slow start threshold,
-which only occurs after persistent congestion is declared.
+QUIC MUST exit slow start and enter congestion avoidance upon loss or upon
+increase in the ECN-CE counter. When slow start is exited, the congestion
+window halves and the slow start threshold is set to the new congestion

Restating this here: With https://tools.ietf.org/html/rfc5681#section-4.3 ssthresh is reduced upon entering recover but cwnd is set to ssthresh when leaving recovery.

-- 
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/3978#pullrequestreview-471487896

---

From: Jana Iyengar <[email-masked]>
Date: Thu, 20 Aug 2020 17:46:00 -0700

That's not the reason we stick with Reno -- we have RFC 8312 (Cubic) as well. Reno is simple, which is why we went with it. To be clear, this is _not_ a default requirement for QUIC. It's stated that some congestion controller is necessary, and we specify one, but implementations can use others ([Section 7](https://quicwg.org/base-drafts/draft-ietf-quic-recovery.html#section-7-2).)

-- 
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/3978#discussion_r474347005

---

From: Jana Iyengar <[email-masked]>
Date: Thu, 20 Aug 2020 17:54:56 -0700

That's what TCP does, but that's not necessary in QUIC. Consider what happens in TCP: until the sender leaves recovery, it cannot grow the cwnd, and since the left edge of the cwnd is anchored to the cumulative ack point, the sender cannot send anything new. It can only keep retransmitting. In other words, the cwnd governs how much data to send and also _which data to send_. To allow for a sender to send new data, TCP allows inflating the cwnd which is deflated on leaving recovery, which in the best case is 1 RTT after loss.

QUIC deliberately avoids this problem, and the cwnd is not anchored to a left edge. It's an abstract limit on the bytes in flight, and says nothing about which data can be sent. So, as data gets acked, and as the bytes in flight reduces to 1/2 the value at loss, the sender can start sending again. This would happen 1/2 RTT after loss.

So at the end of the "recovery period", i.e., 1 RTT after loss:
- TCP would have sent nothing if its cwnd were reduced immediately on loss. 
- QUIC would have been sending for 1/2 RTT (it would send as much as is allowed by the new cwnd basically).

This is also why simply referencing 5681 won't do. The semantics of cwnd are different.

-- 
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/3978#discussion_r474349285

---

From: Jana Iyengar <[email-masked]>
Date: Thu, 20 Aug 2020 17:57:02 -0700

## Congestion Avoidance

It does not, since this document allows an endpoint to implement a different controller than the one specified here. See [Section 7](https://quicwg.org/base-drafts/draft-ietf-quic-recovery.html#section-7-3):
```
If an endpoint uses a different controller than that specified in this document, the chosen controller MUST conform to the congestion control guidelines specified in Section 3.1 of [RFC8085].
```

-- 
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/3978#discussion_r474349803

---

From: Mike Bishop <[email-masked]>
Date: Thu, 20 Aug 2020 18:36:26 -0700

@MikeBishop commented on this pull request.

We're chartered to use a *Standard* congestion controller, and RFC 8312 is Informational.  But as you say, people are perfectly able to plug in other congestion controllers instead; this is merely "a default scheme," in the language of the charter.

-- 
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/3978#discussion_r474359701

---

From: mirjak <[email-masked]>
Date: Fri, 21 Aug 2020 05:56:19 -0700

I still don't understand what the problem of referring RFC5681 normatively is?

-- 
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/3978#discussion_r474678688

---

From: mirjak <[email-masked]>
Date: Fri, 21 Aug 2020 06:02:19 -0700

Also for TCP it was always possible to send new data because of the packet conservation principle as when you receive an ACK that doesn't ack new data you still know that at least one packet was received and you can reduce your bytes_in_flight by one packet. The thing that TCP does by reducing the cwnd gradually over the recovery RTT is that you don't have the effect you describe above where your don't send anything for half an RTT and then keep sending with exactly the same rate as previously for the second half on the RTT and overload the network again. This is a feature that should be conserved with QUIC 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/pull/3978#discussion_r474681893

---

From: mirjak <[email-masked]>
Date: Fri, 21 Aug 2020 06:05:43 -0700

## Congestion Avoidance

Then I don't see why this need to be a normative requirement.

-- 
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/3978#discussion_r474683521

---

From: mirjak <[email-masked]>
Date: Fri, 21 Aug 2020 06:08:55 -0700

## Congestion Avoidance

Similar as we don't require normatively in the next part that the congestion needs be halved on loss/CE mark.

-- 
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/3978#discussion_r474685190

---

From: ianswett <[email-masked]>
Date: Fri, 21 Aug 2020 06:58:23 -0700

Early in the QUIC process, the preference of the working group was to have the recovery document be complete enough that one can implement from it, and to only reference TCP RFCs informationally.  I think that was the right decision both because there are some differences between QUIC and TCP and because not all the text in TCP RFCs is relevant here.  As such, if there are important requirements, they should be specified in this document.

-- 
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/3978#discussion_r474715025

---

From: ianswett <[email-masked]>
Date: Fri, 21 Aug 2020 07:01:24 -0700

This text is pre-existing, but I agree it doesn't say anything important and probably could be removed.

-- 
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/3978#discussion_r474716875

---

From: ianswett <[email-masked]>
Date: Fri, 21 Aug 2020 07:30:50 -0700

## Congestion Avoidance

I don't have a strong opinion on this, but I see @mirjak point.  If we're going to keep this, I think we should be clear that this MUST NOT only applies to NewReno.

-- 
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/3978#discussion_r474735483

---

From: ianswett <[email-masked]>
Date: Fri, 21 Aug 2020 08:18:20 -0700

16d6dd8f9085f638a88b0be0de122a49eeb71a7f  Clarify NewReno

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3978/files/1548c57ebf2945eabb485a3e33fb0359fff5e730..16d6dd8f9085f638a88b0be0de122a49eeb71a7f

---

From: Praveen Balasubramanian <[email-masked]>
Date: Fri, 21 Aug 2020 09:54:34 -0700

@pravb commented on this pull request.

Yes I think we should remove it because it almost sounds like an incentive to deviate which we don't want to encourage.

-- 
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/3978#discussion_r474814955

---

From: ianswett <[email-masked]>
Date: Fri, 21 Aug 2020 10:10:15 -0700

## Congestion Avoidance

Updated to start the sentence with NewReno to make it a bit clearer.

-- 
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/3978#discussion_r474822478

---

From: ianswett <[email-masked]>
Date: Fri, 21 Aug 2020 10:10:55 -0700

709d8e83339d82ffb7d097e60989a508ff9cb70d  Remove a sentence

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3978/files/16d6dd8f9085f638a88b0be0de122a49eeb71a7f..709d8e83339d82ffb7d097e60989a508ff9cb70d

---

From: ianswett <[email-masked]>
Date: Fri, 21 Aug 2020 10:11:15 -0700

SG, sentence removed.

-- 
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/3978#discussion_r474822984

---

From: ianswett <[email-masked]>
Date: Fri, 21 Aug 2020 10:27:09 -0700

84729dc2a48299facfe2c7eef47d5db015fc7aca  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/3978/files/709d8e83339d82ffb7d097e60989a508ff9cb70d..84729dc2a48299facfe2c7eef47d5db015fc7aca

---

From: mirjak <[email-masked]>
Date: Mon, 24 Aug 2020 04:55:57 -0700

Thanks for trying to make that clearer. However, the normative language for reduction is most less strict (MUST reduce) and not specifying a concrete algorithm. I think that would be also the better approach for the increase behaviour. However, it's probably less clear what this minimum requirement is. So I would still prefer to just not use normative language here and just say NewReno does increase by  one maximum packet size per congestion window acknowledged.

-- 
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/3978#discussion_r475546164

---

From: Jana Iyengar <[email-masked]>
Date: Mon, 24 Aug 2020 18:16:27 -0700

Let me start by saying that QUIC doesn't have to be precisely the same as TCP -- this is why we are not normatively referencing RFC 5681. Even if this is a behavior difference from deployed TCP, this is not a reduction in quality. What is the change you're proposing?

That said, what you say isn't true in TCP. Specifically, RFC 5681 says (in Section 4.3):
```
   That is, when the first loss in a window of data is detected,
   ssthresh MUST be set to no more than the value given by equation (4).
   Second, until all lost segments in the window of data in question are
   repaired, the number of segments transmitted in each RTT MUST be no
   more than half the number of outstanding segments when the loss was
   detected.
```
That last sentence is critical. With just RFC 5681, TCP will do exactly as QUIC does. This isn't true of Linux TCP however, which uses rate halving... the PRR document (RFC 6937, Section 1) goes through this is some detail, and is in fact one of the reasons for PRR's existence.

-- 
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/3978#discussion_r476030650

---

From: mirjak <[email-masked]>
Date: Tue, 25 Aug 2020 02:23:54 -0700

This text from RFC5681 is fine and if you want we can take over that language. That text does not say that you need to immediately need to halve the cwnd but you can't transmit more than half the cwnd _over the whole RTT_. That's also what rate halving does actually but by reducing gradually over one RTT you avoid not sending anything for half an RTT and then send full rate in the second half (what the originally fast recovery did as nicely explain in section 1 of RFC6937). PRR addresses a different problem of rate halving which is that you actually might reduce too much (e.g. for cubic when you only want to reduce by 30%, or when multiple packets are lost). PRR still uses rate halving but then goes back into SS to make sure you reach your ssthresh quickly in case you have reduced too much. This is also what is specified for QUIC at the moment.

-- 
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/3978#discussion_r476307745

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 25 Aug 2020 17:05:16 -0700

@janaiyengar pushed 1 commit.

71bc8a5e932ebb1cb0183238af652b095612f78e  fix duplication and make allowance for not reducing cwnd immediately

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3978/files/84729dc2a48299facfe2c7eef47d5db015fc7aca..71bc8a5e932ebb1cb0183238af652b095612f78e

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 25 Aug 2020 17:09:01 -0700

81c77271865a174467094e5625e8195620f799b2  consistently newreno

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3978/files/71bc8a5e932ebb1cb0183238af652b095612f78e..81c77271865a174467094e5625e8195620f799b2

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 25 Aug 2020 17:09:36 -0700

I've changed the text here to reflect this 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/3978#discussion_r476898563

---

From: mirjak <[email-masked]>
Date: Wed, 26 Aug 2020 01:39:27 -0700

> -during the recovery period is acknowledged.  This is slightly different from
-TCP's definition of recovery, which ends when the lost packet that started
-recovery is acknowledged.
+A NewReno sender enters the recovery period when it detects loss or an ECN-CE
+mark is received. A recovery period ends when a packet sent during the recovery
+period is acknowledged.
+
+A sender that is already in a recovery period stays in it and does not re-enter
+it.
+
+On entering recovery, a sender MUST do the following:
+
+* set the slow start threshold to half the value of the congestion window at the
+  moment that loss is detected, and
+
+* set the congestion window to the new value of the slow start

This still say that you have to set the cwnd to ssthresh when entering recovery. Saying that you have to do this when entering recovery and then saying that you may do it it later seems confusing.

-- 
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/3978#pullrequestreview-475274428

---

From: Jana Iyengar <[email-masked]>
Date: Wed, 26 Aug 2020 12:26:00 -0700

04f523dccd5f87a88847dcf4034b96283b0bac6f  cwnd reduction not on entering recovery

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3978/files/81c77271865a174467094e5625e8195620f799b2..04f523dccd5f87a88847dcf4034b96283b0bac6f

---

From: Jana Iyengar <[email-masked]>
Date: Wed, 26 Aug 2020 12:26:43 -0700

Good point - fixed

-- 
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/3978#discussion_r477537305

---

From: mirjak <[email-masked]>
Date: Thu, 27 Aug 2020 02:00:27 -0700

Looks good now! Thanks!

-- 
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/3978#discussion_r478265873

---

From: ianswett <[email-masked]>
Date: Sat, 29 Aug 2020 06:22:59 -0700

Thanks for updating this while I was gone, looks like a good resolution.

 The recovery period aims to limit congestion window reduction to once per round
 trip. Therefore during recovery, the congestion window remains unchanged
 irrespective of new losses or increases in the ECN-CE counter.
 
-When entering recovery, a single packet MAY be sent even if bytes in flight
-now exceeds the recently reduced congestion window.  This speeds up loss
+When entering a recovery period, a single packet MAY be sent even if bytes in

I find this paragraph a bit odd given the adjusted text about when to reduce CWND, but I guess it's fine.

-- 
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/3978#pullrequestreview-478127709

---

From: ianswett <[email-masked]>
Date: Sat, 29 Aug 2020 09:32:36 -0700

942f94e70a3ddf8c72a7d2f353ef15cd637a5213  Unify the recovery period text

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3978/files/04f523dccd5f87a88847dcf4034b96283b0bac6f..942f94e70a3ddf8c72a7d2f353ef15cd637a5213

---

From: ianswett <[email-masked]>
Date: Sat, 29 Aug 2020 09:35:31 -0700

08e4abb131f757b6c9b220ad5c305efefdf77e89  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/3978/files/942f94e70a3ddf8c72a7d2f353ef15cd637a5213..08e4abb131f757b6c9b220ad5c305efefdf77e89

---

From: mirjak <[email-masked]>
Date: Mon, 31 Aug 2020 02:16:08 -0700

@mirjak 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/3978#pullrequestreview-478448601