ackhandler: reject duplicate packets in ReceivedPacket (#3568)

Before we process a packet, IsPotentiallyDuplicate is called.
This should filter out all duplicates before the reach ReceivedPacket.
This commit is contained in:
Marten Seemann 2022-10-11 12:27:10 +03:00 committed by GitHub
parent 597e00549d
commit 438942b888
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 63 additions and 77 deletions

View file

@ -24,7 +24,7 @@ var _ = Describe("Received Packet Tracker", func() {
Context("accepting packets", func() {
It("saves the time when each packet arrived", func() {
tracker.ReceivedPacket(protocol.PacketNumber(3), protocol.ECNNon, time.Now(), true)
Expect(tracker.ReceivedPacket(protocol.PacketNumber(3), protocol.ECNNon, time.Now(), true)).To(Succeed())
Expect(tracker.largestObservedReceivedTime).To(BeTemporally("~", time.Now(), 10*time.Millisecond))
})
@ -32,7 +32,7 @@ var _ = Describe("Received Packet Tracker", func() {
now := time.Now()
tracker.largestObserved = 3
tracker.largestObservedReceivedTime = now.Add(-1 * time.Second)
tracker.ReceivedPacket(5, protocol.ECNNon, now, true)
Expect(tracker.ReceivedPacket(5, protocol.ECNNon, now, true)).To(Succeed())
Expect(tracker.largestObserved).To(Equal(protocol.PacketNumber(5)))
Expect(tracker.largestObservedReceivedTime).To(Equal(now))
})
@ -42,7 +42,7 @@ var _ = Describe("Received Packet Tracker", func() {
timestamp := now.Add(-1 * time.Second)
tracker.largestObserved = 5
tracker.largestObservedReceivedTime = timestamp
tracker.ReceivedPacket(4, protocol.ECNNon, now, true)
Expect(tracker.ReceivedPacket(4, protocol.ECNNon, now, true)).To(Succeed())
Expect(tracker.largestObserved).To(Equal(protocol.PacketNumber(5)))
Expect(tracker.largestObservedReceivedTime).To(Equal(timestamp))
})
@ -52,35 +52,35 @@ var _ = Describe("Received Packet Tracker", func() {
Context("queueing ACKs", func() {
receiveAndAck10Packets := func() {
for i := 1; i <= 10; i++ {
tracker.ReceivedPacket(protocol.PacketNumber(i), protocol.ECNNon, time.Time{}, true)
Expect(tracker.ReceivedPacket(protocol.PacketNumber(i), protocol.ECNNon, time.Time{}, true)).To(Succeed())
}
Expect(tracker.GetAckFrame(true)).ToNot(BeNil())
Expect(tracker.ackQueued).To(BeFalse())
}
It("always queues an ACK for the first packet", func() {
tracker.ReceivedPacket(1, protocol.ECNNon, time.Now(), true)
Expect(tracker.ReceivedPacket(1, protocol.ECNNon, time.Now(), true)).To(Succeed())
Expect(tracker.ackQueued).To(BeTrue())
Expect(tracker.GetAlarmTimeout()).To(BeZero())
Expect(tracker.GetAckFrame(true).DelayTime).To(BeNumerically("~", 0, time.Second))
})
It("works with packet number 0", func() {
tracker.ReceivedPacket(0, protocol.ECNNon, time.Now(), true)
Expect(tracker.ReceivedPacket(0, protocol.ECNNon, time.Now(), true)).To(Succeed())
Expect(tracker.ackQueued).To(BeTrue())
Expect(tracker.GetAlarmTimeout()).To(BeZero())
Expect(tracker.GetAckFrame(true).DelayTime).To(BeNumerically("~", 0, time.Second))
})
It("sets ECN flags", func() {
tracker.ReceivedPacket(0, protocol.ECT0, time.Now(), true)
Expect(tracker.ReceivedPacket(0, protocol.ECT0, time.Now(), true)).To(Succeed())
pn := protocol.PacketNumber(1)
for i := 0; i < 2; i++ {
tracker.ReceivedPacket(pn, protocol.ECT1, time.Now(), true)
Expect(tracker.ReceivedPacket(pn, protocol.ECT1, time.Now(), true)).To(Succeed())
pn++
}
for i := 0; i < 3; i++ {
tracker.ReceivedPacket(pn, protocol.ECNCE, time.Now(), true)
Expect(tracker.ReceivedPacket(pn, protocol.ECNCE, time.Now(), true)).To(Succeed())
pn++
}
ack := tracker.GetAckFrame(false)
@ -93,10 +93,10 @@ var _ = Describe("Received Packet Tracker", func() {
receiveAndAck10Packets()
p := protocol.PacketNumber(11)
for i := 0; i <= 20; i++ {
tracker.ReceivedPacket(p, protocol.ECNNon, time.Time{}, true)
Expect(tracker.ReceivedPacket(p, protocol.ECNNon, time.Time{}, true)).To(Succeed())
Expect(tracker.ackQueued).To(BeFalse())
p++
tracker.ReceivedPacket(p, protocol.ECNNon, time.Time{}, true)
Expect(tracker.ReceivedPacket(p, protocol.ECNNon, time.Time{}, true)).To(Succeed())
Expect(tracker.ackQueued).To(BeTrue())
p++
// dequeue the ACK frame
@ -107,57 +107,43 @@ var _ = Describe("Received Packet Tracker", func() {
It("resets the counter when a non-queued ACK frame is generated", func() {
receiveAndAck10Packets()
rcvTime := time.Now()
tracker.ReceivedPacket(11, protocol.ECNNon, rcvTime, true)
Expect(tracker.ReceivedPacket(11, protocol.ECNNon, rcvTime, true)).To(Succeed())
Expect(tracker.GetAckFrame(false)).ToNot(BeNil())
tracker.ReceivedPacket(12, protocol.ECNNon, rcvTime, true)
Expect(tracker.ReceivedPacket(12, protocol.ECNNon, rcvTime, true)).To(Succeed())
Expect(tracker.GetAckFrame(true)).To(BeNil())
tracker.ReceivedPacket(13, protocol.ECNNon, rcvTime, true)
Expect(tracker.ReceivedPacket(13, protocol.ECNNon, rcvTime, true)).To(Succeed())
Expect(tracker.GetAckFrame(false)).ToNot(BeNil())
})
It("only sets the timer when receiving a ack-eliciting packets", func() {
receiveAndAck10Packets()
tracker.ReceivedPacket(11, protocol.ECNNon, time.Now(), false)
Expect(tracker.ReceivedPacket(11, protocol.ECNNon, time.Now(), false)).To(Succeed())
Expect(tracker.ackQueued).To(BeFalse())
Expect(tracker.GetAlarmTimeout()).To(BeZero())
rcvTime := time.Now().Add(10 * time.Millisecond)
tracker.ReceivedPacket(12, protocol.ECNNon, rcvTime, true)
Expect(tracker.ReceivedPacket(12, protocol.ECNNon, rcvTime, true)).To(Succeed())
Expect(tracker.ackQueued).To(BeFalse())
Expect(tracker.GetAlarmTimeout()).To(Equal(rcvTime.Add(protocol.MaxAckDelay)))
})
It("queues an ACK if it was reported missing before", func() {
receiveAndAck10Packets()
tracker.ReceivedPacket(11, protocol.ECNNon, time.Now(), true)
tracker.ReceivedPacket(13, protocol.ECNNon, time.Now(), true)
Expect(tracker.ReceivedPacket(11, protocol.ECNNon, time.Now(), true)).To(Succeed())
Expect(tracker.ReceivedPacket(13, protocol.ECNNon, time.Now(), true)).To(Succeed())
ack := tracker.GetAckFrame(true) // ACK: 1-11 and 13, missing: 12
Expect(ack).ToNot(BeNil())
Expect(ack.HasMissingRanges()).To(BeTrue())
Expect(tracker.ackQueued).To(BeFalse())
tracker.ReceivedPacket(12, protocol.ECNNon, time.Now(), true)
Expect(tracker.ReceivedPacket(12, protocol.ECNNon, time.Now(), true)).To(Succeed())
Expect(tracker.ackQueued).To(BeTrue())
})
It("doesn't queue an ACK if it was reported missing before, but is below the threshold", func() {
receiveAndAck10Packets()
// 11 is missing
tracker.ReceivedPacket(12, protocol.ECNNon, time.Now(), true)
tracker.ReceivedPacket(13, protocol.ECNNon, time.Now(), true)
ack := tracker.GetAckFrame(true) // ACK: 1-10, 12-13
Expect(ack).ToNot(BeNil())
// now receive 11
tracker.IgnoreBelow(12)
tracker.ReceivedPacket(11, protocol.ECNNon, time.Now(), false)
ack = tracker.GetAckFrame(true)
Expect(ack).To(BeNil())
})
It("doesn't recognize in-order packets as out-of-order after raising the threshold", func() {
receiveAndAck10Packets()
Expect(tracker.lastAck.LargestAcked()).To(Equal(protocol.PacketNumber(10)))
Expect(tracker.ackQueued).To(BeFalse())
tracker.IgnoreBelow(11)
tracker.ReceivedPacket(11, protocol.ECNNon, time.Now(), true)
Expect(tracker.ReceivedPacket(11, protocol.ECNNon, time.Now(), true)).To(Succeed())
Expect(tracker.GetAckFrame(true)).To(BeNil())
})
@ -166,7 +152,7 @@ var _ = Describe("Received Packet Tracker", func() {
Expect(tracker.lastAck.LargestAcked()).To(Equal(protocol.PacketNumber(10)))
Expect(tracker.ackQueued).To(BeFalse())
tracker.IgnoreBelow(11)
tracker.ReceivedPacket(12, protocol.ECNNon, time.Now(), true)
Expect(tracker.ReceivedPacket(12, protocol.ECNNon, time.Now(), true)).To(Succeed())
ack := tracker.GetAckFrame(true)
Expect(ack).ToNot(BeNil())
Expect(ack.AckRanges).To(Equal([]wire.AckRange{{Smallest: 12, Largest: 12}}))
@ -174,38 +160,38 @@ var _ = Describe("Received Packet Tracker", func() {
It("doesn't queue an ACK if for non-ack-eliciting packets arriving out-of-order", func() {
receiveAndAck10Packets()
tracker.ReceivedPacket(11, protocol.ECNNon, time.Now(), true)
Expect(tracker.ReceivedPacket(11, protocol.ECNNon, time.Now(), true)).To(Succeed())
Expect(tracker.GetAckFrame(true)).To(BeNil())
tracker.ReceivedPacket(13, protocol.ECNNon, time.Now(), false) // receive a non-ack-eliciting packet out-of-order
Expect(tracker.ReceivedPacket(13, protocol.ECNNon, time.Now(), false)).To(Succeed()) // receive a non-ack-eliciting packet out-of-order
Expect(tracker.GetAckFrame(true)).To(BeNil())
})
It("doesn't queue an ACK if packets arrive out-of-order, but haven't been acknowledged yet", func() {
receiveAndAck10Packets()
Expect(tracker.lastAck).ToNot(BeNil())
tracker.ReceivedPacket(12, protocol.ECNNon, time.Now(), false)
Expect(tracker.ReceivedPacket(12, protocol.ECNNon, time.Now(), false)).To(Succeed())
Expect(tracker.GetAckFrame(true)).To(BeNil())
// 11 is received out-of-order, but this hasn't been reported in an ACK frame yet
tracker.ReceivedPacket(11, protocol.ECNNon, time.Now(), true)
Expect(tracker.ReceivedPacket(11, protocol.ECNNon, time.Now(), true)).To(Succeed())
Expect(tracker.GetAckFrame(true)).To(BeNil())
})
})
Context("ACK generation", func() {
It("generates an ACK for an ack-eliciting packet, if no ACK is queued yet", func() {
tracker.ReceivedPacket(1, protocol.ECNNon, time.Now(), true)
Expect(tracker.ReceivedPacket(1, protocol.ECNNon, time.Now(), true)).To(Succeed())
// The first packet is always acknowledged.
Expect(tracker.GetAckFrame(true)).ToNot(BeNil())
})
It("doesn't generate ACK for a non-ack-eliciting packet, if no ACK is queued yet", func() {
tracker.ReceivedPacket(1, protocol.ECNNon, time.Now(), true)
Expect(tracker.ReceivedPacket(1, protocol.ECNNon, time.Now(), true)).To(Succeed())
// The first packet is always acknowledged.
Expect(tracker.GetAckFrame(true)).ToNot(BeNil())
tracker.ReceivedPacket(2, protocol.ECNNon, time.Now(), false)
Expect(tracker.ReceivedPacket(2, protocol.ECNNon, time.Now(), false)).To(Succeed())
Expect(tracker.GetAckFrame(false)).To(BeNil())
tracker.ReceivedPacket(3, protocol.ECNNon, time.Now(), true)
Expect(tracker.ReceivedPacket(3, protocol.ECNNon, time.Now(), true)).To(Succeed())
ack := tracker.GetAckFrame(false)
Expect(ack).ToNot(BeNil())
Expect(ack.LowestAcked()).To(Equal(protocol.PacketNumber(1)))
@ -218,8 +204,8 @@ var _ = Describe("Received Packet Tracker", func() {
})
It("generates a simple ACK frame", func() {
tracker.ReceivedPacket(1, protocol.ECNNon, time.Now(), true)
tracker.ReceivedPacket(2, protocol.ECNNon, time.Now(), true)
Expect(tracker.ReceivedPacket(1, protocol.ECNNon, time.Now(), true)).To(Succeed())
Expect(tracker.ReceivedPacket(2, protocol.ECNNon, time.Now(), true)).To(Succeed())
ack := tracker.GetAckFrame(true)
Expect(ack).ToNot(BeNil())
Expect(ack.LargestAcked()).To(Equal(protocol.PacketNumber(2)))
@ -228,7 +214,7 @@ var _ = Describe("Received Packet Tracker", func() {
})
It("generates an ACK for packet number 0", func() {
tracker.ReceivedPacket(0, protocol.ECNNon, time.Now(), true)
Expect(tracker.ReceivedPacket(0, protocol.ECNNon, time.Now(), true)).To(Succeed())
ack := tracker.GetAckFrame(true)
Expect(ack).ToNot(BeNil())
Expect(ack.LargestAcked()).To(Equal(protocol.PacketNumber(0)))
@ -237,26 +223,26 @@ var _ = Describe("Received Packet Tracker", func() {
})
It("sets the delay time", func() {
tracker.ReceivedPacket(1, protocol.ECNNon, time.Now(), true)
tracker.ReceivedPacket(2, protocol.ECNNon, time.Now().Add(-1337*time.Millisecond), true)
Expect(tracker.ReceivedPacket(1, protocol.ECNNon, time.Now(), true)).To(Succeed())
Expect(tracker.ReceivedPacket(2, protocol.ECNNon, time.Now().Add(-1337*time.Millisecond), true)).To(Succeed())
ack := tracker.GetAckFrame(true)
Expect(ack).ToNot(BeNil())
Expect(ack.DelayTime).To(BeNumerically("~", 1337*time.Millisecond, 50*time.Millisecond))
})
It("uses a 0 delay time if the delay would be negative", func() {
tracker.ReceivedPacket(0, protocol.ECNNon, time.Now().Add(time.Hour), true)
Expect(tracker.ReceivedPacket(0, protocol.ECNNon, time.Now().Add(time.Hour), true)).To(Succeed())
ack := tracker.GetAckFrame(true)
Expect(ack).ToNot(BeNil())
Expect(ack.DelayTime).To(BeZero())
})
It("saves the last sent ACK", func() {
tracker.ReceivedPacket(1, protocol.ECNNon, time.Now(), true)
Expect(tracker.ReceivedPacket(1, protocol.ECNNon, time.Now(), true)).To(Succeed())
ack := tracker.GetAckFrame(true)
Expect(ack).ToNot(BeNil())
Expect(tracker.lastAck).To(Equal(ack))
tracker.ReceivedPacket(2, protocol.ECNNon, time.Now(), true)
Expect(tracker.ReceivedPacket(2, protocol.ECNNon, time.Now(), true)).To(Succeed())
tracker.ackQueued = true
ack = tracker.GetAckFrame(true)
Expect(ack).ToNot(BeNil())
@ -264,8 +250,8 @@ var _ = Describe("Received Packet Tracker", func() {
})
It("generates an ACK frame with missing packets", func() {
tracker.ReceivedPacket(1, protocol.ECNNon, time.Now(), true)
tracker.ReceivedPacket(4, protocol.ECNNon, time.Now(), true)
Expect(tracker.ReceivedPacket(1, protocol.ECNNon, time.Now(), true)).To(Succeed())
Expect(tracker.ReceivedPacket(4, protocol.ECNNon, time.Now(), true)).To(Succeed())
ack := tracker.GetAckFrame(true)
Expect(ack).ToNot(BeNil())
Expect(ack.LargestAcked()).To(Equal(protocol.PacketNumber(4)))
@ -277,9 +263,9 @@ var _ = Describe("Received Packet Tracker", func() {
})
It("generates an ACK for packet number 0 and other packets", func() {
tracker.ReceivedPacket(0, protocol.ECNNon, time.Now(), true)
tracker.ReceivedPacket(1, protocol.ECNNon, time.Now(), true)
tracker.ReceivedPacket(3, protocol.ECNNon, time.Now(), true)
Expect(tracker.ReceivedPacket(0, protocol.ECNNon, time.Now(), true)).To(Succeed())
Expect(tracker.ReceivedPacket(1, protocol.ECNNon, time.Now(), true)).To(Succeed())
Expect(tracker.ReceivedPacket(3, protocol.ECNNon, time.Now(), true)).To(Succeed())
ack := tracker.GetAckFrame(true)
Expect(ack).ToNot(BeNil())
Expect(ack.LargestAcked()).To(Equal(protocol.PacketNumber(3)))
@ -290,19 +276,15 @@ var _ = Describe("Received Packet Tracker", func() {
}))
})
It("doesn't add delayed packets to the packetHistory", func() {
It("errors when called with an old packet", func() {
tracker.IgnoreBelow(7)
tracker.ReceivedPacket(4, protocol.ECNNon, time.Now(), true)
tracker.ReceivedPacket(10, protocol.ECNNon, time.Now(), true)
ack := tracker.GetAckFrame(true)
Expect(ack).ToNot(BeNil())
Expect(ack.LargestAcked()).To(Equal(protocol.PacketNumber(10)))
Expect(ack.LowestAcked()).To(Equal(protocol.PacketNumber(10)))
Expect(tracker.IsPotentiallyDuplicate(4)).To(BeTrue())
Expect(tracker.ReceivedPacket(4, protocol.ECNNon, time.Now(), true)).To(MatchError("recevedPacketTracker BUG: ReceivedPacket called for old / duplicate packet 4"))
})
It("deletes packets from the packetHistory when a lower limit is set", func() {
for i := 1; i <= 12; i++ {
tracker.ReceivedPacket(protocol.PacketNumber(i), protocol.ECNNon, time.Now(), true)
Expect(tracker.ReceivedPacket(protocol.PacketNumber(i), protocol.ECNNon, time.Now(), true)).To(Succeed())
}
tracker.IgnoreBelow(7)
// check that the packets were deleted from the receivedPacketHistory by checking the values in an ACK frame
@ -314,7 +296,7 @@ var _ = Describe("Received Packet Tracker", func() {
})
It("resets all counters needed for the ACK queueing decision when sending an ACK", func() {
tracker.ReceivedPacket(1, protocol.ECNNon, time.Now(), true)
Expect(tracker.ReceivedPacket(1, protocol.ECNNon, time.Now(), true)).To(Succeed())
tracker.ackAlarm = time.Now().Add(-time.Minute)
Expect(tracker.GetAckFrame(true)).ToNot(BeNil())
Expect(tracker.GetAlarmTimeout()).To(BeZero())
@ -323,21 +305,21 @@ var _ = Describe("Received Packet Tracker", func() {
})
It("doesn't generate an ACK when none is queued and the timer is not set", func() {
tracker.ReceivedPacket(1, protocol.ECNNon, time.Now(), true)
Expect(tracker.ReceivedPacket(1, protocol.ECNNon, time.Now(), true)).To(Succeed())
tracker.ackQueued = false
tracker.ackAlarm = time.Time{}
Expect(tracker.GetAckFrame(true)).To(BeNil())
})
It("doesn't generate an ACK when none is queued and the timer has not yet expired", func() {
tracker.ReceivedPacket(1, protocol.ECNNon, time.Now(), true)
Expect(tracker.ReceivedPacket(1, protocol.ECNNon, time.Now(), true)).To(Succeed())
tracker.ackQueued = false
tracker.ackAlarm = time.Now().Add(time.Minute)
Expect(tracker.GetAckFrame(true)).To(BeNil())
})
It("generates an ACK when the timer has expired", func() {
tracker.ReceivedPacket(1, protocol.ECNNon, time.Now(), true)
Expect(tracker.ReceivedPacket(1, protocol.ECNNon, time.Now(), true)).To(Succeed())
tracker.ackQueued = false
tracker.ackAlarm = time.Now().Add(-time.Minute)
Expect(tracker.GetAckFrame(true)).ToNot(BeNil())