- Remove all experiments that tunes blackhole detection delay or path degrading delay, and - Ensure network blackhole delay is at least path degrading delay plus 2 PTOs. This is to avoid issues like https://bugs.chromium.org/p/chromium/issues/detail?id=1334029. Protected by FLAGS_quic_reloadable_flag_quic_remove_blackhole_detection_experiments. PiperOrigin-RevId: 454883575
diff --git a/quiche/quic/core/quic_connection.cc b/quiche/quic/core/quic_connection.cc index 2645c58..3c9034f 100644 --- a/quiche/quic/core/quic_connection.cc +++ b/quiche/quic/core/quic_connection.cc
@@ -563,21 +563,23 @@ if (config.HasClientSentConnectionOption(kNBHD, perspective_)) { blackhole_detection_disabled_ = true; } - if (config.HasClientSentConnectionOption(k2RTO, perspective_)) { - QUIC_CODE_COUNT(quic_2rto_blackhole_detection); - num_rtos_for_blackhole_detection_ = 2; - } - if (config.HasClientSentConnectionOption(k3RTO, perspective_)) { - QUIC_CODE_COUNT(quic_3rto_blackhole_detection); - num_rtos_for_blackhole_detection_ = 3; - } - if (config.HasClientSentConnectionOption(k4RTO, perspective_)) { - QUIC_CODE_COUNT(quic_4rto_blackhole_detection); - num_rtos_for_blackhole_detection_ = 4; - } - if (config.HasClientSentConnectionOption(k6RTO, perspective_)) { - QUIC_CODE_COUNT(quic_6rto_blackhole_detection); - num_rtos_for_blackhole_detection_ = 6; + if (!sent_packet_manager_.remove_blackhole_detection_experiments()) { + if (config.HasClientSentConnectionOption(k2RTO, perspective_)) { + QUIC_CODE_COUNT(quic_2rto_blackhole_detection); + num_rtos_for_blackhole_detection_ = 2; + } + if (config.HasClientSentConnectionOption(k3RTO, perspective_)) { + QUIC_CODE_COUNT(quic_3rto_blackhole_detection); + num_rtos_for_blackhole_detection_ = 3; + } + if (config.HasClientSentConnectionOption(k4RTO, perspective_)) { + QUIC_CODE_COUNT(quic_4rto_blackhole_detection); + num_rtos_for_blackhole_detection_ = 4; + } + if (config.HasClientSentConnectionOption(k6RTO, perspective_)) { + QUIC_CODE_COUNT(quic_6rto_blackhole_detection); + num_rtos_for_blackhole_detection_ = 6; + } } } @@ -6439,11 +6441,35 @@ return QuicTime::Zero(); } QUICHE_DCHECK_LT(0u, num_rtos_for_blackhole_detection_); + if (sent_packet_manager_.remove_blackhole_detection_experiments()) { + QUIC_RELOADABLE_FLAG_COUNT(quic_remove_blackhole_detection_experiments); + const QuicTime::Delta blackhole_delay = + sent_packet_manager_.GetNetworkBlackholeDelay( + num_rtos_for_blackhole_detection_); + if (!ShouldDetectPathDegrading()) { + return clock_->ApproximateNow() + blackhole_delay; + } + return clock_->ApproximateNow() + + CalculateNetworkBlackholeDelay( + blackhole_delay, sent_packet_manager_.GetPathDegradingDelay(), + sent_packet_manager_.GetPtoDelay()); + } return clock_->ApproximateNow() + sent_packet_manager_.GetNetworkBlackholeDelay( num_rtos_for_blackhole_detection_); } +// static +QuicTime::Delta QuicConnection::CalculateNetworkBlackholeDelay( + QuicTime::Delta blackhole_delay, QuicTime::Delta path_degrading_delay, + QuicTime::Delta pto_delay) { + const QuicTime::Delta min_delay = path_degrading_delay + pto_delay * 2; + if (blackhole_delay < min_delay) { + QUIC_CODE_COUNT(quic_extending_short_blackhole_delay); + } + return std::max(min_delay, blackhole_delay); +} + bool QuicConnection::ShouldDetectBlackhole() const { if (!connected_ || blackhole_detection_disabled_) { return false;
diff --git a/quiche/quic/core/quic_connection.h b/quiche/quic/core/quic_connection.h index aab56b7..87f9c14 100644 --- a/quiche/quic/core/quic_connection.h +++ b/quiche/quic/core/quic_connection.h
@@ -1230,6 +1230,11 @@ return quic_bug_10511_43_error_detail_; } + // Ensures the network blackhole delay is longer than path degrading delay. + static QuicTime::Delta CalculateNetworkBlackholeDelay( + QuicTime::Delta blackhole_delay, QuicTime::Delta path_degrading_delay, + QuicTime::Delta pto_delay); + protected: // Calls cancel() on all the alarms owned by this connection. void CancelAllAlarms();
diff --git a/quiche/quic/core/quic_connection_test.cc b/quiche/quic/core/quic_connection_test.cc index b3c01d5..2a89850 100644 --- a/quiche/quic/core/quic_connection_test.cc +++ b/quiche/quic/core/quic_connection_test.cc
@@ -10643,7 +10643,8 @@ } TEST_P(QuicConnectionTest, 2RtoBlackholeDetection) { - if (!GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection2)) { + if (!GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection2) || + GetQuicReloadableFlag(quic_remove_blackhole_detection_experiments)) { return; } QuicConfig config; @@ -10670,7 +10671,8 @@ } TEST_P(QuicConnectionTest, 3RtoBlackholeDetection) { - if (!GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection2)) { + if (!GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection2) || + GetQuicReloadableFlag(quic_remove_blackhole_detection_experiments)) { return; } QuicConfig config; @@ -10697,7 +10699,8 @@ } TEST_P(QuicConnectionTest, 4RtoBlackholeDetection) { - if (!GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection2)) { + if (!GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection2) || + GetQuicReloadableFlag(quic_remove_blackhole_detection_experiments)) { return; } QuicConfig config; @@ -10724,7 +10727,8 @@ } TEST_P(QuicConnectionTest, 6RtoBlackholeDetection) { - if (!GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection2)) { + if (!GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection2) || + GetQuicReloadableFlag(quic_remove_blackhole_detection_experiments)) { return; } QuicConfig config; @@ -15405,6 +15409,26 @@ EXPECT_FALSE(connection_.GetRetransmissionAlarm()->IsSet()); } +TEST_P(QuicConnectionTest, CalculateNetworkBlackholeDelay) { + if (!IsDefaultTestConfiguration()) { + return; + } + + const QuicTime::Delta kOneSec = QuicTime::Delta::FromSeconds(1); + const QuicTime::Delta kTwoSec = QuicTime::Delta::FromSeconds(2); + const QuicTime::Delta kFourSec = QuicTime::Delta::FromSeconds(4); + + // Normal case: blackhole_delay longer than path_degrading_delay + + // 2*pto_delay. + EXPECT_EQ(QuicConnection::CalculateNetworkBlackholeDelay(kFourSec, kOneSec, + kOneSec), + kFourSec); + + EXPECT_EQ(QuicConnection::CalculateNetworkBlackholeDelay(kFourSec, kOneSec, + kTwoSec), + QuicTime::Delta::FromSeconds(5)); +} + } // namespace } // namespace test } // namespace quic
diff --git a/quiche/quic/core/quic_flags_list.h b/quiche/quic/core/quic_flags_list.h index e17b850..938446d 100644 --- a/quiche/quic/core/quic_flags_list.h +++ b/quiche/quic/core/quic_flags_list.h
@@ -17,6 +17,8 @@ QUIC_FLAG(quic_restart_flag_quic_testonly_default_true, true) // If bytes in flight has dipped below 1.25*MaxBW in the last round, do not exit PROBE_UP due to excess queue buildup. QUIC_FLAG(quic_reloadable_flag_quic_bbr2_no_probe_up_exit_if_no_queue, true) +// If true, 1) remove all experiments that tunes blackhole detection delay or path degrading delay, and 2) ensure network blackhole delay is at least path degrading delay plus 2 PTOs. +QUIC_FLAG(quic_reloadable_flag_quic_remove_blackhole_detection_experiments, true) // If true, QUIC Legacy Version Encapsulation will be disabled. QUIC_FLAG(quic_restart_flag_quic_disable_legacy_version_encapsulation, false) // If true, QUIC will default enable MTU discovery at server, with a target of 1450 bytes.
diff --git a/quiche/quic/core/quic_sent_packet_manager.cc b/quiche/quic/core/quic_sent_packet_manager.cc index 1083583..0725631 100644 --- a/quiche/quic/core/quic_sent_packet_manager.cc +++ b/quiche/quic/core/quic_sent_packet_manager.cc
@@ -131,17 +131,19 @@ ignore_ack_delay_ = true; } - if (config.HasClientRequestedIndependentOption(kPDP1, perspective)) { - num_ptos_for_path_degrading_ = 1; - } - if (config.HasClientRequestedIndependentOption(kPDP2, perspective)) { - num_ptos_for_path_degrading_ = 2; - } - if (config.HasClientRequestedIndependentOption(kPDP3, perspective)) { - num_ptos_for_path_degrading_ = 3; - } - if (config.HasClientRequestedIndependentOption(kPDP5, perspective)) { - num_ptos_for_path_degrading_ = 5; + if (!remove_blackhole_detection_experiments_) { + if (config.HasClientRequestedIndependentOption(kPDP1, perspective)) { + num_ptos_for_path_degrading_ = 1; + } + if (config.HasClientRequestedIndependentOption(kPDP2, perspective)) { + num_ptos_for_path_degrading_ = 2; + } + if (config.HasClientRequestedIndependentOption(kPDP3, perspective)) { + num_ptos_for_path_degrading_ = 3; + } + if (config.HasClientRequestedIndependentOption(kPDP5, perspective)) { + num_ptos_for_path_degrading_ = 5; + } } // Configure congestion control.
diff --git a/quiche/quic/core/quic_sent_packet_manager.h b/quiche/quic/core/quic_sent_packet_manager.h index f0a6bc6..731bd3d 100644 --- a/quiche/quic/core/quic_sent_packet_manager.h +++ b/quiche/quic/core/quic_sent_packet_manager.h
@@ -26,6 +26,7 @@ #include "quiche/quic/core/quic_types.h" #include "quiche/quic/core/quic_unacked_packet_map.h" #include "quiche/quic/platform/api/quic_export.h" +#include "quiche/quic/platform/api/quic_flags.h" #include "quiche/common/quiche_circular_deque.h" namespace quic { @@ -464,6 +465,11 @@ // kMinUntrustedInitialRoundTripTimeUs if not |trusted|. void SetInitialRtt(QuicTime::Delta rtt, bool trusted); + // Latched value of --quic_remove_blackhole_detection_experiments. + bool remove_blackhole_detection_experiments() const { + return remove_blackhole_detection_experiments_; + } + private: friend class test::QuicConnectionPeer; friend class test::QuicSentPacketManagerPeer; @@ -668,6 +674,9 @@ // Whether to ignore the ack_delay in received ACKs. bool ignore_ack_delay_; + + const bool remove_blackhole_detection_experiments_ = + GetQuicReloadableFlag(quic_remove_blackhole_detection_experiments); }; } // namespace quic
diff --git a/quiche/quic/core/quic_sent_packet_manager_test.cc b/quiche/quic/core/quic_sent_packet_manager_test.cc index 8c8076b..67605e4 100644 --- a/quiche/quic/core/quic_sent_packet_manager_test.cc +++ b/quiche/quic/core/quic_sent_packet_manager_test.cc
@@ -2499,6 +2499,9 @@ } TEST_F(QuicSentPacketManagerTest, GetPathDegradingDelayUsing2PTO) { + if (GetQuicReloadableFlag(quic_remove_blackhole_detection_experiments)) { + return; + } QuicConfig client_config; QuicTagVector client_options; client_options.push_back(kPDP2); @@ -2512,6 +2515,9 @@ } TEST_F(QuicSentPacketManagerTest, GetPathDegradingDelayUsing1PTO) { + if (GetQuicReloadableFlag(quic_remove_blackhole_detection_experiments)) { + return; + } QuicConfig client_config; QuicTagVector client_options; client_options.push_back(kPDP1);