From 497c57d54a296be60826fc4da55aca068618ca49 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Mon, 27 Jun 2016 17:18:44 +0700 Subject: [PATCH] use ReceivedPacketHistory to get ACK ranges in new ReceivedPacketHandler fixes #190 --- ackhandlernew/received_packet_handler.go | 41 ++----- ackhandlernew/received_packet_handler_test.go | 109 +++--------------- 2 files changed, 28 insertions(+), 122 deletions(-) diff --git a/ackhandlernew/received_packet_handler.go b/ackhandlernew/received_packet_handler.go index 9ec625d5..b370e171 100644 --- a/ackhandlernew/received_packet_handler.go +++ b/ackhandlernew/received_packet_handler.go @@ -23,6 +23,7 @@ type receivedPacketHandler struct { currentAckFrame *frames.AckFrameNew stateChanged bool // has an ACK for this state already been sent? Will be set to false every time a new packet arrives, and to false every time an ACK is sent + packetHistory *receivedPacketHistory receivedTimes map[protocol.PacketNumber]time.Time lowestInReceivedTimes protocol.PacketNumber } @@ -31,6 +32,7 @@ type receivedPacketHandler struct { func NewReceivedPacketHandler() ReceivedPacketHandler { return &receivedPacketHandler{ receivedTimes: make(map[protocol.PacketNumber]time.Time), + packetHistory: newReceivedPacketHistory(), } } @@ -43,6 +45,8 @@ func (h *receivedPacketHandler) ReceivedPacket(packetNumber protocol.PacketNumbe return ErrDuplicatePacket } + h.packetHistory.ReceivedPacket(packetNumber) + h.stateChanged = true h.currentAckFrame = nil @@ -50,10 +54,9 @@ func (h *receivedPacketHandler) ReceivedPacket(packetNumber protocol.PacketNumbe h.largestObserved = packetNumber } - // TODO: figure out when to increase this value - // if packetNumber == h.highestInOrderObserved+1 { - // h.highestInOrderObserved = packetNumber - // } + if packetNumber == h.highestInOrderObserved+1 { + h.highestInOrderObserved = packetNumber + } h.receivedTimes[packetNumber] = time.Now() @@ -75,37 +78,13 @@ func (h *receivedPacketHandler) ReceivedStopWaiting(f *frames.StopWaitingFrame) // the LeastUnacked is the smallest packet number of any packet for which the sender is still awaiting an ack. So the highestInOrderObserved is one less than that h.highestInOrderObserved = f.LeastUnacked - 1 + h.packetHistory.DeleteBelow(f.LeastUnacked) + h.garbageCollect() return nil } -// getNackRanges gets all the ACK ranges -func (h *receivedPacketHandler) getAckRanges() []frames.AckRange { - // TODO: use a better data structure here - var ranges []frames.AckRange - inRange := false - - for i := h.largestObserved; i > h.highestInOrderObserved; i-- { - _, ok := h.receivedTimes[i] - if ok { - if !inRange { - r := frames.AckRange{ - FirstPacketNumber: i, - LastPacketNumber: i, - } - ranges = append(ranges, r) - inRange = true - } else { - ranges[len(ranges)-1].FirstPacketNumber-- - } - } else { - inRange = false - } - } - return ranges -} - func (h *receivedPacketHandler) GetAckFrame(dequeue bool) (*frames.AckFrameNew, error) { if !h.stateChanged { return nil, nil @@ -124,7 +103,7 @@ func (h *receivedPacketHandler) GetAckFrame(dequeue bool) (*frames.AckFrameNew, return nil, ErrMapAccess } - ackRanges := h.getAckRanges() + ackRanges := h.packetHistory.GetAckRanges() h.currentAckFrame = &frames.AckFrameNew{ LargestAcked: h.largestObserved, LowestAcked: ackRanges[len(ackRanges)-1].FirstPacketNumber, diff --git a/ackhandlernew/received_packet_handler_test.go b/ackhandlernew/received_packet_handler_test.go index 527c35a5..b3d23b02 100644 --- a/ackhandlernew/received_packet_handler_test.go +++ b/ackhandlernew/received_packet_handler_test.go @@ -73,86 +73,6 @@ var _ = Describe("receivedPacketHandler", func() { }) }) - Context("ACK range calculation", func() { - It("Returns one ACK range for continously received packets", func() { - for i := 1; i < 100; i++ { - err := handler.ReceivedPacket(protocol.PacketNumber(i)) - Expect(err).ToNot(HaveOccurred()) - } - Expect(handler.largestObserved).To(Equal(protocol.PacketNumber(99))) - // Expect(handler.highestInOrderObserved).To(Equal(protocol.PacketNumber(99))) - ackRanges := handler.getAckRanges() - Expect(ackRanges).To(HaveLen(1)) - Expect(ackRanges[0]).To(Equal(frames.AckRange{FirstPacketNumber: 1, LastPacketNumber: 99})) - }) - - It("handles a single lost package", func() { - for i := 1; i < 10; i++ { - if i == 5 { - continue - } - err := handler.ReceivedPacket(protocol.PacketNumber(i)) - Expect(err).ToNot(HaveOccurred()) - } - Expect(handler.largestObserved).To(Equal(protocol.PacketNumber(9))) - ackRanges := handler.getAckRanges() - Expect(ackRanges).To(HaveLen(2)) - Expect(ackRanges[0]).To(Equal(frames.AckRange{FirstPacketNumber: 6, LastPacketNumber: 9})) - Expect(ackRanges[1]).To(Equal(frames.AckRange{FirstPacketNumber: 1, LastPacketNumber: 4})) - // Expect(handler.highestInOrderObserved).To(Equal(protocol.PacketNumber(4))) - }) - - It("handles two consecutive lost packages", func() { - for i := 1; i < 12; i++ { - if i == 5 || i == 6 { - continue - } - err := handler.ReceivedPacket(protocol.PacketNumber(i)) - Expect(err).ToNot(HaveOccurred()) - } - Expect(handler.largestObserved).To(Equal(protocol.PacketNumber(11))) - ackRanges := handler.getAckRanges() - Expect(ackRanges).To(HaveLen(2)) - Expect(ackRanges[0]).To(Equal(frames.AckRange{FirstPacketNumber: 7, LastPacketNumber: 11})) - Expect(ackRanges[1]).To(Equal(frames.AckRange{FirstPacketNumber: 1, LastPacketNumber: 4})) - // Expect(handler.highestInOrderObserved).To(Equal(protocol.PacketNumber(4))) - }) - - It("handles two non-consecutively lost packages", func() { - for i := 1; i < 10; i++ { - if i == 3 || i == 7 { - continue - } - err := handler.ReceivedPacket(protocol.PacketNumber(i)) - Expect(err).ToNot(HaveOccurred()) - } - Expect(handler.largestObserved).To(Equal(protocol.PacketNumber(9))) - ackRanges := handler.getAckRanges() - Expect(ackRanges).To(HaveLen(3)) - Expect(ackRanges[0]).To(Equal(frames.AckRange{FirstPacketNumber: 8, LastPacketNumber: 9})) - Expect(ackRanges[1]).To(Equal(frames.AckRange{FirstPacketNumber: 4, LastPacketNumber: 6})) - Expect(ackRanges[2]).To(Equal(frames.AckRange{FirstPacketNumber: 1, LastPacketNumber: 2})) - // Expect(handler.highestInOrderObserved).To(Equal(protocol.PacketNumber(2))) - }) - - It("handles two sequences of lost packages", func() { - for i := 1; i < 15; i++ { - if i == 2 || i == 3 || i == 4 || i == 7 || i == 8 { - continue - } - err := handler.ReceivedPacket(protocol.PacketNumber(i)) - Expect(err).ToNot(HaveOccurred()) - } - Expect(handler.largestObserved).To(Equal(protocol.PacketNumber(14))) - ackRanges := handler.getAckRanges() - Expect(ackRanges).To(HaveLen(3)) - Expect(ackRanges[0]).To(Equal(frames.AckRange{FirstPacketNumber: 9, LastPacketNumber: 14})) - Expect(ackRanges[1]).To(Equal(frames.AckRange{FirstPacketNumber: 5, LastPacketNumber: 6})) - Expect(ackRanges[2]).To(Equal(frames.AckRange{FirstPacketNumber: 1, LastPacketNumber: 1})) - // Expect(handler.highestInOrderObserved).To(Equal(protocol.PacketNumber(1))) - }) - }) - Context("handling STOP_WAITING frames", func() { It("increases the highestInOrderObserved packet number", func() { // We simulate 20 packets, numbers 10, 11 and 12 lost @@ -167,17 +87,6 @@ var _ = Describe("receivedPacketHandler", func() { Expect(err).ToNot(HaveOccurred()) Expect(handler.highestInOrderObserved).To(Equal(protocol.PacketNumber(11))) }) - - It("does not emit ACK ranges after STOP_WAITING", func() { - err := handler.ReceivedPacket(10) - Expect(err).ToNot(HaveOccurred()) - ranges := handler.getAckRanges() - Expect(ranges).To(HaveLen(1)) - err = handler.ReceivedStopWaiting(&frames.StopWaitingFrame{LeastUnacked: protocol.PacketNumber(10)}) - Expect(err).ToNot(HaveOccurred()) - ranges = handler.getAckRanges() - Expect(ranges).To(HaveLen(1)) - }) }) Context("ACK package generation", func() { @@ -273,6 +182,24 @@ var _ = Describe("receivedPacketHandler", func() { Expect(ack).ToNot(BeNil()) Expect(ack.AckRanges).To(BeEmpty()) }) + + It("does send old ACK ranges after receiving a StopWaiting", func() { + err := handler.ReceivedPacket(5) + Expect(err).ToNot(HaveOccurred()) + err = handler.ReceivedPacket(10) + Expect(err).ToNot(HaveOccurred()) + err = handler.ReceivedPacket(11) + Expect(err).ToNot(HaveOccurred()) + err = handler.ReceivedPacket(12) + Expect(err).ToNot(HaveOccurred()) + err = handler.ReceivedStopWaiting(&frames.StopWaitingFrame{LeastUnacked: protocol.PacketNumber(11)}) + Expect(err).ToNot(HaveOccurred()) + ack, _ := handler.GetAckFrame(true) + Expect(ack).ToNot(BeNil()) + Expect(ack.LargestAcked).To(Equal(protocol.PacketNumber(12))) + Expect(ack.LowestAcked).To(Equal(protocol.PacketNumber(11))) + Expect(ack.HasMissingRanges()).To(BeFalse()) + }) }) Context("Garbage Collector", func() {