gr1ffon/quic-go
1
0
Fork 0
forked from quic-go/quic-go
Code Pull Requests Activity

don't pass the STOP_WAITING to the receivedPacketHandler

Browse Source 
Only pass the LeastUnacked. This makes easier to remove STOP_WAITINGs
later.
This commit is contained in:
Marten Seemann
2017-06-09 12:41:42 +02:00
parent 698c8ceee8
commit 01baba83a5
7 changed files with 66 additions and 92 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
ackhandler/interfaces.go
View File

@@ -25,7 +25,7 @@ type SentPacketHandler interface {
// ReceivedPacketHandler handles ACKs needed to send for incoming packets
type ReceivedPacketHandler interface {
ReceivedPacket(packetNumber protocol.PacketNumber, shouldInstigateAck bool) error
ReceivedStopWaiting(*frames.StopWaitingFrame) error
SetLowerLimit(protocol.PacketNumber)
GetAlarmTimeout() time.Time
GetAckFrame() *frames.AckFrame

30
ackhandler/received_packet_handler.go
View File

@@ -12,7 +12,7 @@ var errInvalidPacketNumber = errors.New("ReceivedPacketHandler: Invalid packet n
type receivedPacketHandler struct {
largestObserved protocol.PacketNumber
ignorePacketsBelow protocol.PacketNumber
lowerLimit protocol.PacketNumber
largestObservedReceivedTime time.Time
packetHistory *receivedPacketHistory
@@ -39,31 +39,27 @@ func (h *receivedPacketHandler) ReceivedPacket(packetNumber protocol.PacketNumbe
return errInvalidPacketNumber
}
if packetNumber > h.ignorePacketsBelow {
if err := h.packetHistory.ReceivedPacket(packetNumber); err != nil {
return err
}
}
if packetNumber > h.largestObserved {
h.largestObserved = packetNumber
h.largestObservedReceivedTime = time.Now()
}
if packetNumber <= h.lowerLimit {
return nil
}
if err := h.packetHistory.ReceivedPacket(packetNumber); err != nil {
return err
}
h.maybeQueueAck(packetNumber, shouldInstigateAck)
return nil
}
func (h *receivedPacketHandler) ReceivedStopWaiting(f *frames.StopWaitingFrame) error {
// ignore if StopWaiting is unneeded, because we already received a StopWaiting with a higher LeastUnacked
if h.ignorePacketsBelow >= f.LeastUnacked {
return nil
}
h.ignorePacketsBelow = f.LeastUnacked - 1
h.packetHistory.DeleteBelow(f.LeastUnacked)
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)
}
func (h *receivedPacketHandler) maybeQueueAck(packetNumber protocol.PacketNumber, shouldInstigateAck bool) {

71
ackhandler/received_packet_handler_test.go
View File

@@ -34,15 +34,6 @@ var _ = Describe("receivedPacketHandler", func() {
Expect(err).To(MatchError(errInvalidPacketNumber))
})
It("does not ignore a packet with PacketNumber equal to LeastUnacked of a previously received StopWaiting", func() {
err := handler.ReceivedPacket(5, true)
Expect(err).ToNot(HaveOccurred())
err = handler.ReceivedStopWaiting(&frames.StopWaitingFrame{LeastUnacked: 10})
Expect(err).ToNot(HaveOccurred())
err = handler.ReceivedPacket(10, true)
Expect(err).ToNot(HaveOccurred())
})
It("saves the time when each packet arrived", func() {
err := handler.ReceivedPacket(protocol.PacketNumber(3), true)
Expect(err).ToNot(HaveOccurred())
@@ -93,33 +84,6 @@ var _ = Describe("receivedPacketHandler", func() {
})
})
Context("handling STOP_WAITING frames", func() {
It("increases the ignorePacketsBelow number", func() {
err := handler.ReceivedStopWaiting(&frames.StopWaitingFrame{LeastUnacked: protocol.PacketNumber(12)})
Expect(err).ToNot(HaveOccurred())
Expect(handler.ignorePacketsBelow).To(Equal(protocol.PacketNumber(11)))
})
It("increase the ignorePacketsBelow number, even if all packets below the LeastUnacked were already acked", func() {
for i := 1; i < 20; i++ {
err := handler.ReceivedPacket(protocol.PacketNumber(i), true)
Expect(err).ToNot(HaveOccurred())
}
err := handler.ReceivedStopWaiting(&frames.StopWaitingFrame{LeastUnacked: protocol.PacketNumber(12)})
Expect(err).ToNot(HaveOccurred())
Expect(handler.ignorePacketsBelow).To(Equal(protocol.PacketNumber(11)))
})
It("does not decrease the ignorePacketsBelow number when an out-of-order StopWaiting arrives", func() {
err := handler.ReceivedStopWaiting(&frames.StopWaitingFrame{LeastUnacked: protocol.PacketNumber(12)})
Expect(err).ToNot(HaveOccurred())
Expect(handler.ignorePacketsBelow).To(Equal(protocol.PacketNumber(11)))
err = handler.ReceivedStopWaiting(&frames.StopWaitingFrame{LeastUnacked: protocol.PacketNumber(6)})
Expect(err).ToNot(HaveOccurred())
Expect(handler.ignorePacketsBelow).To(Equal(protocol.PacketNumber(11)))
})
})
Context("ACKs", func() {
Context("queueing ACKs", func() {
receiveAndAck10Packets := func() {
@@ -197,9 +161,12 @@ var _ = Describe("receivedPacketHandler", func() {
err := handler.ReceivedPacket(protocol.PacketNumber(i), true)
Expect(err).ToNot(HaveOccurred())
}
Expect(handler.GetAckFrame()).ToNot(BeNil())
handler.ReceivedPacket(20, true) // we now know that packets 16 to 19 are missing
err := handler.ReceivedPacket(20, true) // we now know that packets 16 to 19 are missing
Expect(err).ToNot(HaveOccurred())
Expect(handler.ackQueued).To(BeTrue())
ack := handler.GetAckFrame()
Expect(ack.HasMissingRanges()).To(BeTrue())
Expect(ack).ToNot(BeNil())
})
})
@@ -248,10 +215,15 @@ var _ = Describe("receivedPacketHandler", func() {
Expect(ack.AckRanges[1]).To(Equal(frames.AckRange{FirstPacketNumber: 1, LastPacketNumber: 1}))
})
It("doesn't add delayed packets to the packetHistory", func() {
err := handler.ReceivedStopWaiting(&frames.StopWaitingFrame{LeastUnacked: protocol.PacketNumber(6)})
It("accepts packets below the lower limit", func() {
handler.SetLowerLimit(5)
err := handler.ReceivedPacket(2, true)
Expect(err).ToNot(HaveOccurred())
err = handler.ReceivedPacket(4, true)
})
It("doesn't add delayed packets to the packetHistory", func() {
handler.SetLowerLimit(6)
err := handler.ReceivedPacket(4, true)
Expect(err).ToNot(HaveOccurred())
err = handler.ReceivedPacket(10, true)
Expect(err).ToNot(HaveOccurred())
@@ -261,21 +233,30 @@ var _ = Describe("receivedPacketHandler", func() {
Expect(ack.LowestAcked).To(Equal(protocol.PacketNumber(10)))
})
It("deletes packets from the packetHistory after receiving a StopWaiting, after continuously received packets", func() {
It("deletes packets from the packetHistory when a lower limit is set", func() {
for i := 1; i <= 12; i++ {
err := handler.ReceivedPacket(protocol.PacketNumber(i), true)
Expect(err).ToNot(HaveOccurred())
}
err := handler.ReceivedStopWaiting(&frames.StopWaitingFrame{LeastUnacked: protocol.PacketNumber(6)})
Expect(err).ToNot(HaveOccurred())
handler.SetLowerLimit(6)
// check that the packets were deleted from the receivedPacketHistory by checking the values in an ACK frame
ack := handler.GetAckFrame()
Expect(ack).ToNot(BeNil())
Expect(ack.LargestAcked).To(Equal(protocol.PacketNumber(12)))
Expect(ack.LowestAcked).To(Equal(protocol.PacketNumber(6)))
Expect(ack.LowestAcked).To(Equal(protocol.PacketNumber(7)))
Expect(ack.HasMissingRanges()).To(BeFalse())
})
// TODO: remove this test when dropping support for STOP_WAITINGs
It("handles a lower limit of 0", func() {
handler.SetLowerLimit(0)
err := handler.ReceivedPacket(1337, true)
Expect(err).ToNot(HaveOccurred())
ack := handler.GetAckFrame()
Expect(ack).ToNot(BeNil())
Expect(ack.LargestAcked).To(Equal(protocol.PacketNumber(1337)))
})
It("resets all counters needed for the ACK queueing decision when sending an ACK", func() {
err := handler.ReceivedPacket(1, true)
Expect(err).ToNot(HaveOccurred())

16
ackhandler/received_packet_history.go
View File

@@ -7,6 +7,8 @@ import (
"github.com/lucas-clemente/quic-go/qerr"
)
// The receivedPacketHistory stores if a packet number has already been received.
// It does not store packet contents.
type receivedPacketHistory struct {
ranges *utils.PacketIntervalList
@@ -84,20 +86,20 @@ func (h *receivedPacketHistory) ReceivedPacket(p protocol.PacketNumber) error {
return nil
}
// DeleteBelow deletes all entries below the leastUnacked packet number
func (h *receivedPacketHistory) DeleteBelow(leastUnacked protocol.PacketNumber) {
h.lowestInReceivedPacketNumbers = utils.MaxPacketNumber(h.lowestInReceivedPacketNumbers, leastUnacked)
// DeleteUpTo deletes all entries up to (and including) p
func (h *receivedPacketHistory) DeleteUpTo(p protocol.PacketNumber) {
h.lowestInReceivedPacketNumbers = utils.MaxPacketNumber(h.lowestInReceivedPacketNumbers, p+1)
nextEl := h.ranges.Front()
for el := h.ranges.Front(); nextEl != nil; el = nextEl {
nextEl = el.Next()
if leastUnacked > el.Value.Start && leastUnacked <= el.Value.End {
for i := el.Value.Start; i < leastUnacked; i++ { // adjust start value of a range
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 = leastUnacked
} else if el.Value.End < leastUnacked { // delete a whole range
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)
}

25
ackhandler/received_packet_history_test.go
View File

@@ -168,7 +168,7 @@ var _ = Describe("receivedPacketHistory", func() {
Context("deleting", func() {
It("does nothing when the history is empty", func() {
hist.DeleteBelow(5)
hist.DeleteUpTo(5)
Expect(hist.ranges.Len()).To(BeZero())
Expect(historiesConsistent()).To(BeTrue())
})
@@ -177,7 +177,7 @@ var _ = Describe("receivedPacketHistory", func() {
hist.ReceivedPacket(4)
hist.ReceivedPacket(5)
hist.ReceivedPacket(10)
hist.DeleteBelow(6)
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())
@@ -187,37 +187,38 @@ var _ = Describe("receivedPacketHistory", func() {
hist.ReceivedPacket(1)
hist.ReceivedPacket(5)
hist.ReceivedPacket(10)
hist.DeleteBelow(8)
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 leastUnacked lies inside it", func() {
It("adjusts a range, if packets are delete from an existing range", func() {
hist.ReceivedPacket(3)
hist.ReceivedPacket(4)
hist.ReceivedPacket(5)
hist.ReceivedPacket(6)
hist.DeleteBelow(4)
hist.ReceivedPacket(7)
hist.DeleteUpTo(4)
Expect(hist.ranges.Len()).To(Equal(1))
Expect(hist.ranges.Front().Value).To(Equal(utils.PacketInterval{Start: 4, End: 6}))
Expect(hist.ranges.Front().Value).To(Equal(utils.PacketInterval{Start: 5, End: 7}))
Expect(historiesConsistent()).To(BeTrue())
})
It("adjusts a range, if leastUnacked is the last of the range", func() {
It("adjusts a range, if only one packet remains in the range", func() {
hist.ReceivedPacket(4)
hist.ReceivedPacket(5)
hist.ReceivedPacket(10)
hist.DeleteBelow(5)
hist.DeleteUpTo(4)
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 leastUnacked is exactly that value", func() {
It("keeps a one-packet range, if deleting up to the packet directly below", func() {
hist.ReceivedPacket(4)
hist.DeleteBelow(4)
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())
@@ -252,7 +253,7 @@ var _ = Describe("receivedPacketHistory", func() {
}
err := hist.ReceivedPacket(2*protocol.MaxTrackedReceivedAckRanges + 2)
Expect(err).To(MatchError(errTooManyOutstandingReceivedAckRanges))
hist.DeleteBelow(protocol.MaxTrackedReceivedAckRanges) // deletes about half of the ranges
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())
@@ -277,7 +278,7 @@ var _ = Describe("receivedPacketHistory", func() {
hist.ReceivedPacket(2)
hist.ReceivedPacket(3)
hist.ReceivedPacket(6)
hist.DeleteBelow(5)
hist.DeleteUpTo(4)
for i := 1; i < 5; i++ {
Expect(hist.IsDuplicate(protocol.PacketNumber(i))).To(BeTrue())
}