mirrors/uquic
1
0
Fork 0
mirror of https://github.com/refraction-networking/uquic.git synced 2025-04-04 12:47:36 +03:00
Code Issues Projects Releases Packages Wiki Activity

fix usage of ackhandler.Packet pool for non-ack-eliciting packets (#3538)

Browse source
This commit is contained in:
Marten Seemann 2022-08-30 14:09:11 +03:00 • committed by GitHub
parent 31995601a9
commit 80c3afed34
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 66 additions and 54 deletions
Download patch file Download diff file Expand all files Collapse all files

16
internal/ackhandler/sent_packet_handler.go
View file

@ -226,14 +226,20 @@ func (h *sentPacketHandler) packetsInFlight() int {
return packetsInFlight return packetsInFlight
} }
func (h *sentPacketHandler) SentPacket(packet *Packet) { func (h *sentPacketHandler) SentPacket(p *Packet) {
h.bytesSent += packet.Length h.bytesSent += p.Length
// For the client, drop the Initial packet number space when the first Handshake packet is sent. // For the client, drop the Initial packet number space when the first Handshake packet is sent.
if h.perspective == protocol.PerspectiveClient && packet.EncryptionLevel == protocol.EncryptionHandshake && h.initialPackets != nil { if h.perspective == protocol.PerspectiveClient && p.EncryptionLevel == protocol.EncryptionHandshake && h.initialPackets != nil {
h.dropPackets(protocol.EncryptionInitial) h.dropPackets(protocol.EncryptionInitial)
} }
isAckEliciting := h.sentPacketImpl(packet) isAckEliciting := h.sentPacketImpl(p)
h.getPacketNumberSpace(packet.EncryptionLevel).history.SentPacket(packet, isAckEliciting) if isAckEliciting {
h.getPacketNumberSpace(p.EncryptionLevel).history.SentAckElicitingPacket(p)
} else {
h.getPacketNumberSpace(p.EncryptionLevel).history.SentNonAckElicitingPacket(p.PacketNumber, p.EncryptionLevel, p.SendTime)
putPacket(p)
p = nil //nolint:ineffassign // This is just to be on the safe side.
}
if h.tracer != nil && isAckEliciting { if h.tracer != nil && isAckEliciting {
h.tracer.UpdatedMetrics(h.rttStats, h.congestion.GetCongestionWindow(), h.bytesInFlight, h.packetsInFlight()) h.tracer.UpdatedMetrics(h.rttStats, h.congestion.GetCongestionWindow(), h.bytesInFlight, h.packetsInFlight())
} }

42
internal/ackhandler/sent_packet_history.go
View file

@ -27,31 +27,37 @@ func newSentPacketHistory(rttStats *utils.RTTStats) *sentPacketHistory {
} }
} }
func (h *sentPacketHistory) SentPacket(p *Packet, isAckEliciting bool) { func (h *sentPacketHistory) SentNonAckElicitingPacket(pn protocol.PacketNumber, encLevel protocol.EncryptionLevel, t time.Time) {
if p.PacketNumber <= h.highestSent { h.registerSentPacket(pn, encLevel, t)
}
func (h *sentPacketHistory) SentAckElicitingPacket(p *Packet) {
h.registerSentPacket(p.PacketNumber, p.EncryptionLevel, p.SendTime)
var el *list.Element[*Packet]
if p.outstanding() {
el = h.outstandingPacketList.PushBack(p)
} else {
el = h.etcPacketList.PushBack(p)
}
h.packetMap[p.PacketNumber] = el
}
func (h *sentPacketHistory) registerSentPacket(pn protocol.PacketNumber, encLevel protocol.EncryptionLevel, t time.Time) {
if pn <= h.highestSent {
panic("non-sequential packet number use") panic("non-sequential packet number use")
} }
// Skipped packet numbers. // Skipped packet numbers.
for pn := h.highestSent + 1; pn < p.PacketNumber; pn++ { for p := h.highestSent + 1; p < pn; p++ {
el := h.etcPacketList.PushBack(&Packet{ el := h.etcPacketList.PushBack(&Packet{
PacketNumber: pn, PacketNumber: p,
EncryptionLevel: p.EncryptionLevel, EncryptionLevel: encLevel,
SendTime: p.SendTime, SendTime: t,
skippedPacket: true, skippedPacket: true,
}) })
h.packetMap[pn] = el h.packetMap[p] = el
}
h.highestSent = p.PacketNumber
if isAckEliciting {
var el *list.Element[*Packet]
if p.outstanding() {
el = h.outstandingPacketList.PushBack(p)
} else {
el = h.etcPacketList.PushBack(p)
}
h.packetMap[p.PacketNumber] = el
} }
h.highestSent = pn
} }
// Iterate iterates through all packets. // Iterate iterates through all packets.

