From d04e93bc2eb695f35b66233f0f5b80e2e5c2193a Mon Sep 17 00:00:00 2001 From: Amaury Denoyelle Date: Thu, 30 Jan 2025 11:58:27 +0100 Subject: [PATCH] MINOR: quic: allow BBR testing without pacing Pacing is activated per bind line via an optional boolean argument of quic-cc-algo keyword. Contrary to the default usage, pacing is automatically activated when BBR is chosen. This is because this algorithm is expected to run on top of pacing, else its behavior is undefined. Previously, pacing argument was thus ignored when BBR was selected. Change this to support explicit deactivation of pacing with it. This could be useful to test BBR without pacing when debugging some issues. This should be backported up to 3.1, after a period of observation. --- doc/configuration.txt | 10 +++++----- src/cfgparse-quic.c | 11 ++++++----- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/doc/configuration.txt b/doc/configuration.txt index da9d8b540..0ebfd3291 100644 --- a/doc/configuration.txt +++ b/doc/configuration.txt @@ -17297,9 +17297,10 @@ quic-cc-algo { cubic | newreno | bbr | nocc }[()] The purpose of pacing is to smooth emission of data to reduce network losses. In most scenario, it can significantly improve network throughput by avoiding retransmissions. Pacing support is still experimental, as such it requires - "expose-experimental-directives". The BBR congestion control algorithm - depends on the pacing support which is in this case implicitly enabled by - choosing the "bbr" algorithm. Note that haproxy's BBR implementation is still + "expose-experimental-directives". Selecting BBR congestion algorithm will + implicitly activate pacing as it relies on it to work as expected. Explicitly + disabling pacing in conjunction with BBR may cause slowdowns or high loss + rates during transfers. Also note that haproxy's BBR implementation is also considered as experimental and cannot be enabled without "expose-experimental-directives". @@ -17311,8 +17312,7 @@ quic-cc-algo { cubic | newreno | bbr | nocc }[()] 4g. By default "tune.quic.frontend.default-max-window-size" value is used. - pacing activation. By default, it is set to 0, which means pacing is not used. To activate it, specify a positive value. Burst size will be - dynamically adjusted to adapt to the network conditions. This parameter is - ignored by BBR as pacing is automatically activated for this algorithm. + dynamically adjusted to adapt to the network conditions. Example: # newreno congestion control algorithm diff --git a/src/cfgparse-quic.c b/src/cfgparse-quic.c index adbd020d2..acb7d0d69 100644 --- a/src/cfgparse-quic.c +++ b/src/cfgparse-quic.c @@ -191,11 +191,7 @@ static int bind_parse_quic_cc_algo(char **args, int cur_arg, struct proxy *px, if (pacing < 0) goto fail; - if (!(cc_algo->flags & QUIC_CC_ALGO_FL_OPT_PACING)) { - ha_warning("'%s' : pacing parameter ignored for '%s' congestion algorithm\n", - args[cur_arg], algo); - } - else if (pacing) { + if (pacing) { if (!experimental_directives_allowed) { memprintf(err, "'%s' : support for pacing is experimental, must be allowed via a global " "'expose-experimental-directives'\n", args[cur_arg]); @@ -204,6 +200,11 @@ static int bind_parse_quic_cc_algo(char **args, int cur_arg, struct proxy *px, cc_algo->pacing_inter = quic_cc_default_pacing_inter; } + else if (!(cc_algo->flags & QUIC_CC_ALGO_FL_OPT_PACING)) { + ha_warning("'%s' : '%s' algorithm without pacing may cause slowdowns or high loss rates during transfers\n", + args[cur_arg], algo); + cc_algo->pacing_inter = NULL; + } if (*end_opt == ')') { goto out; -- 2.47.2