Commit Graph

23507 Commits

Author SHA1 Message Date
Willy Tarreau
450528b9f5 DOC: ot: mention planned deprecation of the OT filter
Miroslav mentioned below that he's currently working on an OpenTelemetry
replacement for the OpenTracing filter since OpenTracing itself is no
longer maintained nor supported:

  https://github.com/haproxy/haproxy/issues/2782#issuecomment-2493576327

Given that he aims for 3.2, let's already settle on an upcoming deprecation
of the filter for 3.3 with a removal for 3.5. This will leave time to finish
the development and permit users to switch smoothly. At this point no warning
is emitted (since the users have no alternative) but better mention this plan
in the doc to make them aware of future changes.
2024-11-22 16:11:51 +01:00
William Lallemand
03f57fcf94 CI: update to the latest WolfSSL version
Update the CI to the 5.7.4 WolfSSL version.
2024-11-22 16:05:32 +01:00
William Lallemand
0022962ecb CI: update to the latest AWS-LC version
Update the CI to the 1.39.0 AWS-LC version.
2024-11-22 16:03:28 +01:00
Frederic Lecaille
7472990f86 BUG/MINOR: quic: Avoid BUG_ON() on ->on_pkt_lost() BBR callback call
The per-packet delivery rate sample is applied to ack-eliciting packet only
calling ->drs_on_transmit() BBR callback. So, ->on_pkt_lost() which inspects the
delivery rate sampling information during packet loss detection must not be
called for non ack-eliciting packet. If not, it would be facing with non
initialized variables with big chance to trigger a BUG_ON().

