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

Proposal for adding ECN support to QUIC. #1372

Merged
merged 43 commits into from Jun 27, 2018
Merged

Proposal for adding ECN support to QUIC. #1372

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
961d5d9
According to text in wiki. Some MD fixes to get right formating.
gloinul May 18, 2018
b50fe18
Modified proposed text to integrate better and take care of issues ar…
gloinul May 18, 2018
57278fe
Fixed a typo on anchor creation
gloinul May 21, 2018
44a70f8
According to text in wiki. Some MD fixes to get right formating.
gloinul May 18, 2018
afd15f2
Modified proposed text to integrate better and take care of issues ar…
gloinul May 18, 2018
da68127
Fixed a typo on anchor creation
gloinul May 21, 2018
a9755ce
Merge branch 'ecn' of https://github.com/gloinul/base-drafts into ecn
gloinul May 22, 2018
3a95c40
Clarified that ACK or ACK_ECN can be used for acknowledgment in hands…
gloinul May 22, 2018
12b1054
Added ECN intro paragraph. Various editorial improvements of ECN text.
gloinul May 22, 2018
bfe80d5
Fixed bullet list, Added parenthis around section refs.
gloinul May 22, 2018
88f76dc
Fixed indentation and formating
gloinul May 22, 2018
f14134f
Spelling fixed
gloinul May 22, 2018
71c2794
Rewrote the ECN check algorithm for connection migration to be robust.
gloinul May 24, 2018
2ff5ac9
Removed trailing spaces.
gloinul May 24, 2018
19d3b53
Fixed trailing spaces in recovery.
gloinul May 24, 2018
f71e933
Rewraped text in both recovery and transport. Added a new sub-section…
gloinul May 24, 2018
fc94546
Added textual description of ECN-CE indicating congestion events.
gloinul May 28, 2018
66505a3
Editorial fixes
martinthomson May 29, 2018
d525046
Merge pull request #1 from quicwg/ecn
gloinul May 29, 2018
28ef4c1
Editing some formulations. Rewraping more text
gloinul May 29, 2018
6dd9237
Fixed trailing spaces and remaining line length issues, I hope.
gloinul May 30, 2018
0bb5bad
Reflowing now saved
gloinul May 30, 2018
0f94847
Merge branch 'master' of https://github.com/quicwg/base-drafts into ecn
gloinul Jun 4, 2018
0e7d43b
Merge branch 'master' of https://github.com/quicwg/base-drafts into ecn
gloinul Jun 7, 2018
822aded
Changed so that additional ECN-CE marks are sent in immediate ACKs to…
gloinul Jun 7, 2018
345f593
Fixed lint issues
gloinul Jun 7, 2018
e60cc8f
Adding security consideration around ECN into transport.
gloinul Jun 8, 2018
fdf9df7
Reverting some rewrapping that are not necessary to reduce clutter.
gloinul Jun 12, 2018
ec97fd8
Addressing issues raised by Ian Sweet on 180611. Editorial improvemen…
gloinul Jun 12, 2018
bf30190
Clarifying that both cases are capability checks. Correcting a number…
gloinul Jun 12, 2018
60468a7
Fixed a too too much
gloinul Jun 12, 2018
f577c8e
Addressing Martin Thomson's comments. Several editorial changes. The …
gloinul Jun 13, 2018
2d76935
Added paragraph on dealing with persistent loss of acknowledgement of…
gloinul Jun 13, 2018
4af108c
Fixing typo in new paragraph.
gloinul Jun 13, 2018
55fa3d6
Merge branch 'master' of https://github.com/quicwg/base-drafts into ecn
gloinul Jun 18, 2018
e06a2a0
Added requirement on ECN marking suppression for packet duplicates.
gloinul Jun 18, 2018
1636ebc
Clarifying that idefinite state are not requeired in duplication dete…
gloinul Jun 19, 2018
139f1ee
Martin Thomson's editorial suggestions. Restructured pseudo code sect…
gloinul Jun 19, 2018
dbfe4d8
partial editorial fixes
janaiyengar Jun 20, 2018
73bf8cb
Merge pull request #2 from janaiyengar/ecn
gloinul Jun 20, 2018
be91e8f
Moved the ECN block in the ACK prior to the ACK blocks themselves. Re…
gloinul Jun 21, 2018
f8d3d5c
Merge branch 'ecn' of https://github.com/gloinul/base-drafts into ecn
gloinul Jun 21, 2018
cf312f9
Merged ECN counters into single ACK frame section and move them befor…
gloinul Jun 25, 2018
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
88 changes: 44 additions & 44 deletions draft-ietf-quic-recovery.md
View file
Open in desktop
Expand Up @@ -419,8 +419,8 @@ largest received packet number.

Also, reception of an packet marked as ECN Congestion Experience (ECN-CE)
SHOULD be acknowledged more quickly to quicker react to congesiton events.
Additional ECN-CE marks received during the same recovery period does
not need to be immediately acknowledged. See {{congestion-ecn}}.
Additional ECN-CE marks received during the same recovery period does
not need to be immediately acknowledged, see {{congestion-ecn}}.

As an optimization, a receiver MAY process multiple packets before
sending any ACK frames in response. In this case they can determine
Expand Down Expand Up @@ -630,7 +630,7 @@ Pseudocode for OnPacketSent follows:

### On Ack Receipt

When an ack (ACK or ACK_ECN frame) is received, it may acknowledge 0
When an ack (ACK or ACK_ECN frame) is received, it may acknowledge 0
or more packets. Reception of ACK_ECN frame requires that the receiver
check if ECN indicates a congestion event, see {{congestion-ecn}}.

