gfe-relnote: In QUIC, add QuicPacketNumber::UpdateMax function. No functional change expected. Not protected. UpdateMax updates packet number to be new_value if it is greater. PiperOrigin-RevId: 239268272 Change-Id: I07be6cc28cb77dc57bb4205e4052ca8a7b68d995
diff --git a/quic/core/congestion_control/tcp_cubic_sender_bytes.cc b/quic/core/congestion_control/tcp_cubic_sender_bytes.cc index 2ddf769..fde8d75 100644 --- a/quic/core/congestion_control/tcp_cubic_sender_bytes.cc +++ b/quic/core/congestion_control/tcp_cubic_sender_bytes.cc
@@ -148,12 +148,7 @@ QuicByteCount acked_bytes, QuicByteCount prior_in_flight, QuicTime event_time) { - if (largest_acked_packet_number_.IsInitialized()) { - largest_acked_packet_number_ = - std::max(acked_packet_number, largest_acked_packet_number_); - } else { - largest_acked_packet_number_ = acked_packet_number; - } + largest_acked_packet_number_.UpdateMax(acked_packet_number); if (InRecovery()) { if (!no_prr_) { // PRR is used when in recovery.
diff --git a/quic/core/quic_framer.cc b/quic/core/quic_framer.cc index 2d1e665..08c0121 100644 --- a/quic/core/quic_framer.cc +++ b/quic/core/quic_framer.cc
@@ -1850,12 +1850,7 @@ // Update the largest packet number after we have decrypted the packet // so we are confident is not attacker controlled. - if (largest_packet_number_.IsInitialized()) { - largest_packet_number_ = - std::max(header->packet_number, largest_packet_number_); - } else { - largest_packet_number_ = header->packet_number; - } + largest_packet_number_.UpdateMax(header->packet_number); if (!visitor_->OnPacketHeader(*header)) { RecordDroppedPacketReason(DroppedPacketReason::INVALID_PACKET_NUMBER); @@ -1928,12 +1923,7 @@ // Update the largest packet number after we have decrypted the packet // so we are confident is not attacker controlled. - if (largest_packet_number_.IsInitialized()) { - largest_packet_number_ = - std::max(header->packet_number, largest_packet_number_); - } else { - largest_packet_number_ = header->packet_number; - } + largest_packet_number_.UpdateMax(header->packet_number); if (!visitor_->OnPacketHeader(*header)) { // The visitor suppresses further processing of the packet.
diff --git a/quic/core/quic_packet_number.cc b/quic/core/quic_packet_number.cc index 8a799c1..3996993 100644 --- a/quic/core/quic_packet_number.cc +++ b/quic/core/quic_packet_number.cc
@@ -19,6 +19,17 @@ packet_number_ = UninitializedPacketNumber(); } +void QuicPacketNumber::UpdateMax(QuicPacketNumber new_value) { + if (!new_value.IsInitialized()) { + return; + } + if (!IsInitialized()) { + packet_number_ = new_value.ToUint64(); + } else { + packet_number_ = std::max(packet_number_, new_value.ToUint64()); + } +} + uint64_t QuicPacketNumber::Hash() const { DCHECK(IsInitialized()); return packet_number_;
diff --git a/quic/core/quic_packet_number.h b/quic/core/quic_packet_number.h index e32d0c3..a6559d9 100644 --- a/quic/core/quic_packet_number.h +++ b/quic/core/quic_packet_number.h
@@ -30,6 +30,10 @@ // Packet number becomes uninitialized after calling this function. void Clear(); + // Updates this packet number to be |new_value| if it is greater than current + // value. + void UpdateMax(QuicPacketNumber new_value); + // REQUIRES: IsInitialized() == true. uint64_t Hash() const;
diff --git a/quic/core/quic_packet_number_test.cc b/quic/core/quic_packet_number_test.cc index 5e32b8c..9c0b6d8 100644 --- a/quic/core/quic_packet_number_test.cc +++ b/quic/core/quic_packet_number_test.cc
@@ -21,8 +21,16 @@ EXPECT_TRUE(num2.IsInitialized()); EXPECT_EQ(10u, num2.ToUint64()); EXPECT_EQ(10u, num2.Hash()); + num2.UpdateMax(num); + EXPECT_EQ(10u, num2.ToUint64()); + num2.UpdateMax(QuicPacketNumber(9)); + EXPECT_EQ(10u, num2.ToUint64()); + num2.UpdateMax(QuicPacketNumber(11)); + EXPECT_EQ(11u, num2.ToUint64()); num2.Clear(); EXPECT_FALSE(num2.IsInitialized()); + num2.UpdateMax(QuicPacketNumber(9)); + EXPECT_EQ(9u, num2.ToUint64()); if (!GetQuicRestartFlag(quic_uint64max_uninitialized_pn)) { QuicPacketNumber num3(std::numeric_limits<uint64_t>::max());
diff --git a/quic/core/quic_sent_packet_manager.cc b/quic/core/quic_sent_packet_manager.cc index efe0ab6..b1ba487 100644 --- a/quic/core/quic_sent_packet_manager.cc +++ b/quic/core/quic_sent_packet_manager.cc
@@ -1146,14 +1146,7 @@ QUIC_DVLOG(1) << ENDPOINT << "Got an ack for packet " << acked_packet.packet_number; last_ack_frame_.packets.Add(acked_packet.packet_number); - if (info->largest_acked.IsInitialized()) { - if (largest_packet_peer_knows_is_acked_.IsInitialized()) { - largest_packet_peer_knows_is_acked_ = - std::max(largest_packet_peer_knows_is_acked_, info->largest_acked); - } else { - largest_packet_peer_knows_is_acked_ = info->largest_acked; - } - } + largest_packet_peer_knows_is_acked_.UpdateMax(info->largest_acked); // If data is associated with the most recent transmission of this // packet, then inform the caller. if (info->in_flight) {
diff --git a/quic/core/quic_unacked_packet_map.cc b/quic/core/quic_unacked_packet_map.cc index fcd5308..0f5fa49 100644 --- a/quic/core/quic_unacked_packet_map.cc +++ b/quic/core/quic_unacked_packet_map.cc
@@ -68,12 +68,7 @@ packet->encryption_level, packet->packet_number_length, transmission_type, sent_time, bytes_sent, has_crypto_handshake, packet->num_padding_bytes); info.largest_acked = packet->largest_acked; - if (packet->largest_acked.IsInitialized()) { - largest_sent_largest_acked_ = - largest_sent_largest_acked_.IsInitialized() - ? std::max(largest_sent_largest_acked_, packet->largest_acked) - : packet->largest_acked; - } + largest_sent_largest_acked_.UpdateMax(packet->largest_acked); if (old_packet_number.IsInitialized()) { TransferRetransmissionInfo(old_packet_number, packet_number, transmission_type, &info); @@ -234,12 +229,7 @@ DCHECK(use_uber_loss_algorithm_); const PacketNumberSpace packet_number_space = GetPacketNumberSpace(encryption_level); - if (!largest_acked_packets_[packet_number_space].IsInitialized()) { - largest_acked_packets_[packet_number_space] = packet_number; - } else { - largest_acked_packets_[packet_number_space] = - std::max(largest_acked_packets_[packet_number_space], packet_number); - } + largest_acked_packets_[packet_number_space].UpdateMax(packet_number); } bool QuicUnackedPacketMap::IsPacketUsefulForMeasuringRtt(