make use of shorter PublicHeaders when packing a packet

fixes #67
2016-05-10 23:17:04 +07:00
parent b05b52a2a8
commit 174095d5aa
3 changed files with 59 additions and 43 deletions
--- a/packet_packer.go
+++ b/packet_packer.go
@@ -43,12 +43,8 @@ func (p *packetPacker) AddHighPrioStreamFrame(f frames.StreamFrame) {
 func (p *packetPacker) PackPacket(stopWaitingFrame *frames.StopWaitingFrame, controlFrames []frames.Frame, includeStreamFrames bool) (*packedPacket, error) {
 	// TODO: save controlFrames as a member variable, makes it easier to handle in the unlikely event that there are more controlFrames than you can put into on packet

-	payloadFrames, err := p.composeNextPacket(stopWaitingFrame, controlFrames, includeStreamFrames)
-	if err != nil {
-		return nil, err
-	}
-
-	if len(payloadFrames) == 0 {
+	// don't send out packets that only contain a StopWaitingFrame
+	if len(controlFrames) == 0 && (p.streamFrameQueue.Len() == 0 || !includeStreamFrames) {
 		return nil, nil
 	}

@@ -57,6 +53,23 @@ func (p *packetPacker) PackPacket(stopWaitingFrame *frames.StopWaitingFrame, con
 		1,
 	))

+	responsePublicHeader := &PublicHeader{
+		ConnectionID:         p.connectionID,
+		PacketNumber:         currentPacketNumber,
+		PacketNumberLen:      getPacketNumberLength(currentPacketNumber, p.sentPacketHandler.GetLargestObserved()),
+		TruncateConnectionID: p.connectionParametersManager.TruncateConnectionID(),
+	}
+
+	publicHeaderLength, err := responsePublicHeader.GetLength()
+	if err != nil {
+		return nil, err
+	}
+
+	payloadFrames, err := p.composeNextPacket(stopWaitingFrame, controlFrames, publicHeaderLength, includeStreamFrames)
+	if err != nil {
+		return nil, err
+	}
+
 	payload, err := p.getPayload(payloadFrames, currentPacketNumber)
 	if err != nil {
 		return nil, err
@@ -71,12 +84,6 @@ func (p *packetPacker) PackPacket(stopWaitingFrame *frames.StopWaitingFrame, con
 	}

 	var raw bytes.Buffer
-	responsePublicHeader := PublicHeader{
-		ConnectionID:         p.connectionID,
-		PacketNumber:         currentPacketNumber,
-		PacketNumberLen:      getPacketNumberLength(currentPacketNumber, p.sentPacketHandler.GetLargestObserved()),
-		TruncateConnectionID: p.connectionParametersManager.TruncateConnectionID(),
-	}
 	if err := responsePublicHeader.WritePublicHeader(&raw); err != nil {
 		return nil, err
 	}
@@ -105,15 +112,18 @@ func (p *packetPacker) getPayload(frames []frames.Frame, currentPacketNumber pro
 	return payload.Bytes(), nil
 }

-func (p *packetPacker) composeNextPacket(stopWaitingFrame *frames.StopWaitingFrame, controlFrames []frames.Frame, includeStreamFrames bool) ([]frames.Frame, error) {
+// func (p *packetPacker) composeNextPacketControlFrames(stopWaitingFrame *frames.StopWaitingFrame, controlFrames []frames.Frame) (uint8, []frames.Frame, error) {
+//
+// }
+//
+// func (p *packetPacker) composeNextPacketStreamFrames() ([]frames.Frame, error) {
+//
+// }
+
+func (p *packetPacker) composeNextPacket(stopWaitingFrame *frames.StopWaitingFrame, controlFrames []frames.Frame, publicHeaderLength uint8, includeStreamFrames bool) ([]frames.Frame, error) {
 	payloadLength := 0
 	var payloadFrames []frames.Frame

-	// don't send out packets that only contain a StopWaitingFrame
-	if len(controlFrames) == 0 && p.streamFrameQueue.Len() == 0 {
-		return nil, nil
-	}
-
 	// TODO: handle the case where there are more controlFrames than we can put into one packet
 	if stopWaitingFrame != nil {
 		payloadFrames = append(payloadFrames, stopWaitingFrame)
@@ -127,7 +137,9 @@ func (p *packetPacker) composeNextPacket(stopWaitingFrame *frames.StopWaitingFra
 		controlFrames = controlFrames[1:]
 	}

-	if payloadLength > protocol.MaxFrameSize {
+	maxFrameSize := protocol.MaxFrameAndPublicHeaderSize - int(publicHeaderLength)
+
+	if payloadLength > maxFrameSize {
 		panic("internal inconsistency: packet payload too large")
 	}

@@ -138,17 +150,17 @@ func (p *packetPacker) composeNextPacket(stopWaitingFrame *frames.StopWaitingFra
 	for p.streamFrameQueue.Len() > 0 {
 		frame := p.streamFrameQueue.Front()

-		if payloadLength > protocol.MaxFrameSize {
+		if payloadLength > maxFrameSize {
 			panic("internal inconsistency: packet payload too large")
 		}

 		// Does the frame fit into the remaining space?
-		if payloadLength+frame.MinLength() > protocol.MaxFrameSize {
+		if payloadLength+frame.MinLength() > maxFrameSize {
 			break
 		}

 		// Split stream frames if necessary
-		previousFrame := frame.MaybeSplitOffFrame(protocol.MaxFrameSize - payloadLength)
+		previousFrame := frame.MaybeSplitOffFrame(maxFrameSize - payloadLength)
 		if previousFrame != nil {
 			// Don't pop the queue, leave the modified frame in
 			frame = previousFrame
--- a/packet_packer_test.go
+++ b/packet_packer_test.go
@@ -110,18 +110,19 @@ var _ = Describe("Packet packer", func() {
 	})

 	It("packs many control frames into 1 packets", func() {
+		publicHeaderLength := uint8(10)
 		f := &frames.AckFrame{LargestObserved: 1}
 		b := &bytes.Buffer{}
 		f.Write(b, 3, protocol.PacketNumberLen6, 32)
-		maxFramesPerPacket := protocol.MaxFrameSize / b.Len()
+		maxFramesPerPacket := (protocol.MaxFrameAndPublicHeaderSize - int(publicHeaderLength)) / b.Len()
 		var controlFrames []frames.Frame
 		for i := 0; i < maxFramesPerPacket; i++ {
 			controlFrames = append(controlFrames, f)
 		}
-		payloadFrames, err := packer.composeNextPacket(nil, controlFrames, true)
+		payloadFrames, err := packer.composeNextPacket(nil, controlFrames, publicHeaderLength, true)
 		Expect(err).ToNot(HaveOccurred())
 		Expect(len(payloadFrames)).To(Equal(maxFramesPerPacket))
-		payloadFrames, err = packer.composeNextPacket(nil, []frames.Frame{}, true)
+		payloadFrames, err = packer.composeNextPacket(nil, []frames.Frame{}, publicHeaderLength, true)
 		Expect(err).ToNot(HaveOccurred())
 		Expect(len(payloadFrames)).To(BeZero())
 	})
@@ -149,16 +150,17 @@ var _ = Describe("Packet packer", func() {

 	Context("Stream Frame handling", func() {
 		It("does not splits a stream frame with maximum size", func() {
-			maxStreamFrameDataLen := protocol.MaxFrameSize - (1 + 4 + 8 + 2)
+			publicHeaderLength := uint8(12)
+			maxStreamFrameDataLen := protocol.MaxFrameAndPublicHeaderSize - int(publicHeaderLength) - (1 + 4 + 8 + 2)
 			f := frames.StreamFrame{
 				Data:   bytes.Repeat([]byte{'f'}, maxStreamFrameDataLen),
 				Offset: 1,
 			}
 			packer.AddStreamFrame(f)
-			payloadFrames, err := packer.composeNextPacket(nil, []frames.Frame{}, true)
+			payloadFrames, err := packer.composeNextPacket(nil, []frames.Frame{}, publicHeaderLength, true)
 			Expect(err).ToNot(HaveOccurred())
 			Expect(len(payloadFrames)).To(Equal(1))
-			payloadFrames, err = packer.composeNextPacket(nil, []frames.Frame{}, true)
+			payloadFrames, err = packer.composeNextPacket(nil, []frames.Frame{}, publicHeaderLength, true)
 			Expect(err).ToNot(HaveOccurred())
 			Expect(len(payloadFrames)).To(Equal(0))
 		})
@@ -185,28 +187,29 @@ var _ = Describe("Packet packer", func() {
 		})

 		It("splits one stream frame larger than maximum size", func() {
-			maxStreamFrameDataLen := protocol.MaxFrameSize - (1 + 4 + 8 + 2)
+			publicHeaderLength := uint8(5)
+			maxStreamFrameDataLen := protocol.MaxFrameAndPublicHeaderSize - int(publicHeaderLength) - (1 + 4 + 8 + 2)
 			f := frames.StreamFrame{
 				Data:   bytes.Repeat([]byte{'f'}, maxStreamFrameDataLen+200),
 				Offset: 1,
 			}
 			packer.AddStreamFrame(f)
-			payloadFrames, err := packer.composeNextPacket(nil, []frames.Frame{}, true)
+			payloadFrames, err := packer.composeNextPacket(nil, []frames.Frame{}, publicHeaderLength, true)
 			Expect(err).ToNot(HaveOccurred())
 			Expect(len(payloadFrames)).To(Equal(1))
 			Expect(len(payloadFrames[0].(*frames.StreamFrame).Data)).To(Equal(maxStreamFrameDataLen))
-			payloadFrames, err = packer.composeNextPacket(nil, []frames.Frame{}, true)
+			payloadFrames, err = packer.composeNextPacket(nil, []frames.Frame{}, publicHeaderLength, true)
 			Expect(err).ToNot(HaveOccurred())
 			Expect(len(payloadFrames)).To(Equal(1))
 			Expect(len(payloadFrames[0].(*frames.StreamFrame).Data)).To(Equal(200))
-			payloadFrames, err = packer.composeNextPacket(nil, []frames.Frame{}, true)
+			payloadFrames, err = packer.composeNextPacket(nil, []frames.Frame{}, publicHeaderLength, true)
 			Expect(err).ToNot(HaveOccurred())
 			Expect(len(payloadFrames)).To(Equal(0))
 		})

-		// set pending until https://github.com/lucas-clemente/quic-go/issues/67 is fixed
-		PIt("packs 2 stream frames that are too big for one packet correctly", func() {
-			maxStreamFrameDataLen := protocol.MaxFrameSize - (1 + 4 + 8 + 2)
+		It("packs 2 stream frames that are too big for one packet correctly", func() {
+			publicHeaderLength := uint8(5)
+			maxStreamFrameDataLen := protocol.MaxFrameAndPublicHeaderSize - int(publicHeaderLength) - (1 + 4 + 8 + 2)
 			f1 := frames.StreamFrame{
 				Data:   bytes.Repeat([]byte{'f'}, maxStreamFrameDataLen+100),
 				Offset: 1,
@@ -231,10 +234,10 @@ var _ = Describe("Packet packer", func() {
 			Expect(p).To(BeNil())
 		})

-		// set pending until https://github.com/lucas-clemente/quic-go/issues/67 is fixed
-		PIt("packs a packet that has the maximum packet size when given a large enough stream frame", func() {
+		It("packs a packet that has the maximum packet size when given a large enough stream frame", func() {
+			publicHeaderLength := uint8(3)
 			f := frames.StreamFrame{
-				Data:   bytes.Repeat([]byte{'f'}, protocol.MaxFrameSize-(1+4+8+2)),
+				Data:   bytes.Repeat([]byte{'f'}, protocol.MaxFrameAndPublicHeaderSize-int(publicHeaderLength)-(1+4+8+2)),
 				Offset: 1,
 			}
 			packer.AddStreamFrame(f)
@@ -245,15 +248,16 @@ var _ = Describe("Packet packer", func() {
 		})

 		It("splits a stream frame larger than the maximum size", func() {
+			publicHeaderLength := uint8(13)
 			f := frames.StreamFrame{
-				Data:   bytes.Repeat([]byte{'f'}, protocol.MaxFrameSize-(1+4+8+2)+1),
+				Data:   bytes.Repeat([]byte{'f'}, protocol.MaxFrameAndPublicHeaderSize-int(publicHeaderLength)-(1+4+8+2)+1),
 				Offset: 1,
 			}
 			packer.AddStreamFrame(f)
-			payloadFrames, err := packer.composeNextPacket(nil, []frames.Frame{}, true)
+			payloadFrames, err := packer.composeNextPacket(nil, []frames.Frame{}, publicHeaderLength, true)
 			Expect(err).ToNot(HaveOccurred())
 			Expect(len(payloadFrames)).To(Equal(1))
-			payloadFrames, err = packer.composeNextPacket(nil, []frames.Frame{}, true)
+			payloadFrames, err = packer.composeNextPacket(nil, []frames.Frame{}, publicHeaderLength, true)
 			Expect(err).ToNot(HaveOccurred())
 			Expect(len(payloadFrames)).To(Equal(1))
 		})
--- a/protocol/protocol.go
+++ b/protocol/protocol.go
@@ -32,8 +32,8 @@ type ErrorCode uint32
 // MaxPacketSize is the maximum packet size, including the public header
 const MaxPacketSize = 1452

-// MaxFrameSize is the maximum size of a QUIC frame
-const MaxFrameSize = MaxPacketSize - (1 + 8 + 6) /*public header*/ - 1 /*private header*/ - 12 /*crypto signature*/
+// MaxFrameAndPublicHeaderSize is the maximum size of a QUIC frame plus PublicHeader
+const MaxFrameAndPublicHeaderSize = MaxPacketSize - 1 /*private header*/ - 12 /*crypto signature*/

 // DefaultTCPMSS is the default maximum packet size used in the Linux TCP implementation.
 // Used in QUIC for congestion window computations in bytes.