Use time_of_last_sent_packet #923
Conversation
time_of_last_sent_packet should be used to set the loss detection timer, instead of using now.
| @@ -687,7 +687,7 @@ Pseudocode for SetLossDetectionAlarm follows: | |||
| alarm_duration = alarm_duration * (2 ^ handshake_count) | |||
| else if (loss_time != 0): | |||
There was a problem hiding this comment.
this is also weird, it's possible that loss_time is greater than tlp timer or RTO, shouldn't we take the min(loss_time, rto_timer, tlp_timer) as the early retransmit timer, or at least min(loss_time, rto)
There was a problem hiding this comment.
I dont think this is an issue. This is at the beginning of the connection, and loss_time only applies to early retransmit. It's possible that loss_time is > TLP or RTO period, but that would be very very strange.
| @@ -701,7 +701,8 @@ Pseudocode for SetLossDetectionAlarm follows: | |||
| alarm_duration = max(alarm_duration, kMinRTOTimeout) | |||
| alarm_duration = alarm_duration * (2 ^ rto_count) | |||
|
|
|||
| loss_detection_alarm.set(now + alarm_duration) | |||
| loss_detection_alarm.set(time_of_last_sent_packet | |||
There was a problem hiding this comment.
for handshake packets should this be the time since the last handshake packet was transmitted?
There was a problem hiding this comment.
That's a good point, given the other packets are not decryptable, and hence will not generate an acknowledgement until the handshake completes, #962 filed.
|
github won't let me comment on the particular line of code, however in detect loss packets the delay until lost is also wrong, it should be 1/8 * max(srtt, lrtt) vs 9/8 -> or maybe it should be 1 /4 because we should at least wait for a 1/4 rtt |
| @@ -687,7 +687,7 @@ Pseudocode for SetLossDetectionAlarm follows: | |||
| alarm_duration = alarm_duration * (2 ^ handshake_count) | |||
| else if (loss_time != 0): | |||
There was a problem hiding this comment.
I dont think this is an issue. This is at the beginning of the connection, and loss_time only applies to early retransmit. It's possible that loss_time is > TLP or RTO period, but that would be very very strange.
time_of_last_sent_packet should be used to set the loss detection timer, instead of using now.
Fixes #540