Frédéric Lécaille [Wed, 2 Jun 2021 13:27:34 +0000 (15:27 +0200)]
MINOR: quic: quic_update_ack_ranges_list() code factorization
Very minor modification to avoid repeating the same code section in this function
when allocation new ack range.
Frédéric Lécaille [Wed, 2 Jun 2021 13:04:03 +0000 (15:04 +0200)]
BUG/MINOR: quic: Wrong memory free in quic_update_ack_ranges_list()
Wrong call to free() in place of pool_free() for an object allocated from a pool
memory.
Frédéric Lécaille [Mon, 31 May 2021 16:04:07 +0000 (18:04 +0200)]
MINOR: quic: Remove header protection also for Initial packets
Make qc_try_rm_hp() be able to remove the header protection of Initial packets
which are the first incoming packets of a connection without context.
Frédéric Lécaille [Mon, 31 May 2021 13:16:13 +0000 (15:16 +0200)]
MINOR: quic: Derive the initial secrets asap
Make depends qc_new_isecs() only on quic_conn struct initialization only (no more
dependency on connection struct initialization) to be able to run it as soon as
the quic_conn struct is initialized (from the I/O handler) before running ->accept()
quic proto callback.
Frédéric Lécaille [Mon, 31 May 2021 08:24:49 +0000 (10:24 +0200)]
MINOR: quic: Remove header protection for conn with context
We remove the header protection of packet only for connection with already
initialized context. This latter keep traces of the connection state.
Furthermore, we enqueue the first Initial packet for a new connection
after having completely parsed the packet so that to not start the accept
process for nothing.
Frédéric Lécaille [Mon, 31 May 2021 07:30:14 +0000 (09:30 +0200)]
MINOR: quic: QUIC conn initialization from I/O handler
Move the QUIC conn (struct quic_conn) initialization from quic_sock_accept_conn()
to qc_lstnr_pkt_rcv() as this is done for the server part.
Move the timer initialization to ->start xprt callback to ensure the connection
context is done : it is initialized by the ->accept callback which may be run
by another thread than the one for the I/O handler which also run ->start.
Frédéric Lécaille [Fri, 28 May 2021 08:25:07 +0000 (10:25 +0200)]
BUG/MINOR: quic: Do not check the acception of a new conn from I/O handler.
As the ->conn member of quic_conn struct is reset to NULL value by the ->accept
callback potentially run by another thread, this check is irrelevant.
Frédéric Lécaille [Thu, 27 May 2021 15:12:36 +0000 (17:12 +0200)]
MINOR: quic: Initialize the session before starting the xprt.
We must ensure the session and the mux are initialized before starting the xprt.
Frédéric Lécaille [Thu, 27 May 2021 12:57:09 +0000 (14:57 +0200)]
MINOR: quic: Move an SSL func call from QUIC I/O handler to the xprt init.
Move the call to SSL_set_quic_transport_params() from the listener I/O dgram
handler to the ->init() callback of the xprt (qc_conn_init()) which initializes
its context where is stored the SSL context itself, needed by
SSL_set_quic_transport_params(). Furthermore this is already what is done for the
server counterpart of ->init() QUIC xprt callback. As the ->init() may be run
by another thread than the one for the I/O handler, the xprt context could
not be potentially already initialized before calling SSL_set_quic_transport_params()
from the I/O handler.
Frédéric Lécaille [Tue, 25 May 2021 07:12:35 +0000 (09:12 +0200)]
MINOR: quic: Enable some quic, h3 and qpack modules compilation.
Enable src/h3.c compilation for h3, src/qpack-dec.c and sr/qpack-tbl.c
compilation for qpack, and src/mux_quic.c for the QUIC mux.
Frédéric Lécaille [Mon, 17 May 2021 14:42:21 +0000 (16:42 +0200)]
MINOR: quic: Replace max_packet_size by max_udp_payload size.
The name the maximum packet size transport parameter was ambiguous and replaced
by maximum UDP payload size. Our code would be also ambiguous if it does not
reflect this change.
Frédéric Lécaille [Tue, 23 Mar 2021 13:46:56 +0000 (14:46 +0100)]
MINOR: quic: Avoid header collisions
Extract the QUIC varints encoding functions from xprt_quic.h to avoid
header collisions.
Frédéric Lécaille [Thu, 18 Mar 2021 14:05:18 +0000 (15:05 +0100)]
MINOR: quic: Update the streams transport parameters.
Set the streams transport parameters which could not be initialized because they
were not available during initializations. Indeed, the streams transport parameters
are provided by the peer during the handshake.
Frédéric Lécaille [Thu, 18 Mar 2021 14:01:27 +0000 (15:01 +0100)]
MINOR: quic: Add a wrapper function to update transport parameters.
This function calls quic_mux_transport_params_update() to update the related
streams transport parameter of the mux. It is there only so that not to have
to include mux_quic.h to update these parameters.
Frédéric Lécaille [Thu, 11 Mar 2021 16:06:30 +0000 (17:06 +0100)]
BUG/MINOR: quic: Wrong ->accept() error handling
Really signal the caller that ->accept() has failed if the session could not
be initialized because conn_complete_session() has failed. This is the case
if the mux could not be initialized too.
When it fails an ->accept() must returns -1 in case of resource shortage.
Frédéric Lécaille [Thu, 4 Mar 2021 18:12:20 +0000 (19:12 +0100)]
MINOR: quic: Variable-length integer encoding/decoding into/from buffer struct.
Add a function to encode a QUIC varint into a buffer struct. Samething for the
deconding part.
Frédéric Lécaille [Wed, 3 Mar 2021 16:28:34 +0000 (17:28 +0100)]
MINOR: quic: Add callbacks for (un)scribing to QUIC xprt.
Add these callbacks so that the QUIC mux may (un)scribe to the read/write xprt
events.
Frédéric Lécaille [Wed, 3 Mar 2021 15:23:44 +0000 (16:23 +0100)]
MINOR: quic: Disable the action of ->rcv_buf() xprt callback
Deactivate the action of this callback at this time. I am not sure
we will keep it for QUIC as it does not really make sense for QUIC:
the QUIC packet are already recvfrom()'ed by the low level I/O handler
used for all the connections.
Frédéric Lécaille [Wed, 3 Mar 2021 15:21:00 +0000 (16:21 +0100)]
MINOR: quic_sock: Finalize the QUIC connections.
Add a call to conn_connection_complete() so that to install the mux any
QUIC connection.
Frédéric Lécaille [Wed, 3 Mar 2021 15:13:10 +0000 (16:13 +0100)]
MINOR: qpack: Add QPACK compression.
Implement QPACK used for HTTP header compression by h3.
Frédéric Lécaille [Wed, 3 Mar 2021 15:09:02 +0000 (16:09 +0100)]
MINOR: h3: Add HTTP/3 definitions.
Add all the definitions for HTTP/3 implementation.
Frédéric Lécaille [Wed, 3 Mar 2021 15:04:28 +0000 (16:04 +0100)]
MINOR: quic: Add a new definition to store STREAM frames.
Add a new structure to store enough information about STREAM frames which
must be stored before being delivered to the application layer, for any
reason.
Frédéric Lécaille [Thu, 18 Feb 2021 09:16:06 +0000 (10:16 +0100)]
MINOR: quic: Attach QUIC mux connection objet to QUIC connection.
This add a qcc struct for QUIC mux/demux connection layer to quic_conn struct
at low level connection layer.
Frédéric Lécaille [Thu, 18 Feb 2021 09:04:27 +0000 (10:04 +0100)]
MINOR: connection: Add callbacks definitions for QUIC.
The flow control at stream level is organized by types (client bidi, server bidi,
client uni, server uni). Adds at least callback to retrieve the number
of available streams by direction.
Frédéric Lécaille [Thu, 18 Feb 2021 08:59:01 +0000 (09:59 +0100)]
MINOR: mux_quic: Add QUIC mux layer.
This file has been derived from mux_h2.c removing all h2 parts. At
QUIC mux layer, there must not be any reference to http. This will be the
responsability of the application layer (h3) to open streams handled by the mux.
Frédéric Lécaille [Thu, 28 Jan 2021 15:22:52 +0000 (16:22 +0100)]
MINOR: quic: Move transport parmaters to anynomous struct.
We move ->params transport parameters to ->rx.params. They are the
transport parameters which will be sent to the peer, and used for
the endpoint flow control. So, they will be used to received packets
from the peer (RX part).
Also move ->rx_tps transport parameters to ->tx.params. They are the
transport parameter which are sent by the peer, and used to respect
its flow control limits. So, they will be used when sending packets
to the peer (TX part).
Frédéric Lécaille [Mon, 11 Jan 2021 14:10:06 +0000 (15:10 +0100)]
BUG/MINOR: quic: Possible NULL pointer dereferencing when dumping streams.
This bug may occur when displaying streams traces. It came with this commit:
242fb1b63 ("MINOR: quic: Drop packets with STREAM frames with wrong direction.").
Willy Tarreau [Wed, 22 Sep 2021 05:15:57 +0000 (07:15 +0200)]
BUG/MEDIUM: leastconn: fix rare possibility of divide by zero
An optimization was brought in commit
5064ab6a9 ("OPTIM: lb-leastconn:
do not unlink the server if it did not change") to avoid locking the
server just to discover it did not move. However a mistake was made
because the operation involves a divide with a value that is read
outside of its usual lock, which makes it possible to be zero at the
exact moment we watch it if another thread takes the server down under
the lbprm lock, resulting in a divide by zero.
Therefore we must check that the value is not null there.
This must be backported to 2.4.
Willy Tarreau [Tue, 21 Sep 2021 12:31:29 +0000 (14:31 +0200)]
MEDIUM: listener: deprecate "process" in favor of "thread" on bind lines
The "process" directive on "bind" lines becomes quite confusing considering
that the only allowed value is 1 for the process, and that threads are
optional and come after the mandatory "1/".
Let's introduce a new "thread" directive to directly configure thread
numbers, and mark "process" as deprecated. Now "process" will emit a
warning and will suggest how to be replaced with "thread" instead.
The doc was updated accordingly (mostly a copy-paste of the previous
description which was already up to date).
This is marked as MEDIUM as it will impact users having "zero-warning"
and "process" specified.
Amaury Denoyelle [Tue, 21 Sep 2021 09:51:54 +0000 (11:51 +0200)]
MINOR: server: enable slowstart for dynamic server
Enable the 'slowstart' keyword for dynamic servers. The slowstart task
is allocated in 'add server' handler if slowstart is used.
As the server is created in disabled state, there is no need to start
the task. The slowstart task will be automatically started on the first
'enable server' invocation.
Amaury Denoyelle [Tue, 21 Sep 2021 09:51:29 +0000 (11:51 +0200)]
REORG: server: move slowstart init outside of checks
'slowstart' can be used without check on a server, with the CLI handlers
'enable/disable server'. Move the code to initialize and start the
slowstart task outside of check.c.
This change will also be reused to enable slowstart for dynamic servers.
Amaury Denoyelle [Mon, 20 Sep 2021 13:16:12 +0000 (15:16 +0200)]
MINOR: server: enable more check related keywords for dynamic servers
Allow to use the check related keywords defined in server.c. These
keywords can be enabled now that checks have been implemented for
dynamic servers.
Here is the list of the new keywords supported :
- error-limit
- observe
- on-error
- on-marked-down
- on-marked-up
Amaury Denoyelle [Mon, 20 Sep 2021 13:15:19 +0000 (15:15 +0200)]
MINOR: server: enable more keywords for ssl checks for dynamic servers
Allow to configure ssl support for dynamic server checks independently
of the ssl server configuration. This is done via the keyword
"check-ssl". Also enable to configure the sni/alpn used for the check
via "check-sni/alpn".
Amaury Denoyelle [Mon, 20 Sep 2021 13:31:42 +0000 (15:31 +0200)]
BUG/MINOR: server: alloc dynamic srv ssl ctx if proxy uses ssl chk rule
The ssl context is not initialized for a dynamic server, even if there
is a tcpcheck rule which uses ssl on the related backed. This will cause
the check initialization to failed with the message :
"Out of memory when initializing an SSL connection"
This can be reproduced by having the following config in the backend :
option tcp-check
tcp-check connect ssl
and create a dynamic server with check activated and a ca-file.
Fix this by calling the prepare_srv xprt callback when the proxy options
PR_O_TCPCKH_SSL is set.
Check support for dynamic servers has been merged in the current branch.
No backport needed.
Amaury Denoyelle [Tue, 21 Sep 2021 08:29:09 +0000 (10:29 +0200)]
BUG/MINOR: server: allow 'enable health' only if check configured
Test that checks have been configured on the server before enabling via
the 'enable health' CLI. This mirrors the 'enable agent' command.
Without this, a user can use the command on the server without checks.
This leaves the server in an undefined state. Notably, the stat page
reports the server in check transition.
This condition was left on the following reorg commit.
2c04eda8b58636ad2ae44e42b1f50f3b5a24a642
REORG: cli: move "{enable|disable} health" to server.c
This should be backported up to 1.8.
Tim Duesterhus [Thu, 16 Sep 2021 15:38:27 +0000 (17:38 +0200)]
CLEANUP: Remove unreachable `break` from parse_time_err()
The `return` already leaves the function.
Tim Duesterhus [Thu, 16 Sep 2021 15:38:26 +0000 (17:38 +0200)]
CLEANUP: Include check.h in flt_spoe.c
This is required for the prototype of spoe_prepare_healthcheck_request().
William Lallemand [Mon, 20 Sep 2021 14:19:15 +0000 (16:19 +0200)]
MINOR: httpclient: add the EOH when no headers where provided
httpclient_req_gen() now adds the end of headers block when no header
was provided, which avoid adding it manually.
Dragan Dosen [Mon, 20 Sep 2021 07:29:19 +0000 (09:29 +0200)]
BUG/MINOR: flt-trace: fix an infinite loop when random-parsing is set
The issue is introduced with the commit
c41d8bd65 ("CLEANUP: flt-trace:
Remove unused random-parsing option").
This must be backported everywhere the above commit is.
Tim Duesterhus [Wed, 15 Sep 2021 11:58:47 +0000 (13:58 +0200)]
DEV: coccinelle: Add xalloc_cast.cocci
This remove's C++ style casts from the return value of malloc/calloc.
see
403fd722ace1d98d3cfe17bbee1382bf58040466
Tim Duesterhus [Wed, 15 Sep 2021 11:58:46 +0000 (13:58 +0200)]
CLEANUP: Apply xalloc_size.cocci
This fixes a few locations with a hardcoded type within `sizeof()`.
Tim Duesterhus [Wed, 15 Sep 2021 11:58:48 +0000 (13:58 +0200)]
DEV: coccinelle: Add bug_on.cocci
This replaces an if + ABORT_NOW() by BUG_ON(). It might change behavior,
because BUG_ON will result in a no-op if not enabled.
Tim Duesterhus [Wed, 15 Sep 2021 11:58:45 +0000 (13:58 +0200)]
DEV: coccinelle: Add xalloc_size.cocci
This commits the Coccinelle patch to clean up sizeof handling for malloc/calloc.
Tim Duesterhus [Wed, 15 Sep 2021 11:58:49 +0000 (13:58 +0200)]
CLEANUP: Apply bug_on.cocci
The changes look safe to me, even if `DEBUG_STRICT` is not enabled.
Tim Duesterhus [Wed, 15 Sep 2021 11:58:43 +0000 (13:58 +0200)]
DEV: coccinelle: Add ist.cocci
This commits the Coccinelle patch to clean up ist handling.
Tim Duesterhus [Wed, 15 Sep 2021 11:58:44 +0000 (13:58 +0200)]
CLEANUP: Apply ist.cocci
This cleans up ist handling.
Willy Tarreau [Mon, 13 Sep 2021 16:42:07 +0000 (18:42 +0200)]
REORG: threads: move ha_get_pthread_id() to tinfo.h
This solely manipulates the thread_info struct, it ought to be in
tinfo.h, not in thread.h.
Willy Tarreau [Mon, 13 Sep 2021 08:07:38 +0000 (10:07 +0200)]
MINOR: applet: remove the thread mask from appctx_new()
appctx_new() is exclusively called with tid_bit and it only uses the
mask to pass it to the accompanying task. There is no point requiring
the caller to know about a mask there, nor is there any point in
creating an applet outside of the context of its own thread anyway.
Let's drop this and pass tid_bit to task_new() directly.
Willy Tarreau [Fri, 17 Sep 2021 10:00:27 +0000 (12:00 +0200)]
BUILD: fd: remove unused variable totlen in fd_write_frag_line()
Ilya reports in GH #1392 that clang 13 complains about totlen being
calculated and not used in fd_write_frag_line(), which is true. It's
a leftover of some older code.
Willy Tarreau [Fri, 17 Sep 2021 09:56:25 +0000 (11:56 +0200)]
BUILD: proto_uxst: do not set unused flag
Similarly to previous patch for sockpair, UNIX sockets set the
CONNECT_HAS_DATA flag without using it later, we can drop it.
Willy Tarreau [Fri, 17 Sep 2021 09:56:25 +0000 (11:56 +0200)]
BUILD: sockpair: do not set unused flag
Ilya reports in GH #1392 that clang 13 complains about a flag being added
to the "flags" parameter without being used later. That's generic code
that was shared from TCP but we can indeed drop this flag since it's used
for TFO which we don't have in socketpairs.
Willy Tarreau [Fri, 17 Sep 2021 09:07:45 +0000 (11:07 +0200)]
BUG/MINOR: cli/payload: do not search for args inside payload
The CLI's payload parser is over-complicated and as such contains more
bugs than needed. One of them is that it uses strstr() to find the
ending tag, ignoring spaces before it, while the argument locator
creates a new arg on each space, without checking if the end of the
word appears past the previously found end. This results in "<<" being
considered as the start of a new argument if preceeded by more than
one space, and the payload being damaged with a \0 inserted at the
first space or tab.
Let's make an easily backportable fix for now. This fix makes sure that
the trailing zero from the first line is properly kept after '<<' and
that the end tag is looked for only as an isolated argument and nothing
else. This also gets rid of the unsuitable strstr() call and now makes
sure that strcspn() will not return elements that are found in the
payload.
For the long term the loop must be rewritten to get rid of those
unsuitable strcspn() and strstr() calls which work past each other, and
the cli_parse_request() function should be split into a tokenizer and
an executor that are used from the caller instead of letting the caller
play games with what it finds there.
This should be backported wherever CLI payload is supported, i.e. 2.0+.
Amaury Denoyelle [Tue, 18 May 2021 09:33:57 +0000 (11:33 +0200)]
BUILD: ist: prevent gcc11 maybe-uninitialized warning on istalloc
A new warning is reported by gcc11 when using a pointer to uninitialized
memory block for a function with a const pointer argument. The warning
is triggered for istalloc, used by http_client.c / proxy.c / tcpcheck.c.
This warning is reported because the uninitialized memory block
allocated by malloc should not be passed to a const argument as in ist2.
See https://gcc.gnu.org/onlinedocs/gcc-11.1.0/gcc/Warning-Options.html#index-Wmaybe-uninitialized
This should be backported up to 2.2.
Amaury Denoyelle [Thu, 16 Sep 2021 10:15:12 +0000 (12:15 +0200)]
BUG/MINOR: connection: prevent null deref on mux cleanup task allocation
Move the code to allocate/free the mux cleanup task outside of the polling
loop. A new thread_alloc/free handler is registered for this in
connection.c.
This has the benefit to clean up the polling loop code. And as another
benefit, if the task allocation fails, the handler can report an error
to exit the haproxy process. This prevents a potential null pointer
dereferencing.
This should fix the github issue #1389.
This must be backported up to 2.4.
William Lallemand [Thu, 16 Sep 2021 15:30:51 +0000 (17:30 +0200)]
DOC: management: certificate files must be sanitized before injection
A lot of people encounter problems when trying to inject a certificate
file which contains extra informations or empty lines.
This patch adds a paragraph and a sanitizing example.
Must be backported as far as 2.1.
Christopher Faulet [Thu, 16 Sep 2021 14:01:09 +0000 (16:01 +0200)]
BUG/MINOR: tcpcheck: Improve LDAP response parsing to fix LDAP check
When the LDAP response is parsed, the message length is not properly
decoded. While it works for LDAP servers encoding it on 1 byte, it does not
work for those using a multi-bytes encoding. Among others, Active Directory
servers seems to encode messages or elements length on 4 bytes.
In this patch, we only handle length of BindResponse messages encoded on 1,
2 or 4 bytes. In theory, it may be encoded on any bytes number less than 127
bytes. But it is useless to make this part too complex. It should be ok this
way.
This patch should fix the issue #1390. It should be backported to all stable
versions. While it should be easy to backport it as far as 2.2, the patch
will have to be totally rewritten for lower versions.
Willy Tarreau [Thu, 16 Sep 2021 07:18:21 +0000 (09:18 +0200)]
MINOR: pools: use mallinfo2() when available instead of mallinfo()
Ilya reported in issue #1391 a build warning on Fedora about mallinfo()
being deprecated in favor of mallinfo2() since glibc-2.33. Let's add
support for it. This should be backported where the following commit is
also backported:
157e39303 ("MINOR: pools: automatically disable
malloc_trim() with external allocators").
Willy Tarreau [Thu, 16 Sep 2021 07:07:41 +0000 (09:07 +0200)]
DOC: update Tim's address in .mailmap
Let's keep the ASCII-only version that's already used in most commits
and presents well everywhere.
Christopher Faulet [Thu, 16 Sep 2021 06:16:23 +0000 (08:16 +0200)]
BUG/MAJOR: mux-h1: Don't eval input data if an error was reported
If an error was already reported on the H1 connection, pending input data
must not be (re)evaluated in h1_process(). Otherwise an unexpected internal
error will be reported, in addition of the first one. And on some
conditions, this may generate an infinite loop because the mux tries to send
an internal error but it fails to do so thus it loops to retry.
This patch should fix the issue #1356. It must be backported to 2.4.
Christopher Faulet [Fri, 10 Sep 2021 13:17:45 +0000 (15:17 +0200)]
CLEANUP: acl: Remove unused variable when releasing an acl expression
The "unresolved" variable is unused since commit 9fa0df5 ("BUG/MINOR: acl:
Fix freeing of expr->smp in prune_acl_expr").
This patch should fix the issue #1359.
Tim Duesterhus [Sun, 12 Sep 2021 11:21:54 +0000 (13:21 +0200)]
CLEANUP: Remove prototype for non-existent thread_get_default_count()
This is the only location of `thread_get_default_count` within the codebase.
Tim Duesterhus [Sun, 12 Sep 2021 10:49:33 +0000 (12:49 +0200)]
CLEANUP: tree-wide: fix prototypes for functions taking no arguments.
"f(void)" is the correct and preferred form for a function taking no
argument, while some places use the older "f()". These were reported
by clang's -Wmissing-prototypes, for example:
src/cpuset.c:111:5: warning: no previous prototype for function 'ha_cpuset_size' [-Wmissing-prototypes]
int ha_cpuset_size()
include/haproxy/cpuset.h:42:5: note: this declaration is not a prototype; add 'void' to make it a prototype for a zero-parameter function
int ha_cpuset_size();
^
void
This aggregate patch fixes this for the following functions:
ha_backtrace_to_stderr(), ha_cpuset_size(), ha_panic(), ha_random64(),
ha_thread_dump_all_to_trash(), get_exec_path(), check_config_validity(),
mworker_child_nb(), mworker_cli_proxy_(create|stop)(),
mworker_cleantasks(), mworker_cleanlisteners(), mworker_ext_launch_all(),
mworker_reload(), mworker_(env|proc_list)_to_(proc_list|env)(),
mworker_(un|)block_signals(), proxy_adjust_all_maxconn(),
proxy_destroy_all_defaults(), get_tainted(),
pool_total_(allocated|used)(), thread_isolate(_full|)(),
thread(_sync|)_release(), thread_harmless_till_end(),
thread_cpu_mask_forced(), dequeue_all_listeners(), next_timer_expiry(),
wake_expired_tasks(), process_runnable_tasks(), init_acl(),
init_buffer(), (de|)init_log_buffers(), (de|)init_pollers(),
fork_poller(), pool_destroy_all(), pool_evict_from_local_caches(),
pool_total_failures(), dump_pools_to_trash(), cfg_run_diagnostics(),
tv_init_(process|thread)_date(), __signal_process_queue(),
deinit_signals(), haproxy_unblock_signals()
Tim Duesterhus [Sun, 12 Sep 2021 13:55:04 +0000 (15:55 +0200)]
DOC: Add .mailmap
Ensure that Tim's last name is consistent no matter how the patch is generated
and applied, preventing Tim from showing up as two different persons in the
shortlog in releases.
Willy Tarreau [Wed, 15 Sep 2021 08:41:24 +0000 (10:41 +0200)]
MINOR: pools: report it when malloc_trim() is enabled
Since we can detect it at runtime now, it could help to have it mentioned
in haproxy -vv.
Willy Tarreau [Wed, 15 Sep 2021 08:05:48 +0000 (10:05 +0200)]
MINOR: pools: automatically disable malloc_trim() with external allocators
Pierre Cheynier reported some occasional crashes in malloc_trim() on a
recent glibc when running with jemalloc(). While in theory there should
not be any link between the two, it remains plausible that something
allocated early with one is tentatively freed with the other and that
attempts to trim end up badly. There's no point calling the glibc specific
malloc_trim() with external allocators anyway. However these ones are often
enabled at link time or even at run time with LD_PRELOAD, so we cannot rely
on build options for this.
This patch implements runtime detection for the allocator in use by checking
with mallinfo() that a malloc() call is properly accounted for in glibc's
malloc. It only enables malloc_trim() in this case, and ignores it for
other cases. It's fine to proceed like this because mallinfo() is provided
by a wider range of glibcs than malloc_trim().
This could be backported to 2.4 and 2.3. If so, it will also need previous
patch "CLEANUP: pools: factor all malloc_trim() calls into trim_all_pools()".
Willy Tarreau [Wed, 15 Sep 2021 08:38:21 +0000 (10:38 +0200)]
CLEANUP: pools: factor all malloc_trim() calls into trim_all_pools()
The code was slightly cleaned up by removing repeated occurrences of ifdefs
and moving that into a single trim_all_pools() function.
Willy Tarreau [Wed, 15 Sep 2021 08:30:40 +0000 (10:30 +0200)]
BUILD: sample: fix format warning on 32-bit archs in sample_conv_be2dec_check()
The sizeof() was printed as a long but it's just an unsigned on some
32-bit platforms, hence the format warning. No backport is needed, as
this arrived in 2.5 with commit
40ca09c7b ("MINOR: sample: Add be2dec
converter").
Willy Tarreau [Wed, 15 Sep 2021 08:15:03 +0000 (10:15 +0200)]
BUG/MINOR: compat: make sure __WORDSIZE is always defined
-Wundef triggered on a MIPS-based musl build on __WORDSIZE that's used
in ultoa_o() and some Lua initialization. The former will fail to convert
integers larger to 1 billion to proper string in this case. Let's make
sure this macro is defined and fall back to values determined from
__SIZEOF_LONG__ otherwise. A cleaner long-term approach would consist
in removing all remaining occurrences of this macro.
This can be backported to all versions.
Willy Tarreau [Wed, 15 Sep 2021 08:12:04 +0000 (10:12 +0200)]
BUILD: threads: fix -Wundef for _POSIX_PRIORITY_SCHEDULING on libmusl
Building with an old musl-based toolchain reported this warning:
include/haproxy/thread.h: In function 'ha_thread_relax':
include/haproxy/thread.h:256:5: warning: "_POSIX_PRIORITY_SCHEDULING" is not defined [-Wundef]
#if _POSIX_PRIORITY_SCHEDULING
^
There were indeed two "#if" insteadd of #ifdef" for this macro, let's
fix them.
Willy Tarreau [Mon, 13 Sep 2021 07:32:01 +0000 (09:32 +0200)]
BUILD: halog: fix a -Wundef warning on non-glibc systems
Dmitry reported this warning on FreeBSD since the introduction of -Wundef:
admin/halog/fgets2.c:38:30: warning: '__GLIBC__' is not defined, evaluates to 0 [-Wundef]
#if defined(__x86_64__) && (__GLIBC__ > 2 || (__GLIBC__ == 2 && __GLIBC_MINOR__ >= 15))
^
A defined() was missing.
Willy Tarreau [Mon, 13 Sep 2021 07:30:09 +0000 (09:30 +0200)]
BUILD: compiler: fixed a missing test on defined(__GNUC__)
This one could theoretically trigger -Wundef on non-gcc compatible
compilers if DEBUG_USE_ABORT is not set.
Willy Tarreau [Sun, 12 Sep 2021 09:36:38 +0000 (11:36 +0200)]
[RELEASE] Released version 2.5-dev7
Released version 2.5-dev7 with the following main changes :
- BUG/MINOR: config: reject configs using HTTP with bufsize >= 256 MB
- CLEANUP: htx: remove comments about "must be < 256 MB"
- BUG/MAJOR: htx: fix missing header name length check in htx_add_header/trailer
- Revert "BUG/MINOR: stream-int: Don't block reads in si_update_rx() if chn may receive"
- MINOR: proxy: add a global "grace" directive to postpone soft-stop
- MINOR: vars: rename vars_init() to vars_init_head()
- CLEANUP: vars: rename sample_clear_stream() to var_unset()
- REORG: vars: remerge sample_store{,_stream}() into var_set()
- MEDIUM: vars: make the ifexist variant of set-var only apply to the proc scope
- MINOR: vars: add a VF_CREATEONLY flag for creation
- MINOR: vars: support storing empty sample data with a variable
- MINOR: vars: store flags into variables and add VF_PERMANENT
- MEDIUM: vars: make var_clear() only reset VF_PERMANENT variables
- MEDIUM: vars: pre-create parsed SCOPE_PROC variables as permanent ones
- MINOR: vars: preset a random seed to hash variables names
- MEDIUM: vars: replace the global name index with a hash
- CLEANUP: vars: remove the now unused var_names array
- MINOR: vars: centralize the lock/unlock into static inlines
- OPTIM: vars: only takes the variables lock on shared entries
- OPTIM: vars: remove internal bookkeeping for vars_global_size
- OPTIM: vars: do not keep variables usage stats if no limit is set
- BUILD: fix dragonfly build again on __read_mostly
- CI: Github Actions: temporarily disable Opentracing
- BUG/MEDIUM: mux-h1: Remove "Upgrade:" header for requests with payload
- MINOR: htx: Skip headers with no value when adding a header list to a message
- CLEANUP: mux-h1: Remove condition rejecting upgrade requests with payload
- BUG/MEDIUM: stream-int: Don't block SI on a channel policy if EOI is reached
- BUG/MEDIUM: http-ana: Reset channels analysers when returning an error
- BUG/MINOR: filters: Set right FLT_END analyser depending on channel
- CLEANUP: Add haproxy/xxhash.h to avoid modifying import/xxhash.h
- CLEANUP: ebmbtree: Replace always-taken elseif by else
- CLEANUP: Move XXH3 macro from haproxy/compat.h to haproxy/xxhash.h
- BUILD: opentracing: exclude the use of haproxy variables for the OpenTracing context
- BUG/MINOR: opentracing: enable the use of http headers without a set value
- CLEANUP: opentracing: use the haproxy function to generate uuid
- MINOR: opentracing: change the scope of the variable 'ot.uuid' from 'sess' to 'txn'
- CI: Github Actions: re-enable Opentracing
- CLEANUP: opentracing: simplify the condition on the empty header
- BUG/MEDIUM lua: Add missing call to RESET_SAFE_LJMP in hlua_filter_new()
Tim Duesterhus [Sat, 11 Sep 2021 21:17:25 +0000 (23:17 +0200)]
BUG/MEDIUM lua: Add missing call to RESET_SAFE_LJMP in hlua_filter_new()
In one case before exiting leaving the function the panic handler was not
reset.
Introduced in
69c581a09271e91d306e7b9080502a36abdc415e, which is 2.5+.
No backport required.
Miroslav Zagorac [Sun, 12 Sep 2021 06:15:37 +0000 (08:15 +0200)]
CLEANUP: opentracing: simplify the condition on the empty header
The second patch from the last series of patches has been redesigned
here, the ist() function is used to set an empty string instead of
working directly with the string pointer.
I thank Tim Düsterhus for his advice.
Willy Tarreau [Sun, 12 Sep 2021 05:01:36 +0000 (07:01 +0200)]
CI: Github Actions: re-enable Opentracing
Miroslav already fixed the build of OpenTracing so we can re-enable it
in the CI. For now variables are disabled but this will change soon.
Miroslav Zagorac [Thu, 9 Sep 2021 12:19:25 +0000 (14:19 +0200)]
MINOR: opentracing: change the scope of the variable 'ot.uuid' from 'sess' to 'txn'
At the suggestion of Willy Tarreau, the scope of the 'ot.uuid' variable was
changed from 'sess' to 'txn', so it is now limited to the transaction only.
Miroslav Zagorac [Thu, 9 Sep 2021 08:31:12 +0000 (10:31 +0200)]
CLEANUP: opentracing: use the haproxy function to generate uuid
To avoid duplicate source code, the original haproxy function is used to
generate the OpenTracing runtime context UUID.
Also, the structure flt_ot_runtime_context is simplified because the
detailed definition of UUID is removed from it (struct flt_ot_uuid),
ie the UUID is left only in the form of a string.
Miroslav Zagorac [Wed, 8 Sep 2021 23:23:42 +0000 (01:23 +0200)]
BUG/MINOR: opentracing: enable the use of http headers without a set value
In case we transfer some data that does not have a set value via the HTTP
header, adding that header in the text map was done incorrectly.
This simple patch allows the use of HTTP headers without a set value.
Miroslav Zagorac [Wed, 8 Sep 2021 18:56:05 +0000 (20:56 +0200)]
BUILD: opentracing: exclude the use of haproxy variables for the OpenTracing context
Due to a recent change in the handling of haproxy variables, their use for
OpenTracing context transfer has been excluded from the compilation process.
The use of variables can be re-enabled if the newly defined variable
OT_USE_VARS is set to 1 when calling the 'make' utility. However,
this should not be used for now as the compilation will end in error.
This change prevents the use of haproxy variables to convey the OpenTracing
context. This means that the 'use-vars' parameter cannot be used in the
OpenTracing filter configuration for 'inject' and 'extract' operations.
An example configuration that uses this feature is in the test/ctx
directory, while the script to run that test is test/run-ctx.sh.
Then, the 'sess.ot.uuid' variable is no longer set when initializing the
OpenTracing session. This means that this variable can still be used in
the OpenTracing configuration, but its contents will be empty.
Tim Duesterhus [Sat, 11 Sep 2021 18:29:46 +0000 (20:29 +0200)]
CLEANUP: Move XXH3 macro from haproxy/compat.h to haproxy/xxhash.h
This moves all the xxhash functionality into a single location.
see
d5fc8fcb86eb99831626051b3055bea7ca93a074
Tim Düsterhus [Sat, 11 Sep 2021 15:02:33 +0000 (17:02 +0200)]
CLEANUP: ebmbtree: Replace always-taken elseif by else
`diff` is guaranteed to be less than 0, because the `if` handles the `>= 0`
case.
Found using GitHub's CodeQL scan in HAProxy's codebase.
Tim Duesterhus [Sat, 11 Sep 2021 15:51:13 +0000 (17:51 +0200)]
CLEANUP: Add haproxy/xxhash.h to avoid modifying import/xxhash.h
This solves setting XXH_INLINE_ALL in a cleaner way, because the imported
header is not modified, easing future updates.
see
6f7cc11e6dd0f01b437fba893da2edd2362660a2
Christopher Faulet [Fri, 10 Sep 2021 08:29:54 +0000 (10:29 +0200)]
BUG/MINOR: filters: Set right FLT_END analyser depending on channel
A bug was introduced by the commit
26eb5ea35 ("BUG/MINOR: filters: Always
set FLT_END analyser when CF_FLT_ANALYZE flag is set"). Depending on the
channel evaluated, the rigth FLT_END analyser must be set. AN_REQ_FLT_END
for the request channel and AN_RES_FLT_END for the response one.
Ths patch must be backported everywhere the above commit was backported.
Christopher Faulet [Fri, 10 Sep 2021 07:17:50 +0000 (09:17 +0200)]
BUG/MEDIUM: http-ana: Reset channels analysers when returning an error
When an error is returned to the client, via a call to
http_reply_and_close(), the request channel is flushed and shut down and
HTTP analysis on both direction is finished. So it is safer to centralize
reset of channels analysers at this place. It is especially important when a
filter is attached to the stream when a client abort is detected. Because,
otherwise, the stream remains blocked because request analysers are not
reset.
This bug was hidden for a while. But since the fix
6fcd2d328 ("BUG/MINOR:
stream: Don't release a stream if FLT_END is still registered"), it is
possible to trigger it.
This patch must be backported everywhere the above commit was backported.
Christopher Faulet [Thu, 9 Sep 2021 08:17:59 +0000 (10:17 +0200)]
BUG/MEDIUM: stream-int: Don't block SI on a channel policy if EOI is reached
If the end of input is reported by the mux on the conn-stream during a
receive, we leave without evaluating the channel policies. It is especially
important to be able to catch client aborts during server connection
establishment. Indeed, in this case, without this patch, the
stream-interface remains blocked and read events are not forwarded to the
stream. It means it is not possible to detect client aborts.
Thanks to this fix, the abortonclose option should fixed for HAProxy 2.3 and
lower. On 2.4 and 2.5, it seems to work because the stream is created after
the request parsing.
Note that a previous fix of abortonclose option was reverted. This one
should be the right way to fix it. It must carefully be backported as far as
2.0. A observation period on the 2.3 is probably a good idea.
Christopher Faulet [Thu, 9 Sep 2021 12:51:26 +0000 (14:51 +0200)]
CLEANUP: mux-h1: Remove condition rejecting upgrade requests with payload
Now, "Upgrade:" header is removed from such requests. Thus, the condition to
reject them is now useless and can be removed. Code to handle unimplemented
features is now unused but is preserved for future uses.
This patch may be backported to 2.4.
Christopher Faulet [Thu, 9 Sep 2021 07:44:18 +0000 (09:44 +0200)]
MINOR: htx: Skip headers with no value when adding a header list to a message
When the header list is added, after the message parsing, headers with no
value are now ignored. It is not the same than headers with empty value
fields. Only headers with a NULL pointer as value are skipped. This only
happens if the header value is removed during the message
parsing. Concretly, such headers are now ignored when htx_add_all_headers()
is called. However, htx_add_header() is not affected by this change.
Symetrically, the same is true for trailers. It may be backported to 2.4
because of the previous fix ("BUG/MEDIUM: mux-h1: Remove "Upgrade:" header
for requests with payload").
Christopher Faulet [Thu, 9 Sep 2021 07:52:51 +0000 (09:52 +0200)]
BUG/MEDIUM: mux-h1: Remove "Upgrade:" header for requests with payload
Instead of returning a 501-Not-implemented error when "Ugrade:" header is
found for a request with a payload, the header is removed. This way, the
upgrade is disabled and the request is still sent to the server. It is
required because some frameworks seem to try to perform H2 upgrade on every
requests, including POST ones.
The h2 mux was slightly fixed to convert Upgrade requests to extended
connect ones only if the rigth HTX flag is set.
This patch should fix the issue #1381. It must be backported to 2.4.
Willy Tarreau [Thu, 9 Sep 2021 12:27:37 +0000 (14:27 +0200)]
CI: Github Actions: temporarily disable Opentracing
As discussed in the thread below, the recent variables changes
unfortunately broke Opentracing. Discussions are ongoing about
possible solutions but none of them can be done in a 3-liner so
we'd rather disable opentracing from the full-features build for
the time being.
Link: https://www.mail-archive.com/haproxy@formilux.org/msg41131.html
devnexen@gmail.com [Sat, 4 Sep 2021 08:58:57 +0000 (09:58 +0100)]
BUILD: fix dragonfly build again on __read_mostly
It looks like some versions define it and others not. Better rely on
the macro itself rather than checking for a particular OS.
Willy Tarreau [Wed, 8 Sep 2021 13:51:06 +0000 (15:51 +0200)]
OPTIM: vars: do not keep variables usage stats if no limit is set
The sole purpose of the variable's usage accounting is to enforce
limits at the session or process level, but very commonly these are not
set, yet the bookkeeping (especially at the process level) is extremely
expensive.
Let's simply disable it when the limits are not set. This further
increases the performance of 12 variables on 16-thread from 1.06M
to 1.24M req/s.
Willy Tarreau [Wed, 8 Sep 2021 13:40:58 +0000 (15:40 +0200)]
OPTIM: vars: remove internal bookkeeping for vars_global_size
Right now we have a per-process max variable size and a per-scope one,
with the proc scope covering all others. As such, the per-process global
one is always exactly equal to the per-proc-scope one. And bookkeeping
on these process-wide variables is extremely expensive (up to 38% CPU
seen in var_accounting_diff() just for them).
Let's kill vars_global_size and only rely on the proc one. Doing this
increased the request rate from 770k to 1.06M in a config having only
12 variables on a 16-thread machine.
Willy Tarreau [Wed, 8 Sep 2021 13:20:45 +0000 (15:20 +0200)]
OPTIM: vars: only takes the variables lock on shared entries
There's no point taking the variables locks for sess/txn/req/res
contexts since these ones always run inside the same thread anyway.
This patch conditions the lock on the variable's scope to avoid
flushing cache lines when not needed.
This showed an improvement of ~5% on a 16-thread machine with 12
variables.
Willy Tarreau [Wed, 8 Sep 2021 13:19:57 +0000 (15:19 +0200)]
MINOR: vars: centralize the lock/unlock into static inlines
The goal it to simplify the variables locking in order to later
simplify it.
Willy Tarreau [Tue, 31 Aug 2021 07:00:16 +0000 (09:00 +0200)]
CLEANUP: vars: remove the now unused var_names array
This was the table of all variable names known to the haproxy process.
It's not used anymore.
Willy Tarreau [Tue, 31 Aug 2021 06:51:02 +0000 (08:51 +0200)]
MEDIUM: vars: replace the global name index with a hash
The global table of known variables names can only grow and was designed
for static names that are registered at boot. Nowadays it's possible to
set dynamic variable names from Lua or from the CLI, which causes a real
problem that was partially addressed in 2.2 with commit
4e172c93f
("MEDIUM: lua: Add `ifexist` parameter to `set_var`"). Please see github
issue #624 for more context.
This patch simplifies all this by removing the need for a central
registry of known names, and storing 64-bit hashes instead. This is
highly sufficient given the low number of variables in each context.
The hash is calculated using XXH64() which is bijective over the 64-bit
space thus is guaranteed collision-free for 1..8 chars. Above that the
risk remains around 1/2^64 per extra 8 chars so in practice this is
highly sufficient for our usage. A random seed is used at boot to seed
the hash so that it's not attackable from Lua for example.
There's one particular nit though. The "ifexist" hack mentioned above
is now limited to variables of scope "proc" only, and will only match
variables that were already created or declared, but will now verify
the scope as well. This may affect some bogus Lua scripts and SPOE
agents which used to accidentally work because a similarly named
variable used to exist in a different scope. These ones may need to be
fixed to comply with the doc.
Now we can sum up the situation as this one:
- ephemeral variables (scopes sess, txn, req, res) will always be
usable, regardless of any prior declaration. This effectively
addresses the most problematic change from the commit above that
in order to work well could have required some script auditing ;
- process-wide variables (scope proc) that are mentioned in the
configuration, referenced in a "register-var-names" SPOE directive,
or created via "set-var" in the global section or the CLI, are
permanent and will always accept to be set, with or without the
"ifexist" restriction (SPOE uses this internally as well).
- process-wide variables (scope proc) that are only created via a
set-var() tcp/http action, via Lua's set_var() calls, or via an
SPOE with the "force-set-var" directive), will not be permanent
but will always accept to be replaced once they are created, even
if "ifexist" is present
- process-wide variables (scope proc) that do not exist will only
support being created via the set-var() tcp/http action, Lua's
set_var() calls without "ifexist", or an SPOE declared with
"force-set-var".
This means that non-proc variables do not care about "ifexist" nor
prior declaration, and that using "ifexist" should most often be
reliable in Lua and that SPOE should most often work without any
prior declaration. It may be doable to turn "ifexist" to 1 by default
in Lua to further ease the transition. Note: regtests were adjusted.
Cc: Tim Düsterhus <tim@bastelstu.be>
Willy Tarreau [Tue, 31 Aug 2021 06:48:55 +0000 (08:48 +0200)]
MINOR: vars: preset a random seed to hash variables names
Variables names will be hashed, but for this we need a random seed.
The XXH3() algorithms is bijective over the whole 64-bit space, which
is great as it guarantees no collision for 1..8 byte names. But above
that even if the risk is extremely faint, it theoretically exists and
since variables may be set from Lua we'd rather do our best to limit
the risk of controlled collision, hence the random seed.
Willy Tarreau [Wed, 8 Sep 2021 09:07:32 +0000 (11:07 +0200)]
MEDIUM: vars: pre-create parsed SCOPE_PROC variables as permanent ones
All variables whose names are parsed by the config parser, the
command-line parser or the SPOE's register-var-names parser are
now preset as permanent. This will guarantee that these variables
will exist through out all the process' life, and that it will be
possible to implement the "ifexist" feature by looking them up.
This was marked medium because pre-setting a variable with an empty
value may always have side effects, even though none was spotted at
this stage.
Willy Tarreau [Wed, 8 Sep 2021 13:03:58 +0000 (15:03 +0200)]
MEDIUM: vars: make var_clear() only reset VF_PERMANENT variables
We certainly do not want that a permanent variable (one that is listed
in the configuration) be erased by accident by an "unset-var" action.
Let's make sure these ones are only reset to an empty sample, like at
the moment of their initial registration. One trick is that the same
function is used to purge the memory at the end and to delete, so we
need to add an extra "force" argument to make the choice.
Willy Tarreau [Wed, 8 Sep 2021 09:07:32 +0000 (11:07 +0200)]
MINOR: vars: store flags into variables and add VF_PERMANENT
In order to continue to honor the ifexist Lua option and prevent rogue
SPOA agents from creating too many variables, we'll need to keep the
ability to mark certain proc.* variables as permanent when they're
known from the config file.
Let's add a flag there for this. It's added to the variable when the
variable is created with this flag set by the caller.
Another approach could have been to use a distinct list or distinct
scope but that sounds complicated and bug-prone.