From a0b7e468ff89796220b962a4bf09b82114539443 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Tue, 30 Jul 2019 11:02:52 +0700 Subject: [PATCH 1/3] optimize deleting of ACK ranges --- internal/ackhandler/received_packet_history.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/internal/ackhandler/received_packet_history.go b/internal/ackhandler/received_packet_history.go index 0daba413d..629beb24e 100644 --- a/internal/ackhandler/received_packet_history.go +++ b/internal/ackhandler/received_packet_history.go @@ -86,10 +86,11 @@ func (h *receivedPacketHistory) DeleteBelow(p protocol.PacketNumber) { 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 - } else if el.Value.End < p { // delete a whole range + if el.Value.End < p { // delete a whole range h.ranges.Remove(el) + } else if p > el.Value.Start && p <= el.Value.End { + el.Value.Start = p + return } else { // no ranges affected. Nothing to do return } From f4bb3c12be761c83b23405be74f4cd876c4d1ea3 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Tue, 30 Jul 2019 11:17:26 +0700 Subject: [PATCH 2/3] remove unneeded tracking variable in the received packet history The value was not updated correctly when receiving reordered packets. Since it's trivial to determine it from the list of received packets, the easiest fix is to remove it. --- internal/ackhandler/received_packet_history.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/internal/ackhandler/received_packet_history.go b/internal/ackhandler/received_packet_history.go index 629beb24e..d449bc51d 100644 --- a/internal/ackhandler/received_packet_history.go +++ b/internal/ackhandler/received_packet_history.go @@ -12,8 +12,6 @@ import ( // It does not store packet contents. type receivedPacketHistory struct { ranges *utils.PacketIntervalList - - lowestInReceivedPacketNumbers protocol.PacketNumber } var errTooManyOutstandingReceivedAckRanges = qerr.Error(qerr.InternalError, "Too many outstanding received ACK ranges") @@ -77,11 +75,6 @@ func (h *receivedPacketHistory) ReceivedPacket(p protocol.PacketNumber) error { // DeleteBelow deletes all entries below (but not including) p func (h *receivedPacketHistory) DeleteBelow(p protocol.PacketNumber) { - if p <= h.lowestInReceivedPacketNumbers { - return - } - h.lowestInReceivedPacketNumbers = p - nextEl := h.ranges.Front() for el := h.ranges.Front(); nextEl != nil; el = nextEl { nextEl = el.Next() From fb9fafe3b4755cf8c820a07ab538d62ff2b1457b Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Tue, 30 Jul 2019 11:19:16 +0700 Subject: [PATCH 3/3] don't add ACK ranges for delayed packets, if history was already deleted --- internal/ackhandler/received_packet_history.go | 11 +++++++++++ internal/ackhandler/received_packet_history_test.go | 12 ++++++++++++ 2 files changed, 23 insertions(+) diff --git a/internal/ackhandler/received_packet_history.go b/internal/ackhandler/received_packet_history.go index d449bc51d..e75b0b0e2 100644 --- a/internal/ackhandler/received_packet_history.go +++ b/internal/ackhandler/received_packet_history.go @@ -12,6 +12,8 @@ import ( // It does not store packet contents. type receivedPacketHistory struct { ranges *utils.PacketIntervalList + + deletedBelow protocol.PacketNumber } var errTooManyOutstandingReceivedAckRanges = qerr.Error(qerr.InternalError, "Too many outstanding received ACK ranges") @@ -25,6 +27,10 @@ func newReceivedPacketHistory() *receivedPacketHistory { // ReceivedPacket registers a packet with PacketNumber p and updates the ranges func (h *receivedPacketHistory) ReceivedPacket(p protocol.PacketNumber) error { + // ignore delayed packets, if we already deleted the range + if p < h.deletedBelow { + return nil + } if h.ranges.Len() >= protocol.MaxTrackedReceivedAckRanges { return errTooManyOutstandingReceivedAckRanges } @@ -75,6 +81,11 @@ func (h *receivedPacketHistory) ReceivedPacket(p protocol.PacketNumber) error { // DeleteBelow deletes all entries below (but not including) p func (h *receivedPacketHistory) DeleteBelow(p protocol.PacketNumber) { + if p < h.deletedBelow { + return + } + h.deletedBelow = p + nextEl := h.ranges.Front() for el := h.ranges.Front(); nextEl != nil; el = nextEl { nextEl = el.Next() diff --git a/internal/ackhandler/received_packet_history_test.go b/internal/ackhandler/received_packet_history_test.go index 4a24b95c4..db3afb08d 100644 --- a/internal/ackhandler/received_packet_history_test.go +++ b/internal/ackhandler/received_packet_history_test.go @@ -174,6 +174,18 @@ var _ = Describe("receivedPacketHistory", func() { Expect(hist.ranges.Front().Value).To(Equal(utils.PacketInterval{Start: 4, End: 4})) }) + It("doesn't add delayed packets below deleted ranges", func() { + hist.ReceivedPacket(4) + hist.ReceivedPacket(5) + hist.ReceivedPacket(6) + hist.DeleteBelow(5) + Expect(hist.ranges.Len()).To(Equal(1)) + Expect(hist.ranges.Front().Value).To(Equal(utils.PacketInterval{Start: 5, End: 6})) + hist.ReceivedPacket(2) + Expect(hist.ranges.Len()).To(Equal(1)) + Expect(hist.ranges.Front().Value).To(Equal(utils.PacketInterval{Start: 5, End: 6})) + }) + Context("DoS protection", func() { It("doesn't create more than MaxTrackedReceivedAckRanges ranges", func() { for i := protocol.PacketNumber(1); i <= protocol.MaxTrackedReceivedAckRanges; i++ {