From c527fd1d035065a27918e3a9e8a6b57e5f2afbed Mon Sep 17 00:00:00 2001 From: Joseph Beshay Date: Thu, 23 Jan 2025 17:45:56 -0800 Subject: [PATCH] Add a new option to control bbr2 exiting startup on loss Summary: As title. Reviewed By: ritengupta, mjoras Differential Revision: D68546411 fbshipit-source-id: ae45727fb8443a98e4ac69778d5d85e2512f7a7b --- quic/congestion_control/Bbr2.cpp | 2 +- quic/state/TransportSettings.h | 4 ++++ quic/state/TransportSettingsFunctions.cpp | 3 ++- quic/state/test/TransportSettingsFunctionsTest.cpp | 3 +++ 4 files changed, 10 insertions(+), 2 deletions(-) diff --git a/quic/congestion_control/Bbr2.cpp b/quic/congestion_control/Bbr2.cpp index e47695fa3..dd41f41ec 100644 --- a/quic/congestion_control/Bbr2.cpp +++ b/quic/congestion_control/Bbr2.cpp @@ -466,7 +466,7 @@ void Bbr2CongestionController::checkStartupHighLoss() { the same. */ if (fullBwReached_ || !roundStart_ || isAppLimited() || - conn_.transportSettings.ccaConfig.ignoreLoss) { + !conn_.transportSettings.ccaConfig.exitStartupOnLoss) { // TODO: the appLimited condition means we could tolerate losses in startup // if we haven't found the full bandwidth. This may need to be revisited. diff --git a/quic/state/TransportSettings.h b/quic/state/TransportSettings.h index 8d226db21..cc9433221 100644 --- a/quic/state/TransportSettings.h +++ b/quic/state/TransportSettings.h @@ -76,6 +76,10 @@ struct CongestionControlConfig { // Whether BBR2 should ignore packet loss (i.e. act more like BBR1) bool ignoreLoss{false}; + // Used by: BBR2 + // Whether BBR2 should check packet loss to exit startup + bool exitStartupOnLoss{true}; + // Used by: BBR2 // Whether BBR2 should enable reno coexistence. bool enableRenoCoexistence{false}; diff --git a/quic/state/TransportSettingsFunctions.cpp b/quic/state/TransportSettingsFunctions.cpp index f969672dc..5ee1b952b 100644 --- a/quic/state/TransportSettingsFunctions.cpp +++ b/quic/state/TransportSettingsFunctions.cpp @@ -64,7 +64,7 @@ quic::CongestionControlConfig parseCongestionControlConfig( quic::CongestionControlConfig ccaConfig; // Parse known boolean fields - const std::array, 12> boolFields = { + const std::array, 13> boolFields = { {{"conservativeRecovery", ccaConfig.conservativeRecovery}, {"largeProbeRttCwnd", ccaConfig.largeProbeRttCwnd}, {"enableAckAggregationInStartup", @@ -76,6 +76,7 @@ quic::CongestionControlConfig parseCongestionControlConfig( {"leaveHeadroomForCwndLimited", ccaConfig.leaveHeadroomForCwndLimited}, {"ignoreInflightHi", ccaConfig.ignoreInflightHi}, {"ignoreLoss", ccaConfig.ignoreLoss}, + {"exitStartupOnLoss", ccaConfig.exitStartupOnLoss}, {"enableRenoCoexistence", ccaConfig.enableRenoCoexistence}, {"paceInitCwnd", ccaConfig.paceInitCwnd}}}; diff --git a/quic/state/test/TransportSettingsFunctionsTest.cpp b/quic/state/test/TransportSettingsFunctionsTest.cpp index d0239290b..401f474ac 100644 --- a/quic/state/test/TransportSettingsFunctionsTest.cpp +++ b/quic/state/test/TransportSettingsFunctionsTest.cpp @@ -66,6 +66,7 @@ TEST_F(TransportSettingsFunctionsTest, FullConfig) { "}," "\"ignoreInflightHi\": true, " "\"ignoreLoss\": true, " + "\"exitStartupOnLoss\": false, " "\"enableRenoCoexistence\": true, " "\"paceInitCwnd\": false, " "\"overrideCruisePacingGain\": 7.9, " @@ -83,6 +84,7 @@ TEST_F(TransportSettingsFunctionsTest, FullConfig) { EXPECT_EQ(config.leaveHeadroomForCwndLimited, true); EXPECT_EQ(config.ignoreInflightHi, true); EXPECT_EQ(config.ignoreLoss, true); + EXPECT_EQ(config.exitStartupOnLoss, false); EXPECT_EQ(config.enableRenoCoexistence, true); EXPECT_EQ(config.paceInitCwnd, false); EXPECT_EQ(config.overrideCruisePacingGain, 7.9f); @@ -112,6 +114,7 @@ TEST_F(TransportSettingsFunctionsTest, UnspecifiedFieldsAreDefaulted) { EXPECT_EQ(config.drainToTarget, false); EXPECT_EQ(config.ignoreInflightHi, false); EXPECT_EQ(config.ignoreLoss, false); + EXPECT_EQ(config.exitStartupOnLoss, true); EXPECT_EQ(config.enableRenoCoexistence, false); EXPECT_EQ(config.overrideCruisePacingGain, -1.0f); EXPECT_EQ(config.overrideCruiseCwndGain, -1.0f);