diff --git a/internal/ackhandler/sent_packet_handler.go b/internal/ackhandler/sent_packet_handler.go index e2bf8b2f1..e1bfa4eac 100644 --- a/internal/ackhandler/sent_packet_handler.go +++ b/internal/ackhandler/sent_packet_handler.go @@ -278,28 +278,29 @@ func (h *sentPacketHandler) ReceivedAck(ack *wire.AckFrame, encLevel protocol.En h.setLossDetectionTimer() } - // maybe update the RTT - if p := pnSpace.history.GetPacket(ack.LargestAcked()); p != nil { - // don't use the ack delay for Initial and Handshake packets - var ackDelay time.Duration - if encLevel == protocol.Encryption1RTT { - ackDelay = utils.MinDuration(ack.DelayTime, h.rttStats.MaxAckDelay()) - } - h.rttStats.UpdateRTT(rcvTime.Sub(p.SendTime), ackDelay, rcvTime) - if h.logger.Debug() { - h.logger.Debugf("\tupdated RTT: %s (σ: %s)", h.rttStats.SmoothedRTT(), h.rttStats.MeanDeviation()) - } - h.congestion.MaybeExitSlowStart() - if h.tracer != nil { - h.tracer.UpdatedMetrics(h.rttStats, h.congestion.GetCongestionWindow(), h.bytesInFlight, h.packetsInFlight()) - } - } - priorInFlight := h.bytesInFlight ackedPackets, err := h.detectAndRemoveAckedPackets(ack, encLevel) if err != nil || len(ackedPackets) == 0 { return err } + // update the RTT, if the largest acked is newly acknowledged + if len(ackedPackets) > 0 { + if p := ackedPackets[len(ackedPackets)-1]; p.PacketNumber == ack.LargestAcked() { + // don't use the ack delay for Initial and Handshake packets + var ackDelay time.Duration + if encLevel == protocol.Encryption1RTT { + ackDelay = utils.MinDuration(ack.DelayTime, h.rttStats.MaxAckDelay()) + } + h.rttStats.UpdateRTT(rcvTime.Sub(p.SendTime), ackDelay, rcvTime) + if h.logger.Debug() { + h.logger.Debugf("\tupdated RTT: %s (σ: %s)", h.rttStats.SmoothedRTT(), h.rttStats.MeanDeviation()) + } + h.congestion.MaybeExitSlowStart() + if h.tracer != nil { + h.tracer.UpdatedMetrics(h.rttStats, h.congestion.GetCongestionWindow(), h.bytesInFlight, h.packetsInFlight()) + } + } + } lostPackets, err := h.detectAndRemoveLostPackets(rcvTime, encLevel) if err != nil { return err @@ -330,6 +331,7 @@ func (h *sentPacketHandler) GetLowestPacketNotConfirmedAcked() protocol.PacketNu return h.lowestNotConfirmedAcked } +// 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 @@ -374,9 +376,6 @@ func (h *sentPacketHandler) detectAndRemoveAckedPackets(ack *wire.AckFrame, encL } for _, p := range ackedPackets { - if packet := pnSpace.history.GetPacket(p.PacketNumber); packet == nil { - continue - } if p.LargestAcked != protocol.InvalidPacketNumber && encLevel == protocol.Encryption1RTT { h.lowestNotConfirmedAcked = utils.MaxPacketNumber(h.lowestNotConfirmedAcked, p.LargestAcked+1) } diff --git a/internal/ackhandler/sent_packet_history.go b/internal/ackhandler/sent_packet_history.go index e8b9bd02e..66b7b42ff 100644 --- a/internal/ackhandler/sent_packet_history.go +++ b/internal/ackhandler/sent_packet_history.go @@ -23,13 +23,6 @@ func (h *sentPacketHistory) SentPacket(p *Packet) { h.packetMap[p.PacketNumber] = el } -func (h *sentPacketHistory) GetPacket(p protocol.PacketNumber) *Packet { - if el, ok := h.packetMap[p]; ok { - return &el.Value - } - return nil -} - // Iterate iterates through all packets. // The callback must not modify the history. func (h *sentPacketHistory) Iterate(cb func(*Packet) (cont bool, err error)) error { diff --git a/internal/ackhandler/sent_packet_history_test.go b/internal/ackhandler/sent_packet_history_test.go index c0e25d087..8ac96f87f 100644 --- a/internal/ackhandler/sent_packet_history_test.go +++ b/internal/ackhandler/sent_packet_history_test.go @@ -56,16 +56,6 @@ var _ = Describe("SentPacketHistory", func() { }) }) - It("gets a packet by packet number", func() { - p := &Packet{PacketNumber: 2} - hist.SentPacket(p) - Expect(hist.GetPacket(2)).To(Equal(p)) - }) - - It("returns nil if the packet doesn't exist", func() { - Expect(hist.GetPacket(1337)).To(BeNil()) - }) - It("removes packets", func() { hist.SentPacket(&Packet{PacketNumber: 1}) hist.SentPacket(&Packet{PacketNumber: 4})