BUG/MINOR: quic: fix BBB max bandwidth oscillation issue.

Remove the code in relation with BBR.ack_phase as per this commit:
https://github.com/ietf-wg-ccwg/draft-ietf-ccwg-bbr/pull/5/commits/ee98c12ad6f0e93153656218a7df1b1ef92618d7

I do now kwow at this time why such a request was pushed on GH for the BBR v3 draft
pseudo-code. That said, the use of such an ack phase seemed confusing, adding much
more information about a BBR flow state than needed. Indeed, the ack phase
state is modified several times in the BBR draft pseudo-code but only used to
decide if the max bandwidth filter virtual clock had to be incremented by
BBRAdvanceMaxBwFilter().

In addition to this, when discussing about haproxy BBR implementation with
Neal Cardwell on the BBR development google group about an oscillation issue
of the max bandwidth (BBR.max_bw), I concluded that this was due to the fact
that its filter virutal clock was too often update, due to the ack phase wich
was stalled in BBR_ACK_PHASE_ACKS_PROBE_STOPPING state for too long. This is
where Neal asked me to test the aforementioned commit. This definitively
makes the max bandwidth (BBR.max_bw) oscillation issue disappear.

Another solution would have been to add a new ack phase enum afer
BBR_ACK_PHASE_ACKS_PROBE_STOPPING. BBR_ACK_PHASE_ACKS_PROBE_STOPPED
would have been a good candidate.

Remove the code in relation with BBR.ack_phase.

Must be backported to 3.1.

diff --git a/src/quic_cc_bbr.c b/src/quic_cc_bbr.c
index 9a3b0a83ea67dce46af92661d88680a58fe90bc3..a164c1f3f2befd9d4c836835c2ecb11dc72a76e9 100644 (file)
--- a/src/quic_cc_bbr.c
+++ b/src/quic_cc_bbr.c
@@ -177,13 +177,6 @@ enum bbr_state {
        BBR_ST_PROBE_RTT,
 };
 
-enum bbr_ack_phase {
-       BBR_ACK_PHASE_ACKS_PROBE_STARTING,
-       BBR_ACK_PHASE_ACKS_PROBE_STOPPING,
-       BBR_ACK_PHASE_ACKS_PROBE_FEEDBACK,
-       BBR_ACK_PHASE_ACKS_REFILLING,
-};
-
 struct bbr {
        /* Delivery rate sampling information. */
        struct quic_cc_drs drs;
@@ -218,7 +211,6 @@ struct bbr {
        uint64_t offload_budget;
        uint64_t probe_up_cnt;
        uint32_t cycle_stamp;
-       enum bbr_ack_phase ack_phase;
        unsigned int bw_probe_wait;
        int bw_probe_samples;
        int bw_probe_up_rounds;
@@ -599,7 +591,6 @@ static int bbr_init(struct quic_cc *cc)
        bbr->offload_budget = 0;
        bbr->probe_up_cnt = UINT64_MAX;
        bbr->cycle_stamp = TICK_ETERNITY;
-       bbr->ack_phase = 0;
        bbr->bw_probe_wait = 0;
        bbr->bw_probe_samples = 0;
        bbr->bw_probe_up_rounds = 0;
@@ -736,6 +727,11 @@ static void bbr_raise_inflight_hi_slope(struct bbr *bbr, struct quic_cc_path *p)
        bbr->probe_up_cnt = MAX(p->cwnd / growth_this_round, 1) * p->mtu;
 }
 
+static inline void bbr_advance_max_bw_filter(struct bbr *bbr)
+{
+       bbr->cycle_count++;
+}
+
 /* 4.3.3. ProbeBW
  *
  * Long-lived BBR flows tend to spend the vast majority of their time in the
@@ -767,11 +763,12 @@ static void bbr_start_probe_bw_down(struct bbr *bbr)
        bbr->probe_up_cnt = UINT64_MAX;
        bbr_pick_probe_wait(bbr);
        bbr->cycle_stamp = now_ms;
-       bbr->ack_phase = BBR_ACK_PHASE_ACKS_PROBE_STOPPING;
        bbr_start_round(bbr);
        bbr->state = BBR_ST_PROBE_BW_DOWN;
        bbr->pacing_gain = 90;
        bbr->cwnd_gain = BBR_DEFAULT_CWND_GAIN_MULT;
+       if (!bbr->drs.rs.is_app_limited)
+               bbr_advance_max_bw_filter(bbr);
 }
 
 /*  4.3.3.2. ProbeBW_CRUISE
@@ -800,7 +797,6 @@ static void bbr_start_probe_bw_refill(struct bbr *bbr)
        bbr_reset_lower_bounds(bbr);
        bbr->bw_probe_up_rounds = 0;
        bbr->bw_probe_up_acks = 0;
-       bbr->ack_phase = BBR_ACK_PHASE_ACKS_REFILLING;
        bbr_start_round(bbr);
        bbr->state = BBR_ST_PROBE_BW_REFILL;
        bbr->pacing_gain = 100;
@@ -809,7 +805,6 @@ static void bbr_start_probe_bw_refill(struct bbr *bbr)
 
 static void bbr_start_probe_bw_up(struct bbr *bbr, struct quic_cc_path *p)
 {
-       bbr->ack_phase = BBR_ACK_PHASE_ACKS_PROBE_STARTING;
        bbr_start_round(bbr);
        bbr_reset_full_bw(bbr);
        bbr->full_bw = p->delivery_rate;
@@ -843,11 +838,6 @@ static void bbr_exit_probe_rtt(struct bbr *bbr)
        }
 }
 
-static void bbr_advance_max_bw_filter(struct bbr *bbr)
-{
-       bbr->cycle_count++;
-}
-
 static uint64_t bbr_target_inflight(struct bbr *bbr, struct quic_cc_path *p)
 {
        uint64_t bdp = bbr_inflight(bbr, p, bbr->bw, bbr->cwnd_gain);
@@ -907,16 +897,6 @@ static void bbr_probe_inflight_hi_upward(struct bbr *bbr, struct quic_cc_path *p
 static void bbr_adapt_upper_bounds(struct bbr *bbr, struct quic_cc_path *p,
                                    uint32_t acked)
 {
-       if (bbr->ack_phase == BBR_ACK_PHASE_ACKS_PROBE_STARTING && bbr->round_start)
-               /* starting to get bw probing samples */
-               bbr->ack_phase = BBR_ACK_PHASE_ACKS_PROBE_FEEDBACK;
-
-       if (bbr->ack_phase == BBR_ACK_PHASE_ACKS_PROBE_STOPPING && bbr->round_start) {
-               /* end of samples from bw probing phase */
-               if (bbr_is_probing_bw(bbr) && !bbr->drs.rs.is_app_limited)
-                       bbr_advance_max_bw_filter(bbr);
-       }
-
        if (bbr_is_inflight_too_high(bbr, p))
                return;
 
@@ -1241,7 +1221,6 @@ static inline void bbr_check_probe_rtt(struct bbr *bbr, struct quic_cc_path *p)
                bbr_enter_probe_rtt(bbr);
                bbr_save_cwnd(bbr, p);
                bbr->probe_rtt_done_stamp = TICK_ETERNITY;
-               bbr->ack_phase = BBR_ACK_PHASE_ACKS_PROBE_STOPPING;
                bbr_start_round(bbr);
        }