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.

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

Add some MUSTs to congestion control #3978

Merged
merged 17 commits into from Sep 1, 2020
Merged
Changes from 1 commit
Commits
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
9 changes: 5 additions & 4 deletions draft-ietf-quic-recovery.md
Expand Up @@ -777,10 +777,11 @@ is declared.
## Congestion Avoidance

Congestion avoidance uses an Additive Increase Multiplicative Decrease (AIMD)
approach that typically and at most increases the congestion window by one
maximum packet size per congestion window acknowledged. 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.
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.

## Recovery Period

Expand Down