From 2e98150cd491dd4e63321e59ba0a542eceb1794a Mon Sep 17 00:00:00 2001
From: Marten Seemann <martenseemann@gmail.com>
Date: Fri, 20 May 2022 12:10:39 +0200
Subject: [PATCH] don't send path MTU probe packets on a timer (#3423)

---
 connection.go               |  5 -----
 connection_test.go          |  6 +-----
 mock_mtu_discoverer_test.go | 14 --------------
 mtu_discoverer.go           | 12 +-----------
 mtu_discoverer_test.go      |  4 ----
 5 files changed, 2 insertions(+), 39 deletions(-)

diff --git a/connection.go b/connection.go
index 9c25f89eb97..47f4ca99f05 100644
--- a/connection.go
+++ b/connection.go
@@ -751,11 +751,6 @@ func (s *connection) maybeResetTimer() {
 			deadline = s.idleTimeoutStartTime().Add(s.idleTimeout)
 		}
 	}
-	if s.handshakeConfirmed && !s.config.DisablePathMTUDiscovery {
-		if probeTime := s.mtuDiscoverer.NextProbeTime(); !probeTime.IsZero() {
-			deadline = utils.MinTime(deadline, probeTime)
-		}
-	}
 
 	if ackAlarm := s.receivedPacketHandler.GetAlarmTimeout(); !ackAlarm.IsZero() {
 		deadline = utils.MinTime(deadline, ackAlarm)
diff --git a/connection_test.go b/connection_test.go
index cab29632596..1677a6d2ec0 100644
--- a/connection_test.go
+++ b/connection_test.go
@@ -1691,11 +1691,7 @@ var _ = Describe("Connection", func() {
 			written := make(chan struct{}, 1)
 			sender.EXPECT().WouldBlock().AnyTimes()
 			sender.EXPECT().Send(gomock.Any()).DoAndReturn(func(p *packetBuffer) { written <- struct{}{} })
-			gomock.InOrder(
-				mtuDiscoverer.EXPECT().NextProbeTime(),
-				mtuDiscoverer.EXPECT().ShouldSendProbe(gomock.Any()).Return(true),
-				mtuDiscoverer.EXPECT().NextProbeTime(),
-			)
+			mtuDiscoverer.EXPECT().ShouldSendProbe(gomock.Any()).Return(true)
 			ping := ackhandler.Frame{Frame: &wire.PingFrame{}}
 			mtuDiscoverer.EXPECT().GetPing().Return(ping, protocol.ByteCount(1234))
 			packer.EXPECT().PackMTUProbePacket(ping, protocol.ByteCount(1234)).Return(getPacket(1), nil)
diff --git a/mock_mtu_discoverer_test.go b/mock_mtu_discoverer_test.go
index 530180a4c21..3d36b3bb941 100644
--- a/mock_mtu_discoverer_test.go
+++ b/mock_mtu_discoverer_test.go
@@ -51,20 +51,6 @@ func (mr *MockMtuDiscovererMockRecorder) GetPing() *gomock.Call {
 	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetPing", reflect.TypeOf((*MockMtuDiscoverer)(nil).GetPing))
 }
 
-// NextProbeTime mocks base method.
-func (m *MockMtuDiscoverer) NextProbeTime() time.Time {
-	m.ctrl.T.Helper()
-	ret := m.ctrl.Call(m, "NextProbeTime")
-	ret0, _ := ret[0].(time.Time)
-	return ret0
-}
-
-// NextProbeTime indicates an expected call of NextProbeTime.
-func (mr *MockMtuDiscovererMockRecorder) NextProbeTime() *gomock.Call {
-	mr.mock.ctrl.T.Helper()
-	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "NextProbeTime", reflect.TypeOf((*MockMtuDiscoverer)(nil).NextProbeTime))
-}
-
 // ShouldSendProbe mocks base method.
 func (m *MockMtuDiscoverer) ShouldSendProbe(now time.Time) bool {
 	m.ctrl.T.Helper()
diff --git a/mtu_discoverer.go b/mtu_discoverer.go
index a5a83021935..bf38eaac7d6 100644
--- a/mtu_discoverer.go
+++ b/mtu_discoverer.go
@@ -11,7 +11,6 @@ import (
 
 type mtuDiscoverer interface {
 	ShouldSendProbe(now time.Time) bool
-	NextProbeTime() time.Time
 	GetPing() (ping ackhandler.Frame, datagramSize protocol.ByteCount)
 }
 
@@ -53,16 +52,7 @@ func (f *mtuFinder) ShouldSendProbe(now time.Time) bool {
 	if f.probeInFlight || f.done() {
 		return false
 	}
-	return !now.Before(f.NextProbeTime())
-}
-
-// NextProbeTime returns the time when the next probe packet should be sent.
-// It returns the zero value if no probe packet should be sent.
-func (f *mtuFinder) NextProbeTime() time.Time {
-	if f.probeInFlight || f.done() {
-		return time.Time{}
-	}
-	return f.lastProbeTime.Add(mtuProbeDelay * f.rttStats.SmoothedRTT())
+	return !now.Before(f.lastProbeTime.Add(mtuProbeDelay * f.rttStats.SmoothedRTT()))
 }
 
 func (f *mtuFinder) GetPing() (ackhandler.Frame, protocol.ByteCount) {
diff --git a/mtu_discoverer_test.go b/mtu_discoverer_test.go
index b603f2fc29d..89b5150eb10 100644
--- a/mtu_discoverer_test.go
+++ b/mtu_discoverer_test.go
@@ -38,17 +38,14 @@ var _ = Describe("MTU Discoverer", func() {
 	It("only allows a probe 5 RTTs after the handshake completes", func() {
 		Expect(d.ShouldSendProbe(now)).To(BeFalse())
 		Expect(d.ShouldSendProbe(now.Add(rtt * 9 / 2))).To(BeFalse())
-		Expect(d.NextProbeTime()).To(BeTemporally("~", now.Add(5*rtt), scaleDuration(20*time.Millisecond)))
 		Expect(d.ShouldSendProbe(now.Add(rtt * 5))).To(BeTrue())
 	})
 
 	It("doesn't allow a probe if another probe is still in flight", func() {
 		ping, _ := d.GetPing()
 		Expect(d.ShouldSendProbe(now.Add(10 * rtt))).To(BeFalse())
-		Expect(d.NextProbeTime()).To(BeZero())
 		ping.OnLost(ping.Frame)
 		Expect(d.ShouldSendProbe(now.Add(10 * rtt))).To(BeTrue())
-		Expect(d.NextProbeTime()).ToNot(BeZero())
 	})
 
 	It("tries a lower size when a probe is lost", func() {
@@ -79,7 +76,6 @@ var _ = Describe("MTU Discoverer", func() {
 		}
 		Expect(sizes).To(Equal([]protocol.ByteCount{1500, 1750, 1875, 1937, 1968, 1984}))
 		Expect(d.ShouldSendProbe(t.Add(10 * rtt))).To(BeFalse())
-		Expect(d.NextProbeTime()).To(BeZero())
 	})
 
 	It("finds the MTU", func() {