From fb6a536adbff74b47adbcad9a603f436d005377a Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Sun, 22 Nov 2020 11:02:48 +0700 Subject: [PATCH 1/2] add a unit test for ACK of skipped packet detection --- internal/ackhandler/sent_packet_handler_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/internal/ackhandler/sent_packet_handler_test.go b/internal/ackhandler/sent_packet_handler_test.go index 55660f166..8a3e1b563 100644 --- a/internal/ackhandler/sent_packet_handler_test.go +++ b/internal/ackhandler/sent_packet_handler_test.go @@ -171,6 +171,13 @@ var _ = Describe("SentPacketHandler", func() { Expect(handler.appDataPackets.largestAcked).To(Equal(protocol.PacketNumber(4))) }) + It("rejects ACKs that acknowledge a skipped packet number", func() { + handler.SentPacket(ackElicitingPacket(&Packet{PacketNumber: 100})) + handler.SentPacket(ackElicitingPacket(&Packet{PacketNumber: 102})) + ack := &wire.AckFrame{AckRanges: []wire.AckRange{{Smallest: 100, Largest: 102}}} + Expect(handler.ReceivedAck(ack, protocol.Encryption1RTT, time.Now())).To(MatchError("received an ACK for skipped packet number: 101 (1-RTT)")) + }) + It("rejects ACKs with a too high LargestAcked packet number", func() { ack := &wire.AckFrame{AckRanges: []wire.AckRange{{Smallest: 0, Largest: 9999}}} Expect(handler.ReceivedAck(ack, protocol.Encryption1RTT, time.Now())).To(MatchError("PROTOCOL_VIOLATION: Received ACK for an unsent packet")) From 8d14d762e5b2e9c675e0d1f8aef829ebf61fa967 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Sun, 22 Nov 2020 11:39:28 +0700 Subject: [PATCH 2/2] cache the slice used when detecting acked packets --- internal/ackhandler/sent_packet_handler.go | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/internal/ackhandler/sent_packet_handler.go b/internal/ackhandler/sent_packet_handler.go index 0341ae79b..ae5dce830 100644 --- a/internal/ackhandler/sent_packet_handler.go +++ b/internal/ackhandler/sent_packet_handler.go @@ -66,6 +66,8 @@ type sentPacketHandler struct { // Only applies to the application-data packet number space. lowestNotConfirmedAcked protocol.PacketNumber + ackedPackets []*Packet // to avoid allocations in detectAndRemoveAckedPackets + bytesInFlight protocol.ByteCount congestion congestion.SendAlgorithmWithDebugInfos @@ -345,7 +347,7 @@ func (h *sentPacketHandler) GetLowestPacketNotConfirmedAcked() protocol.PacketNu // Packets are returned in ascending packet number order. func (h *sentPacketHandler) detectAndRemoveAckedPackets(ack *wire.AckFrame, encLevel protocol.EncryptionLevel) ([]*Packet, error) { pnSpace := h.getPacketNumberSpace(encLevel) - var ackedPackets []*Packet + h.ackedPackets = h.ackedPackets[:0] ackRangeIndex := 0 lowestAcked := ack.LowestAcked() largestAcked := ack.LargestAcked() @@ -371,22 +373,22 @@ func (h *sentPacketHandler) detectAndRemoveAckedPackets(ack *wire.AckFrame, encL if p.PacketNumber > ackRange.Largest { return false, fmt.Errorf("BUG: ackhandler would have acked wrong packet %d, while evaluating range %d -> %d", p.PacketNumber, ackRange.Smallest, ackRange.Largest) } - ackedPackets = append(ackedPackets, p) + h.ackedPackets = append(h.ackedPackets, p) } } else { - ackedPackets = append(ackedPackets, p) + h.ackedPackets = append(h.ackedPackets, p) } return true, nil }) - if h.logger.Debug() && len(ackedPackets) > 0 { - pns := make([]protocol.PacketNumber, len(ackedPackets)) - for i, p := range ackedPackets { + if h.logger.Debug() && len(h.ackedPackets) > 0 { + pns := make([]protocol.PacketNumber, len(h.ackedPackets)) + for i, p := range h.ackedPackets { pns[i] = p.PacketNumber } h.logger.Debugf("\tnewly acked packets (%d): %d", len(pns), pns) } - for _, p := range ackedPackets { + for _, p := range h.ackedPackets { if p.LargestAcked != protocol.InvalidPacketNumber && encLevel == protocol.Encryption1RTT { h.lowestNotConfirmedAcked = utils.MaxPacketNumber(h.lowestNotConfirmedAcked, p.LargestAcked+1) } @@ -401,7 +403,7 @@ func (h *sentPacketHandler) detectAndRemoveAckedPackets(ack *wire.AckFrame, encL } } - return ackedPackets, err + return h.ackedPackets, err } func (h *sentPacketHandler) getLossTimeAndSpace() (time.Time, protocol.EncryptionLevel) {