Expand Down Expand Up @@ -892,28 +892,28 @@ in {{tlp}} and {{rto}}.

## Explicit Congestion Notification {#congestion-ecn}

If ECN {!RFC3168} has been verified to work for the current path QUIC
will use the ECN Congestion Experienced (ECN-CE) IP packet marking as a
signal of congestion as a complement to paket loss. This document
specifies to use the classical ECN-CE response, i.e. the same response
as for packet loss. However, there exist potenatial for future
experimentation in using other response functions as discussed in
If ECN {!RFC3168} has been verified to work for the current path QUIC
will use the ECN Congestion Experienced (ECN-CE) IP packet marking as a
signal of congestion as a complement to paket loss. This document
specifies to use the classical ECN-CE response, i.e. the same response
as for packet loss. However, there exist potenatial for future
experimentation in using other response functions as discussed in
{!RFC8311}.

The ACK_ECN frame defined in {{QUIC-TRANSPORT}} does not provide
information on which of the newely acknowledged packets that
was marked with ECN-CE. Therefore, it will be assumed that
The ACK_ECN frame defined in {{QUIC-TRANSPORT}} does not provide
information on which of the newely acknowledged packets that
was marked with ECN-CE. Therefore, it will be assumed that
Copy link
Contributor

Choose a reason for hiding this comment

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

I would suggest rewriting as: "Because newly received marks cause an ACK_ECN be sent immediately, is is assumed the congestion event starts at the highest newly acknowledged packet number."

the congestion event starts at the highest acknowledged packet
number.
number.

## Slow Start

QUIC begins every connection in slow start and exits slow start upon
loss or reception of ECN-CE packet markings. QUIC re-enters slow start
anytime the congestion window is less than sshthresh, which typically
only occurs after an RTO. While in slow start, QUIC increases the
congestion window by the number of acknowledged bytes when each ack is
processed.
QUIC begins every connection in slow start and exits slow start upon
loss or reception of ECN-CE packet markings. QUIC re-enters slow start
anytime the congestion window is less than sshthresh, which typically
only occurs after an RTO. While in slow start, QUIC increases the
congestion window by the number of acknowledged bytes when each ack is
processed.


## Congestion Avoidance
Expand All @@ -926,19 +926,19 @@ window and sets the slow start threshold to the new congestion window.

## Recovery Period

Recovery is a period of time beginning with detection of a lost packet
or the reception of an ECN-CE mark. Because QUIC retransmits stream data
and control frames, not packets, it defines the end of recovery as a
packet sent after the start of recovery being acknowledged. This is
slightly different from TCP's definition of recovery ending when the
Recovery is a period of time beginning with detection of a lost packet
or the reception of an ECN-CE mark. Because QUIC retransmits stream data
and control frames, not packets, it defines the end of recovery as a
packet sent after the start of recovery being acknowledged. This is
slightly different from TCP's definition of recovery ending when the
lost packet that started recovery is acknowledged.

During recovery, the congestion window is not increased or decreased. As
such, multiple lost packets and/or ECN-CE marks only decrease the
congestion window once as long as they're lost before exiting recovery.
This causes QUIC to decrease the congestion window multiple times if
retransmisions are lost, but limits the reduction to once per round
trip.
During recovery, the congestion window is not increased or decreased. As
such, multiple lost packets and/or ECN-CE marks only decrease the
congestion window once as long as they're lost before exiting recovery.
This causes QUIC to decrease the congestion window multiple times if
retransmisions are lost, but limits the reduction to once per round
trip.


## Tail Loss Probe
Expand Down Expand Up @@ -1008,10 +1008,10 @@ Variables required to implement the congestion control mechanisms
are described in this section.

ack_ce_cntr:
: The ACK_ECN counter for ECN-CE marks previously processed. Used to
: The ACK_ECN counter for ECN-CE marks previously processed. Used to
Copy link
Contributor

Choose a reason for hiding this comment

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

I might suggest calling this previous_ecn_ce or previous_ecn_ce_ctr

determine when one or more packet acknowledged by the ACK_ECN frame
was marked with ECN-CE.
was marked with ECN-CE.

Copy link
Contributor

Choose a reason for hiding this comment

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

extra linebreak?


bytes_in_flight:
: The sum of the size in bytes of all sent packets that contain at least
Expand Down Expand Up @@ -1081,21 +1081,21 @@ acked_packet from sent_packets.

### On Packets Marked

Invoked by an increment in the number of CE marked packets, as
Invoked by an increment in the number of CE marked packets, as
indicated by a newly received ACK_ECN frame when compared to
ack_ce_cntr.
ack_ce_cntr.

~~~
OnPacketsMarked(ce_counter):
if (ce_counter > ack_ce_cntr):
// update ack_ce_cntr
ack_ce_cntr = ce_counter
if (!InRecovery(largest_acked_packet)):
// Start a new congestion epoch
end_of_recovery = largest_sent_packet
congestion_window *= kMarkReductionFactor
congestion_window = max(congestion_window, kMinimumWindow)
ssthresh = congestion_window
if (ce_counter > ack_ce_cntr):
// update ack_ce_cntr
ack_ce_cntr = ce_counter
if (!InRecovery(largest_acked_packet)):
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 we want to use largest newly acked packet ideally.

Also, possibly create a single method like "MaybeStartCongestionEvent" to avoid the code duplication between these methods.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed on both, I have reshuffled the pseudocode and introduced a function CongestionEvent. I also noticed that InRecovery appears to be ending one packet to early: #1440

// Start a new congestion epoch
end_of_recovery = largest_sent_packet
congestion_window *= kMarkReductionFactor
congestion_window = max(congestion_window, kMinimumWindow)
ssthresh = congestion_window
~~~


Expand Down