gr1ffon/quic-go
1
0
Fork 0
forked from quic-go/quic-go
Code Pull Requests Activity

Merge pull request #2942 from lucas-clemente/dont-retransmit-ack-only-pings

Browse Source 
don't retransmit PING frames added to ACK-only packets
This commit is contained in:
Marten Seemann
2020-12-22 16:49:02 +07:00
committed by GitHub
parent 53c8e33323 d8858d767d
commit 9f6d1f5578
2 changed files with 18 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
packet_packer.go
View File

@@ -565,7 +565,8 @@ func (p *packetPacker) maybeGetAppDataPacketWithEncLevel(maxPayloadSize protocol
// the packet only contains an ACK
if p.numNonAckElicitingAcks >= protocol.MaxNonAckElicitingAcks {
ping := &wire.PingFrame{}
payload.frames = append(payload.frames, ackhandler.Frame{Frame: ping})
// don't retransmit the PING frame when it is lost
payload.frames = append(payload.frames, ackhandler.Frame{Frame: ping, OnLost: func(wire.Frame) {}})
payload.length += ping.Length(p.version)
p.numNonAckElicitingAcks = 0
} else {

18
packet_packer_test.go
View File

@@ -728,7 +728,14 @@ var _ = Describe("Packet packer", func() {
p, err := packer.PackPacket()
Expect(p).ToNot(BeNil())
Expect(err).ToNot(HaveOccurred())
Expect(p.frames).To(ContainElement(ackhandler.Frame{Frame: &wire.PingFrame{}}))
var hasPing bool
for _, f := range p.frames {
if _, ok := f.Frame.(*wire.PingFrame); ok {
hasPing = true
Expect(f.OnLost).ToNot(BeNil()) // make sure the PING is not retransmitted if lost
}
}
Expect(hasPing).To(BeTrue())
// 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))
@@ -768,7 +775,14 @@ var _ = Describe("Packet packer", func() {
p, err = packer.PackPacket()
Expect(err).ToNot(HaveOccurred())
Expect(p.ack).To(Equal(ack))
Expect(p.frames).To(Equal([]ackhandler.Frame{{Frame: &wire.PingFrame{}}}))
var hasPing bool
for _, f := range p.frames {
if _, ok := f.Frame.(*wire.PingFrame); ok {
hasPing = true
Expect(f.OnLost).ToNot(BeNil()) // make sure the PING is not retransmitted if lost
}
}
Expect(hasPing).To(BeTrue())
})
It("doesn't send a PING if it already sent another ack-eliciting frame", func() {