]> git.ipfire.org Git - thirdparty/haproxy.git/log
thirdparty/haproxy.git
7 months agoREGTESTS: use -dW by default on every reg-tests
William Lallemand [Tue, 19 Nov 2024 15:51:30 +0000 (16:51 +0100)] 
REGTESTS: use -dW by default on every reg-tests

Every reg-test now runs without any warning, so let's acivate -dW by
default so the new ones will inheritate the option.

This patch reverts 9d511b3c ("REGTESTS: enable -dW on almost all tests
to fail on warnings") and adds -dW in the default HAPROXY_ARGS of
scripts/run-regtests.sh instead.

7 months agoMEDIUM: stats-file: silently ignore be/fe mistmatch
William Lallemand [Tue, 19 Nov 2024 15:39:55 +0000 (16:39 +0100)] 
MEDIUM: stats-file: silently ignore be/fe mistmatch

Most of the invalid or unknow field in the stats-file parser are ignored
silently, which is not the case of the frontend/backend mismatch on a
guid, which is kind of strange.

Since this is ""documented"" to be ignored in the
reg-tests/stats/sample-stats-file file, let's also ignore this kind of
line. This will allow to run the associated reg-test with -dW.

7 months agoMINOR: mux_quic/pacing: display pacing info on show quic
Amaury Denoyelle [Tue, 19 Nov 2024 10:27:00 +0000 (11:27 +0100)] 
MINOR: mux_quic/pacing: display pacing info on show quic

To improve debugging, extend "show quic" output to report if pacing is
activated on a connection. Two values will be displayed for pacing :

* a new counter paced_sent_ctr is defined in QCC structure. It will be
  incremented each time an emission is interrupted due to pacing.

* pacing engine now saves the number of datagrams sent in the last paced
  emission. This will be helpful to ensure burst parameter is valid.

7 months agoMEDIUM: quic: define cubic-pacing congestion algorithm
Amaury Denoyelle [Mon, 18 Nov 2024 10:05:28 +0000 (11:05 +0100)] 
MEDIUM: quic: define cubic-pacing congestion algorithm

Define a new QUIC congestion algorithm token 'cubic-pacing' for
quic-cc-algo bind keyword. This is identical to default cubic
implementation, except that pacing is used for STREAM frames emission.

This algorithm supports an extra argument to specify a burst size. This
is stored into a new bind_conf member named quic_pacing_burst which can
be reuse to initialize quic path.

Pacing support is still considered experimental. As such, 'cubic-pacing'
can only be used with expose-experimental-directives set.

7 months agoMINOR: quic: extend quic-cc-algo optional parameters
Amaury Denoyelle [Tue, 19 Nov 2024 14:38:53 +0000 (15:38 +0100)] 
MINOR: quic: extend quic-cc-algo optional parameters

Modify quic-cc-algo for better extensability of optional parameters
parsing. This will be useful to support a new parameter for maximum
allowed pacing burst size.

Take this opportunity to refine quic-cc-algo documentation. Optional
parameters are now presented as a list which would be soon extended.

7 months agoMINOR: quic: use dynamic cc_algo on bind_conf
Amaury Denoyelle [Tue, 19 Nov 2024 09:12:27 +0000 (10:12 +0100)] 
MINOR: quic: use dynamic cc_algo on bind_conf

A QUIC congestion algorithm can be specified on the bind line via
keyword quic-cc-algo. As such, bind_conf structure has a member
quic_cc_algo.

Previously, if quic-cc-algo was set, bind_conf member was initialized to
one of the globally defined CC algo structure. This patch changes
bind_conf quic_cc_algo initialization to point to a dynamically
allocated copy of CC algo structure.

With this change, it will be possible to tweak individually each CC algo
of a bind line. This will be used to activate pacing on top of the
congestion algorithm.

As bind_conf member is dynamically allocated now, its member is now
freed via free_proxy() to prevent any leak.

7 months agoMAJOR: mux-quic: support pacing emission
Amaury Denoyelle [Mon, 18 Nov 2024 13:55:41 +0000 (14:55 +0100)] 
MAJOR: mux-quic: support pacing emission

Support pacing emission for STREAM frames at the QUIC MUX layer. This is
implemented by adding a quic_pacer engine into QCC structure.

The main changes have been written into qcc_io_send(). It now
differentiates cases when some frames have been rejected by transport
layer. This can occur as previously due to congestion or FD buffer full,
which requires subscribing on transport layer. The new case is when
emission has been interrupted due to pacing timing. In this case, QUIC
MUX I/O tasklet is rescheduled to run with the flag TASK_F_USR1.

On tasklet execution, if TASK_F_USR1 is set, all standard processing for
emission and reception is skipped. Instead, a new function
qcc_purge_sending() is called. Its purpose is to retry emission with the
saved STREAM frames list. Either all remaining frames can now be send,
subscribe is done on transport error or tasklet must be rescheduled for
pacing purging.

In the meantime, if tasklet is rescheduled due to other conditions,
TASK_F_USR1 is reset. This will trigger a full regeneration of STREAM
frames. In this case, pacing expiration must be check before calling
qcc_send_frames() to ensure emission is now allowed.

7 months agoMINOR: mux-quic: encapsulate QCC tasklet wakeup
Amaury Denoyelle [Wed, 16 Oct 2024 15:56:15 +0000 (17:56 +0200)] 
MINOR: mux-quic: encapsulate QCC tasklet wakeup

QUIC MUX will be responsible to drive emission with pacing. This will be
implemented via setting TASK_F_USR1 before I/O tasklet wakeup. To
prepare this, encapsulate each I/O tasklet wakeup into a new function
qcc_wakeup().

This commit is purely refactoring prior to pacing implementation into
QUIC MUX.

7 months agoMINOR: mux-quic: define a tx STREAM frame list member
Amaury Denoyelle [Mon, 18 Nov 2024 08:53:39 +0000 (09:53 +0100)] 
MINOR: mux-quic: define a tx STREAM frame list member

For STREAM emission, MUX QUIC previously used a local list defined under
qcc_io_send(). This was suitable as either all frames were sent, or
emission must be interrupted due to transport congestion or fatal error.
In the latter case, the list was emptied anyway and a new frame list was
built on future qcc_io_send() invokation.

For pacing, MUX QUIC may have to save the frame list if pacing should be
applied across emission. This is necessary to avoid to unnecessarily
rebuilt stream frame list between each paced emission. To support this,
STREAM list is now stored as a member of QCC structure.

Ensure frame list is always deleted, even on QCC release, using newly
defined utility function qcc_tx_frms_free().

7 months agoMINOR: quic/pacing: add burst support
Amaury Denoyelle [Mon, 18 Nov 2024 14:31:12 +0000 (15:31 +0100)] 
MINOR: quic/pacing: add burst support

qc_send_mux() has been extended previously to support pacing emission.
This will ensure that no more than one datagram will be emitted during
each invokation. However, to achieve better performance, it may be
necessary to emit a batch of several datagrams one one turn.

A so-called burst value can be specified by the user in the
configuration. However, some congestion control algos may defined their
owned dynamic value. As such, a new CC callback pacing_burst is defined.

quic_cc_default_pacing_burst() can be used for algo without pacing
interaction, such as cubic. It will returns a static value based on user
selected configuration.

7 months agoMINOR: quic/pacing: support pacing emission on quic_conn layer
Amaury Denoyelle [Thu, 24 Oct 2024 14:06:48 +0000 (16:06 +0200)] 
MINOR: quic/pacing: support pacing emission on quic_conn layer

Pacing will be implemented for STREAM frames emission. As such,
qc_send_mux() API has been extended to add an argument to a quic_pacer
engine.

If non NULL, engine will be used to pace emission. In short, no more
than one datagram will be emitted for each qc_send_mux() invokation.
Pacer is then notified about the emission and a timer for a future
emission is calculated. qc_send_mux() will return PACING error value, to
inform QUIC MUX layer that it will be responsible to retry emission
after some delay.

7 months agoMINOR: quic/pacing: implement quic_pacer engine
Amaury Denoyelle [Mon, 18 Nov 2024 13:57:39 +0000 (14:57 +0100)] 
MINOR: quic/pacing: implement quic_pacer engine

Extend quic_pacer engine to support pacing emission. Several functions
are defined.
* quic_pacing_sent_done() to notify engine about an emission of one or
  several datagrams
* quic_pacing_expired() to check if emission should be delayed or can be
  conducted immediately

7 months agoMINOR: quic: define quic_pacing module
Amaury Denoyelle [Thu, 24 Oct 2024 13:53:17 +0000 (15:53 +0200)] 
MINOR: quic: define quic_pacing module

Add a new module quic_pacing. A new structure quic_pacer is defined.
This will be used as a pacing engine to implement smooth emission of
QUIC data.

7 months agoMINOR: quic: extend qc_send_mux() return type with a dedicated enum
Amaury Denoyelle [Fri, 25 Oct 2024 14:31:26 +0000 (16:31 +0200)] 
MINOR: quic: extend qc_send_mux() return type with a dedicated enum

This commit is part of a adjustment on QUIC transport send API to
support pacing. Here, qc_send_mux() return type has been changed to use
a new enum quic_tx_err.

This is useful to explain different failure causes of emission. For now,
only two values have been defined : NONE and FATAL. When pacing will be
implemented, a new value would be added to specify that emission was
interrupted on pacing. This won't be a fatal error as this allows to
retry emission but not immediately.

7 months agoMINOR: quic: support a max number of built packet per send iteration
Amaury Denoyelle [Fri, 11 Oct 2024 15:57:57 +0000 (17:57 +0200)] 
MINOR: quic: support a max number of built packet per send iteration

Extend QUIC transport emission function to support a maximum datagram
argument. The purpose is to ensure that qc_send() won't emit more than
the specified value, unless it is 0 which is considered as unlimited.

In qc_prep_pkts(), a counter of built datagram has been added to support
this. The packet building loop is interrupted if it reaches a specified
maximum value. Also, its return value has been changed to the number of
prepared datagrams. This is reused by qc_send() to interrupt its work if
a specified max datagram argument value is reached over one or several
iteration of prepared/sent datagrams.

This change is necessary to support pacing emission. Note that ideally,
the total length in bytes of emitted datagrams should be taken into
account instead of the raw number of datagrams. However, for a first
implementation, it was deemed easier to implement it with the latter.

7 months agoMINOR: quic: simplify qc_prep_pkts() exit path
Amaury Denoyelle [Wed, 16 Oct 2024 15:50:30 +0000 (17:50 +0200)] 
MINOR: quic: simplify qc_prep_pkts() exit path

To prepare pacing support, qc_prep_pkts() exit path have been rewritten
to be easily modified. This is purely refactoring which should not have
any functional change :
* a dedicated error path has been added
* ensure qc_txb_store() is always called to finalize datagram on normal
  exit path if first_pkt is not NULL. Needed to support breaking from
  packet building loop in a easier way.

7 months agoMINOR: mux-quic: add missing values for show flags
Amaury Denoyelle [Tue, 19 Nov 2024 10:01:57 +0000 (11:01 +0100)] 
MINOR: mux-quic: add missing values for show flags

Add QCC QC_CF_WAIT_FOR_HS and QCS QC_SF_TXBUB_OOB flags to their
respective show_flags to be able to decipher them via dev flags utility.

These values have been added in the current dev version, thus no need to
backport this patch.

7 months agoDOC: quic: rename max-window-size as with default prefix
Amaury Denoyelle [Tue, 19 Nov 2024 14:11:20 +0000 (15:11 +0100)] 
DOC: quic: rename max-window-size as with default prefix

Rename 'tune.quic.frontend.max-window-size' with the prefix 'default-'.
This highlights the fact that it is not a hard limit, as it can be
overriden if specifying an optional window size via quic-cc-algo on a
bind line.

No need to backport as this keyword was added on the current dev
version.

7 months agoMEDIUM: stats-file: explicitely ignore comments starting by //
William Lallemand [Tue, 19 Nov 2024 14:43:35 +0000 (15:43 +0100)] 
MEDIUM: stats-file: explicitely ignore comments starting by //

Explicitely ignore comments starting by // so they don't emit a warning.

7 months agoMINOR: stats-file: add the filename in the warning
William Lallemand [Tue, 19 Nov 2024 14:42:05 +0000 (15:42 +0100)] 
MINOR: stats-file: add the filename in the warning

Add the name of the stats-file in the warning so it's clear that the
warning was provoked by the stats-file and not the config file.

7 months agoDOC: config: Move fs.* and bs.* in section about L5 samples
Christopher Faulet [Tue, 19 Nov 2024 07:49:05 +0000 (08:49 +0100)] 
DOC: config: Move fs.* and bs.* in section about L5 samples

These sample fetch functions were added in the wrong section. Move them in
the section about sample fetch functions at L5 layer.

7 months agoDOC: config: Move wait_end in section about internal samples
Christopher Faulet [Tue, 19 Nov 2024 07:45:29 +0000 (08:45 +0100)] 
DOC: config: Move wait_end in section about internal samples

wait_end is an internal sample fetch functions and not a L6 one. So move it
in the corresponding section.

7 months agoDOC: config: Slightly improve the %Tr documentation
Christopher Faulet [Mon, 18 Nov 2024 21:48:23 +0000 (22:48 +0100)] 
DOC: config: Slightly improve the %Tr documentation

Specify -1 can also be reported for %Tr delay when the response is invalid.

7 months agoBUG/MINOR: http_ana: Report -1 for %Tr for invalid response only
Christopher Faulet [Mon, 18 Nov 2024 21:37:52 +0000 (22:37 +0100)] 
BUG/MINOR: http_ana: Report -1 for %Tr for invalid response only

The server response time is erroneously reported as -1 when it is
intercepted by HAProxy.

As stated in the documentation, the server response time is reported as -1
when the last response header was never seen. It happens when a server
timeout is triggered before the server managed to process the request. It
also happens if the response is invalid. This may be reported by the mux
during the response parsing, but also by the HTTP analyzers. However, in
this last case, the response time must only be reported as -1 on 502.

This patch must be backported to all stable versions. It should fix the
issue #2384.

7 months agoMINIR: mux-h1: Return 414 or 431 when appropriate
Christopher Faulet [Mon, 18 Nov 2024 21:19:00 +0000 (22:19 +0100)] 
MINIR: mux-h1: Return 414 or 431 when appropriate

When the request is too large to fit in a buffer a 414 or a 431 error
message is returned depending on the error state of the request parser. A
414 is returned if the URI is too long, otherwise a 431 is returned.

This patch should fix the issue #1309.

7 months agoDEV: phash: Update 414 and 431 status codes to phash
Christopher Faulet [Mon, 18 Nov 2024 21:12:28 +0000 (22:12 +0100)] 
DEV: phash: Update 414 and 431 status codes to phash

The phash tool was updated to reflect the previous change. 414 and 431 are
now part of the handled status codes.

7 months agoMINOR: http: Add support for HTTP 414/431 status codes
Christopher Faulet [Mon, 18 Nov 2024 21:06:17 +0000 (22:06 +0100)] 
MINOR: http: Add support for HTTP 414/431 status codes

414-Uri-Too-Long and 431-Request-Header-Fields-Too-Large are now part of
supported status codes that can be define as error files. The hash table
defined in http_get_status_idx() was updated accordingly.

7 months agoDOC: config: Fix a typo in "1.3.1. The Request line"
Christopher Faulet [Mon, 18 Nov 2024 17:11:04 +0000 (18:11 +0100)] 
DOC: config: Fix a typo in "1.3.1. The Request line"

At the beginning of the last paragraph of this section, HTTP/3 was used
instead of HTTP/2. It is not fixed.

7 months agoDOC: config: A a space before ':' for {bs,fs}.aborted and {bs,fs}.rst_code
Christopher Faulet [Mon, 18 Nov 2024 14:34:54 +0000 (15:34 +0100)] 
DOC: config: A a space before ':' for {bs,fs}.aborted and {bs,fs}.rst_code

A space was missing before the ':' for the sample fetch functions above. It
was an issue for the text to HTML conversion script. So, let's fix it.

7 months agoMINOR: stream: Add an option to "show sess" command to dump the captured URI
Christopher Faulet [Mon, 18 Nov 2024 14:17:13 +0000 (15:17 +0100)] 
MINOR: stream: Add an option to "show sess" command to dump the captured URI

"show sess" command now supports a list of options that can be set after all
other possible arguments (<id>, all...). For now, "show-uri" is the only
supported option. With this options, the captured URI, if non-null, is added
to the dump of a stream, complete or now. The URI may be anonymized if
necessary.

This patch should fix the issue #663.

7 months agoMINOR: agent-check: Be able to set absolute weight via an agent
Christopher Faulet [Mon, 18 Nov 2024 07:35:22 +0000 (08:35 +0100)] 
MINOR: agent-check: Be able to set absolute weight via an agent

Historically, an agent-check program is only able to set a proportial weight
to the initial server's weight. However, it could be handy to also set an
absolute value. It is the purpose of this patch.

Instead of changing the current way to set a server's weight, a new
agent-check command is introduced. The string "weight:", followed by an
positive interger or a positive interger percentage, can now be used. If the
value ends with the '%' sign, then the new weight will be proportional to
the initially weight of the server. Otherwise, the value is considered as an
absolute weight and must be between 0 and 256.

This patch should fix the issue #360.

7 months agoMINOR: http-ana: Add support for "set-cookie-fmt" option to redirect rules
Christopher Faulet [Mon, 18 Nov 2024 17:03:23 +0000 (18:03 +0100)] 
MINOR: http-ana: Add support for "set-cookie-fmt" option to redirect rules

It is now possible to use a log-format string to define the "Set-Cookie"
header value of a response generated by a redirect rule. There is no special
check on the result format and it is not possible during the configuration
parsing. It is proably not a big deal because already existing "set-cookie"
and "clear-cookie" options don't perform any check.

Here is an example:

  http-request redirect location https://someurl.com/ set-cookie haproxy="%[var(txn.var)]"

This patch should fix the issue #1784.

7 months agoMINOR: http-ana: Add option to keep query-string on a localtion-based redirect
Christopher Faulet [Fri, 15 Nov 2024 16:03:06 +0000 (17:03 +0100)] 
MINOR: http-ana: Add option to keep query-string on a localtion-based redirect

On prefix-based redirect, there is an option to drop the query-string of the
location. Here it is the opposite. an option is added to preserve the
query-string of the original URI for a localtion-based redirect.

By setting "keep-query" option, for a location-based redirect only, the
query-string of the original URI is appended to the location. If there is no
query-string, nothing is added (no empty '?'). If there is already a
non-empty query-string on the localtion, the original one is appended with
'&' separator.

This patch should fix issue #2728.

7 months agoMINOR: config: show HAPROXY_BRANCH in "show env" output
Valentine Krasnobaeva [Mon, 18 Nov 2024 16:54:25 +0000 (17:54 +0100)] 
MINOR: config: show HAPROXY_BRANCH in "show env" output

Before this patch HAPROXY_BRANCH was unset just after configuration parsing.
Let's keep it, as it could be used in conditional blocks and some
configuration directives and it's handy to check its runtime value via "show
env".

In master-worker mode, this variable is set to the same value for both
processes.

7 months agoMINOR: cli: make "show env" accessible via master CLI without enabling debug
Valentine Krasnobaeva [Mon, 18 Nov 2024 09:46:33 +0000 (10:46 +0100)] 
MINOR: cli: make "show env" accessible via master CLI without enabling debug

Before this patch, we have need to put the master CLI in debug mode to be able
to issue 'show env' command for the master process. Output of this command is
handy even for the master process context, as it allows to control its
environment variables, which could be used/modified in the 'global' section.

So, let's provide in 'show env' command structure the level ACCESS_MASTER.
This allows to see and to access this command in master CLI without putting it
in debug mode.

7 months agoBUG/MINOR: mworker-prog: don't warn about deprecated section with expose-deprecated...
Valentine Krasnobaeva [Tue, 19 Nov 2024 09:32:28 +0000 (10:32 +0100)] 
BUG/MINOR: mworker-prog: don't warn about deprecated section with expose-deprecated-directives

As master parses now expose-deprecated-directives option, let's emit warning
about deprecated 'progam' section only in case, if this option wasn't set in
the 'global' section. This allows to people, who don't prefer to remove the
'program' section immediately to continue to start the process in zero-warning
mode.

Adjust the warning message accordingly and mcli_start_progs.vtc test. As
expose-deprecated-directives option is a 'global' section keyword, this section
must always precede any 'program' section, if users still continue to keep
'program' section.

This doesn't need to be backported, as related to the latest changes in
the master-worker architecture.

7 months agoMINOR: cfgparse-global: parse options to allow non std keywords in discovery mode
Valentine Krasnobaeva [Tue, 19 Nov 2024 09:47:38 +0000 (10:47 +0100)] 
MINOR: cfgparse-global: parse options to allow non std keywords in discovery mode

'Program' section is considered as deprecated now, see the commit 581c8a27d98c
("MEDIUM: mworker: depreciate the 'program' section"). So, the 'program'
section parser emits a warning every time since this commit, if its section is
presented. This makes impossible to launch the process in zero-warning mode.

After master-worker refactoring only the master process parses the 'program'
section. So, at first, in order to be able to start in zero-warning mode, we
need to parse in master process option, which allows deprecated keywords. Thus,
let's set in this commit KWF_DISCOVERY flag to
cfg_parse_global_non_std_directives parser, which parses
'expose-deprecated-directives' and 'expose-deprecated-directives' options.

7 months agoMINOR: ring: support unit suffixes in the size
Willy Tarreau [Tue, 19 Nov 2024 09:56:45 +0000 (10:56 +0100)] 
MINOR: ring: support unit suffixes in the size

The ring size used to take only numbers and silently ignore letters (due
to atol()), resulting it tiny buffers when trying to collect traces and
using e.g. "size 10g". Let's make use of parse_size_err() to properly
parse units.

7 months agoMINOR: tools: make parse_size_err() support 32/64 bits
Willy Tarreau [Tue, 19 Nov 2024 08:15:19 +0000 (09:15 +0100)] 
MINOR: tools: make parse_size_err() support 32/64 bits

parse_size_err() currently is a function working only on an uint. It's
not convenient for certain elements such as rings on large machines.

This commit addresses this by having one function for uints and one
for ullong, and making parse_size_err() a macro that automatically
calls one or the other. It also has the benefit of automatically
supporting compatible types (long, size_t etc).

7 months agoMEDIUM: config: warn on unitless timeouts < 100 ms
Willy Tarreau [Mon, 18 Nov 2024 18:47:59 +0000 (19:47 +0100)] 
MEDIUM: config: warn on unitless timeouts < 100 ms

From time to time we face a configuration with very small timeouts which
look accidental because there could be expectations that they're expressed
in seconds and not milliseconds.

This commit adds a check for non-nul unitless values smaller than 100
and emits a warning suggesting to append an explicit unit if that was
the intent.

Only the common timeouts, the server check intervals and the resolvers
hold and timeout values were covered for now. All the code needs to be
manually reviewed to verify if it supports emitting warnings.

This may break some configs using "zero-warning", but greps in existing
configs indicate that these are extremely rare and solely intentionally
done during tests. At least even if a user leaves that after a test, it
will be more obvious when reading 10ms that something's probably not
correct.

7 months agoREGTESTS: enable -dW on almost all tests to fail on warnings
Willy Tarreau [Tue, 19 Nov 2024 06:11:02 +0000 (07:11 +0100)] 
REGTESTS: enable -dW on almost all tests to fail on warnings

Now that warnings were almost all removed, let's enable zero-warning
via -dW. All tests were adjusted, but two:

  - mcli/mcli_start_progs.vtc:
      the programs section currently cannot be silenced

  - stats/stats-file.vtc:
      the warning comes from the stats file itself on comment lines.

All other ones are now OK.

7 months agoREGTESTS: only use tune.ssl.default-dh-param when not using AWS-LC
Willy Tarreau [Tue, 19 Nov 2024 08:26:12 +0000 (09:26 +0100)] 
REGTESTS: only use tune.ssl.default-dh-param when not using AWS-LC

This option is not available with AWS-LC and emits a warning, so let's
properly enclose the test to cover this special case.

7 months agoREGTESTS: add missing timeouts to 30 tests
Willy Tarreau [Tue, 19 Nov 2024 06:43:04 +0000 (07:43 +0100)] 
REGTESTS: add missing timeouts to 30 tests

No less than 30 tests were missing timeouts, preventing them from being
started with zero-warning. Since they were not supposed to trigger, they
have been set to 30s so as never to trigger, and now they do not produce
any warning anymore.

7 months agoREGTESTS: silence warning "L6 sample fetches ignored" in cond_set_var
Willy Tarreau [Tue, 19 Nov 2024 07:27:47 +0000 (08:27 +0100)] 
REGTESTS: silence warning "L6 sample fetches ignored" in cond_set_var

This reg-test uses req.len in an HTTP backend. It does work but emits a
warning suggesting that this is ignored, so most likely its days are
counted now. Let's just use req.hdrs,length instead.

7 months agoREGTESTS: remove a duplicate "option httpslog" in the defaults section
Willy Tarreau [Tue, 19 Nov 2024 07:05:27 +0000 (08:05 +0100)] 
REGTESTS: remove a duplicate "option httpslog" in the defaults section

This triggers the following warning:

  'option httpslog' overrides previous 'option httpslog' in 'defaults' section.

7 months agoREGTESTS: silence warnings about content-type being ignored
Willy Tarreau [Tue, 19 Nov 2024 06:59:21 +0000 (07:59 +0100)] 
REGTESTS: silence warnings about content-type being ignored

The following rules are triggering warnings about content-type being
ignored:

  http-request return content-type "text/plain" if { path /def-4 }
  http-request return content-type "text/plain" file /dev/null hdr "x-custom-hdr" "%[url]"  if { path /empty-file }

Annoyingly, the content-type is mandatory when the file is not empty,
that might be something to revisit in the future to relax at least one
of the rules so that the config doesn't strictly require to know the
file contents upfront.

7 months agoREGTESTS: make the unit explicit for very short timeouts
Willy Tarreau [Tue, 19 Nov 2024 06:50:54 +0000 (07:50 +0100)] 
REGTESTS: make the unit explicit for very short timeouts

Two tests were using "timeout {client,server} 1" to forcefully trigger
them, but a forthcoming patch will emit a warning for such small unitless
values, so let's be explicit about the unit.

7 months agoREGTESTS: silence warning "previous 'http-response' action is final"
Willy Tarreau [Tue, 19 Nov 2024 06:48:01 +0000 (07:48 +0100)] 
REGTESTS: silence warning "previous 'http-response' action is final"

The regtest "h1or2_to_h1c" contains both an allow and a deny at the end,
likely to help catch rare bugs. But this triggers a warning that we can
silence by placing a condition on the penultimate rule.

7 months agoREGTESTS: silence the "log format ignored" warnings
Willy Tarreau [Tue, 19 Nov 2024 06:46:26 +0000 (07:46 +0100)] 
REGTESTS: silence the "log format ignored" warnings

Several tests were declaring a log format without having an explicit
log server configured, causing a warning. Let's clean them up.

7 months agoMINOR: cfgparse: parse tune.bufsize.small as a size
Willy Tarreau [Mon, 18 Nov 2024 18:07:05 +0000 (19:07 +0100)] 
MINOR: cfgparse: parse tune.bufsize.small as a size

Till now this value was parsed as raw integer using atol() and would
silently ignore any trailing suffix, causing unexpected behaviors when
set, e.g. to "4k". Let's make use of parse_size_err() on it so that
units are supported. This requires to turn it to uint as well, which
was verified to be OK.

7 months agoMINOR: cfgparse: parse tune.bufsize as a size
Willy Tarreau [Mon, 18 Nov 2024 18:04:57 +0000 (19:04 +0100)] 
MINOR: cfgparse: parse tune.bufsize as a size

Till now this value was parsed as raw integer using atol() and would
silently ignore any trailing suffix, preventing from starting when set
e.g. to "64k". Let's make use of parse_size_err() on it so that units are
supported. This requires to turn it to uint as well, and to explicitly
limit its range to INT_MAX - 2*sizeof(void*), which was previously
partially handled as part of the sign check.

7 months agoMINOR: cfgparse: parse tune.recv_enough as a size
Willy Tarreau [Mon, 18 Nov 2024 18:01:28 +0000 (19:01 +0100)] 
MINOR: cfgparse: parse tune.recv_enough as a size

Till now this value was parsed as raw integer using atol() and would
silently ignore any trailing suffix, causing unexpected behaviors when
set, e.g. to "512k". Let's make use of parse_size_err() on it so that
units are supported. This requires to turn it to uint as well, and
since it's sometimes compared to an int, we limit its range to
0..INT_MAX.

7 months agoMINOR: cfgparse: parse tune.pipesize as a size
Willy Tarreau [Mon, 18 Nov 2024 17:51:31 +0000 (18:51 +0100)] 
MINOR: cfgparse: parse tune.pipesize as a size

Till now this value was parsed as raw integer using atol() and would
silently ignore any trailing suffix, causing unexpected behaviors when
set, e.g. to "512k". Let's make use of parse_size_err() on it so that
units are supported. This requires to turn it to uint as well, which
was verified to be OK.

7 months agoMINOR: cfgparse: parse tune.{rcvbuf,sndbuf}.{frontend,backend} as sizes
Willy Tarreau [Mon, 18 Nov 2024 17:50:02 +0000 (18:50 +0100)] 
MINOR: cfgparse: parse tune.{rcvbuf,sndbuf}.{frontend,backend} as sizes

Till now these values were parsed as raw integer using atol() and would
silently ignore any trailing suffix, causing unexpected behaviors when
set, e.g. to "512k". Let's make use of parse_size_err() on them so that
units are supported. This requires to turn them to uint as well, which
is OK.

7 months agoMINOR: cfgparse: parse tune.{rcvbuf,sndbuf}.{client,server} as sizes
Willy Tarreau [Mon, 18 Nov 2024 17:45:45 +0000 (18:45 +0100)] 
MINOR: cfgparse: parse tune.{rcvbuf,sndbuf}.{client,server} as sizes

Till now these values were parsed as raw integer using atol() and would
silently ignore any trailing suffix, causing unexpected behaviors when
set, e.g. to "512k". Let's make use of parse_size_err() on them so that
units are supported. This requires to turn them to uint as well, which
is OK.

7 months agoMINOR: sample: extend the "when" converter to support an ACL
Willy Tarreau [Mon, 18 Nov 2024 14:27:28 +0000 (15:27 +0100)] 
MINOR: sample: extend the "when" converter to support an ACL

Sometimes conditions to decide of an anomaly are not as easy to define
as just an error or a success. One example use case would be to monitor
the transfer time and fix a threshold.

An idea suggested by Tristan would be to make permit the "when"
converter to refer to a more variable or dynamic condition.

Here we make this possible by making "when" rely on a named ACL. The
ACL then needs to be specified in either the proxy or the defaults
section. Since it is evaluated inline, it may even refer to information
available at the end (at log time) such as the data transfer time. If
the ACL evalutates to true, the converter passes the data.

Example: log "dbg={-}" when fine, or "dbg={... debug info ...}" on slow
transfers:

  acl slow_xfer res.timer.data ge 10000   # more than 10s is slow
  log-format "$HAPROXY_HTTP_LOG_FMT                                \
              fsdbg={%[fs.debug_str,when(acl,slow_xfer)]}          \
              bsdbg={%[bs.debug_str,when(acl,slow_xfer)]}"

7 months agoMINOR: acl: export find_acl_default()
Willy Tarreau [Mon, 18 Nov 2024 14:15:54 +0000 (15:15 +0100)] 
MINOR: acl: export find_acl_default()

It will be needed in a future patch, so let's export it (it was static).

7 months ago[RELEASE] Released version 3.1-dev13 v3.1-dev13
Willy Tarreau [Fri, 15 Nov 2024 17:42:29 +0000 (18:42 +0100)] 
[RELEASE] Released version 3.1-dev13

Released version 3.1-dev13 with the following main changes :
    - MEDIUM: mworker: depreciate the 'program' section
    - BUILD: ot: use a cebtree instead of a list for variable names
    - MINOR: startup: replace HAPROXY_LOAD_SUCCESS with global load_status
    - BUG/MINOR: startup: set HAPROXY_CFGFILES in read_cfg
    - BUG/MINOR: cli: don't show sockpairs in HAPROXY_CLI and HAPROXY_MASTER_CLI
    - BUG/MEDIUM: stconn: Don't forward shut for SC in connecting state
    - BUG/MEDIUM: resolvers: Insert a non-executed resulution in front of the wait list
    - MINOR: debug: explicitly permit the counter condition to be empty
    - MINOR: debug: add a new counter type for glitches
    - MINOR: mux-h2: count glitches when they're reported
    - BUG/MINOR: deinit: release uri_auth admin rules
    - MINOR: uri_auth: add stats_uri_auth_free helper
    - MEDIUM: uri_auth: implement clean uri_auth cleaning
    - MINOR: mux-quic/h3: count glitches when they're reported
    - BUG/MEDIUM: mux-h2: Don't send RST_STREAM frame for streams with no ID
    - BUG/MINOR: Don't report early srv aborts on request forwarding in DONE state
    - MINOR: promex: Expose the global node and description in process metrics
    - MINOR: promex: Add global and proxies description as labels to all metrics
    - OPTIM: pattern: only apply LRU cache for large enough lists
    - BUG/MEDIUM: checks: make sure to always apply offsets to now_ms in expiration
    - BUG/MINOR: debug: do not set task expiration to TICK_ETERNITY
    - BUG/MEDIUM: mailers: make sure to always apply offsets to now_ms in expiration
    - BUG/MINOR: mux_quic: make sure to always apply offsets to now_ms in expiration
    - BUG/MINOR: peers: make sure to always apply offsets to now_ms in expiration
    - BUG/MEDIUM: clock: make sure now_ms cannot be TICK_ETERNITY
    - MINOR: debug/cli: replace "debug dev counters" with "debug counters"
    - DOC: config: add tune.h2.{be,fe}.rxbuf to the global keywords index
    - MINOR: chunk: add a BUG_ON upon the next init_trash_buffer()

7 months agoMINOR: chunk: add a BUG_ON upon the next init_trash_buffer()
William Lallemand [Fri, 15 Nov 2024 16:15:06 +0000 (17:15 +0100)] 
MINOR: chunk: add a BUG_ON upon the next init_trash_buffer()

The trash pool is initialized twice in haproxy, first during STG_POOL,
and 2nd after configuration parsing.

Doing alloc_trash_chunk() between this 2 phases can lead to strange
things if we are using it after, indeed the pool is destroyed and
trying to do a free_trash_chunk() or accessing the pointer will lead to
crashes.

This patch checks that we don't have used buffers from the trash pool
before initializing the pool again.

7 months agoDOC: config: add tune.h2.{be,fe}.rxbuf to the global keywords index
Willy Tarreau [Fri, 15 Nov 2024 15:32:37 +0000 (16:32 +0100)] 
DOC: config: add tune.h2.{be,fe}.rxbuf to the global keywords index

These two keywords were missing from the index, let's add them.

7 months agoMINOR: debug/cli: replace "debug dev counters" with "debug counters"
Willy Tarreau [Fri, 15 Nov 2024 15:19:05 +0000 (16:19 +0100)] 
MINOR: debug/cli: replace "debug dev counters" with "debug counters"

"debug dev" commands are not meant to be used by end-users, and are
purposely not documented. Yet due to their usefulness in troubleshooting
sessions, users are increasingly invited by developers to use some of
them.

"debug dev counters" is one of them. Better move it to "debug counters"
and document it so that users can check them even if the output can look
cryptic at times. This, combined with DEBUG_GLITCHES, can be convenient
to observe suspcious activity. The doc however precises that the format
may change between versions and that new entries/types might appear
within a stable branch.

7 months agoBUG/MEDIUM: clock: make sure now_ms cannot be TICK_ETERNITY
Willy Tarreau [Fri, 15 Nov 2024 14:52:17 +0000 (15:52 +0100)] 
BUG/MEDIUM: clock: make sure now_ms cannot be TICK_ETERNITY

In clock ticks, 0 is TICK_ETERNITY. Long ago we used to make sure now_ms
couldn't be zero so that it could be assigned to expiration timers, but
it has long changed after functions like tick_add() were instrumented to
make the check. The problem is that aside the rare few accidental direct
assignments to expiration dates, it's also used to mark the beginning of
an event that's later checked against TICK_ETERNITY to know if it has
already struck. The problem in this case is that certain events may just
be replaced or dropped just because they apparently never appeared. It's
probably the case for stconn's "lra" and "fsb" fields, just like it is
for all those involving tick_add_ifset(), like h2c->idle_start.

The right approach would be to change the type of now_ms to something
else that cannot take direct computations and that represents a timestamp,
forcing to always use the conversion functions. The variables holding such
timestamps would also be distinguished from intervals. At first glance we
could have for timestamps:
  - 0 = never happened (for the past), eternity (for the future)
  - X = date
and for intervals:
  - 0 = not set
  - X = interval

However this requires significant changes. Instead for now, let's just
make sure again that now_ms is never 0 by setting it to 1 when this
happens (1 / 4 billion times, or 1ms every 49.7 days).

This will need to be carefully backported to older versions. Note that
with this patch backported, the previous ones fixing the zero date are
not strictly needed.

7 months agoBUG/MINOR: peers: make sure to always apply offsets to now_ms in expiration
Willy Tarreau [Fri, 15 Nov 2024 14:44:05 +0000 (15:44 +0100)] 
BUG/MINOR: peers: make sure to always apply offsets to now_ms in expiration

Now_ms can be zero nowadays, so it's not suitable for direct assignment to
t->expire, as there's a risk that the timer never wakes up once assigned
(TICK_ETERNITY). Let's use tick_add(now_ms, 0) for an immediate wakeup
instead. The impact here might be a reconnect programmed upon signal
receipt at the wrapping date not having a working timeout.

This should be backported where it applies.

7 months agoBUG/MINOR: mux_quic: make sure to always apply offsets to now_ms in expiration
Willy Tarreau [Fri, 15 Nov 2024 14:41:21 +0000 (15:41 +0100)] 
BUG/MINOR: mux_quic: make sure to always apply offsets to now_ms in expiration

Now_ms can be zero nowadays, so it's not suitable for direct assignment to
t->expire, as there's a risk that the timer never wakes up once assigned
(TICK_ETERNITY). Let's use tick_add(now_ms, 0) for an immediate wakeup
instead. The impact looks nul since the task is also woken up, but better
not leave such tasks in the timer tree anyway.

This should be backported where it applies.

7 months agoBUG/MEDIUM: mailers: make sure to always apply offsets to now_ms in expiration
Willy Tarreau [Fri, 15 Nov 2024 14:39:58 +0000 (15:39 +0100)] 
BUG/MEDIUM: mailers: make sure to always apply offsets to now_ms in expiration

Now_ms can be zero nowadays, so it's not suitable for direct assignment to
t->expire, as there's a risk that the timer never wakes up once assigned
(TICK_ETERNITY). Let's use tick_add(now_ms, 0) for an immediate wakeup
instead. The impact here might be mailers suddenly stopping.

This should be backported where it applies.

7 months agoBUG/MINOR: debug: do not set task expiration to TICK_ETERNITY
Willy Tarreau [Fri, 15 Nov 2024 14:37:38 +0000 (15:37 +0100)] 
BUG/MINOR: debug: do not set task expiration to TICK_ETERNITY

Using "debug task", it's possible to change a task's expiration, but
we must be careful not to set it to TICK_ETERNITY. Let's use tick_add()
instead. The risk is basically nul since it's a debugging command, so
no backport is needed.

7 months agoBUG/MEDIUM: checks: make sure to always apply offsets to now_ms in expiration
Willy Tarreau [Fri, 15 Nov 2024 14:34:46 +0000 (15:34 +0100)] 
BUG/MEDIUM: checks: make sure to always apply offsets to now_ms in expiration

Now_ms can be zero nowadays, so it's not suitable for direct assignment to
t->expire, as there's a risk that the timer never wakes up once assigned
(TICK_ETERNITY). Let's use tick_add(now_ms, 0) for an immediate wakeup
instead. The impact here might be health checks suddenly stopping.

This should be backported where it applies.

7 months agoOPTIM: pattern: only apply LRU cache for large enough lists
Willy Tarreau [Sun, 3 Nov 2024 17:42:26 +0000 (18:42 +0100)] 
OPTIM: pattern: only apply LRU cache for large enough lists

As shown in issue #1518, the LRU cache has a non-null cost that can
sometimes be above the match cost it's trying to avoid. After a number
of tests, it appears that:
  - "simple" match operations (sub, beg, end, int etc) reach a break-even
    after ~20 patterns in list
  - "heavy" match operations (reg) reach a break-even after ~5 patterns in
    list

Let's only consult the LRU cache when the number of patterns in the
expression is at least as large as this limit. Of course there will
always be outliers but it already starts good.

Another improvement consists in reducing the cache size to further
speed up lookups, which makes sense if less expressions use the cache.

7 months agoMINOR: promex: Add global and proxies description as labels to all metrics
Christopher Faulet [Fri, 15 Nov 2024 13:15:43 +0000 (14:15 +0100)] 
MINOR: promex: Add global and proxies description as labels to all metrics

While the global description is exposed, when defined, in a dedicated
metric, it is not possible to dump the description defined in a
frontend/listen/backend sections. So, thanks to this patch, it is now
possible to dump it as a label of all metrics of the corresponding
section. To do so, "desc-labels" parameter must be provided on the URL:

    /metrics?desc-labels

When this parameter is set, if a description is provided in a section,
including the global one, the "desc" label will be added to all metrics of
this section. For instance:

  haproxy_frontend_current_sessions{proxy="front-http",desc="..."} 1

Note that servers metrics inherit the description of their backend/listen
section.

This patch should solve the issue #1531.

7 months agoMINOR: promex: Expose the global node and description in process metrics
Christopher Faulet [Fri, 15 Nov 2024 13:09:15 +0000 (14:09 +0100)] 
MINOR: promex: Expose the global node and description in process metrics

The global node value is now exposed via "haproxy_process_node" metrics. The
metric value is always set to 1 and the node name itself is the "node"
label. The same is performed for the global description. But only if it is
defined. In that case "haproxy_process_description" metric is defined, with
1 as value and the description itself is set in the "desc" label.

7 months agoBUG/MINOR: Don't report early srv aborts on request forwarding in DONE state
Christopher Faulet [Fri, 15 Nov 2024 09:51:18 +0000 (10:51 +0100)] 
BUG/MINOR: Don't report early srv aborts on request forwarding in DONE state

L7-retries may be ignored if server aborts are detected during the request
forwarding, when the request is already in DONE state.

When a request was fully processed (so in HTTP_MSG_DONE state) and is
waiting for be forwarded to the server, there is a test to detect server
aborts, to be able to report the error. However, this test must be skipped
if the response was not received yet, to let the reponse analyszers handle
the abort. It is important to properly handle the retries. This test must
only be performed if the response analysis was finished. It means the
response must be at least in HTTP_MSG_BODY state.

This patch should be backported as far as 2.8.

7 months agoBUG/MEDIUM: mux-h2: Don't send RST_STREAM frame for streams with no ID
Christopher Faulet [Fri, 15 Nov 2024 09:25:20 +0000 (10:25 +0100)] 
BUG/MEDIUM: mux-h2: Don't send RST_STREAM frame for streams with no ID

On server side, the H2 stream is first created with an unassigned ID (ID ==
0). Its ID is assigned when the request is emitted, before formatting the
HEADERS frame. However, the session may be aborted during that stage. We
must take care to not emit RST_STREAM frame for this stream, because it does
not exist yet for the server.

It is especially important to do so because, depending on the timing, it may
also happens before the H2 PREFACE was sent.

This patch must be backported to all stable versions. It is related to issue

7 months agoMINOR: mux-quic/h3: count glitches when they're reported
Willy Tarreau [Thu, 14 Nov 2024 14:38:25 +0000 (15:38 +0100)] 
MINOR: mux-quic/h3: count glitches when they're reported

The qcc_report_glitch() function is now replaced with a macro to support
enumerating counters for each individual glitch line. For now this adds
36 such counters. The macro supports an optional description, though that
is not being used for now.

As a reminder, this requires to build with -DDEBUG_GLITCHES=1.

7 months agoMEDIUM: uri_auth: implement clean uri_auth cleaning
Aurelien DARRAGON [Wed, 13 Nov 2024 18:54:32 +0000 (19:54 +0100)] 
MEDIUM: uri_auth: implement clean uri_auth cleaning

proxy auth_uri struct was manually cleaned up during deinit, but the logic
behind was kind of akward because it was required to find out which ones
were shared or not. Instead, let's switch to a proper refcount mechanism
and free the auth_uri struct directly in proxy_free_common().

7 months agoMINOR: uri_auth: add stats_uri_auth_free helper
Aurelien DARRAGON [Wed, 13 Nov 2024 18:14:10 +0000 (19:14 +0100)] 
MINOR: uri_auth: add stats_uri_auth_free helper

Let's now leverage stats_uri_auth_free() helper to free uri_auth struct
instead of manually performing the cleanup, which is error-prone.

7 months agoBUG/MINOR: deinit: release uri_auth admin rules
Aurelien DARRAGON [Wed, 13 Nov 2024 17:26:34 +0000 (18:26 +0100)] 
BUG/MINOR: deinit: release uri_auth admin rules

When uri_auth admin rules were implemented in 474be415
("[MEDIUM] stats: add an admin level") no attempt was made to free the
list of allocated rules, which makes valgrind unhappy upon deinit when
"stats admin" is used in the config.

To fix the issue, let's cleanup the admin rules list upon deinit where
uri_auth freeing is already handled.

While this could be backported to every stable versions, given how minor
this is and has no impact on the dying process, it is probably not worth
the effort.

7 months agoMINOR: mux-h2: count glitches when they're reported
Willy Tarreau [Thu, 14 Nov 2024 07:54:32 +0000 (08:54 +0100)] 
MINOR: mux-h2: count glitches when they're reported

The h2c_report_glitch() function is now replaced with a macro to support
enumerating counters for each individual glitch line. For now this adds
43 such counters. The macro supports an optional description, though that
is not being used for now. It gives outputs like this (note that the last
one was purposely instrumented to pass a description):

   > debug dev counters glt all
   0          GLT mux_h2.c:5976 h2c_dec_hdrs()
   0          GLT mux_h2.c:5960 h2c_dec_hdrs()
   (...)
   0          GLT mux_h2.c:2207 h2c_frt_recv_preface()
   0          GLT mux_h2.c:1954 h2c_frt_stream_new(): new stream too early

As a reminder, this requires to build with -DDEBUG_GLITCHES=1.

7 months agoMINOR: debug: add a new counter type for glitches
Willy Tarreau [Thu, 14 Nov 2024 07:49:38 +0000 (08:49 +0100)] 
MINOR: debug: add a new counter type for glitches

COUNT_GLITCH() will implement an unconditional counter on its declaration
line when DEBUG_GLITCHES is set, and do nothing otherwise. The output will
be reported as "GLT" and can be filtered as "glt" on the CLI. The purpose
is to help figure what's happening if some glitches counters start going
through the roof. The macro supports an optional string argument to
describe the cause of the glitch (e.g. "truncated header"), which is then
reported in the dump.

For now this is conditioned by DEBUG_GLITCHES but if it turns out to be
light enough, maybe we'll keep it enabled full time. In this case it
might have to be moved away from debug dev, or at least documented (or
done as debug counters maybe so that dev can remain undocumented and
updatable within a branch?).

7 months agoMINOR: debug: explicitly permit the counter condition to be empty
Willy Tarreau [Thu, 14 Nov 2024 07:47:00 +0000 (08:47 +0100)] 
MINOR: debug: explicitly permit the counter condition to be empty

In order to count new event types, we'll need to support empty conditions
so that we don't have to fake if (1) that would pollute the output. This
change checks if #cond is an empty string before concatenating it with
the optional var args, and avoids dumping the colon on the dump if the
whole description is empty.

7 months agoBUG/MEDIUM: resolvers: Insert a non-executed resulution in front of the wait list
Christopher Faulet [Tue, 12 Nov 2024 17:51:20 +0000 (18:51 +0100)] 
BUG/MEDIUM: resolvers: Insert a non-executed resulution in front of the wait list

When a resolver is woken up to process DNS resolutions, it is possible to
trigger an infinite loop on the resolver's wait list because delayed
resolutions are always reinserted at the end of this list. This leads the
watchdog to kill the process. By re-inserting them in front of the list,
that fixes the bug.

When a resolver tries to send the queries for the resolutions in its wait
list, it may be unable to proceed for a resolution. This may happen because
the resolution must be skipped (no hostname to resolv, a resolution already
in-progress) or when an error occurred. In that case, the resolution is
re-inserted in the resolver's wait list to be retry later, on a next wakeup.

However, the resolution is inserted at the end of the wait list. So it is
immediately reevaluated, in the same execution loop, instead of to be
delayed. Most of time, it is not an issue because the resolution is
considered as not expired on the second run. But it is an problem when the
internal time wraps and is equal to 0. In that case, the resolution
expiration date is badly computed and it is always considered as expired. If
two or more resolutions are in that state, the resolver loops for ever on
its wait list, until the process is killed by the watchdog.

So we can argue that the way the resolution expiration date is computed must
be fixed. And it would be true in a perfect world. However, the resolvers
code is so crapy that it is hard to be sure to not introduce regressions. It
is farly easier to re-insert delayed resolutions in front of the wait
list. This fixes the issue and at worst, these resolutions will be evaluated
one time too many on the next wakeup and only if now_ms was equal to 0 on
the prior wakeup.

This patch should be backported to all stable versions. On 2.2, LIST_ADD()
must be used instead of LIST_INSERT()

7 months agoBUG/MEDIUM: stconn: Don't forward shut for SC in connecting state
Christopher Faulet [Wed, 30 Oct 2024 15:41:39 +0000 (16:41 +0100)] 
BUG/MEDIUM: stconn: Don't forward shut for SC in connecting state

In connecting state, shutdown must not be forwarded or scheduled because
otherwise this will prevent any connection retries. Indeed, if a EOS is
reported by the mux during the connection establishment, this should be
handled by the stream to eventually retries. If the write side is closed
first, this will not be possible because the stconn will be switched in DIS
state. If the shut is scheduled because pending data are blocked, the same
may happen, depending on the abort-on-close option.

This patch should be slowly be backported as far as 2.4. But an observation
period is mandatory. On 2.4, the patch must be adapted to use the
stream-interface API.

7 months agoBUG/MINOR: cli: don't show sockpairs in HAPROXY_CLI and HAPROXY_MASTER_CLI
Valentine Krasnobaeva [Tue, 12 Nov 2024 21:43:49 +0000 (22:43 +0100)] 
BUG/MINOR: cli: don't show sockpairs in HAPROXY_CLI and HAPROXY_MASTER_CLI

Before this fix, HAPROXY_CLI and HAPROXY_MASTER_CLI have contained along with
CLI sockets addresses internal sockpairs, which are used only for master CLI
(reload sockpair and sockpair shared with a worker process). These internal
sockpairs are always need to be hidden.

At the moment there is no any client, who uses sockpair addresses for the
stats listener or in order to connect to master CLI. So, let's simply not copy
these internal sockpair addresses of MASTER and GLOBAL proxy listeners.

As listeners with sockpairs are skipped and they can be presented in the
listeners list in any order, let's add semicolon separator between addresses
only in the case, when there are already some string saved in the trash and we
are sure, that we are adding a new address to it. Otherwise, we could have such
weird output:

HAPROXY_MASTER_CLI=unix@/tmp/mcli.sock;;

This fix is need to be backported in all stable versions.

7 months agoBUG/MINOR: startup: set HAPROXY_CFGFILES in read_cfg
Valentine Krasnobaeva [Tue, 12 Nov 2024 21:43:34 +0000 (22:43 +0100)] 
BUG/MINOR: startup: set HAPROXY_CFGFILES in read_cfg

load_cfg() is called only once before the first reading of the configuration
(we parse here only the global section). Then, before reading the rest of the
sections (second call of read_cfg()), we call clean_env(). As
HAPROXY_CFGFILES is set in load_cfg(), which is called only once, clean_env()
erases it. Thus, it's not longer shown in "show env" output.

To fix this, let's set HAPROXY_CFGFILES in read_cfg(). Like this in
master-worker mode it is set for master and for worker processes, as it was
before the refactoring.

This fix doesn't need to be backported as related to the latest master-worker
architecture change.

7 months agoMINOR: startup: replace HAPROXY_LOAD_SUCCESS with global load_status
Valentine Krasnobaeva [Tue, 12 Nov 2024 10:28:46 +0000 (11:28 +0100)] 
MINOR: startup: replace HAPROXY_LOAD_SUCCESS with global load_status

After master-worker refactoring, master performs re-exec only once up to
receiving "reload" command or USR2 signal. There is no more the second
master's re-exec to free unused memory. Thus, there is no longer need to export
environment variable HAPROXY_LOAD_SUCCESS with worker process load status. This
status can be simply saved in a global variable load_status.

7 months agoBUILD: ot: use a cebtree instead of a list for variable names
Miroslav Zagorac [Mon, 28 Oct 2024 14:31:55 +0000 (15:31 +0100)] 
BUILD: ot: use a cebtree instead of a list for variable names

In order for the function flt_ot_vars_scope_dump() to work, it is
necessary to take into account the changes made by the commits 47ec7c681
("OPTIM: vars: use a cebtree instead of a list for variable names") and
5d350d1e5 ("OPTIM: vars: use multiple name heads in the vars struct").

The function is only used if the OT_DEBUG=1 option is set when compiling
HAProxy.

8 months agoMEDIUM: mworker: depreciate the 'program' section
William Lallemand [Fri, 8 Nov 2024 16:06:58 +0000 (17:06 +0100)] 
MEDIUM: mworker: depreciate the 'program' section

The program section is unreliable and should not be used, more reliable
alternatives exist outside HAProxy. Let's depreciate the section so we
could remove it completely in 3.3.

8 months ago[RELEASE] Released version 3.1-dev12 v3.1-dev12
Willy Tarreau [Fri, 8 Nov 2024 14:46:54 +0000 (15:46 +0100)] 
[RELEASE] Released version 3.1-dev12

Released version 3.1-dev12 with the following main changes :
    - MINOR: startup: tune.renice.{startup,runtime} allow to change priorities
    - BUG/MEDIUM: promex: Fix dump of extra counters
    - BUILD: import/mt_list: support building with TCC
    - BUILD: compiler: define __builtin_prefetch() for tcc
    - CLEANUP: quic: Remove the useless directive "tune.quic.backend.max-idle-timeou"
    - DOC: config: document connection error 44 (reverse connect failure)
    - CLEANUP: connection: properly name the CO_ER_SSL_FATAL enum entry
    - DEBUG: cli: support closing "hard" using close() in addition to fd_delete()
    - MINOR: connection: add more connection error codes to cover common errno
    - MINOR: rawsock: set connection error codes when returning from recv/send/splice
    - MINOR: connection: add new sample fetch functions fc_err_name and bc_err_name
    - MINOR: quic: Help diagnosing malformed probing packets
    - BUG/MINOR: quic: fix malformed probing packet building
    - MINOR: listener: Remove useless checks on the receiver protocol existence
    - MINOR: http-conv: Remove unreachable goto statement in sample_conv_q_preferred
    - MINOR: http: don't %-encode the payload when not relevant
    - MINOR: quic: simplify qc_parse_pkt_frms() return path
    - MINOR: quic: use dynamically allocated frame on parsing
    - MINOR: quic: extend return value of CRYPTO parsing
    - BUG/MINOR: quic: repeat packet parsing to deal with fragmented CRYPTO
    - BUG/MINOR: mworker: do 'program' postparser checks in read_cfg_in_discovery_mode
    - EXAMPLES: add "traces.cfg" with traces examples
    - BUG/MEDIUM: quic: do not consider ACK on released stream as error
    - CLEANUP: stats: fix misleading comment on top of stat_idx_info
    - MINOR: wdt: move the local timers to a struct
    - MINOR: debug: add a function to dump a stuck thread
    - DEBUG: wdt: better detect apparently locked up threads and warn about them
    - DEBUG: cli: make it possible for "debug dev loop" to trigger warnings
    - DEBUG: wdt: make the blocked traffic warning delay configurable
    - DEBUG: wdt: add a stats counter "BlockedTrafficWarnings" in show info
    - DEBUG: wdt: set the default blocked task delay to 100 ms
    - MINOR: debug: move the "recover now" warn message after the optional notes
    - MINOR: event_hdl: add event_hdl_sub_list_empty() helper func
    - MINOR: pattern: add _pat_ref_new() helper func
    - OPTIM: pattern: use malloc() to initialize new pat_ref struct
    - MINOR: pattern: add pat_ref_free() helper func
    - CLEANUP: guid: remove global tree export
    - BUG/MINOR: guid/server: ensure thread-safety on GUID insert/delete
    - DOC: management: explain the change of behavior of the program section
    - BUG/MEDIUM: mux-h2: try to wait for the peer to read the GOAWAY
    - BUG/MEDIUM: quic: prevent crash due to CRYPTO parsing error

8 months agoBUG/MEDIUM: quic: prevent crash due to CRYPTO parsing error
Amaury Denoyelle [Fri, 8 Nov 2024 11:40:29 +0000 (12:40 +0100)] 
BUG/MEDIUM: quic: prevent crash due to CRYPTO parsing error

A packet which contains several splitted and out of order CRYPTO frames
may be parsed multiple times to ensure it can be handled via ncbuf. Only
3 iterations can be performed to prevent excessive CPU usage.

There is a risk of crash if packet parsing is interrupted after maximum
iterations is reached, or no progress can be made on the ncbuf. This is
because <frm> may be dangling after list_for_each_entry_safe()

The crash occurs on qc_frm_free() invokation, on error path of
qc_parse_pkt_frms(). To fix it, always reset frm to NULL after
list_for_each_entry_safe() to ensure it is not dangling.

This should fix new report on github isue #2776. This regression has
been triggered by the following patch :
  1767196d5b2d8d1e557f7b3911a940000166ecda
  BUG/MINOR: quic: repeat packet parsing to deal with fragmented CRYPTO

As such, it must be backported up to 2.6, after the above patch.

8 months agoBUG/MEDIUM: mux-h2: try to wait for the peer to read the GOAWAY
Willy Tarreau [Fri, 8 Nov 2024 09:02:47 +0000 (10:02 +0100)] 
BUG/MEDIUM: mux-h2: try to wait for the peer to read the GOAWAY

When timeout http-keep-alive is very short (e.g. 10ms), it's possible
sometimes for a client to face truncated responses due to an early
close that happens while the system is still pushing the last data,
colliding with the client's WINDOW_UPDATEs that trigger RSTs.

Here we're trying to do better: first we send a GOAWAY on timeout, then
we wait up to clientfin/client timeout for the peer to react so that we
don't immediately close. This is sufficient to avoid truncation as soon
as the timeout is more than a few hundred ms.

It's not certain it should be backported, because it's a bit sensistive
and might possibly fall into certain edge cases.

8 months agoDOC: management: explain the change of behavior of the program section
William Lallemand [Fri, 8 Nov 2024 10:57:11 +0000 (11:57 +0100)] 
DOC: management: explain the change of behavior of the program section

The program section does not work exactly the same way with the
master-worker rework of HAProxy 3.1. Let's explain it in the program
documentation.

8 months agoBUG/MINOR: guid/server: ensure thread-safety on GUID insert/delete
Amaury Denoyelle [Thu, 7 Nov 2024 10:08:40 +0000 (11:08 +0100)] 
BUG/MINOR: guid/server: ensure thread-safety on GUID insert/delete

Since 3.0, it is possible to assign a GUID to proxies, listeners and
servers. These objects are stored in a global tree guid_tree.

Proxies and listeners are static. However, servers may be added or
deleted at runtime, which imply that guid_tree must be protected. Fix
this by declaring a read-write lock to protect tree access.

For now, only guid_insert() and guid_remove() are protected using a
write lock. Outside of these, GUID tree is not accessed at runtime. If
server CLI commands are extended to support GUID as server identifier,
lookup operation should be extended with a read lock protection.

Note that during stat-file preloading, GUID tree is accessed for lookup.
However, as it is performed on startup which is single threaded, there
is no need for lock here. A BUG_ON() has been added to ensure this
precondition remains true.

This bug could caused a segfault when using dynamic servers with GUID.
However, it was never reproduced for now.

This must be backported up to 3.0. To avoid a conflict issue, the
previous cleanup patch can be merged before it.

8 months agoCLEANUP: guid: remove global tree export
Amaury Denoyelle [Thu, 7 Nov 2024 10:08:05 +0000 (11:08 +0100)] 
CLEANUP: guid: remove global tree export

guid_tree is not directly used outside of functions provided by the guid
module. Remove its export from the include file.

8 months agoMINOR: pattern: add pat_ref_free() helper func
Aurelien DARRAGON [Thu, 7 Nov 2024 09:11:17 +0000 (10:11 +0100)] 
MINOR: pattern: add pat_ref_free() helper func

For now, pat_ref struct are never freed, except during init in case of
error. The freeing is done directly in the init functions because we
don't have an helper for that.

No having an helper func to properly free pat_ref struct doesn't encourage
us to free unused pat_ref structs, plus it is error-prone if new dynamic
members are added to pat_ref struct in the future.

To fix that, let's add a pat_ref_free() helper func and use it where
relevant (which means only under pat_ref init function for now..)

8 months agoOPTIM: pattern: use malloc() to initialize new pat_ref struct
Aurelien DARRAGON [Thu, 7 Nov 2024 09:05:30 +0000 (10:05 +0100)] 
OPTIM: pattern: use malloc() to initialize new pat_ref struct

As mentioned in the previous commit, in _pat_ref_new(), it was not
strictly needed to explicitly assign all struct members to 0 since
the struct was allocated with calloc() which does the zeroing for us.

However, it was verified that we already initialize all fields explictly,
thus there is no reason to keep using calloc() instead of malloc(). In
fact using malloc() is less expensive, so let's use that instead now.

8 months agoMINOR: pattern: add _pat_ref_new() helper func
Aurelien DARRAGON [Thu, 7 Nov 2024 09:01:40 +0000 (10:01 +0100)] 
MINOR: pattern: add _pat_ref_new() helper func

pat_ref_newid() and pat_ref_new() are two functions to create and
initialize a pat_ref struct based on input parameters.

Both function perform the same generic allocation and initialization
for pat_ref struct, thus there is quite a lot of code redundancy.

This is error-prone if the pat_ref init sequence has to be updated at
some point.

To reduce maintenance costs, let's add a _pat_ref_new() helper func that
takes care of the generic allocation and base initialization for pat_ref
struct.

8 months agoMINOR: event_hdl: add event_hdl_sub_list_empty() helper func
Aurelien DARRAGON [Fri, 18 Oct 2024 16:45:45 +0000 (18:45 +0200)] 
MINOR: event_hdl: add event_hdl_sub_list_empty() helper func

event_hdl_sub_list_empty() may be used to know if the subscription list
passed as argument is empty or not (ie: if there currently are any
subcribers or not). It can be useful to know if the subscription is empty
is order to avoid unecessary preparation work and skip event publishing to
save CPU time if we already know that no one is interested in tracking the
changes for a given subscription list.

8 months agoMINOR: debug: move the "recover now" warn message after the optional notes
Willy Tarreau [Thu, 7 Nov 2024 06:56:13 +0000 (07:56 +0100)] 
MINOR: debug: move the "recover now" warn message after the optional notes

At the end of the too long processing warning added by commit 0950778b3a
("MINOR: debug: add a function to dump a stuck thread"), there can be some
optional notes about lua and memory trimming. However it's a bit awkward
that they appear after the "trying to recover now" message. Let's just move
that message after the notes.

8 months agoDEBUG: wdt: set the default blocked task delay to 100 ms
Willy Tarreau [Wed, 6 Nov 2024 16:51:57 +0000 (17:51 +0100)] 
DEBUG: wdt: set the default blocked task delay to 100 ms

The warn-blocked-traffic-after can be significantly lowered. In any
case, in order to be usable it must be well below the limit to have a
chance to emit exploitable traces before the watchdog finally fires.
Even configured at 1ms it looks very difficult to trigger it on a
laptop doing SSL and compression, so applying a 100-fold factor to
cover for large configs and small machines sounds sane for 3.1. In any
case, even at 100ms, the service degradation becomes quite visible.

8 months agoDEBUG: wdt: add a stats counter "BlockedTrafficWarnings" in show info
Willy Tarreau [Wed, 6 Nov 2024 17:10:01 +0000 (18:10 +0100)] 
DEBUG: wdt: add a stats counter "BlockedTrafficWarnings" in show info

Every time a warning is issued about traffic being blocked, let's
increment a global counter so that we can check for this situation
in "show info".

8 months agoDEBUG: wdt: make the blocked traffic warning delay configurable
Willy Tarreau [Wed, 6 Nov 2024 16:48:41 +0000 (17:48 +0100)] 
DEBUG: wdt: make the blocked traffic warning delay configurable

The new global "warn-blocked-traffic-after" allows one to configure
after how much time a warning should be emitted when traffic is blocked.