remove error var and internalize member variable in SentPacketHandler

This commit is contained in:
Marten Seemann 2017-12-04 10:57:19 +07:00
parent 196efa873d
commit 6695983a43
2 changed files with 23 additions and 33 deletions

View file

@ -29,17 +29,8 @@ const (
maxRTOTimeout = 60 * time.Second maxRTOTimeout = 60 * time.Second
) )
var ( // ErrDuplicateOrOutOfOrderAck occurs when a duplicate or an out-of-order ACK is received
// ErrDuplicateOrOutOfOrderAck occurs when a duplicate or an out-of-order ACK is received var ErrDuplicateOrOutOfOrderAck = errors.New("SentPacketHandler: Duplicate or out-of-order ACK")
ErrDuplicateOrOutOfOrderAck = errors.New("SentPacketHandler: Duplicate or out-of-order ACK")
// ErrTooManyTrackedSentPackets occurs when the sentPacketHandler has to keep track of too many packets
ErrTooManyTrackedSentPackets = errors.New("Too many outstanding non-acked and non-retransmitted packets")
// ErrAckForSkippedPacket occurs when the client sent an ACK for a packet number that we intentionally skipped
ErrAckForSkippedPacket = qerr.Error(qerr.InvalidAckData, "Received an ACK for a skipped packet number")
errAckForUnsentPacket = qerr.Error(qerr.InvalidAckData, "Received ACK for an unsent package")
)
var errPacketNumberNotIncreasing = errors.New("Already sent a packet with a higher packet number")
type sentPacketHandler struct { type sentPacketHandler struct {
lastSentPacketNumber protocol.PacketNumber lastSentPacketNumber protocol.PacketNumber
@ -47,8 +38,7 @@ type sentPacketHandler struct {
numNonRetransmittablePackets int // number of non-retransmittable packets since the last retransmittable packet numNonRetransmittablePackets int // number of non-retransmittable packets since the last retransmittable packet
LargestAcked protocol.PacketNumber largestAcked protocol.PacketNumber
largestReceivedPacketWithAck protocol.PacketNumber largestReceivedPacketWithAck protocol.PacketNumber
packetHistory *PacketList packetHistory *PacketList
@ -97,7 +87,7 @@ func (h *sentPacketHandler) largestInOrderAcked() protocol.PacketNumber {
if f := h.packetHistory.Front(); f != nil { if f := h.packetHistory.Front(); f != nil {
return f.Value.PacketNumber - 1 return f.Value.PacketNumber - 1
} }
return h.LargestAcked return h.largestAcked
} }
func (h *sentPacketHandler) ShouldSendRetransmittablePacket() bool { func (h *sentPacketHandler) ShouldSendRetransmittablePacket() bool {
@ -110,11 +100,11 @@ func (h *sentPacketHandler) SetHandshakeComplete() {
func (h *sentPacketHandler) SentPacket(packet *Packet) error { func (h *sentPacketHandler) SentPacket(packet *Packet) error {
if packet.PacketNumber <= h.lastSentPacketNumber { if packet.PacketNumber <= h.lastSentPacketNumber {
return errPacketNumberNotIncreasing return errors.New("Already sent a packet with a higher packet number")
} }
if protocol.PacketNumber(len(h.retransmissionQueue)+h.packetHistory.Len()+1) > protocol.MaxTrackedSentPackets { if protocol.PacketNumber(len(h.retransmissionQueue)+h.packetHistory.Len()+1) > protocol.MaxTrackedSentPackets {
return ErrTooManyTrackedSentPackets return errors.New("Too many outstanding non-acked and non-retransmitted packets")
} }
for p := h.lastSentPacketNumber + 1; p < packet.PacketNumber; p++ { for p := h.lastSentPacketNumber + 1; p < packet.PacketNumber; p++ {
@ -154,7 +144,7 @@ func (h *sentPacketHandler) SentPacket(packet *Packet) error {
func (h *sentPacketHandler) ReceivedAck(ackFrame *wire.AckFrame, withPacketNumber protocol.PacketNumber, encLevel protocol.EncryptionLevel, rcvTime time.Time) error { func (h *sentPacketHandler) ReceivedAck(ackFrame *wire.AckFrame, withPacketNumber protocol.PacketNumber, encLevel protocol.EncryptionLevel, rcvTime time.Time) error {
if ackFrame.LargestAcked > h.lastSentPacketNumber { if ackFrame.LargestAcked > h.lastSentPacketNumber {
return errAckForUnsentPacket return qerr.Error(qerr.InvalidAckData, "Received ACK for an unsent package")
} }
// duplicate or out-of-order ACK // duplicate or out-of-order ACK
@ -167,10 +157,10 @@ func (h *sentPacketHandler) ReceivedAck(ackFrame *wire.AckFrame, withPacketNumbe
if ackFrame.LargestAcked <= h.largestInOrderAcked() { if ackFrame.LargestAcked <= h.largestInOrderAcked() {
return nil return nil
} }
h.LargestAcked = ackFrame.LargestAcked h.largestAcked = ackFrame.LargestAcked
if h.skippedPacketsAcked(ackFrame) { if h.skippedPacketsAcked(ackFrame) {
return ErrAckForSkippedPacket return qerr.Error(qerr.InvalidAckData, "Received an ACK for a skipped packet number")
} }
rttUpdated := h.maybeUpdateRTT(ackFrame.LargestAcked, ackFrame.DelayTime, rcvTime) rttUpdated := h.maybeUpdateRTT(ackFrame.LargestAcked, ackFrame.DelayTime, rcvTime)
@ -286,7 +276,7 @@ func (h *sentPacketHandler) detectLostPackets() {
for el := h.packetHistory.Front(); el != nil; el = el.Next() { for el := h.packetHistory.Front(); el != nil; el = el.Next() {
packet := el.Value packet := el.Value
if packet.PacketNumber > h.LargestAcked { if packet.PacketNumber > h.largestAcked {
break break
} }

View file

@ -105,7 +105,7 @@ var _ = Describe("SentPacketHandler", func() {
} }
It("gets the LeastUnacked packet number", func() { It("gets the LeastUnacked packet number", func() {
handler.LargestAcked = 0x1337 handler.largestAcked = 0x1337
Expect(handler.GetLeastUnacked()).To(Equal(protocol.PacketNumber(0x1337 + 1))) Expect(handler.GetLeastUnacked()).To(Equal(protocol.PacketNumber(0x1337 + 1)))
}) })
@ -130,7 +130,7 @@ var _ = Describe("SentPacketHandler", func() {
err := handler.SentPacket(&packet1) err := handler.SentPacket(&packet1)
Expect(err).ToNot(HaveOccurred()) Expect(err).ToNot(HaveOccurred())
err = handler.SentPacket(&packet2) err = handler.SentPacket(&packet2)
Expect(err).To(MatchError(errPacketNumberNotIncreasing)) Expect(err).To(MatchError("Already sent a packet with a higher packet number"))
Expect(handler.lastSentPacketNumber).To(Equal(protocol.PacketNumber(1))) Expect(handler.lastSentPacketNumber).To(Equal(protocol.PacketNumber(1)))
Expect(handler.packetHistory.Front().Value.PacketNumber).To(Equal(protocol.PacketNumber(1))) Expect(handler.packetHistory.Front().Value.PacketNumber).To(Equal(protocol.PacketNumber(1)))
Expect(handler.bytesInFlight).To(Equal(protocol.ByteCount(1))) Expect(handler.bytesInFlight).To(Equal(protocol.ByteCount(1)))
@ -143,7 +143,7 @@ var _ = Describe("SentPacketHandler", func() {
err := handler.SentPacket(&packet1) err := handler.SentPacket(&packet1)
Expect(err).ToNot(HaveOccurred()) Expect(err).ToNot(HaveOccurred())
err = handler.SentPacket(&packet2) err = handler.SentPacket(&packet2)
Expect(err).To(MatchError(errPacketNumberNotIncreasing)) Expect(err).To(MatchError("Already sent a packet with a higher packet number"))
Expect(handler.lastSentPacketNumber).To(Equal(protocol.PacketNumber(2))) Expect(handler.lastSentPacketNumber).To(Equal(protocol.PacketNumber(2)))
Expect(handler.packetHistory.Front().Value.PacketNumber).To(Equal(protocol.PacketNumber(2))) Expect(handler.packetHistory.Front().Value.PacketNumber).To(Equal(protocol.PacketNumber(2)))
Expect(handler.bytesInFlight).To(Equal(protocol.ByteCount(1))) Expect(handler.bytesInFlight).To(Equal(protocol.ByteCount(1)))
@ -219,21 +219,21 @@ var _ = Describe("SentPacketHandler", func() {
Context("garbage collection", func() { Context("garbage collection", func() {
It("keeps all packet numbers above the LargestAcked", func() { It("keeps all packet numbers above the LargestAcked", func() {
handler.skippedPackets = []protocol.PacketNumber{2, 5, 8, 10} handler.skippedPackets = []protocol.PacketNumber{2, 5, 8, 10}
handler.LargestAcked = 1 handler.largestAcked = 1
handler.garbageCollectSkippedPackets() handler.garbageCollectSkippedPackets()
Expect(handler.skippedPackets).To(Equal([]protocol.PacketNumber{2, 5, 8, 10})) Expect(handler.skippedPackets).To(Equal([]protocol.PacketNumber{2, 5, 8, 10}))
}) })
It("doesn't keep packet numbers below the LargestAcked", func() { It("doesn't keep packet numbers below the LargestAcked", func() {
handler.skippedPackets = []protocol.PacketNumber{1, 5, 8, 10} handler.skippedPackets = []protocol.PacketNumber{1, 5, 8, 10}
handler.LargestAcked = 5 handler.largestAcked = 5
handler.garbageCollectSkippedPackets() handler.garbageCollectSkippedPackets()
Expect(handler.skippedPackets).To(Equal([]protocol.PacketNumber{8, 10})) Expect(handler.skippedPackets).To(Equal([]protocol.PacketNumber{8, 10}))
}) })
It("deletes all packet numbers if LargestAcked is sufficiently high", func() { It("deletes all packet numbers if LargestAcked is sufficiently high", func() {
handler.skippedPackets = []protocol.PacketNumber{1, 5, 10} handler.skippedPackets = []protocol.PacketNumber{1, 5, 10}
handler.LargestAcked = 15 handler.largestAcked = 15
handler.garbageCollectSkippedPackets() handler.garbageCollectSkippedPackets()
Expect(handler.skippedPackets).To(BeEmpty()) Expect(handler.skippedPackets).To(BeEmpty())
}) })
@ -273,7 +273,7 @@ var _ = Describe("SentPacketHandler", func() {
Expect(err).ToNot(HaveOccurred()) Expect(err).ToNot(HaveOccurred())
} }
err := handler.SentPacket(retransmittablePacket(i)) err := handler.SentPacket(retransmittablePacket(i))
Expect(err).To(MatchError(ErrTooManyTrackedSentPackets)) Expect(err).To(MatchError("Too many outstanding non-acked and non-retransmitted packets"))
}) })
// TODO: add a test that the length of the retransmission queue is considered, even if packets have already been ACKed. Relevant once we drop support for QUIC 33 and earlier // TODO: add a test that the length of the retransmission queue is considered, even if packets have already been ACKed. Relevant once we drop support for QUIC 33 and earlier
@ -329,7 +329,7 @@ var _ = Describe("SentPacketHandler", func() {
Expect(handler.bytesInFlight).To(Equal(protocol.ByteCount(len(packets) - 3))) Expect(handler.bytesInFlight).To(Equal(protocol.ByteCount(len(packets) - 3)))
err = handler.ReceivedAck(&ack, 1337-1, protocol.EncryptionUnencrypted, time.Now()) err = handler.ReceivedAck(&ack, 1337-1, protocol.EncryptionUnencrypted, time.Now())
Expect(err).To(MatchError(ErrDuplicateOrOutOfOrderAck)) Expect(err).To(MatchError(ErrDuplicateOrOutOfOrderAck))
Expect(handler.LargestAcked).To(Equal(protocol.PacketNumber(3))) Expect(handler.largestAcked).To(Equal(protocol.PacketNumber(3)))
Expect(handler.bytesInFlight).To(Equal(protocol.ByteCount(len(packets) - 3))) Expect(handler.bytesInFlight).To(Equal(protocol.ByteCount(len(packets) - 3)))
}) })
@ -338,7 +338,7 @@ var _ = Describe("SentPacketHandler", func() {
LargestAcked: packets[len(packets)-1].PacketNumber + 1337, LargestAcked: packets[len(packets)-1].PacketNumber + 1337,
} }
err := handler.ReceivedAck(&ack, 1, protocol.EncryptionUnencrypted, time.Now()) err := handler.ReceivedAck(&ack, 1, protocol.EncryptionUnencrypted, time.Now())
Expect(err).To(MatchError(errAckForUnsentPacket)) Expect(err).To(MatchError("InvalidAckData: Received ACK for an unsent package"))
Expect(handler.bytesInFlight).To(Equal(protocol.ByteCount(len(packets)))) Expect(handler.bytesInFlight).To(Equal(protocol.ByteCount(len(packets))))
}) })
@ -352,7 +352,7 @@ var _ = Describe("SentPacketHandler", func() {
Expect(handler.bytesInFlight).To(Equal(protocol.ByteCount(len(packets) - 3))) Expect(handler.bytesInFlight).To(Equal(protocol.ByteCount(len(packets) - 3)))
err = handler.ReceivedAck(&ack, 1337+1, protocol.EncryptionUnencrypted, time.Now()) err = handler.ReceivedAck(&ack, 1337+1, protocol.EncryptionUnencrypted, time.Now())
Expect(err).ToNot(HaveOccurred()) Expect(err).ToNot(HaveOccurred())
Expect(handler.LargestAcked).To(Equal(protocol.PacketNumber(3))) Expect(handler.largestAcked).To(Equal(protocol.PacketNumber(3)))
Expect(handler.bytesInFlight).To(Equal(protocol.ByteCount(len(packets) - 3))) Expect(handler.bytesInFlight).To(Equal(protocol.ByteCount(len(packets) - 3)))
}) })
@ -362,7 +362,7 @@ var _ = Describe("SentPacketHandler", func() {
LowestAcked: 5, LowestAcked: 5,
} }
err := handler.ReceivedAck(&ack, 1337, protocol.EncryptionUnencrypted, time.Now()) err := handler.ReceivedAck(&ack, 1337, protocol.EncryptionUnencrypted, time.Now())
Expect(err).To(MatchError(ErrAckForSkippedPacket)) Expect(err).To(MatchError("InvalidAckData: Received an ACK for a skipped packet number"))
}) })
It("accepts an ACK that correctly nacks a skipped packet", func() { It("accepts an ACK that correctly nacks a skipped packet", func() {
@ -376,7 +376,7 @@ var _ = Describe("SentPacketHandler", func() {
} }
err := handler.ReceivedAck(&ack, 1337, protocol.EncryptionUnencrypted, time.Now()) err := handler.ReceivedAck(&ack, 1337, protocol.EncryptionUnencrypted, time.Now())
Expect(err).ToNot(HaveOccurred()) Expect(err).ToNot(HaveOccurred())
Expect(handler.LargestAcked).ToNot(BeZero()) Expect(handler.largestAcked).ToNot(BeZero())
}) })
}) })
@ -388,7 +388,7 @@ var _ = Describe("SentPacketHandler", func() {
} }
err := handler.ReceivedAck(&ack, 1, protocol.EncryptionUnencrypted, time.Now()) err := handler.ReceivedAck(&ack, 1, protocol.EncryptionUnencrypted, time.Now())
Expect(err).ToNot(HaveOccurred()) Expect(err).ToNot(HaveOccurred())
Expect(handler.LargestAcked).To(Equal(protocol.PacketNumber(5))) Expect(handler.largestAcked).To(Equal(protocol.PacketNumber(5)))
el := handler.packetHistory.Front() el := handler.packetHistory.Front()
for i := 6; i <= 10; i++ { for i := 6; i <= 10; i++ {
Expect(el.Value.PacketNumber).To(Equal(protocol.PacketNumber(i))) Expect(el.Value.PacketNumber).To(Equal(protocol.PacketNumber(i)))