From 5140addd8af87f0e97cab051da55136f723086dc Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Tue, 1 May 2018 12:26:28 +0900 Subject: [PATCH] don't send an ACK when receiving a packet that wouldn't be acked There's a lower bound which packets get acknowledged in an ACK frame. When receiving a packet smaller than that bound, which was reported missing before, it's not necessary to immediately queue an ACK, since it wouldn't be included in the ACK frame anyway. --- .../ackhandler/received_packet_handler.go | 2 +- .../received_packet_handler_test.go | 19 ++++++++++++++++++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/internal/ackhandler/received_packet_handler.go b/internal/ackhandler/received_packet_handler.go index a8318e5c5..6bc25412a 100644 --- a/internal/ackhandler/received_packet_handler.go +++ b/internal/ackhandler/received_packet_handler.go @@ -88,7 +88,7 @@ func (h *receivedPacketHandler) IgnoreBelow(p protocol.PacketNumber) { // isMissing says if a packet was reported missing in the last ACK. func (h *receivedPacketHandler) isMissing(p protocol.PacketNumber) bool { - if h.lastAck == nil { + if h.lastAck == nil || p < h.ignoreBelow { return false } return p < h.lastAck.LargestAcked() && !h.lastAck.AcksPacket(p) diff --git a/internal/ackhandler/received_packet_handler_test.go b/internal/ackhandler/received_packet_handler_test.go index c953de7bb..edfa6c154 100644 --- a/internal/ackhandler/received_packet_handler_test.go +++ b/internal/ackhandler/received_packet_handler_test.go @@ -159,7 +159,7 @@ var _ = Describe("receivedPacketHandler", func() { Expect(err).ToNot(HaveOccurred()) err = handler.ReceivedPacket(13, time.Time{}, true) Expect(err).ToNot(HaveOccurred()) - ack := handler.GetAckFrame() // ACK: 1 and 3, missing: 2 + ack := handler.GetAckFrame() // ACK: 1-11 and 13, missing: 12 Expect(ack).ToNot(BeNil()) Expect(ack.HasMissingRanges()).To(BeTrue()) Expect(handler.ackQueued).To(BeFalse()) @@ -168,6 +168,23 @@ var _ = Describe("receivedPacketHandler", func() { Expect(handler.ackQueued).To(BeTrue()) }) + It("doesn't queue an ACK if it was reported missing before, but is below the threshold", func() { + receiveAndAck10Packets() + // 11 is missing + err := handler.ReceivedPacket(12, time.Time{}, true) + Expect(err).ToNot(HaveOccurred()) + err = handler.ReceivedPacket(13, time.Time{}, true) + Expect(err).ToNot(HaveOccurred()) + ack := handler.GetAckFrame() // ACK: 1-10, 12-13 + Expect(ack).ToNot(BeNil()) + // now receive 11 + handler.IgnoreBelow(12) + err = handler.ReceivedPacket(11, time.Time{}, false) + Expect(err).ToNot(HaveOccurred()) + ack = handler.GetAckFrame() + Expect(ack).To(BeNil()) + }) + It("doesn't queue an ACK if the packet closes a gap that was not yet reported", func() { receiveAndAckPacketsUntilAckDecimation() p := protocol.PacketNumber(minReceivedBeforeAckDecimation + 1)