force sending of a retransmittable packet every 20 packets

ackhandler/interfaces.go

@@ -15,6 +15,7 @@ type SentPacketHandler interface {
 
 	SendingAllowed() bool
 	GetStopWaitingFrame(force bool) *frames.StopWaitingFrame
+	ShouldSendRetransmittablePacket() bool
 	DequeuePacketForRetransmission() (packet *Packet)
 	GetLeastUnacked() protocol.PacketNumber
 

ackhandler/sent_packet_handler.go

@@ -40,6 +40,8 @@ type sentPacketHandler struct {
 	lastSentPacketNumber protocol.PacketNumber
 	skippedPackets       []protocol.PacketNumber
 
+	numNonRetransmittablePackets int // number of non-retransmittable packets since the last retransmittable packet
+
 	LargestAcked protocol.PacketNumber
 
 	largestReceivedPacketWithAck protocol.PacketNumber
@@ -89,6 +91,10 @@ func (h *sentPacketHandler) largestInOrderAcked() protocol.PacketNumber {
 	return h.LargestAcked
 }
 
+func (h *sentPacketHandler) ShouldSendRetransmittablePacket() bool {
+	return h.numNonRetransmittablePackets >= protocol.MaxNonRetransmittablePackets
+}
+
 func (h *sentPacketHandler) SentPacket(packet *Packet) error {
 	if packet.PacketNumber <= h.lastSentPacketNumber {
 		return errPacketNumberNotIncreasing
@@ -116,6 +122,9 @@ func (h *sentPacketHandler) SentPacket(packet *Packet) error {
 		packet.SendTime = now
 		h.bytesInFlight += packet.Length
 		h.packetHistory.PushBack(*packet)
+		h.numNonRetransmittablePackets = 0
+	} else {
+		h.numNonRetransmittablePackets++
 	}
 
 	h.congestion.OnPacketSent(

ackhandler/sent_packet_handler_test.go

@@ -61,6 +61,10 @@ func retransmittablePacket(num protocol.PacketNumber) *Packet {
 	return &Packet{PacketNumber: num, Length: 1, Frames: []frames.Frame{&frames.PingFrame{}}}
 }
 
+func nonRetransmittablePacket(num protocol.PacketNumber) *Packet {
+	return &Packet{PacketNumber: num, Length: 1, Frames: []frames.Frame{&frames.AckFrame{}}}
+}
+
 var _ = Describe("SentPacketHandler", func() {
 	var (
 		handler     *sentPacketHandler
@@ -222,6 +226,30 @@ var _ = Describe("SentPacketHandler", func() {
 		})
 	})
 
+	Context("forcing retransmittable packets", func() {
+		It("says that every 20th packet should be retransmittable", func() {
+			// send 19 non-retransmittable packets
+			for i := 1; i <= protocol.MaxNonRetransmittablePackets; i++ {
+				Expect(handler.ShouldSendRetransmittablePacket()).To(BeFalse())
+				err := handler.SentPacket(nonRetransmittablePacket(protocol.PacketNumber(i)))
+				Expect(err).ToNot(HaveOccurred())
+			}
+			Expect(handler.ShouldSendRetransmittablePacket()).To(BeTrue())
+		})
+
+		It("resets the counter when a retransmittable packet is sent", func() {
+			// send 19 non-retransmittable packets
+			for i := 1; i <= protocol.MaxNonRetransmittablePackets; i++ {
+				Expect(handler.ShouldSendRetransmittablePacket()).To(BeFalse())
+				err := handler.SentPacket(nonRetransmittablePacket(protocol.PacketNumber(i)))
+				Expect(err).ToNot(HaveOccurred())
+			}
+			err := handler.SentPacket(retransmittablePacket(20))
+			Expect(err).ToNot(HaveOccurred())
+			Expect(handler.ShouldSendRetransmittablePacket()).To(BeFalse())
+		})
+	})
+
 	Context("DoS mitigation", func() {
 		It("checks the size of the packet history, for unacked packets", func() {
 			i := protocol.PacketNumber(1)

protocol/server_parameters.go

@@ -99,6 +99,9 @@ const MaxTrackedReceivedAckRanges = DefaultMaxCongestionWindow
 // MaxPacketsReceivedBeforeAckSend is the number of packets that can be received before an ACK frame is sent
 const MaxPacketsReceivedBeforeAckSend = 20
 
+// MaxNonRetransmittablePackets is the maximum number of non-retransmittable packets that we send in a row
+const MaxNonRetransmittablePackets = 19
+
 // RetransmittablePacketsBeforeAck is the number of retransmittable that an ACK is sent for
 const RetransmittablePacketsBeforeAck = 2
 

session.go

@@ -667,6 +667,10 @@ func (s *session) sendPacket() error {
 				s.packer.QueueControlFrame(swf)
 			}
 		}
+		// add a retransmittable frame
+		if s.sentPacketHandler.ShouldSendRetransmittablePacket() {
+			s.packer.QueueControlFrame(&frames.PingFrame{})
+		}
 		packet, err := s.packer.PackPacket()
 		if err != nil || packet == nil {
 			return err

session_test.go

@@ -60,10 +60,11 @@ func (m *mockUnpacker) Unpack(publicHeaderBinary []byte, hdr *PublicHeader, data
 }
 
 type mockSentPacketHandler struct {
-	retransmissionQueue  []*ackhandler.Packet
+	retransmissionQueue             []*ackhandler.Packet
-	sentPackets          []*ackhandler.Packet
+	sentPackets                     []*ackhandler.Packet
-	congestionLimited    bool
+	congestionLimited               bool
-	requestedStopWaiting bool
+	requestedStopWaiting            bool
+	shouldSendRetransmittablePacket bool
 }
 
 func (h *mockSentPacketHandler) SentPacket(packet *ackhandler.Packet) error {
@@ -79,6 +80,11 @@ func (h *mockSentPacketHandler) GetLeastUnacked() protocol.PacketNumber { return
 func (h *mockSentPacketHandler) GetAlarmTimeout() time.Time             { panic("not implemented") }
 func (h *mockSentPacketHandler) OnAlarm()                               { panic("not implemented") }
 func (h *mockSentPacketHandler) SendingAllowed() bool                   { return !h.congestionLimited }
+func (h *mockSentPacketHandler) ShouldSendRetransmittablePacket() bool {
+	b := h.shouldSendRetransmittablePacket
+	h.shouldSendRetransmittablePacket = false
+	return b
+}
 
 func (h *mockSentPacketHandler) GetStopWaitingFrame(force bool) *frames.StopWaitingFrame {
 	h.requestedStopWaiting = true
@@ -949,6 +955,14 @@ var _ = Describe("Session", func() {
 			Expect(mconn.written[0]).To(ContainSubstring(string([]byte{0x5E, 0x03})))
 		})
 
+		It("sends a retransmittable packet when required by the SentPacketHandler", func() {
+			sess.sentPacketHandler = &mockSentPacketHandler{shouldSendRetransmittablePacket: true}
+			err := sess.sendPacket()
+			Expect(err).ToNot(HaveOccurred())
+			Expect(mconn.written).To(HaveLen(1))
+			Expect(sess.sentPacketHandler.(*mockSentPacketHandler).sentPackets[0].Frames).To(ContainElement(&frames.PingFrame{}))
+		})
+
 		It("sends two WindowUpdate frames", func() {
 			_, err := sess.GetOrOpenStream(5)
 			Expect(err).ToNot(HaveOccurred())