Skip to content

Commit

Permalink
Enable packet conservation only when conservativeRecovery is set; dis…
Browse files Browse the repository at this point in the history
…able cwnd decrease on loss when ignoreLoss is set

Summary:
1. Noticed that bbr2 was not discovering the available BtlBw because it exited startup early due to loss. Disabling packet conservation seems to help, in sims. bbr1 has conservativeRecovery flag set to false by default. In the same spirit, we should enable/disable packet conservation in bbr2 based on that flag.
2. The CWND is decreased by the loss amount. This should be disabled when ignoreLoss is true.

Reviewed By: jbeshay, mjoras

Differential Revision: D55612685

fbshipit-source-id: 0c811de78e4a6f273f340631d29ff9dcd27d898e
  • Loading branch information
ritengupta authored and facebook-github-bot committed Apr 5, 2024
1 parent 1c2c5fa commit de0349d
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 3 deletions.
8 changes: 6 additions & 2 deletions quic/congestion_control/Bbr2.cpp
Expand Up @@ -119,14 +119,18 @@ void Bbr2CongestionController::onPacketAckOrLoss(
<< ")";
};

if (lossEvent && lossEvent->lostPackets > 0) {
if (lossEvent && lossEvent->lostPackets > 0 &&
conn_.transportSettings.ccaConfig.conservativeRecovery) {
// The pseudo code in BBRHandleLostPacket is included in
// updateProbeBwCyclePhase. No need to repeat it here.

// We don't expose the loss type here, so always use fast recovery for
// non-persistent congestion
saveCwnd();
inPacketConservation_ = true;
// Mark the connection as app-limited so bw samples during recovery are not
// taken into account.
setAppLimited();
packetConservationStartTime_ = Clock::now();
if (lossEvent->persistentCongestion) {
cwndBytes_ = kMinCwndInMssForBbr * conn_.udpSendPacketLen;
Expand Down Expand Up @@ -280,7 +284,7 @@ void Bbr2CongestionController::setCwnd(
auto inflightMax = addQuantizationBudget(
getBDPWithGain(cwndGain_) + maxExtraAckedFilter_.GetBest());
// BBRModulateCwndForRecovery()
if (lostBytes > 0) {
if (lostBytes > 0 && !conn_.transportSettings.ccaConfig.ignoreLoss) {
cwndBytes_ = std::max(
cwndBytes_ - std::min(lostBytes, cwndBytes_),
kMinCwndInMssForBbr * conn_.udpSendPacketLen);
Expand Down
2 changes: 1 addition & 1 deletion quic/state/TransportSettings.h
Expand Up @@ -16,7 +16,7 @@
namespace quic {

struct CongestionControlConfig {
// Used by: BBR1
// Used by: BBR1, BBR2
bool conservativeRecovery{false};

// Used by: BBR1
Expand Down

0 comments on commit de0349d

Please sign in to comment.