Modify task_instant_wakeup() to use __task_set_state_and_tid().
It uses the new ownership behavior, but that's okay because
task_instant_wakeup() was not used anywhere.
Totally remove the per-thread group wait queue. This was potentially a
source of contention, because there were only a global lock for all
those wait queues.
Instead, for shared tasks, there is now the concept of ownership for the
task. When a task is in the wait queue, run queue, or is running on that
particular thread, the task's tid is set to -2 - thread_tid, and only
that thread will be responsible for it until it is no longer running,
and in none of its queue.
When a shared task is scheduled to be run at a later time, if its
current tid is -1, then the current thread will take ownership, and put
it in its own wait queue. If it is already owned, then TASK_WOKEN_WQ is
added to the task's state, and a task_wakeup() is done, so that the
owner thread will add it in its wait queue.
If there is any owner, then a task_wakeup() will just add the task to
the owner's runqueue, otherwise the current thread will become the
owner.
Introduce a new function, __task_get_current_owner, that returns the
owner of a task based on its current tid.
-1 means there is no current owner, otherwise either the tid is >= 0, in
which case it will just return it, or it's < -1, in which case it will
return -2 - tid, the tid of the thread with the current ownership.
Introduce __task_get_new_tid_field(), that provides the tid to be used
for a task.
For shared task, to mark temporary ownership of a task, instead of -1,
the tid will be set to -2-tid, tid being the tid of the current thread.
Introduce a new function, __task_set_state_and_tid, that atomically can
set a task's state and its tid. This will be used later, as the tid will
be used to indicate task ownership even for shared tasks.
Add EVENT_HDL_SUB_ACME_DEPLOY to the ACME family. It is published in
the dns-01 challenge path after the TXT record information has been
prepared, carrying the certificate store name, domain, account
thumbprint, dns_record value, and optionally the provider and vars
strings.
Lua subscribers using core.event_sub() receive the event data as an
AcmeEvent object, which is the same class used for ACME_NEWCERT and
carries the fields relevant to the event type.
Add a new EVENT_HDL_SUB_ACME_NEWCERT event type in the ACME family.
It is published after a new certificate has been successfully fetched
and installed. The event carries the certificate store name, allowing
subscribers to act on newly available certificates.
Lua subscribers using core.event_sub() receive the event data as an
AcmeEvent object with a crtname field containing the certificate store
name.
Right now the only way to report info that is only displayed in diag
mode with -dD is to use ha_diag_warning(). The problem is that this is
then counted as a warning and may result in errors when combined with
-dW, as happens for the CPU topology info:
$ printf "global\nstats socket /tmp/sock1\n" | ./haproxy -dD -dW -c -f /dev/stdin; echo $?
[NOTICE] (10406) : haproxy version is 3.5-dev0-5091ac-35
[NOTICE] (10406) : path to executable is ./haproxy
[DIAG] (10406) : Created 20 threads split into 2 groups
[ALERT] (10406) : Some warnings were found and 'zero-warning' is set. Aborting.
1
We need another level. This commit introduces ha_diag_notice() which only
emits a notification that doesn't count as a warning. Note that we could
even introduce an info level and revisit various messages so that notice
only reports certain events while info is for anything (like versions
above). That could be a future improvement.
The Linux tls module requires a socket to be in TCP_ESTABLISHED state
before we can enable the TLS ULP on the socket, if the socket is in any
other state, then the setsockopt() call will fail, and we won't use
kTLS on that socket.
To make sure we're not doing it too early, defer it until the TLS
handshake is done, which means the TCP connection is established.
This should be backported up to 3.3.
Signed-off-by: Karol Kucharski <kkucharski@fastlogic.pl>
__LJMP, WILL_LJMP() and MAY_LJMP() were defined locally in hlua.c,
making them unavailable to other modules that implement Lua bindings.
Move them to include/haproxy/hlua.h so they can be used outside of
hlua.c.
Add a registration mechanism so that modules outside of hlua.c can hook
into each lua_State creation. Modules call hap_register_hlua_state_init()
(or the REGISTER_HLUA_STATE_INIT() macro) with a callback of the form:
int my_init(lua_State *L, char **errmsg);
The callback returns an ERR_* code. ERR_ALERT and ERR_WARN trigger
ha_alert()/ha_warning() respectively; any other non-zero errmsg is
emitted via ha_notice(). ERR_FATAL or ERR_ABORT cause exit(1).
Registered entries are freed in hlua_deinit().
Extract the challenge-readiness logic from cli_acme_chall_ready_parse()
into a new acme_challenge_ready(crt, dns) function so it can be called
from other contexts such as Lua event handlers.
It slightly changes the messages on the CLI.
Having a single task to take care of idle connection cleanup across all
servers leads to high contention. It uses a lock to maintain its tree of
servers to track, and then can acquire the idle_conns lock for each thread.
Instead, have one task per thread. Each thread will maintain its own
tree, so there will be no need for any lock, and it will just acquire
its own idle_conns lock, so it will lead to less contention.
This is a performance improvement, so backporting is optional, but may be
considered if it is worth it. That would require backporting commit
6f8dab2583 too.
There's a corner case with get_trash_chunk_sz() combined with the use
of small bufs: if some incoming data is going to be inflated by a
converter in a non-predictable way (say url_enc etc) then there are
two possibilities:
- either we try to allocate a size that corresponds to the data, but
we risk to allocate a small buf to convert a 900B chunk, that will
now fail if it contains too many non-printable chars;
- or we try to allocate 3x the size to be conservative, but without
large bufs we'd fail to transcode any chunk larger than 5.3kB, even
if it contains only printable chars.
The approach should definitely be refined and it is not 100% reliable
for now. Better temporarily ignore the small buffers for these particular
cases where the savings are not relevant, and see how to pass the knowledge
of the expected size ranges deeper down the API in 3.5. We may possibly rely
on the current trash size (instead of contents) or other mechanisms that
are yet to be specified. alloc_small_trash_chunk() gets the same change
BTW for the same reasons.
The comment for get_trash_chunk_sz() was updated to restate the importance
of being conservative when requesting a size.
No backport is needed.
Historically, we considered a channel cannot send before the connection was
established. This was useful to know if the reserve should still be
respected for the receives. This was because it was possible to rewrite the
request on connection retry (because of http-send-name-header option).
However noadays, it is a useless limitation. Once data forwarding is
started, there is no longer rewrites on the request at the stream layer
(http-send-name-header option is handled by the muxes). And, since it is
possible to use small buffers to queue requests, it could be an issue,
because the reserve and the small buffer size are the same by default. Once
a small request was finally dequeued, the receives on client side were not
re-armed because we should still respect the reserve on receives
(channel_recv_limit() was returning 0 in that case).
To fix the issue, we must consider a channel may send since the underlying
stconn has reached the SC_ST_REQ state, instead of SC_ST_EST. Doing so, we
are able to ignore the reserve earlier and the receives can be re-armed even
with small buffers.
There is no reason to backport this patch, except if an issue is reported,
because only the 3.4 is concerned. But it could theorically be backported to
all stable versions.
This adds "-dA[file]" on the command line, which dumps an archive of all
dependencies detected at runtime into the designated file in tar format.
This is equivalent to "set-dumpable libs", but instead of keeping the libs
in memory, it dumps them into a file. This may be used after a core dump,
in order to provide all necessary libraries to developers to permit them
to exploit the core. This may not be available on all operating systems.
When shared libs were loaded via "set-dumpable libs", better release
them upon deinit, it will make valgrind happier. For this we now have
a new function free_collected_libs() in tools.c and call it in deinit().
In in46un_to_addr(), when copying a struct sockaddr_in6, copy the
sin6_flowinfo and sin6_scope_id, as they are part of the structure too.
They are unlikely to be of any use for us, but this is more correct
anyway.
Allocating and freeing an OpenSSL EVP_PKEY_CTX context via
EVP_PKEY_CTX_new_id() and EVP_PKEY_CTX_free() on every HKDF cryptographic
operation (such as during stateless reset token generation) induces
unnecessary memory allocation overhead.
Optimize this by introducing a global per-thread context array
'quic_tls_hkdf_ctxs'. These contexts are allocated and initialized once
at startup via a POST_CHECK hook (quic_tls_alloc_hkdf_ctxs) and are
properly freed at exit via a POST_DEINIT hook (quic_tls_dealloc_hkdf_ctxs).
The functions quic_hkdf_extract(), quic_hkdf_expand(), and
quic_hkdf_extract_and_expand() now reuse the pre-allocated context
corresponding to the current thread ID ('tid'), removing dynamic
allocations from these frequent execution paths.
As a cleanup, quic_hkdf_expand() is now static and unexported from the
header file.
Should be easily backported to all versions for optimization purposes.
QPACK_LFL_WLN_BIT and related encoded field line bitmasks were defined
in both qpack-enc.c and qpack-dec.c. Moved them to qpack-t.h where
they are shared between encoder and decoder, eliminating the duplicate
definitions.
Should be backported to ease any further commit to come.
Although qpack_idx_to_name and qpack_idx_to_value are currently only
called within uncompiled debug code, they contained an index bug. They
passed absolute indexes directly to qpack_get_dte instead of relative
dynamic table indexes.
This patch fixes the logic by subtracting QPACK_SHT_SIZE and guarding
against static table index lookups.
Should be easily backported to all versions.
Thanks to previous patches, the request messages are now sanitized to
properly handle Upgrade requests. Now, if a 'connection: upgrade' header
value was found while no 'Upgrade' header, the 'upgrade' values is removed
from the 'connection' header. Conversely the opposite is also performed. If
'Upgrade' header was found, but no "conneciotn: upgrade" header value, all
occurrences of 'Upgrade' header are refused.
This patch depends on following ones:
* MINOR: h1: Add a H1M flag to specify a non-empty 'Upgrade:' header was parsed
* MINOR: http: Add function to remove all occurrences of a value in a header
It should fix the issue 3397. But the H2 part should be reviewed too, and
probably the H1 response parsing, to be consistent with this change.
The series should be backported as far as 2.4.
http_remove_header_value() function was added to parse a header value and
remove all occurrences of a specific value.
This patch is mandatory to fix a bug.
The current PRNG is xoroshiro128**, it was introduced in 2.2 with
commit 52bf83939 ("BUG/MEDIUM: random: implement a thread-safe and
process-safe PRNG"). It features a 2^128 sequence and can perform
2^64 or 2^96 jumps, though only the 2^96 jump is implemented. It
was initially designed to support both processes and threads, and
implements a shared state between threads instead of allocating
distinct sequences based on PID and thread numbers.
Since then, the PRNG's usage grew and processes have disappeared,
but the lock or the DWCAS are still there due to its shared nature,
and it's possible to trigger watchdog warnings by issuing 100 UUIDs
in a single log-format string.
Also, UUID and QUIC retry tokens now consume 128 bits from the PRNG
in two 64-bit calls, and used to weaken the PRNG by rapidly disclosing
its internal state on reasonably idle systems. This indicates that
most of the time we now need 128 bits.
This patch modernizes the internal generator by switching to xoshiro256**,
which has comparable properties (it's even faster), and features even
longer 2^256 periods, still returning 64 bits per call. It can be
initialized with 2^128 and 2^192 jumps. More details here:
https://prng.di.unimi.it/https://prng.di.unimi.it/xoshiro256starstar.c
Here we implement a thread-local state instead of the old shared one,
so there is no more need for synchronization. The state is seeded at
boot, and each thread performs as many 2^192 jumps as their TID is
large. The master process performs a 2^128 jump where it used to
perform a 2^96 jump so that it doesn't overlap with any worker thread.
However a cleaner approach could be to perform a 2^128 jump for each
fork() (here the worker) and 2^192 for each thread. This might be for
a future improvement.
ha_random64_internal() is now the new PRNG, so that everything else
remains totally transparent. _ha_random64_pair_hashed() continues to
hash the first 128 bits of the state.
A simple config generating 100 UUID on 20 threads jumps from 135k to
1.25M req/s, which translates to a bump from 13.5M to 125M UUID/s,
or 9 times faster. And there is no more DWCAS can be seen anymore
in perf top:
Before: 13.5M/s
Overhead Shared Object Symbol
99.04% haproxy [.] ha_random64_internal
0.66% haproxy [.] _ha_random64_pair_hashed
0.03% libc-2.42.so [.] __printf_buffer
0.02% [kernel] [k] _raw_spin_lock
0.01% libc-2.42.so [.] __strchrnul_avx2
0.01% [kernel] [k] ktime_get
0.01% [kernel] [k] lapic_next_deadline
0.01% haproxy [.] sample_process
0.01% haproxy [.] chunk_printf
0.01% libc-2.42.so [.] __printf_buffer_write
0.01% [kernel] [k] hrtimer_active
0.01% libc-2.42.so [.] __memmove_avx_unaligned_erms
0.01% libc-2.42.so [.] _itoa_word
After: 125M/s
18.84% libc-2.42.so [.] __printf_buffer
9.84% haproxy [.] sample_process
8.33% libc-2.42.so [.] __strchrnul_avx2
6.61% libc-2.42.so [.] __memmove_avx_unaligned_erms
6.06% libc-2.42.so [.] __printf_buffer_write
4.43% haproxy [.] strlcpy2
4.09% libc-2.42.so [.] _itoa_word
2.62% haproxy [.] sess_build_logline_orig
2.12% haproxy [.] _ha_random64_pair_hashed
1.28% haproxy [.] pool_put_to_cache
1.06% haproxy [.] __pool_alloc
1.00% haproxy [.] smp_fetch_uuid
0.93% haproxy [.] lf_text_len
0.82% haproxy [.] ha_generate_uuid_v4
A lot of places call two ha_random64() in a row to generate a 128-bit
random. While it's now safe against linear analysis thanks to the XXH64
call, it's still particularly expensive due to the lock.
Here we introduce a new function ha_random64_pair_hashed(), that feeds
two uint64_t with a hash of the PRNG's internal state, and make it
advance. This will cut in half the number of calls to ha_random64()
and should recover a part of the performance lost in the lock. For
now it's not used.
Consuming randoms in pairs directly exposes the internal PRNG's state
on moderately idle system. It can allow to predict next (or previous)
UUIDs, QUIC retry tokens, and WS keys for example. Let's insert an XXH64
call on the ha_random64() output to avoid this. We expand the boot seed
as the secret at boot, and use now_ns as the seed for each call. The
original ha_random64() function was renamed to ha_random64_internal()
for use cases where it's not a problem to directly use the internal
state.
The performance loss is only measurable when single-threaded. It drops
from 7.32M UUID per second to 7.16M. Above that there is no longer any
difference due to the DWCAS loop which reaches up to 98.5% CPU at 20
threads.
This will need to be backported to stable releases after a period of
observation.
In resolv_validate_dns_response(), the second DNS record parsing path
manually constructs a 32-bit big-endian TTL value from four individual
bytes using the expression:
reader[0] * 16777216 + reader[1] * 65536 + reader[2] * 256 + reader[3]
We have read_n32() to do this, and it's more robust against unexpected
signedness surprises (which should not happen right here since reader is
unsigned char and we use -fwrapv so the result is defined). Also, let's
make the ttl an uint instead of an int. The TTL is only retrieved and not
used for now, so better clean it now.
Cross-compilation on m68k fails in ssl_sock_resize_passphrase_cache()
where the compiler noticed the SIZE_MAX passed to realloc() in the
error path and complained that it's larger than PTRDIFF_MAX. This can
be disabled with -Walloc-size-larger-than=SIZE_MAX but in practice we
can simply hide the value and keep the warning to detect real failures
elsewhere. Let's pass it through DISGUISE() and also take this
opportunity for doing that inside an unlikely() clause since it's never
supposed to happen.
The check on the OCSP response expire time is based on the "Next Update"
field of the response, converted by my_timegm function that returns a
time_t (signed long). It is then stored in the 'expire' field of the
certificate_ocsp structure which is typed as a signed long.
When loading an OCSP response, if the "Next Update" time is too far in
the future and we are running on a 32 bits machine, we might end up with
negative times ireturned by my_timegm, which make the comparison with
the current date fail and raises the "OCSP single response: no longer
valid." error message.
This problem typically happens in the ocsp_auto_update.vtc regtest since
the loaded OCSP response have a "Next Update" field in 2050.
This patch simply changes the type of the expire field to an unsigned
long since the 'my_timegm' function does not return '-1' in case of
error, contrary to the standard 'timegm' one.
Ths patch can be backported to all stable branches.
Define a new connection flag mask CO_FL_WAIT_XPRT_L6. This will be used
to indicate that a XPRT layer is running on top of layer 6. For now,
only xprt_qmux implements this method of operation.
When QMux protocol is used, xprt_qmux layer is setup after SSL handshake
completion but prior to the MUX initialization. Once transport
parameters exchange is successful, the layer is removed and the MUX is
started.
The layer setup operation was performed directly on ssl_sock_io_cb().
Simplify the code by extracting it in a dedicated function
xprt_add_l6hs(). The function is generic so the requested XPRT layer
must be passed as argument.
The code is mostly identical. One difference is that a check is
performed to ensure no SSL handshake is pending. If this is the case,
the function is a noop. This will become useful to support QMux
transparently both in clear or on top of SSL.
Another minor addition is that CO_FL_XPRT_READY flag is automatically
resetted by xprt_add_l6hs(). This allows the code to use
conn_xprt_start() standard function after XPRT init.
Sometimes we'd like to know if some products overflow, so let's add a
pair of functions for this, for ulong and for size_t. For recent enough
compilers (gcc >= 5, clang >= 3.4) we just use __builtin_mul_overflow()
otherwise we rely on a division and a comparison before performing the
operation.
A third function, array_size_or_fail() computes the size of an array
of m elements of n bytes each, and returns the total size if it fits
in a size_t, otherwise ~0 if it does not so that passing this to
malloc() or any other variant would fail by trying to exhaust the
entire memory space.
This patch add the ability to the htx_xfer() function to transfer data
without acounting the meta-data. By default, the <count> variable includes
the meta-data. But by setting the flag HTX_XFER_NO_METADATA, It is possible
to transfer HTX blocks without count meta-data. In that case, <count> will
not contain the blocks meta-data and the return value will not include them.
HTX_XFER_* flags must be declared as a bitfield. However, value of
HTX_XFER_HDRS_ONLY was set of 0x03 while it should be 0x04. So let's fix it.
This patch must be backported where the htx_xfer() function was backported
(5ead611cc "MEDIUM: htx: Add htx_xfer function to replace htx_xfer_blks").
Fix usage of stream.max-concurrent QUIC setting on the backend side.
Contrary to frontend connections, this limit must be enforced by QUIC
MUX directly. This is necessary as the peer may allow a larger number of
concurrent streams via its flow control.
First, QUIC TP initial max bidi streams value is now set to 0. This is
fine as only the HTTP/3 client is expected to open bidirectional
streams.
The most important changes is performed in qcm_avail_streams(). The
value first depends on the peer flow control. Now, it is further reduced
if necessary to not exceed the configured BE stream.max-concurrent.
Note that this new behavior may further increases current limitation on
QUIC BE reuse when a QCS instance is kept while its upper stream layer
is detached. In this case there is a risk that the connection is not
reinserted in the correct server pool, as an idle or avail one.
This is a breaking change as BE stream.max-concurrent keyword setting
meaning is changed in effect. However, this does not necessitate extra
warnings as the previous usage was in effect useless. Furthermore, QUIC
on the backend side is still considered as experimental.
This can be backported up to 3.3.
This reduces the total code size by 6-10% and speeds up the build a
bit. It can be further reduced by disabling the trace decoding code
inside certain subsystems like muxes. But at least like this it will
help users on small systems to reduce the footprint when not needed
by explicitly passing USE_TRACE=0 (they remain enabled by default).
When we store the negociated server parameters, such as the ALPN, also
store the calculated hash with the connection. If it is different, as
can happen because the IP address is different because set-dst was used,
we certainly do not want to reuse the information in the cache,
otherwise we could end up using the wrong ALPN and mux.
That means we already have to calculate the hash in connect_server()
now, while before we would not do it for Websockets, if we could not do
connection reuse, as that's all the hash was used for.
This should fix Github issue #3386
This should be backported as far as 3.2.
Add a new QCC member <ms_bidi_rel>. This represents the number of
concurrent streams advertised similarly to ms_bidi, but as a relative
value.
This patch does not introduce any functional change. For now,
<ms_bidi_rel> will be equal to <ms_bidi_init>. However, with the
implementation of stream elasticity and dynamic adjustment for
concurrent max-streams-bidi, the former will be required to keep the
last advertised value.
When server fleets are constantly updated, using a stable distribution
across a bunch of load balancers can be convenient. The addr and port
already provide a bit of this but for situations were addresses might
differ between sites or change dynamically this does not work. The guid
is perfect for this because by definition it's supposed to designate a
single server and be unique. So when two servers anywhere have the same,
the tool that provisionned them promises that they are the same server.
So here we introduce "hash-key guid" which performs a 32-bit hash on
the GUID value. When no guid is provided, a fallback is performed on
ID, as is done for other keys.
The "id" hash-key scales the ID by a factor of 16 that tries to leave
room between the nodes on the 32-bit space to permit smooth weight
variations (e.g. during slowstart). However this does not deal well
with overlaps between server IDs. For example, assigning IDs that are
only multiples of 256 million to 16 servers yields traffic only on
one since in practice they all have the same 28 lower bits.
The new "id32" hash key bridges this gap by using the full 32-bit ID
of the server as the key. On the other hand, the user must be careful
not to switch the hash function to "none" when using incremental IDs
because in this case they might be very poorly distributed. But this
can be convenient for automated provisionning systems which assign
IDs themselves, as the full 32 bits are used now.
The first part of this patch defines a new mux_proto_list field named
<xprt_init>. This allows to define an extra XPRT layer which should be
activated first prior to the MUX creation both on frontend and backend
sides.
This is immediately used for QMux mux_proto_list to require XPRT_QMUX
handshake. With this change, activation of QMux connection flags in
session_accept_fd() and connect_server() are adjusted to take into
account <init_xprt> field. This approach is much more evolutive than
relying on the previous MUX name.
Change in connect_server() will also be necessary to support QMux
activation on a TCP server with h3 ALPN without explicit "proto qmux".
This guarantees that MUX initialization is delayed after QMux handshake.
This patch is similar to the previous one but this time for backend
connections. The MUX selection code is directly extracted from
conn_install_mux_chk() and conn_install_mux_be().
Define a new function conn_select_mux_fe().
The objective is to have a preliminary function to determine the MUX
which will be used without initializing it. This will be useful for MUX
which relies on a specific XPRT handshake prior to its startup, which is
the case for QMux protocol.
The code of conn_select_mux_fe() is identical to the beginning of
conn_install_mux_fe() with a similar MUX selection logic. However,
connection MUX initialization is not performed in this case. In a future
patch, both functions should be merged together to reduce code
duplication.
In conn_get_best_mux() and conn_get_best_mux_entry(), the mux name was
provided sometimes based on the "proto" directive, sometimes based on
the ALPN, but in any case, it was compared again the mux_proto_list
mux_proto field. This is not correct, as ALPN can be different from the
internal mux_proto. So enhance those functions so that they wll accept
an ALPN as well. If a mux_proto is provided, that will be used, if not,
and if an ALPN is provided, then that will be used, and compared against
the ALPN provided by the mux, if any.
In struct mux_proto_list, rename the "token" field to "mux_proto". That
field should only be used to match the name provided in the "proto"
directive, and it will be soon.
This should be a no-op.
During the H1 message parsing, the Upgrade header values are checked to
detect "h2c" and "h2" tokens and skip them. To do so, we rely on
H1_MF_UPG_H2C flag, set during the parsing. And during the request
post-parsing, if this flag was set, all Upgrade headers are removed.
This was fixed by the commit 7b89aa5b1 ("BUG/MINOR: h1: do not forward h2c
upgrade header token").
However, there are two issues here and the commit above must be refined.
First, the flag is reset for each new Upgrade header. So "h2c" or "h2"
tokens will be properly detected if all tokens are set on the same Upgrade
header. But if splitted on several headers, previously detected tokens will
be hidden by a next ones.
Concretly, the following will be properly caught
Connection: upgrade
Upgrade: foo, h2c, bar
But then following not:
Connection: upgrade:
Upgrade: foo, h2c
Upgrade: bar
Then, when a "h2c" or "h2" token is finally reported, all Upgrade headers
are removed, regardless other tokens.
So, to fix the both issues, everything is now handled during the message
parsing by skipping "h2c" and "h2" tokens, rebuilding the Upgrade header
value without then offending tokens. The same was already performed for the
Connection header, to skip "keep-alive" and "close" value. So it is not a so
fancy change.
Thanks to this change, it is no longer necessary to handle H1_MF_UPG_H2C
during the request post-parsing. And in fact, this flag is no longer
necessary. So let's remove it too.
Thanks to Vincent55 for finding and reporting this.
This patch must be backported as far as 2.4.
Unlike stated in the configuration manual, the server 'init-state' parameter
was not evaluated during haproxy startup/reload. After a review, it appeared
there were also issues if combined with the 'track' parameter. In addtition,
this parameter was only evaluated when health-checks were enabled for the
server, leading to unexpected behavior if the serve settings are dynamically
changed via the CLI.
To fix those issues, behavior of the 'init-state' parameter was slightly
adapted. It is always evaluated, even when there is no running health-checks
for the server. An error is reported if the 'track' parameter is also
defined. Both cannot work together.
In addition, the "none" state was introduced to be able to restore the
default behavior. It will be especially useful when the parameter is
inherited from a 'default-server' directive.
This patch should fix the issue #3298. It must be backported as far as 3.2.
The function cli_find_frontend was declared twice identically at lines 98-99
of include/haproxy/proxy.h. The second declaration should have been for
cli_find_backend, which is defined in src/proxy.c and used in several places
but was missing from the header's exported symbols.
This is a simple copy-paste mistake where line 99 duplicated line 98 verbatim
instead of declaring cli_find_backend.
Support of pseudo-headers was removed as unused, but mention of it
in the description remains and disrupt the numbering in comment, which
can be confusing.
Add a new "alias" member in trace_source structure. Its purpose is to be
an alternative to the member "name". This will be used in the next patch
to allow renaming of QUIC mux traces while preserving compatibility.
This new member is only used in trace_find_source() which is the helper
used to retrieve a trace source from its name.
This is a follow-up on the QUIC MUX renaming process.
The current patch performs renaming in xprt_qmux layer. Older "qstrm"
identifier is replaced by the new name "qmux". Every remaining functions
and structures in xprt_qmux are changed. Outside effects are only
present in QUIC MUX which directly uses some of these functions.
This is a follow-up on the QUIC MUX renaming process.
The current patch performs renaming of "qstrm" to "qmux" in connection
flags. These flags are only used in linked with the xprt_qmux layer.
This has an impact on every files which manipulates these flags, namely
backend, session and ssl_sock sources.
Also, internal xprt identifier is renamed from XPRT_QSTRM to XPRT_QMUX,
This is a follow-up on the QUIC MUX renaming process.
The current patch replaces "qstrm" naming with "qmux" in QUIC MUX source
file. Some members are impacted in qcc and qcs structures, as well as
some internal functions used for QMux receive/send. Internal mux_ops is
also rename to qmux_ops. This is not a breaking change as its externally
visible name was already set to "qmux" originally.
This is a follow-up on the QUIC MUX renaming process. Now most of "qmux"
generic usages as been replaced in favor of "qcm" naming. The next part
of the renaming is to replace "qstrm" naming with "qmux" for stuffs
related to the new QMux protocol specifically.
This is first applied on filenames. As with the previous renaming,
Makefile and include statements are updated as well to prevent
compilation issues.
This is a follow-up on the QUIC MUX renaming process.
This patch renames several definitions in QUIC MUX traces source code.
This is only an internal change. Trace source name is kept to "qmux" for
now, but will be changed in a dedicated patch.
This is a follow-up on the QUIC MUX renaming process.
A previous patch already renames MUX ops callbacks. The current patch
proceed to a similar operation for the rest of the MUX functions.
This patch is the first one of the renaming serie, affecting the QUIC
MUX module. The objective is to remove older "qmux" naming which was
used as a generic identifier. Now it should be restricted to the QMux
experimental protocol. A new "qcm" naming will replace the generic
usage.
The current patch renames the files themselves. Token "qmux" is replaced
by the new "qcm" identifier. Makefile and include statements are
adjusted as required.
This patch adds the missing include of xprt_qstrm header into its
companion source file. This helped to detect an incoherence in the
xprt_qstrm_xfer_rxbuf() prototype which is now fixed.
Header files is also updated with mandatory include statements and
forward declaration.
No backport needed.
Some outdated comments, as well as typos were fixed in the following files:
dgram.h protocol.h queue-t.h cpu_topo.c debug.c dict.c
protocol.c queue.c raw_sock.c trace.c wdt.c
Dmitry Sivachenko reported a build warning on FreeBSD -dev, where
__nonstring is apparently already defined. Let's guard our own
definition to avoid such issues. It could make sense to backport
this to recent stable versions which may soon be exposed to modern
compilers.
In task_schedule(), it is not enough to get the TASK_RUNNING flag before
setting the expire field, we also have to keep it while queueing the
taks, otherwise the task may run in the meanwhile and set expire to 0,
triggering the BUG_ON() in __task_queue() again. So now, only drop the
running flag once it's done.
This should be backported up to 2.8.
In comp_prepare_compress_request(), the compression rate limit and CPU
usage checks were duplicated. The first set runs before selecting the
algorithm, and the second set runs after. That's definitely a copy-paste
issue or a patch being applied twice. Let's just drop one.
This fixes typos and spelling mistakes in the following files:
channel-t.h channel.h filters-t.h http_htx.h htx-t.h tools.h
cfgcond.c channel.c flt_http_comp.c http_ana.c htx.c mqtt.c
mux_h1.c regex.c stats-proxy.c
This lets lb_ops specify the conditions necessary to bind to this set of
ops. The condition is expressed as a list of mask and match fields on
the algorithm flags. This is then used in proxy_finalize() to locate the
lb_ops corresponding to the current configuration, by iterating over
the list of lb_ops structures. This list is implemented using the same
mechanisms used for configuration keywords: an INITCALL1 macro to a
registration function.
This also moves the lookup and property flags into the lb_ops structure
that were previously applied manually on a case by case basis.
This adds a new tune.streams-elasticity parameter. This parameter
indicates, as a percentage, the average number of streams per connection
at full load. It is used to calculate limits of the number of streams to
advertise on new connections. 0 means that no such limit is set.
When a limit is set, the new function conn_calc_max_streams() determines
the optimal number of streams to allow on a connection. It will assign at
least the ratio of streams left to connections left, and at least a fair
share of what's left times the number of desired streams. It will always
ensure that each connection gets at least 1 stream, and everything beyond
this will be evenly distributed. For now the function is not used.
In order to be able to enforce global streams limitations, we'll first
have to be able to account how many streams we promised to serve via
frontend muxes. We'll always need to support at least one stream, which
is why here we're only counting extra streams beyond the first one. It
also has the benefit of leaving H1 out of this, and save it from updating
a variable. Also in order to avoid an important update cost, we're storing
this value per thread group. For now only H2 is implemented, but QUIC
should follow shortly and should only count bidirectional streams.
This implements configuration for custom mac alg in EAB.
I don't think there are any reasons to allow that TBH,
but it is something that exists in the spec.
Depends on the EAB impl.
No backport needed
Patch introduces ACME EAB support.
Configuring EAB requires two parts: Key ID and MAC Key.
Key ID is an ASCII string that specifies the name of the record CA
should look up. MAC Key is a base64url encoded key that is used
for the sake of JWS signing, using HS256 or other algorithms.
They are the credentials so must be stored securely.
A thing about EAB is that it is required only during account creation
so it is unexpectedly complex to think about.
Some CAs provide EAB credential pair that is reused between
multiple account order requests, for example ZeroSSL, but others like
Google Trusted Services require an unique EAB credential for each new
account creation request.
There are a lot of ways config could be implemented, I decided to make
so that Key ID and MAC Key are stored in separate files on disk,
that decision was made because of the security concerns.
File based approach in particular works well with systemd credentials,
works well with systems that have config world readable, or immutable,
and is compatible with existing setups that specify credentials in a
file.
EAB is configured through options like this in an acme section:
eab-mac-alg HS512
eab-mac-key pebble.eab.mac-key
eab-key-id pebble.eab.key-id
I decided to not error out on empty files, but issue a log msg instead,
so that credentials can be removed without changing the haproxy config.
Used read_line_to_trash function from tools.c for reading files,
that is something that could be replaced by a dedicated function too.
No backport needed
New jws_b64_hmac_signature() duplicates the same functionality as
jws_b64_signature(), but for the use case of HMAC signing.
Intended to be used for ACME EAB.
OpenSSL allows to use EVP_PKEY for HMAC functionality, so
jws_b64_signature() could be reused, but the problem is that although
isn't deprecated it was removed in BoringSSL, and was removed
(due to BoringSSL roots) but then readded back in AWS-LC, because of
"legacy clients" (citing them), for that reason alone I say that having
a dedicated function for hmac is better, HMAC() macro seems to be widely
supported unlike other ways of doing same thing. Another alternative
would be to use EVP_MD API, but it was introduced in OpenSSL 3.0,
so not as widely supported.
In order for the code behind the "commit ssl cert" logic to be usable
outside of the CLI context, some new "ckch_store_update_" functions are
created. They allow to perform all the operations on ckch_stores to be
performed without needing an appctx.
The first function being called is ckch_store_update_init which mainly
takes the ckch_store lock and checks that there is an ongoing
transaction with the proper path (which was already done in
cli_parse_commit_cert).
The main one is ckch_store_update_process which replicates the logic
that could be found in the cli_io_handler_commit_cert function. We
iterate over the ckch instances of an existing ckch store and duplicate
them in the new ckch store which is still detached from the tree, before
replacing the old store with the new one. This whole operation could
take some time so we were yielding every 10 instances or when
applet_putstr calls would fail. The actual ckch_store operations and the
applet related calls are now decorrelated in order to stop having to
have an appctx during the ckch store/instances processing.
The ckch_store_update_process will now update a "msg" buffer and a
"state" that allow to send processing messages to the caller as well as
keep the state of the processing "state machine".
When the ckch_store_update_process loop is over,
ckch_store_update_cleanup can be called to release the lock and free
some now useless structures.
The ckch instances for a given ckch_store have to be rebuilt when a
certificate is updated during runtime (via cli or lua). The code was
duplicated in lua so factorizing the actual loop avoids future errors
if the code changes. The new 'ckch_store_rebuild_instances' will have a
dedicated 0 return code if it needs to be called again (because of the
yielding logic since ckch instance rebuild might take some time).
Muxes are responsible to release connections once they are inactive and
won't be reusable. In QUIC mux, such connections are detected via
qcc_is_dead(). The first precondition is that there is no more upper
streams attached. This was accounted via QCC <nb_sc> counter.
A special characteristic of QCS instances is that they can be in
detached state : upper stream has been removed but there is still data
to emit. Such QCS were not taken into account in qcc_is_dead(), so a
connection could be freed with some remaining data not yet emitted.
It is also not possible for QUIC MUX to simply look at the QCS tree to
determine if the connection is inactive. Indeed, some streams are opened
for protocol internal usage. This is the case for example with HTTP/3
unidirectional control stream or QPACK encoder/decoder streams. These
streams are never closed. In the end, only requests streams should be
taken into account for the connection activity.
This patch improves the situation by reworking <nb_hreq> QCC counter.
Previously, it served for http-request timeout implementation. However,
this timeout only relies on <opening_list> now. Thus, <nb_hreq> scope is
changed : it is now incremented via qcs_wait_http_req(), used by app
protocol layer once a request stream is identified. Decrement is
performed on qcs_free(), so this guarantees that a connection cannot be
freed anymore if request streams still exists, unless if inactivity
timeout fires. As such, <nb_hreq> now supersedes <nb_sc> entirely, so
the qcc_is_dead() can now relies on the former.
Along with this change, qcc_timeout_task() must be updated. Call to
qcc_is_dead() was unnecessary prior to this patch as timeout handling
was only active when no upper streams were attached. When tested, both
<nb_sc> and QCC <task> were already null, so a connection was always
released on timeout, as expected. With qcc_is_dead() now checking
<nb_hreq> instead, this is not always the case anymore. In fact, this
check is unnecessary as inactivity timeout serves precisely to free a
stucked connection with remaining data to emit.
This patch also has some impact on http-keep-alive timeout. Previously,
this timeout could be armed if only detached streams remained. Now, it
is only applicable if all QCS request instances are closed and freed.
Thus, qcc_reset_idle_start() is now closed directly on qcs_free().
Ideally this should be backported up to 2.6, or at least 2.8 as QUIC
experimental status was removed there.
MUX implements a timeout for HTTP keep-alive which monitors the delay
between two HTTP requests. This is only applicable for frontend
connections, as on the backend side idle connections can be kept in the
server pool. In QUIC mux, this timeout relies on QCC <idle_start> which
is refresh when the last request is terminated.
This patch modifies the refresh operation so that it is only performed
for frontend connections. This is not strictly necessary but the timeout
timeout management is now clearer and it eliminates an unnecessary
operation for backend connections.
Similarly, http-request timeout is also only applicable for frontend
connections. This relies on qcs_wait_http_req() function. A request QCS
is inserted in <opening_list> until the headers are received. This is
unnecessary on the backend side so this is excluded as well.
req_body field is no longer used, except in trace messages. And in fact, it
is not necessarily true if some data are received with the request headers.
So no reason to still use it.
Function h2_phdr_to_list() was missing the decoding for the :protocol
header and would emit :UNKNOWN in this case. It's only used in traces
so it's not important. The fix can be backported in all versions.
New type sockaddr_in46 has been recently introduced. It serves as a
union which can store either an IPv4 or IPv6 address. The objective is
to reduce the storage size for QUIC datagrams which previously uses a
sockaddr_storage field.
On qc_new_conn(), source and destination addresses from the datagram are
passed to the function as sockaddr_storage so that they are copied into
the newly built quic_conn instance. However, the involved memcpy() is
producing a buffer overflow as sockaddr_in46 is smaller than
sockaddr_storage type.
This patch fixes this by defining a new helper function
in46un_to_addr(). This allows to convert safely sockaddr_in46 to a plain
sockaddr type. The function is now used before invoking qc_new_conn().
Note that there is still other several places where union sockaddr_in46
is casted as sockaddr_storage type. However, these should be safe as in
these cases sockaddr fields are accessed individually after checking
ss_family. The memory issue only exists when plain memcpy is used.
This bug was detected using ASAN. It generates the following traces when
a QUIC connection is instantiated.
==37474==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7c3bb9a61100 at pc 0x5631f52c3946 bp 0x7ffc83e45b50 sp 0x7ffc83e45310
READ of size 128 at 0x7c3bb9a61100 thread T0
#0 0x5631f52c3945 in __asan_memcpy (/home/amaury/work/haproxy-quic-dev/haproxy+0x3ae945) (BuildId: a7ccfd74b7a71a869b8ff8d13f6dcde8c82c1487)
#1 0x5631f55f9e34 in qc_new_conn /home/amaury/work/haproxy-quic-dev/src/quic_conn.c:1311:2
#2 0x5631f558d5c3 in quic_rx_pkt_retrieve_conn /home/amaury/work/haproxy-quic-dev/src/quic_rx.c:1875:10
#3 0x5631f558330b in quic_dgram_parse /home/amaury/work/haproxy-quic-dev/src/quic_rx.c:2463:29
#4 0x5631f5625da6 in quic_lstnr_dghdlr /home/amaury/work/haproxy-quic-dev/src/quic_sock.c:206:3
#5 0x5631f6a64173 in run_tasks_from_lists /home/amaury/work/haproxy-quic-dev/src/task.c:660:26
#6 0x5631f6a6ba1e in process_runnable_tasks /home/amaury/work/haproxy-quic-dev/src/task.c:913:9
#7 0x5631f5e984c3 in run_poll_loop /home/amaury/work/haproxy-quic-dev/src/haproxy.c:2982:3
#8 0x5631f5e9a715 in run_thread_poll_loop /home/amaury/work/haproxy-quic-dev/src/haproxy.c:3212:2
#9 0x5631f5e9f732 in main /home/amaury/work/haproxy-quic-dev/src/haproxy.c:3853:2
#10 0x7f2bba8276c0 (/usr/lib/libc.so.6+0x276c0) (BuildId: ca0db5ab57a36507d61bbcf4988d344974331f19)
#11 0x7f2bba8277f8 in __libc_start_main (/usr/lib/libc.so.6+0x277f8) (BuildId: ca0db5ab57a36507d61bbcf4988d344974331f19)
#12 0x5631f51be594 in _start (/home/amaury/work/haproxy-quic-dev/haproxy+0x2a9594) (BuildId: a7ccfd74b7a71a869b8ff8d13f6dcde8c82c1487)
No need to backport.
Previously, shut callback was entirely implemented in QUIC mux layer.
However, this operation depends on the above application protocol, as it
may define its own closure procedure and error codes. This is the case
notably with HTTP/3 specification.
This patch defines a stream shut API between QUIC mux and application
protocol layers via the new qcc_app_ops callback lclose(). The closure
reason is specified via an enum argument. Application protcol can then
perform the stream closure as intended.
This patch is only an architecture adjustment but should not have any
functional impact. Stream closure logic was moved identically from QUIC
mux into h3 and h09 lclose callback.
The QUIC code can only handle IPv4 or IPv6 addresses, so using two
sockaddr_storage structs wastes a lot of space in the quic_dgram struct.
This is a very large overhead since this structure is written in the MPSC
ring buffers before every datagram, while many of those datagrams are only
50 bytes or less. Using an union instead saves 200 bytes per datagram,
increasing the capacity of the buffers significantly.
Using an offset instead of a pointer into the datagram buffer is less
error-prone as we do not have to manually fixup that pointer when the
datagram is moved somewhere else in memory.
