From 0e4b25958ff62c22745f954727c9d87b3176d8b5 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Thu, 8 Aug 2019 15:47:51 +0700 Subject: [PATCH] use early retransmit for crypto packets --- internal/ackhandler/sent_packet_handler.go | 12 +++--- .../ackhandler/sent_packet_handler_test.go | 42 +++++++++++-------- 2 files changed, 30 insertions(+), 24 deletions(-) diff --git a/internal/ackhandler/sent_packet_handler.go b/internal/ackhandler/sent_packet_handler.go index d1a336f4..242cc182 100644 --- a/internal/ackhandler/sent_packet_handler.go +++ b/internal/ackhandler/sent_packet_handler.go @@ -317,11 +317,11 @@ func (h *sentPacketHandler) getEarliestLossTime() (time.Time, protocol.Encryptio lossTime = h.initialPackets.lossTime encLevel = protocol.EncryptionInitial } - if h.handshakePackets != nil && (lossTime.IsZero() || h.handshakePackets.lossTime.Before(lossTime)) { + if h.handshakePackets != nil && (lossTime.IsZero() || (!h.handshakePackets.lossTime.IsZero() && h.handshakePackets.lossTime.Before(lossTime))) { lossTime = h.handshakePackets.lossTime encLevel = protocol.EncryptionHandshake } - if lossTime.IsZero() || h.oneRTTPackets.lossTime.Before(lossTime) { + if lossTime.IsZero() || (!h.oneRTTPackets.lossTime.IsZero() && h.oneRTTPackets.lossTime.Before(lossTime)) { lossTime = h.oneRTTPackets.lossTime encLevel = protocol.Encryption1RTT } @@ -382,11 +382,11 @@ func (h *sentPacketHandler) detectLostPackets( timeSinceSent := now.Sub(packet.SendTime) if timeSinceSent > lossDelay { lostPackets = append(lostPackets, packet) - } else if pnSpace.lossTime.IsZero() && encLevel == protocol.Encryption1RTT { - if h.logger.Debug() { - h.logger.Debugf("\tsetting loss timer for packet %#x to %s (in %s)", packet.PacketNumber, lossDelay, lossDelay-timeSinceSent) - } + } else if pnSpace.lossTime.IsZero() { // Note: This conditional is only entered once per call + if h.logger.Debug() { + h.logger.Debugf("\tsetting loss timer for packet %#x (%s) to %s (in %s)", packet.PacketNumber, encLevel, lossDelay, lossDelay-timeSinceSent) + } pnSpace.lossTime = now.Add(lossDelay - timeSinceSent) } return true, nil diff --git a/internal/ackhandler/sent_packet_handler_test.go b/internal/ackhandler/sent_packet_handler_test.go index 39a05a0d..c43391cc 100644 --- a/internal/ackhandler/sent_packet_handler_test.go +++ b/internal/ackhandler/sent_packet_handler_test.go @@ -795,26 +795,11 @@ var _ = Describe("SentPacketHandler", func() { Expect(handler.bytesInFlight).To(BeZero()) }) - It("uses early retransmit for crypto packets", func() { - now := time.Now() - handler.SentPacket(cryptoPacket(&Packet{PacketNumber: 1, SendTime: now.Add(-time.Hour)})) - handler.SentPacket(cryptoPacket(&Packet{PacketNumber: 2, SendTime: now.Add(-time.Second)})) - Expect(handler.oneRTTPackets.lossTime.IsZero()).To(BeTrue()) - - ack := &wire.AckFrame{AckRanges: []wire.AckRange{{Smallest: 2, Largest: 2}}} - Expect(handler.ReceivedAck(ack, 1, protocol.EncryptionInitial, now)).To(Succeed()) - Expect(handler.DequeuePacketForRetransmission()).ToNot(BeNil()) - Expect(handler.DequeuePacketForRetransmission()).To(BeNil()) - // no need to set an alarm, since packet 1 was already declared lost - Expect(handler.initialPackets.lossTime.IsZero()).To(BeTrue()) - Expect(handler.bytesInFlight).To(BeZero()) - }) - It("sets the early retransmit alarm", func() { now := time.Now() - handler.SentPacket(ackElicitingPacket(&Packet{PacketNumber: 1, SendTime: now.Add(-2 * time.Second), EncryptionLevel: protocol.Encryption1RTT})) - handler.SentPacket(ackElicitingPacket(&Packet{PacketNumber: 2, SendTime: now.Add(-2 * time.Second), EncryptionLevel: protocol.Encryption1RTT})) - handler.SentPacket(ackElicitingPacket(&Packet{PacketNumber: 3, SendTime: now.Add(-time.Second), EncryptionLevel: protocol.Encryption1RTT})) + handler.SentPacket(ackElicitingPacket(&Packet{PacketNumber: 1, SendTime: now.Add(-2 * time.Second)})) + handler.SentPacket(ackElicitingPacket(&Packet{PacketNumber: 2, SendTime: now.Add(-2 * time.Second)})) + handler.SentPacket(ackElicitingPacket(&Packet{PacketNumber: 3, SendTime: now.Add(-time.Second)})) Expect(handler.oneRTTPackets.lossTime.IsZero()).To(BeTrue()) ack := &wire.AckFrame{AckRanges: []wire.AckRange{{Smallest: 2, Largest: 2}}} @@ -830,6 +815,27 @@ var _ = Describe("SentPacketHandler", func() { // make sure this is not an RTO: only packet 1 is retransmissted Expect(handler.DequeuePacketForRetransmission()).To(BeNil()) }) + + It("sets the early retransmit alarm for crypto packets", func() { + now := time.Now() + handler.SentPacket(cryptoPacket(&Packet{PacketNumber: 1, SendTime: now.Add(-2 * time.Second)})) + handler.SentPacket(cryptoPacket(&Packet{PacketNumber: 2, SendTime: now.Add(-2 * time.Second)})) + handler.SentPacket(cryptoPacket(&Packet{PacketNumber: 3, SendTime: now.Add(-time.Second)})) + Expect(handler.initialPackets.lossTime.IsZero()).To(BeTrue()) + + ack := &wire.AckFrame{AckRanges: []wire.AckRange{{Smallest: 2, Largest: 2}}} + Expect(handler.ReceivedAck(ack, 1, protocol.EncryptionInitial, now.Add(-time.Second))).To(Succeed()) + Expect(handler.rttStats.SmoothedRTT()).To(Equal(time.Second)) + + // Packet 1 should be considered lost (1+1/8) RTTs after it was sent. + Expect(handler.initialPackets.lossTime.IsZero()).To(BeFalse()) + Expect(handler.initialPackets.lossTime.Sub(getPacket(1, protocol.EncryptionInitial).SendTime)).To(Equal(time.Second * 9 / 8)) + + Expect(handler.OnLossDetectionTimeout()).To(Succeed()) + Expect(handler.DequeuePacketForRetransmission()).NotTo(BeNil()) + // make sure this is not an RTO: only packet 1 is retransmissted + Expect(handler.DequeuePacketForRetransmission()).To(BeNil()) + }) }) Context("crypto packets", func() {