gfe-relnote: In QUIC, skip packet threshold loss detection if largest acked is a runt. Protected by gfe2_reloadable_flag_quic_skip_packet_threshold_loss_detection_with_runt.
PiperOrigin-RevId: 299908950
Change-Id: Ie2d6b5201447cf1352a14faabddc7a40e694c805
diff --git a/quic/core/congestion_control/general_loss_algorithm.cc b/quic/core/congestion_control/general_loss_algorithm.cc
index 5ef2237..5136648 100644
--- a/quic/core/congestion_control/general_loss_algorithm.cc
+++ b/quic/core/congestion_control/general_loss_algorithm.cc
@@ -18,6 +18,7 @@
reordering_threshold_(kNumberOfNacksBeforeRetransmission),
use_adaptive_reordering_threshold_(true),
use_adaptive_time_threshold_(false),
+ use_packet_threshold_for_runt_packets_(true),
least_in_flight_(1),
packet_number_space_(NUM_PACKET_NUMBER_SPACES) {}
@@ -80,7 +81,17 @@
continue;
}
// Packet threshold loss detection.
- if (largest_newly_acked - packet_number >= reordering_threshold_) {
+ // Skip packet threshold loss detection if largest_newly_acked is a runt.
+ const bool skip_packet_threshold_detection =
+ !use_packet_threshold_for_runt_packets_ &&
+ it->bytes_sent >
+ unacked_packets.GetTransmissionInfo(largest_newly_acked).bytes_sent;
+ if (skip_packet_threshold_detection) {
+ QUIC_RELOADABLE_FLAG_COUNT_N(
+ quic_skip_packet_threshold_loss_detection_with_runt, 2, 2);
+ }
+ if (!skip_packet_threshold_detection &&
+ largest_newly_acked - packet_number >= reordering_threshold_) {
packets_lost->push_back(LostPacket(packet_number, it->bytes_sent));
continue;
}
diff --git a/quic/core/congestion_control/general_loss_algorithm.h b/quic/core/congestion_control/general_loss_algorithm.h
index 9f7175f..57c483e 100644
--- a/quic/core/congestion_control/general_loss_algorithm.h
+++ b/quic/core/congestion_control/general_loss_algorithm.h
@@ -88,6 +88,14 @@
void enable_adaptive_time_threshold() { use_adaptive_time_threshold_ = true; }
+ bool use_packet_threshold_for_runt_packets() const {
+ return use_packet_threshold_for_runt_packets_;
+ }
+
+ void disable_packet_threshold_for_runt_packets() {
+ use_packet_threshold_for_runt_packets_ = false;
+ }
+
private:
QuicTime loss_detection_timeout_;
// Fraction of a max(SRTT, latest_rtt) to permit reordering before declaring
@@ -100,6 +108,8 @@
bool use_adaptive_reordering_threshold_;
// If true, uses adaptive time threshold for time based loss detection.
bool use_adaptive_time_threshold_;
+ // If true, uses packet threshold when largest acked is a runt packet.
+ bool use_packet_threshold_for_runt_packets_;
// The least in flight packet. Loss detection should start from this. Please
// note, least_in_flight_ could be largest packet ever sent + 1.
QuicPacketNumber least_in_flight_;
diff --git a/quic/core/congestion_control/general_loss_algorithm_test.cc b/quic/core/congestion_control/general_loss_algorithm_test.cc
index f3fb1f1..7bef933 100644
--- a/quic/core/congestion_control/general_loss_algorithm_test.cc
+++ b/quic/core/congestion_control/general_loss_algorithm_test.cc
@@ -32,19 +32,24 @@
~GeneralLossAlgorithmTest() override {}
- void SendDataPacket(uint64_t packet_number) {
+ void SendDataPacket(uint64_t packet_number,
+ QuicPacketLength encrypted_length) {
QuicStreamFrame frame;
frame.stream_id = QuicUtils::GetFirstBidirectionalStreamId(
CurrentSupportedVersions()[0].transport_version,
Perspective::IS_CLIENT);
SerializedPacket packet(QuicPacketNumber(packet_number),
- PACKET_1BYTE_PACKET_NUMBER, nullptr, kDefaultLength,
- false, false);
+ PACKET_1BYTE_PACKET_NUMBER, nullptr,
+ encrypted_length, false, false);
packet.retransmittable_frames.push_back(QuicFrame(frame));
unacked_packets_.AddSentPacket(&packet, NOT_RETRANSMISSION, clock_.Now(),
true);
}
+ void SendDataPacket(uint64_t packet_number) {
+ SendDataPacket(packet_number, kDefaultLength);
+ }
+
void SendAckPacket(uint64_t packet_number) {
SerializedPacket packet(QuicPacketNumber(packet_number),
PACKET_1BYTE_PACKET_NUMBER, nullptr, kDefaultLength,
@@ -431,6 +436,30 @@
EXPECT_EQ(QuicTime::Zero(), loss_algorithm_.GetLossTimeout());
}
+TEST_F(GeneralLossAlgorithmTest, NoPacketThresholdForRuntPackets) {
+ loss_algorithm_.disable_packet_threshold_for_runt_packets();
+ for (size_t i = 1; i <= 6; ++i) {
+ SendDataPacket(i);
+ }
+ // Send a small packet.
+ SendDataPacket(7, /*encrypted_length=*/kDefaultLength / 2);
+ // No packet threshold for runt packet.
+ AckedPacketVector packets_acked;
+ unacked_packets_.RemoveFromInFlight(QuicPacketNumber(7));
+ packets_acked.push_back(AckedPacket(
+ QuicPacketNumber(7), kMaxOutgoingPacketSize, QuicTime::Zero()));
+ // Verify no packet is detected lost because packet 7 is a runt.
+ VerifyLosses(7, packets_acked, std::vector<uint64_t>{});
+ EXPECT_EQ(clock_.Now() + rtt_stats_.smoothed_rtt() +
+ (rtt_stats_.smoothed_rtt() >> 2),
+ loss_algorithm_.GetLossTimeout());
+ clock_.AdvanceTime(rtt_stats_.smoothed_rtt() +
+ (rtt_stats_.smoothed_rtt() >> 2));
+ // Verify packets are declared lost because time threshold has passed.
+ VerifyLosses(7, packets_acked, {1, 2, 3, 4, 5, 6});
+ EXPECT_EQ(QuicTime::Zero(), loss_algorithm_.GetLossTimeout());
+}
+
} // namespace
} // namespace test
} // namespace quic
diff --git a/quic/core/congestion_control/uber_loss_algorithm.cc b/quic/core/congestion_control/uber_loss_algorithm.cc
index 30d213f..abea529 100644
--- a/quic/core/congestion_control/uber_loss_algorithm.cc
+++ b/quic/core/congestion_control/uber_loss_algorithm.cc
@@ -106,4 +106,10 @@
}
}
+void UberLossAlgorithm::DisablePacketThresholdForRuntPackets() {
+ for (int8_t i = INITIAL_DATA; i < NUM_PACKET_NUMBER_SPACES; ++i) {
+ general_loss_algorithms_[i].disable_packet_threshold_for_runt_packets();
+ }
+}
+
} // namespace quic
diff --git a/quic/core/congestion_control/uber_loss_algorithm.h b/quic/core/congestion_control/uber_loss_algorithm.h
index 397dda7..b133f09 100644
--- a/quic/core/congestion_control/uber_loss_algorithm.h
+++ b/quic/core/congestion_control/uber_loss_algorithm.h
@@ -85,6 +85,9 @@
// Enable adaptive time threshold of all packet number spaces.
void EnableAdaptiveTimeThreshold();
+ // Disable packet threshold loss detection for *runt* packets.
+ void DisablePacketThresholdForRuntPackets();
+
private:
friend class test::QuicSentPacketManagerPeer;
diff --git a/quic/core/crypto/crypto_protocol.h b/quic/core/crypto/crypto_protocol.h
index 9532b99..7f8d959 100644
--- a/quic/core/crypto/crypto_protocol.h
+++ b/quic/core/crypto/crypto_protocol.h
@@ -182,6 +182,8 @@
// threshold (default 1/4 RTT)
// and adaptive packet
// threshold
+const QuicTag kRUNT = TAG('R', 'U', 'N', 'T'); // No packet threshold loss
+ // detection for "runt" packet.
// TODO(fayang): Remove this connection option when QUIC_VERSION_35, is removed
// Since MAX_HEADER_LIST_SIZE settings frame is supported instead.
const QuicTag kSMHL = TAG('S', 'M', 'H', 'L'); // Support MAX_HEADER_LIST_SIZE
diff --git a/quic/core/quic_sent_packet_manager.cc b/quic/core/quic_sent_packet_manager.cc
index eb59a8e..bad8aa6 100644
--- a/quic/core/quic_sent_packet_manager.cc
+++ b/quic/core/quic_sent_packet_manager.cc
@@ -292,6 +292,13 @@
uber_loss_algorithm_.EnableAdaptiveReorderingThreshold();
uber_loss_algorithm_.EnableAdaptiveTimeThreshold();
}
+ if (GetQuicReloadableFlag(
+ quic_skip_packet_threshold_loss_detection_with_runt) &&
+ config.HasClientRequestedIndependentOption(kRUNT, perspective)) {
+ QUIC_RELOADABLE_FLAG_COUNT_N(
+ quic_skip_packet_threshold_loss_detection_with_runt, 1, 2);
+ uber_loss_algorithm_.DisablePacketThresholdForRuntPackets();
+ }
if (config.HasClientSentConnectionOption(kCONH, perspective)) {
conservative_handshake_retransmits_ = true;
}
diff --git a/quic/core/quic_sent_packet_manager_test.cc b/quic/core/quic_sent_packet_manager_test.cc
index 2a39f9b..b47e528 100644
--- a/quic/core/quic_sent_packet_manager_test.cc
+++ b/quic/core/quic_sent_packet_manager_test.cc
@@ -3745,6 +3745,24 @@
manager_.NeuterUnencryptedPackets();
}
+TEST_F(QuicSentPacketManagerTest, NoPacketThresholdDetectionForRuntPackets) {
+ EXPECT_TRUE(
+ QuicSentPacketManagerPeer::UsePacketThresholdForRuntPackets(&manager_));
+
+ SetQuicReloadableFlag(quic_skip_packet_threshold_loss_detection_with_runt,
+ true);
+ QuicConfig config;
+ QuicTagVector options;
+ options.push_back(kRUNT);
+ QuicConfigPeer::SetReceivedConnectionOptions(&config, options);
+ EXPECT_CALL(*send_algorithm_, SetFromConfig(_, _));
+ EXPECT_CALL(*network_change_visitor_, OnCongestionChange());
+ manager_.SetFromConfig(config);
+
+ EXPECT_FALSE(
+ QuicSentPacketManagerPeer::UsePacketThresholdForRuntPackets(&manager_));
+}
+
} // namespace
} // namespace test
} // namespace quic
diff --git a/quic/test_tools/quic_sent_packet_manager_peer.cc b/quic/test_tools/quic_sent_packet_manager_peer.cc
index cd8297a..9666145 100644
--- a/quic/test_tools/quic_sent_packet_manager_peer.cc
+++ b/quic/test_tools/quic_sent_packet_manager_peer.cc
@@ -221,5 +221,12 @@
.use_adaptive_time_threshold();
}
+// static
+bool QuicSentPacketManagerPeer::UsePacketThresholdForRuntPackets(
+ QuicSentPacketManager* sent_packet_manager) {
+ return sent_packet_manager->uber_loss_algorithm_.general_loss_algorithms_[0]
+ .use_packet_threshold_for_runt_packets();
+}
+
} // namespace test
} // namespace quic
diff --git a/quic/test_tools/quic_sent_packet_manager_peer.h b/quic/test_tools/quic_sent_packet_manager_peer.h
index 3927189..aaa615b 100644
--- a/quic/test_tools/quic_sent_packet_manager_peer.h
+++ b/quic/test_tools/quic_sent_packet_manager_peer.h
@@ -101,6 +101,9 @@
static bool AdaptiveTimeThresholdEnabled(
QuicSentPacketManager* sent_packet_manager);
+
+ static bool UsePacketThresholdForRuntPackets(
+ QuicSentPacketManager* sent_packet_manager);
};
} // namespace test
