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.
