Fix the bug where loss parameters are chosen but not applied in loss algorithms. protected by existing --gfe2_reloadable_flag_quic_enable_loss_detection_experiment_at_gfe.
PiperOrigin-RevId: 313824964
Change-Id: If1a12d46a04ae54383347ac5b0494f7e8f2a3216
diff --git a/quic/core/congestion_control/uber_loss_algorithm.cc b/quic/core/congestion_control/uber_loss_algorithm.cc
index 6fd23d6..28463cb 100644
--- a/quic/core/congestion_control/uber_loss_algorithm.cc
+++ b/quic/core/congestion_control/uber_loss_algorithm.cc
@@ -103,6 +103,21 @@
}
tuner_started_ = tuner_->Start(&tuned_parameters_);
+ if (!tuner_started_) {
+ return;
+ }
+
+ if (tuned_parameters_.reordering_shift.has_value() &&
+ tuned_parameters_.reordering_threshold.has_value()) {
+ QUIC_DLOG(INFO) << "Setting reordering shift to "
+ << *tuned_parameters_.reordering_shift
+ << ", and reordering threshold to "
+ << *tuned_parameters_.reordering_threshold;
+ SetReorderingShift(*tuned_parameters_.reordering_shift);
+ SetReorderingThreshold(*tuned_parameters_.reordering_threshold);
+ } else {
+ QUIC_BUG << "Tuner started but some parameters are missing";
+ }
}
void UberLossAlgorithm::OnConfigNegotiated() {}
@@ -153,6 +168,10 @@
return general_loss_algorithms_[APPLICATION_DATA].reordering_threshold();
}
+int UberLossAlgorithm::GetPacketReorderingShift() const {
+ return general_loss_algorithms_[APPLICATION_DATA].reordering_shift();
+}
+
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();
diff --git a/quic/core/congestion_control/uber_loss_algorithm.h b/quic/core/congestion_control/uber_loss_algorithm.h
index 86b6525..7dacb66 100644
--- a/quic/core/congestion_control/uber_loss_algorithm.h
+++ b/quic/core/congestion_control/uber_loss_algorithm.h
@@ -93,12 +93,25 @@
// Always 3 when adaptive reordering is not enabled.
QuicPacketCount GetPacketReorderingThreshold() const;
+ // Get the packet reordering shift from the APPLICATION_DATA PN space.
+ int GetPacketReorderingShift() const;
+
// Disable packet threshold loss detection for *runt* packets.
void DisablePacketThresholdForRuntPackets();
// Called to reset loss detection of |space|.
void ResetLossDetection(PacketNumberSpace space);
+ bool use_adaptive_reordering_threshold() const {
+ return general_loss_algorithms_[APPLICATION_DATA]
+ .use_adaptive_reordering_threshold();
+ }
+
+ bool use_adaptive_time_threshold() const {
+ return general_loss_algorithms_[APPLICATION_DATA]
+ .use_adaptive_time_threshold();
+ }
+
private:
friend class test::QuicSentPacketManagerPeer;
diff --git a/quic/core/congestion_control/uber_loss_algorithm_test.cc b/quic/core/congestion_control/uber_loss_algorithm_test.cc
index 6fd949f..318e730 100644
--- a/quic/core/congestion_control/uber_loss_algorithm_test.cc
+++ b/quic/core/congestion_control/uber_loss_algorithm_test.cc
@@ -4,9 +4,12 @@
#include "net/third_party/quiche/src/quic/core/congestion_control/uber_loss_algorithm.h"
+#include <memory>
#include <utility>
#include "net/third_party/quiche/src/quic/core/congestion_control/rtt_stats.h"
+#include "net/third_party/quiche/src/quic/core/crypto/crypto_protocol.h"
+#include "net/third_party/quiche/src/quic/core/quic_types.h"
#include "net/third_party/quiche/src/quic/core/quic_utils.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_test.h"
#include "net/third_party/quiche/src/quic/test_tools/mock_clock.h"
@@ -204,6 +207,137 @@
VerifyLosses(6, packets_acked_, std::vector<uint64_t>{2});
}
+class TestLossTuner : public LossDetectionTunerInterface {
+ public:
+ TestLossTuner(bool forced_start_result,
+ LossDetectionParameters forced_parameters)
+ : forced_start_result_(forced_start_result),
+ forced_parameters_(std::move(forced_parameters)) {}
+
+ ~TestLossTuner() override = default;
+
+ bool Start(LossDetectionParameters* params) override {
+ start_called_ = true;
+ *params = forced_parameters_;
+ return forced_start_result_;
+ }
+
+ void Finish(const LossDetectionParameters& /*params*/) override {}
+
+ bool start_called() const { return start_called_; }
+
+ private:
+ bool forced_start_result_;
+ LossDetectionParameters forced_parameters_;
+ bool start_called_ = false;
+};
+
+// Verify the parameters are changed if first call SetFromConfig(), then call
+// OnMinRttAvailable().
+TEST_F(UberLossAlgorithmTest, LossDetectionTuning_SetFromConfigFirst) {
+ const int old_reordering_shift = loss_algorithm_.GetPacketReorderingShift();
+ const QuicPacketCount old_reordering_threshold =
+ loss_algorithm_.GetPacketReorderingThreshold();
+
+ // Not owned.
+ TestLossTuner* test_tuner = new TestLossTuner(
+ /*forced_start_result=*/true,
+ LossDetectionParameters{
+ /*reordering_shift=*/old_reordering_shift + 1,
+ /*reordering_threshold=*/old_reordering_threshold * 2});
+ loss_algorithm_.SetLossDetectionTuner(
+ std::unique_ptr<LossDetectionTunerInterface>(test_tuner));
+
+ QuicConfig config;
+ QuicTagVector connection_options;
+ connection_options.push_back(kELDT);
+ config.SetInitialReceivedConnectionOptions(connection_options);
+ loss_algorithm_.SetFromConfig(config, Perspective::IS_SERVER);
+
+ // MinRtt was not available when SetFromConfig was called.
+ EXPECT_FALSE(test_tuner->start_called());
+ EXPECT_EQ(old_reordering_shift, loss_algorithm_.GetPacketReorderingShift());
+ EXPECT_EQ(old_reordering_threshold,
+ loss_algorithm_.GetPacketReorderingThreshold());
+
+ // Tuning should start when MinRtt becomes available.
+ loss_algorithm_.OnMinRttAvailable();
+ EXPECT_TRUE(test_tuner->start_called());
+ EXPECT_NE(old_reordering_shift, loss_algorithm_.GetPacketReorderingShift());
+ EXPECT_NE(old_reordering_threshold,
+ loss_algorithm_.GetPacketReorderingThreshold());
+}
+
+// Verify the parameters are changed if first call OnMinRttAvailable(), then
+// call SetFromConfig().
+TEST_F(UberLossAlgorithmTest, LossDetectionTuning_OnMinRttAvailableFirst) {
+ const int old_reordering_shift = loss_algorithm_.GetPacketReorderingShift();
+ const QuicPacketCount old_reordering_threshold =
+ loss_algorithm_.GetPacketReorderingThreshold();
+
+ // Not owned.
+ TestLossTuner* test_tuner = new TestLossTuner(
+ /*forced_start_result=*/true,
+ LossDetectionParameters{
+ /*reordering_shift=*/old_reordering_shift + 1,
+ /*reordering_threshold=*/old_reordering_threshold * 2});
+ loss_algorithm_.SetLossDetectionTuner(
+ std::unique_ptr<LossDetectionTunerInterface>(test_tuner));
+
+ loss_algorithm_.OnMinRttAvailable();
+ EXPECT_FALSE(test_tuner->start_called());
+ EXPECT_EQ(old_reordering_shift, loss_algorithm_.GetPacketReorderingShift());
+ EXPECT_EQ(old_reordering_threshold,
+ loss_algorithm_.GetPacketReorderingThreshold());
+
+ QuicConfig config;
+ QuicTagVector connection_options;
+ connection_options.push_back(kELDT);
+ config.SetInitialReceivedConnectionOptions(connection_options);
+ // Should start tuning since MinRtt is available.
+ loss_algorithm_.SetFromConfig(config, Perspective::IS_SERVER);
+
+ EXPECT_TRUE(test_tuner->start_called());
+ EXPECT_NE(old_reordering_shift, loss_algorithm_.GetPacketReorderingShift());
+ EXPECT_NE(old_reordering_threshold,
+ loss_algorithm_.GetPacketReorderingThreshold());
+}
+
+// Verify the parameters are not changed if Tuner.Start() returns false.
+TEST_F(UberLossAlgorithmTest, LossDetectionTuning_StartFailed) {
+ const int old_reordering_shift = loss_algorithm_.GetPacketReorderingShift();
+ const QuicPacketCount old_reordering_threshold =
+ loss_algorithm_.GetPacketReorderingThreshold();
+
+ // Not owned.
+ TestLossTuner* test_tuner = new TestLossTuner(
+ /*forced_start_result=*/false,
+ LossDetectionParameters{
+ /*reordering_shift=*/old_reordering_shift + 1,
+ /*reordering_threshold=*/old_reordering_threshold * 2});
+ loss_algorithm_.SetLossDetectionTuner(
+ std::unique_ptr<LossDetectionTunerInterface>(test_tuner));
+
+ QuicConfig config;
+ QuicTagVector connection_options;
+ connection_options.push_back(kELDT);
+ config.SetInitialReceivedConnectionOptions(connection_options);
+ loss_algorithm_.SetFromConfig(config, Perspective::IS_SERVER);
+
+ // MinRtt was not available when SetFromConfig was called.
+ EXPECT_FALSE(test_tuner->start_called());
+ EXPECT_EQ(old_reordering_shift, loss_algorithm_.GetPacketReorderingShift());
+ EXPECT_EQ(old_reordering_threshold,
+ loss_algorithm_.GetPacketReorderingThreshold());
+
+ // Parameters should not change since test_tuner->Start() returns false.
+ loss_algorithm_.OnMinRttAvailable();
+ EXPECT_TRUE(test_tuner->start_called());
+ EXPECT_EQ(old_reordering_shift, loss_algorithm_.GetPacketReorderingShift());
+ EXPECT_EQ(old_reordering_threshold,
+ loss_algorithm_.GetPacketReorderingThreshold());
+}
+
} // namespace
} // namespace test
} // namespace quic
