diff --git a/internal/protocol/params.go b/internal/protocol/params.go index fe88b495..7e3dab51 100644 --- a/internal/protocol/params.go +++ b/internal/protocol/params.go @@ -81,6 +81,10 @@ const MaxStreamFrameSorterGaps = 1000 // very small STREAM frames to consume a lot of memory. const MinStreamFrameBufferSize = 128 +// MinCoalescedPacketSize is the minimum size of a coalesced packet that we pack. +// If a packet has less than this number of bytes, we won't coalesce any more packets onto it. +const MinCoalescedPacketSize = 128 + // MaxCryptoStreamOffset is the maximum offset allowed on any of the crypto streams. // This limits the size of the ClientHello and Certificates that can be received. const MaxCryptoStreamOffset = 16 * (1 << 10) diff --git a/mock_packer_test.go b/mock_packer_test.go index e257b877..1fc58d12 100644 --- a/mock_packer_test.go +++ b/mock_packer_test.go @@ -109,10 +109,10 @@ func (mr *MockPackerMockRecorder) PackConnectionClose(arg0 interface{}) *gomock. } // PackPacket mocks base method -func (m *MockPacker) PackPacket() (*packedPacket, error) { +func (m *MockPacker) PackPacket() (*coalescedPacket, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "PackPacket") - ret0, _ := ret[0].(*packedPacket) + ret0, _ := ret[0].(*coalescedPacket) ret1, _ := ret[1].(error) return ret0, ret1 } diff --git a/packet_packer.go b/packet_packer.go index b8dfe6d5..2eaa8410 100644 --- a/packet_packer.go +++ b/packet_packer.go @@ -15,7 +15,7 @@ import ( ) type packer interface { - PackPacket() (*packedPacket, error) + PackPacket() (*coalescedPacket, error) PackAppDataPacket() (*packedPacket, error) MaybePackProbePacket(protocol.EncryptionLevel) (*packedPacket, error) MaybePackAckPacket(handshakeConfirmed bool) (*packedPacket, error) @@ -36,14 +36,25 @@ type payload struct { } type packedPacket struct { + buffer *packetBuffer + *packetContents +} + +type packetContents struct { header *wire.ExtendedHeader ack *wire.AckFrame frames []ackhandler.Frame - buffer *packetBuffer + length protocol.ByteCount } -func (p *packedPacket) EncryptionLevel() protocol.EncryptionLevel { +type coalescedPacket struct { + buffer *packetBuffer + + packets []*packetContents +} + +func (p *packetContents) EncryptionLevel() protocol.EncryptionLevel { if !p.header.IsLongHeader { return protocol.Encryption1RTT } @@ -59,11 +70,11 @@ func (p *packedPacket) EncryptionLevel() protocol.EncryptionLevel { } } -func (p *packedPacket) IsAckEliciting() bool { +func (p *packetContents) IsAckEliciting() bool { return ackhandler.HasAckElicitingFrames(p.frames) } -func (p *packedPacket) ToAckHandlerPacket(now time.Time, q *retransmissionQueue) *ackhandler.Packet { +func (p *packetContents) ToAckHandlerPacket(now time.Time, q *retransmissionQueue) *ackhandler.Packet { largestAcked := protocol.InvalidPacketNumber if p.ack != nil { largestAcked = p.ack.LargestAcked() @@ -86,7 +97,7 @@ func (p *packedPacket) ToAckHandlerPacket(now time.Time, q *retransmissionQueue) PacketNumber: p.header.PacketNumber, LargestAcked: largestAcked, Frames: p.frames, - Length: p.buffer.Len(), + Length: p.length, EncryptionLevel: encLevel, SendTime: now, } @@ -256,36 +267,79 @@ func (p *packetPacker) MaybePackAckPacket(handshakeConfirmed bool) (*packedPacke // PackPacket packs a new packet. // It packs an Initial / Handshake if there is data to send in these packet number spaces. // It should only be called before the handshake is confirmed. -func (p *packetPacker) PackPacket() (*packedPacket, error) { - packet, err := p.maybePackCryptoPacket() - if err != nil || packet != nil { - return packet, err +func (p *packetPacker) PackPacket() (*coalescedPacket, error) { + buffer := getPacketBuffer() + packet, err := p.packCoalescedPacket(buffer) + if err != nil { + return nil, err } - return p.maybePackAppDataPacket() + + if len(packet.packets) == 0 { // nothing to send + buffer.Release() + return nil, nil + } + return packet, nil +} + +func (p *packetPacker) packCoalescedPacket(buffer *packetBuffer) (*coalescedPacket, error) { + packet := &coalescedPacket{ + buffer: buffer, + packets: make([]*packetContents, 0, 3), + } + // Try packing an Initial packet. + contents, err := p.maybeAppendInitialPacket(buffer) + if err != nil && err != handshake.ErrKeysDropped { + return nil, err + } + if contents != nil { + packet.packets = append(packet.packets, contents) + } + if buffer.Len() >= p.maxPacketSize-protocol.MinCoalescedPacketSize { + return packet, nil + } + + // Add a Handshake packet. + contents, err = p.maybeAppendHandshakePacket(buffer) + if err != nil && err != handshake.ErrKeysDropped && err != handshake.ErrKeysNotYetAvailable { + return nil, err + } + if contents != nil { + packet.packets = append(packet.packets, contents) + } + if buffer.Len() >= p.maxPacketSize-protocol.MinCoalescedPacketSize { + return packet, nil + } + + // Add a 0-RTT / 1-RTT packet. + contents, err = p.maybeAppendAppDataPacket(buffer) + if err == handshake.ErrKeysNotYetAvailable { + return packet, nil + } + if err != nil { + return nil, err + } + if contents != nil { + packet.packets = append(packet.packets, contents) + } + return packet, nil } // PackAppDataPacket packs a packet in the application data packet number space. // It should be called after the handshake is confirmed. func (p *packetPacker) PackAppDataPacket() (*packedPacket, error) { - return p.maybePackAppDataPacket() + buffer := getPacketBuffer() + contents, err := p.maybeAppendAppDataPacket(buffer) + if err != nil || contents == nil { + buffer.Release() + return nil, err + } + return &packedPacket{ + buffer: buffer, + packetContents: contents, + }, nil } -func (p *packetPacker) maybePackCryptoPacket() (*packedPacket, error) { - // Try packing an Initial packet. - packet, err := p.maybePackInitialPacket() - if (err != nil && err != handshake.ErrKeysDropped) || packet != nil { - return packet, err - } - - // No Initial was packed. Try packing a Handshake packet. - packet, err = p.maybePackHandshakePacket() - if err == handshake.ErrKeysDropped || err == handshake.ErrKeysNotYetAvailable { - return nil, nil - } - return packet, err -} - -func (p *packetPacker) maybePackInitialPacket() (*packedPacket, error) { +func (p *packetPacker) maybeAppendInitialPacket(buffer *packetBuffer) (*packetContents, error) { sealer, err := p.cryptoSetup.GetInitialSealer() if err != nil { return nil, err @@ -297,69 +351,76 @@ func (p *packetPacker) maybePackInitialPacket() (*packedPacket, error) { // nothing to send return nil, nil } - return p.packCryptoPacket(protocol.EncryptionInitial, sealer, ack, hasRetransmission) + return p.appendCryptoPacket(buffer, protocol.EncryptionInitial, sealer, ack, hasRetransmission) } -func (p *packetPacker) maybePackHandshakePacket() (*packedPacket, error) { +func (p *packetPacker) maybeAppendHandshakePacket(buffer *packetBuffer) (*packetContents, error) { sealer, err := p.cryptoSetup.GetHandshakeSealer() - if err != nil { return nil, err } hasRetransmission := p.retransmissionQueue.HasHandshakeData() + // TODO: make sure that the ACK always fits ack := p.acks.GetAckFrame(protocol.EncryptionHandshake) if !p.handshakeStream.HasData() && !hasRetransmission && ack == nil { // nothing to send return nil, nil } - return p.packCryptoPacket(protocol.EncryptionHandshake, sealer, ack, hasRetransmission) + return p.appendCryptoPacket(buffer, protocol.EncryptionHandshake, sealer, ack, hasRetransmission) } -func (p *packetPacker) packCryptoPacket( +func (p *packetPacker) appendCryptoPacket( + buffer *packetBuffer, encLevel protocol.EncryptionLevel, sealer handshake.LongHeaderSealer, ack *wire.AckFrame, hasRetransmission bool, -) (*packedPacket, error) { - s := p.initialStream - if encLevel == protocol.EncryptionHandshake { - s = p.handshakeStream +) (*packetContents, error) { + s := p.handshakeStream + maxPacketSize := p.maxPacketSize + if encLevel == protocol.EncryptionInitial { + s = p.initialStream + if p.perspective == protocol.PerspectiveClient { + maxPacketSize = protocol.MinInitialPacketSize + } } + remainingLen := maxPacketSize - buffer.Len() - protocol.ByteCount(sealer.Overhead()) var payload payload if ack != nil { payload.ack = ack payload.length = ack.Length(p.version) + remainingLen -= payload.length } hdr := p.getLongHeader(encLevel) - hdrLen := hdr.GetLength(p.version) + remainingLen -= hdr.GetLength(p.version) if hasRetransmission { for { var f wire.Frame switch encLevel { case protocol.EncryptionInitial: - remainingLen := protocol.MinInitialPacketSize - hdrLen - protocol.ByteCount(sealer.Overhead()) - payload.length f = p.retransmissionQueue.GetInitialFrame(remainingLen) case protocol.EncryptionHandshake: - remainingLen := p.maxPacketSize - hdrLen - protocol.ByteCount(sealer.Overhead()) - payload.length f = p.retransmissionQueue.GetHandshakeFrame(remainingLen) } if f == nil { break } payload.frames = append(payload.frames, ackhandler.Frame{Frame: f}) - payload.length += f.Length(p.version) + frameLen := f.Length(p.version) + payload.length += frameLen + remainingLen -= frameLen } } else if s.HasData() { - cf := s.PopCryptoFrame(p.maxPacketSize - hdrLen - protocol.ByteCount(sealer.Overhead()) - payload.length) + cf := s.PopCryptoFrame(remainingLen) payload.frames = []ackhandler.Frame{{Frame: cf}} payload.length += cf.Length(p.version) } - return p.writeSinglePacket(hdr, payload, encLevel, sealer) + return p.appendPacket(buffer, hdr, payload, encLevel, sealer) } -func (p *packetPacker) maybePackAppDataPacket() (*packedPacket, error) { +func (p *packetPacker) maybeAppendAppDataPacket(buffer *packetBuffer) (*packetContents, error) { var sealer sealer var header *wire.ExtendedHeader var encLevel protocol.EncryptionLevel @@ -382,7 +443,7 @@ func (p *packetPacker) maybePackAppDataPacket() (*packedPacket, error) { } headerLen := header.GetLength(p.version) - maxSize := p.maxPacketSize - protocol.ByteCount(sealer.Overhead()) - headerLen + maxSize := p.maxPacketSize - buffer.Len() - protocol.ByteCount(sealer.Overhead()) - headerLen payload := p.composeNextPacket(maxSize) // check if we have anything to send @@ -402,7 +463,7 @@ func (p *packetPacker) maybePackAppDataPacket() (*packedPacket, error) { p.numNonAckElicitingAcks = 0 } - return p.writeSinglePacket(header, payload, encLevel, sealer) + return p.appendPacket(buffer, header, payload, encLevel, sealer) } func (p *packetPacker) composeNextPacket(maxFrameSize protocol.ByteCount) payload { @@ -437,16 +498,26 @@ func (p *packetPacker) composeNextPacket(maxFrameSize protocol.ByteCount) payloa } func (p *packetPacker) MaybePackProbePacket(encLevel protocol.EncryptionLevel) (*packedPacket, error) { + var contents *packetContents + var err error + buffer := getPacketBuffer() switch encLevel { case protocol.EncryptionInitial: - return p.maybePackInitialPacket() + contents, err = p.maybeAppendInitialPacket(buffer) case protocol.EncryptionHandshake: - return p.maybePackHandshakePacket() + contents, err = p.maybeAppendHandshakePacket(buffer) case protocol.Encryption1RTT: - return p.maybePackAppDataPacket() + contents, err = p.maybeAppendAppDataPacket(buffer) default: panic("unknown encryption level") } + if err != nil { + return nil, err + } + return &packedPacket{ + buffer: buffer, + packetContents: contents, + }, nil } func (p *packetPacker) getSealerAndHeader(encLevel protocol.EncryptionLevel) (sealer, *wire.ExtendedHeader, error) { @@ -536,14 +607,13 @@ func (p *packetPacker) writeSinglePacket( sealer sealer, ) (*packedPacket, error) { buffer := getPacketBuffer() - if err := p.appendPacket(buffer, header, payload, encLevel, sealer); err != nil { + contents, err := p.appendPacket(buffer, header, payload, encLevel, sealer) + if err != nil { return nil, err } return &packedPacket{ - buffer: buffer, - header: header, - ack: payload.ack, - frames: payload.frames, + buffer: buffer, + packetContents: contents, }, nil } @@ -553,7 +623,7 @@ func (p *packetPacker) appendPacket( payload payload, encLevel protocol.EncryptionLevel, sealer sealer, -) error { +) (*packetContents, error) { var paddingLen protocol.ByteCount pnLen := protocol.ByteCount(header.PacketNumberLen) if encLevel != protocol.Encryption1RTT { @@ -571,13 +641,13 @@ func (p *packetPacker) appendPacket( hdrOffset := buffer.Len() buf := bytes.NewBuffer(buffer.Data) if err := header.Write(buf, p.version); err != nil { - return err + return nil, err } payloadOffset := buf.Len() if payload.ack != nil { if err := payload.ack.Write(buf, p.version); err != nil { - return err + return nil, err } } if paddingLen > 0 { @@ -585,15 +655,15 @@ func (p *packetPacker) appendPacket( } for _, frame := range payload.frames { if err := frame.Write(buf, p.version); err != nil { - return err + return nil, err } } if payloadSize := protocol.ByteCount(buf.Len()-payloadOffset) - paddingLen; payloadSize != payload.length { - return fmt.Errorf("PacketPacker BUG: payload size inconsistent (expected %d, got %d bytes)", payload.length, payloadSize) + return nil, fmt.Errorf("PacketPacker BUG: payload size inconsistent (expected %d, got %d bytes)", payload.length, payloadSize) } if size := protocol.ByteCount(buf.Len() + sealer.Overhead()); size > p.maxPacketSize { - return fmt.Errorf("PacketPacker BUG: packet too large (%d bytes, allowed %d bytes)", size, p.maxPacketSize) + return nil, fmt.Errorf("PacketPacker BUG: packet too large (%d bytes, allowed %d bytes)", size, p.maxPacketSize) } raw := buffer.Data @@ -603,14 +673,19 @@ func (p *packetPacker) appendPacket( raw = raw[0 : buf.Len()+sealer.Overhead()] // apply header protection pnOffset := payloadOffset - int(header.PacketNumberLen) - sealer.EncryptHeader(raw[pnOffset+4:pnOffset+4+16], &raw[0], raw[pnOffset:payloadOffset]) + sealer.EncryptHeader(raw[pnOffset+4:pnOffset+4+16], &raw[hdrOffset], raw[pnOffset:payloadOffset]) buffer.Data = raw num := p.pnManager.PopPacketNumber(encLevel) if num != header.PacketNumber { - return errors.New("packetPacker BUG: Peeked and Popped packet numbers do not match") + return nil, errors.New("packetPacker BUG: Peeked and Popped packet numbers do not match") } - return nil + return &packetContents{ + header: header, + ack: payload.ack, + frames: payload.frames, + length: buffer.Len() - hdrOffset, + }, nil } func (p *packetPacker) SetToken(token []byte) { diff --git a/packet_packer_test.go b/packet_packer_test.go index f045a586..b5ee06bb 100644 --- a/packet_packer_test.go +++ b/packet_packer_test.go @@ -178,7 +178,8 @@ var _ = Describe("Packet packer", func() { p, err := packer.PackPacket() Expect(err).ToNot(HaveOccurred()) Expect(p).ToNot(BeNil()) - Expect(p.frames).To(Equal([]ackhandler.Frame{{Frame: f}})) + Expect(p.packets).To(HaveLen(1)) + Expect(p.packets[0].frames).To(Equal([]ackhandler.Frame{{Frame: f}})) hdrRawEncrypted := append([]byte{}, hdrRaw...) hdrRawEncrypted[0] ^= 0xff hdrRawEncrypted[len(hdrRaw)-2] ^= 0xff @@ -189,17 +190,17 @@ var _ = Describe("Packet packer", func() { }) Context("packing packets", func() { - var sealer *mocks.MockShortHeaderSealer - - BeforeEach(func() { - sealer = mocks.NewMockShortHeaderSealer(mockCtrl) + // getSealer gets a sealer that's expected to seal exactly one packet + getSealer := func() *mocks.MockShortHeaderSealer { + sealer := mocks.NewMockShortHeaderSealer(mockCtrl) sealer.EXPECT().KeyPhase().Return(protocol.KeyPhaseOne).AnyTimes() sealer.EXPECT().Overhead().Return(7).AnyTimes() sealer.EXPECT().EncryptHeader(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes() sealer.EXPECT().Seal(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn(func(dst, src []byte, pn protocol.PacketNumber, associatedData []byte) []byte { return append(src, bytes.Repeat([]byte{0}, sealer.Overhead())...) }).AnyTimes() - }) + return sealer + } Context("packing ACK packets", func() { It("doesn't pack a packet if there's no ACK to send", func() { @@ -214,7 +215,7 @@ var _ = Describe("Packet packer", func() { It("packs Handshake ACK-only packets", func() { pnManager.EXPECT().PeekPacketNumber(protocol.EncryptionHandshake).Return(protocol.PacketNumber(0x42), protocol.PacketNumberLen2) pnManager.EXPECT().PopPacketNumber(protocol.EncryptionHandshake).Return(protocol.PacketNumber(0x42)) - sealingManager.EXPECT().GetHandshakeSealer().Return(sealer, nil) + sealingManager.EXPECT().GetHandshakeSealer().Return(getSealer(), nil) ack := &wire.AckFrame{AckRanges: []wire.AckRange{{Smallest: 1, Largest: 10}}} ackFramer.EXPECT().GetAckFrame(protocol.EncryptionInitial) ackFramer.EXPECT().GetAckFrame(protocol.EncryptionHandshake).Return(ack) @@ -228,7 +229,7 @@ var _ = Describe("Packet packer", func() { It("packs 1-RTT ACK-only packets", func() { pnManager.EXPECT().PeekPacketNumber(protocol.Encryption1RTT).Return(protocol.PacketNumber(0x42), protocol.PacketNumberLen2) pnManager.EXPECT().PopPacketNumber(protocol.Encryption1RTT).Return(protocol.PacketNumber(0x42)) - sealingManager.EXPECT().Get1RTTSealer().Return(sealer, nil) + sealingManager.EXPECT().Get1RTTSealer().Return(getSealer(), nil) ack := &wire.AckFrame{AckRanges: []wire.AckRange{{Smallest: 1, Largest: 10}}} ackFramer.EXPECT().GetAckFrame(protocol.Encryption1RTT).Return(ack) p, err := packer.MaybePackAckPacket(true) @@ -253,22 +254,24 @@ var _ = Describe("Packet packer", func() { }) It("packs a 0-RTT packet", func() { - sealingManager.EXPECT().Get0RTTSealer().Return(sealer, nil).AnyTimes() + sealingManager.EXPECT().Get0RTTSealer().Return(getSealer(), nil).AnyTimes() pnManager.EXPECT().PeekPacketNumber(protocol.Encryption0RTT).Return(protocol.PacketNumber(0x42), protocol.PacketNumberLen2) pnManager.EXPECT().PopPacketNumber(protocol.Encryption0RTT).Return(protocol.PacketNumber(0x42)) cf := ackhandler.Frame{Frame: &wire.MaxDataFrame{ByteOffset: 0x1337}} framer.EXPECT().AppendControlFrames(nil, gomock.Any()).DoAndReturn(func(frames []ackhandler.Frame, _ protocol.ByteCount) ([]ackhandler.Frame, protocol.ByteCount) { return append(frames, cf), cf.Length(packer.version) }) + // TODO: check sizes framer.EXPECT().AppendStreamFrames(gomock.Any(), gomock.Any()).DoAndReturn(func(frames []ackhandler.Frame, _ protocol.ByteCount) ([]ackhandler.Frame, protocol.ByteCount) { return frames, 0 }) p, err := packer.PackPacket() Expect(p).ToNot(BeNil()) Expect(err).ToNot(HaveOccurred()) - Expect(p.header.Type).To(Equal(protocol.PacketType0RTT)) - Expect(p.EncryptionLevel()).To(Equal(protocol.Encryption0RTT)) - Expect(p.frames).To(Equal([]ackhandler.Frame{cf})) + Expect(p.packets).To(HaveLen(1)) + Expect(p.packets[0].header.Type).To(Equal(protocol.PacketType0RTT)) + Expect(p.packets[0].EncryptionLevel()).To(Equal(protocol.Encryption0RTT)) + Expect(p.packets[0].frames).To(Equal([]ackhandler.Frame{cf})) }) }) @@ -283,7 +286,7 @@ var _ = Describe("Packet packer", func() { It("returns nil when no packet is queued", func() { pnManager.EXPECT().PeekPacketNumber(protocol.Encryption1RTT).Return(protocol.PacketNumber(0x42), protocol.PacketNumberLen2) // don't expect any calls to PopPacketNumber - sealingManager.EXPECT().Get1RTTSealer().Return(sealer, nil) + sealingManager.EXPECT().Get1RTTSealer().Return(getSealer(), nil) ackFramer.EXPECT().GetAckFrame(protocol.Encryption1RTT) framer.EXPECT().AppendControlFrames(nil, gomock.Any()) framer.EXPECT().AppendStreamFrames(nil, gomock.Any()) @@ -295,7 +298,7 @@ var _ = Describe("Packet packer", func() { It("packs single packets", func() { pnManager.EXPECT().PeekPacketNumber(protocol.Encryption1RTT).Return(protocol.PacketNumber(0x42), protocol.PacketNumberLen2) pnManager.EXPECT().PopPacketNumber(protocol.Encryption1RTT).Return(protocol.PacketNumber(0x42)) - sealingManager.EXPECT().Get1RTTSealer().Return(sealer, nil) + sealingManager.EXPECT().Get1RTTSealer().Return(getSealer(), nil) ackFramer.EXPECT().GetAckFrame(protocol.Encryption1RTT) expectAppendControlFrames() f := &wire.StreamFrame{ @@ -315,7 +318,7 @@ var _ = Describe("Packet packer", func() { It("stores the encryption level a packet was sealed with", func() { pnManager.EXPECT().PeekPacketNumber(protocol.Encryption1RTT).Return(protocol.PacketNumber(0x42), protocol.PacketNumberLen2) pnManager.EXPECT().PopPacketNumber(protocol.Encryption1RTT).Return(protocol.PacketNumber(0x42)) - sealingManager.EXPECT().Get1RTTSealer().Return(sealer, nil) + sealingManager.EXPECT().Get1RTTSealer().Return(getSealer(), nil) ackFramer.EXPECT().GetAckFrame(protocol.Encryption1RTT) expectAppendControlFrames() expectAppendStreamFrames(ackhandler.Frame{Frame: &wire.StreamFrame{ @@ -332,7 +335,7 @@ var _ = Describe("Packet packer", func() { pnManager.EXPECT().PopPacketNumber(protocol.Encryption1RTT).Return(protocol.PacketNumber(0x42)) ack := &wire.AckFrame{AckRanges: []wire.AckRange{{Largest: 42, Smallest: 1}}} ackFramer.EXPECT().GetAckFrame(protocol.Encryption1RTT).Return(ack) - sealingManager.EXPECT().Get1RTTSealer().Return(sealer, nil) + sealingManager.EXPECT().Get1RTTSealer().Return(getSealer(), nil) expectAppendControlFrames() expectAppendStreamFrames() p, err := packer.PackAppDataPacket() @@ -349,7 +352,7 @@ var _ = Describe("Packet packer", func() { ErrorCode: 0x1337, ReasonPhrase: "foobar", } - sealingManager.EXPECT().Get1RTTSealer().Return(sealer, nil) + sealingManager.EXPECT().Get1RTTSealer().Return(getSealer(), nil) p, err := packer.PackConnectionClose(&ccf) Expect(err).ToNot(HaveOccurred()) Expect(p.frames).To(HaveLen(1)) @@ -359,7 +362,7 @@ var _ = Describe("Packet packer", func() { It("packs control frames", func() { pnManager.EXPECT().PeekPacketNumber(protocol.Encryption1RTT).Return(protocol.PacketNumber(0x42), protocol.PacketNumberLen2) pnManager.EXPECT().PopPacketNumber(protocol.Encryption1RTT).Return(protocol.PacketNumber(0x42)) - sealingManager.EXPECT().Get1RTTSealer().Return(sealer, nil) + sealingManager.EXPECT().Get1RTTSealer().Return(getSealer(), nil) ackFramer.EXPECT().GetAckFrame(protocol.Encryption1RTT) frames := []ackhandler.Frame{ {Frame: &wire.ResetStreamFrame{}}, @@ -376,7 +379,7 @@ var _ = Describe("Packet packer", func() { 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(sealer, nil) + sealingManager.EXPECT().Get1RTTSealer().Return(getSealer(), nil) ackFramer.EXPECT().GetAckFrame(protocol.Encryption1RTT) var maxSize protocol.ByteCount gomock.InOrder( @@ -401,6 +404,7 @@ var _ = Describe("Packet packer", func() { Expect(f.Length(packer.version)).To(BeEquivalentTo(2)) pnManager.EXPECT().PeekPacketNumber(protocol.Encryption1RTT).Return(protocol.PacketNumber(0x42), protocol.PacketNumberLen1) pnManager.EXPECT().PopPacketNumber(protocol.Encryption1RTT).Return(protocol.PacketNumber(0x42)) + sealer := getSealer() sealingManager.EXPECT().Get1RTTSealer().Return(sealer, nil) ackFramer.EXPECT().GetAckFrame(protocol.Encryption1RTT) expectAppendControlFrames() @@ -450,7 +454,7 @@ var _ = Describe("Packet packer", func() { } pnManager.EXPECT().PeekPacketNumber(protocol.Encryption1RTT).Return(protocol.PacketNumber(0x42), protocol.PacketNumberLen2) pnManager.EXPECT().PopPacketNumber(protocol.Encryption1RTT).Return(protocol.PacketNumber(0x42)) - sealingManager.EXPECT().Get1RTTSealer().Return(sealer, nil) + sealingManager.EXPECT().Get1RTTSealer().Return(getSealer(), nil) ackFramer.EXPECT().GetAckFrame(protocol.Encryption1RTT) expectAppendControlFrames() expectAppendStreamFrames(ackhandler.Frame{Frame: f1}, ackhandler.Frame{Frame: f2}, ackhandler.Frame{Frame: f3}) @@ -468,7 +472,7 @@ var _ = Describe("Packet packer", func() { for i := 0; i < protocol.MaxNonAckElicitingAcks; i++ { pnManager.EXPECT().PeekPacketNumber(protocol.Encryption1RTT).Return(protocol.PacketNumber(0x42), protocol.PacketNumberLen2) pnManager.EXPECT().PopPacketNumber(protocol.Encryption1RTT).Return(protocol.PacketNumber(0x42)) - sealingManager.EXPECT().Get1RTTSealer().Return(sealer, nil) + sealingManager.EXPECT().Get1RTTSealer().Return(getSealer(), nil) ackFramer.EXPECT().GetAckFrame(protocol.Encryption1RTT).Return(&wire.AckFrame{AckRanges: []wire.AckRange{{Smallest: 1, Largest: 1}}}) expectAppendControlFrames() expectAppendStreamFrames() @@ -484,7 +488,7 @@ var _ = Describe("Packet packer", func() { sendMaxNumNonAckElicitingAcks() pnManager.EXPECT().PeekPacketNumber(protocol.Encryption1RTT).Return(protocol.PacketNumber(0x42), protocol.PacketNumberLen2) pnManager.EXPECT().PopPacketNumber(protocol.Encryption1RTT).Return(protocol.PacketNumber(0x42)) - sealingManager.EXPECT().Get1RTTSealer().Return(sealer, nil) + sealingManager.EXPECT().Get1RTTSealer().Return(getSealer(), nil) ackFramer.EXPECT().GetAckFrame(protocol.Encryption1RTT).Return(&wire.AckFrame{AckRanges: []wire.AckRange{{Smallest: 1, Largest: 1}}}) expectAppendControlFrames() expectAppendStreamFrames() @@ -495,7 +499,7 @@ var _ = Describe("Packet packer", func() { // make sure the next packet doesn't contain another PING pnManager.EXPECT().PeekPacketNumber(protocol.Encryption1RTT).Return(protocol.PacketNumber(0x42), protocol.PacketNumberLen2) pnManager.EXPECT().PopPacketNumber(protocol.Encryption1RTT).Return(protocol.PacketNumber(0x42)) - sealingManager.EXPECT().Get1RTTSealer().Return(sealer, nil) + sealingManager.EXPECT().Get1RTTSealer().Return(getSealer(), nil) ackFramer.EXPECT().GetAckFrame(protocol.Encryption1RTT).Return(&wire.AckFrame{AckRanges: []wire.AckRange{{Smallest: 1, Largest: 1}}}) expectAppendControlFrames() expectAppendStreamFrames() @@ -510,7 +514,7 @@ var _ = Describe("Packet packer", func() { sendMaxNumNonAckElicitingAcks() // nothing to send pnManager.EXPECT().PeekPacketNumber(protocol.Encryption1RTT).Return(protocol.PacketNumber(0x42), protocol.PacketNumberLen2) - sealingManager.EXPECT().Get1RTTSealer().Return(sealer, nil) + sealingManager.EXPECT().Get1RTTSealer().Return(getSealer(), nil) expectAppendControlFrames() expectAppendStreamFrames() ackFramer.EXPECT().GetAckFrame(protocol.Encryption1RTT) @@ -522,7 +526,7 @@ var _ = Describe("Packet packer", func() { expectAppendStreamFrames() pnManager.EXPECT().PeekPacketNumber(protocol.Encryption1RTT).Return(protocol.PacketNumber(0x42), protocol.PacketNumberLen2) pnManager.EXPECT().PopPacketNumber(protocol.Encryption1RTT).Return(protocol.PacketNumber(0x42)) - sealingManager.EXPECT().Get1RTTSealer().Return(sealer, nil) + sealingManager.EXPECT().Get1RTTSealer().Return(getSealer(), nil) ack := &wire.AckFrame{AckRanges: []wire.AckRange{{Smallest: 1, Largest: 1}}} ackFramer.EXPECT().GetAckFrame(protocol.Encryption1RTT).Return(ack) p, err = packer.PackAppDataPacket() @@ -535,7 +539,7 @@ var _ = Describe("Packet packer", func() { sendMaxNumNonAckElicitingAcks() pnManager.EXPECT().PeekPacketNumber(protocol.Encryption1RTT).Return(protocol.PacketNumber(0x42), protocol.PacketNumberLen2) pnManager.EXPECT().PopPacketNumber(protocol.Encryption1RTT).Return(protocol.PacketNumber(0x42)) - sealingManager.EXPECT().Get1RTTSealer().Return(sealer, nil) + sealingManager.EXPECT().Get1RTTSealer().Return(getSealer(), nil) ackFramer.EXPECT().GetAckFrame(protocol.Encryption1RTT) expectAppendStreamFrames() expectAppendControlFrames(ackhandler.Frame{Frame: &wire.MaxDataFrame{}}) @@ -549,7 +553,7 @@ var _ = Describe("Packet packer", func() { Context("max packet size", func() { It("sets the maximum packet size", func() { pnManager.EXPECT().PeekPacketNumber(protocol.Encryption1RTT).Return(protocol.PacketNumber(0x42), protocol.PacketNumberLen2).Times(2) - sealingManager.EXPECT().Get1RTTSealer().Return(sealer, nil).Times(2) + sealingManager.EXPECT().Get1RTTSealer().Return(getSealer(), nil).Times(2) ackFramer.EXPECT().GetAckFrame(protocol.Encryption1RTT).Times(2) var initialMaxPacketSize protocol.ByteCount framer.EXPECT().AppendControlFrames(gomock.Any(), gomock.Any()).Do(func(_ []ackhandler.Frame, maxLen protocol.ByteCount) ([]ackhandler.Frame, protocol.ByteCount) { @@ -574,7 +578,7 @@ var _ = Describe("Packet packer", func() { It("doesn't increase the max packet size", func() { pnManager.EXPECT().PeekPacketNumber(protocol.Encryption1RTT).Return(protocol.PacketNumber(0x42), protocol.PacketNumberLen2).Times(2) - sealingManager.EXPECT().Get1RTTSealer().Return(sealer, nil).Times(2) + sealingManager.EXPECT().Get1RTTSealer().Return(getSealer(), nil).Times(2) ackFramer.EXPECT().GetAckFrame(protocol.Encryption1RTT).Times(2) var initialMaxPacketSize protocol.ByteCount framer.EXPECT().AppendControlFrames(gomock.Any(), gomock.Any()).Do(func(_ []ackhandler.Frame, maxLen protocol.ByteCount) ([]ackhandler.Frame, protocol.ByteCount) { @@ -610,7 +614,9 @@ var _ = Describe("Packet packer", func() { ackFramer.EXPECT().GetAckFrame(protocol.EncryptionInitial) initialStream.EXPECT().HasData().Return(true).AnyTimes() initialStream.EXPECT().PopCryptoFrame(gomock.Any()).Return(f) - sealingManager.EXPECT().GetInitialSealer().Return(sealer, nil) + sealingManager.EXPECT().GetInitialSealer().Return(getSealer(), nil) + sealingManager.EXPECT().GetHandshakeSealer().Return(nil, handshake.ErrKeysNotYetAvailable) + sealingManager.EXPECT().Get1RTTSealer().Return(nil, handshake.ErrKeysNotYetAvailable) p, err := packer.PackPacket() Expect(err).ToNot(HaveOccurred()) checkLength(p.buffer.Data) @@ -621,7 +627,7 @@ var _ = Describe("Packet packer", func() { pnManager.EXPECT().PeekPacketNumber(protocol.EncryptionHandshake).Return(protocol.PacketNumber(0x42), protocol.PacketNumberLen2) pnManager.EXPECT().PopPacketNumber(protocol.EncryptionHandshake).Return(protocol.PacketNumber(0x42)) sealingManager.EXPECT().GetInitialSealer().Return(mocks.NewMockShortHeaderSealer(mockCtrl), nil) - sealingManager.EXPECT().GetHandshakeSealer().Return(sealer, nil) + sealingManager.EXPECT().GetHandshakeSealer().Return(getSealer(), nil) ackFramer.EXPECT().GetAckFrame(protocol.EncryptionInitial) ackFramer.EXPECT().GetAckFrame(protocol.EncryptionHandshake) initialStream.EXPECT().HasData() @@ -634,9 +640,68 @@ var _ = Describe("Packet packer", func() { }) p, err := packer.PackPacket() Expect(err).ToNot(HaveOccurred()) - Expect(p.frames).To(HaveLen(1)) + Expect(p.packets).To(HaveLen(1)) + Expect(p.packets[0].frames).To(HaveLen(1)) + Expect(p.packets[0].header.IsLongHeader).To(BeTrue()) Expect(p.buffer.Len()).To(BeEquivalentTo(packer.maxPacketSize)) - Expect(p.header.IsLongHeader).To(BeTrue()) + checkLength(p.buffer.Data) + }) + + It("packs a coalesced packet", 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) + pnManager.EXPECT().PopPacketNumber(protocol.EncryptionHandshake).Return(protocol.PacketNumber(0x42)) + sealingManager.EXPECT().GetInitialSealer().Return(getSealer(), nil) + sealingManager.EXPECT().GetHandshakeSealer().Return(getSealer(), nil) + sealingManager.EXPECT().Get1RTTSealer().Return(nil, handshake.ErrKeysNotYetAvailable) + ackFramer.EXPECT().GetAckFrame(protocol.EncryptionInitial) + ackFramer.EXPECT().GetAckFrame(protocol.EncryptionHandshake) + 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")} + }) + 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")} + }) + p, err := packer.PackPacket() + Expect(err).ToNot(HaveOccurred()) + Expect(p.packets).To(HaveLen(2)) + Expect(p.packets[0].EncryptionLevel()).To(Equal(protocol.EncryptionInitial)) + Expect(p.packets[0].frames).To(HaveLen(1)) + Expect(p.packets[0].frames[0].Frame.(*wire.CryptoFrame).Data).To(Equal([]byte("initial"))) + Expect(p.packets[1].EncryptionLevel()).To(Equal(protocol.EncryptionHandshake)) + Expect(p.packets[1].frames).To(HaveLen(1)) + Expect(p.packets[1].frames[0].Frame.(*wire.CryptoFrame).Data).To(Equal([]byte("handshake"))) + hdr, _, rest, err := wire.ParsePacket(p.buffer.Data, 0) + Expect(err).ToNot(HaveOccurred()) + Expect(hdr.Type).To(Equal(protocol.PacketTypeInitial)) + hdr, _, rest, err = wire.ParsePacket(rest, 0) + Expect(err).ToNot(HaveOccurred()) + Expect(hdr.Type).To(Equal(protocol.PacketTypeHandshake)) + 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)) + sealingManager.EXPECT().GetInitialSealer().Return(getSealer(), nil) + // don't EXPECT any calls to GetHandshakeSealer and Get1RTTSealer + ackFramer.EXPECT().GetAckFrame(protocol.EncryptionInitial) + initialStream.EXPECT().HasData().Return(true).Times(2) + initialStream.EXPECT().PopCryptoFrame(gomock.Any()).DoAndReturn(func(size protocol.ByteCount) *wire.CryptoFrame { + s := size - protocol.MinCoalescedPacketSize + f := &wire.CryptoFrame{Offset: 0x1337} + f.Data = bytes.Repeat([]byte{'f'}, int(s-f.Length(packer.version)-1)) + Expect(f.Length(packer.version)).To(Equal(s)) + return f + }) + p, err := packer.PackPacket() + Expect(err).ToNot(HaveOccurred()) + Expect(p.packets).To(HaveLen(1)) + Expect(p.packets[0].EncryptionLevel()).To(Equal(protocol.EncryptionInitial)) + Expect(len(p.buffer.Data)).To(BeEquivalentTo(maxPacketSize - protocol.MinCoalescedPacketSize)) checkLength(p.buffer.Data) }) @@ -646,14 +711,17 @@ var _ = Describe("Packet packer", func() { retransmissionQueue.AddHandshake(&wire.CryptoFrame{Data: []byte("Handshake")}) pnManager.EXPECT().PeekPacketNumber(protocol.EncryptionInitial).Return(protocol.PacketNumber(0x42), protocol.PacketNumberLen2) pnManager.EXPECT().PopPacketNumber(protocol.EncryptionInitial).Return(protocol.PacketNumber(0x42)) - sealingManager.EXPECT().GetInitialSealer().Return(sealer, nil) + sealingManager.EXPECT().GetInitialSealer().Return(getSealer(), nil) + sealingManager.EXPECT().GetHandshakeSealer().Return(nil, handshake.ErrKeysNotYetAvailable) + sealingManager.EXPECT().Get1RTTSealer().Return(nil, handshake.ErrKeysNotYetAvailable) ackFramer.EXPECT().GetAckFrame(protocol.EncryptionInitial) initialStream.EXPECT().HasData() p, err := packer.PackPacket() Expect(err).ToNot(HaveOccurred()) - Expect(p.EncryptionLevel()).To(Equal(protocol.EncryptionInitial)) - Expect(p.frames).To(Equal([]ackhandler.Frame{{Frame: f}})) - Expect(p.header.IsLongHeader).To(BeTrue()) + Expect(p.packets).To(HaveLen(1)) + Expect(p.packets[0].EncryptionLevel()).To(Equal(protocol.EncryptionInitial)) + Expect(p.packets[0].frames).To(Equal([]ackhandler.Frame{{Frame: f}})) + Expect(p.packets[0].header.IsLongHeader).To(BeTrue()) checkLength(p.buffer.Data) }) @@ -661,16 +729,19 @@ var _ = Describe("Packet packer", func() { ack := &wire.AckFrame{AckRanges: []wire.AckRange{{Smallest: 10, Largest: 20}}} ackFramer.EXPECT().GetAckFrame(protocol.EncryptionInitial).Return(ack) initialStream.EXPECT().HasData().Times(2) - sealingManager.EXPECT().GetInitialSealer().Return(sealer, nil) + sealingManager.EXPECT().GetInitialSealer().Return(getSealer(), nil) + sealingManager.EXPECT().GetHandshakeSealer().Return(nil, handshake.ErrKeysNotYetAvailable) + sealingManager.EXPECT().Get1RTTSealer().Return(nil, handshake.ErrKeysNotYetAvailable) pnManager.EXPECT().PeekPacketNumber(protocol.EncryptionInitial).Return(protocol.PacketNumber(0x42), protocol.PacketNumberLen2) pnManager.EXPECT().PopPacketNumber(protocol.EncryptionInitial).Return(protocol.PacketNumber(0x42)) p, err := packer.PackPacket() Expect(err).ToNot(HaveOccurred()) - Expect(p.ack).To(Equal(ack)) + Expect(p.packets).To(HaveLen(1)) + Expect(p.packets[0].ack).To(Equal(ack)) }) It("doesn't pack anything if there's nothing to send at Initial and Handshake keys are not yet available", func() { - sealingManager.EXPECT().GetInitialSealer().Return(sealer, nil) + sealingManager.EXPECT().GetInitialSealer().Return(getSealer(), nil) sealingManager.EXPECT().GetHandshakeSealer().Return(nil, handshake.ErrKeysNotYetAvailable) sealingManager.EXPECT().Get1RTTSealer().Return(nil, handshake.ErrKeysNotYetAvailable) initialStream.EXPECT().HasData() @@ -687,12 +758,14 @@ var _ = Describe("Packet packer", func() { initialStream.EXPECT().HasData() handshakeStream.EXPECT().HasData().Times(2) sealingManager.EXPECT().GetInitialSealer().Return(mocks.NewMockShortHeaderSealer(mockCtrl), nil) - sealingManager.EXPECT().GetHandshakeSealer().Return(sealer, nil) + sealingManager.EXPECT().GetHandshakeSealer().Return(getSealer(), nil) + sealingManager.EXPECT().Get1RTTSealer().Return(nil, handshake.ErrKeysNotYetAvailable) pnManager.EXPECT().PeekPacketNumber(protocol.EncryptionHandshake).Return(protocol.PacketNumber(0x42), protocol.PacketNumberLen2) pnManager.EXPECT().PopPacketNumber(protocol.EncryptionHandshake).Return(protocol.PacketNumber(0x42)) p, err := packer.PackPacket() Expect(err).ToNot(HaveOccurred()) - Expect(p.ack).To(Equal(ack)) + Expect(p.packets).To(HaveLen(1)) + Expect(p.packets[0].ack).To(Equal(ack)) }) It("pads Initial packets to the required minimum packet size", func() { @@ -701,19 +774,23 @@ var _ = Describe("Packet packer", func() { f := &wire.CryptoFrame{Data: []byte("foobar")} pnManager.EXPECT().PeekPacketNumber(protocol.EncryptionInitial).Return(protocol.PacketNumber(0x42), protocol.PacketNumberLen2) pnManager.EXPECT().PopPacketNumber(protocol.EncryptionInitial).Return(protocol.PacketNumber(0x42)) - sealingManager.EXPECT().GetInitialSealer().Return(sealer, nil) + sealingManager.EXPECT().GetInitialSealer().Return(getSealer(), nil) + sealingManager.EXPECT().GetHandshakeSealer().Return(nil, handshake.ErrKeysNotYetAvailable) + sealingManager.EXPECT().Get0RTTSealer().Return(nil, handshake.ErrKeysNotYetAvailable) + sealingManager.EXPECT().Get1RTTSealer().Return(nil, handshake.ErrKeysNotYetAvailable) ackFramer.EXPECT().GetAckFrame(protocol.EncryptionInitial) initialStream.EXPECT().HasData().Return(true).Times(2) initialStream.EXPECT().PopCryptoFrame(gomock.Any()).Return(f) packer.perspective = protocol.PerspectiveClient - packet, err := packer.PackPacket() + p, err := packer.PackPacket() Expect(err).ToNot(HaveOccurred()) - Expect(packet.header.Token).To(Equal(token)) - Expect(packet.buffer.Len()).To(BeEquivalentTo(protocol.MinInitialPacketSize)) - Expect(packet.frames).To(HaveLen(1)) - cf := packet.frames[0].Frame.(*wire.CryptoFrame) + Expect(p.buffer.Len()).To(BeEquivalentTo(protocol.MinInitialPacketSize)) + Expect(p.packets).To(HaveLen(1)) + Expect(p.packets[0].header.Token).To(Equal(token)) + Expect(p.packets[0].frames).To(HaveLen(1)) + cf := p.packets[0].frames[0].Frame.(*wire.CryptoFrame) Expect(cf.Data).To(Equal([]byte("foobar"))) - checkLength(packet.buffer.Data) + checkLength(p.buffer.Data) }) It("adds an ACK frame", func() { @@ -721,17 +798,21 @@ var _ = Describe("Packet packer", func() { ack := &wire.AckFrame{AckRanges: []wire.AckRange{{Smallest: 42, Largest: 1337}}} pnManager.EXPECT().PeekPacketNumber(protocol.EncryptionInitial).Return(protocol.PacketNumber(0x42), protocol.PacketNumberLen2) pnManager.EXPECT().PopPacketNumber(protocol.EncryptionInitial).Return(protocol.PacketNumber(0x42)) - sealingManager.EXPECT().GetInitialSealer().Return(sealer, nil) + sealingManager.EXPECT().GetInitialSealer().Return(getSealer(), nil) + sealingManager.EXPECT().GetHandshakeSealer().Return(nil, handshake.ErrKeysNotYetAvailable) + sealingManager.EXPECT().Get0RTTSealer().Return(nil, handshake.ErrKeysNotYetAvailable) + sealingManager.EXPECT().Get1RTTSealer().Return(nil, handshake.ErrKeysNotYetAvailable) ackFramer.EXPECT().GetAckFrame(protocol.EncryptionInitial).Return(ack) initialStream.EXPECT().HasData().Return(true).Times(2) initialStream.EXPECT().PopCryptoFrame(gomock.Any()).Return(f) packer.version = protocol.VersionTLS packer.perspective = protocol.PerspectiveClient - packet, err := packer.PackPacket() + p, err := packer.PackPacket() Expect(err).ToNot(HaveOccurred()) - Expect(packet.buffer.Len()).To(BeEquivalentTo(protocol.MinInitialPacketSize)) - Expect(packet.ack).To(Equal(ack)) - Expect(packet.frames).To(HaveLen(1)) + Expect(p.packets).To(HaveLen(1)) + Expect(p.packets[0].ack).To(Equal(ack)) + Expect(p.packets[0].frames).To(HaveLen(1)) + Expect(p.buffer.Len()).To(BeEquivalentTo(protocol.MinInitialPacketSize)) }) }) @@ -739,7 +820,7 @@ var _ = Describe("Packet packer", func() { It("packs an Initial probe packet", func() { f := &wire.CryptoFrame{Data: []byte("Initial")} retransmissionQueue.AddInitial(f) - sealingManager.EXPECT().GetInitialSealer().Return(sealer, nil) + sealingManager.EXPECT().GetInitialSealer().Return(getSealer(), nil) ackFramer.EXPECT().GetAckFrame(protocol.EncryptionInitial) initialStream.EXPECT().HasData() pnManager.EXPECT().PeekPacketNumber(protocol.EncryptionInitial).Return(protocol.PacketNumber(0x42), protocol.PacketNumberLen2) @@ -757,7 +838,7 @@ var _ = Describe("Packet packer", func() { It("packs a Handshake probe packet", func() { f := &wire.CryptoFrame{Data: []byte("Handshake")} retransmissionQueue.AddHandshake(f) - sealingManager.EXPECT().GetHandshakeSealer().Return(sealer, nil) + sealingManager.EXPECT().GetHandshakeSealer().Return(getSealer(), nil) ackFramer.EXPECT().GetAckFrame(protocol.EncryptionHandshake) handshakeStream.EXPECT().HasData() pnManager.EXPECT().PeekPacketNumber(protocol.EncryptionHandshake).Return(protocol.PacketNumber(0x42), protocol.PacketNumberLen2) @@ -775,7 +856,7 @@ var _ = Describe("Packet packer", func() { It("packs a 1-RTT probe packet", func() { f := &wire.StreamFrame{Data: []byte("1-RTT")} retransmissionQueue.AddInitial(f) - sealingManager.EXPECT().Get1RTTSealer().Return(sealer, nil) + sealingManager.EXPECT().Get1RTTSealer().Return(getSealer(), nil) ackFramer.EXPECT().GetAckFrame(protocol.Encryption1RTT) pnManager.EXPECT().PeekPacketNumber(protocol.Encryption1RTT).Return(protocol.PacketNumber(0x42), protocol.PacketNumberLen2) pnManager.EXPECT().PopPacketNumber(protocol.Encryption1RTT).Return(protocol.PacketNumber(0x42)) @@ -795,25 +876,22 @@ var _ = Describe("Packet packer", func() { var _ = Describe("Converting to AckHandler packets", func() { It("convert a packet", func() { - buffer := getPacketBuffer() - buffer.Data = append(buffer.Data, []byte("foobar")...) - packet := &packedPacket{ - buffer: buffer, + packet := &packetContents{ header: &wire.ExtendedHeader{Header: wire.Header{}}, frames: []ackhandler.Frame{{Frame: &wire.MaxDataFrame{}}, {Frame: &wire.PingFrame{}}}, ack: &wire.AckFrame{AckRanges: []wire.AckRange{{Largest: 100, Smallest: 80}}}, + length: 42, } t := time.Now() p := packet.ToAckHandlerPacket(t, nil) - Expect(p.Length).To(Equal(protocol.ByteCount(6))) + Expect(p.Length).To(Equal(protocol.ByteCount(42))) Expect(p.Frames).To(Equal(packet.frames)) Expect(p.LargestAcked).To(Equal(protocol.PacketNumber(100))) Expect(p.SendTime).To(Equal(t)) }) It("sets the LargestAcked to invalid, if the packet doesn't have an ACK frame", func() { - packet := &packedPacket{ - buffer: getPacketBuffer(), + packet := &packetContents{ header: &wire.ExtendedHeader{Header: wire.Header{}}, frames: []ackhandler.Frame{{Frame: &wire.MaxDataFrame{}}, {Frame: &wire.PingFrame{}}}, } @@ -823,8 +901,7 @@ var _ = Describe("Converting to AckHandler packets", func() { It("doesn't overwrite the OnLost callback, if it is set", func() { var pingLost bool - packet := &packedPacket{ - buffer: getPacketBuffer(), + packet := &packetContents{ header: &wire.ExtendedHeader{Header: wire.Header{Type: protocol.PacketTypeHandshake}}, frames: []ackhandler.Frame{ {Frame: &wire.MaxDataFrame{}}, diff --git a/session.go b/session.go index 4595ace3..2795c9a6 100644 --- a/session.go +++ b/session.go @@ -1309,13 +1309,24 @@ func (s *session) sendPacket() (bool, error) { } s.windowUpdateQueue.QueueAll() - var packet *packedPacket - var err error if !s.handshakeConfirmed { - packet, err = s.packer.PackPacket() - } else { - packet, err = s.packer.PackAppDataPacket() + now := time.Now() + packet, err := s.packer.PackPacket() + if err != nil || packet == nil { + return false, err + } + for _, p := range packet.packets { + if s.firstAckElicitingPacketAfterIdleSentTime.IsZero() && p.IsAckEliciting() { + s.firstAckElicitingPacketAfterIdleSentTime = now + } + s.sentPacketHandler.SentPacket(p.ToAckHandlerPacket(now, s.retransmissionQueue)) + } + s.connIDManager.SentPacket() + s.logCoalescedPacket(now, packet) + s.sendQueue.Send(packet.buffer) + return true, nil } + packet, err := s.packer.PackAppDataPacket() if err != nil || packet == nil { return false, err } @@ -1324,35 +1335,13 @@ func (s *session) sendPacket() (bool, error) { } func (s *session) sendPackedPacket(packet *packedPacket) { - if s.firstAckElicitingPacketAfterIdleSentTime.IsZero() && packet.IsAckEliciting() { - s.firstAckElicitingPacketAfterIdleSentTime = time.Now() - } now := time.Now() - s.sentPacketHandler.SentPacket(packet.ToAckHandlerPacket(now, s.retransmissionQueue)) - if s.traceCallback != nil { - frames := make([]wire.Frame, 0, len(packet.frames)) - for _, f := range packet.frames { - frames = append(frames, f.Frame) - } - s.traceCallback(quictrace.Event{ - Time: now, - EventType: quictrace.PacketSent, - TransportState: s.sentPacketHandler.GetStats(), - EncryptionLevel: packet.EncryptionLevel(), - PacketNumber: packet.header.PacketNumber, - PacketSize: packet.buffer.Len(), - Frames: frames, - }) + if s.firstAckElicitingPacketAfterIdleSentTime.IsZero() && packet.IsAckEliciting() { + s.firstAckElicitingPacketAfterIdleSentTime = now } - if s.qlogger != nil { - frames := make([]wire.Frame, 0, len(packet.frames)) - for _, f := range packet.frames { - frames = append(frames, f.Frame) - } - s.qlogger.SentPacket(now, packet.header, packet.buffer.Len(), packet.ack, frames) - } - s.logPacket(packet) + s.sentPacketHandler.SentPacket(packet.ToAckHandlerPacket(time.Now(), s.retransmissionQueue)) s.connIDManager.SentPacket() + s.logPacket(now, packet) s.sendQueue.Send(packet.buffer) } @@ -1375,25 +1364,66 @@ func (s *session) sendConnectionClose(quicErr *qerr.QuicError) ([]byte, error) { if err != nil { return nil, err } - s.logPacket(packet) + s.logPacket(time.Now(), packet) return packet.buffer.Data, s.conn.Write(packet.buffer.Data) } -func (s *session) logPacket(packet *packedPacket) { +func (s *session) logPacketContents(now time.Time, p *packetContents) { + // qlog + if s.qlogger != nil { + frames := make([]wire.Frame, 0, len(p.frames)) + for _, f := range p.frames { + frames = append(frames, f.Frame) + } + s.qlogger.SentPacket(now, p.header, p.length, p.ack, frames) + } + + // quic-trace + if s.traceCallback != nil { + frames := make([]wire.Frame, 0, len(p.frames)) + for _, f := range p.frames { + frames = append(frames, f.Frame) + } + s.traceCallback(quictrace.Event{ + Time: now, + EventType: quictrace.PacketSent, + TransportState: s.sentPacketHandler.GetStats(), + EncryptionLevel: p.EncryptionLevel(), + PacketNumber: p.header.PacketNumber, + PacketSize: p.length, + Frames: frames, + }) + } + + // quic-go logging if !s.logger.Debug() { - // We don't need to allocate the slices for calling the format functions return } - s.logger.Debugf("-> Sending packet 0x%x (%d bytes) for connection %s, %s", packet.header.PacketNumber, packet.buffer.Len(), s.logID, packet.EncryptionLevel()) - packet.header.Log(s.logger) - if packet.ack != nil { - wire.LogFrame(s.logger, packet.ack, true) + p.header.Log(s.logger) + if p.ack != nil { + wire.LogFrame(s.logger, p.ack, true) } - for _, frame := range packet.frames { + for _, frame := range p.frames { wire.LogFrame(s.logger, frame.Frame, true) } } +func (s *session) logCoalescedPacket(now time.Time, packet *coalescedPacket) { + if s.logger.Debug() { + s.logger.Debugf("-> Sending coalesced packet (%d parts, %d bytes) for connection %s", len(packet.packets), packet.buffer.Len(), s.logID) + } + for _, p := range packet.packets { + s.logPacketContents(now, p) + } +} + +func (s *session) logPacket(now time.Time, packet *packedPacket) { + if s.logger.Debug() { + s.logger.Debugf("-> Sending packet %#x (%d bytes) for connection %s, %s", packet.header.PacketNumber, packet.buffer.Len(), s.logID, packet.EncryptionLevel()) + } + s.logPacketContents(now, packet.packetContents) +} + // AcceptStream returns the next stream openend by the peer func (s *session) AcceptStream(ctx context.Context) (Stream, error) { return s.streamsMap.AcceptStream(ctx) diff --git a/session_test.go b/session_test.go index 0833c7df..28744de2 100644 --- a/session_test.go +++ b/session_test.go @@ -57,7 +57,9 @@ var _ = Describe("Session", func() { buffer.Data = append(buffer.Data, []byte("foobar")...) return &packedPacket{ buffer: buffer, - header: &wire.ExtendedHeader{PacketNumber: pn}, + packetContents: &packetContents{ + header: &wire.ExtendedHeader{PacketNumber: pn}, + }, } } @@ -884,7 +886,8 @@ var _ = Describe("Session", func() { }) It("sends packets", func() { - packer.EXPECT().PackPacket().Return(getPacket(1), nil) + sess.handshakeConfirmed = true + packer.EXPECT().PackAppDataPacket().Return(getPacket(1), nil) sess.receivedPacketHandler.ReceivedPacket(0x035e, protocol.Encryption1RTT, time.Now(), true) mconn.EXPECT().Write(gomock.Any()) sent, err := sess.sendPacket() @@ -893,7 +896,8 @@ var _ = Describe("Session", func() { }) It("doesn't send packets if there's nothing to send", func() { - packer.EXPECT().PackPacket().Return(nil, nil) + sess.handshakeConfirmed = true + packer.EXPECT().PackAppDataPacket().Return(nil, nil) sess.receivedPacketHandler.ReceivedPacket(0x035e, protocol.Encryption1RTT, time.Now(), true) sent, err := sess.sendPacket() Expect(err).NotTo(HaveOccurred()) @@ -911,9 +915,10 @@ var _ = Describe("Session", func() { }) It("adds a BLOCKED frame when it is connection-level flow control blocked", func() { + sess.handshakeConfirmed = true fc := mocks.NewMockConnectionFlowController(mockCtrl) fc.EXPECT().IsNewlyBlocked().Return(true, protocol.ByteCount(1337)) - packer.EXPECT().PackPacket().Return(getPacket(1), nil) + packer.EXPECT().PackAppDataPacket().Return(getPacket(1), nil) sess.connFlowController = fc mconn.EXPECT().Write(gomock.Any()) sent, err := sess.sendPacket() @@ -996,6 +1001,7 @@ var _ = Describe("Session", func() { BeforeEach(func() { sph = mockackhandler.NewMockSentPacketHandler(mockCtrl) sph.EXPECT().GetLossDetectionTimeout().AnyTimes() + sess.handshakeConfirmed = true sess.sentPacketHandler = sph streamManager.EXPECT().CloseWithError(gomock.Any()) }) @@ -1016,8 +1022,8 @@ var _ = Describe("Session", func() { sph.EXPECT().TimeUntilSend().Return(time.Now()).Times(2) sph.EXPECT().TimeUntilSend().Return(time.Now().Add(time.Hour)) sph.EXPECT().SendMode().Return(ackhandler.SendAny).Times(2) // allow 2 packets... - packer.EXPECT().PackPacket().Return(getPacket(10), nil) - packer.EXPECT().PackPacket().Return(getPacket(11), nil) + packer.EXPECT().PackAppDataPacket().Return(getPacket(10), nil) + packer.EXPECT().PackAppDataPacket().Return(getPacket(11), nil) mconn.EXPECT().Write(gomock.Any()).Times(2) go func() { defer GinkgoRecover() @@ -1036,7 +1042,7 @@ var _ = Describe("Session", func() { sph.EXPECT().TimeUntilSend().Return(time.Now()) sph.EXPECT().SendMode().Return(ackhandler.SendAny) sph.EXPECT().SendMode().Return(ackhandler.SendAck) - packer.EXPECT().PackPacket().Return(getPacket(100), nil) + packer.EXPECT().PackAppDataPacket().Return(getPacket(100), nil) mconn.EXPECT().Write(gomock.Any()) go func() { defer GinkgoRecover() @@ -1055,8 +1061,8 @@ var _ = Describe("Session", func() { sph.EXPECT().TimeUntilSend().Return(time.Now().Add(time.Hour)) sph.EXPECT().ShouldSendNumPackets().Times(2).Return(1) sph.EXPECT().SendMode().Return(ackhandler.SendAny).AnyTimes() - packer.EXPECT().PackPacket().Return(getPacket(100), nil) - packer.EXPECT().PackPacket().Return(getPacket(101), nil) + packer.EXPECT().PackAppDataPacket().Return(getPacket(100), nil) + packer.EXPECT().PackAppDataPacket().Return(getPacket(101), nil) written := make(chan struct{}, 2) mconn.EXPECT().Write(gomock.Any()).DoAndReturn(func(p []byte) (int, error) { written <- struct{}{} @@ -1079,9 +1085,9 @@ var _ = Describe("Session", func() { sph.EXPECT().TimeUntilSend().Return(time.Now()) sph.EXPECT().TimeUntilSend().Return(time.Now().Add(time.Hour)) sph.EXPECT().SendMode().Return(ackhandler.SendAny).Times(3) - packer.EXPECT().PackPacket().Return(getPacket(1000), nil) - packer.EXPECT().PackPacket().Return(getPacket(1001), nil) - packer.EXPECT().PackPacket().Return(getPacket(1002), nil) + packer.EXPECT().PackAppDataPacket().Return(getPacket(1000), nil) + packer.EXPECT().PackAppDataPacket().Return(getPacket(1001), nil) + packer.EXPECT().PackAppDataPacket().Return(getPacket(1002), nil) written := make(chan struct{}, 3) mconn.EXPECT().Write(gomock.Any()).DoAndReturn(func(p []byte) (int, error) { written <- struct{}{} @@ -1100,7 +1106,7 @@ var _ = Describe("Session", func() { sph.EXPECT().TimeUntilSend().Return(time.Now()) sph.EXPECT().ShouldSendNumPackets().Return(1) sph.EXPECT().SendMode().Return(ackhandler.SendAny).AnyTimes() - packer.EXPECT().PackPacket() + packer.EXPECT().PackAppDataPacket() // don't EXPECT any calls to mconn.Write() go func() { defer GinkgoRecover() @@ -1113,6 +1119,10 @@ var _ = Describe("Session", func() { }) Context("scheduling sending", func() { + BeforeEach(func() { + sess.handshakeConfirmed = true + }) + AfterEach(func() { // make the go routine return expectReplaceWithClosed() @@ -1132,7 +1142,7 @@ var _ = Describe("Session", func() { sph.EXPECT().ShouldSendNumPackets().AnyTimes().Return(1) sph.EXPECT().SentPacket(gomock.Any()) sess.sentPacketHandler = sph - packer.EXPECT().PackPacket().Return(getPacket(1), nil) + packer.EXPECT().PackAppDataPacket().Return(getPacket(1), nil) go func() { defer GinkgoRecover() @@ -1149,7 +1159,7 @@ var _ = Describe("Session", func() { }) It("sets the timer to the ack timer", func() { - packer.EXPECT().PackPacket().Return(getPacket(1234), nil) + packer.EXPECT().PackAppDataPacket().Return(getPacket(1234), nil) sph := mockackhandler.NewMockSentPacketHandler(mockCtrl) sph.EXPECT().TimeUntilSend().Return(time.Now()) sph.EXPECT().TimeUntilSend().Return(time.Now().Add(time.Hour)) @@ -1177,6 +1187,75 @@ var _ = Describe("Session", func() { }) }) + It("sends coalesced packets before the handshake is confirmed", func() { + sess.handshakeConfirmed = false + sph := mockackhandler.NewMockSentPacketHandler(mockCtrl) + sess.sentPacketHandler = sph + buffer := getPacketBuffer() + buffer.Data = append(buffer.Data, []byte("foobar")...) + packer.EXPECT().PackPacket().Return(&coalescedPacket{ + buffer: buffer, + packets: []*packetContents{ + { + header: &wire.ExtendedHeader{ + Header: wire.Header{ + IsLongHeader: true, + Type: protocol.PacketTypeInitial, + }, + PacketNumber: 13, + }, + length: 123, + }, + { + header: &wire.ExtendedHeader{ + Header: wire.Header{ + IsLongHeader: true, + Type: protocol.PacketTypeHandshake, + }, + PacketNumber: 37, + }, + length: 1234, + }, + }, + }, nil) + packer.EXPECT().PackPacket().AnyTimes() + + sph.EXPECT().GetLossDetectionTimeout().AnyTimes() + sph.EXPECT().SendMode().Return(ackhandler.SendAny).AnyTimes() + sph.EXPECT().TimeUntilSend().Return(time.Now()).AnyTimes() + sph.EXPECT().ShouldSendNumPackets().Return(1).AnyTimes() + sph.EXPECT().SentPacket(gomock.Any()).Do(func(p *ackhandler.Packet) { + Expect(p.EncryptionLevel).To(Equal(protocol.EncryptionInitial)) + Expect(p.PacketNumber).To(Equal(protocol.PacketNumber(13))) + Expect(p.Length).To(BeEquivalentTo(123)) + }) + sph.EXPECT().SentPacket(gomock.Any()).Do(func(p *ackhandler.Packet) { + Expect(p.EncryptionLevel).To(Equal(protocol.EncryptionHandshake)) + Expect(p.PacketNumber).To(Equal(protocol.PacketNumber(37))) + Expect(p.Length).To(BeEquivalentTo(1234)) + }) + sent := make(chan struct{}) + mconn.EXPECT().Write([]byte("foobar")).Do(func([]byte) { close(sent) }) + + go func() { + defer GinkgoRecover() + cryptoSetup.EXPECT().RunHandshake().MaxTimes(1) + sess.run() + }() + + sess.scheduleSending() + Eventually(sent).Should(BeClosed()) + + // make sure the go routine returns + streamManager.EXPECT().CloseWithError(gomock.Any()) + expectReplaceWithClosed() + packer.EXPECT().PackConnectionClose(gomock.Any()).Return(&packedPacket{buffer: getPacketBuffer()}, nil) + cryptoSetup.EXPECT().Close() + mconn.EXPECT().Write(gomock.Any()) + sess.shutdown() + Eventually(sess.Context().Done()).Should(BeClosed()) + }) + It("cancels the HandshakeComplete context and informs the SentPacketHandler when the handshake completes", func() { packer.EXPECT().PackPacket().AnyTimes() finishHandshake := make(chan struct{}) @@ -1282,7 +1361,9 @@ var _ = Describe("Session", func() { Expect(frames[0].Frame).To(BeEquivalentTo(&wire.HandshakeDoneFrame{})) defer close(done) return &packedPacket{ - header: &wire.ExtendedHeader{}, + packetContents: &packetContents{ + header: &wire.ExtendedHeader{}, + }, buffer: getPacketBuffer(), }, nil }) @@ -1303,7 +1384,6 @@ var _ = Describe("Session", func() { expectReplaceWithClosed() packer.EXPECT().PackConnectionClose(gomock.Any()).Return(&packedPacket{buffer: getPacketBuffer()}, nil) cryptoSetup.EXPECT().Close() - mconn.EXPECT().Write(gomock.Any()) sess.shutdown() Eventually(sess.Context().Done()).Should(BeClosed()) })