Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add some MUSTs to congestion control #3978

Merged
merged 17 commits into from Sep 1, 2020
Merged

Add some MUSTs to congestion control #3978

Changes from 1 commit
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
8d117c6
Add some MUSTs
ianswett Aug 2, 2020
02981cd
Update draft-ietf-quic-recovery.md
ianswett Aug 11, 2020
4cd67d6
Update draft-ietf-quic-recovery.md
ianswett Aug 11, 2020
7c85987
Update draft-ietf-quic-recovery.md
ianswett Aug 13, 2020
6c0ad3c
Update draft-ietf-quic-recovery.md
ianswett Aug 13, 2020
350846d
Update draft-ietf-quic-recovery.md
ianswett Aug 18, 2020
3ff276c
Update draft-ietf-quic-recovery.md
ianswett Aug 18, 2020
97b4712
Update draft-ietf-quic-recovery.md
ianswett Aug 18, 2020
1548c57
Update draft-ietf-quic-recovery.md
ianswett Aug 18, 2020
16d6dd8
Clarify NewReno
ianswett Aug 21, 2020
709d8e8
Remove a sentence
ianswett Aug 21, 2020
84729dc
Update draft-ietf-quic-recovery.md
ianswett Aug 21, 2020
71bc8a5
fix duplication and make allowance for not reducing cwnd immediately
janaiyengar Aug 26, 2020
81c7727
consistently newreno
janaiyengar Aug 26, 2020
04f523d
cwnd reduction not on entering recovery
janaiyengar Aug 26, 2020
942f94e
Unify the recovery period text
ianswett Aug 29, 2020
08e4abb
Update draft-ietf-quic-recovery.md
ianswett Aug 29, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
17 changes: 9 additions & 8 deletions draft-ietf-quic-recovery.md
View file
Open in desktop
Expand Up @@ -779,9 +779,10 @@ is declared.
Congestion avoidance uses an Additive Increase Multiplicative Decrease (AIMD)
approach that typically increases the congestion window by one maximum packet
ianswett marked this conversation as resolved.
Show resolved Hide resolved
size per congestion window acknowledged, and MUST NOT increase the congestion
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does not, since this document allows an endpoint to implement a different controller than the one specified here. See Section 7:

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].

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

window faster. When a loss or ECN-CE marking is detected, the congestion
window MUST be halved and the slow start threshold MUST be set to the new
congestion window.
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
ianswett marked this conversation as resolved.
Show resolved Hide resolved
threshold to the new congestion window, and then enters the recovery period.
[RFC8511] specifies an alternate cwnd reduction.
ianswett marked this conversation as resolved.
Show resolved Hide resolved

## Recovery Period

Expand Down Expand Up @@ -997,11 +998,11 @@ 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.


# IANA Considerations
Expand Down