remove unused duplicate packet detection in the ackHandler

This commit is contained in:
Marten Seemann 2017-09-14 17:55:55 +07:00
parent 34996e2a29
commit d9b9d83458
4 changed files with 2 additions and 135 deletions

View file

@ -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++ {

View file

@ -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 {

View file

@ -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())

View file

@ -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