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

don't pack ACK frames in the second part of a coalesced packet

Browse Source 
This prevents a possible overflow of the maximum packet size if the ACK
frames ends up being really large.
This commit is contained in:
Marten Seemann
2020-02-14 14:43:31 +07:00
parent db7fc0eb02
commit 7a532326ec
2 changed files with 50 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files

17
packet_packer.go
View File

@@ -365,7 +365,11 @@ func (p *packetPacker) maybeAppendCryptoPacket(buffer *packetBuffer, encLevel pr
return nil, err
}
}
ack := p.acks.GetAckFrame(encLevel)
var ack *wire.AckFrame
if encLevel != protocol.EncryptionHandshake || buffer.Len() == 0 {
ack = p.acks.GetAckFrame(encLevel)
}
if !s.HasData() && !hasRetransmission && ack == nil {
// nothing to send
return nil, nil
@@ -430,7 +434,7 @@ func (p *packetPacker) maybeAppendAppDataPacket(buffer *packetBuffer) (*packetCo
headerLen := header.GetLength(p.version)
maxSize := p.maxPacketSize - buffer.Len() - protocol.ByteCount(sealer.Overhead()) - headerLen
payload := p.composeNextPacket(maxSize)
payload := p.composeNextPacket(maxSize, encLevel != protocol.Encryption0RTT && buffer.Len() == 0)
// check if we have anything to send
if len(payload.frames) == 0 && payload.ack == nil {
@@ -452,14 +456,17 @@ func (p *packetPacker) maybeAppendAppDataPacket(buffer *packetBuffer) (*packetCo
return p.appendPacket(buffer, header, payload, encLevel, sealer)
}
func (p *packetPacker) composeNextPacket(maxFrameSize protocol.ByteCount) payload {
func (p *packetPacker) composeNextPacket(maxFrameSize protocol.ByteCount, ackAllowed bool) payload {
var payload payload
// TODO: we don't need to request ACKs when sending 0-RTT packets
if ack := p.acks.GetAckFrame(protocol.Encryption1RTT); ack != nil {
var ack *wire.AckFrame
if ackAllowed {
ack = p.acks.GetAckFrame(protocol.Encryption1RTT)
if ack != nil {
payload.ack = ack
payload.length += ack.Length(p.version)
}
}
for {
remainingLen := maxFrameSize - payload.length

38
packet_packer_test.go
View File

@@ -647,7 +647,7 @@ var _ = Describe("Packet packer", func() {
checkLength(p.buffer.Data)
})
It("packs a coalesced packet", func() {
It("packs a coalesced packet with Initial / Handshake", func() {
pnManager.EXPECT().PeekPacketNumber(protocol.EncryptionInitial).Return(protocol.PacketNumber(0x24), protocol.PacketNumberLen2)
pnManager.EXPECT().PopPacketNumber(protocol.EncryptionInitial).Return(protocol.PacketNumber(0x24))
pnManager.EXPECT().PeekPacketNumber(protocol.EncryptionHandshake).Return(protocol.PacketNumber(0x42), protocol.PacketNumberLen2)
@@ -656,7 +656,7 @@ var _ = Describe("Packet packer", func() {
sealingManager.EXPECT().GetHandshakeSealer().Return(getSealer(), nil)
sealingManager.EXPECT().Get1RTTSealer().Return(nil, handshake.ErrKeysNotYetAvailable)
ackFramer.EXPECT().GetAckFrame(protocol.EncryptionInitial)
ackFramer.EXPECT().GetAckFrame(protocol.EncryptionHandshake)
// don't EXPECT any calls for a Handshake ACK frame
initialStream.EXPECT().HasData().Return(true).Times(2)
initialStream.EXPECT().PopCryptoFrame(gomock.Any()).DoAndReturn(func(size protocol.ByteCount) *wire.CryptoFrame {
return &wire.CryptoFrame{Offset: 0x42, Data: []byte("initial")}
@@ -683,6 +683,40 @@ var _ = Describe("Packet packer", func() {
Expect(rest).To(BeEmpty())
})
It("packs a coalesced packet with Handshake / 1-RTT", func() {
pnManager.EXPECT().PeekPacketNumber(protocol.EncryptionHandshake).Return(protocol.PacketNumber(0x24), protocol.PacketNumberLen2)
pnManager.EXPECT().PopPacketNumber(protocol.EncryptionHandshake).Return(protocol.PacketNumber(0x24))
pnManager.EXPECT().PeekPacketNumber(protocol.Encryption1RTT).Return(protocol.PacketNumber(0x42), protocol.PacketNumberLen2)
pnManager.EXPECT().PopPacketNumber(protocol.Encryption1RTT).Return(protocol.PacketNumber(0x42))
sealingManager.EXPECT().GetInitialSealer().Return(nil, handshake.ErrKeysDropped)
sealingManager.EXPECT().GetHandshakeSealer().Return(getSealer(), nil)
sealingManager.EXPECT().Get1RTTSealer().Return(getSealer(), nil)
ackFramer.EXPECT().GetAckFrame(protocol.EncryptionHandshake)
// don't EXPECT any calls for a 1-RTT ACK frame
handshakeStream.EXPECT().HasData().Return(true).Times(2)
handshakeStream.EXPECT().PopCryptoFrame(gomock.Any()).DoAndReturn(func(size protocol.ByteCount) *wire.CryptoFrame {
return &wire.CryptoFrame{Offset: 0x1337, Data: []byte("handshake")}
})
expectAppendControlFrames()
expectAppendStreamFrames(ackhandler.Frame{Frame: &wire.StreamFrame{Data: []byte("foobar")}})
p, err := packer.PackCoalescedPacket()
Expect(err).ToNot(HaveOccurred())
Expect(p.packets).To(HaveLen(2))
Expect(p.packets[0].EncryptionLevel()).To(Equal(protocol.EncryptionHandshake))
Expect(p.packets[0].frames).To(HaveLen(1))
Expect(p.packets[0].frames[0].Frame.(*wire.CryptoFrame).Data).To(Equal([]byte("handshake")))
Expect(p.packets[1].EncryptionLevel()).To(Equal(protocol.Encryption1RTT))
Expect(p.packets[1].frames).To(HaveLen(1))
Expect(p.packets[1].frames[0].Frame.(*wire.StreamFrame).Data).To(Equal([]byte("foobar")))
hdr, _, rest, err := wire.ParsePacket(p.buffer.Data, 0)
Expect(err).ToNot(HaveOccurred())
Expect(hdr.Type).To(Equal(protocol.PacketTypeHandshake))
hdr, _, rest, err = wire.ParsePacket(rest, 0)
Expect(err).ToNot(HaveOccurred())
Expect(hdr.IsLongHeader).To(BeFalse())
Expect(rest).To(BeEmpty())
})
It("doesn't add a coalesced packet if the remaining size is smaller than MaxCoalescedPacketSize", func() {
pnManager.EXPECT().PeekPacketNumber(protocol.EncryptionInitial).Return(protocol.PacketNumber(0x24), protocol.PacketNumberLen2)
pnManager.EXPECT().PopPacketNumber(protocol.EncryptionInitial).Return(protocol.PacketNumber(0x24))