In quic, clear last_inflight_packets_sent_time_ of a packet number space if there is no bytes in flight. protected by gfe2_reloadable_flag_quic_fix_last_inflight_packets_sent_time. PiperOrigin-RevId: 312363743 Change-Id: Ie497ea4cd11d68595993f7597b039d4101d2f011
diff --git a/quic/core/quic_sent_packet_manager_test.cc b/quic/core/quic_sent_packet_manager_test.cc index d479c68..7125986 100644 --- a/quic/core/quic_sent_packet_manager_test.cc +++ b/quic/core/quic_sent_packet_manager_test.cc
@@ -4003,6 +4003,68 @@ manager_.MaybeSendProbePackets(); } +// Regression test for b/156487311 +TEST_F(QuicSentPacketManagerTest, ClearLastInflightPacketsSentTime) { + manager_.EnableMultiplePacketNumberSpacesSupport(); + EXPECT_CALL(*send_algorithm_, PacingRate(_)) + .WillRepeatedly(Return(QuicBandwidth::Zero())); + EXPECT_CALL(*send_algorithm_, GetCongestionWindow()) + .WillRepeatedly(Return(10 * kDefaultTCPMSS)); + + // Send INITIAL 1. + SendDataPacket(1, ENCRYPTION_INITIAL); + const QuicTime packet1_sent_time = clock_.Now(); + // Send HANDSHAKE 2. + clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(10)); + SendDataPacket(2, ENCRYPTION_HANDSHAKE); + SendDataPacket(3, ENCRYPTION_HANDSHAKE); + SendDataPacket(4, ENCRYPTION_HANDSHAKE); + const QuicTime packet2_sent_time = clock_.Now(); + + // Send half RTT 5. + clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(10)); + SendDataPacket(5, ENCRYPTION_FORWARD_SECURE); + + // Received ACK for INITIAL 1. + ExpectAck(1); + clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(90)); + manager_.OnAckFrameStart(QuicPacketNumber(1), QuicTime::Delta::Infinite(), + clock_.Now()); + manager_.OnAckRange(QuicPacketNumber(1), QuicPacketNumber(2)); + EXPECT_EQ(PACKETS_NEWLY_ACKED, + manager_.OnAckFrameEnd(clock_.Now(), QuicPacketNumber(1), + ENCRYPTION_INITIAL)); + RttStats* rtt_stats = const_cast<RttStats*>(manager_.GetRttStats()); + int pto_rttvar_multiplier = + GetQuicReloadableFlag(quic_default_on_pto) ? 2 : 4; + const QuicTime::Delta pto_delay = + rtt_stats->smoothed_rtt() + + pto_rttvar_multiplier * rtt_stats->mean_deviation() + + QuicTime::Delta::FromMilliseconds(kDefaultDelayedAckTimeMs); + if (GetQuicReloadableFlag(quic_fix_last_inflight_packets_sent_time)) { + // Verify PTO is armed based on handshake data. + EXPECT_EQ(packet2_sent_time + pto_delay, manager_.GetRetransmissionTime()); + } else { + // Problematic: PTO is still armed based on initial data. + EXPECT_EQ(packet1_sent_time + pto_delay, manager_.GetRetransmissionTime()); + clock_.AdvanceTime(pto_delay); + manager_.OnRetransmissionTimeout(); + // Nothing to retransmit in INITIAL space. + EXPECT_CALL(notifier_, RetransmitFrames(_, _)).Times(0); + manager_.MaybeSendProbePackets(); + // PING packet gets sent. + SendPingPacket(6, ENCRYPTION_INITIAL); + + // Verify PTO is armed based on packet 2. + EXPECT_EQ(packet2_sent_time + pto_delay * 2, + manager_.GetRetransmissionTime()); + clock_.AdvanceTime(pto_delay * 2); + manager_.OnRetransmissionTimeout(); + EXPECT_CALL(notifier_, RetransmitFrames(_, _)).Times(testing::AtLeast(1)); + manager_.MaybeSendProbePackets(); + } +} + } // namespace } // namespace test } // namespace quic
diff --git a/quic/core/quic_unacked_packet_map.cc b/quic/core/quic_unacked_packet_map.cc index 8276f1c..f06d074 100644 --- a/quic/core/quic_unacked_packet_map.cc +++ b/quic/core/quic_unacked_packet_map.cc
@@ -28,6 +28,7 @@ : perspective_(perspective), least_unacked_(FirstSendingPacketNumber()), bytes_in_flight_(0), + bytes_in_flight_per_packet_number_space_{0, 0, 0}, packets_in_flight_(0), last_inflight_packet_sent_time_(QuicTime::Zero()), last_inflight_packets_sent_time_{{QuicTime::Zero()}, @@ -72,6 +73,7 @@ const PacketNumberSpace packet_number_space = GetPacketNumberSpace(info.encryption_level); bytes_in_flight_ += bytes_sent; + bytes_in_flight_per_packet_number_space_[packet_number_space] += bytes_sent; ++packets_in_flight_; info.in_flight = true; largest_sent_retransmittable_packets_[packet_number_space] = packet_number; @@ -195,6 +197,27 @@ QUIC_BUG_IF(packets_in_flight_ == 0); bytes_in_flight_ -= info->bytes_sent; --packets_in_flight_; + + const PacketNumberSpace packet_number_space = + GetPacketNumberSpace(info->encryption_level); + if (bytes_in_flight_per_packet_number_space_[packet_number_space] < + info->bytes_sent) { + QUIC_BUG << "bytes_in_flight: " + << bytes_in_flight_per_packet_number_space_[packet_number_space] + << " is smaller than bytes_sent: " << info->bytes_sent + << " for packet number space: " + << PacketNumberSpaceToString(packet_number_space); + bytes_in_flight_per_packet_number_space_[packet_number_space] = 0; + } else { + bytes_in_flight_per_packet_number_space_[packet_number_space] -= + info->bytes_sent; + } + if (GetQuicReloadableFlag(quic_fix_last_inflight_packets_sent_time) && + bytes_in_flight_per_packet_number_space_[packet_number_space] == 0) { + QUIC_RELOADABLE_FLAG_COUNT(quic_fix_last_inflight_packets_sent_time); + last_inflight_packets_sent_time_[packet_number_space] = QuicTime::Zero(); + } + info->in_flight = false; } } @@ -230,7 +253,12 @@ } } if (supports_multiple_packet_number_spaces_) { - last_inflight_packets_sent_time_[INITIAL_DATA] = QuicTime::Zero(); + if (GetQuicReloadableFlag(quic_fix_last_inflight_packets_sent_time)) { + DCHECK_EQ(QuicTime::Zero(), + last_inflight_packets_sent_time_[INITIAL_DATA]); + } else { + last_inflight_packets_sent_time_[INITIAL_DATA] = QuicTime::Zero(); + } } return neutered_packets; } @@ -254,7 +282,12 @@ } } if (supports_multiple_packet_number_spaces()) { - last_inflight_packets_sent_time_[HANDSHAKE_DATA] = QuicTime::Zero(); + if (GetQuicReloadableFlag(quic_fix_last_inflight_packets_sent_time)) { + DCHECK_EQ(QuicTime::Zero(), + last_inflight_packets_sent_time_[HANDSHAKE_DATA]); + } else { + last_inflight_packets_sent_time_[HANDSHAKE_DATA] = QuicTime::Zero(); + } } return neutered_packets; }
diff --git a/quic/core/quic_unacked_packet_map.h b/quic/core/quic_unacked_packet_map.h index bcf5061..43e9008 100644 --- a/quic/core/quic_unacked_packet_map.h +++ b/quic/core/quic_unacked_packet_map.h
@@ -281,6 +281,9 @@ QuicPacketNumber least_unacked_; QuicByteCount bytes_in_flight_; + // Bytes in flight per packet number space. + QuicByteCount + bytes_in_flight_per_packet_number_space_[NUM_PACKET_NUMBER_SPACES]; QuicPacketCount packets_in_flight_; // Time that the last inflight packet was sent.