Re: [quicwg/base-drafts] First version of congestion control (#395)

[janaiyengar <[e-mail]>]

janaiyengar commented on this pull request.



> @@ -202,8 +202,8 @@ kDefaultInitialRtt (default 100ms):
 
 ## Variables of interest
 
-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.

"are described in this section"

>  ~~~
 
 # Congestion Control
 
-(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}}

NewReno -- 1 word

>  ~~~
 
 # Congestion Control
 
-(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}}
+congestion control to determine the congestion window and pacing rate.
+
+## Slow Start
+
+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.

remove the 1/8th RTT -- that's mixing hystart into this doc

> +QUIC's congestion control is based on TCP New Reno{{?RFC6582}}
+congestion control to determine the congestion window and pacing rate.
+
+## Slow Start
+
+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.
+
+## Recovery
+
+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.

I'd remove mention of "pacing gain"

> +     else:
+       congestion_window +=
+           1500 * acked_packets.bytes / congestion_window
+~~~
+
+## On Packets Lost
+
+Invoked by loss detection from DetectLostPackets when new packets
+are detected lost.
+
+~~~
+   OnPacketsLost(lost_packets):
+     largest_lost_packet = lost_packets.last()
+     if (end_of_recovery < largest_lost_packet.packet_number):
+       end_of_recovery = largest_sent_packet
+       congestion_window *= kLossReductionFactor

ssthresh = congestion_window

> +~~~
+
+## On Packets Lost
+
+Invoked by loss detection from DetectLostPackets when new packets
+are detected lost.
+
+~~~
+   OnPacketsLost(lost_packets):
+     largest_lost_packet = lost_packets.last()
+     if (end_of_recovery < largest_lost_packet.packet_number):
+       end_of_recovery = largest_sent_packet
+       congestion_window *= kLossReductionFactor
+~~~
+
+## Sending Packets

I would rename this section  "Pacing Packets" or something like it

> +
+QUIC sends a packet if there is available congestion window and
+sending the packet does not exceed the pacing rate.
+
+TimeToSend returns infinite if the congestion controller is congestion
+window limited, a time in the past if the packet can be sent
+immediately, and a time in the future if sending is pacing limited.
+
+~~~
+   TimeToSend(packet_size):
+     if (bytes_in_flight + packet_size > congestion_window)
+       return infinite
+     return time_of_last_sent_packet + 
+         packet_size * smoothed_rtt / congestion_window
+~~~
+
 (describe min_rtt based hystart.)

Remove this line

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/395#pullrequestreview-26667457