simplify pacing logic by introducing a SendPacingLimited send mode

connection.go

@@ -1739,7 +1739,7 @@ func (s *connection) sendPackets() error {
 	var sentPacket bool // only used in for packets sent in send mode SendAny
 	for {
 		sendMode := s.sentPacketHandler.SendMode()
-		if sendMode == ackhandler.SendAny && !s.sentPacketHandler.HasPacingBudget() {
+		if sendMode == ackhandler.SendPacingLimited {
 			deadline := s.sentPacketHandler.TimeUntilSend()
 			if deadline.IsZero() {
 				deadline = deadlineSendImmediately
@@ -1753,6 +1753,7 @@ func (s *connection) sendPackets() error {
 			}
 			sendMode = ackhandler.SendAck
 		}
+		//nolint:exhaustive // No need to handle pacing limited here.
 		switch sendMode {
 		case ackhandler.SendNone:
 			return nil

connection_test.go

@@ -604,7 +604,6 @@ var _ = Describe("Connection", func() {
 			sph := mockackhandler.NewMockSentPacketHandler(mockCtrl)
 			sph.EXPECT().GetLossDetectionTimeout().Return(time.Now().Add(time.Hour)).AnyTimes()
 			sph.EXPECT().SendMode().Return(ackhandler.SendAny).AnyTimes()
-			sph.EXPECT().HasPacingBudget().Return(true).AnyTimes()
 			// only expect a single SentPacket() call
 			sph.EXPECT().SentPacket(gomock.Any())
 			tracer.EXPECT().SentShortHeaderPacket(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())
@@ -1206,7 +1205,6 @@ var _ = Describe("Connection", func() {
 			sph.EXPECT().TimeUntilSend().AnyTimes()
 			sph.EXPECT().GetLossDetectionTimeout().AnyTimes()
 			sph.EXPECT().SendMode().Return(ackhandler.SendAny).AnyTimes()
-			sph.EXPECT().HasPacingBudget().Return(true).AnyTimes()
 			sph.EXPECT().SentPacket(gomock.Any())
 			conn.sentPacketHandler = sph
 			runConn()
@@ -1254,7 +1252,6 @@ var _ = Describe("Connection", func() {
 			sph.EXPECT().TimeUntilSend().AnyTimes()
 			sph.EXPECT().GetLossDetectionTimeout().AnyTimes()
 			sph.EXPECT().SendMode().Return(ackhandler.SendAny).AnyTimes()
-			sph.EXPECT().HasPacingBudget().Return(true).AnyTimes()
 			sph.EXPECT().SentPacket(gomock.Any())
 			conn.sentPacketHandler = sph
 			fc := mocks.NewMockConnectionFlowController(mockCtrl)
@@ -1397,10 +1394,9 @@ var _ = Describe("Connection", func() {
 
 		It("sends multiple packets one by one immediately", func() {
 			sph.EXPECT().SentPacket(gomock.Any()).Times(2)
-			sph.EXPECT().HasPacingBudget().Return(true).Times(2)
+			sph.EXPECT().SendMode().Return(ackhandler.SendAny).Times(2)
-			sph.EXPECT().HasPacingBudget()
+			sph.EXPECT().SendMode().Return(ackhandler.SendPacingLimited)
 			sph.EXPECT().TimeUntilSend().Return(time.Now().Add(time.Hour))
-			sph.EXPECT().SendMode().Return(ackhandler.SendAny).Times(3)
 			p, buffer := getShortHeaderPacket(10)
 			packer.EXPECT().PackPacket(false, gomock.Any(), gomock.Any(), conn.version).Return(p, buffer, nil)
 			p, buffer = getShortHeaderPacket(11)
@@ -1418,7 +1414,6 @@ var _ = Describe("Connection", func() {
 
 		It("sends multiple packets, when the pacer allows immediate sending", func() {
 			sph.EXPECT().SentPacket(gomock.Any())
-			sph.EXPECT().HasPacingBudget().Return(true).AnyTimes()
 			sph.EXPECT().SendMode().Return(ackhandler.SendAny).Times(2)
 			p, buffer := getShortHeaderPacket(10)
 			packer.EXPECT().PackPacket(false, gomock.Any(), gomock.Any(), conn.version).Return(p, buffer, nil)
@@ -1436,9 +1431,8 @@ var _ = Describe("Connection", func() {
 
 		It("allows an ACK to be sent when pacing limited", func() {
 			sph.EXPECT().SentPacket(gomock.Any())
-			sph.EXPECT().HasPacingBudget()
 			sph.EXPECT().TimeUntilSend().Return(time.Now().Add(time.Hour))
-			sph.EXPECT().SendMode().Return(ackhandler.SendAny)
+			sph.EXPECT().SendMode().Return(ackhandler.SendPacingLimited)
 			p, buffer := getShortHeaderPacket(10)
 			packer.EXPECT().PackPacket(true, gomock.Any(), gomock.Any(), conn.version).Return(p, buffer, nil)
 
@@ -1457,7 +1451,6 @@ var _ = Describe("Connection", func() {
 		// we shouldn't send the ACK in the same run
 		It("doesn't send an ACK right after becoming congestion limited", func() {
 			sph.EXPECT().SentPacket(gomock.Any())
-			sph.EXPECT().HasPacingBudget().Return(true)
 			sph.EXPECT().SendMode().Return(ackhandler.SendAny)
 			sph.EXPECT().SendMode().Return(ackhandler.SendAck)
 			p, buffer := getShortHeaderPacket(100)
@@ -1475,19 +1468,18 @@ var _ = Describe("Connection", func() {
 
 		It("paces packets", func() {
 			pacingDelay := scaleDuration(100 * time.Millisecond)
-			sph.EXPECT().SendMode().Return(ackhandler.SendAny).AnyTimes()
 			p1, buffer1 := getShortHeaderPacket(100)
 			p2, buffer2 := getShortHeaderPacket(101)
 			gomock.InOrder(
-				sph.EXPECT().HasPacingBudget().Return(true),
+				sph.EXPECT().SendMode().Return(ackhandler.SendAny),
 				packer.EXPECT().PackPacket(false, gomock.Any(), gomock.Any(), conn.version).Return(p1, buffer1, nil),
 				sph.EXPECT().SentPacket(gomock.Any()),
-				sph.EXPECT().HasPacingBudget(),
+				sph.EXPECT().SendMode().Return(ackhandler.SendPacingLimited),
 				sph.EXPECT().TimeUntilSend().Return(time.Now().Add(pacingDelay)),
-				sph.EXPECT().HasPacingBudget().Return(true),
+				sph.EXPECT().SendMode().Return(ackhandler.SendAny),
 				packer.EXPECT().PackPacket(false, gomock.Any(), gomock.Any(), conn.version).Return(p2, buffer2, nil),
 				sph.EXPECT().SentPacket(gomock.Any()),
-				sph.EXPECT().HasPacingBudget(),
+				sph.EXPECT().SendMode().Return(ackhandler.SendPacingLimited),
 				sph.EXPECT().TimeUntilSend().Return(time.Now().Add(time.Hour)),
 			)
 			written := make(chan struct{}, 2)
@@ -1506,10 +1498,9 @@ var _ = Describe("Connection", func() {
 
 		It("sends multiple packets at once", func() {
 			sph.EXPECT().SentPacket(gomock.Any()).Times(3)
-			sph.EXPECT().HasPacingBudget().Return(true).Times(3)
+			sph.EXPECT().SendMode().Return(ackhandler.SendAny).Times(3)
-			sph.EXPECT().HasPacingBudget()
+			sph.EXPECT().SendMode().Return(ackhandler.SendPacingLimited)
 			sph.EXPECT().TimeUntilSend().Return(time.Now().Add(time.Hour))
-			sph.EXPECT().SendMode().Return(ackhandler.SendAny).Times(4)
 			for pn := protocol.PacketNumber(1000); pn < 1003; pn++ {
 				p, buffer := getShortHeaderPacket(pn)
 				packer.EXPECT().PackPacket(false, gomock.Any(), gomock.Any(), conn.version).Return(p, buffer, nil)
@@ -1541,7 +1532,6 @@ var _ = Describe("Connection", func() {
 			written := make(chan struct{})
 			sender.EXPECT().WouldBlock().AnyTimes()
 			sph.EXPECT().SentPacket(gomock.Any())
-			sph.EXPECT().HasPacingBudget().Return(true).AnyTimes()
 			sph.EXPECT().SendMode().Return(ackhandler.SendAny).AnyTimes()
 			p, buffer := getShortHeaderPacket(1000)
 			packer.EXPECT().PackPacket(false, gomock.Any(), gomock.Any(), conn.version).Return(p, buffer, nil)
@@ -1565,7 +1555,6 @@ var _ = Describe("Connection", func() {
 				sph.EXPECT().ReceivedBytes(gomock.Any())
 				conn.handlePacket(&receivedPacket{buffer: getPacketBuffer()})
 			})
-			sph.EXPECT().HasPacingBudget().Return(true).AnyTimes()
 			sph.EXPECT().SendMode().Return(ackhandler.SendAny).AnyTimes()
 			p, buffer := getShortHeaderPacket(1000)
 			packer.EXPECT().PackPacket(false, gomock.Any(), gomock.Any(), conn.version).Return(p, buffer, nil)
@@ -1580,7 +1569,6 @@ var _ = Describe("Connection", func() {
 
 		It("stops sending when the send queue is full", func() {
 			sph.EXPECT().SentPacket(gomock.Any())
-			sph.EXPECT().HasPacingBudget().Return(true).AnyTimes()
 			sph.EXPECT().SendMode().Return(ackhandler.SendAny)
 			p, buffer := getShortHeaderPacket(1000)
 			packer.EXPECT().PackPacket(false, gomock.Any(), gomock.Any(), conn.version).Return(p, buffer, nil)
@@ -1601,7 +1589,6 @@ var _ = Describe("Connection", func() {
 
 			// now make room in the send queue
 			sph.EXPECT().SentPacket(gomock.Any())
-			sph.EXPECT().HasPacingBudget().Return(true).AnyTimes()
 			sph.EXPECT().SendMode().Return(ackhandler.SendAny).AnyTimes()
 			sender.EXPECT().WouldBlock().AnyTimes()
 			p, buffer = getShortHeaderPacket(1001)
@@ -1617,7 +1604,6 @@ var _ = Describe("Connection", func() {
 		})
 
 		It("doesn't set a pacing timer when there is no data to send", func() {
-			sph.EXPECT().HasPacingBudget().Return(true)
 			sph.EXPECT().SendMode().Return(ackhandler.SendAny).AnyTimes()
 			sender.EXPECT().WouldBlock().AnyTimes()
 			packer.EXPECT().PackPacket(false, gomock.Any(), gomock.Any(), conn.version).Return(shortHeaderPacket{}, nil, errNothingToPack)
@@ -1636,7 +1622,6 @@ var _ = Describe("Connection", func() {
 			conn.mtuDiscoverer = mtuDiscoverer
 			conn.config.DisablePathMTUDiscovery = false
 			sph.EXPECT().SentPacket(gomock.Any())
-			sph.EXPECT().HasPacingBudget().Return(true).AnyTimes()
 			sph.EXPECT().SendMode().Return(ackhandler.SendAny)
 			sph.EXPECT().SendMode().Return(ackhandler.SendNone)
 			written := make(chan struct{}, 1)
@@ -1688,7 +1673,7 @@ var _ = Describe("Connection", func() {
 			sph.EXPECT().GetLossDetectionTimeout().AnyTimes()
 			sph.EXPECT().TimeUntilSend().AnyTimes()
 			sph.EXPECT().SendMode().Return(ackhandler.SendAny).AnyTimes()
-			sph.EXPECT().HasPacingBudget().Return(true).AnyTimes()
+
 			sph.EXPECT().SentPacket(gomock.Any())
 			conn.sentPacketHandler = sph
 			p, buffer := getShortHeaderPacket(1)
@@ -1717,7 +1702,6 @@ var _ = Describe("Connection", func() {
 			sph := mockackhandler.NewMockSentPacketHandler(mockCtrl)
 			sph.EXPECT().GetLossDetectionTimeout().AnyTimes()
 			sph.EXPECT().SendMode().Return(ackhandler.SendAny).AnyTimes()
-			sph.EXPECT().HasPacingBudget().Return(true).AnyTimes()
 			sph.EXPECT().SentPacket(gomock.Any()).Do(func(p *ackhandler.Packet) {
 				Expect(p.PacketNumber).To(Equal(protocol.PacketNumber(1234)))
 			})
@@ -1771,7 +1755,6 @@ var _ = Describe("Connection", func() {
 		sph.EXPECT().GetLossDetectionTimeout().AnyTimes()
 		sph.EXPECT().SendMode().Return(ackhandler.SendAny).AnyTimes()
 		sph.EXPECT().TimeUntilSend().Return(time.Now()).AnyTimes()
-		sph.EXPECT().HasPacingBudget().Return(true).AnyTimes()
 		gomock.InOrder(
 			sph.EXPECT().SentPacket(gomock.Any()).Do(func(p *ackhandler.Packet) {
 				Expect(p.EncryptionLevel).To(Equal(protocol.EncryptionInitial))
@@ -1923,7 +1906,6 @@ var _ = Describe("Connection", func() {
 		sph.EXPECT().SendMode().Return(ackhandler.SendAny).AnyTimes()
 		sph.EXPECT().GetLossDetectionTimeout().AnyTimes()
 		sph.EXPECT().TimeUntilSend().AnyTimes()
-		sph.EXPECT().HasPacingBudget().Return(true).AnyTimes()
 		sph.EXPECT().SetHandshakeConfirmed()
 		sph.EXPECT().SentPacket(gomock.Any())
 		mconn.EXPECT().Write(gomock.Any())

internal/ackhandler/interfaces.go

@@ -24,8 +24,6 @@ type SentPacketHandler interface {
 	// TimeUntilSend is the time when the next packet should be sent.
 	// It is used for pacing packets.
 	TimeUntilSend() time.Time
-	// HasPacingBudget says if the pacer allows sending of a (full size) packet at this moment.
-	HasPacingBudget() bool
 	SetMaxDatagramSize(count protocol.ByteCount)
 
 	// only to be called once the handshake is complete

internal/ackhandler/send_mode.go

@@ -16,6 +16,10 @@ const (
 	SendPTOHandshake
 	// SendPTOAppData means that an Application data probe packet should be sent
 	SendPTOAppData
+	// SendPacingLimited means that the pacer doesn't allow sending of a packet right now,
+	// but will do in a little while.
+	// The timestamp when sending is allowed again can be obtained via the SentPacketHandler.TimeUntilSend.
+	SendPacingLimited
 	// SendAny means that any packet should be sent
 	SendAny
 )
@@ -34,6 +38,8 @@ func (s SendMode) String() string {
 		return "pto (Application Data)"
 	case SendAny:
 		return "any"
+	case SendPacingLimited:
+		return "pacing limited"
 	default:
 		return fmt.Sprintf("invalid send mode: %d", s)
 	}

internal/ackhandler/send_mode_test.go

@@ -9,6 +9,7 @@ var _ = Describe("Send Mode", func() {
 	It("has a string representation", func() {
 		Expect(SendNone.String()).To(Equal("none"))
 		Expect(SendAny.String()).To(Equal("any"))
+		Expect(SendPacingLimited.String()).To(Equal("pacing limited"))
 		Expect(SendAck.String()).To(Equal("ack"))
 		Expect(SendPTOInitial.String()).To(Equal("pto (Initial)"))
 		Expect(SendPTOHandshake.String()).To(Equal("pto (Handshake)"))

internal/ackhandler/sent_packet_handler.go

@@ -756,6 +756,9 @@ func (h *sentPacketHandler) SendMode() SendMode {
 		}
 		return SendAck
 	}
+	if !h.congestion.HasPacingBudget() {
+		return SendPacingLimited
+	}
 	return SendAny
 }
 

internal/ackhandler/sent_packet_handler_test.go

@@ -599,12 +599,14 @@ var _ = Describe("SentPacketHandler", func() {
 				SendTime:        time.Now(),
 			})
 			cong.EXPECT().CanSend(protocol.ByteCount(42)).Return(true)
+			cong.EXPECT().HasPacingBudget().Return(true)
 			handler.SendMode()
 		})
 
 		It("allows sending of ACKs when congestion limited", func() {
 			handler.ReceivedPacket(protocol.EncryptionHandshake)
 			cong.EXPECT().CanSend(gomock.Any()).Return(true)
+			cong.EXPECT().HasPacingBudget().Return(true)
 			Expect(handler.SendMode()).To(Equal(SendAny))
 			cong.EXPECT().CanSend(gomock.Any()).Return(false)
 			Expect(handler.SendMode()).To(Equal(SendAck))
@@ -613,6 +615,7 @@ var _ = Describe("SentPacketHandler", func() {
 		It("allows sending of ACKs when we're keeping track of MaxOutstandingSentPackets packets", func() {
 			handler.ReceivedPacket(protocol.EncryptionHandshake)
 			cong.EXPECT().CanSend(gomock.Any()).Return(true).AnyTimes()
+			cong.EXPECT().HasPacingBudget().Return(true).AnyTimes()
 			cong.EXPECT().OnPacketSent(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes()
 			for i := protocol.PacketNumber(0); i < protocol.MaxOutstandingSentPackets; i++ {
 				Expect(handler.SendMode()).To(Equal(SendAny))
@@ -889,6 +892,7 @@ var _ = Describe("SentPacketHandler", func() {
 
 	Context("amplification limit, for the server", func() {
 		It("limits the window to 3x the bytes received, to avoid amplification attacks", func() {
+			now := time.Now()
 			handler.ReceivedPacket(protocol.EncryptionInitial) // receiving an Initial packet doesn't validate the client's address
 			handler.ReceivedBytes(200)
 			handler.SentPacket(&Packet{
@@ -896,7 +900,7 @@ var _ = Describe("SentPacketHandler", func() {
 				Length:          599,
 				EncryptionLevel: protocol.EncryptionInitial,
 				Frames:          []Frame{{Frame: &wire.PingFrame{}}},
-				SendTime:        time.Now(),
+				SendTime:        now,
 			})
 			Expect(handler.SendMode()).To(Equal(SendAny))
 			handler.SentPacket(&Packet{
@@ -904,7 +908,7 @@ var _ = Describe("SentPacketHandler", func() {
 				Length:          1,
 				EncryptionLevel: protocol.EncryptionInitial,
 				Frames:          []Frame{{Frame: &wire.PingFrame{}}},
-				SendTime:        time.Now(),
+				SendTime:        now,
 			})
 			Expect(handler.SendMode()).To(Equal(SendNone))
 		})

internal/mocks/ackhandler/sent_packet_handler.go

@@ -63,20 +63,6 @@ func (mr *MockSentPacketHandlerMockRecorder) GetLossDetectionTimeout() *gomock.C
 	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetLossDetectionTimeout", reflect.TypeOf((*MockSentPacketHandler)(nil).GetLossDetectionTimeout))
 }
 
-// HasPacingBudget mocks base method.
-func (m *MockSentPacketHandler) HasPacingBudget() bool {
-	m.ctrl.T.Helper()
-	ret := m.ctrl.Call(m, "HasPacingBudget")
-	ret0, _ := ret[0].(bool)
-	return ret0
-}
-
-// HasPacingBudget indicates an expected call of HasPacingBudget.
-func (mr *MockSentPacketHandlerMockRecorder) HasPacingBudget() *gomock.Call {
-	mr.mock.ctrl.T.Helper()
-	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "HasPacingBudget", reflect.TypeOf((*MockSentPacketHandler)(nil).HasPacingBudget))
-}
-
 // OnLossDetectionTimeout mocks base method.
 func (m *MockSentPacketHandler) OnLossDetectionTimeout() error {
 	m.ctrl.T.Helper()