Merge pull request #2388 from lucas-clemente/crypto-retransmissions

refactor the way crypto retransmissions are packed
This commit is contained in:
Marten Seemann
2020-02-28 15:19:15 +07:00
committed by GitHub
6 changed files with 154 additions and 14 deletions

View File

@@ -69,3 +69,34 @@ func (f *CryptoFrame) MaxDataLen(maxSize protocol.ByteCount) protocol.ByteCount
}
return maxDataLen
}
// MaybeSplitOffFrame splits a frame such that it is not bigger than n bytes.
// It returns if the frame was actually split.
// The frame might not be split if:
// * the size is large enough to fit the whole frame
// * the size is too small to fit even a 1-byte frame. In that case, the frame returned is nil.
func (f *CryptoFrame) MaybeSplitOffFrame(maxSize protocol.ByteCount, version protocol.VersionNumber) (*CryptoFrame, bool /* was splitting required */) {
if f.Length(version) <= maxSize {
return nil, false
}
n := f.MaxDataLen(maxSize)
if n == 0 {
return nil, true
}
newLen := protocol.ByteCount(len(f.Data)) - n
new := &CryptoFrame{}
new.Offset = f.Offset
new.Data = make([]byte, newLen)
// swap the data slices
new.Data, f.Data = f.Data, new.Data
copy(f.Data, new.Data[n:])
new.Data = new.Data[:n]
f.Offset += n
return new, true
}

View File

@@ -102,4 +102,46 @@ var _ = Describe("CRYPTO frame", func() {
Expect(f.Length(versionIETFFrames)).To(Equal(1 + utils.VarIntLen(0x1337) + utils.VarIntLen(6) + 6))
})
})
Context("splitting", func() {
It("splits a frame", func() {
f := &CryptoFrame{
Offset: 0x1337,
Data: []byte("foobar"),
}
hdrLen := f.Length(versionIETFFrames) - 6
new, needsSplit := f.MaybeSplitOffFrame(hdrLen+3, versionIETFFrames)
Expect(needsSplit).To(BeTrue())
Expect(new.Data).To(Equal([]byte("foo")))
Expect(new.Offset).To(Equal(protocol.ByteCount(0x1337)))
Expect(f.Data).To(Equal([]byte("bar")))
Expect(f.Offset).To(Equal(protocol.ByteCount(0x1337 + 3)))
})
It("doesn't split if there's enough space in the frame", func() {
f := &CryptoFrame{
Offset: 0x1337,
Data: []byte("foobar"),
}
f, needsSplit := f.MaybeSplitOffFrame(f.Length(versionIETFFrames), versionIETFFrames)
Expect(needsSplit).To(BeFalse())
Expect(f).To(BeNil())
})
It("doesn't split if the size is too small", func() {
f := &CryptoFrame{
Offset: 0x1337,
Data: []byte("foobar"),
}
length := f.Length(versionIETFFrames) - 6
for i := protocol.ByteCount(0); i <= length; i++ {
f, needsSplit := f.MaybeSplitOffFrame(i, versionIETFFrames)
Expect(needsSplit).To(BeTrue())
Expect(f).To(BeNil())
}
f, needsSplit := f.MaybeSplitOffFrame(length+1, versionIETFFrames)
Expect(needsSplit).To(BeTrue())
Expect(f).ToNot(BeNil())
})
})
})

View File

