ackhandler: record RTT measurements for non-ack-eliciting packets (#5494)

* ackhandler: also save non-ack-eliciting packets in the packet history

* ackhandler: also measure RTTs for non-ack-eliciting packets

diff --git a/internal/ackhandler/packet.go b/internal/ackhandler/packet.go
index f0500e9f2d1b6b8ba3b3d43b2d59e55ff8468243..7bfe599f4a46f57559ba3056980962cb0feeb4f0 100644 (file)
--- a/internal/ackhandler/packet.go
+++ b/internal/ackhandler/packet.go
@@ -28,8 +28,12 @@ type packet struct {
        isPathProbePacket       bool
 }
 
-func (p *packet) outstanding() bool {
-       return !p.declaredLost && !p.IsPathMTUProbePacket && !p.isPathProbePacket
+func (p *packet) Outstanding() bool {
+       return !p.declaredLost && !p.IsPathMTUProbePacket && !p.isPathProbePacket && p.IsAckEliciting()
+}
+
+func (p *packet) IsAckEliciting() bool {
+       return len(p.StreamFrames) > 0 || len(p.Frames) > 0
 }
 
 var packetPool = sync.Pool{New: func() any { return &packet{} }}

diff --git a/internal/ackhandler/sent_packet_handler.go b/internal/ackhandler/sent_packet_handler.go
index 40e8e9b77b28b7bb031fcb495c492acb423545a0..bfdb5855fc3d45a14d03d863765645bb6f09f217 100644 (file)
--- a/internal/ackhandler/sent_packet_handler.go
+++ b/internal/ackhandler/sent_packet_handler.go
@@ -270,15 +270,19 @@ func (h *sentPacketHandler) SentPacket(
        }
 
        pnSpace.largestSent = pn
-       isAckEliciting := len(streamFrames) > 0 || len(frames) > 0
+
+       p := getPacket()
+       p.SendTime = t
+       p.EncryptionLevel = encLevel
+       p.Length = size
+       p.Frames = frames
+       p.LargestAcked = largestAcked
+       p.StreamFrames = streamFrames
+       p.IsPathMTUProbePacket = isPathMTUProbePacket
+       p.isPathProbePacket = isPathProbePacket
+       isAckEliciting := p.IsAckEliciting()
 
        if isPathProbePacket {
-               p := getPacket()
-               p.SendTime = t
-               p.EncryptionLevel = encLevel
-               p.Length = size
-               p.Frames = frames
-               p.isPathProbePacket = true
                pnSpace.history.SentPathProbePacket(pn, p)
                h.setLossDetectionTimer(t)
                return
@@ -286,6 +290,7 @@ func (h *sentPacketHandler) SentPacket(
        if isAckEliciting {
                pnSpace.lastAckElicitingPacketTime = t
                h.bytesInFlight += size
+               p.includedInBytesInFlight = true
                if h.numProbesToSend > 0 {
                        h.numProbesToSend--
                }
@@ -296,25 +301,13 @@ func (h *sentPacketHandler) SentPacket(
                h.ecnTracker.SentPacket(pn, ecn)
        }
 
+       pnSpace.history.SentPacket(pn, p)
        if !isAckEliciting {
-               pnSpace.history.SentNonAckElicitingPacket(pn)
                if !h.peerCompletedAddressValidation {
                        h.setLossDetectionTimer(t)
                }
                return
        }
-
-       p := getPacket()
-       p.SendTime = t
-       p.EncryptionLevel = encLevel
-       p.Length = size
-       p.LargestAcked = largestAcked
-       p.StreamFrames = streamFrames
-       p.Frames = frames
-       p.IsPathMTUProbePacket = isPathMTUProbePacket
-       p.includedInBytesInFlight = true
-
-       pnSpace.history.SentAckElicitingPacket(pn, p)
        if h.qlogger != nil {
                h.qlogMetricsUpdated()
        }
@@ -400,13 +393,15 @@ func (h *sentPacketHandler) ReceivedAck(ack *wire.AckFrame, encLevel protocol.En
        }
 
        priorInFlight := h.bytesInFlight
-       ackedPackets, err := h.detectAndRemoveAckedPackets(ack, encLevel)
+       ackedPackets, hasAckEliciting, err := h.detectAndRemoveAckedPackets(ack, encLevel)
        if err != nil || len(ackedPackets) == 0 {
                return false, err
        }
-       // update the RTT, if the largest acked is newly acknowledged
+       // update the RTT, if:
+       // * the largest acked is newly acknowledged, AND
+       // * at least one new ack-eliciting packet was acknowledged
        if len(ackedPackets) > 0 {
-               if p := ackedPackets[len(ackedPackets)-1]; p.PacketNumber == ack.LargestAcked() && !p.isPathProbePacket {
+               if p := ackedPackets[len(ackedPackets)-1]; p.PacketNumber == ack.LargestAcked() && !p.isPathProbePacket && hasAckEliciting {
                        // don't use the ack delay for Initial and Handshake packets
                        var ackDelay time.Duration
                        if encLevel == protocol.Encryption1RTT {
@@ -522,9 +517,12 @@ func (h *sentPacketHandler) detectSpuriousLosses(ack *wire.AckFrame, ackTime mon
 }
 
 // Packets are returned in ascending packet number order.
-func (h *sentPacketHandler) detectAndRemoveAckedPackets(ack *wire.AckFrame, encLevel protocol.EncryptionLevel) ([]packetWithPacketNumber, error) {
+func (h *sentPacketHandler) detectAndRemoveAckedPackets(
+       ack *wire.AckFrame,
+       encLevel protocol.EncryptionLevel,
+) (_ []packetWithPacketNumber, hasAckEliciting bool, _ error) {
        if len(h.ackedPackets) > 0 {
-               return nil, errors.New("ackhandler BUG: ackedPackets slice not empty")
+               return nil, false, errors.New("ackhandler BUG: ackedPackets slice not empty")
        }
 
        pnSpace := h.getPacketNumberSpace(encLevel)
@@ -532,7 +530,7 @@ func (h *sentPacketHandler) detectAndRemoveAckedPackets(ack *wire.AckFrame, encL
        if encLevel == protocol.Encryption1RTT {
                for p := range pnSpace.history.SkippedPackets() {
                        if ack.AcksPacket(p) {
-                               return nil, &qerr.TransportError{
+                               return nil, false, &qerr.TransportError{
                                        ErrorCode:    qerr.ProtocolViolation,
                                        ErrorMessage: fmt.Sprintf("received an ACK for skipped packet number: %d (%s)", p, encLevel),
                                }
@@ -564,7 +562,7 @@ func (h *sentPacketHandler) detectAndRemoveAckedPackets(ack *wire.AckFrame, encL
                                continue
                        }
                        if pn > ackRange.Largest {
-                               return nil, fmt.Errorf("BUG: ackhandler would have acked wrong packet %d, while evaluating range %d -> %d", pn, ackRange.Smallest, ackRange.Largest)
+                               return nil, false, fmt.Errorf("BUG: ackhandler would have acked wrong packet %d, while evaluating range %d -> %d", pn, ackRange.Smallest, ackRange.Largest)
                        }
                }
                if p.isPathProbePacket {
@@ -575,6 +573,9 @@ func (h *sentPacketHandler) detectAndRemoveAckedPackets(ack *wire.AckFrame, encL
                        }
                        continue
                }
+               if p.IsAckEliciting() {
+                       hasAckEliciting = true
+               }
                h.ackedPackets = append(h.ackedPackets, packetWithPacketNumber{PacketNumber: pn, packet: p})
        }
        if h.logger.Debug() && len(h.ackedPackets) > 0 {
@@ -601,11 +602,11 @@ func (h *sentPacketHandler) detectAndRemoveAckedPackets(ack *wire.AckFrame, encL
                        }
                }
                if err := pnSpace.history.Remove(p.PacketNumber); err != nil {
-                       return nil, err
+                       return nil, false, err
                }
        }
        // TODO: add support for the transport:packets_acked qlog event
-       return h.ackedPackets, nil
+       return h.ackedPackets, hasAckEliciting, nil
 }
 
 func (h *sentPacketHandler) getLossTimeAndSpace() (monotime.Time, protocol.EncryptionLevel) {
@@ -799,7 +800,7 @@ func (h *sentPacketHandler) detectLostPackets(now monotime.Time, encLevel protoc
                var packetLost bool
                if !p.SendTime.After(lostSendTime) {
                        packetLost = true
-                       if !p.isPathProbePacket {
+                       if !p.isPathProbePacket && p.IsAckEliciting() {
                                if h.logger.Debug() {
                                        h.logger.Debugf("\tlost packet %d (time threshold)", pn)
                                }
@@ -815,7 +816,7 @@ func (h *sentPacketHandler) detectLostPackets(now monotime.Time, encLevel protoc
                        }
                } else if pnSpace.history.Difference(pnSpace.largestAcked, pn) >= packetThreshold {
                        packetLost = true
-                       if !p.isPathProbePacket {
+                       if !p.isPathProbePacket && p.IsAckEliciting() {
                                if h.logger.Debug() {
                                        h.logger.Debugf("\tlost packet %d (reordering threshold)", pn)
                                }
@@ -842,7 +843,7 @@ func (h *sentPacketHandler) detectLostPackets(now monotime.Time, encLevel protoc
                                h.lostPackets.Add(pn, p.SendTime)
                        }
                        pnSpace.history.DeclareLost(pn)
-                       if !p.isPathProbePacket {
+                       if !p.isPathProbePacket && p.IsAckEliciting() {
                                // the bytes in flight need to be reduced no matter if the frames in this packet will be retransmitted
                                h.removeFromBytesInFlight(p)
                                h.queueFramesForRetransmission(p)
@@ -1070,14 +1071,14 @@ func (h *sentPacketHandler) ResetForRetry(now monotime.Time) {
                if firstPacketSendTime.IsZero() {
                        firstPacketSendTime = p.SendTime
                }
-               if !p.declaredLost {
+               if !p.declaredLost && p.IsAckEliciting() {
                        h.queueFramesForRetransmission(p)
                }
        }
        // All application data packets sent at this point are 0-RTT packets.
        // In the case of a Retry, we can assume that the server dropped all of them.
        for _, p := range h.appDataPackets.history.Packets() {
-               if !p.declaredLost {
+               if !p.declaredLost && p.IsAckEliciting() {
                        h.queueFramesForRetransmission(p)
                }
        }
@@ -1115,7 +1116,9 @@ func (h *sentPacketHandler) MigratedPath(now monotime.Time, initialMaxDatagramSi
                h.appDataPackets.history.DeclareLost(pn)
                if !p.isPathProbePacket {
                        h.removeFromBytesInFlight(p)
-                       h.queueFramesForRetransmission(p)
+                       if p.IsAckEliciting() {
+                               h.queueFramesForRetransmission(p)
+                       }
                }
        }
        for pn := range h.appDataPackets.history.PathProbes() {

diff --git a/internal/ackhandler/sent_packet_handler_test.go b/internal/ackhandler/sent_packet_handler_test.go
index a482716f3dfb11121d6a40e4d19cf2d3bef29dcd..a1e80b1027126fff45393115105a753998ce854a 100644 (file)
--- a/internal/ackhandler/sent_packet_handler_test.go
+++ b/internal/ackhandler/sent_packet_handler_test.go
@@ -202,19 +202,76 @@ func TestSentPacketHandlerAcknowledgeSkippedPacket(t *testing.T) {
        require.ErrorContains(t, err, fmt.Sprintf("received an ACK for skipped packet number: %d (1-RTT)", skippedPN))
 }
 
-func TestSentPacketHandlerRTTs(t *testing.T) {
+func TestSentPacketHandlerRTT(t *testing.T) {
+       rttStats := utils.NewRTTStats()
+       sph := NewSentPacketHandler(
+               0,
+               1200,
+               rttStats,
+               &utils.ConnectionStats{},
+               false,
+               false,
+               nil,
+               protocol.PerspectiveClient,
+               nil,
+               utils.DefaultLogger,
+       )
+
+       sendPacket := func(t *testing.T, ti monotime.Time, ackEliciting bool) protocol.PacketNumber {
+               t.Helper()
+               pn := sph.PopPacketNumber(protocol.Encryption1RTT)
+               var frames []Frame
+               if ackEliciting {
+                       frames = []Frame{{Frame: &wire.PingFrame{}}}
+               }
+               sph.SentPacket(ti, pn, protocol.InvalidPacketNumber, nil, frames, protocol.Encryption1RTT, protocol.ECNNon, 1200, false, false)
+               return pn
+       }
+
+       ackPackets := func(t *testing.T, ti monotime.Time, pns ...protocol.PacketNumber) {
+               t.Helper()
+               _, err := sph.ReceivedAck(&wire.AckFrame{AckRanges: ackRanges(pns...)}, protocol.Encryption1RTT, ti)
+               require.NoError(t, err)
+       }
+
+       now := monotime.Now()
+       pn1 := sendPacket(t, now, true)
+       pn2 := sendPacket(t, now, false)
+       pn3 := sendPacket(t, now, true)
+       // the RTT is recorded, since the largest acknowledged packet is ack-eliciting
+       now = now.Add(200 * time.Millisecond)
+       ackPackets(t, now, pn1, pn2, pn3)
+       require.Equal(t, 200*time.Millisecond, rttStats.LatestRTT())
+
+       pn4 := sendPacket(t, now, false)
+       pn5 := sendPacket(t, now, false)
+       now = now.Add(500 * time.Millisecond)
+       // only non-ack-eliciting packets are newly acknowledged, so the RTT is not updated
+       ackPackets(t, now, pn2, pn3, pn4, pn5)
+       require.Equal(t, 200*time.Millisecond, rttStats.LatestRTT())
+
+       pn6 := sendPacket(t, now, true)
+       pn7 := sendPacket(t, now, false)
+       now = now.Add(800 * time.Millisecond)
+       // largest acknowledged packet is not ack-eliciting, but one new ack-eliciting
+       // packet was acknowledged, so the RTT is updated
+       ackPackets(t, now, pn6, pn7)
+       require.Equal(t, 800*time.Millisecond, rttStats.LatestRTT())
+}
+
+func TestSentPacketHandlerRTTAckDelays(t *testing.T) {
        t.Run("Initial", func(t *testing.T) {
-               testSentPacketHandlerRTTs(t, protocol.EncryptionInitial, false)
+               testSentPacketHandlerRTTAckDelays(t, protocol.EncryptionInitial, false)
        })
        t.Run("Handshake", func(t *testing.T) {
-               testSentPacketHandlerRTTs(t, protocol.EncryptionHandshake, false)
+               testSentPacketHandlerRTTAckDelays(t, protocol.EncryptionHandshake, false)
        })
        t.Run("1-RTT", func(t *testing.T) {
-               testSentPacketHandlerRTTs(t, protocol.Encryption1RTT, true)
+               testSentPacketHandlerRTTAckDelays(t, protocol.Encryption1RTT, true)
        })
 }
 
-func testSentPacketHandlerRTTs(t *testing.T, encLevel protocol.EncryptionLevel, usesAckDelay bool) {
+func testSentPacketHandlerRTTAckDelays(t *testing.T, encLevel protocol.EncryptionLevel, usesAckDelay bool) {
        expectedRTTStats := utils.NewRTTStats()
        expectedRTTStats.SetMaxAckDelay(time.Second)
        rttStats := utils.NewRTTStats()

diff --git a/internal/ackhandler/sent_packet_history.go b/internal/ackhandler/sent_packet_history.go
index 54b74cf7450155156c60ebbc30274bcc0146f1f4..14a7049c244d8b682f9ea0e190b9eb4ab7712915 100644 (file)
--- a/internal/ackhandler/sent_packet_history.go
+++ b/internal/ackhandler/sent_packet_history.go
@@ -58,17 +58,10 @@ func (h *sentPacketHistory) SkippedPacket(pn protocol.PacketNumber) {
        h.skippedPackets = append(h.skippedPackets, pn)
 }
 
-func (h *sentPacketHistory) SentNonAckElicitingPacket(pn protocol.PacketNumber) {
-       h.checkSequentialPacketNumberUse(pn)
-       if len(h.packets) > 0 {
-               h.packets = append(h.packets, nil)
-       }
-}
-
-func (h *sentPacketHistory) SentAckElicitingPacket(pn protocol.PacketNumber, p *packet) {
+func (h *sentPacketHistory) SentPacket(pn protocol.PacketNumber, p *packet) {
        h.checkSequentialPacketNumberUse(pn)
        h.packets = append(h.packets, p)
-       if p.outstanding() {
+       if p.Outstanding() {
                h.numOutstanding++
        }
 }
@@ -111,7 +104,7 @@ func (h *sentPacketHistory) FirstOutstanding() (protocol.PacketNumber, *packet)
                return protocol.InvalidPacketNumber, nil
        }
        for i, p := range h.packets {
-               if p != nil && p.outstanding() {
+               if p != nil && p.Outstanding() {
                        return h.firstPacketNumber + protocol.PacketNumber(i), p
                }
        }
@@ -148,7 +141,7 @@ func (h *sentPacketHistory) Remove(pn protocol.PacketNumber) error {
                return fmt.Errorf("packet %d not found in sent packet history", pn)
        }
        p := h.packets[idx]
-       if p.outstanding() {
+       if p.Outstanding() {
                h.numOutstanding--
                if h.numOutstanding < 0 {
                        panic("negative number of outstanding packets")
@@ -243,7 +236,7 @@ func (h *sentPacketHistory) DeclareLost(pn protocol.PacketNumber) {
                return
        }
        p := h.packets[idx]
-       if p.outstanding() {
+       if p.Outstanding() {
                h.numOutstanding--
                if h.numOutstanding < 0 {
                        panic("negative number of outstanding packets")

diff --git a/internal/ackhandler/sent_packet_history_test.go b/internal/ackhandler/sent_packet_history_test.go
index 15e259f929d55a413af7ebb1cba43dda262a8b73..0740ac8d4b1bd9ea44b798df5c71515da1c657a0 100644 (file)
--- a/internal/ackhandler/sent_packet_history_test.go
+++ b/internal/ackhandler/sent_packet_history_test.go
@@ -4,12 +4,16 @@ import (
        "slices"
        "testing"
 
-       "github.com/quic-go/quic-go/internal/monotime"
        "github.com/quic-go/quic-go/internal/protocol"
+       "github.com/quic-go/quic-go/internal/wire"
 
        "github.com/stretchr/testify/require"
 )
 
+func ackElicitingPacket() *packet {
+       return &packet{StreamFrames: []StreamFrame{{Frame: &wire.StreamFrame{StreamID: 1}}}}
+}
+
 func (h *sentPacketHistory) getPacketNumbers() []protocol.PacketNumber {
        pns := make([]protocol.PacketNumber, 0, len(h.packets))
        for pn := range h.Packets() {
@@ -29,68 +33,57 @@ func TestSentPacketHistoryPacketTracking(t *testing.T) {
 
 func testSentPacketHistoryPacketTracking(t *testing.T, firstPacketAckEliciting bool) {
        hist := newSentPacketHistory(true)
-       now := monotime.Now()
 
-       var firstPacketNumber []protocol.PacketNumber
        require.False(t, hist.HasOutstandingPackets())
        if firstPacketAckEliciting {
-               hist.SentAckElicitingPacket(0, &packet{})
+               hist.SentPacket(0, ackElicitingPacket())
                require.True(t, hist.HasOutstandingPackets())
-               firstPacketNumber = append(firstPacketNumber, 0)
        } else {
-               hist.SentNonAckElicitingPacket(0)
+               hist.SentPacket(0, &packet{})
                require.False(t, hist.HasOutstandingPackets())
        }
-       hist.SentAckElicitingPacket(1, &packet{})
-       hist.SentAckElicitingPacket(2, &packet{})
-       require.Equal(t, append(firstPacketNumber, 1, 2), hist.getPacketNumbers())
+       hist.SentPacket(1, ackElicitingPacket())
+       hist.SentPacket(2, ackElicitingPacket())
+       require.Equal(t, []protocol.PacketNumber{0, 1, 2}, hist.getPacketNumbers())
        require.Empty(t, slices.Collect(hist.SkippedPackets()))
-       if firstPacketAckEliciting {
-               require.Equal(t, 3, hist.Len())
-       } else {
-               require.Equal(t, 2, hist.Len())
-       }
+       require.Equal(t, 3, hist.Len())
 
-       // non-ack-eliciting packets are not saved
-       hist.SentNonAckElicitingPacket(3)
-       hist.SentAckElicitingPacket(4, &packet{SendTime: now})
-       hist.SentNonAckElicitingPacket(5)
-       hist.SentAckElicitingPacket(6, &packet{SendTime: now})
-       require.Equal(t, append(firstPacketNumber, 1, 2, 4, 6), hist.getPacketNumbers())
+       // non-ack-eliciting packets are saved, but don't count as outstanding
+       hist.SentPacket(3, &packet{})
+       hist.SentPacket(4, ackElicitingPacket())
+       hist.SentPacket(5, &packet{})
+       hist.SentPacket(6, ackElicitingPacket())
+       require.Equal(t, []protocol.PacketNumber{0, 1, 2, 3, 4, 5, 6}, hist.getPacketNumbers())
 
        // handle skipped packet numbers
        hist.SkippedPacket(7)
-       hist.SentAckElicitingPacket(8, &packet{SendTime: now})
-       hist.SentNonAckElicitingPacket(9)
+       hist.SentPacket(8, ackElicitingPacket())
+       hist.SentPacket(9, &packet{})
        hist.SkippedPacket(10)
-       hist.SentAckElicitingPacket(11, &packet{SendTime: now})
-       require.Equal(t, append(firstPacketNumber, 1, 2, 4, 6, 8, 11), hist.getPacketNumbers())
+       hist.SentPacket(11, ackElicitingPacket())
+       require.Equal(t, []protocol.PacketNumber{0, 1, 2, 3, 4, 5, 6, 8, 9, 11}, hist.getPacketNumbers())
        require.Equal(t, []protocol.PacketNumber{7, 10}, slices.Collect(hist.SkippedPackets()))
-       if firstPacketAckEliciting {
-               require.Equal(t, 12, hist.Len())
-       } else {
-               require.Equal(t, 11, hist.Len())
-       }
+       require.Equal(t, 12, hist.Len())
 }
 
 func TestSentPacketHistoryNonSequentialPacketNumberUse(t *testing.T) {
        hist := newSentPacketHistory(true)
-       hist.SentAckElicitingPacket(100, &packet{})
+       hist.SentPacket(100, ackElicitingPacket())
        require.Panics(t, func() {
-               hist.SentAckElicitingPacket(102, &packet{})
+               hist.SentPacket(102, ackElicitingPacket())
        })
 }
 
 func TestSentPacketHistoryRemovePackets(t *testing.T) {
        hist := newSentPacketHistory(true)
 
-       hist.SentAckElicitingPacket(0, &packet{})
-       hist.SentAckElicitingPacket(1, &packet{})
+       hist.SentPacket(0, ackElicitingPacket())
+       hist.SentPacket(1, ackElicitingPacket())
        hist.SkippedPacket(2)
        hist.SkippedPacket(3)
-       hist.SentAckElicitingPacket(4, &packet{})
+       hist.SentPacket(4, ackElicitingPacket())
        hist.SkippedPacket(5)
-       hist.SentAckElicitingPacket(6, &packet{})
+       hist.SentPacket(6, ackElicitingPacket())
        require.Equal(t, []protocol.PacketNumber{0, 1, 4, 6}, hist.getPacketNumbers())
        require.Equal(t, []protocol.PacketNumber{2, 3, 5}, slices.Collect(hist.SkippedPackets()))
 
@@ -102,12 +95,12 @@ func TestSentPacketHistoryRemovePackets(t *testing.T) {
        require.Equal(t, []protocol.PacketNumber{2, 3, 5}, slices.Collect(hist.SkippedPackets()))
 
        // add one more packet
-       hist.SentAckElicitingPacket(7, &packet{})
+       hist.SentPacket(7, ackElicitingPacket())
        require.Equal(t, []protocol.PacketNumber{4, 6, 7}, hist.getPacketNumbers())
 
        // remove last packet and add another
        require.NoError(t, hist.Remove(7))
-       hist.SentAckElicitingPacket(8, &packet{})
+       hist.SentPacket(8, ackElicitingPacket())
        require.Equal(t, []protocol.PacketNumber{4, 6, 8}, hist.getPacketNumbers())
 
        // try to remove non-existent packet
@@ -118,7 +111,7 @@ func TestSentPacketHistoryRemovePackets(t *testing.T) {
        // only the last 4 skipped packets should be preserved
        hist.SkippedPacket(9)
        hist.SkippedPacket(10)
-       hist.SentAckElicitingPacket(11, &packet{})
+       hist.SentPacket(11, ackElicitingPacket())
        hist.SkippedPacket(12)
        require.Equal(t, []protocol.PacketNumber{5, 9, 10, 12}, slices.Collect(hist.SkippedPackets()))
 
@@ -139,8 +132,8 @@ func TestSentPacketHistoryFirstOutstandingPacket(t *testing.T) {
        require.Equal(t, protocol.InvalidPacketNumber, pn)
        require.Nil(t, p)
 
-       hist.SentAckElicitingPacket(2, &packet{})
-       hist.SentAckElicitingPacket(3, &packet{})
+       hist.SentPacket(2, ackElicitingPacket())
+       hist.SentPacket(3, ackElicitingPacket())
        pn, p = hist.FirstOutstanding()
        require.Equal(t, protocol.PacketNumber(2), pn)
        require.NotNil(t, p)
@@ -153,9 +146,11 @@ func TestSentPacketHistoryFirstOutstandingPacket(t *testing.T) {
 
        // Path MTU packets are not regarded as outstanding
        hist = newSentPacketHistory(true)
-       hist.SentAckElicitingPacket(2, &packet{})
+       hist.SentPacket(2, ackElicitingPacket())
        hist.SkippedPacket(3)
-       hist.SentAckElicitingPacket(4, &packet{IsPathMTUProbePacket: true})
+       p = ackElicitingPacket()
+       p.IsPathMTUProbePacket = true
+       hist.SentPacket(4, p)
        pn, p = hist.FirstOutstanding()
        require.NotNil(t, p)
        require.Equal(t, protocol.PacketNumber(2), pn)
@@ -164,12 +159,12 @@ func TestSentPacketHistoryFirstOutstandingPacket(t *testing.T) {
 func TestSentPacketHistoryIterating(t *testing.T) {
        hist := newSentPacketHistory(true)
        hist.SkippedPacket(0)
-       hist.SentAckElicitingPacket(1, &packet{})
-       hist.SentAckElicitingPacket(2, &packet{})
-       hist.SentAckElicitingPacket(3, &packet{})
+       hist.SentPacket(1, ackElicitingPacket())
+       hist.SentPacket(2, ackElicitingPacket())
+       hist.SentPacket(3, ackElicitingPacket())
        hist.SkippedPacket(4)
        hist.SkippedPacket(5)
-       hist.SentAckElicitingPacket(6, &packet{})
+       hist.SentPacket(6, ackElicitingPacket())
        require.Equal(t, []protocol.PacketNumber{0, 4, 5}, slices.Collect(hist.SkippedPackets()))
        require.NoError(t, hist.Remove(3))
 
@@ -185,12 +180,12 @@ func TestSentPacketHistoryIterating(t *testing.T) {
 
 func TestSentPacketHistoryDeleteWhileIterating(t *testing.T) {
        hist := newSentPacketHistory(true)
-       hist.SentAckElicitingPacket(0, &packet{})
-       hist.SentAckElicitingPacket(1, &packet{})
+       hist.SentPacket(0, ackElicitingPacket())
+       hist.SentPacket(1, ackElicitingPacket())
        hist.SkippedPacket(2)
-       hist.SentAckElicitingPacket(3, &packet{})
+       hist.SentPacket(3, ackElicitingPacket())
        hist.SkippedPacket(4)
-       hist.SentAckElicitingPacket(5, &packet{})
+       hist.SentPacket(5, ackElicitingPacket())
 
        var iterations []protocol.PacketNumber
        for pn := range hist.Packets() {
@@ -210,12 +205,12 @@ func TestSentPacketHistoryDeleteWhileIterating(t *testing.T) {
 
 func TestSentPacketHistoryPathProbes(t *testing.T) {
        hist := newSentPacketHistory(true)
-       hist.SentAckElicitingPacket(0, &packet{})
-       hist.SentAckElicitingPacket(1, &packet{})
-       hist.SentPathProbePacket(2, &packet{})
-       hist.SentAckElicitingPacket(3, &packet{})
-       hist.SentAckElicitingPacket(4, &packet{})
-       hist.SentPathProbePacket(5, &packet{})
+       hist.SentPacket(0, ackElicitingPacket())
+       hist.SentPacket(1, ackElicitingPacket())
+       hist.SentPathProbePacket(2, ackElicitingPacket())
+       hist.SentPacket(3, ackElicitingPacket())
+       hist.SentPacket(4, ackElicitingPacket())
+       hist.SentPathProbePacket(5, ackElicitingPacket())
 
        getPacketsInHistory := func(t *testing.T) []protocol.PacketNumber {
                t.Helper()
@@ -288,7 +283,7 @@ func TestSentPacketHistoryPathProbes(t *testing.T) {
        require.Nil(t, p)
 
        // path probe packets are considered outstanding
-       hist.SentPathProbePacket(6, &packet{})
+       hist.SentPathProbePacket(6, ackElicitingPacket())
        require.False(t, hist.HasOutstandingPackets())
        require.True(t, hist.HasOutstandingPathProbes())
        pn, p = hist.FirstOutstandingPathProbe()
@@ -308,16 +303,16 @@ func TestSentPacketHistoryPathProbes(t *testing.T) {
 
 func TestSentPacketHistoryDifference(t *testing.T) {
        hist := newSentPacketHistory(true)
-       hist.SentNonAckElicitingPacket(0)
-       hist.SentAckElicitingPacket(1, &packet{})
-       hist.SentAckElicitingPacket(2, &packet{})
-       hist.SentAckElicitingPacket(3, &packet{})
+       hist.SentPacket(0, &packet{})
+       hist.SentPacket(1, ackElicitingPacket())
+       hist.SentPacket(2, ackElicitingPacket())
+       hist.SentPacket(3, ackElicitingPacket())
        hist.SkippedPacket(4)
        hist.SkippedPacket(5)
-       hist.SentAckElicitingPacket(6, &packet{})
-       hist.SentNonAckElicitingPacket(7)
+       hist.SentPacket(6, ackElicitingPacket())
+       hist.SentPacket(7, &packet{})
        hist.SkippedPacket(8)
-       hist.SentAckElicitingPacket(9, &packet{})
+       hist.SentPacket(9, ackElicitingPacket())
 
        require.Zero(t, hist.Difference(1, 1))
        require.Zero(t, hist.Difference(2, 2))