In QuicReceivedPacketManager, only record receive timestamps for packets received in order for IETF QUIC.
PiperOrigin-RevId: 424344189
diff --git a/quic/core/quic_received_packet_manager.cc b/quic/core/quic_received_packet_manager.cc
index 079778c..d0f1578 100644
--- a/quic/core/quic_received_packet_manager.cc
+++ b/quic/core/quic_received_packet_manager.cc
@@ -38,6 +38,7 @@
max_ack_ranges_(0),
time_largest_observed_(QuicTime::Zero()),
save_timestamps_(false),
+ save_timestamps_for_in_order_packets_(false),
stats_(stats),
num_retransmittable_packets_received_since_last_ack_sent_(0),
min_received_before_ack_decimation_(kMinReceivedBeforeAckDecimation),
@@ -80,9 +81,12 @@
}
ack_frame_updated_ = true;
+ // Whether |packet_number| is received out of order.
+ bool packet_reordered = false;
if (LargestAcked(ack_frame_).IsInitialized() &&
LargestAcked(ack_frame_) > packet_number) {
// Record how out of order stats.
+ packet_reordered = true;
++stats_->packets_reordered;
stats_->max_sequence_reordering =
std::max(stats_->max_sequence_reordering,
@@ -101,8 +105,11 @@
if (save_timestamps_) {
// The timestamp format only handles packets in time order.
- if (!ack_frame_.received_packet_times.empty() &&
- ack_frame_.received_packet_times.back().second > receipt_time) {
+ if (save_timestamps_for_in_order_packets_ && packet_reordered) {
+ QUIC_DLOG(WARNING) << "Not saving receive timestamp for packet "
+ << packet_number;
+ } else if (!ack_frame_.received_packet_times.empty() &&
+ ack_frame_.received_packet_times.back().second > receipt_time) {
QUIC_LOG(WARNING)
<< "Receive time went backwards from: "
<< ack_frame_.received_packet_times.back().second.ToDebuggingValue()
diff --git a/quic/core/quic_received_packet_manager.h b/quic/core/quic_received_packet_manager.h
index 822725d..21f7f04 100644
--- a/quic/core/quic_received_packet_manager.h
+++ b/quic/core/quic_received_packet_manager.h
@@ -103,8 +103,9 @@
max_ack_ranges_ = max_ack_ranges;
}
- void set_save_timestamps(bool save_timestamps) {
+ void set_save_timestamps(bool save_timestamps, bool in_order_packets_only) {
save_timestamps_ = save_timestamps;
+ save_timestamps_for_in_order_packets_ = in_order_packets_only;
}
size_t min_received_before_ack_decimation() const {
@@ -167,6 +168,10 @@
// If true, save timestamps in the ack_frame_.
bool save_timestamps_;
+ // If true and |save_timestamps_|, only save timestamps for packets that are
+ // received in order.
+ bool save_timestamps_for_in_order_packets_;
+
// Least packet number received from peer.
QuicPacketNumber least_received_packet_number_;
diff --git a/quic/core/quic_received_packet_manager_test.cc b/quic/core/quic_received_packet_manager_test.cc
index 9d6b664..cc04381 100644
--- a/quic/core/quic_received_packet_manager_test.cc
+++ b/quic/core/quic_received_packet_manager_test.cc
@@ -46,7 +46,7 @@
QuicReceivedPacketManagerTest() : received_manager_(&stats_) {
clock_.AdvanceTime(QuicTime::Delta::FromSeconds(1));
rtt_stats_.UpdateRtt(kMinRttMs, QuicTime::Delta::Zero(), QuicTime::Zero());
- received_manager_.set_save_timestamps(true);
+ received_manager_.set_save_timestamps(true, false);
}
void RecordPacketReceipt(uint64_t packet_number) {
@@ -189,6 +189,21 @@
EXPECT_EQ(2u, received_manager_.ack_frame().received_packet_times.size());
}
+TEST_F(QuicReceivedPacketManagerTest, IgnoreOutOfOrderPackets) {
+ received_manager_.set_save_timestamps(true, true);
+ EXPECT_FALSE(received_manager_.ack_frame_updated());
+ RecordPacketReceipt(1, QuicTime::Zero());
+ EXPECT_TRUE(received_manager_.ack_frame_updated());
+ EXPECT_EQ(1u, received_manager_.ack_frame().received_packet_times.size());
+ RecordPacketReceipt(4,
+ QuicTime::Zero() + QuicTime::Delta::FromMilliseconds(1));
+ EXPECT_EQ(2u, received_manager_.ack_frame().received_packet_times.size());
+
+ RecordPacketReceipt(3,
+ QuicTime::Zero() + QuicTime::Delta::FromMilliseconds(3));
+ EXPECT_EQ(2u, received_manager_.ack_frame().received_packet_times.size());
+}
+
TEST_F(QuicReceivedPacketManagerTest, HasMissingPackets) {
EXPECT_QUIC_BUG(received_manager_.PeerFirstSendingPacketNumber(),
"No packets have been received yet");
diff --git a/quic/core/uber_received_packet_manager.cc b/quic/core/uber_received_packet_manager.cc
index 8f20b33..c25329c 100644
--- a/quic/core/uber_received_packet_manager.cc
+++ b/quic/core/uber_received_packet_manager.cc
@@ -228,7 +228,8 @@
void UberReceivedPacketManager::set_save_timestamps(bool save_timestamps) {
for (auto& received_packet_manager : received_packet_managers_) {
- received_packet_manager.set_save_timestamps(save_timestamps);
+ received_packet_manager.set_save_timestamps(
+ save_timestamps, supports_multiple_packet_number_spaces_);
}
}