From f90029ef64ece9edaeb348b78821e0db8cc69e21 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Mon, 4 Dec 2017 11:42:16 +0700 Subject: [PATCH] change ReceivedPacketHandler such that it can generate ACKs for packet number 0 --- ackhandler/interfaces.go | 2 +- ackhandler/received_packet_handler.go | 14 +++++++------- ackhandler/received_packet_handler_test.go | 8 ++++---- ackhandler/received_packet_history.go | 12 ++++++------ ackhandler/received_packet_history_test.go | 14 +++++++------- session.go | 4 +--- session_test.go | 2 +- 7 files changed, 27 insertions(+), 29 deletions(-) diff --git a/ackhandler/interfaces.go b/ackhandler/interfaces.go index 8492fd4c4..7b68faadd 100644 --- a/ackhandler/interfaces.go +++ b/ackhandler/interfaces.go @@ -27,7 +27,7 @@ type SentPacketHandler interface { // ReceivedPacketHandler handles ACKs needed to send for incoming packets type ReceivedPacketHandler interface { ReceivedPacket(packetNumber protocol.PacketNumber, shouldInstigateAck bool) error - SetLowerLimit(protocol.PacketNumber) + IgnoreBelow(protocol.PacketNumber) GetAlarmTimeout() time.Time GetAckFrame() *wire.AckFrame diff --git a/ackhandler/received_packet_handler.go b/ackhandler/received_packet_handler.go index 2ef9c1972..b54e15a02 100644 --- a/ackhandler/received_packet_handler.go +++ b/ackhandler/received_packet_handler.go @@ -12,7 +12,7 @@ var errInvalidPacketNumber = errors.New("ReceivedPacketHandler: Invalid packet n type receivedPacketHandler struct { largestObserved protocol.PacketNumber - lowerLimit protocol.PacketNumber + ignoreBelow protocol.PacketNumber largestObservedReceivedTime time.Time packetHistory *receivedPacketHistory @@ -47,7 +47,7 @@ func (h *receivedPacketHandler) ReceivedPacket(packetNumber protocol.PacketNumbe h.largestObservedReceivedTime = time.Now() } - if packetNumber <= h.lowerLimit { + if packetNumber < h.ignoreBelow { return nil } @@ -58,11 +58,11 @@ func (h *receivedPacketHandler) ReceivedPacket(packetNumber protocol.PacketNumbe return nil } -// SetLowerLimit sets a lower limit for acking packets. -// Packets with packet numbers smaller or equal than p will not be acked. -func (h *receivedPacketHandler) SetLowerLimit(p protocol.PacketNumber) { - h.lowerLimit = p - h.packetHistory.DeleteUpTo(p) +// IgnoreBelow sets a lower limit for acking packets. +// Packets with packet numbers smaller than p will not be acked. +func (h *receivedPacketHandler) IgnoreBelow(p protocol.PacketNumber) { + h.ignoreBelow = p + h.packetHistory.DeleteBelow(p) } func (h *receivedPacketHandler) maybeQueueAck(packetNumber protocol.PacketNumber, shouldInstigateAck bool) { diff --git a/ackhandler/received_packet_handler_test.go b/ackhandler/received_packet_handler_test.go index 88e5d67b9..c1d7ecd59 100644 --- a/ackhandler/received_packet_handler_test.go +++ b/ackhandler/received_packet_handler_test.go @@ -202,13 +202,13 @@ var _ = Describe("receivedPacketHandler", func() { }) It("accepts packets below the lower limit", func() { - handler.SetLowerLimit(5) + handler.IgnoreBelow(6) err := handler.ReceivedPacket(2, true) Expect(err).ToNot(HaveOccurred()) }) It("doesn't add delayed packets to the packetHistory", func() { - handler.SetLowerLimit(6) + handler.IgnoreBelow(7) err := handler.ReceivedPacket(4, true) Expect(err).ToNot(HaveOccurred()) err = handler.ReceivedPacket(10, true) @@ -224,7 +224,7 @@ var _ = Describe("receivedPacketHandler", func() { err := handler.ReceivedPacket(protocol.PacketNumber(i), true) Expect(err).ToNot(HaveOccurred()) } - handler.SetLowerLimit(6) + handler.IgnoreBelow(7) // check that the packets were deleted from the receivedPacketHistory by checking the values in an ACK frame ack := handler.GetAckFrame() Expect(ack).ToNot(BeNil()) @@ -235,7 +235,7 @@ var _ = Describe("receivedPacketHandler", func() { // TODO: remove this test when dropping support for STOP_WAITINGs It("handles a lower limit of 0", func() { - handler.SetLowerLimit(0) + handler.IgnoreBelow(0) err := handler.ReceivedPacket(1337, true) Expect(err).ToNot(HaveOccurred()) ack := handler.GetAckFrame() diff --git a/ackhandler/received_packet_history.go b/ackhandler/received_packet_history.go index 14bdfd540..b8321f607 100644 --- a/ackhandler/received_packet_history.go +++ b/ackhandler/received_packet_history.go @@ -74,17 +74,17 @@ func (h *receivedPacketHistory) ReceivedPacket(p protocol.PacketNumber) error { return nil } -// DeleteUpTo deletes all entries up to (and including) p -func (h *receivedPacketHistory) DeleteUpTo(p protocol.PacketNumber) { - h.lowestInReceivedPacketNumbers = utils.MaxPacketNumber(h.lowestInReceivedPacketNumbers, p+1) +// DeleteBelow deletes all entries below (but not including) p +func (h *receivedPacketHistory) DeleteBelow(p protocol.PacketNumber) { + h.lowestInReceivedPacketNumbers = utils.MaxPacketNumber(h.lowestInReceivedPacketNumbers, p) nextEl := h.ranges.Front() for el := h.ranges.Front(); nextEl != nil; el = nextEl { nextEl = el.Next() - if p >= el.Value.Start && p < el.Value.End { - el.Value.Start = p + 1 - } else if el.Value.End <= p { // delete a whole range + if p > el.Value.Start && p <= el.Value.End { + el.Value.Start = p + } else if el.Value.End < p { // delete a whole range h.ranges.Remove(el) } else { // no ranges affected. Nothing to do return diff --git a/ackhandler/received_packet_history_test.go b/ackhandler/received_packet_history_test.go index bca058159..ed9789b3d 100644 --- a/ackhandler/received_packet_history_test.go +++ b/ackhandler/received_packet_history_test.go @@ -124,7 +124,7 @@ var _ = Describe("receivedPacketHistory", func() { Context("deleting", func() { It("does nothing when the history is empty", func() { - hist.DeleteUpTo(5) + hist.DeleteBelow(5) Expect(hist.ranges.Len()).To(BeZero()) }) @@ -132,7 +132,7 @@ var _ = Describe("receivedPacketHistory", func() { hist.ReceivedPacket(4) hist.ReceivedPacket(5) hist.ReceivedPacket(10) - hist.DeleteUpTo(5) + hist.DeleteBelow(6) Expect(hist.ranges.Len()).To(Equal(1)) Expect(hist.ranges.Front().Value).To(Equal(utils.PacketInterval{Start: 10, End: 10})) }) @@ -141,7 +141,7 @@ var _ = Describe("receivedPacketHistory", func() { hist.ReceivedPacket(1) hist.ReceivedPacket(5) hist.ReceivedPacket(10) - hist.DeleteUpTo(8) + hist.DeleteBelow(8) Expect(hist.ranges.Len()).To(Equal(1)) Expect(hist.ranges.Front().Value).To(Equal(utils.PacketInterval{Start: 10, End: 10})) }) @@ -152,7 +152,7 @@ var _ = Describe("receivedPacketHistory", func() { hist.ReceivedPacket(5) hist.ReceivedPacket(6) hist.ReceivedPacket(7) - hist.DeleteUpTo(4) + hist.DeleteBelow(5) Expect(hist.ranges.Len()).To(Equal(1)) Expect(hist.ranges.Front().Value).To(Equal(utils.PacketInterval{Start: 5, End: 7})) }) @@ -161,7 +161,7 @@ var _ = Describe("receivedPacketHistory", func() { hist.ReceivedPacket(4) hist.ReceivedPacket(5) hist.ReceivedPacket(10) - hist.DeleteUpTo(4) + hist.DeleteBelow(5) Expect(hist.ranges.Len()).To(Equal(2)) Expect(hist.ranges.Front().Value).To(Equal(utils.PacketInterval{Start: 5, End: 5})) Expect(hist.ranges.Back().Value).To(Equal(utils.PacketInterval{Start: 10, End: 10})) @@ -169,7 +169,7 @@ var _ = Describe("receivedPacketHistory", func() { It("keeps a one-packet range, if deleting up to the packet directly below", func() { hist.ReceivedPacket(4) - hist.DeleteUpTo(3) + hist.DeleteBelow(4) Expect(hist.ranges.Len()).To(Equal(1)) Expect(hist.ranges.Front().Value).To(Equal(utils.PacketInterval{Start: 4, End: 4})) }) @@ -191,7 +191,7 @@ var _ = Describe("receivedPacketHistory", func() { } err := hist.ReceivedPacket(2*protocol.MaxTrackedReceivedAckRanges + 2) Expect(err).To(MatchError(errTooManyOutstandingReceivedAckRanges)) - hist.DeleteUpTo(protocol.MaxTrackedReceivedAckRanges) // deletes about half of the ranges + hist.DeleteBelow(protocol.MaxTrackedReceivedAckRanges) // deletes about half of the ranges err = hist.ReceivedPacket(2*protocol.MaxTrackedReceivedAckRanges + 4) Expect(err).ToNot(HaveOccurred()) }) diff --git a/session.go b/session.go index a163f7eea..2fb390161 100644 --- a/session.go +++ b/session.go @@ -478,9 +478,7 @@ func (s *session) handleFrames(fs []wire.Frame, encLevel protocol.EncryptionLeve case *wire.GoawayFrame: err = errors.New("unimplemented: handling GOAWAY frames") case *wire.StopWaitingFrame: - // LeastUnacked is guaranteed to have LeastUnacked > 0 - // therefore this will never underflow - s.receivedPacketHandler.SetLowerLimit(frame.LeastUnacked - 1) + s.receivedPacketHandler.IgnoreBelow(frame.LeastUnacked) case *wire.RstStreamFrame: err = s.handleRstStreamFrame(frame) case *wire.MaxDataFrame: diff --git a/session_test.go b/session_test.go index e53e1b702..5b83dd377 100644 --- a/session_test.go +++ b/session_test.go @@ -130,7 +130,7 @@ func (m *mockReceivedPacketHandler) GetAckFrame() *wire.AckFrame { func (m *mockReceivedPacketHandler) ReceivedPacket(packetNumber protocol.PacketNumber, shouldInstigateAck bool) error { panic("not implemented") } -func (m *mockReceivedPacketHandler) SetLowerLimit(protocol.PacketNumber) { +func (m *mockReceivedPacketHandler) IgnoreBelow(protocol.PacketNumber) { panic("not implemented") } func (m *mockReceivedPacketHandler) GetAlarmTimeout() time.Time { return m.ackAlarm }