Incorporate F-RTO #409
Incorporate F-RTO #409
Conversation
draft-ietf-quic-recovery.md
Outdated
| @@ -248,6 +248,10 @@ tlp_count: | |||
| rto_count: | |||
| : The number of times an rto has been sent without receiving an ack. | |||
|
|
|||
| largest_sent_before_rto: | |||
| : The last packet number sent prior to the first transmission due to | |||
There was a problem hiding this comment.
nit: change phrase to "prior to the first retransmission timeout"
draft-ietf-quic-recovery.md
Outdated
| @@ -372,6 +377,12 @@ Pseudocode for OnPacketAcked follows: | |||
|
|
|||
| ~~~ | |||
| OnPacketAcked(acked_packet_number): | |||
| // If a packet sent prior to RTO was acked, then the RTO | |||
| // was spurious. Otherwise, inform congestion control. | |||
| // Similar to the goal of F-RTO {{?RFC5682}} | |||
There was a problem hiding this comment.
I would remove this last line from here and add it (now or later) to the text description.
| if (end_of_recovery < largest_lost_packet.packet_number): | ||
| end_of_recovery = largest_sent_packet | ||
| congestion_window *= kLossReductionFactor | ||
| congestion_window = max(congestion_window, kMinimumWindow) |
There was a problem hiding this comment.
nit: combine this line with the previous one:
congestion_window =
max(congestion_window * kLossReductionFactor, kMinimumWindow)
There was a problem hiding this comment.
I tried, but it ends up taking up 3 lines, so I just kept it as is.
draft-ietf-quic-recovery.md
Outdated
|
|
||
| ~~~ | ||
| TimeToSend(packet_size): | ||
| if (bytes_in_flight + packet_size > congestion_window) | ||
| return infinite | ||
| pacing_coefficient = 1.25 | ||
| if (congestion_window < ssthresh) | ||
| pacing_coefficient = 2 |
There was a problem hiding this comment.
I don't think we need a pacing coeff of 2 here. I never understood why FQ did this, but I don't think we need to have this in the doc. In general I'm less certain that we want to describe the pacing impl as part of the congestion controller... I'm comfortable with definiing the TimeToSend interface without implementing it here. For now, I would at least remove the use of pacing_coeff here.
martinthomson
added
the
design
No description provided.