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

Attempt to define slow start/congestion avoidance #4005

Merged
merged 9 commits into from Sep 4, 2020
Merged

Attempt to define slow start/congestion avoidance #4005

Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
2a9cb75
Attempt to define slow start/congestion avoidance
martinthomson Aug 17, 2020
2f3927e
detected/increased
martinthomson Aug 17, 2020
755f59a
Integrate the changes from #3978
martinthomson Aug 18, 2020
2775fdd
ssthresh being initiallly infinite means that you start in slow start
martinthomson Aug 18, 2020
bb96175
Reword congestion signal response
martinthomson Aug 25, 2020
4478c2b
Merge branch 'master' into define-ss-ca
martinthomson Aug 25, 2020
72c57b9
Merge remote-tracking branch 'origin/ianswett-must-new-reno' into def…
martinthomson Aug 25, 2020
fb99434
Merge branch 'master' into define-ss-ca
martinthomson Sep 1, 2020
3ad13ce
Sprinkle some NewReno pixie dust
martinthomson Sep 1, 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
2 changes: 1 addition & 1 deletion draft-ietf-quic-recovery.md
View file
Open in desktop
Expand Up @@ -787,7 +787,7 @@ approach that increases the congestion window by one maximum packet size per
congestion window acknowledged.

When first entering congestion avoidance, when a packet is declared lost, or
when the ECN-CE count is detected, the congestion controller halves the
when the ECN-CE count is increased, the congestion controller halves the
congestion window, reduces the slow start threshold to the resulting size, and
enters the recovery period.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm just realising this now but this is actually not how TCP works. When you enter recovery you halve the ssthresh (basically to remember the right value for later). Then what TCP does is that is keeps sending one new packet for each ACK received (conservation principle: not know which or how many packets have been ACKed but know there must be at least one successful received and leaving the network) and then when all losses are repaired it sets the cwnd to ssthresh and leaves recovery. If you halve cwnd immediately when entering recovery, you can't send data for half an RTT until you received enough ACKs to be below the cwnd. This is usually not what you want.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think you're correct, do you have a good RFC reference handy? I couldn't easily find this text.

Copy link
Contributor

@mirjak mirjak Aug 18, 2020 •
edited

Choose a reason for hiding this comment

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

https://tools.ietf.org/html/rfc5681#section-4.3
"Finally, after all loss in the given window of segments
has been successfully retransmitted, cwnd MUST be set to no more than
ssthresh and congestion avoidance MUST be used to further increase
cwnd."

Copy link
Member Author

Choose a reason for hiding this comment

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

@janaiyengar just explained to me why this difference exists, and I found the argument convincing. I don't know whether this is worth calling out though, it seems like it might be a little tricky to get down in text.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the MUST requirement is that the cwnd is set to ssthresh when LEAVING recovery. In order to decrement the cwnd gradually we could say that only one new packet for two ACKed ones should be send which then would be mostly inline with TCP or we can leave the detailed scheme to the implementation and just put some warnings to not overload the link in recovery. Alternatively you could also e.g. halve the cwnd and then pace out packets over one RTT (which is assume is what implementation do right now?)

Copy link
Contributor

Choose a reason for hiding this comment

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

If we're going to make a change, I think it needs to be on #3978, which is a design PR that's going to be sent out for consensus call soon.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. This PR is intended to be a strictly editorial conversion. I hope that I have faithfully translated the text from #3978.

Copy link
Contributor

Choose a reason for hiding this comment

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

(This is being discussed in #3978.)


Expand Down