diff --git a/ackhandler/packet.go b/ackhandler/packet.go index 65f4a60f..96eba2b7 100644 --- a/ackhandler/packet.go +++ b/ackhandler/packet.go @@ -19,7 +19,6 @@ type Packet struct { Retransmitted bool // has this Packet ever been retransmitted sendTime time.Time - rtoTime time.Time } // GetStreamFramesForRetransmission gets all the streamframes for retransmission diff --git a/ackhandler/sent_packet_handler.go b/ackhandler/sent_packet_handler.go index b5739298..411e62d6 100644 --- a/ackhandler/sent_packet_handler.go +++ b/ackhandler/sent_packet_handler.go @@ -19,7 +19,7 @@ var ( // ErrMapAccess occurs when a NACK contains invalid NACK ranges ErrMapAccess = qerr.Error(qerr.InvalidAckData, "Packet does not exist in PacketHistory") // ErrTooManyTrackedSentPackets occurs when the sentPacketHandler has to keep track of too many packets - ErrTooManyTrackedSentPackets = errors.New("To many outstanding not-acked and not retransmitted packets.") + ErrTooManyTrackedSentPackets = errors.New("Too many outstanding non-acked and non-retransmitted packets") errAckForUnsentPacket = qerr.Error(qerr.InvalidAckData, "Received ACK for an unsent package") ) @@ -31,6 +31,7 @@ var ( type sentPacketHandler struct { lastSentPacketNumber protocol.PacketNumber lastSentPacketEntropy EntropyAccumulator + lastSentPacketTime time.Time highestInOrderAckedPacketNumber protocol.PacketNumber LargestObserved protocol.PacketNumber LargestObservedEntropy EntropyAccumulator @@ -118,8 +119,8 @@ func (h *sentPacketHandler) SentPacket(packet *Packet) error { return errWrongPacketNumberIncrement } now := time.Now() + h.lastSentPacketTime = now packet.sendTime = now - packet.rtoTime = now.Add(h.getRTO()) if packet.Length == 0 { return errors.New("SentPacketHandler: packet cannot be empty") } @@ -250,7 +251,7 @@ func (h *sentPacketHandler) ReceivedAck(ackFrame *frames.AckFrame) error { } func (h *sentPacketHandler) HasPacketForRetransmission() bool { - h.queuePacketsRTO() + h.maybeQueuePacketsRTO() if len(h.retransmissionQueue) > 0 { return true @@ -290,6 +291,20 @@ func (h *sentPacketHandler) CheckForError() error { return nil } +func (h *sentPacketHandler) maybeQueuePacketsRTO() { + if time.Now().Before(h.TimeOfFirstRTO()) { + return + } + for p := h.highestInOrderAckedPacketNumber + 1; p <= h.lastSentPacketNumber; p++ { + packet := h.packetHistory[p] + if packet != nil && !packet.Retransmitted { + h.queuePacketForRetransmission(packet) + h.congestion.OnRetransmissionTimeout(true) + return + } + } +} + func (h *sentPacketHandler) getRTO() time.Duration { rto := h.congestion.RetransmissionDelay() if rto == 0 { @@ -298,30 +313,9 @@ func (h *sentPacketHandler) getRTO() time.Duration { return utils.MaxDuration(rto, protocol.MinRetransmissionTime) } -func (h *sentPacketHandler) queuePacketsRTO() { - queued := false - now := time.Now() - for _, p := range h.packetHistory { - if p == nil || p.Retransmitted || p.rtoTime.After(now) { - continue - } - h.queuePacketForRetransmission(p) - queued = true - } - if queued { - h.congestion.OnRetransmissionTimeout(true) - } -} - func (h *sentPacketHandler) TimeOfFirstRTO() time.Time { - var min time.Time - for _, p := range h.packetHistory { - if p == nil || p.Retransmitted { - continue - } - if min.IsZero() || min.After(p.rtoTime) { - min = p.rtoTime - } + if h.lastSentPacketTime.IsZero() { + return time.Time{} } - return min + return h.lastSentPacketTime.Add(h.getRTO()) } diff --git a/ackhandler/sent_packet_handler_test.go b/ackhandler/sent_packet_handler_test.go index c4b2951b..ba9224d8 100644 --- a/ackhandler/sent_packet_handler_test.go +++ b/ackhandler/sent_packet_handler_test.go @@ -6,7 +6,6 @@ import ( "github.com/lucas-clemente/quic-go/congestion" "github.com/lucas-clemente/quic-go/frames" "github.com/lucas-clemente/quic-go/protocol" - "github.com/lucas-clemente/quic-go/utils" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" ) @@ -160,6 +159,13 @@ var _ = Describe("SentPacketHandler", func() { Expect(err).ToNot(HaveOccurred()) Expect(handler.packetHistory[1].sendTime.Unix()).To(BeNumerically("~", time.Now().Unix(), 1)) }) + + It("updates the last sent time", func() { + packet := Packet{PacketNumber: 1, Frames: []frames.Frame{&streamFrame}, EntropyBit: true, Length: 1} + err := handler.SentPacket(&packet) + Expect(err).ToNot(HaveOccurred()) + Expect(handler.lastSentPacketTime.Unix()).To(BeNumerically("~", time.Now().Unix(), 1)) + }) }) Context("DOS mitigation", func() { @@ -524,7 +530,7 @@ var _ = Describe("SentPacketHandler", func() { // Simulate protocol.RetransmissionThreshold more NACKs for i := uint8(0); i < protocol.RetransmissionThreshold; i++ { - _, err := handler.nackPacket(1) + _, err = handler.nackPacket(1) Expect(err).ToNot(HaveOccurred()) } Expect(handler.BytesInFlight()).To(Equal(protocol.ByteCount(0))) @@ -612,8 +618,8 @@ var _ = Describe("SentPacketHandler", func() { It("should call OnRetransmissionTimeout", func() { err := handler.SentPacket(&Packet{PacketNumber: 1, Frames: []frames.Frame{}, Length: 1}) Expect(err).NotTo(HaveOccurred()) - handler.packetHistory[1].rtoTime = time.Now().Add(-time.Second) - handler.queuePacketsRTO() + handler.lastSentPacketTime = time.Now().Add(-time.Second) + handler.maybeQueuePacketsRTO() Expect(cong.onRetransmissionTimeout).To(BeTrue()) }) }) @@ -635,13 +641,6 @@ var _ = Describe("SentPacketHandler", func() { handler.rttStats.UpdateRTT(rtt, 0, time.Now()) Expect(handler.getRTO()).To(Equal(protocol.MinRetransmissionTime)) }) - - It("stores RTO in sent packets", func() { - handler.SentPacket(&Packet{PacketNumber: 1, Frames: []frames.Frame{}, Length: 1}) - val := handler.packetHistory[1].rtoTime - expected := time.Now().Add(protocol.DefaultRetransmissionTime) - Expect(utils.AbsDuration(expected.Sub(val))).To(BeNumerically("<", time.Millisecond)) - }) }) Context("RTO retransmission", func() { @@ -658,7 +657,7 @@ var _ = Describe("SentPacketHandler", func() { It("ignores nil packets", func() { handler.packetHistory[1] = nil - handler.queuePacketsRTO() + handler.maybeQueuePacketsRTO() Expect(handler.TimeOfFirstRTO().IsZero()).To(BeTrue()) }) }) @@ -667,7 +666,7 @@ var _ = Describe("SentPacketHandler", func() { It("does nothing if not required", func() { err := handler.SentPacket(&Packet{PacketNumber: 1, Frames: []frames.Frame{}, Length: 1}) Expect(err).NotTo(HaveOccurred()) - handler.queuePacketsRTO() + handler.maybeQueuePacketsRTO() Expect(handler.retransmissionQueue).To(BeEmpty()) }) @@ -675,8 +674,8 @@ var _ = Describe("SentPacketHandler", func() { p := &Packet{PacketNumber: 1, Frames: []frames.Frame{}, Length: 1} err := handler.SentPacket(p) Expect(err).NotTo(HaveOccurred()) - handler.packetHistory[1].rtoTime = time.Now().Add(-time.Second) - handler.queuePacketsRTO() + handler.lastSentPacketTime = time.Now().Add(-time.Second) + handler.maybeQueuePacketsRTO() Expect(handler.retransmissionQueue).To(HaveLen(1)) Expect(handler.retransmissionQueue[0]).To(Equal(p)) }) @@ -685,14 +684,14 @@ var _ = Describe("SentPacketHandler", func() { p := &Packet{PacketNumber: 1, Frames: []frames.Frame{}, Length: 1, Retransmitted: true} err := handler.SentPacket(p) Expect(err).NotTo(HaveOccurred()) - handler.packetHistory[1].rtoTime = time.Now().Add(-time.Second) - handler.queuePacketsRTO() + handler.lastSentPacketTime = time.Now().Add(-time.Second) + handler.maybeQueuePacketsRTO() Expect(handler.retransmissionQueue).To(BeEmpty()) }) It("ignores nil packets", func() { handler.packetHistory[1] = nil - handler.queuePacketsRTO() + handler.maybeQueuePacketsRTO() Expect(handler.retransmissionQueue).To(BeEmpty()) }) }) @@ -701,7 +700,7 @@ var _ = Describe("SentPacketHandler", func() { p := &Packet{PacketNumber: 1, Frames: []frames.Frame{}, Length: 1} err := handler.SentPacket(p) Expect(err).NotTo(HaveOccurred()) - handler.packetHistory[1].rtoTime = time.Now().Add(-time.Second) + handler.lastSentPacketTime = time.Now().Add(-time.Second) Expect(handler.HasPacketForRetransmission()).To(BeTrue()) }) @@ -709,7 +708,7 @@ var _ = Describe("SentPacketHandler", func() { p := &Packet{PacketNumber: 1, Frames: []frames.Frame{}, Length: 1} err := handler.SentPacket(p) Expect(err).NotTo(HaveOccurred()) - handler.packetHistory[1].rtoTime = time.Now().Add(-time.Second) + handler.lastSentPacketTime = time.Now().Add(-time.Second) Expect(handler.DequeuePacketForRetransmission()).To(Equal(p)) }) })