diff --git a/packet_packer.go b/packet_packer.go index 484a08fd..b2e339e1 100644 --- a/packet_packer.go +++ b/packet_packer.go @@ -27,12 +27,13 @@ type packetPacker struct { packetNumberGenerator *packetNumberGenerator streamFramer *streamFramer - controlFrames []wire.Frame - stopWaiting *wire.StopWaitingFrame - ackFrame *wire.AckFrame - leastUnacked protocol.PacketNumber - omitConnectionID bool - hasSentPacket bool // has the packetPacker already sent a packet + controlFrames []wire.Frame + stopWaiting *wire.StopWaitingFrame + ackFrame *wire.AckFrame + leastUnacked protocol.PacketNumber + omitConnectionID bool + hasSentPacket bool // has the packetPacker already sent a packet + makeNextPacketRetransmittable bool } func newPacketPacker(connectionID protocol.ConnectionID, @@ -153,6 +154,15 @@ func (p *packetPacker) PackPacket() (*packedPacket, error) { if len(payloadFrames) == 1 && p.stopWaiting != nil { return nil, nil } + // check if this packet only contains an ACK and / or STOP_WAITING + if !ackhandler.HasRetransmittableFrames(payloadFrames) { + if p.makeNextPacketRetransmittable { + payloadFrames = append(payloadFrames, &wire.PingFrame{}) + p.makeNextPacketRetransmittable = false + } + } else { // this packet already contains a retransmittable frame. No need to send a PING + p.makeNextPacketRetransmittable = false + } p.stopWaiting = nil p.ackFrame = nil @@ -369,3 +379,7 @@ func (p *packetPacker) SetLeastUnacked(leastUnacked protocol.PacketNumber) { func (p *packetPacker) SetOmitConnectionID() { p.omitConnectionID = true } + +func (p *packetPacker) MakeNextPacketRetransmittable() { + p.makeNextPacketRetransmittable = true +} diff --git a/packet_packer_test.go b/packet_packer_test.go index 97057703..179f674c 100644 --- a/packet_packer_test.go +++ b/packet_packer_test.go @@ -361,6 +361,49 @@ var _ = Describe("Packet packer", func() { Expect(packer.packetNumberGenerator.Peek()).To(Equal(protocol.PacketNumber(2))) }) + It("adds a PING frame when it's supposed to send a retransmittable packet", func() { + packer.QueueControlFrame(&wire.AckFrame{}) + packer.QueueControlFrame(&wire.StopWaitingFrame{}) + packer.MakeNextPacketRetransmittable() + p, err := packer.PackPacket() + Expect(p).ToNot(BeNil()) + Expect(err).ToNot(HaveOccurred()) + Expect(p.frames).To(HaveLen(3)) + Expect(p.frames).To(ContainElement(&wire.PingFrame{})) + // make sure the next packet doesn't contain another PING + packer.QueueControlFrame(&wire.AckFrame{}) + p, err = packer.PackPacket() + Expect(p).ToNot(BeNil()) + Expect(err).ToNot(HaveOccurred()) + Expect(p.frames).To(HaveLen(1)) + }) + + It("waits until there's something to send before adding a PING frame", func() { + packer.MakeNextPacketRetransmittable() + p, err := packer.PackPacket() + Expect(err).ToNot(HaveOccurred()) + Expect(p).To(BeNil()) + packer.QueueControlFrame(&wire.AckFrame{}) + p, err = packer.PackPacket() + Expect(err).ToNot(HaveOccurred()) + Expect(p.frames).To(HaveLen(2)) + Expect(p.frames).To(ContainElement(&wire.PingFrame{})) + }) + + It("doesn't send a PING if it already sent another retransmittable frame", func() { + packer.MakeNextPacketRetransmittable() + packer.QueueControlFrame(&wire.MaxDataFrame{}) + p, err := packer.PackPacket() + Expect(p).ToNot(BeNil()) + Expect(err).ToNot(HaveOccurred()) + Expect(p.frames).To(HaveLen(1)) + packer.QueueControlFrame(&wire.AckFrame{}) + p, err = packer.PackPacket() + Expect(p).ToNot(BeNil()) + Expect(err).ToNot(HaveOccurred()) + Expect(p.frames).To(HaveLen(1)) + }) + Context("STREAM Frame handling", func() { It("does not splits a STREAM frame with maximum size, for gQUIC frames", func() { f := &wire.StreamFrame{ diff --git a/session.go b/session.go index 9975307f..83e10564 100644 --- a/session.go +++ b/session.go @@ -766,7 +766,7 @@ func (s *session) sendPacket() error { } // add a retransmittable frame if s.sentPacketHandler.ShouldSendRetransmittablePacket() { - s.packer.QueueControlFrame(&wire.PingFrame{}) + s.packer.MakeNextPacketRetransmittable() } packet, err := s.packer.PackPacket() if err != nil || packet == nil { diff --git a/session_test.go b/session_test.go index ea2394a2..d697097b 100644 --- a/session_test.go +++ b/session_test.go @@ -824,6 +824,7 @@ var _ = Describe("Session", func() { It("sends a retransmittable packet when required by the SentPacketHandler", func() { sess.sentPacketHandler = &mockSentPacketHandler{shouldSendRetransmittablePacket: true} + sess.packer.QueueControlFrame(&wire.AckFrame{LargestAcked: 1000}) err := sess.sendPacket() Expect(err).ToNot(HaveOccurred()) Expect(mconn.written).To(HaveLen(1))