From 1420b138d5d94c58fb92680b7b2601c71ac484d3 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Sat, 17 Mar 2018 10:57:43 +0000 Subject: [PATCH] implement TLPs --- internal/ackhandler/send_mode.go | 6 +- internal/ackhandler/send_mode_test.go | 1 + internal/ackhandler/sent_packet_handler.go | 36 +++++- .../ackhandler/sent_packet_handler_test.go | 120 ++++++++++++++---- session.go | 12 ++ session_test.go | 41 ++---- 6 files changed, 157 insertions(+), 59 deletions(-) diff --git a/internal/ackhandler/send_mode.go b/internal/ackhandler/send_mode.go index 61573a47..76c833c4 100644 --- a/internal/ackhandler/send_mode.go +++ b/internal/ackhandler/send_mode.go @@ -14,7 +14,9 @@ const ( SendRetransmission // SendRTO means that an RTO probe packet should be sent SendRTO - // SendAny packet should be sent + // SendTLP means that a TLP probe packet should be sent + SendTLP + // SendAny means that any packet should be sent SendAny ) @@ -28,6 +30,8 @@ func (s SendMode) String() string { return "retransmission" case SendRTO: return "rto" + case SendTLP: + return "tlp" case SendAny: return "any" default: diff --git a/internal/ackhandler/send_mode_test.go b/internal/ackhandler/send_mode_test.go index 9ca9caf8..c267d38c 100644 --- a/internal/ackhandler/send_mode_test.go +++ b/internal/ackhandler/send_mode_test.go @@ -11,6 +11,7 @@ var _ = Describe("Send Mode", func() { Expect(SendAny.String()).To(Equal("any")) Expect(SendAck.String()).To(Equal("ack")) Expect(SendRTO.String()).To(Equal("rto")) + Expect(SendTLP.String()).To(Equal("tlp")) Expect(SendRetransmission.String()).To(Equal("retransmission")) Expect(SendMode(123).String()).To(Equal("invalid send mode: 123")) }) diff --git a/internal/ackhandler/sent_packet_handler.go b/internal/ackhandler/sent_packet_handler.go index e775af05..72911e13 100644 --- a/internal/ackhandler/sent_packet_handler.go +++ b/internal/ackhandler/sent_packet_handler.go @@ -23,6 +23,8 @@ const ( defaultRTOTimeout = 500 * time.Millisecond // Minimum time in the future a tail loss probe alarm may be set for. minTPLTimeout = 10 * time.Millisecond + // Maximum number of tail loss probes before an RTO fires. + maxTLPs = 2 // Minimum time in the future an RTO alarm may be set for. minRTOTimeout = 200 * time.Millisecond // maxRTOTimeout is the maximum RTO time @@ -59,6 +61,10 @@ type sentPacketHandler struct { // The number of times the handshake packets have been retransmitted without receiving an ack. handshakeCount uint32 + // The number of times a TLP has been sent without receiving an ack. + tlpCount uint32 + allowTLP bool + // The number of times an RTO has been sent without receiving an ack. rtoCount uint32 // The number of RTO probe packets that should be sent. @@ -168,6 +174,7 @@ func (h *sentPacketHandler) sentPacketImpl(packet *Packet) bool /* isRetransmitt if h.numRTOs > 0 { h.numRTOs-- } + h.allowTLP = false } h.congestion.OnPacketSent(packet.SendTime, h.bytesInFlight, packet.PacketNumber, packet.Length, isRetransmittable) @@ -288,15 +295,20 @@ func (h *sentPacketHandler) updateLossDetectionAlarm() { return } - // TODO(#497): TLP if !h.handshakeComplete { h.alarm = h.lastSentHandshakePacketTime.Add(h.computeHandshakeTimeout()) } else if !h.lossTime.IsZero() { // Early retransmit timer or time loss detection. h.alarm = h.lossTime } else { - // RTO - h.alarm = h.lastSentRetransmittablePacketTime.Add(h.computeRTOTimeout()) + // RTO or TLP alarm + alarmDuration := h.computeRTOTimeout() + if h.tlpCount < maxTLPs { + tlpAlarm := h.computeTLPTimeout() + // if the RTO duration is shorter than the TLP duration, use the RTO duration + alarmDuration = utils.MinDuration(alarmDuration, tlpAlarm) + } + h.alarm = h.lastSentRetransmittablePacketTime.Add(alarmDuration) } } @@ -343,7 +355,6 @@ func (h *sentPacketHandler) detectLostPackets(now time.Time, priorInFlight proto func (h *sentPacketHandler) OnAlarm() error { now := time.Now() - // TODO(#497): TLP var err error if !h.handshakeComplete { h.handshakeCount++ @@ -351,6 +362,9 @@ func (h *sentPacketHandler) OnAlarm() error { } else if !h.lossTime.IsZero() { // Early retransmit or time loss detection err = h.detectLostPackets(now, h.bytesInFlight) + } else if h.tlpCount < maxTLPs { + h.allowTLP = true + h.tlpCount++ } else { // RTO h.rtoCount++ @@ -407,8 +421,8 @@ func (h *sentPacketHandler) onPacketAcked(p *Packet) error { return err } h.rtoCount = 0 + h.tlpCount = 0 h.handshakeCount = 0 - // TODO(#497): h.tlpCount = 0 return h.packetHistory.Remove(p.PacketNumber) } @@ -469,6 +483,9 @@ func (h *sentPacketHandler) SendMode() SendMode { h.logger.Debugf("Limited by the number of tracked packets: tracking %d packets, maximum %d", numTrackedPackets, protocol.MaxTrackedSentPackets) return SendNone } + if h.allowTLP { + return SendTLP + } if h.numRTOs > 0 { return SendRTO } @@ -561,6 +578,15 @@ func (h *sentPacketHandler) computeHandshakeTimeout() time.Duration { return duration << h.handshakeCount } +func (h *sentPacketHandler) computeTLPTimeout() time.Duration { + // TODO(#1236): include the max_ack_delay + srtt := h.rttStats.SmoothedRTT() + if srtt == 0 { + srtt = defaultInitialRTT + } + return utils.MaxDuration(srtt*3/2, minTPLTimeout) +} + func (h *sentPacketHandler) computeRTOTimeout() time.Duration { var rto time.Duration rtt := h.rttStats.SmoothedRTT() diff --git a/internal/ackhandler/sent_packet_handler_test.go b/internal/ackhandler/sent_packet_handler_test.go index 451b28d4..37df5965 100644 --- a/internal/ackhandler/sent_packet_handler_test.go +++ b/internal/ackhandler/sent_packet_handler_test.go @@ -84,6 +84,11 @@ var _ = Describe("SentPacketHandler", func() { } } + updateRTT := func(rtt time.Duration) { + handler.rttStats.UpdateRTT(rtt, 0, time.Now()) + ExpectWithOffset(1, handler.rttStats.SmoothedRTT()).To(Equal(rtt)) + } + It("determines the packet number length", func() { handler.largestAcked = 0x1337 Expect(handler.GetPacketNumberLen(0x1338)).To(Equal(protocol.PacketNumberLen2)) @@ -225,7 +230,7 @@ var _ = Describe("SentPacketHandler", func() { handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: i})) } // Increase RTT, because the tests would be flaky otherwise - handler.rttStats.UpdateRTT(time.Hour, 0, time.Now()) + updateRTT(time.Hour) Expect(handler.bytesInFlight).To(Equal(protocol.ByteCount(10))) }) @@ -579,12 +584,16 @@ var _ = Describe("SentPacketHandler", func() { }) It("doesn't call OnPacketLost and OnRetransmissionTimeout when queuing RTOs", func() { - cong.EXPECT().OnPacketSent(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Times(3) - cong.EXPECT().TimeUntilSend(gomock.Any()).Times(3) - handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 1})) - handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 2})) - handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 3})) - handler.OnAlarm() // RTO, meaning 2 lost packets + for i := protocol.PacketNumber(1); i < 3; i++ { + cong.EXPECT().OnPacketSent(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()) + cong.EXPECT().TimeUntilSend(gomock.Any()) + handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: i})) + } + handler.OnAlarm() // TLP + Expect(handler.DequeuePacketForRetransmission()).To(BeNil()) + handler.OnAlarm() // TLP + Expect(handler.DequeuePacketForRetransmission()).To(BeNil()) + handler.OnAlarm() // RTO Expect(handler.DequeuePacketForRetransmission()).ToNot(BeNil()) Expect(handler.DequeuePacketForRetransmission()).ToNot(BeNil()) }) @@ -596,7 +605,9 @@ var _ = Describe("SentPacketHandler", func() { handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 2, SendTime: time.Now().Add(-time.Hour)})) handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 3, SendTime: time.Now().Add(-time.Hour)})) handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 4, SendTime: time.Now().Add(-time.Hour)})) - handler.OnAlarm() // RTO, meaning 2 lost packets + handler.OnAlarm() // TLP + handler.OnAlarm() // TLP + handler.OnAlarm() // RTO Expect(handler.DequeuePacketForRetransmission()).ToNot(BeNil()) Expect(handler.DequeuePacketForRetransmission()).ToNot(BeNil()) // send one probe packet and receive an ACK for it @@ -620,7 +631,9 @@ var _ = Describe("SentPacketHandler", func() { cong.EXPECT().TimeUntilSend(gomock.Any()).Times(3) handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 1, SendTime: time.Now().Add(-time.Hour)})) handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 2, SendTime: time.Now()})) - handler.OnAlarm() // RTO, meaning 2 lost packets + handler.OnAlarm() // TLP + handler.OnAlarm() // TLP + handler.OnAlarm() // RTO Expect(handler.DequeuePacketForRetransmission()).ToNot(BeNil()) Expect(handler.DequeuePacketForRetransmission()).ToNot(BeNil()) // send one probe packet @@ -767,6 +780,53 @@ var _ = Describe("SentPacketHandler", func() { }) }) + Context("TLPs", func() { + It("uses the default RTT", func() { + Expect(handler.computeTLPTimeout()).To(Equal(defaultInitialRTT * 3 / 2)) + }) + + It("uses the RTT from RTT stats", func() { + rtt := 2 * time.Second + updateRTT(rtt) + Expect(handler.computeTLPTimeout()).To(Equal(rtt * 3 / 2)) + }) + + It("uses the minTLPTimeout for short RTTs", func() { + rtt := 2 * time.Microsecond + updateRTT(rtt) + Expect(handler.computeTLPTimeout()).To(Equal(minTPLTimeout)) + }) + + It("sets the TLP send mode until one retransmittable packet is sent", func() { + handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 1, SendTime: time.Now().Add(-time.Hour)})) + handler.OnAlarm() + Expect(handler.SendMode()).To(Equal(SendTLP)) + // Send a non-retransmittable packet. + // It doesn't count as a probe packet. + handler.SentPacket(nonRetransmittablePacket(&Packet{PacketNumber: 2})) + Expect(handler.SendMode()).To(Equal(SendTLP)) + // Send a retransmittable packet. + handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 3})) + Expect(handler.SendMode()).ToNot(Equal(SendTLP)) + }) + + It("sends two TLPs, then RTOs", func() { + handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 1, SendTime: time.Now().Add(-time.Hour)})) + handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 2, SendTime: time.Now().Add(-time.Hour)})) + // first TLP + handler.OnAlarm() + Expect(handler.SendMode()).To(Equal(SendTLP)) + handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 3})) + // second TLP + handler.OnAlarm() + Expect(handler.SendMode()).To(Equal(SendTLP)) + handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 4})) + // fire alarm a third time + handler.OnAlarm() + Expect(handler.SendMode()).To(Equal(SendRTO)) + }) + }) + Context("RTOs", func() { It("uses default RTO", func() { Expect(handler.computeRTOTimeout()).To(Equal(defaultRTOTimeout)) @@ -782,14 +842,13 @@ var _ = Describe("SentPacketHandler", func() { }) It("limits RTO min", func() { - rtt := time.Millisecond - handler.rttStats.UpdateRTT(rtt, 0, time.Now()) + rtt := 3 * time.Millisecond + updateRTT(rtt) Expect(handler.computeRTOTimeout()).To(Equal(minRTOTimeout)) }) It("limits RTO max", func() { - rtt := time.Hour - handler.rttStats.UpdateRTT(rtt, 0, time.Now()) + updateRTT(time.Hour) Expect(handler.computeRTOTimeout()).To(Equal(maxRTOTimeout)) }) @@ -806,12 +865,13 @@ var _ = Describe("SentPacketHandler", func() { handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 1})) handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 2})) - handler.rttStats.UpdateRTT(time.Hour, 0, time.Now()) + updateRTT(time.Hour) Expect(handler.lossTime.IsZero()).To(BeTrue()) Expect(time.Until(handler.GetAlarmTimeout())).To(BeNumerically("~", handler.computeRTOTimeout(), time.Minute)) - err := handler.OnAlarm() - Expect(err).ToNot(HaveOccurred()) + handler.OnAlarm() // TLP + handler.OnAlarm() // TLP + handler.OnAlarm() // RTO p := handler.DequeuePacketForRetransmission() Expect(p).ToNot(BeNil()) Expect(p.PacketNumber).To(Equal(protocol.PacketNumber(1))) @@ -827,8 +887,9 @@ var _ = Describe("SentPacketHandler", func() { handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 1, SendTime: time.Now().Add(-time.Hour)})) handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 2, SendTime: time.Now().Add(-time.Hour)})) handler.rttStats.UpdateRTT(time.Second, 0, time.Now()) - err := handler.OnAlarm() - Expect(err).ToNot(HaveOccurred()) + handler.OnAlarm() // TLP + handler.OnAlarm() // TLP + handler.OnAlarm() // RTO Expect(handler.DequeuePacketForRetransmission()).ToNot(BeNil()) Expect(handler.DequeuePacketForRetransmission()).ToNot(BeNil()) expectInPacketHistory([]protocol.PacketNumber{1, 2}) @@ -837,7 +898,7 @@ var _ = Describe("SentPacketHandler", func() { // This verifies the RTO. handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 3})) ack := &wire.AckFrame{AckRanges: []wire.AckRange{{Smallest: 3, Largest: 3}}} - err = handler.ReceivedAck(ack, 1, protocol.EncryptionForwardSecure, time.Now()) + err := handler.ReceivedAck(ack, 1, protocol.EncryptionForwardSecure, time.Now()) Expect(err).ToNot(HaveOccurred()) Expect(handler.packetHistory.Len()).To(BeZero()) Expect(handler.bytesInFlight).To(BeZero()) @@ -846,11 +907,15 @@ var _ = Describe("SentPacketHandler", func() { It("allows sending of two probe packets", func() { handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 1, SendTime: time.Now().Add(-time.Hour)})) - handler.OnAlarm() + handler.OnAlarm() // TLP + handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 2})) // send the first TLP + handler.OnAlarm() // TLP + handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 3})) // send the second TLP + handler.OnAlarm() // RTO Expect(handler.SendMode()).To(Equal(SendRTO)) - handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 2})) + handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 4})) Expect(handler.SendMode()).To(Equal(SendRTO)) - handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 3})) + handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 5})) Expect(handler.SendMode()).ToNot(Equal(SendRTO)) }) @@ -860,7 +925,9 @@ var _ = Describe("SentPacketHandler", func() { handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 3, SendTime: time.Now().Add(-time.Hour)})) handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 4, SendTime: time.Now().Add(-time.Hour)})) handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 5, SendTime: time.Now().Add(-time.Hour)})) - handler.OnAlarm() + handler.OnAlarm() // TLP + handler.OnAlarm() // TLP + handler.OnAlarm() // RTO Expect(handler.DequeuePacketForRetransmission()).ToNot(BeNil()) Expect(handler.DequeuePacketForRetransmission()).ToNot(BeNil()) expectInPacketHistory([]protocol.PacketNumber{1, 2, 3, 4, 5}) @@ -878,12 +945,13 @@ var _ = Describe("SentPacketHandler", func() { It("handles ACKs for the original packet", func() { handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 5, SendTime: time.Now().Add(-time.Hour)})) handler.rttStats.UpdateRTT(time.Second, 0, time.Now()) - err := handler.OnAlarm() - Expect(err).ToNot(HaveOccurred()) + handler.OnAlarm() // TLP + handler.OnAlarm() // TLP + handler.OnAlarm() // RTO Expect(handler.DequeuePacketForRetransmission()).ToNot(BeNil()) handler.SentPacketsAsRetransmission([]*Packet{retransmittablePacket(&Packet{PacketNumber: 6})}, 5) ack := &wire.AckFrame{AckRanges: []wire.AckRange{{Smallest: 5, Largest: 5}}} - err = handler.ReceivedAck(ack, 1, protocol.EncryptionForwardSecure, time.Now()) + err := handler.ReceivedAck(ack, 1, protocol.EncryptionForwardSecure, time.Now()) Expect(err).ToNot(HaveOccurred()) err = handler.OnAlarm() Expect(err).ToNot(HaveOccurred()) diff --git a/session.go b/session.go index 210733c6..8a7668ee 100644 --- a/session.go +++ b/session.go @@ -909,6 +909,18 @@ sendLoop: } } numPacketsSent++ + case ackhandler.SendTLP: + // In TLP mode, a probe packet has to be sent. + // Add a PING frame to make sure a (retransmittable) packet will be sent. + s.queueControlFrame(&wire.PingFrame{}) + sentPacket, err := s.sendPacket() + if err != nil { + return err + } + if !sentPacket { + return errors.New("session BUG: expected a packet to be sent in TLP mode") + } + return nil case ackhandler.SendRetransmission: sentPacket, err := s.maybeSendRetransmission() if err != nil { diff --git a/session_test.go b/session_test.go index c05e30ce..4c6623fb 100644 --- a/session_test.go +++ b/session_test.go @@ -805,6 +805,20 @@ var _ = Describe("Session", func() { Expect(err).ToNot(HaveOccurred()) }) + It("sends a TLP probe packet", func() { + sph := mockackhandler.NewMockSentPacketHandler(mockCtrl) + sph.EXPECT().GetPacketNumberLen(gomock.Any()).Return(protocol.PacketNumberLen2).AnyTimes() + sph.EXPECT().SendMode().Return(ackhandler.SendTLP) + sph.EXPECT().ShouldSendNumPackets().Return(1) + sph.EXPECT().SentPacket(gomock.Any()).Do(func(p *ackhandler.Packet) { + Expect(p.Frames).To(HaveLen(1)) + Expect(p.Frames[0]).To(BeAssignableToTypeOf(&wire.PingFrame{})) + }) + sess.sentPacketHandler = sph + err := sess.sendPackets() + Expect(err).ToNot(HaveOccurred()) + }) + It("sends an RTO probe packets", func() { sph := mockackhandler.NewMockSentPacketHandler(mockCtrl) sph.EXPECT().GetPacketNumberLen(gomock.Any()).Return(protocol.PacketNumberLen2).AnyTimes() @@ -1197,33 +1211,6 @@ var _ = Describe("Session", func() { }) }) - It("retransmits RTO packets", func() { - sess.packer.hasSentPacket = true // make sure this is not the first packet the packer sends - sess.sentPacketHandler.SetHandshakeComplete() - n := protocol.PacketNumber(10) - sess.packer.cryptoSetup = &mockCryptoSetup{encLevelSeal: protocol.EncryptionForwardSecure} - // We simulate consistently low RTTs, so that the test works faster - rtt := time.Millisecond - sess.rttStats.UpdateRTT(rtt, 0, time.Now()) - Expect(sess.rttStats.SmoothedRTT()).To(Equal(rtt)) // make sure it worked - sess.packer.packetNumberGenerator.next = n + 1 - // Now, we send a single packet, and expect that it was retransmitted later - sess.sentPacketHandler.SentPacket(&ackhandler.Packet{ - PacketNumber: n, - Length: 1, - Frames: []wire.Frame{&wire.StreamFrame{ - Data: []byte("foobar"), - }}, - EncryptionLevel: protocol.EncryptionForwardSecure, - }) - go sess.run() - defer sess.Close(nil) - sess.scheduleSending() - Eventually(func() int { return len(mconn.written) }).ShouldNot(BeZero()) - Expect(mconn.written).To(Receive(ContainSubstring("foobar"))) - streamManager.EXPECT().CloseWithError(gomock.Any()) - }) - Context("scheduling sending", func() { BeforeEach(func() { sess.packer.hasSentPacket = true // make sure this is not the first packet the packer sends