From a962c63642b9330db27bc40dd991ef76570cbc9e Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Tue, 26 Apr 2016 22:46:39 +0700 Subject: [PATCH] don't queue ACK frames --- packet_packer.go | 15 ++++++++++++--- packet_packer_test.go | 26 +++++++++++++------------- session.go | 9 +++++++-- 3 files changed, 32 insertions(+), 18 deletions(-) diff --git a/packet_packer.go b/packet_packer.go index 02e5d3bc..ca5902b8 100644 --- a/packet_packer.go +++ b/packet_packer.go @@ -34,7 +34,8 @@ func (p *packetPacker) AddFrame(f frames.Frame) { p.mutex.Unlock() } -func (p *packetPacker) PackPacket() (*packedPacket, error) { +func (p *packetPacker) PackPacket(controlFrames []frames.Frame) (*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 p.mutex.Lock() defer p.mutex.Unlock() // TODO: Split up? @@ -47,7 +48,7 @@ func (p *packetPacker) PackPacket() (*packedPacket, error) { 1, )) - payloadFrames, err := p.composeNextPacket() + payloadFrames, err := p.composeNextPacket(controlFrames) if err != nil { return nil, err } @@ -98,10 +99,18 @@ func (p *packetPacker) getPayload(frames []frames.Frame, currentPacketNumber pro return payload.Bytes(), nil } -func (p *packetPacker) composeNextPacket() ([]frames.Frame, error) { +func (p *packetPacker) composeNextPacket(controlFrames []frames.Frame) ([]frames.Frame, error) { payloadLength := 0 var payloadFrames []frames.Frame + // TODO: handle the case where there are more controlFrames than we can put into one packet + for len(controlFrames) > 0 { + frame := controlFrames[0] + payloadFrames = append(payloadFrames, frame) + payloadLength += frame.MinLength() + controlFrames = controlFrames[1:] + } + for len(p.queuedFrames) > 0 { frame := p.queuedFrames[0] diff --git a/packet_packer_test.go b/packet_packer_test.go index 102eba77..7b6173c1 100644 --- a/packet_packer_test.go +++ b/packet_packer_test.go @@ -21,7 +21,7 @@ var _ = Describe("Packet packer", func() { }) It("returns nil when no packet is queued", func() { - p, err := packer.PackPacket() + p, err := packer.PackPacket([]frames.Frame{}) Expect(p).To(BeNil()) Expect(err).ToNot(HaveOccurred()) }) @@ -29,7 +29,7 @@ var _ = Describe("Packet packer", func() { It("packs single packets", func() { f := &frames.AckFrame{} packer.AddFrame(f) - p, err := packer.PackPacket() + p, err := packer.PackPacket([]frames.Frame{}) Expect(p).ToNot(BeNil()) Expect(err).ToNot(HaveOccurred()) b := &bytes.Buffer{} @@ -43,7 +43,7 @@ var _ = Describe("Packet packer", func() { f2 := &frames.AckFrame{LargestObserved: 2} packer.AddFrame(f1) packer.AddFrame(f2) - p, err := packer.PackPacket() + p, err := packer.PackPacket([]frames.Frame{}) Expect(p).ToNot(BeNil()) Expect(err).ToNot(HaveOccurred()) b := &bytes.Buffer{} @@ -63,10 +63,10 @@ var _ = Describe("Packet packer", func() { packer.AddFrame(f) counter++ } - payloadFrames, err := packer.composeNextPacket() + payloadFrames, err := packer.composeNextPacket([]frames.Frame{}) Expect(err).ToNot(HaveOccurred()) Expect(len(payloadFrames)).To(Equal(maxFramesPerPacket)) - payloadFrames, err = packer.composeNextPacket() + payloadFrames, err = packer.composeNextPacket([]frames.Frame{}) Expect(err).ToNot(HaveOccurred()) Expect(len(payloadFrames)).To(Equal(counter - maxFramesPerPacket)) }) @@ -79,10 +79,10 @@ var _ = Describe("Packet packer", func() { Offset: 1, } packer.AddFrame(f) - payloadFrames, err := packer.composeNextPacket() + payloadFrames, err := packer.composeNextPacket([]frames.Frame{}) Expect(err).ToNot(HaveOccurred()) Expect(len(payloadFrames)).To(Equal(1)) - payloadFrames, err = packer.composeNextPacket() + payloadFrames, err = packer.composeNextPacket([]frames.Frame{}) Expect(err).ToNot(HaveOccurred()) Expect(len(payloadFrames)).To(Equal(0)) }) @@ -99,13 +99,13 @@ var _ = Describe("Packet packer", func() { } packer.AddFrame(f1) packer.AddFrame(f2) - p, err := packer.PackPacket() + p, err := packer.PackPacket([]frames.Frame{}) Expect(err).ToNot(HaveOccurred()) Expect(len(p.raw)).To(Equal(protocol.MaxPacketSize)) - p, err = packer.PackPacket() + p, err = packer.PackPacket([]frames.Frame{}) Expect(err).ToNot(HaveOccurred()) Expect(len(p.raw)).To(Equal(protocol.MaxPacketSize)) - p, err = packer.PackPacket() + p, err = packer.PackPacket([]frames.Frame{}) Expect(err).ToNot(HaveOccurred()) Expect(p).ToNot(BeNil()) }) @@ -116,7 +116,7 @@ var _ = Describe("Packet packer", func() { Offset: 1, } packer.AddFrame(f) - p, err := packer.PackPacket() + p, err := packer.PackPacket([]frames.Frame{}) Expect(err).ToNot(HaveOccurred()) Expect(p).ToNot(BeNil()) Expect(len(p.raw)).To(Equal(protocol.MaxPacketSize)) @@ -128,10 +128,10 @@ var _ = Describe("Packet packer", func() { Offset: 1, } packer.AddFrame(f) - payloadFrames, err := packer.composeNextPacket() + payloadFrames, err := packer.composeNextPacket([]frames.Frame{}) Expect(err).ToNot(HaveOccurred()) Expect(len(payloadFrames)).To(Equal(1)) - payloadFrames, err = packer.composeNextPacket() + payloadFrames, err = packer.composeNextPacket([]frames.Frame{}) Expect(err).ToNot(HaveOccurred()) Expect(len(payloadFrames)).To(Equal(1)) }) diff --git a/session.go b/session.go index 0e77c297..7a83d9da 100644 --- a/session.go +++ b/session.go @@ -121,7 +121,6 @@ func (s *Session) handlePacket(addr *net.UDPAddr, publicHeader *PublicHeader, r } s.receivedPacketHandler.ReceivedPacket(publicHeader.PacketNumber, packet.entropyBit) - s.QueueFrame(s.receivedPacketHandler.DequeueAckFrame()) for _, ff := range packet.frames { var err error @@ -225,7 +224,13 @@ func (s *Session) closeStreamsWithError(err error) { } func (s *Session) sendPacket() error { - packet, err := s.packer.PackPacket() + var controlFrames []frames.Frame + ack := s.receivedPacketHandler.DequeueAckFrame() + if ack != nil { + controlFrames = append(controlFrames, ack) + } + packet, err := s.packer.PackPacket(controlFrames) + if err != nil { return err }