As BBR is implemented in the current developement version, there is
no need to backport this patch.
2024-11-22 15:51:29 +01:00
Willy Tarreau
b30639848e BUILD: activity/memprofile: fix a build warning in the posix_memalign handler
A "return NULL" statement was placed for error handling in the
posix_memalign() handler instead of an int errno value, by recent
commit 5ddc8b3ad4 ("MINOR: activity/memprofile: monitor non-portable
calls as well"). Surprisingly the warning only triggered on gcc-4.8.
Let's use ENOMEM instead. No backport needed.
2024-11-22 09:42:49 +01:00
Christopher Faulet
b150ae46dd BUG/MAJOR: mux-h1: Properly handle wrapping on obuf when dumping the first-line
The formatting of the first-line, for a request or a response, does not
properly handle the wrapping of the output buffer. This may lead to a data
corruption for the current response or eventually for the previous one.

Utility functions used to format the first-line of the request or the
response rely on the chunk API. So it is not expected to pass a buffer that
wraps. Unfortunatly, because of a change performed during the 2.9 dev cycle,
the output buffer was direclty used instead of a non-wrapping buffer created
from it with b_make() function. It is not an issue for the request because
its start-line is always the first block formatted in the output buffer. But
for the response, the output may be not empty and may wrap. In that case,
the response start-line is dumped at a random position in the buffer,
corrupting data. AFAIK, it is only an issue if the HTTP request pipelining
is used.

To fix the issue, we now take care to create a non-wapping buffer from the
output buffer.

This patch should fix issues #2779 and #2996. It must be backported as far as
2.9.
2024-11-22 08:48:53 +01:00
Willy Tarreau
c5d0342fa2 [RELEASE] Released version 3.1-dev14
Released version 3.1-dev14 with the following main changes :
    - MINOR: acl: export find_acl_default()
    - MINOR: sample: extend the "when" converter to support an ACL
    - MINOR: cfgparse: parse tune.{rcvbuf,sndbuf}.{client,server} as sizes
    - MINOR: cfgparse: parse tune.{rcvbuf,sndbuf}.{frontend,backend} as sizes
    - MINOR: cfgparse: parse tune.pipesize as a size
    - MINOR: cfgparse: parse tune.recv_enough as a size
    - MINOR: cfgparse: parse tune.bufsize as a size
    - MINOR: cfgparse: parse tune.bufsize.small as a size
    - REGTESTS: silence the "log format ignored" warnings
    - REGTESTS: silence warning "previous 'http-response' action is final"
    - REGTESTS: make the unit explicit for very short timeouts
    - REGTESTS: silence warnings about content-type being ignored
    - REGTESTS: remove a duplicate "option httpslog" in the defaults section
    - REGTESTS: silence warning "L6 sample fetches ignored" in cond_set_var
    - REGTESTS: add missing timeouts to 30 tests
    - REGTESTS: only use tune.ssl.default-dh-param when not using AWS-LC
    - REGTESTS: enable -dW on almost all tests to fail on warnings
    - MEDIUM: config: warn on unitless timeouts < 100 ms
    - MINOR: tools: make parse_size_err() support 32/64 bits
    - MINOR: ring: support unit suffixes in the size
    - MINOR: cfgparse-global: parse options to allow non std keywords in discovery mode
    - BUG/MINOR: mworker-prog: don't warn about deprecated section with expose-deprecated-directives
    - MINOR: cli: make "show env" accessible via master CLI without enabling debug
    - MINOR: config: show HAPROXY_BRANCH in "show env" output
    - MINOR: http-ana: Add option to keep query-string on a localtion-based redirect
    - MINOR: http-ana: Add support for "set-cookie-fmt" option to redirect rules
    - MINOR: agent-check: Be able to set absolute weight via an agent
    - MINOR: stream: Add an option to "show sess" command to dump the captured URI
    - DOC: config: A a space before ':' for {bs,fs}.aborted and {bs,fs}.rst_code
    - DOC: config: Fix a typo in "1.3.1. The Request line"
    - MINOR: http: Add support for HTTP 414/431 status codes
    - DEV: phash: Update 414 and 431 status codes to phash
    - MINIR: mux-h1: Return 414 or 431 when appropriate
    - BUG/MINOR: http_ana: Report -1 for %Tr for invalid response only
    - DOC: config: Slightly improve the %Tr documentation
    - DOC: config: Move wait_end in section about internal samples
    - DOC: config: Move fs.* and bs.* in section about L5 samples
    - MINOR: stats-file: add the filename in the warning
    - MEDIUM: stats-file: explicitely ignore comments starting by //
    - DOC: quic: rename max-window-size as with default prefix
    - MINOR: mux-quic: add missing values for show flags
    - MINOR: quic: simplify qc_prep_pkts() exit path
    - MINOR: quic: support a max number of built packet per send iteration
    - MINOR: quic: extend qc_send_mux() return type with a dedicated enum
    - MINOR: quic: define quic_pacing module
    - MINOR: quic/pacing: implement quic_pacer engine
    - MINOR: quic/pacing: support pacing emission on quic_conn layer
    - MINOR: quic/pacing: add burst support
    - MINOR: mux-quic: define a tx STREAM frame list member
    - MINOR: mux-quic: encapsulate QCC tasklet wakeup
    - MAJOR: mux-quic: support pacing emission
    - MINOR: quic: use dynamic cc_algo on bind_conf
    - MINOR: quic: extend quic-cc-algo optional parameters
    - MEDIUM: quic: define cubic-pacing congestion algorithm
    - MINOR: mux_quic/pacing: display pacing info on show quic
    - MEDIUM: stats-file: silently ignore be/fe mistmatch
    - REGTESTS: use -dW by default on every reg-tests
    - DOC: lua: fix yield-dependent methods expected contexts
    - DOC: sched: add missing scheduler API documentation for tasklet_wakeup_after()
    - DOC: sched: document the missing TASK_F_UEVT* flags
    - CLEANUP: tinfo: move sched_*_date/*_mono_time to the thread-local area
    - MINOR: stream: don't update s->lat_time when the wakeup date is not set
    - MINOR: tinfo/clock: turn sched_call_date to 64-bits
    - MINOR: sched: add TASK_F_WANTS_TIME to make the scheduler update the call date
    - MINOR: tools: add new macro DEFZERO to provide a default zero argument
    - MINOR: tasklet: make the low-level tasklet API take a flag
    - MINOR: tasklet: support an optional set of wakeup flags to tasklet_wakeup_on()
    - DOC: configuration: explain the rules regarding spaces in arguments
    - DOC: configuration: explain quotes and spaces in conditional blocks
    - DOC: configuration: wrap long line for "strstr()" conditional expression
    - BUG/MINOR: http-ana: Adjust the server status before the L7 retries
    - MINOR: http-fetch: Add an option to 'query" to get the QS with the '?'
    - BUG/MINOR: cfgparse-quic: fix renaming of max-window-size
    - MEDIUM: mworker: remove USE_SYSTEMD requirement for -Ws
    - CI: vtest: temporarily build from the sd-notify PR
    - MINOR: systemd: replace SOCK_CLOEXEC by fcntl call to FD_CLOEXEC
    - BUILD: makefile: make ERR apply to build options as well
    - MINOR: startup: set HAPROXY_LOCALPEER only once
    - DOC: configuration: update "Environment variables" chapter
    - DOC: config: indent the list of environment variables
    - OPTION: map/hlua: make core.set_map() lookup more efficient
    - REGTESTS: switch to -Ws for master-worker reg-tests
    - REGTESTS: disable temporarly mworker test on OSX
    - MINOR: quic: Add the congestion window initial value to QUIC path
    - MINOR: window_filter: Implement windowed filter (only max)
    - MINOR: quic: implement delivery rate sampling algorithm
    - MINOR: quic: implement BBR congestion control algorithm for QUIC
    - MINOR: quic: quic_cc modifications to support BBR
    - MINOR: quic: quic_loss modifications to support BBR
    - MINOR: quic: RX part modifications to support BBR
    - MINOR: quic: TX part modifications to support BBR.
    - MINOR: quic: add "bbr" new "quic-cc-algo" option
    - BUG/MEDIUM: mux-h2: Increase max number of headers when encoding HEADERS frames
    - BUG/MEDIUM: mux-h2: Check the number of headers in HEADERS frame after decoding
    - BUG/MEDIUM: h3: Properly limit the number of headers received
    - BUG/MEDIUM: h3: Increase max number of headers when sending headers
    - DOC: config: Improve documentation of tune.http.maxhdr directive
    - DOC: management: Clearly state "show errors" only reports malformed H1 messages
    - BUILD: makefile: build flags.c before haproxy to speed up the build
    - BUILD: makefile: reorder object files by build time
    - MINOR: config: Improve warnings on misplaced rules by adding an optional arg
    - CLEANUP: cfgparse: Add direction in functions name that warn on misplaced rules
    - MINOR: cfgparse: Emit a warning for misplaced "tcp-response content" rules
    - BUG/MINOR: cfgparse-quic: fix bbr initialization
    - MINOR: cfgparse-quic: activate pacing only via burst argument
    - MINOR: quic: Useless rate sample member initialization
    - BUG/MINOR: cfgparse-quic: fix warning for cc-aglo with 0 burst
    - MINOR: quic: support pacing for newreno and nocc
    - BUG/MINOR: quic: Missing application limitations tracking for BBR
    - MINOR: cfgparse-global: add cfg_parse_global_chroot
    - MINOR: cfgparse-global: add more checks for "chroot" argument
    - BUG/MINOR: startup: fix UAF when set the default for log_tag
    - MINOR: capabilities: rename program_name argument to progname
    - MINOR: startup: use global progname variable
    - MINOR: cfgparse-global: add cfg_parse_global_localpeer
    - BUG/MINOR: config: allow to check HAPROXY_LOCALPEER in config
    - BUG/MINOR: startup: init_early: remove obsolete comment
    - BUG/MEDIUM: debug: don't set the STUCK flag from debug_handler()
    - BUG/MEDIUM: wdt: fix the stuck detection for warnings
    - BUG/MINOR: activity/memprofile: reinitialize the free calls on DSO summary
    - MINOR: activity/memprofile: offer a function to unregister stale info
    - BUG/MEDIUM: pools/memprofile: always clean stale pool info on pool_destroy()
    - MINOR: activity: better report nil than ffff in unknown callers
    - CLEANUP: activity: better use a mask to tests freeing methods
    - MINOR: activity/memprofile: also monitor strdup() activity
    - MINOR: activity/memprofile: monitor non-portable calls as well
    - MINOR: activity: interrupt the show profile dump more often
    - MINOR: tools: resolve main() only once in resolve_sym_name()
    - MINOR: tools: add a new function "resolve_dso_name" to find a symbol's DSO
    - MINOR: activity/memprofile: use resolve_dso_name() for the DSO summary
    - REGTESTS: relax strerror matching to avoid a failure on libmusl
    - REGTESTS: don't rely on the base64 utility when openssl base64 is already used
2024-11-21 23:26:41 +01:00
Willy Tarreau
a89a2d8902 REGTESTS: don't rely on the base64 utility when openssl base64 is already used
Regtest ocsp_auto_update.vtc used to fail here on FreeBSD because the
base64 utility was not installed by default. Once installed it would
still fail because the utility doesn't support -w to wrap lines. Since
the regtest already relies on openssl base64 for a few commands, let's
just rely on it for the other ones. The only limitation is that openssl
freezes on lines longer than 1024 bytes, and doesn't seem to process more
than 255 chars at once, which might be the reason for using base64 -w 1000
in the first place (the script was probably tested like this). Instead
sed is efficient at wrapping long lines and does the job pretty well.
The output was fixed at 72 chars so that the output is also readable on
a terminal for debugging.
2024-11-21 21:10:09 +01:00
Willy Tarreau
a1ace74b7e REGTESTS: relax strerror matching to avoid a failure on libmusl
The regtest4be_1srv_smtpchk_httpchk_layer47errors.vtc fails on musl
because it reports "Network unreachable" for -EUNREACH while the
check matches "Network is unreachable" as on other OSes. Let's just
replace " is" with ".*". It now works on both glibc and musl.
2024-11-21 20:26:46 +01:00
Willy Tarreau
ead0b0154b MINOR: activity/memprofile: use resolve_dso_name() for the DSO summary
Let's simplify the code by making use of this simpler and sometimes
more efficient variant.
2024-11-21 19:58:06 +01:00
Willy Tarreau
670507a66e MINOR: tools: add a new function "resolve_dso_name" to find a symbol's DSO
In the memprofile summary per DSO, we currently have to pay a high price
by calling dladdr() on each symbol when doing the summary per DSO at the
end, while we're not interested in these details, we just want the DSO
name which can be made cheaper to obtain, and easier to manipulate. So
let's create resolve_dso_name() to only extract minimal information from
an address. At the moment it still uses dladdr() though it avoids all the
extra expensive work, and will further be able to leverage the same
mechanism as "show libs" to instantly spot DSO from address ranges.
2024-11-21 19:58:06 +01:00
Willy Tarreau
a205a91bb3 MINOR: tools: resolve main() only once in resolve_sym_name()
resolv_sym_name() calls dladdr(main) for each symbol in order to compare
the first address with other symbols. But this is pointless and quite
expensive in outputs to "show profiling" for example. Let's just keep a
local copy and have a variable indicating if the resolution is needed/
in progress/done to save the value for subsequent calls.
2024-11-21 19:58:06 +01:00
Willy Tarreau
9a8b834435 MINOR: activity: interrupt the show profile dump more often
The calls to resolv_sym_name() can be a bit expensive. Forcing to
yield more often is better for the latency and will avoid the
watchdog reporting warnings.

Note that it's still called in the sort at the end, but that one
cannot be avoided. At best we could try to rely on the list of libs
but that's not trivial and not always present.
2024-11-21 19:58:06 +01:00
Willy Tarreau
5ddc8b3ad4 MINOR: activity/memprofile: monitor non-portable calls as well
Some dependencies might very well rely on posix_memalign(), strndup()
or other less portable callsn making us miss them when chasing memory
leaks, resulting in negative global allocation counters. Let's provide
the handlers for the following functions:

  strndup()        // _POSIX_C_SOURCE >= 200809L || glibc >= 2.10
  valloc()         // _BSD_SOURCE || _XOPEN_SOURCE>=500 || glibc >= 2.12
  aligned_alloc()  // _ISOC11_SOURCE
  posix_memalign() // _POSIX_C_SOURCE >= 200112L
  memalign()       // obsolete
  pvalloc()        // obsolete

This time we don't fail if they're not found, we just silently forward
the calls.
2024-11-21 19:58:06 +01:00
Willy Tarreau
33c0ce299d MINOR: activity/memprofile: also monitor strdup() activity
Some memory profiling outputs have showed negative counters, very likely
due to some libs calling strdup(). Let's add it to the list of monitored
activities.

Actually even haproxy itself uses some. Having "profiling.memory on" in
the config reveals 35 call places.
2024-11-21 19:58:06 +01:00
Willy Tarreau
623a2c4e19 CLEANUP: activity: better use a mask to tests freeing methods
In "show profiling memory", we need to distinguish methods which really
free memory from those which do not so that we don't account for the
free value twice. However for now it's done using multiple tests, which
are going to complicate the addition of new methods. Let's switch to a
bit field defined as a mask in a single place instead, as we don't
intend to use more than 32/64 methods!
2024-11-21 19:58:06 +01:00
Willy Tarreau
f3547d0b74 MINOR: activity: better report nil than ffff in unknown callers
For unknown callers we try to get the lowest known address and we
purposely ignore NULL during calculation of the min. But the side
effect is that we also report ffff in the per-DSO address. Better
catch this case and finally accept to report nil. Before it would
report this:

  $ socat - /tmp/sock1 <<< "show profiling memory" |grep nil
        50000          10        9600000           9440|            (nil) [other] unknown(192) [delta=9590560] [pool=http_txn]
        50000          10        9600000           9440|            (nil) DSO:other; delta_calls=49990; delta_bytes=9590560

now it reports this:

  $ socat - /tmp/sock1 <<< "show profiling memory" |grep nil
        50000          11        9600000           9656|            (nil) [other] unknown(192) [delta=9590344] [pool=connection]
        50000          11        9600000           9656|            (nil) DSO:other; delta_calls=49989; delta_bytes=9590344
2024-11-21 19:58:06 +01:00
Willy Tarreau
ed3ed35867 BUG/MEDIUM: pools/memprofile: always clean stale pool info on pool_destroy()
There's actually a problem with memprofiles: the pool pointer is stored
in ->info but some pools are replaced during startup, such as the trash
pool, leaving a dangling pointer there, that may randomly report crap or
even crash during "show profile memory".

Let's make pool_destroy() call memprof_remove_stale_info() added
by previous patch so that these entries are properly unregistered.

This must be backported along with the previous patch (MINOR:
activity/memprofile: offer a function to unregister stale info) as
far as 2.8.
2024-11-21 19:58:06 +01:00
Willy Tarreau
859341c1ec MINOR: activity/memprofile: offer a function to unregister stale info
There's actually a problem with memprofiles: the pool pointer is stored
in ->info but some pools are replaced during startup, such as the trash
pool, leaving a dangling pointer there.

Let's complete the API with a new function memprof_remove_stale_info()
that will remove all stale references to this info pointer. It's also
present when USE_MEMORY_PROFILING is not set so as to ease the job on
callers.
2024-11-21 19:58:06 +01:00
Willy Tarreau
c42a2b8c94 BUG/MINOR: activity/memprofile: reinitialize the free calls on DSO summary
In commit 401fb0e87a ("MINOR: activity/memprofile: show per-DSO stats")
we added a summary per DSO. However the free calls/tot were not initialized
when creating a new entry because initially they were applied to any entry,
but since we don't update free calls for non-free capable callers, we still
need to reinitialize these entries when reassigning one. Because of this
bug, a "show profiling memory" output can randomly show highly negative
values on the DSO lines if it turns out that the DSO entry was created on
an alloc instead of a realloc/free.

Since the commit above was backported to 2.9, this one must go there as
well.
2024-11-21 19:58:05 +01:00
Willy Tarreau
24ce001771 BUG/MEDIUM: wdt: fix the stuck detection for warnings
If two slow tasks trigger one warning even a few seconds apart, the
watchdog code will mistakenly take this for a definite stuck task and
kill the process. The reason is that since commit 148eb5875f ("DEBUG:
wdt: better detect apparently locked up threads and warn about them")
the updated ctxsw count is not the correct one, instead of updating
the private counter it resets the public one, preventing it from making
progress and making the wdt believe that no progress was made. In
addition the initial value was read from [tid] instead of [thr].

Please note that another fix is needed in debug_handler() otherwise the
watchdog will fire early after the first warning or thread dump.

A simple test for this is to issue several of these commands back-to-back
on the CLI, which crashes an unfixed 3.1 very quickly:

  $ socat /tmp/sock1 - <<< "expert-mode on; debug dev loop 1000"

This needs to be backported to 2.9 since the fix above was backported
there. The impact on 3.0 and 2.9 is almost inexistent since the watchdog
there doesn't apply the shorter warning delay, so the first call already
indicates that the thread is stuck.
2024-11-21 19:58:05 +01:00
Willy Tarreau
1151fe6818 BUG/MEDIUM: debug: don't set the STUCK flag from debug_handler()
Since 2.0 with commit e6a02fa65a ("MINOR: threads: add a "stuck" flag
to the thread_info struct"), the TH_FL_STUCK flag was set by the
debugger to flag that a thread was stuck and report it in the output.

However, two commits later (2bfefdbaef "MAJOR: watchdog: implement a
thread lockup detection mechanism"), this flag was used to detect that
a thread had already been reported as stuck. The problem is that it
seldom happens that a "show threads" command instantly crashes because
it calls debug_handler(), which sets the flag, and if the watchdog timer
was about to trigger before going back to the scheduler, the watchdog
believes that the thread has been stuck for a while and will kill the
process.

The issue was magnified in 3.1 with the lower-delay warning, because
it's possible for a thread to die on the next wakeup after the first
warning (which calls debug_handler() hence sets the STUCK flag).

One good approach would have been to use two distinct flags, one for
"stuck" as reported by the debug handler, and one for "stuck" as seen
by the watchdog. However, one could also argue that since the second
commit, given that the wdt monitors the threads, there's no point any
more for the debug handler to set the flag itself. Removing this code
means that two consecutive "show threads" will not report "stuck" until
the watchdog sets it, which aligns better with expectations.

This can be backported to all stable releases. This code has changed a
bit over time, the "if" block and the harmless variables just need to
be removed.
2024-11-21 19:58:05 +01:00
Valentine Krasnobaeva
332839eb9d BUG/MINOR: startup: init_early: remove obsolete comment
This fixes the commit d6ccd1738b
("MINOR: startup: set HAPROXY_LOCALPEER only once").

Comment "/* preset some environment variables */" is now useless here as
HAPROXY_LOCALPEER is set later during the initialization stage and only once.

This should not be backported, as related to the latest master-worker
refactoring.
2024-11-21 19:55:21 +01:00
Valentine Krasnobaeva
aa88d6ee37 BUG/MINOR: config: allow to check HAPROXY_LOCALPEER in config
This fixes the commit d6ccd1738b
("MINOR: startup: set HAPROXY_LOCALPEER only once"). HAPROXY_LOCALPEER could
be checked in the configuration to set some servers settings or listeners. So,
we need to set it just before we read the configuration at the second time.

Let's mark HAPROXY_LOCALPEER as "usable" in the configuration in the related
documentation chapter.

This should not be backported, as related to the latest master-worker
refactoring.
2024-11-21 19:55:21 +01:00
Valentine Krasnobaeva
d253f30823 MINOR: cfgparse-global: add cfg_parse_global_localpeer
This commit prepares the parsing of localpeer keyword in MODE_DISCOVERY. We
need this, as HAPROXY_LOCALPEER environment variable could be checked in the
configuration in order to enable some backend or frontend settings.

So, let's at first add a dedicated parser for localpeer. At second, we no
longer need to check, if cfg_peers is valid pointer, as in MODE_DISCOVERY we
parse only the "global" section.

In addition, let's make the code of localpeer parser a little bit more
readable.
2024-11-21 19:55:21 +01:00
Valentine Krasnobaeva
bfe0f9d02d MINOR: startup: use global progname variable
Let's store progname in the global variable, as it is handy to use it in
different parts of code to format messages sent to stdout.

This reduces the number of arguments, which we should pass to some functions.
2024-11-21 19:55:21 +01:00
Valentine Krasnobaeva
ef154a49e1 MINOR: capabilities: rename program_name argument to progname
This commit prepares the usage of the global progname variable.
prepare_caps_from_permitted_set() use progname value in warning messages. So,
let's rename program_name argument to progname.
2024-11-21 19:55:21 +01:00
Valentine Krasnobaeva
351ae5dbed BUG/MINOR: startup: fix UAF when set the default for log_tag
In the init_early() global.log_tag is initialized to the string from progname
pointer and global.log_tag.area points to this pointer.

If log-tag keyword is provided in the configuration, its parser at first frees
global.log_tag.area and then it does a new memory allocation to copy
there the argument of log-tag. So, progname no longer points to the valid
memory.

To fix this, let's always keep progname and global.log_tag.area at separate
memory areas. If log_tag will be redefined in the configuration, its parser will
free the memory allocated for the default value in chunk_destroy(). Memory
allocated for progname will be freed in deinit().

This should not be backported as related to the latest master-worker
refactoring.
2024-11-21 19:55:21 +01:00
Valentine Krasnobaeva
d1c3cd8974 MINOR: cfgparse-global: add more checks for "chroot" argument
If directory provided as a "chroot" keyword argument does not exist or
inaccessible, this is reported only at the latest initialization stage, when
haproxy tries to perform chroot. Sometimes it's not very convenient, as the
process is already bound to listen sockets.

This was done explicitly in order not to break the case, when haproxy is
launched with "-c" option in some specific environment, where it's not possible
to create or to modify chroot directory, provided in the configuration.

So, let's add more checks for "chroot" directory during the parsing
stage and let's show diagnostic warnings, if this directory has become
non-accesible or was deleted. Like this, users, who wants to catch errors
related to misconfigured chroot before starting the process, can launch haproxy
with -dW and -dD. zero-warning mode will stop the process with error, if any
warning was emitted during initialization stage.
2024-11-21 19:55:21 +01:00
Valentine Krasnobaeva
c853502cc6 MINOR: cfgparse-global: add cfg_parse_global_chroot
Let's add a dedicated parser for "chroot" keyword, as we add some more checks
for its argument in the next commit.

This reduces the size of cfg_parse_global().
2024-11-21 19:55:21 +01:00
Frederic Lecaille
01fcbd6c08 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.
2024-11-21 19:23:53 +01:00
Amaury Denoyelle
95d3edd68f MINOR: quic: support pacing for newreno and nocc
Extend extra pacing support for newreno and nocc congestion algorithms,
as with cubic.

For better extensibility of cc algo definition, define a new flags field
in quic_cc_algo structure. For now, the only value is
QUIC_CC_ALGO_FL_OPT_PACING which is set if pacing support can be
optionally activated. Both cubic, newreno and nocc now supports this.

This new flag is then reused by QUIC config parser. If set, extra
quic-cc-algo burst parameter is taken into account. If positive, this
will activate pacing support on top of the congestion algorithm. As with
cubic previously, pacing is only supported if running under experimental
mode.

Only BBR is not flagged with this new value as pacing is directly
builtin in the algorithm and cannot be turn off. Furthermore, BBR
calculates automatically its value for maximum burst. As such, any
quic-cc-algo burst argument used with BBR is still ignored with a
warning.
2024-11-21 11:33:44 +01:00
Amaury Denoyelle
99497d23b5 BUG/MINOR: cfgparse-quic: fix warning for cc-aglo with 0 burst
Optional burst argument for quic-cc-algo is used to toggle pacing
support on top of cubic. This is the case if it is positive.

The default value is 0, which do not activate pacing. However, in this
case, an incorrect warning is reported about the parameter being
ignored. Fix this by removing the warning in this case.

No need to backport.
2024-11-21 11:26:36 +01:00
Frederic Lecaille
ea17de01ac MINOR: quic: Useless rate sample member initialization
This poor/inefficient code has been revealed by coverity GH issue in #2788 where
some quic_cc_rs struct member initializations were mentionned as overwritten
(after initialization) before being used as follows:

CID 1565821:  Code maintainability issues  (UNUSED_VALUE)
/src/quic_cc_bbr.c: 1373 in bbr_handle_lost_packet()
1367     }
1368
1369     static void bbr_handle_lost_packet(struct bbr *bbr, struct quic_cc_path *p,
1370                                        struct quic_tx_packet *pkt,
1371                                        uint32_t lost)
1372     {
>>>     CID 1565821:  Code maintainability issues  (UNUSED_VALUE)
>>>     Assigning value "0UL" to "rs.tx_in_flight" here, but that stored value is overwritten before it can be used.
1373            struct quic_cc_rs rs = {0};
1374
1375            /* C.delivered = bbr->drs.delivered */
1376            bbr_note_loss(bbr, bbr->drs.delivered);
1377            if (!bbr->bw_probe_samples)
1378                    return; /* not a packet sent while probing bandwidth */

Remove the {0} initializer for <rs> variable. This is safe because the members
initializations of <rs> local variable passed to functions from
bbr_handle_lost_packet() are done. Add a comment to mention this.
2024-11-21 11:01:53 +01:00
Amaury Denoyelle
de86fd1e6c MINOR: cfgparse-quic: activate pacing only via burst argument
Recently, pacing support was added for cubic congestion algorithm. This
was activated by using the new token "cubic-pacing" on quic-cc-algo.
Furthermore, it was possible to define a burst size with a new
parameters after congestion token between parenthesis.

This configuration is not oblivious to users. In particular, it can
cause to easily forgot to tweak burst size, which can dramatically
impact performance.

Simplify this by removing the extra "-pacing" suffix. Now, pacing will
be activated solely based on the burst parameter. If 0, burst is
considered as infinite and no pacing will be used. Pacing will be
activating for any positive burst. This better reflects the link between
pacing and burst and its importance.

Note that for the moment, if burst is specified, it will be ignored with
a warning for algorithm outside of cubic.

This is not a breaking change as pacing support was implemented in the
current dev version.
2024-11-21 10:55:55 +01:00
Amaury Denoyelle
7b23c9075c BUG/MINOR: cfgparse-quic: fix bbr initialization
To support pacing with cubic, a recent change was introduced to render
quic_cc_algo on bind line dynamically allocated, instead of pointing to
a globally defined variable. This allows customization of the algorithm
callbacks per bind line.

This was not correctly used for BBR as it was set to point to the global
quic_cc_algo_bbr. This causes a segfault on haproxy process closing. Fix
this by properly initializing BBR as other algorithms.

This should fix coverity report from github issue #2786.
2024-11-21 10:49:16 +01:00
Christopher Faulet
e58a30d369 MINOR: cfgparse: Emit a warning for misplaced "tcp-response content" rules
When a "tcp-response content" rule is placed after a "http-response" rule, a
warning is now emitted, just like for rules applied on the requests.
2024-11-21 09:55:04 +01:00
Christopher Faulet
5dcd3b0d99 CLEANUP: cfgparse: Add direction in functions name that warn on misplaced rules
This only concerns functions emitting warnings about misplaced tcp-request
rules. The direction is now specified in the functions name. For instance
"warnif_misplaced_tcp_conn" is replaced by "warnif_misplaced_tcp_req_conn".
2024-11-21 09:51:37 +01:00
Christopher Faulet
7710580428 MINOR: config: Improve warnings on misplaced rules by adding an optional arg
In warnings about misplaced rules, only the first keyword is mentionned. It
works well for http-request or quic-initial rules for instance. But it is a
bit confusing for tcp-request rules, because the layer is missing (session
or content).

To make it a bit systematic (and genric), the second argument can now be
provided. It can be set to NULL if there is no layer or scope. But
otherwise, it may be specified and it will be reported in the warning.

So the following snippet:

    tcp-request content reject if FALSE
    tcp-request session reject if FALSE
    tcp-request connection reject if FALSE

Will now emit the following warnings:

  a 'tcp-request session' rule placed after a 'tcp-request content' rule will still be processed before.
  a 'tcp-request connection' rule placed after a 'tcp-request session' rule will still be processed before.

This patch should fix the issue #2596.
2024-11-21 09:28:42 +01:00
Willy Tarreau
c329bfe3f5 BUILD: makefile: reorder object files by build time
mux_spop is quite long to build and was at the end. The rest did not
change much, but the build time is now dominated by hlua.o and mux_h2.o
and by a large margin. On the 80-core ARM mux_h2.o is present from
beginning to end and on the PC it's hlua.o, so both might have to be
split at some point to benefit from multi-core.

Nevertheless, the changes allowed to shrink about one second out of
the 18 it was taking on that machine.
2024-11-20 18:49:56 +01:00
Willy Tarreau
f16edcd34c BUILD: makefile: build flags.c before haproxy to speed up the build
The end of the build is often super slow. In practice it's flags.o that
now takes ages (3.4 seconds) and blocks everything on a single core at
the end. Let's declare it before the haproxy target so that it starts
earlier. On a quad-2.2 GHz CPU, the build time goes down from 44 to 42s
and the end feels less painful.
2024-11-20 18:49:56 +01:00
Christopher Faulet
667ac8acc6 DOC: management: Clearly state "show errors" only reports malformed H1 messages
For now, only the H1 multiplexer is able to capture malformed messages. So
it is better to update the management guide accordingly to avoid any
confusion.
2024-11-20 18:08:17 +01:00
Christopher Faulet
e863d8d681 DOC: config: Improve documentation of tune.http.maxhdr directive
The description was inproved to clrealy mentionned it is applied on received
requests and responses. In addition, a comment was added about HTTP/2 and
HTTP/3 limitation when messages are encoded to be sent.
2024-11-20 18:02:36 +01:00
Christopher Faulet
3bd9a9e7d7 BUG/MEDIUM: h3: Increase max number of headers when sending headers
In the same way than for the H2, the maximum number of headers that can be
encoded when headers are sent must be increased to match the limit imposed
when they are received.

Reasons are the sames. On receive path, the maximum number of headers
accepted must be higher than the configured limit to be able to handle
pseudo headers and cookies headers. On the sending path, the same limit must
be applied because the pseudo headers will consume some extra slots and the
cookie header could be splitted.

This patch should be backported as far as 2.6.
2024-11-20 17:44:22 +01:00
Christopher Faulet
785e633353 BUG/MEDIUM: h3: Properly limit the number of headers received
The number of headers are limited before the decoding but pseudo headers and
cookie headers consume extra slots. In practice, this lowers the maximum number
of headers that can be received.

To workaround this issue, the limit is doubled during the frame decoding to be
sure to have enough extra slots. And the number of headers is tested against the
configured limit after the HTX message was created to be able to report an
error. Unfortunatly no parsing error are reported because the QUIC multiplexer
is not able to do so for now.

The same is performed on trailers to be consistent with H2.

This patch should be backported as far as 2.6.
2024-11-20 17:44:22 +01:00
Christopher Faulet
63d2760dfa BUG/MEDIUM: mux-h2: Check the number of headers in HEADERS frame after decoding
There is no explicit test on the number of headers when a HEADERS frame is
received. It is implicitely limited by the size of the header list. But it
is twice the configured limit to be sure to decode the frame.

So now, a check is performed after the HTX message was created. This way, we
are sure to not exceed the configured limit after the decoding stage. If
there are too many headers, a parsing error is reported.

Note the same is performed on the trailers.

This patch should patially address the issue #2685. It should be backported
to all stable versions.
2024-11-20 17:44:22 +01:00
Christopher Faulet
e415e3cb7a BUG/MEDIUM: mux-h2: Increase max number of headers when encoding HEADERS frames
When a HEADERS frame is encoded to be sent, the maximum number of headers
allowed in the frame is lower than on receiving path. This can lead to
report a sending error while the message was accepted. It could be
confusing.

In addition, the start-line is splitted into pseudo-headers and consummes
this way some header slots, increasing the difference between HEADERS frames
encoding and decoding. It is even more noticeable because when a HEADERS
frame is decoded, a margin is used to be able to handle splitted cookie
headers. Concretly, on decoding path, a limit of twice the maxumum number of
headers allowed in a message (tune.http.maxhdr * 2) is used. On encoding
path, the exact limit is used. It is not consistent.

Note that when a frame is decoded, we must use a larger limit because the
pseudo headers are reassembled in the start-line and must count for one. But
also because, most of time, the cookies are splitted into several headers
and are reassembled too.

To fix the issue, the same ratio is applied on sending path. A limit must be
defined because an dynamic allocation is not acceptable. Twice of the
configured limit should be good enough to support headers manipulation.

This patch should be backported to all stable versions.
2024-11-20 17:44:22 +01:00
Frederic Lecaille
349954601f MINOR: quic: add "bbr" new "quic-cc-algo" option
Add this new "bbr" option to the list of the congestion control algorithms which
may be set by "quic-cc-algo" setting.

This new algorithm is considered as experimental and may be enabled only if
"expose-experimental-directive" is set.

Also update the documentation for this new setting.
2024-11-20 17:34:22 +01:00
Frederic Lecaille
e778b9a2b6 MINOR: quic: TX part modifications to support BBR.
Very few modifications: call ->on_transmit() and ->drs_on_transmit() congestion
control algorithm (quic_cc) callbacks from qc_send_ppkts() just after having
sents some packets.
2024-11-20 17:34:22 +01:00
Frederic Lecaille
44af88d856 MINOR: quic: RX part modifications to support BBR
qc_notify_cc_of_newly_acked_pkts() aim is to notify the congestion algorithm
of all the packet acknowledgements. It must call quic_cc_drs_update_rate_sample()
to update the delivery rate sampling information. It must also call
quic_cc_drs_on_ack_recv() to update the state of the delivery rate sampling part
used by BBR.
Finally, ->on_ack_rcvd() is called with the total number of bytes delivered
by the sender from the newly acknowledged packets with <bytes_delivered> as
parameter to do so. <pkt_delivered> store the per-packet number of bytes
delivered by the newly sent acknowledged packet (the packet with the highest
packet number). <bytes_lost> is also used and has been set by
qc_packet_loss_lookup() before calling qc_notify_cc_of_newly_acked_pkts().
2024-11-20 17:34:22 +01:00