@@ -636,12 +636,6 @@ func (p *packetPacker) getLongHeader(encLevel protocol.EncryptionLevel) *wire.Ex
hdr.PacketNumber = pn
hdr.PacketNumberLen = pnLen
if encLevel != protocol.Encryption0RTT {
// Always send long header packets with the maximum packet number length.
// This simplifies retransmissions: Since the header can't get any larger,
// we don't need to split CRYPTO frames.
hdr.PacketNumberLen = protocol.PacketNumberLen4
}
switch encLevel {
case protocol.EncryptionInitial:

View File

@@ -113,12 +113,11 @@ var _ = Describe("Packet packer", func() {
Context("generating a packet header", func() {
It("uses the Long Header format", func() {
pnManager.EXPECT().PeekPacketNumber(protocol.EncryptionHandshake).Return(protocol.PacketNumber(0x42), protocol.PacketNumberLen2)
pnManager.EXPECT().PeekPacketNumber(protocol.EncryptionHandshake).Return(protocol.PacketNumber(0x42), protocol.PacketNumberLen3)
h := packer.getLongHeader(protocol.EncryptionHandshake)
Expect(h.IsLongHeader).To(BeTrue())
Expect(h.PacketNumber).To(Equal(protocol.PacketNumber(0x42)))
// long headers always use 4 byte packet numbers, no matter what the packet number generator says
Expect(h.PacketNumberLen).To(Equal(protocol.PacketNumberLen4))
Expect(h.PacketNumberLen).To(Equal(protocol.PacketNumberLen3))
Expect(h.Version).To(Equal(packer.version))
})

View File

@@ -56,10 +56,15 @@ func (q *retransmissionQueue) AddAppData(f wire.Frame) {
func (q *retransmissionQueue) GetInitialFrame(maxLen protocol.ByteCount) wire.Frame {
if len(q.initialCryptoData) > 0 {
if f := q.initialCryptoData[0]; f.Length(q.version) <= maxLen {
f := q.initialCryptoData[0]
newFrame, needsSplit := f.MaybeSplitOffFrame(maxLen, q.version)
if newFrame == nil && !needsSplit { // the whole frame fits
q.initialCryptoData = q.initialCryptoData[1:]
return f
}
if newFrame != nil { // frame was split. Leave the original frame in the queue.
return newFrame
}
}
if len(q.initial) == 0 {
return nil
@@ -74,10 +79,15 @@ func (q *retransmissionQueue) GetInitialFrame(maxLen protocol.ByteCount) wire.Fr
func (q *retransmissionQueue) GetHandshakeFrame(maxLen protocol.ByteCount) wire.Frame {
if len(q.handshakeCryptoData) > 0 {
if f := q.handshakeCryptoData[0]; f.Length(q.version) <= maxLen {
f := q.handshakeCryptoData[0]
newFrame, needsSplit := f.MaybeSplitOffFrame(maxLen, q.version)
if newFrame == nil && !needsSplit { // the whole frame fits
q.handshakeCryptoData = q.handshakeCryptoData[1:]
return f
}
if newFrame != nil { // frame was split. Leave the original frame in the queue.
return newFrame
}
}
if len(q.handshake) == 0 {
return nil

View File

@@ -36,11 +36,43 @@ var _ = Describe("Retransmission queue", func() {
f := &wire.CryptoFrame{Data: []byte("foobar")}
q.AddInitial(f)
Expect(q.HasInitialData()).To(BeTrue())
Expect(q.GetInitialFrame(f.Length(version) - 1)).To(BeNil())
Expect(q.GetInitialFrame(f.Length(version))).To(Equal(f))
Expect(q.HasInitialData()).To(BeFalse())
})
It("returns split CRYPTO frames", func() {
f := &wire.CryptoFrame{
Offset: 100,
Data: []byte("foobar"),
}
q.AddInitial(f)
Expect(q.HasInitialData()).To(BeTrue())
f1 := q.GetInitialFrame(f.Length(version) - 3)
Expect(f1).ToNot(BeNil())
Expect(f1).To(BeAssignableToTypeOf(&wire.CryptoFrame{}))
Expect(f1.(*wire.CryptoFrame).Data).To(Equal([]byte("foo")))
Expect(f1.(*wire.CryptoFrame).Offset).To(Equal(protocol.ByteCount(100)))
Expect(q.HasInitialData()).To(BeTrue())
f2 := q.GetInitialFrame(protocol.MaxByteCount)
Expect(f2).ToNot(BeNil())
Expect(f2).To(BeAssignableToTypeOf(&wire.CryptoFrame{}))
Expect(f2.(*wire.CryptoFrame).Data).To(Equal([]byte("bar")))
Expect(f2.(*wire.CryptoFrame).Offset).To(Equal(protocol.ByteCount(103)))
Expect(q.HasInitialData()).To(BeFalse())
})
It("returns other frames when a CRYPTO frame wouldn't fit", func() {
f := &wire.CryptoFrame{Data: []byte("foobar")}
q.AddInitial(f)
q.AddInitial(&wire.PingFrame{})
f1 := q.GetInitialFrame(2) // too small for a CRYPTO frame
Expect(f1).ToNot(BeNil())
Expect(f1).To(BeAssignableToTypeOf(&wire.PingFrame{}))
Expect(q.HasInitialData()).To(BeTrue())
f2 := q.GetInitialFrame(protocol.MaxByteCount)
Expect(f2).To(Equal(f))
})
It("retrieves both a CRYPTO frame and a control frame", func() {
cf := &wire.MaxDataFrame{ByteOffset: 0x42}
f := &wire.CryptoFrame{Data: []byte("foobar")}
@@ -80,11 +112,43 @@ var _ = Describe("Retransmission queue", func() {
f := &wire.CryptoFrame{Data: []byte("foobar")}
q.AddHandshake(f)
Expect(q.HasHandshakeData()).To(BeTrue())
Expect(q.GetHandshakeFrame(f.Length(version) - 1)).To(BeNil())
Expect(q.GetHandshakeFrame(f.Length(version))).To(Equal(f))
Expect(q.HasHandshakeData()).To(BeFalse())
})
It("returns split CRYPTO frames", func() {
f := &wire.CryptoFrame{
Offset: 100,
Data: []byte("foobar"),
}
q.AddHandshake(f)
Expect(q.HasHandshakeData()).To(BeTrue())
f1 := q.GetHandshakeFrame(f.Length(version) - 3)
Expect(f1).ToNot(BeNil())
Expect(f1).To(BeAssignableToTypeOf(&wire.CryptoFrame{}))
Expect(f1.(*wire.CryptoFrame).Data).To(Equal([]byte("foo")))
Expect(f1.(*wire.CryptoFrame).Offset).To(Equal(protocol.ByteCount(100)))
Expect(q.HasHandshakeData()).To(BeTrue())
f2 := q.GetHandshakeFrame(protocol.MaxByteCount)
Expect(f2).ToNot(BeNil())
Expect(f2).To(BeAssignableToTypeOf(&wire.CryptoFrame{}))
Expect(f2.(*wire.CryptoFrame).Data).To(Equal([]byte("bar")))
Expect(f2.(*wire.CryptoFrame).Offset).To(Equal(protocol.ByteCount(103)))
Expect(q.HasHandshakeData()).To(BeFalse())
})
It("returns other frames when a CRYPTO frame wouldn't fit", func() {
f := &wire.CryptoFrame{Data: []byte("foobar")}
q.AddHandshake(f)
q.AddHandshake(&wire.PingFrame{})
f1 := q.GetHandshakeFrame(2) // too small for a CRYPTO frame
Expect(f1).ToNot(BeNil())
Expect(f1).To(BeAssignableToTypeOf(&wire.PingFrame{}))
Expect(q.HasHandshakeData()).To(BeTrue())
f2 := q.GetHandshakeFrame(protocol.MaxByteCount)
Expect(f2).To(Equal(f))
})
It("retrieves both a CRYPTO frame and a control frame", func() {
cf := &wire.MaxDataFrame{ByteOffset: 0x42}
f := &wire.CryptoFrame{Data: []byte("foobar")}
@@ -96,7 +160,7 @@ var _ = Describe("Retransmission queue", func() {
Expect(q.HasHandshakeData()).To(BeFalse())
})
It("drops all Initial frames", func() {
It("drops all Handshake frames", func() {
q.AddHandshake(&wire.CryptoFrame{Data: []byte("foobar")})
q.AddHandshake(&wire.MaxDataFrame{ByteOffset: 0x42})
q.DropPackets(protocol.EncryptionHandshake)