From def3700fd164129cdde8bac7b8942440306b4cf8 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Mon, 22 Jan 2018 12:21:42 +1100 Subject: [PATCH] ignore the delay in an ACK if it results in an RTT less than minRTT --- ackhandler/sent_packet_handler_test.go | 4 +++- congestion/rtt_stats.go | 6 +++--- congestion/rtt_stats_test.go | 24 ++++++++++++++---------- 3 files changed, 20 insertions(+), 14 deletions(-) diff --git a/ackhandler/sent_packet_handler_test.go b/ackhandler/sent_packet_handler_test.go index a2fd0f65..08bbc301 100644 --- a/ackhandler/sent_packet_handler_test.go +++ b/ackhandler/sent_packet_handler_test.go @@ -499,8 +499,10 @@ var _ = Describe("SentPacketHandler", func() { Expect(handler.rttStats.LatestRTT()).To(BeNumerically("~", 1*time.Minute, 1*time.Second)) }) - It("uses the DelayTime in the ack frame", func() { + It("uses the DelayTime in the ACK frame", func() { now := time.Now() + // make sure the rttStats have a min RTT, so that the delay is used + handler.rttStats.UpdateRTT(5*time.Minute, 0, time.Now()) getPacketElement(1).Value.sendTime = now.Add(-10 * time.Minute) err := handler.ReceivedAck(&wire.AckFrame{LargestAcked: 1, DelayTime: 5 * time.Minute}, 1, protocol.EncryptionUnencrypted, time.Now()) Expect(err).NotTo(HaveOccurred()) diff --git a/congestion/rtt_stats.go b/congestion/rtt_stats.go index 624957ce..9e5e4541 100644 --- a/congestion/rtt_stats.go +++ b/congestion/rtt_stats.go @@ -98,10 +98,10 @@ func (r *RTTStats) UpdateRTT(sendDelta, ackDelay time.Duration, now time.Time) { r.updateRecentMinRTT(sendDelta, now) // Correct for ackDelay if information received from the peer results in a - // positive RTT sample. Otherwise, we use the sendDelta as a reasonable - // measure for smoothedRTT. + // an RTT sample at least as large as minRTT. Otherwise, only use the + // sendDelta. sample := sendDelta - if sample > ackDelay { + if sample-r.minRTT >= ackDelay { sample -= ackDelay } r.latestRTT = sample diff --git a/congestion/rtt_stats_test.go b/congestion/rtt_stats_test.go index 6fae2795..718bec3e 100644 --- a/congestion/rtt_stats_test.go +++ b/congestion/rtt_stats_test.go @@ -24,18 +24,18 @@ var _ = Describe("RTT stats", func() { }) It("SmoothedRTT", func() { - // Verify that ack_delay is corrected for in Smoothed RTT. + // Verify that ack_delay is ignored in the first measurement. rttStats.UpdateRTT((300 * time.Millisecond), (100 * time.Millisecond), time.Time{}) - Expect(rttStats.LatestRTT()).To(Equal((200 * time.Millisecond))) - Expect(rttStats.SmoothedRTT()).To(Equal((200 * time.Millisecond))) - // Verify that effective RTT of zero does not change Smoothed RTT. - rttStats.UpdateRTT((200 * time.Millisecond), (200 * time.Millisecond), time.Time{}) - Expect(rttStats.LatestRTT()).To(Equal((200 * time.Millisecond))) - Expect(rttStats.SmoothedRTT()).To(Equal((200 * time.Millisecond))) + Expect(rttStats.LatestRTT()).To(Equal((300 * time.Millisecond))) + Expect(rttStats.SmoothedRTT()).To(Equal((300 * time.Millisecond))) + // Verify that Smoothed RTT includes max ack delay if it's reasonable. + rttStats.UpdateRTT((350 * time.Millisecond), (50 * time.Millisecond), time.Time{}) + Expect(rttStats.LatestRTT()).To(Equal((300 * time.Millisecond))) + Expect(rttStats.SmoothedRTT()).To(Equal((300 * time.Millisecond))) // Verify that large erroneous ack_delay does not change Smoothed RTT. rttStats.UpdateRTT((200 * time.Millisecond), (300 * time.Millisecond), time.Time{}) Expect(rttStats.LatestRTT()).To(Equal((200 * time.Millisecond))) - Expect(rttStats.SmoothedRTT()).To(Equal((200 * time.Millisecond))) + Expect(rttStats.SmoothedRTT()).To(Equal((287500 * time.Microsecond))) }) It("MinRTT", func() { @@ -197,11 +197,15 @@ var _ = Describe("RTT stats", func() { }) It("ResetAfterConnectionMigrations", func() { + rttStats.UpdateRTT((200 * time.Millisecond), 0, time.Time{}) + Expect(rttStats.LatestRTT()).To(Equal((200 * time.Millisecond))) + Expect(rttStats.SmoothedRTT()).To(Equal((200 * time.Millisecond))) + Expect(rttStats.MinRTT()).To(Equal((200 * time.Millisecond))) rttStats.UpdateRTT((300 * time.Millisecond), (100 * time.Millisecond), time.Time{}) Expect(rttStats.LatestRTT()).To(Equal((200 * time.Millisecond))) Expect(rttStats.SmoothedRTT()).To(Equal((200 * time.Millisecond))) - Expect(rttStats.MinRTT()).To(Equal((300 * time.Millisecond))) - Expect(rttStats.RecentMinRTT()).To(Equal(300 * time.Millisecond)) + Expect(rttStats.MinRTT()).To(Equal((200 * time.Millisecond))) + Expect(rttStats.RecentMinRTT()).To(Equal(200 * time.Millisecond)) // Reset rtt stats on connection migrations. rttStats.OnConnectionMigration()