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

quic/core/congestion_control/uber_loss_algorithm.cc

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();

quic/core/congestion_control/uber_loss_algorithm.h

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;
 

quic/core/congestion_control/uber_loss_algorithm_test.cc

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