discard DATAGRAM frames that don't fit into packets without an ACK (#4221)

If the packet doesn't contain an ACK (i.e. the payload is empty),
there's no point retrying to pack it later. It's extremely unlikely that
the available packet size will suddenly increase.
This commit is contained in:
Marten Seemann 2024-01-01 10:17:25 +07:00 committed by GitHub
parent d6e3f3229f
commit 7f080dd54b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 41 additions and 14 deletions

View file

@ -516,7 +516,6 @@ var _ = Describe("Packet packer", func() {
buffer.Data = append(buffer.Data, []byte("foobar")...)
p, err := packer.AppendPacket(buffer, maxPacketSize, protocol.Version1)
Expect(err).ToNot(HaveOccurred())
Expect(p).ToNot(BeNil())
b, err := f.Append(nil, protocol.Version1)
Expect(err).ToNot(HaveOccurred())
Expect(p.Frames).To(BeEmpty())
@ -535,7 +534,6 @@ var _ = Describe("Packet packer", func() {
sealingManager.EXPECT().Get1RTTSealer().Return(getSealer(), nil)
p, err := packer.AppendPacket(getPacketBuffer(), maxPacketSize, protocol.Version1)
Expect(err).NotTo(HaveOccurred())
Expect(p).ToNot(BeNil())
Expect(p.Ack).To(Equal(ack))
})
@ -553,7 +551,6 @@ var _ = Describe("Packet packer", func() {
expectAppendStreamFrames()
buffer := getPacketBuffer()
p, err := packer.AppendPacket(buffer, maxPacketSize, protocol.Version1)
Expect(p).ToNot(BeNil())
Expect(err).ToNot(HaveOccurred())
Expect(p.Frames).To(HaveLen(2))
for i, f := range p.Frames {
@ -577,7 +574,6 @@ var _ = Describe("Packet packer", func() {
expectAppendStreamFrames()
buffer := getPacketBuffer()
p, err := packer.AppendPacket(buffer, maxPacketSize, protocol.Version1)
Expect(p).ToNot(BeNil())
Expect(err).ToNot(HaveOccurred())
Expect(p.Frames).To(HaveLen(3))
for i, f := range p.Frames {
@ -614,7 +610,6 @@ var _ = Describe("Packet packer", func() {
framer.EXPECT().HasData()
buffer := getPacketBuffer()
p, err := packer.AppendPacket(buffer, maxPacketSize, protocol.Version1)
Expect(p).ToNot(BeNil())
Expect(err).ToNot(HaveOccurred())
Expect(p.Frames).To(HaveLen(1))
Expect(p.Frames[0].Frame).To(Equal(f))
@ -643,15 +638,42 @@ var _ = Describe("Packet packer", func() {
framer.EXPECT().HasData()
buffer := getPacketBuffer()
p, err := packer.AppendPacket(buffer, maxPacketSize, protocol.Version1)
Expect(p).ToNot(BeNil())
Expect(err).ToNot(HaveOccurred())
Expect(p.Ack).ToNot(BeNil())
Expect(p.Frames).To(BeEmpty())
Expect(buffer.Data).ToNot(BeEmpty())
Expect(datagramQueue.Peek()).To(Equal(f)) // make sure the frame is still there
datagramQueue.CloseWithError(nil)
Eventually(done).Should(BeClosed())
})
It("discards a DATAGRAM frame if it doesn't fit into a packet that doesn't contain an ACK", func() {
ackFramer.EXPECT().GetAckFrame(protocol.Encryption1RTT, true)
pnManager.EXPECT().PeekPacketNumber(protocol.Encryption1RTT).Return(protocol.PacketNumber(0x42), protocol.PacketNumberLen2)
sealingManager.EXPECT().Get1RTTSealer().Return(getSealer(), nil)
f := &wire.DatagramFrame{
DataLenPresent: true,
Data: make([]byte, maxPacketSize+10), // won't fit
}
done := make(chan struct{})
go func() {
defer GinkgoRecover()
defer close(done)
datagramQueue.AddAndWait(f)
}()
// make sure the DATAGRAM has actually been queued
time.Sleep(scaleDuration(20 * time.Millisecond))
framer.EXPECT().HasData()
buffer := getPacketBuffer()
p, err := packer.AppendPacket(buffer, maxPacketSize, protocol.Version1)
Expect(err).To(MatchError(errNothingToPack))
Expect(p.Frames).To(BeEmpty())
Expect(p.Ack).To(BeNil())
Expect(datagramQueue.Peek()).To(BeNil())
Eventually(done).Should(BeClosed())
})
It("accounts for the space consumed by control frames", func() {
pnManager.EXPECT().PeekPacketNumber(protocol.Encryption1RTT).Return(protocol.PacketNumber(0x42), protocol.PacketNumberLen2)
sealingManager.EXPECT().Get1RTTSealer().Return(getSealer(), nil)
@ -777,7 +799,6 @@ var _ = Describe("Packet packer", func() {
expectAppendControlFrames()
expectAppendStreamFrames(ackhandler.StreamFrame{Frame: f1}, ackhandler.StreamFrame{Frame: f2}, ackhandler.StreamFrame{Frame: f3})
p, err := packer.AppendPacket(getPacketBuffer(), maxPacketSize, protocol.Version1)
Expect(p).ToNot(BeNil())
Expect(err).ToNot(HaveOccurred())
Expect(p.Frames).To(BeEmpty())
Expect(p.StreamFrames).To(HaveLen(3))
@ -797,7 +818,6 @@ var _ = Describe("Packet packer", func() {
expectAppendControlFrames()
expectAppendStreamFrames()
p, err := packer.AppendPacket(getPacketBuffer(), maxPacketSize, protocol.Version1)
Expect(p).ToNot(BeNil())
Expect(err).ToNot(HaveOccurred())
Expect(p.Ack).ToNot(BeNil())
Expect(p.Frames).To(BeEmpty())
@ -814,7 +834,6 @@ var _ = Describe("Packet packer", func() {
expectAppendControlFrames()
expectAppendStreamFrames()
p, err := packer.AppendPacket(getPacketBuffer(), maxPacketSize, protocol.Version1)
Expect(p).ToNot(BeNil())
Expect(err).ToNot(HaveOccurred())
var hasPing bool
for _, f := range p.Frames {
@ -833,7 +852,6 @@ var _ = Describe("Packet packer", func() {
expectAppendControlFrames()
expectAppendStreamFrames()
p, err = packer.AppendPacket(getPacketBuffer(), maxPacketSize, protocol.Version1)
Expect(p).ToNot(BeNil())
Expect(err).ToNot(HaveOccurred())
Expect(p.Ack).ToNot(BeNil())
Expect(p.Frames).To(BeEmpty())
@ -883,7 +901,6 @@ var _ = Describe("Packet packer", func() {
expectAppendControlFrames(ackhandler.Frame{Frame: &wire.MaxDataFrame{}})
p, err := packer.AppendPacket(getPacketBuffer(), maxPacketSize, protocol.Version1)
Expect(err).ToNot(HaveOccurred())
Expect(p).ToNot(BeNil())
Expect(p.Frames).ToNot(ContainElement(&wire.PingFrame{}))
})
})