From 01fcbd6c083c9154248b110caba24cdafae03b2d Mon Sep 17 00:00:00 2001 From: Frederic Lecaille Date: Thu, 21 Nov 2024 15:39:04 +0100 Subject: [PATCH] BUG/MINOR: quic: Missing application limitations tracking for BBR The ->app_limited member of the delivery rate struct (quic_cc_drs) aim is to store the index of the last transmitted byte marked as application-limited so that to track the application-limited phases. During these phases, BBR must ignore delivery rate samples to properly estimate the delivery rate. Without such a patch, the Startup phase could be exited very quickly with a very low estimated bottleneck bandwidth. This had a very bad impact on little objects with download times smaller than the expected Startup phase duration. For such objects, with enough bandwith, BBR should stay in the Startup state. No need to be backported, as BBR is implemented in the current developement version. --- include/haproxy/quic_cc-t.h | 1 + src/quic_cc_bbr.c | 18 ++++++++++++++++++ src/quic_tx.c | 5 +++++ 3 files changed, 24 insertions(+) diff --git a/include/haproxy/quic_cc-t.h b/include/haproxy/quic_cc-t.h index 40ff88909..c49707533 100644 --- a/include/haproxy/quic_cc-t.h +++ b/include/haproxy/quic_cc-t.h @@ -152,6 +152,7 @@ struct quic_cc_algo { void (*on_pkt_lost)(struct quic_cc *cc, struct quic_tx_packet *pkt, uint32_t lost_bytes); void (*congestion_event)(struct quic_cc *cc, uint32_t ts); + void (*check_app_limited)(const struct quic_cc *cc, int sent); }; #endif /* USE_QUIC */ diff --git a/src/quic_cc_bbr.c b/src/quic_cc_bbr.c index cf9580899..675de2238 100644 --- a/src/quic_cc_bbr.c +++ b/src/quic_cc_bbr.c @@ -1481,6 +1481,23 @@ uint bbr_pacing_burst(const struct quic_cc *cc) return p->send_quantum / p->mtu; } +/* Update the delivery rate sampling state about the application limitation. */ +static void bbr_check_app_limited(const struct quic_cc *cc, int sent) +{ + struct bbr *bbr = quic_cc_priv(cc); + struct quic_cc_drs *drs = &bbr->drs; + struct quic_cc_path *p = container_of(cc, struct quic_cc_path, cc); + + if (p->in_flight >= p->cwnd) { + drs->is_cwnd_limited = 1; + } + else if (!sent) { + drs->app_limited = drs->delivered + p->in_flight; + if (!drs->app_limited) + drs->app_limited = p->mtu; + } +} + static inline const char *bbr_state_str(struct bbr *bbr) { switch (bbr->state) { @@ -1525,6 +1542,7 @@ struct quic_cc_algo quic_cc_algo_bbr = { .on_ack_rcvd = bbr_update_on_ack, .congestion_event = bbr_congestion_event, .on_pkt_lost = bbr_update_on_loss, + .check_app_limited = bbr_check_app_limited, .state_cli = bbr_state_cli, }; diff --git a/src/quic_tx.c b/src/quic_tx.c index e9dcfd545..536350eec 100644 --- a/src/quic_tx.c +++ b/src/quic_tx.c @@ -18,6 +18,7 @@ #include #include +#include #include #include #include @@ -506,6 +507,10 @@ enum quic_tx_err qc_send_mux(struct quic_conn *qc, struct list *frms, TRACE_STATE("preparing data (from MUX)", QUIC_EV_CONN_TXPKT, qc); qel_register_send(&send_list, qc->ael, frms); sent = qc_send(qc, 0, &send_list, max_dgram); + + if (pacer && qc->path->cc.algo->check_app_limited) + qc->path->cc.algo->check_app_limited(&qc->path->cc, sent); + if (sent <= 0) { ret = QUIC_TX_ERR_FATAL; }