62
internal/ackhandler/sent_packet_history_test.go
View file

@ -53,16 +53,16 @@ var _ = Describe("SentPacketHistory", func() {
}) })
It("saves sent packets", func() { It("saves sent packets", func() {
hist.SentPacket(&Packet{PacketNumber: 1}, true) hist.SentAckElicitingPacket(&Packet{PacketNumber: 1})
hist.SentPacket(&Packet{PacketNumber: 3}, true) hist.SentAckElicitingPacket(&Packet{PacketNumber: 3})
hist.SentPacket(&Packet{PacketNumber: 4}, true) hist.SentAckElicitingPacket(&Packet{PacketNumber: 4})
expectInHistory([]protocol.PacketNumber{1, 3, 4}) expectInHistory([]protocol.PacketNumber{1, 3, 4})
}) })
It("doesn't save non-ack-eliciting packets", func() { It("doesn't save non-ack-eliciting packets", func() {
hist.SentPacket(&Packet{PacketNumber: 1}, true) hist.SentAckElicitingPacket(&Packet{PacketNumber: 1})
hist.SentPacket(&Packet{PacketNumber: 3}, false) hist.SentNonAckElicitingPacket(3, protocol.EncryptionLevel(0), time.Time{})
hist.SentPacket(&Packet{PacketNumber: 4}, true) hist.SentAckElicitingPacket(&Packet{PacketNumber: 4})
expectInHistory([]protocol.PacketNumber{1, 4}) expectInHistory([]protocol.PacketNumber{1, 4})
hist.Iterate(func(p *Packet) (bool, error) { hist.Iterate(func(p *Packet) (bool, error) {
Expect(p.PacketNumber).ToNot(Equal(protocol.PacketNumber(3))) Expect(p.PacketNumber).ToNot(Equal(protocol.PacketNumber(3)))
@ -71,9 +71,9 @@ var _ = Describe("SentPacketHistory", func() {
}) })
It("gets the length", func() { It("gets the length", func() {
hist.SentPacket(&Packet{PacketNumber: 0}, true) hist.SentAckElicitingPacket(&Packet{PacketNumber: 0})
hist.SentPacket(&Packet{PacketNumber: 1}, true) hist.SentAckElicitingPacket(&Packet{PacketNumber: 1})
hist.SentPacket(&Packet{PacketNumber: 2}, true) hist.SentAckElicitingPacket(&Packet{PacketNumber: 2})
Expect(hist.Len()).To(Equal(3)) Expect(hist.Len()).To(Equal(3))
}) })
@ -83,16 +83,16 @@ var _ = Describe("SentPacketHistory", func() {
}) })
It("gets the first outstanding packet", func() { It("gets the first outstanding packet", func() {
hist.SentPacket(&Packet{PacketNumber: 2}, true) hist.SentAckElicitingPacket(&Packet{PacketNumber: 2})
hist.SentPacket(&Packet{PacketNumber: 3}, true) hist.SentAckElicitingPacket(&Packet{PacketNumber: 3})
front := hist.FirstOutstanding() front := hist.FirstOutstanding()
Expect(front).ToNot(BeNil()) Expect(front).ToNot(BeNil())
Expect(front.PacketNumber).To(Equal(protocol.PacketNumber(2))) Expect(front.PacketNumber).To(Equal(protocol.PacketNumber(2)))
}) })
It("doesn't regard path MTU packets as outstanding", func() { It("doesn't regard path MTU packets as outstanding", func() {
hist.SentPacket(&Packet{PacketNumber: 2}, true) hist.SentAckElicitingPacket(&Packet{PacketNumber: 2})
hist.SentPacket(&Packet{PacketNumber: 4, IsPathMTUProbePacket: true}, true) hist.SentAckElicitingPacket(&Packet{PacketNumber: 4, IsPathMTUProbePacket: true})
front := hist.FirstOutstanding() front := hist.FirstOutstanding()
Expect(front).ToNot(BeNil()) Expect(front).ToNot(BeNil())
Expect(front.PacketNumber).To(Equal(protocol.PacketNumber(2))) Expect(front.PacketNumber).To(Equal(protocol.PacketNumber(2)))
@ -100,25 +100,25 @@ var _ = Describe("SentPacketHistory", func() {
}) })
It("removes packets", func() { It("removes packets", func() {
hist.SentPacket(&Packet{PacketNumber: 1}, true) hist.SentAckElicitingPacket(&Packet{PacketNumber: 1})
hist.SentPacket(&Packet{PacketNumber: 4}, true) hist.SentAckElicitingPacket(&Packet{PacketNumber: 4})
hist.SentPacket(&Packet{PacketNumber: 8}, true) hist.SentAckElicitingPacket(&Packet{PacketNumber: 8})
err := hist.Remove(4) err := hist.Remove(4)
Expect(err).ToNot(HaveOccurred()) Expect(err).ToNot(HaveOccurred())
expectInHistory([]protocol.PacketNumber{1, 8}) expectInHistory([]protocol.PacketNumber{1, 8})
}) })
It("errors when trying to remove a non existing packet", func() { It("errors when trying to remove a non existing packet", func() {
hist.SentPacket(&Packet{PacketNumber: 1}, true) hist.SentAckElicitingPacket(&Packet{PacketNumber: 1})
err := hist.Remove(2) err := hist.Remove(2)
Expect(err).To(MatchError("packet 2 not found in sent packet history")) Expect(err).To(MatchError("packet 2 not found in sent packet history"))
}) })
Context("iterating", func() { Context("iterating", func() {
BeforeEach(func() { BeforeEach(func() {
hist.SentPacket(&Packet{PacketNumber: 1}, true) hist.SentAckElicitingPacket(&Packet{PacketNumber: 1})
hist.SentPacket(&Packet{PacketNumber: 4}, true) hist.SentAckElicitingPacket(&Packet{PacketNumber: 4})
hist.SentPacket(&Packet{PacketNumber: 8}, true) hist.SentAckElicitingPacket(&Packet{PacketNumber: 8})
}) })
It("iterates over all packets", func() { It("iterates over all packets", func() {
@ -197,29 +197,29 @@ var _ = Describe("SentPacketHistory", func() {
Context("outstanding packets", func() { Context("outstanding packets", func() {
It("says if it has outstanding packets", func() { It("says if it has outstanding packets", func() {
Expect(hist.HasOutstandingPackets()).To(BeFalse()) Expect(hist.HasOutstandingPackets()).To(BeFalse())
hist.SentPacket(&Packet{EncryptionLevel: protocol.Encryption1RTT}, true) hist.SentAckElicitingPacket(&Packet{EncryptionLevel: protocol.Encryption1RTT})
Expect(hist.HasOutstandingPackets()).To(BeTrue()) Expect(hist.HasOutstandingPackets()).To(BeTrue())
}) })
It("accounts for deleted packets", func() { It("accounts for deleted packets", func() {
hist.SentPacket(&Packet{ hist.SentAckElicitingPacket(&Packet{
PacketNumber: 10, PacketNumber: 10,
EncryptionLevel: protocol.Encryption1RTT, EncryptionLevel: protocol.Encryption1RTT,
}, true) })
Expect(hist.HasOutstandingPackets()).To(BeTrue()) Expect(hist.HasOutstandingPackets()).To(BeTrue())
Expect(hist.Remove(10)).To(Succeed()) Expect(hist.Remove(10)).To(Succeed())
Expect(hist.HasOutstandingPackets()).To(BeFalse()) Expect(hist.HasOutstandingPackets()).To(BeFalse())
}) })
It("counts the number of packets", func() { It("counts the number of packets", func() {
hist.SentPacket(&Packet{ hist.SentAckElicitingPacket(&Packet{
PacketNumber: 10, PacketNumber: 10,
EncryptionLevel: protocol.Encryption1RTT, EncryptionLevel: protocol.Encryption1RTT,
}, true) })
hist.SentPacket(&Packet{ hist.SentAckElicitingPacket(&Packet{
PacketNumber: 11, PacketNumber: 11,
EncryptionLevel: protocol.Encryption1RTT, EncryptionLevel: protocol.Encryption1RTT,
}, true) })
Expect(hist.Remove(11)).To(Succeed()) Expect(hist.Remove(11)).To(Succeed())
Expect(hist.HasOutstandingPackets()).To(BeTrue()) Expect(hist.HasOutstandingPackets()).To(BeTrue())
Expect(hist.Remove(10)).To(Succeed()) Expect(hist.Remove(10)).To(Succeed())
@ -237,7 +237,7 @@ var _ = Describe("SentPacketHistory", func() {
It("deletes old packets after 3 PTOs", func() { It("deletes old packets after 3 PTOs", func() {
now := time.Now() now := time.Now()
hist.SentPacket(&Packet{PacketNumber: 10, SendTime: now.Add(-3 * pto), declaredLost: true}, true) hist.SentAckElicitingPacket(&Packet{PacketNumber: 10, SendTime: now.Add(-3 * pto), declaredLost: true})
expectInHistory([]protocol.PacketNumber{10}) expectInHistory([]protocol.PacketNumber{10})
hist.DeleteOldPackets(now.Add(-time.Nanosecond)) hist.DeleteOldPackets(now.Add(-time.Nanosecond))
expectInHistory([]protocol.PacketNumber{10}) expectInHistory([]protocol.PacketNumber{10})
@ -247,8 +247,8 @@ var _ = Describe("SentPacketHistory", func() {
It("doesn't delete a packet if it hasn't been declared lost yet", func() { It("doesn't delete a packet if it hasn't been declared lost yet", func() {
now := time.Now() now := time.Now()
hist.SentPacket(&Packet{PacketNumber: 10, SendTime: now.Add(-3 * pto), declaredLost: true}, true) hist.SentAckElicitingPacket(&Packet{PacketNumber: 10, SendTime: now.Add(-3 * pto), declaredLost: true})
hist.SentPacket(&Packet{PacketNumber: 11, SendTime: now.Add(-3 * pto), declaredLost: false}, true) hist.SentAckElicitingPacket(&Packet{PacketNumber: 11, SendTime: now.Add(-3 * pto), declaredLost: false})
expectInHistory([]protocol.PacketNumber{10, 11}) expectInHistory([]protocol.PacketNumber{10, 11})
hist.DeleteOldPackets(now) hist.DeleteOldPackets(now)
expectInHistory([]protocol.PacketNumber{11}) expectInHistory([]protocol.PacketNumber{11})
@ -256,7 +256,7 @@ var _ = Describe("SentPacketHistory", func() {
It("deletes skipped packets", func() { It("deletes skipped packets", func() {
now := time.Now() now := time.Now()
hist.SentPacket(&Packet{PacketNumber: 10, SendTime: now.Add(-3 * pto)}, true) hist.SentAckElicitingPacket(&Packet{PacketNumber: 10, SendTime: now.Add(-3 * pto)})
expectInHistory([]protocol.PacketNumber{10}) expectInHistory([]protocol.PacketNumber{10})
Expect(hist.Len()).To(Equal(11)) Expect(hist.Len()).To(Equal(11))
hist.DeleteOldPackets(now) hist.DeleteOldPackets(now)