Amaury Denoyelle [Tue, 19 Aug 2025 15:22:28 +0000 (17:22 +0200)]
MEDIUM: mux-quic: protect I/O handler when conn is idle
Add protection in QUIC MUX when an idle connection is being used. Even
if takeover is not yet implemented, this is useful as idle connections
can be purged by any thread, for example when a server goes into
maintenance.
This change should prevent crashes related to idle connections when a
QUIC server is removed via "delete server".
MINOR: backend: set release on takeover for strict maxconn
If strict maxconn is active on a server, it may be necessary to kill an
idle connection to never exceed the configured value. This relies on
takeover, to be able to grab an idle connection from another thread and
free it immediately.
As takeover is used to delete a connection and not reuse it, it can set
<release> argument to true. This will remove extra allocations of
resources just freed so that the connection can migrate to a new thread.
As such, it is a small optimization for strict maxconn implementation.
Note that this patch depends on the previous one which removes any
assumption in takeover implementation that thread isolation is active if
<release> is true.
MINOR: muxes: adjust takeover with buf_wait interaction
Takeover operation defines an argument <release>. This is a boolean
which specify if connection resources which are deleted during the
operation must be reallocated.
Typically, it is set to false when the operation is performed to reuse a
connection. However, if takeover is used prior to be able to delete a
connection from another thread, <release> can be set to true.
Currently, this last possibility was only used on "delete server"
handler. This was used to remove any idle connections prior to remove a
server instance. As this operation was conducted under thread isolation,
connection could be removed from buf_wait if subscribed for a new
buffer. This operation is not thread-safe, as such it is considered only
valid under thread isolation, hence why it was only performed if
<release> is true. In the contrary case, takeover would return an error.
The previous patch removes takeover usage for idle connection cleanup
via "delete server". As such, there is no code path which set <release>
on takeover. However, takeover is also used to kill a connection when
applying strict-maxconn on a server. As described in the first
paragraph, it could be legitimate to set <release> to true in this case.
However, it is not performed under thread isolation, so it is first
necessary to remove the assumption on takeover implementation that if
<release> is true, thread isolation is active. This is the exact purpose
of this commit.
This patch thus simplifies takeover of every muxes. Now, if connection
is subscribed in buf_wait list, takeover fails, even if <release> is
true. This is necessary to be able to use takeover for connection
deletion without thread isolation.
MAJOR: server: do not remove idle conns in delete server
Removes idle connections removal from "delete server" handler. This
concerns both idle, purgeable and private connections.
Thus it is not safe anymore to remove a server if there is idle
connections attached into it. To ensure this cannot be performed,
srv_check_for_deletion() has been extended with checks on server members
<curr_idle_conns> counter and <sess_conns> list.
The main objective of this patch is to reduce the amount of work
performed under thread isolation. Now, idle connection purging must be
scheduled prior to "delete server" invokation, thus switching from a
synchronous to asynchronous process.
This patch is considered as major as it may have an impact on "delete
server" execution. It could be slightly longer to be able to remove a
server, after its idle connections are removed. However, a previous
change has introduced idle connections cleanup, both standard and
private, when a server is switched to maintenance. Also, it is not
possible for a server in maintenance to keep new idle connections. As
such, it is expected that the difference for "delete server" completion
should be minimal.
Also, another objective of this patch is to be able to support dynamic
servers with QUIC, as takeover is not yet implemented for this protocol.
However, there is still some changes to perform to protect IO handler
for QUIC MUX.
MEDIUM: session: account on server idle conns attached to session
This patch adds a new member curr_sess_idle_conns on the server. It will
serve as a counter of idle connections attached on a session instead of
regular idle/safe trees. This happens only for private connections.
The objective is to provide an alternive method to detect if an idle
connections is referencing a server prior to its deletion. Currently,
this is not strictly necessary as idle connections are directly freed
via "delete server" handler. However, this cleanup will be replaced by
an asynchronous mechanism reusing connection purge list. Thus, this new
curr_sess_idle_conns counter will be useful here.
Careful: connections attached to a session but not idle are not
accounted here. This is not necessary for "delete server" as these
connections can be detected via srv_has_streams().
CO_FL_SESS_IDLE flag serves as a marker of a connection accounted in
curr_sess_idle_conns. As such, counter is incremented when the
connection is flagged via session_check_idle_conn(). For the decrement,
there is two paths :
* if still owned by the session, decrement is performed via
session_unown_conn()
* when conn is detached for purge (not yet implemented), it will be
decremented via srv_release_conn()
The latter path is the most critical one as it requires some changes
during conn release. Now, conn_backend_deinit() is called also for idle
private connections, which is necessary to invoke srv_release_conn().
MAJOR: server: implement purging of private idle connections
When a server goes into maintenance, or if its IP address is changed,
idle connections attached to it are scheduled for deletion via the purge
mechanism : each connections is moved from server idle/safe list to the
purge list. Connections are later freed on purge task execution
individually on each thread.
This commit extends this by also implementing deletion of private idle
connections. This is possible as these connections are also attached to
their respective server via <sess_conns> mt-list. Thus, the new function
session_purge_conns() can now be called from there. It moves every
connection from its session list to the purge list, alongside the
standard idle connections.
This change is now possible as private idle connections are protected
similarly to standard ones. First, idle_conns_lock usage has been
extended around them. Second, muxes IO handlers execution is safe as
idle private connections are temporarily removed from the session, so
that a purge cannot be performed on them until IO is completed.
Amaury Denoyelle [Mon, 18 Aug 2025 13:19:18 +0000 (15:19 +0200)]
MEDIUM: conn/muxes: remove BE idle conn from session on IO handler
This is a direct follow-up of previous patch to adjust idle private
connections handling in IO muxes handlers.
This patch is focused on the prologue of handlers. Previously, private
idle connections were not protected as this is not yet needed. This
commit adapts this by treating them like regular idle connections. An
idle private connection is now temporarily removed from its session
using conn_delete_from_sess(). As this is conducted under idle_conns
lock, this guarantee that the connection can then be manipulated, even
if a purge on idle conns is executed in parallel.
Amaury Denoyelle [Mon, 18 Aug 2025 08:41:38 +0000 (10:41 +0200)]
MEDIUM: conn/muxes: reinsert BE priv conn into sess on IO completion
When IO handler of MUX is executed, special care must be taken if the
connection is considered as idle, as it could be stolen from another
thread via a takeover. Thus, connection is first removed from its idle
tree, and reinserted on completion, if the connection is still valid.
Idle private connections are not concerned by this, because takeover is
not applied on them. However, a future patch will implement purging of
these connections along with regular idle ones. As such, it is necessary
to also protect private connections usage now. This is the subject of
this patch and the next one.
On this patch, IO handlers ending of muxes is adjusted. If the
connection was idle and thus temporarily removed from its session on IO
handler start, a new code path is executed to reinsert the connection on
its session. This is implemented via a new function
session_reinsert_idle_conn(). Contrary to session_add_conn(), this is
reserved for reinserting idle connections after a temporary removal.
Also, contrary to _srv_add_idle() used by regular idle connections,
session_reinsert_idle_conn() may fail as an allocation is required. If
this happens, the connection is simply destroyed.
This patch is only a code adjustment, with no effect for now. Indeed,
session_reinsert_idle_conn() is protected by <conn_in_list> variable in
IO handlers, which is not yet set for private connections. Thus,
behavior changes will really start to occur with the next patch.
Amaury Denoyelle [Mon, 18 Aug 2025 13:24:05 +0000 (15:24 +0200)]
MINOR: muxes: strengthen private connection for thread-safety
When a backend connnection becomes idle, muxes must activate some
protection. This is necessary as such connections can therefore for
example be stolen by another thread via a takeover.
Private idle connections are stored into a session instead of the server
tree. They are never targetted by takeover operation, as such they
currently do not need the same protection.
However, a future patch will introduce the purging of such connections,
which can be performed from any thread. Thus, the purpose of this patch
is to activate the same level of protection on idle private connections
in mux detach operations.
TASK_F_USR1 was already set on them as an anticipation for such need.
Complete this with necessary extra operations, most notably
xprt_set_idle() usage. Also, return is performed earlier in detach
operation to prevent unsafe access to the connection after its insertion
into the session list.
MEDIUM: server/session: reject idle conns if server in maintenance
Currently, when a server is set on maintenance mode, its idle connection
are purged. However, this does not prevent currently used connection to
become idle later on, even if the server is still off.
Change this behavior : an idle connection is now rejected by the server
if it is in maintenance. This is implemented with a new condition in
srv_add_to_idle_list() which returns an error value. This instructs
muxes to immediately free the connection in this case.
A similar change is also performed for each MUX I/O handler and in
conn_notify_mux(). If the connection is idle, first check if server is
in maintenance. If this is the case, the connection is not reinserted in
its list but instead immediately freed.
Finally, a similar approach is also performed for private connections
when marking a connection attached to a session as idle. A check is
performed on server state, retrieved via <target> conn member. If in
maintenance, the connection is rejected, as if session max idle conns
limit was reached.
Amaury Denoyelle [Thu, 14 Aug 2025 16:18:01 +0000 (18:18 +0200)]
MINOR: server: shard by thread sess_conns list storage in server struct
Server member <sess_conns> is a mt_list which stores every backend
connections attached to a session which targets this server. These
connecions are therefore not present in idle server trees.
Mostly, this contains idle/non-idle private connections. This can also
contain connections in use store temporarily in the session to prevent
sharing across clients, which is useful when backend multiplexer suffers
from head-of-line blocking.
The main utility of this list is to be able to cleanup idle private
connections prior to removing a server via "delete server" CLI. This
commit transforms <sess_conns> member, which is moved from server
structure to srv_per_thread. Effectively, the list is now shared by
threads.
There should be no functional change. However, this patch is necessary
to adapt connection removal for "delete server" to an asynchronous
method based on connections purging mechanism, as now it is easier to
loop over each private connections on a server one thread at a time.
MEDIUM: session: protect sess conns list by idle_conns_lock
Introduce idle_conns_lock usage to protect manipulation to <priv_conns>
session members. This is the list of all connections attached to a
session.
Currently, this patch is unneeded as such sessions are only manipulated
on a single-thread. However, it will be required to trigger purging of
idle connections attached to a session from an external thread.
Amaury Denoyelle [Wed, 20 Aug 2025 08:01:46 +0000 (10:01 +0200)]
MINOR: session: uninline functions related to BE conns management
Move from header to source file functions related to session management
of backend connections. These functions are big enough to remove inline
attribute.
Amaury Denoyelle [Wed, 13 Aug 2025 16:13:10 +0000 (18:13 +0200)]
MINOR: session: document explicitely that session_add_conn() is safe
A set of recent patches have simplified management of backend connection
attached to sessions. The API is now strictier to prevent any misuse.
One of this change is the addition of a BUG_ON() in session_add_conn(),
which ensures that a connection is not attached to a session if its
<owner> field points to another entry.
This case could break due to NTLM as connection are turned as private
during transfers, possibly with several sessions already using it.
However, NTLM is now only applied for HTTP/1.1 as it does not make sense
if the connection is already shared. To clarify this, BUG_ON() comment
is extended.
Amaury Denoyelle [Wed, 20 Aug 2025 15:16:28 +0000 (17:16 +0200)]
BUG/MINOR: mux-quic: fix return on detach after idle conn insert
Once a connection is inserted into the server idle/safe tree, it must
not be accessed anymore without protection. This is because the
connection could have been already stolen by a takeover operation.
As such, ensure connection is not accessed anymore after
srv_add_to_idle_list() by setting conn to NULL. Note that, no bug can
occur for now because QUIC MUX does not implement takeover yet.
Amaury Denoyelle [Tue, 19 Aug 2025 13:41:43 +0000 (15:41 +0200)]
BUG/MINOR: server: decrement session idle_conns on delete server
When a server is deleted, each of its idle connections are removed. This
is also performed for every private connections stored on sessions which
referenced the target server.
All of these private connections are idle. This is guaranteed by
srv_check_for_deletion() which ensures that "delete server" handler is
only executed if there is no active connection on a server. A BUG_ON()
on CO_FL_SESS_IDLE is already present to enforce this.
Session is also responsible to maintain its counter for idle
connections. This is useful to limit the number of idle connections
usable per sessions. However, in case of "delete server", this counter
is not decremented. Fix this by ensuring that a decrement is performed
for every private connections removed.
Amaury Denoyelle [Tue, 19 Aug 2025 15:20:51 +0000 (17:20 +0200)]
MINOR: cli: display failure reason on wait command
wait CLI command can be used to wait until a specific condition is
reached. So far, srv-removable is the only event supported. This is
implemented via srv_check_for_deletion(), which is able to report a
message describing the reason if the condition is unmet.
Previously, wait handler did not reuse the provided message. Instead,
generic errors were reported, which might not be enough for debugging.
Improve this so that now wait is able to report the final message when
it returns, either on a fatal error or on timeout. This requires the
usage of dynamic string output using cli_dynmsg/err functions.
BUG/MEDIUM: mworker: more verbose error upon loading failure
When a worker crashes during its configuration parsing and without
emitting any messages, the master will emit the message "Failed to load
worker!". However that doesn't give us neither the PID of the worker,
nor the status code.
This patch fixes the problem by emitting a more verbose error.
BUG/MEDIUM: quic-be: do not initialize ->conn too early
This bug arrived with this commit:
BUG/MEDIUM: quic: do not release BE quic-conn prior to upper conn
which added a BUG_ON(qc->conn) statement at the beginning of quic_conn_release().
It is triggered if the connection is not released before releasing the quic_conn.
But this is always the case for a backend quic_conn when its allocation from
qc_new_conn() fails.
Such crashes could be reproduced with -dMfail option. To reach them, the
memory allocations must fail. So, this is relatively rare, except on systems
with limited memory.
To fix this, simply set ->conn quic_conn struct member to a not null value
(the one passed as parameter) after the quic_conn allocation has succeeded.
where qc_new_conn() was modified. The ->cids allocation was moved without
checking if a quic_conn_release() call could lead to crashes due to uninitialized
quic_conn members. Indeed, if qc_new_conn() fails, then quic_conn_release() is
called. This bug could impact both QUIC servers and clients.
Such crashes could be reproduced with -dMfail option. To reach them, the
memory allocations must fail. So, this is relatively rare, except on systems
with limited memory.
This patch ensures all the quic_conn members which could lead to crash
from quic_conn_release() are initialized before any remaining memory allocations
required for the quic_conn.
The <conn_id> variable allocated by the client is no more attached to
the connection during its allocation, but after the ->cids trees is allocated.
BUG/MEDIUM: cli: Report inbuf is no longer full when a line is consumed
When the command line parsing was refactored (20ec1de21 "MAJOR: cli: Refacor
parsing and execution of pipelined commands"), a regression was introduced.
When input data are consumed, information about the applet's input buffer
are no longer updated accordingly to state it is no longer full. So it is
possible to freeze the CLI applet. And a spinning loop may be encountered if
a client shutdown is detected in this state.
The fix is obivous. When data are consumed from the applet's input buffer,
APPCTX_FL_INBLK_FULL flag is removed to notify the input buffer is no longer
full and more data can be sent to the CLI applet.
This patch should fix the issue #3064. It must be backported to 3.2.
BUG/MINOR: spoe: Properly detect and skip empty NOTIFY frames
Since the SPOE was refactored, the detection of empty NOTIFY frames is
broken. So it is possible to send a NOTIFY frames to an agent with no
message at all. The bug happens because the frame type is now added to the
buffer before the messages encoding. So the buffer is never really empty.
To fix the issue, the condition to detect empty frame was adapted.
MEDIUM: dns: don't call connect to dest socket for AF_INET*
When we perform connect call for a datagram socket, used to send DNS requests,
we set for it the default destination address to some given nameserver. Then we
simply use send(), as the destination address is already set. In some usecases
described in GitHub issues #3001 and #2654, this approach becames inefficient,
nameservers change its IP addresses dynamically, this triggers DNS resolution
errors.
To fix this, let's perform the bind() on the wildcard address for the datagram
AF_INET* client socket. Like this we will allocate a port for it. Then let's
use sendto() instead of send().
If the nameserver is local and is listening on the UNIX domain socket, we
continue to use the existed approach (connect() and then send()).
This fixes issues #3001 and #2654.
This may be backported in all stable versions.
BUG/MEDIUM: quic: listener connection stuck during handshakes (OpenSSL 3.5)
This issue was reported in GH #3071 by @famfo where a wireshark capture
reveals that some handshake could not complete after having received
two Initial packets. This could happen when the packets were parsed
in two times, calling qc_ssl_provide_all_quic_data() two times.
This is due to crypto data stream counter which was incremented two times
from qc_ssl_provide_all_quic_data() (see cstream->rx.offset += data
statement around line 1223 in quic_ssl.c). One time by the callback
which "receives" the crypto data, and on time by qc_ssl_provide_all_quic_data().
Then when parsing the second crypto data frame, the parser detected
that the crypto were already provided.
To fix this, one could comment the code which increment the crypto data
stream counter by <data>. That said, when using the OpenSSL 3.5 QUIC API
one should not modified the crypto data stream outside of the OpenSSL 3.5
QUIC API.
So, this patch stop calling qc_ssl_provide_all_quic_data() and
qc_ssl_provide_quic_data() and only calls qc_ssl_do_hanshake() after
having received some crypto data. In addition to this, as these functions
are no more called when building haproxy against OpenSSL 3.5, this patch
disable their compilations (with #ifndef HAVE_OPENSSL_QUIC).
Willy Tarreau [Wed, 13 Aug 2025 15:36:18 +0000 (17:36 +0200)]
MEDIUM: ring: always allocate properly aligned ring structures
The rings were manually padded to place the various areas that compose
them into different cache lines, provided that the allocator returned
a cache-aligned address, which until now was not granted. By now
switching to the aligned API we can finally have this guarantee and
hope for more consistent ring performance between tests. Like previously
the few carefully crafted THREAD_PAD() could simply be replaced by
generic THREAD_ALIGN() that dictate the type's alignment.
This was the last user of THREAD_PAD() by the way.
Willy Tarreau [Wed, 13 Aug 2025 13:07:55 +0000 (15:07 +0200)]
MINOR: server: align server struct to 64 bytes
Several times recently, it was noticed that some benchmarks would
highly vary depending on the position of certain fields in the server
struct, and this could even vary between runs.
The server struct does have separate areas depending on the user cases
and hot/cold aspect of the members stored there, but the areas are
artificially kept apart using fixed padding instead of real alignment,
which has the first sad effect of artificially inflating the struct,
and the second one of misaligning it.
Now that we have all the necessary tools to keep them aligned, let's
just do it. The struct has shrunk from 4160 to 4032 bytes on 64-bit
systems, 152 of which are still holes or padding.
Willy Tarreau [Wed, 13 Aug 2025 09:52:59 +0000 (11:52 +0200)]
MEDIUM: server: introduce srv_alloc()/srv_free() to alloc/free a server
It happens that we free servers at various places in the code, both
on error paths and at runtime thanks to the "server delete" feature. In
order to switch to an aligned struct, we'll need to change the calloc()
and free() calls. Let's first spot them and switch them to srv_alloc()
and srv_free() instead of using calloc() and either free() or ha_free().
An easy trap to fall into is that some of them are default-server
entries. The new srv_free() function also resets the pointer like
ha_free() does.
This was done by running the following coccinelle script all over the
code:
This is marked medium because despite spotting all call places, we can
never rule out the possibility that some out-of-tree patches would
allocate their own servers and continue to use the old API... at their
own risk.
Willy Tarreau [Wed, 13 Aug 2025 15:11:32 +0000 (17:11 +0200)]
MINOR: tools: also implement ha_aligned_alloc_typed()
This one is a macro and will allocate a properly aligned and sized
object. This will help make sure that the alignment promised to the
compiler is respected.
When memstats is used, the type name is passed as a string into the
.extra field so that it can be displayed in "debug dev memstats". Two
tiny mistakes related to memstats macros were also fixed (calloc
instead of malloc for zalloc), and the doc was also added to document
how to use these calls.
Willy Tarreau [Mon, 11 Aug 2025 15:43:51 +0000 (17:43 +0200)]
OPTIM: server: start to use aligned allocs in server
This is currently for per-thread arrays like idle conns etc. We're
now cache-aligning the per-thread arrays so as to put an end to false
sharing. A comparative test between no alignment and alignment on a
simple config with round robin between 4 servers showed an average
rate of 1.75M/s vs 1.72M/s before for 100M requests. The gain seems
to be more commonly less than 1% however. This should mostly help
make measurements more reproducible across multiple runs.
Willy Tarreau [Mon, 11 Aug 2025 15:36:39 +0000 (17:36 +0200)]
OPTIM: connection: align connection pools to 64
The struct connection is used a lot by the muxes during many operations,
particularly at the beginning of the struct (flags, ctrl, xprt and mux).
We definitely want this one not to be falsely shared with another thread,
so let's align the pools to a cache line.
Willy Tarreau [Mon, 11 Aug 2025 15:31:08 +0000 (17:31 +0200)]
OPTIM: buffers: align the buffer pool to 64
This struct is used by memcpy() and friends, particularly during the
early recv() and send(). By keeping it 64-byte aligned, we let the
underlying libs/kernel use optimal operations (e.g. AVX512) for memory
copies while right now it's just random (buffers are found to be equally
aligned to 32 and 64 in practice).
Willy Tarreau [Mon, 11 Aug 2025 15:29:04 +0000 (17:29 +0200)]
OPTIM: tasks: align task and tasklet pools to 64
These structs are intensively used and really must not experience false
sharing, so let's declare them aligned to 64. We don't try to align the
struct themselves, as we don't want the compiler to expand them either.
Willy Tarreau [Wed, 6 Aug 2025 14:43:27 +0000 (16:43 +0200)]
MEDIUM: tree-wide: replace most DECLARE_POOL with DECLARE_TYPED_POOL
This will make the pools size and alignment automatically inherit
the type declaration. It was done like this:
sed -i -e 's:DECLARE_POOL(\([^,]*,[^,]*,\s*\)sizeof(\([^)]*\))):DECLARE_TYPED_POOL(\1\2):g' $(git grep -lw DECLARE_POOL src addons)
sed -i -e 's:DECLARE_STATIC_POOL(\([^,]*,[^,]*,\s*\)sizeof(\([^)]*\))):DECLARE_STATIC_TYPED_POOL(\1\2):g' $(git grep -lw DECLARE_STATIC_POOL src addons)
81 replacements were made. The only remaining ones are those which set
their own size without depending on a structure. The few ones with an
extra size were manually handled.
It also means that the requested alignments are now checked against the
type's. Given that none is specified for now, no issue is reported.
It was verified with "show pools detailed" that the definitions are
exactly the same, and that the binaries are similar.
Willy Tarreau [Mon, 11 Aug 2025 14:47:17 +0000 (16:47 +0200)]
MINOR: pools: always check that requested alignment matches the type's
For pool registrations that are created from the type declaration, we
now have the ability to verify that the requested alignment matches
the type's one. Let's not miss this opportunity, as we've met bugs in
the past that were caused by such mismatches. The principle is simple:
if the type alignment is known, we check that the configured alignment
is at least as large as that one otherwise we refuse to start (since
the code may crash at any moment). Obviously it doesn't crash for now!
Willy Tarreau [Mon, 11 Aug 2025 13:51:32 +0000 (15:51 +0200)]
MINOR: pools: permit to optionally specify extra size and alignment
The common macros REGISTER_TYPED_POOL(), DECLARE_TYPED_POOL() and
DECLARE_STATIC_TYPED_POOL() will now take two optional arguments,
one being the extra size to be added to the structure, and a second
one being the desired alignment to enforce. This will permit to
specify alignments larger than the default ones promised to the
compiler.
Willy Tarreau [Mon, 11 Aug 2025 13:24:33 +0000 (15:24 +0200)]
MINOR: pools: distinguish the requested alignment from the type-specific one
We're letting users request an alignment but that can violate one imposed
by a type, especially if we start seeing REGISTER_TYPED_POOL() grow in
adoption, encouraging users to specify alignment on their types. On the
other hand, if we ask the user to always specify the alignment, no control
is possible and the error is easy. Let's have a second field in the pool
registration, for the type-specific one. We'll set it to zero when unknown,
and to the types's alignment when known. This way it will become possible
to compare them at startup time to detect conflicts. For now no macro
permits to set both separately so this is not visible.
Willy Tarreau [Mon, 11 Aug 2025 16:55:09 +0000 (18:55 +0200)]
CLEANUP: fd: make use of ha_aligned_alloc() for the fdtab
We've forcefully aligned the fdtab in commit 97ea9c49f1 ("BUG/MEDIUM:
fd: always align fdtab[] to 64 bytes"), but now we don't need such hacks
anymore thanks to ha_aligned_alloc(). Let's use it and get rid of
fdtab_addr.
Willy Tarreau [Mon, 11 Aug 2025 16:46:28 +0000 (18:46 +0200)]
MINOR: tools: implement ha_aligned_zalloc()
This one is exactly ha_aligned_alloc() followed by a memset(0), as
it will be convenient for a number of call places as a replacement
for calloc().
Note that ideally we should also have a calloc version that performs
basic multiply overflow checks, but these are essentially used with
numbers of threads times small structs so that's fine, and we already
do the same everywhere in malloc() calls.
MEDIUM: ssl/cli: relax crt insertion in crt-list of type directory
In previous versions of haproxy, insertions of certificates in a
crt-list from the CLI would require to have the path of the directory,
in the path of the certificate. This would help avoiding that the
certificate wasn't loaded upon a reload because it is not at the right
place.
However, since version 3.0 and crt-store, the name stored in the tree
could be an alias and not a path, so that does not make sense anymore.
Even though path would be right, the check is not right anymore in this
case.
The tool or user inserting the certificate must now check itself that
the certificate was placed at the right spot on the filesystem.
In ticket #3065 an user complained that no success message is printed
anymore when using -c. The message does not appear by default since
version 2.9. This patch clarify the documentation.
BUG/MINOR: init: Initialize random seed earlier in the init process
The random seed used in ha_random functions needs to be first
initialized by calling ha_random_boot. This function was called rather
late in the init process, after the init functions (INITCALLS) are
called and after the configuration parsing for instance which means that
any ha_random call in an init function would return 0. This was the case
in 'vars_init' and 'cache_init' which tried to build seeds for specific
hash calculations but ended up not being seeded.
This patch can be backported on all stable branches.
MEDIUM: acme: use lowercase for challenge names in configuration
Both the RFC and the IANA registry refers to challenge names in
lowercase. If we need to implement more challenges, it's better to
use the correct naming.
In order to keep the compatibility with the previous configurations, the
parsing does a strcasecmp() instead of a strcmp().
Also rename every occurence in the code and doc in lowercase.
AWS-LC doesn't provide SSL_in_before(), and doesn't provide an easy way
to know if we already started the handshake or not. So instead, just add
a new field in ssl_sock_ctx, "can_write_early_data", that will be
initialized to 1, and will be set to 0 as soon as we start the
handshake.
In order to send early data, we have to make sure no handshake has been
initiated at all. To do that, we remove the CO_FL_SSL_WAIT_HS flag, so
that we won't attempt to start a handshake. However, by removing those
flags, we allow ssl_sock_to_buf() to call SSL_read(), as it's no longer
aware that no handshake has been done, and SSL_read() will begin the
handshake, thus preventing us from sending early data.
The fix is to just call SSL_in_before() to check if no handshake has
been done yet, in addition to checking CO_FL_SSL_WAIT_HS (both are
needed, as CO_FL_SSL_WAIT_HS may come back in case of renegociation).
In ssl_sock_from_buf(), fix the check to see if we may attempt to send
early data. Use SSL_in_before() instead of SSL_is_init_finished(), as
SSL_is_init_finished() will return 1 if the handshake has been started,
but not terminated, and if the handshake has been started, we can no
longer send early data.
This fixes errors when attempting to send early data (as well as
actually sending early data).
BUG/MINOR: stick-table: cap sticky counter idx with tune.nb_stk_ctr instead of MAX_SESS_STKCTR
Cap sticky counter index with tune.nb_stk_ctr instead of MAX_SESS_STKCTR for
sc-add-gpc. Same logic is already implemented for sc-inc-gpc and sc-set-gpt
keywords. So, it seems missed for sc-add-gpc.
This fixes the issue #3061 reported at GitHub. Thanks to @ma311 for
reporting their analysis of the issue.
This should be backported in all versions until 2.8, included 2.8.
Some optional features may still require the use of shm_open() in the
future. In this patch we restore the USE_SHM_OPEN build option that
was removed in 143be1b59 ("MEDIUM: errors: get rid of shm_open()") and
should guard the use of shm_open() in the code.
Similar to REGISTER_POST_DEINIT() hook (which is invoked during deinit)
but for master process only, when haproxy was started in master-worker
mode. The goal is to be able to register cleanup functions that will
only run for the master process right before exiting.
Since now_offset is a static variable and is not exposed outside from
clock.c, let's add an helper so that it becomes possible to set its
value from another source file.
BUG/MINOR: proxy: avoid NULL-deref in post_section_px_cleanup()
post_section_px_cleanup(), which was implemented in abcc73830
("MEDIUM: proxy: register a post-section cleanup function"), is called
for the current section no matter if the parsing was aborted due to
a fatal error. In this case, the curproxy pointer may point to NULL,
yet post_section_px_cleanup() assumes curproxy pointer is always valid,
which could lead to NULL-deref.
For instance, the config below will cause SEGFAULT:
listen toto titi
To fix the issue, let's simply consider that the curproxy pointer may
be NULL in post_section_px_cleanup(), in which case we skip the cleanup
for the curproxy since there is nothing we can do.
BUG/MINOR: cfgparse-listen: update err_code for fatal error on proxy directive
When improper arguments are provided on proxy directive (listen,
frontend or backend), such alert may be emitted:
"please use the 'bind' keyword for listening addresses"
This was introduced in 6e62fb6405 ("MEDIUM: cfgparse: check section
maximum number of arguments"). However, despite the error being reported
as alert, the err_code isn't updated accordingly, which could make the
upper parser think there was no error, while it isn't the case.
In practise since the proxy directive is ignored following proxy related
directives should raise errors, so this didn't cause much harm, yet
better fix that.
BUG/MINOR: cfgparse: immediately stop after hard error in srv_init()
Since 368d01361 (" MEDIUM: server: add and use srv_init() function"), in
case of srv_init() error, we simply increment cfgerr variable and keep
going.
It isn't enough, some treatment occuring later in check_config_validity()
assume that srv_init() succeeded for servers, and may cause undefined
behavior. To fix the issue, let's consider that if (srv_init() & ERR_CODE)
returns true, then we must stop checking the config immediately.
MINOR: quic: prefer qc_is_back() usage over qc->target
Previously quic_conn <target> member was used to determine if quic_conn
was used on the frontend (as server) or backend side (as client). A new
helper function can now be used to directly check flag
QUIC_FL_CONN_IS_BACK.
This reduces the dependency between quic_conn and their relative
listener/server instances.
Define a new quic_conn flag assign if the connection is used on the
backend side. This is similar to other haproxy components such as struct
connection and muxes element.
This flag is positionned via qc_new_conn(). Also update quic traces to
mark proxy side as 'F' or 'B' suffix.
MINOR: quic: duplicate GSO unsupp status from listener to conn
QUIC emission can use GSO to emit multiple datagrams with a single
syscall invokation. However, this feature relies on several kernel
parameters which are checked on haproxy process startup.
Even if these checks report no issue, GSO may still be unable due to the
underlying network adapter underneath. Thus, if a EIO occured on
sendmsg() with GSO, listener is flagged to mark GSO as unsupported. This
allows every other QUIC connections to share the status and avoid using
GSO when using this listener.
Previously, listener flag was checked for every QUIC emission. This was
done using an atomic operation to prevent races. Improve this by
duplicating GSO unsupported status as the connection level. This is done
on qc_new_conn() and also on thread rebinding if a new listener instance
is used.
The main benefit from this patch is to reduce the dependency between
quic_conn and listener instances.
Willy Tarreau [Wed, 6 Aug 2025 19:50:00 +0000 (21:50 +0200)]
[RELEASE] Released version 3.3-dev6
Released version 3.3-dev6 with the following main changes :
- MINOR: acme: implement traces
- BUG/MINOR: hlua: take default-path into account with lua-load-per-thread
- CLEANUP: counters: rename counters_be_shared_init to counters_be_shared_prepare
- MINOR: clock: make global_now_ms a pointer
- MINOR: clock: make global_now_ns a pointer as well
- MINOR: mux-quic: release conn after shutdown on BE reuse failure
- MINOR: session: strengthen connection attach to session
- MINOR: session: remove redundant target argument from session_add_conn()
- MINOR: session: strengthen idle conn limit check
- MINOR: session: do not release conn in session_check_idle_conn()
- MINOR: session: streamline session_check_idle_conn() usage
- MINOR: muxes: refactor private connection detach
- BUG/MEDIUM: mux-quic: ensure Early-data header is set
- BUILD: acme: avoid declaring TRACE_SOURCE in acme-t.h
- MINOR: acme: emit a log for DNS-01 challenge response
- MINOR: acme: emit the DNS-01 challenge details on the dpapi sink
- MEDIUM: acme: allow to wait and restart the task for DNS-01
- MINOR: acme: update the log for DNS-01
- BUG/MINOR: acme: possible integer underflow in acme_txt_record()
- BUG/MEDIUM: hlua_fcn: ensure systematic watcher cleanup for server list iterator
- MINOR: sample: Add le2dec (little endian to decimal) sample fetch
- BUILD: fcgi: fix the struct name of fcgi_flt_ctx
- BUILD: compat: provide relaxed versions of the MIN/MAX macros
- BUILD: quic: use _MAX() to avoid build issues in pools declarations
- BUILD: compat: always set _POSIX_VERSION to ease comparisons
- MINOR: implement ha_aligned_alloc() to return aligned memory areas
- MINOR: pools: support creating a pool from a pool registration
- MINOR: pools: add a new flag to declare static registrations
- MINOR: pools: force the name at creation time to be a const.
- MEDIUM: pools: change the static pool creation to pass a registration
- DEBUG: pools: store the pool registration file name and line number
- DEBUG: pools: also retrieve file and line for direct callers of create_pool()
- MEDIUM: pools: add an alignment property
- MINOR: pools: add macros to register aligned pools
- MINOR: pools: add macros to declare pools based on a struct type
- MEDIUM: pools: respect pool alignment in allocations
Willy Tarreau [Tue, 5 Aug 2025 16:12:18 +0000 (18:12 +0200)]
MEDIUM: pools: respect pool alignment in allocations
Now pool_alloc_area() takes the alignment in argument and makes use
of ha_aligned_malloc() instead of malloc(). pool_alloc_area_uaf()
simply applies the alignment before returning the mapped area. The
pool_free() functionn calls ha_aligned_free() so as to permit to use
a specific API for aligned alloc/free like mingw requires.
Note that it's possible to see warnings about mismatching sized
during pool_free() since we know both the pool and the type. In
pool_free, adding just this is sufficient to detect potential
offenders:
Willy Tarreau [Wed, 6 Aug 2025 14:33:27 +0000 (16:33 +0200)]
MINOR: pools: add macros to declare pools based on a struct type
DECLARE_TYPED_POOL() and friends take a name, a type and an extra
size (to be added to the size of the element), and will use this
to create the pool. This has the benefit of letting the compiler
automatically adapt sizeof() and alignof() based on the type
declaration.
Willy Tarreau [Tue, 5 Aug 2025 16:26:37 +0000 (18:26 +0200)]
MINOR: pools: add macros to register aligned pools
This adds an alignment argument to create_pool_from_loc() and
completes the existing low-level macros with new ones that expose
the alignment and the new macros permit to specify it. For now
they're not used.
Willy Tarreau [Tue, 5 Aug 2025 16:03:09 +0000 (18:03 +0200)]
MEDIUM: pools: add an alignment property
This will be used to declare aligned pools. For now it's not used,
but it's properly set from the various registrations that compose
a pool, and rounded up to the next power of 2, with a minimum of
sizeof(void*).
The alignment is returned in the "show pools" part that indicates
the entry size. E.g. "(56 bytes/8)" means 56 bytes, aligned by 8.
Willy Tarreau [Wed, 6 Aug 2025 07:50:42 +0000 (09:50 +0200)]
DEBUG: pools: also retrieve file and line for direct callers of create_pool()
Just like previous patch, we want to retrieve the location of the caller.
For this we turn create_pool() into a macro that collects __FILE__ and
__LINE__ and passes them to the now renamed function create_pool_with_loc().
Now the remaining ~30 pools also have their location stored.
Willy Tarreau [Wed, 6 Aug 2025 07:43:39 +0000 (09:43 +0200)]
DEBUG: pools: store the pool registration file name and line number
When pools are declared using DECLARE_POOL(), REGISTER_POOL etc, we
know where they are and it's trivial to retrieve the file name and line
number, so let's store them in the pool_registration, and display them
when known in "show pools detailed".
Willy Tarreau [Tue, 5 Aug 2025 16:59:15 +0000 (18:59 +0200)]
MEDIUM: pools: change the static pool creation to pass a registration
Now we're creating statically allocated registrations instead of
passing all the parameters and allocating them on the fly. Not only
this is simpler to extend (we're limited in number of INITCALL args),
but it also leaves all of these in the data segment where they are
easier to find when debugging.
Willy Tarreau [Tue, 5 Aug 2025 17:07:47 +0000 (19:07 +0200)]
MINOR: pools: force the name at creation time to be a const.
This is already the case as all names are constant so that's fine. If
it would ever change, it's not very hard to just replace it in-situ
via an strdup() and set a flag to mention that it's dynamically
allocated. We just don't need this right now.
One immediately visible effect is in "show pools detailed" where the
names are no longer truncated.
Willy Tarreau [Tue, 5 Aug 2025 16:40:21 +0000 (18:40 +0200)]
MINOR: pools: support creating a pool from a pool registration
We've recently introduced pool registrations to be able to enumerate
all pool creation requests with their respective parameters, but till
now they were only used for debugging ("show pools detailed"). Let's
go a step further and split create_pool() in two:
- the first half only allocates and sets the pool registration
- the second half creates the pool from the registration
This is what this patch does. This now opens the ability to pre-create
registrations and create pools directly from there.
MINOR: implement ha_aligned_alloc() to return aligned memory areas
We have two versions, _safe() which verifies and adjusts alignment,
and the regular one which trusts the caller. There's also a dedicated
ha_aligned_free() due to mingw.
The currently detected OSes are mingw, unixes older than POSIX 200112
which require memalign(), and those post 200112 which will use
posix_memalign(). Solaris 10 reports 200112 (probably through
_GNU_SOURCE since it does not do it by default), and Solaris 11 still
supports memalign() so for all Solaris we use memalign(). The memstats
wrappers are also implemented, and have the exported names. This was
the opportunity for providing a separate free call that lets the caller
specify the size (e.g. for use with pools).
Willy Tarreau [Tue, 5 Aug 2025 13:59:11 +0000 (15:59 +0200)]
BUILD: compat: always set _POSIX_VERSION to ease comparisons
Sometimes we need to compare it to known versions, let's make sure it's
always defined. We set it to zero if undefined so that it cannot match
any comparison.
Willy Tarreau [Wed, 6 Aug 2025 17:10:11 +0000 (19:10 +0200)]
BUILD: quic: use _MAX() to avoid build issues in pools declarations
With the upcoming pool declaration, we're filling a struct's fields,
while older versions were relying on initcalls which could be turned
to function declarations. Thus the compound expressions that were
usable there are not necessarily anymore, as witnessed here with
gcc-5.5 on solaris 10:
In file included from include/haproxy/quic_tx.h:26:0,
from src/quic_tx.c:15:
include/haproxy/compat.h:106:19: error: braced-group within expression allowed only inside a function
#define MAX(a, b) ({ \
^
include/haproxy/pool.h:41:11: note: in definition of macro '__REGISTER_POOL'
.size = _size, \
^
...
include/haproxy/quic_tx-t.h:6:29: note: in expansion of macro 'MAX'
#define QUIC_MAX_CC_BUFSIZE MAX(QUIC_INITIAL_IPV6_MTU, QUIC_INITIAL_IPV4_MTU)
Let's make the macro use _MAX() instead of MAX() since it relies on pure
constants.
Willy Tarreau [Wed, 6 Aug 2025 17:05:29 +0000 (19:05 +0200)]
BUILD: compat: provide relaxed versions of the MIN/MAX macros
In 3.0 the MIN/MAX macros were converted to compound expressions with
commit 0999e3d959 ("CLEANUP: compat: make the MIN/MAX macros more
reliable"). However with older compilers these are not supported out
of code blocks (e.g. to initialize variables or struct members). This
is the case on Solaris 10 with gcc-5.5 when QUIC doesn't compile
anymore with the future pool registration:
In file included from include/haproxy/quic_tx.h:26:0,
from src/quic_tx.c:15:
include/haproxy/compat.h:106:19: error: braced-group within expression allowed only inside a function
#define MAX(a, b) ({ \
^
include/haproxy/pool.h:41:11: note: in definition of macro '__REGISTER_POOL'
.size = _size, \
^
...
include/haproxy/quic_tx-t.h:6:29: note: in expansion of macro 'MAX'
#define QUIC_MAX_CC_BUFSIZE MAX(QUIC_INITIAL_IPV6_MTU, QUIC_INITIAL_IPV4_MTU)
Let's provide the old relaxed versions as _MIN/_MAX for use with constants
like such cases where it's certain that there is no risk. A previous attempt
using __builtin_constant_p() to switch between the variants did not work,
and it's really not worth the hassle of going this far.
Willy Tarreau [Wed, 6 Aug 2025 14:25:00 +0000 (16:25 +0200)]
BUILD: fcgi: fix the struct name of fcgi_flt_ctx
The struct was mistakenly spelled flt_fcgi_ctx() in fcgi_flt_stop()
when it was introduced in 2.1 with commit 78fbb9f991 ("MEDIUM:
fcgi-app: Add FCGI application and filter"), causing build issues
when trying to get the alignment of the object in pool_free() for
debugging purposes. No backport is needed as it's just used to convey
a pointer.
MINOR: sample: Add le2dec (little endian to decimal) sample fetch
This commit introduces a sample fetch, `le2dec`, to convert
little-endian binary input samples into their decimal representations.
The function converts the input into a string containing unsigned
integer numbers, with each number derived from a specified number of
input bytes. The numbers are separated using a user-defined separator.
This new sample is achieved by adding a parametrized sample_conv_2dec
function, unifying the logic for be2dec and le2dec converters.
Co-authored-by: Christian Norbert Menges <christian.norbert.menges@sap.com>
[wt: tracked as GH issue #2915] Signed-off-by: Willy Tarreau <w@1wt.eu>
BUG/MEDIUM: hlua_fcn: ensure systematic watcher cleanup for server list iterator
In 358166a ("BUG/MINOR: hlua_fcn: restore server pairs iterator pointer
consistency"), I wrongly assumed that because the iterator was a temporary
object, no specific cleanup was needed for the watcher.
In fact watcher_detach() is not only relevant for the watcher itself, but
especially for its parent list to remove the current watcher from it.
As iterators are temporary objects, failing to remove their watchers from
the server watcher list causes the server watcher list to be corrupted.
On a normal iteration sequence, the last watcher_next() receives NULL
as target so it successfully detaches the last watcher from the list.
However the corner case here is with interrupted iterators: users are
free to break away from the iteration loop when a specific condition is
met for instance from the lua script, when this happens
hlua_listable_servers_pairs_iterator() doesn't get a chance to detach the
last iterator.
Also, Lua doesn't tell us that the loop was interrupted,
so to fix the issue we rely on the garbage collector to force a last
detach right before the object is freed. To achieve that, watcher_detach()
was slightly modified so that it becomes possible to call it without
knowing if the watcher is already detached or not, if watcher_detach() is
called on a detached watcher, the function does nothing. This way it saves
the caller from having to track the watcher state and makes the API a
little more convenient to use. This way we now systematically call
watcher_detach() for server iterators right before they are garbage
collected.
This was first reported in GH #3055. It can be observed when the server
list is browsed one than more time when it was already browsed from Lua
for a given proxy and the iteration was interrupted before the end. As the
watcher list is corrupted, the common symptom is watcher_attach() or
watcher_next() not ending due to the internal mt_list call looping
forever.
Thanks to GH users @sabretus and @sabretus for their precious help.
BUG/MINOR: acme: possible integer underflow in acme_txt_record()
a2base64url() can return a negative value is olen is too short to
accept ilen. This is not supposed to happen since the sha256 should
always fit in a buffer. But this is confusing since a2base64()
returns a signed integer which is pt in output->data which is unsigned.
Fix the issue by setting ret to 0 instead of -1 upon error. And returns
a unsigned integer instead of a signed one.
This patch also checks the return value from the caller in order
to emit an error instead of setting trash.data which is already done
from the function.
MEDIUM: acme: allow to wait and restart the task for DNS-01
DNS-01 needs a external process which would register a TXT record on a
DNS provider, using a REST API or something else.
To achieve this, the process should read the dpapi sink and wait for
events. With the DNS-01 challenge, HAProxy will put the task to sleep
before asking the ACME server to achieve the challenge. The task then
need to be woke up, using the command implemented by this patch.
This patch implements the "acme challenge_ready" command which should be
used by the agent once the challenge was configured in order to wake the
task up.
MINOR: acme: emit the DNS-01 challenge details on the dpapi sink
This commit adds a new message to the dpapi sink which is emitted during
the new authorization request.
One message is emitted by challenge to resolve. The certificate name as
well as the thumprint of the account key are on the first line of the
message. A dump of the JSON response for 1 challenge is dumped, en the
message ends with a \0.
The agent consuming these messages MUST NOT access the URLs, and SHOULD
only uses the thumbprint, dns and token to configure a challenge.
BUG/MEDIUM: mux-quic: ensure Early-data header is set
QUIC MUX may be initialized prior to handshake completion, when 0-RTT is
used. In this case, connection is flagged with CO_FL_EARLY_SSL_HS, which
is notably used by wait-for-hs http rule.
Early data may be subject to replay attacks. For this reason, haproxy
adds the header 'Early-data: 1' to all requests handled as TLS early
data. Thus the server can reject it if it is deemed unsafe. This header
injection is implemented by http-ana. However, it was not functional
with QUIC due to missing CO_FL_EARLY_DATA connection flag.
Fix this by ensuring that QUIC MUX sets CO_FL_EARLY_DATA when needed.
This is performed during qcc_recv() for STREAM frame reception. It is
only set if QC_CF_WAIT_HS is set, meaning that the handshake is not yet
completed. After this, the request is considered safe and Early-data
header is not necessary anymore.
This should fix github issue #3054.
This must be backported up to 3.2 at least. If possible, it should be
backported to all stable releases as well. On these versions, the
current patch relies on the following refactoring commit :
commit 0a53a008d032b69377869c8caaec38f81bdd5bd6
MINOR: mux-quic: refactor wait-for-handshake support
Following the latest adjustment on session_add_conn() /
session_check_idle_conn(), detach muxes callbacks were rewritten for
private connection handling.
Nothing really fancy here : some more explicit comments and the removal
of a duplicate checks on idle conn status for muxes with true
multipexing support.
session_check_idle_conn() is called by muxes when a connection becomes
idle. It ensures that the session idle limit is not yet reached. Else,
the connection is removed from the session and it can be freed.
Prior to this patch, session_check_idle_conn() was compatible with a
NULL session argument. In this case, it would return true, considering
that no limit was reached and connection not removed.
However, this renders the function error-prone and subject to future
bugs. This patch streamlines it by ensuring it is never called with a
NULL argument. Thus it can now only returns true if connection is kept
in the session or false if it was removed, as first intended.
MINOR: session: do not release conn in session_check_idle_conn()
session_check_idle_conn() is called to flag a connection already
inserted in a session list as idle. If the session limit on the number
of idle connections (max-session-srv-conns) is exceeded, the connection
is removed from the session list.
In addition to the connection removal, session_check_idle_conn()
directly calls MUX destroy callback on the connection. This means the
connection is freed by the function itself and should not be used by the
caller anymore.
This is not practical when an alternative connection closure method
should be used, such as a graceful shutdown with QUIC. As such, remove
MUX destroy invokation : this is now the responsability of the caller to
either close or release immediately the connection.
Add a BUG_ON() on session_check_idle_conn() to ensure the connection is
not already flagged as CO_FL_SESS_IDLE.
This checks that this function is only called one time per connection
transition from active to idle. This is necessary to ensure that session
idle counter is only incremented one time per connection.
MINOR: session: remove redundant target argument from session_add_conn()
session_add_conn() uses three argument : connection and session
instances, plus a void pointer labelled as target. Typically, it
represents the server, but can also be a backend instance (for example
on dispatch).
In fact, this argument is redundant as <target> is already a member of
the connection. This commit simplifies session_add_conn() by removing
it. A BUG_ON() on target is extended to ensure it is never NULL.
MINOR: session: strengthen connection attach to session
This commit is the first one of a serie to refactor insertion of backend
private connection into the session list.
session_add_conn() is used to attach a connection into a session list.
Previously, this function would report an error if the connection
specified was already attached to another session. However, this case
currently never happens and thus can be considered as buggy.
Remove this check and replace it with a BUG_ON(). This allows to ensure
that session insertion remains consistent. The same check is also
transformed in session_check_idle_conn().
projects / thirdparty / haproxy.git / log
