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
First version of congestion control #395
Conversation
draft-ietf-quic-recovery.md
Outdated
We first describe the variables required to implement the loss detection | ||
mechanisms described in this section. | ||
Variables required to implement the congestion control mechanisms | ||
described in this section. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"are described in this section"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
draft-ietf-quic-recovery.md
Outdated
(describe NewReno-style congestion control {{?RFC6582}} for QUIC.) | ||
(describe appropriate byte counting.) | ||
(define recovery based on packet numbers.) | ||
QUIC's congestion control is based on TCP New Reno{{?RFC6582}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NewReno -- 1 word
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
draft-ietf-quic-recovery.md
Outdated
QUIC uses a slow start approach where the congestion window is increased | ||
by the same number of bytes as are acknowledged while in slow start. | ||
QUIC begins every connection in slow start and exits slow start upon | ||
loss or when the min_rtt increases by more than 1/8th for a full round. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove the 1/8th RTT -- that's mixing hystart into this doc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, NewReno doesn't have hystart? How 80s.
draft-ietf-quic-recovery.md
Outdated
Recovery is a period of time beginning with detection of a lost packet. | ||
It ends when all packets outstanding at the time recovery began have been | ||
acknowledged or lost. During recovery, the congestion window is not | ||
increased and the pacing gain is reduced to 1. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd remove mention of "pacing gain"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
largest_lost_packet = lost_packets.last() | ||
if (end_of_recovery < largest_lost_packet.packet_number): | ||
end_of_recovery = largest_sent_packet | ||
congestion_window *= kLossReductionFactor |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ssthresh = congestion_window
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I forgot about ssthresh.
draft-ietf-quic-recovery.md
Outdated
congestion_window *= kLossReductionFactor | ||
~~~ | ||
|
||
## Sending Packets |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would rename this section "Pacing Packets" or something like it
draft-ietf-quic-recovery.md
Outdated
return time_of_last_sent_packet + | ||
packet_size * smoothed_rtt / congestion_window | ||
~~~ | ||
|
||
(describe min_rtt based hystart.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this line
Variables required to implement the congestion control mechanisms | ||
are described in this section. | ||
|
||
bytes_in_flight: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What exactly is this measuring: IP, UDP and QUIC header? ACK's, or just the sum of retransmittable frames, or just stream payload?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mikkelfj, good question. Could you open an issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks
No description provided.