don't add more STREAM frames to a packet if remaining size is too small

This commit is contained in:
Marten Seemann 2017-12-10 13:31:00 +07:00
parent 9b4cdf66fc
commit 4aca4d64b7
3 changed files with 95 additions and 56 deletions

View file

@ -122,3 +122,9 @@ const ClosedSessionDeleteTimeout = time.Minute
// NumCachedCertificates is the number of cached compressed certificate chains, each taking ~1K space
const NumCachedCertificates = 128
// MinStreamFrameSize is the minimum size that has to be left in a packet, so that we add another STREAM frame.
// This avoids splitting up STREAM frames into small pieces, which has 2 advantages:
// 1. it reduces the framing overhead
// 2. it reduces the head-of-line blocking, when a packet is lost
const MinStreamFrameSize ByteCount = 128

View file

@ -70,32 +70,32 @@ func (f *streamFramer) PopCryptoStreamFrame(maxLen protocol.ByteCount) *wire.Str
return frame
}
func (f *streamFramer) maybePopFramesForRetransmission(maxLen protocol.ByteCount) (res []*wire.StreamFrame, currentLen protocol.ByteCount) {
func (f *streamFramer) maybePopFramesForRetransmission(maxTotalLen protocol.ByteCount) (res []*wire.StreamFrame, currentLen protocol.ByteCount) {
for len(f.retransmissionQueue) > 0 {
frame := f.retransmissionQueue[0]
frame.DataLenPresent = true
frameHeaderLen := frame.MinLength(f.version)
if currentLen+frameHeaderLen >= maxLen {
frameHeaderLen := frame.MinLength(f.version) // can never error
maxLen := maxTotalLen - currentLen
if frameHeaderLen+frame.DataLen() > maxLen && maxLen < protocol.MinStreamFrameSize {
break
}
currentLen += frameHeaderLen
splitFrame := maybeSplitOffFrame(frame, maxLen-currentLen)
splitFrame := maybeSplitOffFrame(frame, maxLen-frameHeaderLen)
if splitFrame != nil { // StreamFrame was split
res = append(res, splitFrame)
currentLen += splitFrame.DataLen()
currentLen += frameHeaderLen + splitFrame.DataLen()
break
}
f.retransmissionQueue = f.retransmissionQueue[1:]
res = append(res, frame)
currentLen += frame.DataLen()
currentLen += frameHeaderLen + frame.DataLen()
}
return
}
func (f *streamFramer) maybePopNormalFrames(maxBytes protocol.ByteCount) (res []*wire.StreamFrame) {
func (f *streamFramer) maybePopNormalFrames(maxTotalLen protocol.ByteCount) (res []*wire.StreamFrame) {
frame := &wire.StreamFrame{DataLenPresent: true}
var currentLen protocol.ByteCount
@ -105,16 +105,20 @@ func (f *streamFramer) maybePopNormalFrames(maxBytes protocol.ByteCount) (res []
}
frame.StreamID = s.StreamID()
frame.Offset = s.GetWriteOffset()
// not perfect, but thread-safe since writeOffset is only written when getting data
frame.Offset = s.GetWriteOffset()
frameHeaderBytes := frame.MinLength(f.version)
if currentLen+frameHeaderBytes > maxBytes {
if currentLen+frameHeaderBytes > maxTotalLen {
return false, nil // theoretically, we could find another stream that fits, but this is quite unlikely, so we stop here
}
maxLen := maxBytes - currentLen - frameHeaderBytes
maxLen := maxTotalLen - currentLen
if maxLen < protocol.MinStreamFrameSize { // don't try to add new STREAM frames, if only little space is left in the packet
return false, nil
}
if s.HasDataForWriting() {
frame.Data, frame.FinBit = s.GetDataForWriting(maxLen)
frame.Data, frame.FinBit = s.GetDataForWriting(maxLen - frameHeaderBytes)
}
if len(frame.Data) == 0 && !frame.FinBit {
return true, nil
@ -131,7 +135,7 @@ func (f *streamFramer) maybePopNormalFrames(maxBytes protocol.ByteCount) (res []
res = append(res, frame)
currentLen += frameHeaderBytes + frame.DataLen()
if currentLen == maxBytes {
if currentLen == maxTotalLen {
return false, nil
}

View file

@ -108,6 +108,33 @@ var _ = Describe("Stream Framer", func() {
Expect(framer.PopStreamFrames(1000)).To(BeEmpty())
})
It("doesn't pop frames for retransmission, if the remaining space in the packet is too small, and the frame would be split", func() {
setNoData(stream1)
setNoData(stream2)
framer.AddFrameForRetransmission(&wire.StreamFrame{
StreamID: id1,
Data: bytes.Repeat([]byte{'a'}, int(protocol.MinStreamFrameSize)),
})
fs := framer.PopStreamFrames(protocol.MinStreamFrameSize - 1)
Expect(fs).To(BeEmpty())
})
It("pops frames for retransmission, even if the remaining space in the packet is too small, if the frame doesn't need to be split", func() {
setNoData(stream1)
setNoData(stream2)
framer.AddFrameForRetransmission(retransmittedFrame1)
fs := framer.PopStreamFrames(protocol.MinStreamFrameSize - 1)
Expect(fs).To(Equal([]*wire.StreamFrame{retransmittedFrame1}))
})
It("pops frames for retransmission, if the remaining size is the miniumum STREAM frame size", func() {
setNoData(stream1)
setNoData(stream2)
framer.AddFrameForRetransmission(retransmittedFrame1)
fs := framer.PopStreamFrames(1000)
Expect(fs).To(HaveLen(1))
})
It("returns normal frames", func() {
stream1.EXPECT().GetDataForWriting(gomock.Any()).Return([]byte("foobar"), false)
stream1.EXPECT().HasDataForWriting().Return(true)
@ -155,28 +182,57 @@ var _ = Describe("Stream Framer", func() {
stream1.EXPECT().HasDataForWriting().Return(false)
stream1.EXPECT().GetWriteOffset()
setNoData(stream2)
fs := framer.PopStreamFrames(5)
fs := framer.PopStreamFrames(500)
Expect(fs).To(BeEmpty())
})
It("pops frames that have the minimum size", func() {
streamFrameHeaderLen := protocol.ByteCount(4)
stream1.EXPECT().HasDataForWriting().Return(true)
stream1.EXPECT().GetWriteOffset()
stream1.EXPECT().GetDataForWriting(protocol.MinStreamFrameSize-streamFrameHeaderLen).Return(bytes.Repeat([]byte{'f'}, int(protocol.MinStreamFrameSize-streamFrameHeaderLen)), false)
setNoData(stream2)
fs := framer.PopStreamFrames(protocol.MinStreamFrameSize)
Expect(fs).To(HaveLen(1))
Expect(fs[0].DataLen()).To(Equal(protocol.MinStreamFrameSize - streamFrameHeaderLen))
})
It("does not pop frames smaller than the mimimum size", func() {
setNoData(stream1)
setNoData(stream2)
fs := framer.PopStreamFrames(protocol.MinStreamFrameSize - 1)
Expect(fs).To(BeEmpty())
})
It("uses the round-robin scheduling", func() {
streamFrameHeaderLen := protocol.ByteCount(4)
stream1.EXPECT().GetDataForWriting(10-streamFrameHeaderLen).Return(bytes.Repeat([]byte("f"), int(10-streamFrameHeaderLen)), false)
stream1.EXPECT().GetDataForWriting(1000-streamFrameHeaderLen).Return(bytes.Repeat([]byte("f"), int(1000-streamFrameHeaderLen)), false)
stream1.EXPECT().HasDataForWriting().Return(true)
stream1.EXPECT().GetWriteOffset()
stream2.EXPECT().GetDataForWriting(protocol.ByteCount(10-streamFrameHeaderLen)).Return(bytes.Repeat([]byte("e"), int(10-streamFrameHeaderLen)), false)
stream2.EXPECT().GetDataForWriting(protocol.ByteCount(1000-streamFrameHeaderLen)).Return(bytes.Repeat([]byte("e"), int(1000-streamFrameHeaderLen)), false)
stream2.EXPECT().HasDataForWriting().Return(true)
stream2.EXPECT().GetWriteOffset()
fs := framer.PopStreamFrames(10)
fs := framer.PopStreamFrames(1000)
Expect(fs).To(HaveLen(1))
// it doesn't matter here if this data is from stream1 or from stream2...
firstStreamID := fs[0].StreamID
fs = framer.PopStreamFrames(10)
fs = framer.PopStreamFrames(1000)
Expect(fs).To(HaveLen(1))
// ... but the data popped this time has to be from the other stream
Expect(fs[0].StreamID).ToNot(Equal(firstStreamID))
})
It("stops iterating when the remaining size is smaller than the minimum STREAM frame size", func() {
streamFrameHeaderLen := protocol.ByteCount(4)
// pop a frame such that the remaining size is one byte less than the minimum STREAM frame size
stream1.EXPECT().GetDataForWriting(1000-streamFrameHeaderLen).Return(bytes.Repeat([]byte("f"), int(1000-streamFrameHeaderLen-protocol.MinStreamFrameSize+1)), false)
stream1.EXPECT().HasDataForWriting().Return(true)
stream1.EXPECT().GetWriteOffset()
setNoData(stream2)
fs := framer.PopStreamFrames(1000)
Expect(fs).To(HaveLen(1))
})
Context("splitting of frames", func() {
It("splits off nothing", func() {
f := &wire.StreamFrame{
@ -214,55 +270,28 @@ var _ = Describe("Stream Framer", func() {
It("splits a frame", func() {
setNoData(stream1)
setNoData(stream2)
framer.AddFrameForRetransmission(retransmittedFrame2)
origlen := retransmittedFrame2.DataLen()
fs := framer.PopStreamFrames(6)
frame := &wire.StreamFrame{Data: bytes.Repeat([]byte{0}, 600)}
framer.AddFrameForRetransmission(frame)
fs := framer.PopStreamFrames(500)
Expect(fs).To(HaveLen(1))
minLength := fs[0].MinLength(framer.version)
Expect(minLength + fs[0].DataLen()).To(Equal(protocol.ByteCount(6)))
Expect(framer.retransmissionQueue[0].Data).To(HaveLen(int(origlen - fs[0].DataLen())))
Expect(minLength + fs[0].DataLen()).To(Equal(protocol.ByteCount(500)))
Expect(framer.retransmissionQueue[0].Data).To(HaveLen(int(600 - fs[0].DataLen())))
Expect(framer.retransmissionQueue[0].Offset).To(Equal(fs[0].DataLen()))
})
It("never returns an empty stream frame", func() {
// this one frame will be split off from again and again in this test. Therefore, it has to be large enough (checked again at the end)
origFrame := &wire.StreamFrame{
StreamID: 5,
Offset: 1,
FinBit: false,
Data: bytes.Repeat([]byte{'f'}, 30*30),
}
framer.AddFrameForRetransmission(origFrame)
minFrameDataLen := protocol.MaxPacketSize
for i := 0; i < 30; i++ {
frames, currentLen := framer.maybePopFramesForRetransmission(protocol.ByteCount(i))
if len(frames) == 0 {
Expect(currentLen).To(BeZero())
} else {
Expect(frames).To(HaveLen(1))
Expect(currentLen).ToNot(BeZero())
dataLen := frames[0].DataLen()
Expect(dataLen).ToNot(BeZero())
if dataLen < minFrameDataLen {
minFrameDataLen = dataLen
}
}
}
Expect(framer.retransmissionQueue).To(HaveLen(1)) // check that origFrame was large enough for this test and didn't get used up completely
Expect(minFrameDataLen).To(Equal(protocol.ByteCount(1)))
})
It("only removes a frame from the framer after returning all split parts", func() {
frameHeaderLen := protocol.ByteCount(4)
setNoData(stream1)
setNoData(stream2)
framer.AddFrameForRetransmission(retransmittedFrame2)
fs := framer.PopStreamFrames(6)
frame := &wire.StreamFrame{Data: bytes.Repeat([]byte{0}, int(501-frameHeaderLen))}
framer.AddFrameForRetransmission(frame)
fs := framer.PopStreamFrames(500)
Expect(fs).To(HaveLen(1))
Expect(framer.retransmissionQueue).ToNot(BeEmpty())
fs = framer.PopStreamFrames(1000)
fs = framer.PopStreamFrames(500)
Expect(fs).To(HaveLen(1))
Expect(fs[0].DataLen()).To(BeEquivalentTo(1))
Expect(framer.retransmissionQueue).To(BeEmpty())
})
})