From 1d91fe88d3e037c1653cdff91764f51a0a10cc69 Mon Sep 17 00:00:00 2001 From: ianswett Date: Thu, 30 Nov 2017 11:57:48 -0500 Subject: [PATCH 1/4] Define Min RTT and use it to check ack delay Closes #961 --- draft-ietf-quic-recovery.md | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/draft-ietf-quic-recovery.md b/draft-ietf-quic-recovery.md index 480adec883..6f1589d5e4 100644 --- a/draft-ietf-quic-recovery.md +++ b/draft-ietf-quic-recovery.md @@ -199,11 +199,15 @@ RTT is calculated when an ACK frame arrives by computing the difference between the current time and the time the largest newly acked packet was sent. If no packets are newly acknowledged, RTT cannot be calculated. When RTT is calculated, the ack delay field from the ACK frame SHOULD be subtracted from -the RTT as long as the result is positive. +the RTT as long as the result is larger than the Min RTT. If the result is +smaller than the min_rtt, the RTT should be used, but the ack delay field +should be ignored. Like TCP, QUIC calculates both smoothed RTT and RTT variance as specified in {{?RFC6298}}. +Min RTT is the minimum RTT for a connection prior to adjusting by ack delay. + ## Ack-based Detection Ack-based loss detection implements the spirit of TCP's Fast Retransmit @@ -492,6 +496,9 @@ smoothed_rtt: rttvar: : The RTT variance, computed as described in {{?RFC6298}} +min_rtt: +: The minimum RTT seen in the connection, ignoring ack delay. + reordering_threshold: : The largest delta between the largest acked retransmittable packet and a packet containing retransmittable frames before @@ -530,6 +537,7 @@ follows: loss_time = 0 smoothed_rtt = 0 rttvar = 0 + min_rtt = 0 largest_sent_before_rto = 0 time_of_last_sent_packet = 0 largest_sent_packet = 0 @@ -576,9 +584,7 @@ Pseudocode for OnAckReceived and UpdateRtt follow: // If the largest acked is newly acked, update the RTT. if (sent_packets[ack.largest_acked]): latest_rtt = now - sent_packets[ack.largest_acked].time - if (latest_rtt > ack.ack_delay): - latest_rtt -= ack.delay - UpdateRtt(latest_rtt) + UpdateRtt(latest_rtt, ack.ck_delay) // Find all newly acked packets. for acked_packet in DetermineNewlyAckedPackets(): OnPacketAcked(acked_packet.packet_number) @@ -588,6 +594,11 @@ Pseudocode for OnAckReceived and UpdateRtt follow: UpdateRtt(latest_rtt): + // min_rtt ignores ack delay. + min_rtt = min(min_rtt, latest_rtt) + // Adjust for ack delay if it's plausible. + if (latest_rtt - min_rtt > ack.ack_delay): + latest_rtt -= ack.delay // Based on {{?RFC6298}}. if (smoothed_rtt == 0): smoothed_rtt = latest_rtt From dd7d96091babddb1ef4d09ae3775d3fbce1114cf Mon Sep 17 00:00:00 2001 From: ianswett Date: Thu, 30 Nov 2017 13:23:32 -0500 Subject: [PATCH 2/4] Add rationale --- draft-ietf-quic-recovery.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/draft-ietf-quic-recovery.md b/draft-ietf-quic-recovery.md index 6f1589d5e4..20034d80db 100644 --- a/draft-ietf-quic-recovery.md +++ b/draft-ietf-quic-recovery.md @@ -206,7 +206,10 @@ should be ignored. Like TCP, QUIC calculates both smoothed RTT and RTT variance as specified in {{?RFC6298}}. -Min RTT is the minimum RTT for a connection prior to adjusting by ack delay. +Min RTT is the minimum RTT measured over the connection, prior to adjusting +by ack delay. Ignoring ack delay for min RTT prevents intentional or +unintentional underestimation of min RTT, which in turn prevents +underestimating smoothed RTT. ## Ack-based Detection From 84e2986c2c3fe538c4434e9d7ae7183f66a5af6f Mon Sep 17 00:00:00 2001 From: ianswett Date: Thu, 30 Nov 2017 14:38:09 -0500 Subject: [PATCH 3/4] Fix --- draft-ietf-quic-recovery.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/draft-ietf-quic-recovery.md b/draft-ietf-quic-recovery.md index 20034d80db..361ad79323 100644 --- a/draft-ietf-quic-recovery.md +++ b/draft-ietf-quic-recovery.md @@ -587,7 +587,7 @@ Pseudocode for OnAckReceived and UpdateRtt follow: // If the largest acked is newly acked, update the RTT. if (sent_packets[ack.largest_acked]): latest_rtt = now - sent_packets[ack.largest_acked].time - UpdateRtt(latest_rtt, ack.ck_delay) + UpdateRtt(latest_rtt, ack.ack_delay) // Find all newly acked packets. for acked_packet in DetermineNewlyAckedPackets(): OnPacketAcked(acked_packet.packet_number) @@ -596,7 +596,7 @@ Pseudocode for OnAckReceived and UpdateRtt follow: SetLossDetectionAlarm() - UpdateRtt(latest_rtt): + UpdateRtt(latest_rtt, ack_delay): // min_rtt ignores ack delay. min_rtt = min(min_rtt, latest_rtt) // Adjust for ack delay if it's plausible. From 0b59902375c99f6da841f68bd395236a6be3d281 Mon Sep 17 00:00:00 2001 From: ianswett Date: Thu, 30 Nov 2017 14:38:59 -0500 Subject: [PATCH 4/4] Fix2 --- draft-ietf-quic-recovery.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/draft-ietf-quic-recovery.md b/draft-ietf-quic-recovery.md index 361ad79323..e746d21a11 100644 --- a/draft-ietf-quic-recovery.md +++ b/draft-ietf-quic-recovery.md @@ -600,8 +600,8 @@ Pseudocode for OnAckReceived and UpdateRtt follow: // min_rtt ignores ack delay. min_rtt = min(min_rtt, latest_rtt) // Adjust for ack delay if it's plausible. - if (latest_rtt - min_rtt > ack.ack_delay): - latest_rtt -= ack.delay + if (latest_rtt - min_rtt > ack_delay): + latest_rtt -= ack_delay // Based on {{?RFC6298}}. if (smoothed_rtt == 0): smoothed_rtt = latest_rtt