diff --git a/ackhandler/received_packet_handler_test.go b/ackhandler/received_packet_handler_test.go index 7e2a1f1a..60103773 100644 --- a/ackhandler/received_packet_handler_test.go +++ b/ackhandler/received_packet_handler_test.go @@ -59,17 +59,6 @@ var _ = Describe("receivedPacketHandler", func() { Expect(handler.largestObservedReceivedTime).To(Equal(timestamp)) }) - It("doesn't store more than MaxTrackedReceivedPackets packets", func() { - err := handler.ReceivedPacket(1, true) - Expect(err).ToNot(HaveOccurred()) - for i := protocol.PacketNumber(3); i < 3+protocol.MaxTrackedReceivedPackets-1; i++ { - err := handler.ReceivedPacket(protocol.PacketNumber(i), true) - Expect(err).ToNot(HaveOccurred()) - } - err = handler.ReceivedPacket(protocol.PacketNumber(protocol.MaxTrackedReceivedPackets)+10, true) - Expect(err).To(MatchError(errTooManyOutstandingReceivedPackets)) - }) - It("passes on errors from receivedPacketHistory", func() { var err error for i := protocol.PacketNumber(0); i < 5*protocol.MaxTrackedReceivedAckRanges; i++ { diff --git a/ackhandler/received_packet_history.go b/ackhandler/received_packet_history.go index d8e0880b..11c8f418 100644 --- a/ackhandler/received_packet_history.go +++ b/ackhandler/received_packet_history.go @@ -12,21 +12,15 @@ import ( type receivedPacketHistory struct { ranges *utils.PacketIntervalList - // the map is used as a replacement for a set here. The bool is always supposed to be set to true - receivedPacketNumbers map[protocol.PacketNumber]bool lowestInReceivedPacketNumbers protocol.PacketNumber } -var ( - errTooManyOutstandingReceivedAckRanges = qerr.Error(qerr.TooManyOutstandingReceivedPackets, "Too many outstanding received ACK ranges") - errTooManyOutstandingReceivedPackets = qerr.Error(qerr.TooManyOutstandingReceivedPackets, "Too many outstanding received packets") -) +var errTooManyOutstandingReceivedAckRanges = qerr.Error(qerr.TooManyOutstandingReceivedPackets, "Too many outstanding received ACK ranges") // newReceivedPacketHistory creates a new received packet history func newReceivedPacketHistory() *receivedPacketHistory { return &receivedPacketHistory{ - ranges: utils.NewPacketIntervalList(), - receivedPacketNumbers: make(map[protocol.PacketNumber]bool), + ranges: utils.NewPacketIntervalList(), } } @@ -36,12 +30,6 @@ func (h *receivedPacketHistory) ReceivedPacket(p protocol.PacketNumber) error { return errTooManyOutstandingReceivedAckRanges } - if len(h.receivedPacketNumbers) >= protocol.MaxTrackedReceivedPackets { - return errTooManyOutstandingReceivedPackets - } - - h.receivedPacketNumbers[p] = true - if h.ranges.Len() == 0 { h.ranges.PushBack(utils.PacketInterval{Start: p, End: p}) return nil @@ -95,14 +83,8 @@ func (h *receivedPacketHistory) DeleteUpTo(p protocol.PacketNumber) { nextEl = el.Next() if p >= el.Value.Start && p < el.Value.End { - for i := el.Value.Start; i <= p; i++ { // adjust start value of a range - delete(h.receivedPacketNumbers, i) - } el.Value.Start = p + 1 } else if el.Value.End <= p { // delete a whole range - for i := el.Value.Start; i <= el.Value.End; i++ { - delete(h.receivedPacketNumbers, i) - } h.ranges.Remove(el) } else { // no ranges affected. Nothing to do return @@ -110,17 +92,6 @@ func (h *receivedPacketHistory) DeleteUpTo(p protocol.PacketNumber) { } } -// IsDuplicate determines if a packet should be regarded as a duplicate packet -// note that after receiving a StopWaitingFrame, all packets below the LeastUnacked should be regarded as duplicates, even if the packet was just delayed -func (h *receivedPacketHistory) IsDuplicate(p protocol.PacketNumber) bool { - if p < h.lowestInReceivedPacketNumbers { - return true - } - - _, ok := h.receivedPacketNumbers[p] - return ok -} - // GetAckRanges gets a slice of all AckRanges that can be used in an AckFrame func (h *receivedPacketHistory) GetAckRanges() []wire.AckRange { if h.ranges.Len() == 0 { diff --git a/ackhandler/received_packet_history_test.go b/ackhandler/received_packet_history_test.go index df3f33a3..93dbf964 100644 --- a/ackhandler/received_packet_history_test.go +++ b/ackhandler/received_packet_history_test.go @@ -17,51 +17,17 @@ var _ = Describe("receivedPacketHistory", func() { hist = newReceivedPacketHistory() }) - // check if the ranges PacketIntervalList contains exactly the same packet number as the receivedPacketNumbers - historiesConsistent := func() bool { - // check if a packet number is contained in any of the ranges - containedInRanges := func(p protocol.PacketNumber) bool { - for el := hist.ranges.Front(); el != nil; el = el.Next() { - if p >= el.Value.Start && p <= el.Value.End { - return true - } - } - return false - } - - // first check if all packets contained in the ranges are present in the map - for el := hist.ranges.Front(); el != nil; el = el.Next() { - for i := el.Value.Start; i <= el.Value.Start; i++ { - _, ok := hist.receivedPacketNumbers[i] - if !ok { - return false - } - } - } - - // then check if all packets in the map are contained in any of the ranges - for i := range hist.receivedPacketNumbers { - if !containedInRanges(i) { - return false - } - } - - return true - } - Context("ranges", func() { It("adds the first packet", func() { hist.ReceivedPacket(4) Expect(hist.ranges.Len()).To(Equal(1)) Expect(hist.ranges.Front().Value).To(Equal(utils.PacketInterval{Start: 4, End: 4})) - Expect(historiesConsistent()).To(BeTrue()) }) It("doesn't care about duplicate packets", func() { hist.ReceivedPacket(4) Expect(hist.ranges.Len()).To(Equal(1)) Expect(hist.ranges.Front().Value).To(Equal(utils.PacketInterval{Start: 4, End: 4})) - Expect(historiesConsistent()).To(BeTrue()) }) It("adds a few consecutive packets", func() { @@ -70,7 +36,6 @@ var _ = Describe("receivedPacketHistory", func() { hist.ReceivedPacket(6) Expect(hist.ranges.Len()).To(Equal(1)) Expect(hist.ranges.Front().Value).To(Equal(utils.PacketInterval{Start: 4, End: 6})) - Expect(historiesConsistent()).To(BeTrue()) }) It("doesn't care about a duplicate packet contained in an existing range", func() { @@ -80,7 +45,6 @@ var _ = Describe("receivedPacketHistory", func() { hist.ReceivedPacket(5) Expect(hist.ranges.Len()).To(Equal(1)) Expect(hist.ranges.Front().Value).To(Equal(utils.PacketInterval{Start: 4, End: 6})) - Expect(historiesConsistent()).To(BeTrue()) }) It("extends a range at the front", func() { @@ -88,7 +52,6 @@ var _ = Describe("receivedPacketHistory", func() { hist.ReceivedPacket(3) Expect(hist.ranges.Len()).To(Equal(1)) Expect(hist.ranges.Front().Value).To(Equal(utils.PacketInterval{Start: 3, End: 4})) - Expect(historiesConsistent()).To(BeTrue()) }) It("creates a new range when a packet is lost", func() { @@ -97,7 +60,6 @@ var _ = Describe("receivedPacketHistory", func() { Expect(hist.ranges.Len()).To(Equal(2)) Expect(hist.ranges.Front().Value).To(Equal(utils.PacketInterval{Start: 4, End: 4})) Expect(hist.ranges.Back().Value).To(Equal(utils.PacketInterval{Start: 6, End: 6})) - Expect(historiesConsistent()).To(BeTrue()) }) It("creates a new range in between two ranges", func() { @@ -109,7 +71,6 @@ var _ = Describe("receivedPacketHistory", func() { Expect(hist.ranges.Front().Value).To(Equal(utils.PacketInterval{Start: 4, End: 4})) Expect(hist.ranges.Front().Next().Value).To(Equal(utils.PacketInterval{Start: 7, End: 7})) Expect(hist.ranges.Back().Value).To(Equal(utils.PacketInterval{Start: 10, End: 10})) - Expect(historiesConsistent()).To(BeTrue()) }) It("creates a new range before an existing range for a belated packet", func() { @@ -118,7 +79,6 @@ var _ = Describe("receivedPacketHistory", func() { Expect(hist.ranges.Len()).To(Equal(2)) Expect(hist.ranges.Front().Value).To(Equal(utils.PacketInterval{Start: 4, End: 4})) Expect(hist.ranges.Back().Value).To(Equal(utils.PacketInterval{Start: 6, End: 6})) - Expect(historiesConsistent()).To(BeTrue()) }) It("extends a previous range at the end", func() { @@ -128,7 +88,6 @@ var _ = Describe("receivedPacketHistory", func() { Expect(hist.ranges.Len()).To(Equal(2)) Expect(hist.ranges.Front().Value).To(Equal(utils.PacketInterval{Start: 4, End: 5})) Expect(hist.ranges.Back().Value).To(Equal(utils.PacketInterval{Start: 7, End: 7})) - Expect(historiesConsistent()).To(BeTrue()) }) It("extends a range at the front", func() { @@ -138,7 +97,6 @@ var _ = Describe("receivedPacketHistory", func() { Expect(hist.ranges.Len()).To(Equal(2)) Expect(hist.ranges.Front().Value).To(Equal(utils.PacketInterval{Start: 4, End: 4})) Expect(hist.ranges.Back().Value).To(Equal(utils.PacketInterval{Start: 6, End: 7})) - Expect(historiesConsistent()).To(BeTrue()) }) It("closes a range", func() { @@ -148,7 +106,6 @@ var _ = Describe("receivedPacketHistory", func() { hist.ReceivedPacket(5) Expect(hist.ranges.Len()).To(Equal(1)) Expect(hist.ranges.Front().Value).To(Equal(utils.PacketInterval{Start: 4, End: 6})) - Expect(historiesConsistent()).To(BeTrue()) }) It("closes a range in the middle", func() { @@ -162,7 +119,6 @@ var _ = Describe("receivedPacketHistory", func() { Expect(hist.ranges.Front().Value).To(Equal(utils.PacketInterval{Start: 1, End: 1})) Expect(hist.ranges.Front().Next().Value).To(Equal(utils.PacketInterval{Start: 4, End: 6})) Expect(hist.ranges.Back().Value).To(Equal(utils.PacketInterval{Start: 10, End: 10})) - Expect(historiesConsistent()).To(BeTrue()) }) }) @@ -170,7 +126,6 @@ var _ = Describe("receivedPacketHistory", func() { It("does nothing when the history is empty", func() { hist.DeleteUpTo(5) Expect(hist.ranges.Len()).To(BeZero()) - Expect(historiesConsistent()).To(BeTrue()) }) It("deletes a range", func() { @@ -180,7 +135,6 @@ var _ = Describe("receivedPacketHistory", func() { hist.DeleteUpTo(5) Expect(hist.ranges.Len()).To(Equal(1)) Expect(hist.ranges.Front().Value).To(Equal(utils.PacketInterval{Start: 10, End: 10})) - Expect(historiesConsistent()).To(BeTrue()) }) It("deletes multiple ranges", func() { @@ -190,7 +144,6 @@ var _ = Describe("receivedPacketHistory", func() { hist.DeleteUpTo(8) Expect(hist.ranges.Len()).To(Equal(1)) Expect(hist.ranges.Front().Value).To(Equal(utils.PacketInterval{Start: 10, End: 10})) - Expect(historiesConsistent()).To(BeTrue()) }) It("adjusts a range, if packets are delete from an existing range", func() { @@ -202,7 +155,6 @@ var _ = Describe("receivedPacketHistory", func() { hist.DeleteUpTo(4) Expect(hist.ranges.Len()).To(Equal(1)) Expect(hist.ranges.Front().Value).To(Equal(utils.PacketInterval{Start: 5, End: 7})) - Expect(historiesConsistent()).To(BeTrue()) }) It("adjusts a range, if only one packet remains in the range", func() { @@ -213,7 +165,6 @@ var _ = Describe("receivedPacketHistory", func() { 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})) - Expect(historiesConsistent()).To(BeTrue()) }) It("keeps a one-packet range, if deleting up to the packet directly below", func() { @@ -221,7 +172,6 @@ var _ = Describe("receivedPacketHistory", func() { hist.DeleteUpTo(3) Expect(hist.ranges.Len()).To(Equal(1)) Expect(hist.ranges.Front().Value).To(Equal(utils.PacketInterval{Start: 4, End: 4})) - Expect(historiesConsistent()).To(BeTrue()) }) Context("DoS protection", func() { @@ -232,18 +182,6 @@ var _ = Describe("receivedPacketHistory", func() { } err := hist.ReceivedPacket(2*protocol.MaxTrackedReceivedAckRanges + 2) Expect(err).To(MatchError(errTooManyOutstandingReceivedAckRanges)) - Expect(historiesConsistent()).To(BeTrue()) - }) - - It("doesn't store more than MaxTrackedReceivedPackets packets", func() { - err := hist.ReceivedPacket(1) - Expect(err).ToNot(HaveOccurred()) - for i := protocol.PacketNumber(3); i < 3+protocol.MaxTrackedReceivedPackets-1; i++ { - err := hist.ReceivedPacket(protocol.PacketNumber(i)) - Expect(err).ToNot(HaveOccurred()) - } - err = hist.ReceivedPacket(protocol.PacketNumber(protocol.MaxTrackedReceivedPackets) + 10) - Expect(err).To(MatchError(errTooManyOutstandingReceivedPackets)) }) It("doesn't consider already deleted ranges for MaxTrackedReceivedAckRanges", func() { @@ -256,38 +194,10 @@ var _ = Describe("receivedPacketHistory", func() { hist.DeleteUpTo(protocol.MaxTrackedReceivedAckRanges) // deletes about half of the ranges err = hist.ReceivedPacket(2*protocol.MaxTrackedReceivedAckRanges + 4) Expect(err).ToNot(HaveOccurred()) - Expect(historiesConsistent()).To(BeTrue()) }) }) }) - Context("duplicate packet detection", func() { - It("detects duplicates for existing ranges", func() { - hist.ReceivedPacket(2) - hist.ReceivedPacket(4) - hist.ReceivedPacket(5) - Expect(hist.IsDuplicate(1)).To(BeFalse()) - Expect(hist.IsDuplicate(2)).To(BeTrue()) - Expect(hist.IsDuplicate(3)).To(BeFalse()) - Expect(hist.IsDuplicate(4)).To(BeTrue()) - Expect(hist.IsDuplicate(5)).To(BeTrue()) - Expect(hist.IsDuplicate(6)).To(BeFalse()) - }) - - It("detects duplicates after a range has been deleted", func() { - hist.ReceivedPacket(2) - hist.ReceivedPacket(3) - hist.ReceivedPacket(6) - hist.DeleteUpTo(4) - for i := 1; i < 5; i++ { - Expect(hist.IsDuplicate(protocol.PacketNumber(i))).To(BeTrue()) - } - Expect(hist.IsDuplicate(5)).To(BeFalse()) - Expect(hist.IsDuplicate(6)).To(BeTrue()) - Expect(hist.IsDuplicate(7)).To(BeFalse()) - }) - }) - Context("ACK range export", func() { It("returns nil if there are no ranges", func() { Expect(hist.GetAckRanges()).To(BeNil()) diff --git a/internal/protocol/server_parameters.go b/internal/protocol/server_parameters.go index fe712aba..faea17bd 100644 --- a/internal/protocol/server_parameters.go +++ b/internal/protocol/server_parameters.go @@ -87,9 +87,6 @@ const STKExpiryTime = 24 * time.Hour // MaxTrackedSentPackets is maximum number of sent packets saved for either later retransmission or entropy calculation const MaxTrackedSentPackets = 2 * DefaultMaxCongestionWindow -// MaxTrackedReceivedPackets is the maximum number of received packets saved for doing the entropy calculations -const MaxTrackedReceivedPackets = 2 * DefaultMaxCongestionWindow - // MaxTrackedReceivedAckRanges is the maximum number of ACK ranges tracked const MaxTrackedReceivedAckRanges = DefaultMaxCongestionWindow