]> git.ipfire.org Git - thirdparty/haproxy.git/log
thirdparty/haproxy.git
2 years agoMEDIUM: stick-table: make stksess_kill_if_expired() avoid the exclusive lock
Willy Tarreau [Tue, 11 Oct 2022 18:50:22 +0000 (18:50 +0000)] 
MEDIUM: stick-table: make stksess_kill_if_expired() avoid the exclusive lock

stream_store_counters() calls stksess_kill_if_expired() for each active
counter. And this one takes an exclusive lock on the table before
checking if it has any work to do (hint: it almost never has since it
only wants to delete expired entries). However a lock is still neeed for
now to protect the ref_cnt, but we can do it atomically under the read
lock.

Let's change the mechanism. Now what we do is to check out of the lock
if the entry is expired. If it is, we take the write lock, expire it,
and decrement the refcount. Otherwise we just decrement the refcount
under a read lock. With this change alone, the config based on 3
trackers without the previous patches saw a 2.6x improvement, but here
it doesn't yet change anything because some heavy contention remains
on the lookup part.

2 years agoMEDIUM: stick-table: only take the lock when needed in stktable_touch_with_exp()
Willy Tarreau [Tue, 11 Oct 2022 18:31:04 +0000 (18:31 +0000)] 
MEDIUM: stick-table: only take the lock when needed in stktable_touch_with_exp()

As previously mentioned, this function currently holds an exclusive lock
on the table during all the time it take to check if the entry needs to
be updated and synchronized with peers. The reality is that many setups
do not use peers and that on highly loaded setups, the same entries are
hammered all the time so the key's expiration doesn't change between a
number of consecutive accesses.

With this patch we take a different approach. The function starts
without taking the lock, and will take it only if needed, keeping track
of it. This way we can avoid it most of the time, or even entirely.
Finally if the decrefcnt argument requires that the refcount is
decremented, we either do it using a non-atomic op if the table was
locked (since no other entry may touch it) or via an atomic under the
read lock only.

With this change alone, a 48-thread test with 3 trackers increased
from 193k req/s to 425k req/s, which is a 2.2x factor.

2 years agoMINOR: stick-table: move the write lock inside stktable_touch_with_exp()
Willy Tarreau [Tue, 11 Oct 2022 18:17:58 +0000 (18:17 +0000)] 
MINOR: stick-table: move the write lock inside stktable_touch_with_exp()

Taking the write lock prior to entering that function is a problem
because this function is full of conditions that most of the time can
lead to eliminating the lock.

This commit first moves the write lock inside the function and passes
the extra argument required to implement stktable_touch_remote() and
stktable_touch_local(). It also renames the function to remove the
underscores since there's no other variant and it's exported under
this name (probably an old rename that was not propagated). The code
was stressed under 48 threads using 3 trackers on the same table. It
already shows a tiny 3% improvement from 187k to 193k rps.

2 years agoMINOR: stick-table: do not take an exclusive lock when downing ref_cnt
Willy Tarreau [Tue, 11 Oct 2022 18:10:27 +0000 (18:10 +0000)] 
MINOR: stick-table: do not take an exclusive lock when downing ref_cnt

At plenty of places we decrement ts->ref_cnt under the write lock
because it's held. We don't technically need it to be done that way
if there's contention and an atomic could suffice. However until all
places are turned to atomic, we at least need to do that under a
read lock for now, so that we don't mix atomic and non-atomic uses.
Regardless it already brings ~1.5% req rate improvement with 3 trackers
on the same table under 48 threads at 184k->187k rps.

2 years agoMEDIUM: stick-table: switch the table lock to rwlock
Willy Tarreau [Tue, 11 Oct 2022 10:02:50 +0000 (12:02 +0200)] 
MEDIUM: stick-table: switch the table lock to rwlock

Right now a spinlock is used, but most accesses are for reads, so let's
switch the lock to an rwlock and switch all accesses to exclusive locks
for now. There should be no visible difference at this point.

2 years agoMINOR: freq_ctr: use the thread's local time whenever possible
Willy Tarreau [Tue, 11 Oct 2022 09:55:16 +0000 (11:55 +0200)] 
MINOR: freq_ctr: use the thread's local time whenever possible

Right now when dealing with freq_ctr updates, we're using the process-
wide monotinic time, and accessing it is expensive since every thread
needs to update it, so this adds some contention. However we don't need
it all the time, the thread's local time is most of the time strictly
equal to the global time, and may be off by one millisecond when the
global time is switched to the next one by another thread, and in this
case we don't want to use the local time because it would risk to cause
a rotation of the counter. But that's precisely the condition we're
already relying on for the slow path!

What this patch does is to add a check for the period against the
local time prior to anything else, and immediately return after
updating the counter if still within the period, otherwise fall back
to the existing code. Given that the function starts to inflate a bit,
it was split between s very short inline part that does the hot path,
and the slower fallback that's in a cold function. It was measured that
on a 24-CPU machine it was called ~0.003% of the time.

The resulting improvement sits between 2 and 3% at 500k req/s tracking
an http_req_rate counter.

2 years agoMINOR: plock: support disabling exponential back-off
Willy Tarreau [Tue, 11 Oct 2022 15:02:02 +0000 (17:02 +0200)] 
MINOR: plock: support disabling exponential back-off

The new macro PLOCK_DISABLE_EBO may be defined to disable exponential
backoff. This can be useful to more easily spot functions that cause
contention. In this case the CPU will be spent inside the functions
themselves instead of the pl_wait_unlock_{long,int}() functions, making
them easier to spot using "perf top" even if that causes a significant
degradation of the thread scalability.

2 years agoBUG/MAJOR: stick-tables: do not try to index a server name for applets
Willy Tarreau [Wed, 12 Oct 2022 08:35:41 +0000 (10:35 +0200)] 
BUG/MAJOR: stick-tables: do not try to index a server name for applets

Since commit 03cdf55e6 ("MINOR: stream: Stickiness server lookup by name.")
in 2.0-dev6, server names may be used instead of their IDs, in order to
perform stickiness. However the commit above may end up trying to insert
an empty server name in the dictionary when the server is an applet
instead, resulting in an immediate segfault. This is typically what
happens when a "stick-store" rule is present in a backend featuring a
"stats" directive. As there doesn't seem to be an easy way around it,
it seems to imply that "stick-store" is not much used anymore.

The solution here is to only try to insert non-null keys into the
dictionary. The patch moves the check of the key type before the
first lock so that the test on the key can be performed under the lock
instead of locking twice (the patch is more readable with diff -b).

Note that before 2.4, there's no <key> variable there as it was
introduced by commit 92149f9a8 ("MEDIUM: stick-tables: Add srvkey
option to stick-table"), but the __objt_server(s->target)->id still
needs to be tested.

This needs to be backported as far as 2.0.

2 years agoMINOR: hlua: removing ambiguous lua_pushvalue with 0 index
Aurelien DARRAGON [Fri, 7 Oct 2022 09:54:57 +0000 (11:54 +0200)] 
MINOR: hlua: removing ambiguous lua_pushvalue with 0 index

In cd341d531, I added a FIXME comment because I noticed a
lua_pushvalue with 0 index, whereas lua doc states that 0 is never
an acceptable index.

After reviewing and testing the hlua_applet_http_send_response() code,
it turns out that this pushvalue is not even needed.
So it's safer to remove it as it could lead to undefined
behavior (since it is not supported by Lua API) and it grows lua stack
by 1 for no reason.

No backport needed.

2 years agoDOC: configuration: missing 'if' in tcp-request content example
Aurelien DARRAGON [Wed, 5 Oct 2022 16:09:33 +0000 (18:09 +0200)] 
DOC: configuration: missing 'if' in tcp-request content example

An example given for tcp-request content rule with lua
was missing 'if' keyword. Using it "as is" makes haproxy unhappy.

The example was introduced with 579d83b05.
So it may be backported as far as 1.6, but it is a really minor typo.

2 years agoMINOR: hlua: some luaL_checktype() calls were not guarded with MAY_LJMP
Aurelien DARRAGON [Wed, 5 Oct 2022 09:46:45 +0000 (11:46 +0200)] 
MINOR: hlua: some luaL_checktype() calls were not guarded with MAY_LJMP

In hlua code, we mark every function that may longjump using
MAY_LJMP macro so it's easier to identify them by reading the code.

However, some luaL_checktypes() were performed without the MAY_LJMP.

According to lua doc:
Functions called luaL_check* always raise an error if
the check is not satisfied.

-> Adding the missing MAY_LJMP for those luaLchecktypes() calls.

No backport needed.

2 years agoBUG/MINOR: quic: set IP_PKTINFO socket option for QUIC receivers only
Amaury Denoyelle [Tue, 11 Oct 2022 14:22:18 +0000 (16:22 +0200)] 
BUG/MINOR: quic: set IP_PKTINFO socket option for QUIC receivers only

Move code which activates IP_PKTINFO socket option (or affiliated
options) from sock_inet_bind_receiver() to quic_bind_listener()
function. This change is useful for two reasons :

* first, and the most important one : this activates IP_PKTINFO only for
  QUIC receivers. The previous version impacted all datagram receivers,
  used for example by log-forwarder. This should reduce memory usage for
  these datagram sockets which do not need this option.

* second, USE_QUIC preprocessor statements are removed from
  src/sock_inet.c which clean up the code.

IP_PKTINFO was introduced recently by the following patch :
  97ecc7a8ea5339a753507c3d4e4cd83028c6d038 (quic-dev/qns)
  MEDIUM: quic: retrieve frontend destination address

For the moment, this does not impact any stable release. However, as
previous patch is scheduled for 2.6 backporting, the current change must
also be backported to the same versions.

2 years agoCLEANUP: quic/receiver: remove the now unused tx_qring list
Willy Tarreau [Tue, 11 Oct 2022 06:36:21 +0000 (08:36 +0200)] 
CLEANUP: quic/receiver: remove the now unused tx_qring list

The tx_qrings[] and tx_qring_list in the receiver are not used
anymore since commit f2476053f ("MINOR: quic: replace custom buf on Tx
by default struct buffer"), the only place where they're referenced
was in quic_alloc_tx_rings_listener(), which by the way implies that
these were not even freed on exit.

Let's just remove them. This should be backported to 2.6 since the
commit above also was.

2 years agoCLEANUP: Reapply strcmp.cocci
Tim Duesterhus [Sat, 8 Oct 2022 10:33:19 +0000 (12:33 +0200)] 
CLEANUP: Reapply strcmp.cocci

This reapplies strcmp.cocci across the whole src/ tree.

2 years agoCLEANUP: Reapply ist.cocci (2)
Tim Duesterhus [Sat, 8 Oct 2022 10:33:18 +0000 (12:33 +0200)] 
CLEANUP: Reapply ist.cocci (2)

This reapplies ist.cocci across the whole src/ tree.

2 years agoMEDIUM: quic: retrieve frontend destination address
Amaury Denoyelle [Fri, 23 Sep 2022 15:15:58 +0000 (17:15 +0200)] 
MEDIUM: quic: retrieve frontend destination address

Retrieve the frontend destination address for a QUIC connection. This
address is retrieve from the first received datagram and then stored in
the associated quic-conn.

This feature relies on IP_PKTINFO or affiliated flags support on the
socket. This flag is set for each QUIC listeners in
sock_inet_bind_receiver(). To retrieve the destination address,
recvfrom() has been replaced by recvmsg() syscall. This operation and
parsing of msghdr structure has been extracted in a wrapper quic_recv().

This change is useful to finalize the implementation of 'dst' sample
fetch. As such, quic_sock_get_dst() has been edited to return local
address from the quic-conn. As a best effort, if local address is not
available due to kernel non-support of IP_PKTINFO, address of the
listener is returned instead.

This should be backported up to 2.6.

2 years agoCLEANUP: quic: fix indentation
Amaury Denoyelle [Tue, 27 Sep 2022 08:35:29 +0000 (10:35 +0200)] 
CLEANUP: quic: fix indentation

Fix some indentation in qc_lstnr_pkt_rcv().

This should be backported up to 2.6.

2 years agoMINOR: mux-quic: check quic-conn return code on Tx
Amaury Denoyelle [Mon, 26 Sep 2022 13:02:31 +0000 (15:02 +0200)] 
MINOR: mux-quic: check quic-conn return code on Tx

Inspect return code of qc_send_mux(). If quic-conn layer reports an
error, this will interrupt the current emission process.

This should be backported up to 2.6.

2 years agoMINOR: quic: limit usage of ssl_sock_ctx in favor of quic_conn
Amaury Denoyelle [Mon, 26 Sep 2022 12:53:59 +0000 (14:53 +0200)] 
MINOR: quic: limit usage of ssl_sock_ctx in favor of quic_conn

Continue on the cleanup of QUIC stack and components.

quic_conn uses internally a ssl_sock_ctx to handle mandatory TLS QUIC
integration. However, this is merely as a convenience, and it is not
equivalent to stackable ssl xprt layer in the context of HTTP1 or 2.

To better emphasize this, ssl_sock_ctx usage in quic_conn has been
removed wherever it is not necessary : namely in functions not related
to TLS. quic_conn struct now contains its own wait_event for tasklet
quic_conn_io_cb().

This should be backported up to 2.6.

2 years agoBUG/MINOR: hlua: hlua_channel_insert_data() behavior conflicts with documentation
Aurelien DARRAGON [Tue, 4 Oct 2022 10:16:05 +0000 (12:16 +0200)] 
BUG/MINOR: hlua: hlua_channel_insert_data() behavior conflicts with documentation

Channel.insert(channel, string, [,offset]):

When no offset is provided, hlua_channel_insert_data() inserts
string at the end of incoming data.

This behavior conflicts with the documentation that explicitly says
that the default behavior is to insert the string in front of incoming data.

This patch fixes hlua_channel_insert_data() behavior so that it fully
complies with the documentation.

Thanks to Smackd0wn for noticing it.

This could be backported to 2.6 and 2.5

2 years agoBUILD: http_fetch: silence an uninitiialized warning with gcc-4/5/6 at -Os
Willy Tarreau [Tue, 4 Oct 2022 07:18:34 +0000 (09:18 +0200)] 
BUILD: http_fetch: silence an uninitiialized warning with gcc-4/5/6 at -Os

Gcc 4.x, 5.x and 6.x report this when compiling http_fetch.c:

  src/http_fetch.c: In function 'smp_fetch_meth':
  src/http_fetch.c:357:6: warning: 'htx' may be used uninitialized in this function [-Wmaybe-uninitialized]
     sl = http_get_stline(htx);

That's quite weird since there's no such code path, but presetting the
htx variable to NULL during declaration is enough to shut it up.

This may be backported to any version that has dbbdb25f1 ("BUG/MINOR:
http-fetch: Use integer value when possible in "method" sample fetch")
as it's the one that triggered this warning (hence at least 2.0).

2 years agoBUG/MINOR: http-fetch: Update method after a prefetch in smp_fetch_meth()
Christopher Faulet [Tue, 4 Oct 2022 06:58:02 +0000 (08:58 +0200)] 
BUG/MINOR: http-fetch: Update method after a prefetch in smp_fetch_meth()

In smp_fetch_meth(), smp_prefetch_htx() function may be called to parse the
HTX message and update the HTTP transaction accordingly. In this case, in
smp_fetch_metch() and on success, we must update "meth" variable. Otherwise,
the variable is still equal to HTTP_METH_OTHER and the string version is
always used instead of the enum for known methods.

This patch must be backported as far as 2.0.

2 years agoMINOR: init: do not try to shrink existing RLIMIT_NOFIlE
Willy Tarreau [Thu, 22 Sep 2022 14:12:08 +0000 (16:12 +0200)] 
MINOR: init: do not try to shrink existing RLIMIT_NOFIlE

As seen in issue #1866, some environments will not allow to change the
current FD limit, and actually we don't need to do it, we only do it as
a byproduct of adjusting the limit to the one that fits. Here we're
replacing calls to setrlimit() with calls to raise_rlim_nofile(), which
will avoid making the setrlimit() syscall in case the desired value is
lower than the current process' one.

This depends on previous commit "MINOR: fd: add a new function to only
raise RLIMIT_NOFILE" and may need to be backported to 2.6, possibly
earlier, depending on users' experience in such environments.

2 years agoMINOR: fd: add a new function to only raise RLIMIT_NOFILE
Willy Tarreau [Thu, 22 Sep 2022 14:08:47 +0000 (16:08 +0200)] 
MINOR: fd: add a new function to only raise RLIMIT_NOFILE

In issue #1866 an issue was reported under docker, by which a user cannot
lower the number of FD needed. It looks like a restriction imposed in this
environment, but it results in an error while it ought not have to in the
case of shrinking.

This patch adds a new function raise_rlim_nofile() that takes the desired
new setting, compares it to the current one, and only calls setrlimit() if
one of the values in the new setting is larger than the older one. As such
it will continue to emit warnings and errors in case of failure to raise
the limit but will never shrink it.

This patch is only preliminary to another one, but will have to be
backported where relevant (likely only 2.6).

2 years agoBUILD: h1: silence an initiialized warning with gcc-4.7 and -Os
Willy Tarreau [Tue, 4 Oct 2022 06:02:03 +0000 (08:02 +0200)] 
BUILD: h1: silence an initiialized warning with gcc-4.7 and -Os

Building h1.c with gcc-4.7 -Os produces the following warning:

  src/h1.c: In function 'h1_headers_to_hdr_list':
  src/h1.c:1101:36: warning: 'ptr' may be used uninitialized in this function [-Wmaybe-uninitialized]

In fact ptr may be taken from sl.rq.u.ptr which is only initialized after
passing through the relevant states, but gcc doesn't know which states
are visited. Adding an ALREADY_CHECKED() statement there is sufficient to
shut it up and doesn't affect the emitted code.

This may be backported to stable versions to make sure that builds on older
distros and systems is clean.

2 years agoBUG/MEDIUM: lua: handle stick table implicit arguments right.
Olivier Houchard [Mon, 12 Sep 2022 22:35:53 +0000 (00:35 +0200)] 
BUG/MEDIUM: lua: handle stick table implicit arguments right.

In hlua_lua2arg_check(), we allow for the first argument to not be
provided, if it has a type we know, this is true for frontend, backend,
and stick table. However, the stick table code was changed. It used
to be deduced from the proxy, but it is now directly provided in struct
args. So setting the proxy there no longer work, and we have to
explicitely set the stick table.
Not doing so will lead the code do use the proxy pointer as a stick
table pointer, which will likely cause crashes.

This should be backported up to 2.0.

2 years agoBUG/MEDIUM: lua: Don't crash in hlua_lua2arg_check on failure
Olivier Houchard [Mon, 12 Sep 2022 22:31:17 +0000 (00:31 +0200)] 
BUG/MEDIUM: lua: Don't crash in hlua_lua2arg_check on failure

In hlua_lua2arg_check(), on failure, before calling free_argp(), make
sure to always mark the failed argument as ARGT_STOP. We only want to
free argument prior to that point, because we did not allocate the
strings after this one, and so we don't want to free them.

This should be backported up to 2.2.

2 years agoBUG/MINOR: mux-quic: ignore STOP_SENDING for locally closed stream
Amaury Denoyelle [Mon, 3 Oct 2022 15:20:31 +0000 (17:20 +0200)] 
BUG/MINOR: mux-quic: ignore STOP_SENDING for locally closed stream

It is possible to receive a STOP_SENDING frame for a locally closed
stream. This was not properly managed as this would result in a BUG_ON()
crash from qcs_idle_open() call under qcc_recv_stop_sending().

Now, STOP_SENDING frames are ignored when received on streams already
locally closed. This has two consequences depending on the reason of
closure :

* if a RESET_STREAM was already emitted and closed the stream, this
  patch prevents to emit a new RESET_STREAM. This behavior is thus
  better.

* if stream was closed due to all data transmitted, no RESET_STREAM will
  be built. This is contrary to the RFC 9000 which advice to transmit
  it, even on "Data Sent" state. However, this is not mandatory so the
  new behavior is acceptable, even if it could be improved.

This crash has been detected on haproxy.org. This can be artifically
reproduced by adding the following snippet at the end of qc_send_mux()
when doing a request with a small payload response :
  qcc_recv_stop_sending(qc->qcc, 0, 0);

This must be backported up to 2.6.

2 years agoCLEANUP: quic: create a dedicated quic_conn module
Amaury Denoyelle [Fri, 30 Sep 2022 16:11:13 +0000 (18:11 +0200)] 
CLEANUP: quic: create a dedicated quic_conn module

xprt_quic module was too large and did not reflect the true architecture
by contrast to the other protocols in haproxy.

Extract code related to XPRT layer and keep it under xprt_quic module.
This code should only contains a simple API to communicate between QUIC
lower layer and connection/MUX.

The vast majority of the code has been moved into a new module named
quic_conn. This module is responsible to the implementation of QUIC
lower layer. Conceptually, it overlaps with TCP kernel implementation
when comparing QUIC and HTTP1/2 stacks of haproxy.

This should be backported up to 2.6.

2 years agoCLEANUP: quic: remove duplicated varint code from xprt_quic.h
Amaury Denoyelle [Fri, 30 Sep 2022 15:47:04 +0000 (17:47 +0200)] 
CLEANUP: quic: remove duplicated varint code from xprt_quic.h

There was some identical code between xprt_quic and quic_enc modules.
This concerns helper on QUIC varint type. Keep only the version in
quic_enc file : this should help to reduce dependency on xprt_quic
module.

Note that quic_max_int_by_size() has been removed and is replaced by the
identical quic_max_int().

This should be backported up to 2.6.

2 years agoCLEANUP: quic: remove unused function prototype
Amaury Denoyelle [Fri, 30 Sep 2022 15:34:54 +0000 (17:34 +0200)] 
CLEANUP: quic: remove unused function prototype

Removed hexdump unusued prototype from quic_tls.c.

This should be backported up to 2.6.

2 years agoCLEANUP: quic: fix headers
Amaury Denoyelle [Fri, 30 Sep 2022 15:44:15 +0000 (17:44 +0200)] 
CLEANUP: quic: fix headers

Clean up quic sources by adjusting headers list included depending
on the actual dependency of each source file.

On some occasion, xprt_quic.h was removed from included list. This is
useful to help reducing the dependency on this single file and cleaning
up QUIC haproxy architecture.

This should be backported up to 2.6.

2 years agoBUG/MINOR: quic: adjust quic_tls prototypes
Amaury Denoyelle [Fri, 30 Sep 2022 15:37:38 +0000 (17:37 +0200)] 
BUG/MINOR: quic: adjust quic_tls prototypes

Two prototypes in quic_tls module were not identical to the actual
function definition.

* quic_tls_decrypt2() : the second argument const attribute is not
  present, to be able to use it with EVP_CIPHER_CTX_ctlr(). As a
  consequence of this change, token field of quic_rx_packet is now
  declared as non-const.

* quic_tls_generate_retry_integrity_tag() : the second argument type
  differ between the two. Adjust this by fixing it to as unsigned char
  to match EVP_EncryptUpdate() SSL function.

This situation did not seem to have any visible effect. However, this is
clearly an undefined behavior and should be treated as a bug.

This should be backported up to 2.6.

2 years agoCLEANUP: quic: remove global var definition in quic_tls header
Amaury Denoyelle [Fri, 30 Sep 2022 15:31:18 +0000 (17:31 +0200)] 
CLEANUP: quic: remove global var definition in quic_tls header

Some variables related to QUIC TLS were defined in a header file : their
definitions are now moved properly in the implementation file, with only
declarations in the header.

This should be backported up to 2.6.

2 years agoCLEANUP: mux-quic: remove usage of non-standard ull type
Amaury Denoyelle [Fri, 30 Sep 2022 16:03:03 +0000 (18:03 +0200)] 
CLEANUP: mux-quic: remove usage of non-standard ull type

ull is a typedef to unsigned long long. It is only defined in
xprt_quic-t.h. Its usage should be limited over time to reduce xprt_quic
dependency over the whole code. It can be replaced by ullong typedef
from compat.h.

For the moment, ull references have been replaced in qmux_trace module.
They were only used for printf format and has been replaced by the true
variable type.

This change is useful to reduce dependencies on xprt_quic in other
files.

This should be backported up to 2.6.

2 years agoDOC: config: Fix pgsql-check documentation to make user param mandatory
Christopher Faulet [Mon, 3 Oct 2022 13:00:59 +0000 (15:00 +0200)] 
DOC: config: Fix pgsql-check documentation to make user param mandatory

The username is required in the Start-up message. Thus, since the 2.2, when
this health-check was refactored, the user parameter is mandatory. On prior
versions, when no username is provided, no pgsql check is performed but only
a basic tcpcheck.

This patch should be backported as far as 2.2.

2 years agoBUG/MINOR: checks: update pgsql regex on auth packet
Fatih Acar [Mon, 26 Sep 2022 15:27:11 +0000 (17:27 +0200)] 
BUG/MINOR: checks: update pgsql regex on auth packet

This patch adds support to the following authentication methods:

- AUTH_REQ_GSS (7)
- AUTH_REQ_SSPI (9)
- AUTH_REQ_SASL (10)

Note that since AUTH_REQ_SASL allows multiple authentication mechanisms
such as SCRAM-SHA-256 or SCRAM-SHA-256-PLUS, the auth payload length may
vary since the method is sent in plaintext. In order to allow this, the
regex now matches any payload length.

This partially fixes Github issue #1508 since user authentication is
still broken but should restore pre-2.2 behavior.

This should be backported up to 2.2.

Signed-off-by: Fatih Acar <facar@scaleway.com>
2 years ago[RELEASE] Released version 2.7-dev7 v2.7-dev7
Willy Tarreau [Mon, 3 Oct 2022 13:20:38 +0000 (15:20 +0200)] 
[RELEASE] Released version 2.7-dev7

Released version 2.7-dev7 with the following main changes :
    - BUG/MEDIUM: mux-quic: fix nb_hreq decrement
    - CLEANUP: httpclient: deleted unused variables
    - MINOR: httpclient: enabled the use of SNI presets
    - OPTIM: hpack-huff: reduce the cache footprint of the huffman decoder
    - BUG/MINOR: mux-quic: do not keep detached qcs with empty Tx buffers
    - REORG: mux-quic: extract traces in a dedicated source file
    - REORG: mux-quic: export HTTP related function in a dedicated file
    - MINOR: mux-quic: refactor snd_buf
    - BUG/MEDIUM: mux-quic: properly trim HTX buffer on snd_buf reset
    - BUG/MINOR: mux-h1: Account consumed output data on synchronous connection error
    - BUG/MINOR: log: improper behavior when escaping log data
    - CLEANUP: tools: removing escape_chunk() function
    - MINOR: clock: split local and global date updates
    - MINOR: pollers: only update the local date during busy polling
    - MINOR: clock: do not update the global date too often
    - REGTESTS: 4be_1srv_smtpchk_httpchk_layer47errors: Return valid SMTP replies
    - MINOR: smtpchk: Update expect rule to fully match replies to EHLO commands
    - BUG/MINOR: smtpchk: SMTP Service check should gracefully close SMTP transaction
    - MINOR: list: documenting mt_list_for_each_entry_safe() macro
    - CLEANUP: list: Fix mt_list_for_each_entry_safe indentation
    - BUG/MINOR: hlua: Remove \n in Lua error message built with memprintf
    - MINOR: hlua: Allow argument on lua-lod(-per-thread) directives
    - BUG/MINOR: anon: memory illegal accesses in tools.c with hash_anon and hash_ipanon
    - MEDIUM: mworker/cli: keep the connection of the FD that ask for a reload
    - BUG/MINOR: hlua: fixing ambiguous sizeof in hlua_load_per_thread
    - MINOR: mworker/cli: replace close() by fd_delete()
    - MINOR: mworker: store and shows loading status
    - MINOR: mworker: mworker_cli_proxy_new_listener() returns a bind_conf
    - MINOR: mworker: stores the mcli_reload bind_conf
    - MINOR: mworker/cli: the mcli_reload bind_conf only send the reload status
    - DOC: management: describe the new reload command behavior
    - CLEANUP: list: fix again some style issues in the recent comments
    - BUG/MINOR: stream: Perform errors handling in right order in stream_new()
    - BUG/MEDIUM: stconn: Reset SE descriptor when we fail to create a stream
    - BUG/MEDIUM: resolvers: Remove aborted resolutions from query_ids tree
    - DOC: management: add timeout on the "reload" command
    - BUG/MINOR: ring: fix the size check in ring_make_from_area()
    - BUG/MINOR: config: don't count trailing spaces as empty arg
    - Revert "BUG/MINOR: config: don't count trailing spaces as empty arg"
    - BUG/MINOR: hlua: fixing hlua_http_msg_del_data behavior
    - BUG/MINOR: hlua: fixing hlua_http_msg_insert_data behavior
    - MINOR: cli: Add anonymization on a missed element for 'show sess all'
    - MINOR: cli: remove error message with 'set anon on|off'
    - MINOR: tools: modify hash_ipanon in order to use it in cli
    - MINOR: cli: use hash_ipanon to anonymized address
    - MINOR: cli: Add an anonymization on a missed element in 'show server state'
    - MINOR: config: correct errors about argument number in condition in cfgparse.c
    - MINOR: config: Add other keywords when dump the anonymized configuration file
    - MINOR: config: Add option line when the configuration file is dumped
    - MINOR: cli: correct commentary and replace 'set global-key' name
    - MINOR: tools: Impprove hash_ipanon to support dgram sockets and port offsets
    - MINOR: tools: Impprove hash_ipanon to not hash FD-based addresses
    - BUG/MINOR: hlua: _hlua_http_msg_delete incorrect behavior when offset is used
    - DOC: management: httpclient can resolve server names in URLs
    - BUG/MINOR: hlua: prevent crash when loading numerous arguments using lua-load(per-thread)
    - DOC/CLEANUP: lua-api: removing duplicate date functions doc
    - MINOR: hlua: ambiguous lua_pushvalue with 0 index
    - BUG/MINOR: config: don't count trailing spaces as empty arg (v2)
    - BUG/MEDIUM: config: count line arguments without dereferencing the output
    - BUG/MAJOR: conn-idle: fix hash indexing issues on idle conns
    - BUG/MINOR: config: insufficient syntax check of the global "maxconn" value
    - BUG/MINOR: backend: only enforce turn-around state when not redispatching

2 years agoBUG/MINOR: backend: only enforce turn-around state when not redispatching
Willy Tarreau [Mon, 3 Oct 2022 12:56:34 +0000 (14:56 +0200)] 
BUG/MINOR: backend: only enforce turn-around state when not redispatching

In github issue #1878, Bart Butler reported observing turn-around states
(1 second pause) after connection retries going to different servers,
while this ought not happen.

In fact it does happen because back_handle_st_cer() enforces the TAR
state for any algo that's not round-robin. This means that even leastconn
has it, as well as hashes after the number of servers changed.

Prior to doing that, the call to stream_choose_redispatch() has already
had a chance to perform the correct choice and to check the algo and
the number of retries left. So instead we should just let that function
deal with the algo when needed (and focus on deterministic ones), and
let the former just obey. Bart confirmed that the fixed version works
as expected (no more delays during retries).

This may be backported to older releases, though it doesn't seem very
important. At least Bart would like to have it in 2.4 so let's go there
for now after it has cooked a few weeks in 2.6.

2 years agoBUG/MINOR: config: insufficient syntax check of the global "maxconn" value
Thierry Fournier [Sat, 1 Oct 2022 08:06:59 +0000 (10:06 +0200)] 
BUG/MINOR: config: insufficient syntax check of the global "maxconn" value

The maxconn value is decoded using atol(), so values like "3k" are
rightly converter as interger 3, while the user wants 3000.

This patch fixes this behavior by reporting a parsing error.

This patch could be backported on all maintained version, but it
could break some configuration. The bug is really minor, I recommend
to not backport, or backport a patch which only throws a warning in
place of a fatal error.

2 years agoBUG/MAJOR: conn-idle: fix hash indexing issues on idle conns
Willy Tarreau [Thu, 29 Sep 2022 18:32:43 +0000 (20:32 +0200)] 
BUG/MAJOR: conn-idle: fix hash indexing issues on idle conns

Idle connections do not work on 32-bit machines due to an alignment issue
causing the connection nodes to be indexed with their lower 32-bits set to
zero and the higher 32 ones containing the 32 lower bitss of the hash. The
cause is the use of ebmb_node with an aligned data, as on this platform
ebmb_node is only 32-bit aligned, leaving a hole before the following hash
which is a uint64_t:

  $ pahole -C conn_hash_node ./haproxy
  struct conn_hash_node {
        struct ebmb_node           node;                 /*     0    20 */

        /* XXX 4 bytes hole, try to pack */

        int64_t                    hash;                 /*    24     8 */
        struct connection *        conn;                 /*    32     4 */

        /* size: 40, cachelines: 1, members: 3 */
        /* sum members: 32, holes: 1, sum holes: 4 */
        /* padding: 4 */
        /* last cacheline: 40 bytes */
  };

Instead, eb64 nodes should be used when it comes to simply storing a
64-bit key, and that is what this patch does.

For backports, a variant consisting in simply marking the "hash" member
with a "packed" attribute on the struct also does the job (tested), and
might be preferable if the fix is difficult to adapt. Only 2.6 and 2.5
are affected by this.

2 years agoBUG/MEDIUM: config: count line arguments without dereferencing the output
Willy Tarreau [Mon, 3 Oct 2022 06:27:55 +0000 (08:27 +0200)] 
BUG/MEDIUM: config: count line arguments without dereferencing the output

Previous commit 8a6767d26 ("BUG/MINOR: config: don't count trailing spaces
as empty arg (v2)") was still not enough. As reported by ClusterFuzz in
issue 52049 (https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=52049),
there remains a case where for the sake of reporting the correct argument
count, the function may produce virtual args that span beyond the end of
the output buffer if that one is too short. That's what's happening with
a config file of one empty line followed by a large number of args.

This means that what args[] points to cannot be relied on and that a
different approach is needed. Since no output is produced for spaces and
comments, we know that args[arg] continues to point to out+outpos as long
as only comments or spaces are found, which is what we're interested in.

As such it's safe to check the last arg's pointer against the one before
the trailing zero was emitted, in order to decide to count one final arg.

No backport is needed, unless the commit above is backported.

2 years agoBUG/MINOR: config: don't count trailing spaces as empty arg (v2)
Erwan Le Goas [Fri, 23 Sep 2022 13:06:34 +0000 (15:06 +0200)] 
BUG/MINOR: config: don't count trailing spaces as empty arg (v2)

In parse_line(), spaces increment the arg count and it is incremented
again on '#' or end of line, resulting in an extra empty arg at the
end of arg's list. The visible effect is that the reported arg count
is in excess of 1. It doesn't seem to affect regular function but
specialized ones like anonymisation depends on this count.

This is the second attempt for this problem, here the explanation :

When called for the first line, no <out> was allocated yet so it's NULL,
letting the caller realloc a larger line if needed. However the words are
parsed and their respective args[arg] are filled with out+position, which
means that while the first arg is NULL, the other ones are no and fail the
test that was meant to avoid dereferencing a NULL. Let's simply check <out>
instead of <args> since the latter is always derived from the former and
cannot be NULL without the former also being.

This may need to be backported to stable versions.

2 years agoMINOR: hlua: ambiguous lua_pushvalue with 0 index
Aurelien DARRAGON [Thu, 29 Sep 2022 10:00:04 +0000 (12:00 +0200)] 
MINOR: hlua: ambiguous lua_pushvalue with 0 index

In function hlua_applet_http_send_response(), a pushvalue
is performed with index '0'.

But according to lua doc (https://www.lua.org/manual/5.3/manual.html#4.3):
"Note that 0 is never an acceptable index".

Adding a FIXME comment near to the pushvalue operation
so that this can get some chance to be reviewed later.

No backport needed.

2 years agoDOC/CLEANUP: lua-api: removing duplicate date functions doc
Aurelien DARRAGON [Thu, 29 Sep 2022 08:25:24 +0000 (10:25 +0200)] 
DOC/CLEANUP: lua-api: removing duplicate date functions doc

As reported by Thierry, core.asctime_date() and core.rfc850() were
both documented multiple times in lua-api doc.

This does not need to be backported.

2 years agoBUG/MINOR: hlua: prevent crash when loading numerous arguments using lua-load(per...
Aurelien DARRAGON [Fri, 23 Sep 2022 08:22:14 +0000 (10:22 +0200)] 
BUG/MINOR: hlua: prevent crash when loading numerous arguments using lua-load(per-thread)

When providing multiple optional arguments with lua-load or
lua-load-per-thread directives, arguments where pushed 1 by 1
to the stack using lua_pushstring() without checking if the stack
could handle it.

This could easily lead to program crash when providing too much
arguments. I can easily reproduce the crash starting from ~50 arguments.

Calling lua_checkstack() before pushing to the stack fixes the crash:
  According to lua.org, lua_checkstack() does some housekeeping and
  allow the stack to be expanded as long as some memory is available
  and the hard limit isn't reached.
  When no memory is available to expand the stack or the limit is reached,
  lua_checkstacks returns an error: in this case we force hlua_load_state()
  to return a meaningfull error instead of crashing.
  In practice though, cfgparse complains about too many words
  way before such event may occur on a normal system.

  TLDR: the ~50 arguments limitation is not an issue anymore.

No backport needed, except if 'MINOR: hlua: Allow argument on
lua-lod(-per-thread) directives' (ae6b568) is backported.

2 years agoDOC: management: httpclient can resolve server names in URLs
William Lallemand [Thu, 29 Sep 2022 13:00:15 +0000 (15:00 +0200)] 
DOC: management: httpclient can resolve server names in URLs

The httpclient does support DNS resolution since 2.6.

Must be backported to 2.6.

2 years agoBUG/MINOR: hlua: _hlua_http_msg_delete incorrect behavior when offset is used
Aurelien DARRAGON [Wed, 28 Sep 2022 17:08:15 +0000 (19:08 +0200)] 
BUG/MINOR: hlua: _hlua_http_msg_delete incorrect behavior when offset is used

Calling the function with an offset when "offset + len" was superior or equal
to the targeted blk length caused 'v' value to be improperly set.
And because 'v' is directly provided to htx_replace_blk_value(), blk consistency was compromised.
(It seems that blk was overrunning in htx_replace_blk_value() due to
this and header data was overwritten in this case).

This patch adds the missing checks to make the function behave as
expected when offset is set and offset+len is greater or equals to the targeted blk length.
Some comments were added to the function as well.

It may be backported to 2.6 and 2.5

2 years agoMINOR: tools: Impprove hash_ipanon to not hash FD-based addresses
Christopher Faulet [Thu, 29 Sep 2022 09:53:07 +0000 (11:53 +0200)] 
MINOR: tools: Impprove hash_ipanon to not hash FD-based addresses

"stdout" and "stderr" are not hashed. In the same spirit, "fd@" and
"sockpair@" prefixes are not hashed too. There is no reason to hash such
address and it may be useful to diagnose bugs.

No backport needed, except if anonymization mechanism is backported.

2 years agoMINOR: tools: Impprove hash_ipanon to support dgram sockets and port offsets
Christopher Faulet [Thu, 29 Sep 2022 09:46:34 +0000 (11:46 +0200)] 
MINOR: tools: Impprove hash_ipanon to support dgram sockets and port offsets

Add PA_O_DGRAM and PA_O_PORT_OFS options when str2sa_range() is called. This
way dgram sockets and addresses with port offsets are supported.

No backport needed, except if anonymization mechanism is backported.

2 years agoMINOR: cli: correct commentary and replace 'set global-key' name
Erwan Le Goas [Thu, 29 Sep 2022 08:36:11 +0000 (10:36 +0200)] 
MINOR: cli: correct commentary and replace 'set global-key' name

Correct a commentary in in include/haproxy/global-t.h and include/haproxy/tools.h
Replace the CLI command 'set global-key <key>' by 'set anon global-key <key>' in
order to find it easily when you don't remember it, the recommandation can guide
you when you just tap 'set anon'.

No backport needed, except if anonymization mechanism is backported.

2 years agoMINOR: config: Add option line when the configuration file is dumped
Erwan Le Goas [Thu, 29 Sep 2022 08:34:04 +0000 (10:34 +0200)] 
MINOR: config: Add option line when the configuration file is dumped

Add an option to dump the number lines of the configuration file when
it's dumped. Other options can be easily added. Options are separated
by ',' when tapping the command line:
'./haproxy -dC[key],line -f [file]'

No backport needed, except if anonymization mechanism is backported.

2 years agoMINOR: config: Add other keywords when dump the anonymized configuration file
Erwan Le Goas [Thu, 29 Sep 2022 08:31:18 +0000 (10:31 +0200)] 
MINOR: config: Add other keywords when dump the anonymized configuration file

Add keywords recognized during the dump of the configuration file,
these keywords are followed by sensitive information.

Remove the condition 'localhost' for the second argument of keyword
'server', consider as not essential and can disturb when comparing
it in cli section (there is no exception 'localhost').

No backport needed, except if anonymization mechanism is backported.

2 years agoMINOR: config: correct errors about argument number in condition in cfgparse.c
Erwan Le Goas [Thu, 29 Sep 2022 08:30:00 +0000 (10:30 +0200)] 
MINOR: config: correct errors about argument number in condition in cfgparse.c

Put the right number in condition that takes the wrong number of arguments.

No backport needed, except if anonymization mechanism is backported.

2 years agoMINOR: cli: Add an anonymization on a missed element in 'show server state'
Erwan Le Goas [Thu, 29 Sep 2022 08:28:44 +0000 (10:28 +0200)] 
MINOR: cli: Add an anonymization on a missed element in 'show server state'

Add HA_ANON_CLI to the srv->hostname when using 'show servers state'.
It can contain sensitive information like 'www....com'

No backport needed, except if anonymization mechanism is backported.

2 years agoMINOR: cli: use hash_ipanon to anonymized address
Erwan Le Goas [Thu, 29 Sep 2022 08:27:33 +0000 (10:27 +0200)] 
MINOR: cli: use hash_ipanon to anonymized address

Replace HA_ANON_CLI by hash_ipanon to anonynmized address like
anonymizing address in the configuration file.

No backport needed, except if anonymization mechanism is backported.

2 years agoMINOR: tools: modify hash_ipanon in order to use it in cli
Erwan Le Goas [Thu, 29 Sep 2022 08:25:31 +0000 (10:25 +0200)] 
MINOR: tools: modify hash_ipanon in order to use it in cli

Add a parameter hasport to return a simple hash or ipstring when
ipstring has no port. Doesn't hash if scramble is null. Add
option PA_O_PORT_RESOLVE to str2sa_range. Add a case UNIX.
Those modification permit to use hash_ipanon in cli section
in order to dump the same anonymization of address in the
configuration file and with CLI.

No backport needed, except if anonymization mechanism is backported.

2 years agoMINOR: cli: remove error message with 'set anon on|off'
Erwan Le Goas [Wed, 28 Sep 2022 15:04:29 +0000 (17:04 +0200)] 
MINOR: cli: remove error message with 'set anon on|off'

Removed the error message in 'set anon on|off', it's more user
friendly: users use 'set anon on' even if the mode is already
activated, and the same for 'set anon off'. That allows users
to write the command line in the anonymized mode they want
without errors.

No backport needed, except if anonymization mechanism is backported.

2 years agoMINOR: cli: Add anonymization on a missed element for 'show sess all'
Erwan Le Goas [Wed, 28 Sep 2022 15:02:30 +0000 (17:02 +0200)] 
MINOR: cli: Add anonymization on a missed element for 'show sess all'

Add an anonymization for an element missed in the first merge
for 'show sess all'.

No backport needed, except if anonymization mechanism is backported.

2 years agoBUG/MINOR: hlua: fixing hlua_http_msg_insert_data behavior
Aurelien DARRAGON [Wed, 28 Sep 2022 14:03:45 +0000 (16:03 +0200)] 
BUG/MINOR: hlua: fixing hlua_http_msg_insert_data behavior

hlua_http_msg_insert_data() function is called upon
HTTPMessage.insert() method from lua script.

This function did not work properly for multiple reasons:

  - An incorrect argument check was performed and prevented the user
  from providing optional offset argument.

  - Input and output variables were inverted
  and offset was not handled properly. The same bug
  was also fixed in hlua_http_msg_del_data(), see:
  'BUG/MINOR: hlua: fixing hlua_http_msg_del_data behavior'

The function now behaves as described in the documentation.

This could be backported to 2.6 and 2.5.

2 years agoBUG/MINOR: hlua: fixing hlua_http_msg_del_data behavior
Aurelien DARRAGON [Wed, 28 Sep 2022 13:52:18 +0000 (15:52 +0200)] 
BUG/MINOR: hlua: fixing hlua_http_msg_del_data behavior

GH issue #1885 reported that HTTPMessage.remove() did not
work as expected.

It turns out that underlying hlua_http_msg_del_data() function
was not working properly due to input / output inversion as well
as incorrect user offset handling.

This patch fixes it so that the behavior is the one described in
the documentation.

This could be backported to 2.6 and 2.5.

2 years agoRevert "BUG/MINOR: config: don't count trailing spaces as empty arg"
Christopher Faulet [Wed, 28 Sep 2022 16:22:23 +0000 (18:22 +0200)] 
Revert "BUG/MINOR: config: don't count trailing spaces as empty arg"

This reverts commit 5529424ef1c9aae4ca714a99ffca2082e533781f.

Since this patch, HAProxy crashes when the first line of the configuration
file contains more than one parameter because, on the first call of
parse_line(), the output line is not allocated. Thus elements in the
arguments array may point on invalid memory area.

It may be considered as a bug to reference invalid memory area and should be
fixed. But for now, it is safer to revert this patch

If the reverted commit is backported, this one must be backported too.

2 years agoBUG/MINOR: config: don't count trailing spaces as empty arg
Erwan Le Goas [Fri, 23 Sep 2022 13:06:34 +0000 (15:06 +0200)] 
BUG/MINOR: config: don't count trailing spaces as empty arg

In parse_line(), spaces increment the arg count and it is incremented
again on '#' or end of line, resulting in an extra empty arg at the
end of arg's list. The visible effect is that the reported arg count
is in excess of 1. It doesn't seem to affect regular function but
specialized ones like anonymisation depends on this count.

This may need to be backported to stable versions.

2 years agoBUG/MINOR: ring: fix the size check in ring_make_from_area()
William Lallemand [Tue, 27 Sep 2022 12:31:37 +0000 (14:31 +0200)] 
BUG/MINOR: ring: fix the size check in ring_make_from_area()

Fix the size check in ring_make_from_area() which is checking the size
of the pointer instead of the size of the structure.

No backport needed, 2.7 only.

2 years agoDOC: management: add timeout on the "reload" command
William Lallemand [Tue, 27 Sep 2022 09:38:10 +0000 (11:38 +0200)] 
DOC: management: add timeout on the "reload" command

Add some details about timeout during a reload on the master CLI.

2 years agoBUG/MEDIUM: resolvers: Remove aborted resolutions from query_ids tree
Christopher Faulet [Tue, 27 Sep 2022 08:43:24 +0000 (10:43 +0200)] 
BUG/MEDIUM: resolvers: Remove aborted resolutions from query_ids tree

To avoid any UAF when a resolution is released, a mechanism was added to
abort a resolution and delayed the released at the end of the current
execution path. This mechanism depends on an hard assumption: Any reference
on an aborted resolution must be removed. So, when a resolution is aborted,
it is removed from the resolver lists and inserted into a death row list.

However, a resolution may still be referenced in the query_ids tree. It is
the tree containing all resolutions with a pending request. Because aborted
resolutions are released outside the resolvers lock, it is possible to
release a resolution on a side while a query ansswer is received and
processed on another one. Thus, it is still possible to have a UAF because
of this bug.

To fix the issue, when a resolution is aborted, it is removed from any list,
but it is also removed from the query_ids tree.

This patch should solve the issue #1862 and may be related to #1875. It must
be backported as far as 2.2.

2 years agoBUG/MEDIUM: stconn: Reset SE descriptor when we fail to create a stream
Christopher Faulet [Tue, 27 Sep 2022 07:18:20 +0000 (09:18 +0200)] 
BUG/MEDIUM: stconn: Reset SE descriptor when we fail to create a stream

If stream_new() fails after the frontend SC is attached, the underlying SE
descriptor is not properly reset. Among other things, SE_FL_ORPHAN flag is
not set again. Because of this error, a BUG_ON() is triggered when the mux
stream on the frontend side is destroyed.

Thus, now, when stream_new() fails, SE_FL_ORPHAN flag is set on the SE
descriptor and its stream-connector is set to NULL.

This patch should solve the issue #1880. It must be backported to 2.6.

2 years agoBUG/MINOR: stream: Perform errors handling in right order in stream_new()
Christopher Faulet [Tue, 27 Sep 2022 07:14:47 +0000 (09:14 +0200)] 
BUG/MINOR: stream: Perform errors handling in right order in stream_new()

The frontend SC is attached before the backend one is allocated. Thus an
allocation error on backend SC must be handled before an error on the
frontend SC.

This patch must be backported to 2.6.

2 years agoCLEANUP: list: fix again some style issues in the recent comments
Willy Tarreau [Tue, 27 Sep 2022 05:42:28 +0000 (07:42 +0200)] 
CLEANUP: list: fix again some style issues in the recent comments

While reading the recent changes around mt_list_for_each_entry_safe() I
noticed a spurious "q" at the beginning of a line introduced by commit
455843721 ("CLEANUP: list: Fix mt_list_for_each_entry_safe indentation")
and that visually confusing multi-line comments missing the trailing '\'
character were introduced by previous commit 60cffbaca ("MINOR: list:
documenting mt_list_for_each_entry_safe() macro"), which at first glance
made the macro look broken. In addition, multi-line comments must end
with a "*/" on its own line to instantly spot where it ends without
having to read the whole line, like this:

    /* we know from the above that foo is always valid
     * here so it's safe to end the string:
     */
    *(unsigned char *)foo = 0;

Not like this:

    /* we know from the above that foo is always valid
     * here so it's safe to end the string: */
    *(unsigned char *)foo = 0;

Finally, macro's main comment mentionned the wrong macro name and types,
and was randomly indented.

2 years agoDOC: management: describe the new reload command behavior
William Lallemand [Sat, 24 Sep 2022 14:44:44 +0000 (16:44 +0200)] 
DOC: management: describe the new reload command behavior

The master CLI command now allows to do a synchronous reload with a
status.

2 years agoMINOR: mworker/cli: the mcli_reload bind_conf only send the reload status
William Lallemand [Sat, 24 Sep 2022 14:14:50 +0000 (16:14 +0200)] 
MINOR: mworker/cli: the mcli_reload bind_conf only send the reload status

Upon a reload with the master CLI, the FD of the master CLI session is
received by the internal socketpair listener.

This session is used to display the status of the reload and then will
close.

2 years agoMINOR: mworker: stores the mcli_reload bind_conf
William Lallemand [Sat, 24 Sep 2022 13:56:25 +0000 (15:56 +0200)] 
MINOR: mworker: stores the mcli_reload bind_conf

Stores the mcli_reload bind_conf in order to identify it later.

2 years agoMINOR: mworker: mworker_cli_proxy_new_listener() returns a bind_conf
William Lallemand [Sat, 24 Sep 2022 13:51:27 +0000 (15:51 +0200)] 
MINOR: mworker: mworker_cli_proxy_new_listener() returns a bind_conf

mworker_cli_proxy_new_listener() now returns a bind_conf * or NULL upon
failure.

2 years agoMINOR: mworker: store and shows loading status
William Lallemand [Sat, 24 Sep 2022 13:44:42 +0000 (15:44 +0200)] 
MINOR: mworker: store and shows loading status

The environment variable HAPROXY_LOAD_SUCCESS stores "1" if it
successfully load the configuration and started, "0" otherwise.

The "_loadstatus" master CLI command displays either
"Loading failure!\n" or "Loading success.\n"

2 years agoMINOR: mworker/cli: replace close() by fd_delete()
William Lallemand [Fri, 23 Sep 2022 08:21:32 +0000 (10:21 +0200)] 
MINOR: mworker/cli: replace close() by fd_delete()

Replace the close() call in cli_parse_reload() by a fd_delete() since
the FD is one present in the fdtab.

2 years agoBUG/MINOR: hlua: fixing ambiguous sizeof in hlua_load_per_thread
Aurelien DARRAGON [Fri, 23 Sep 2022 06:48:34 +0000 (08:48 +0200)] 
BUG/MINOR: hlua: fixing ambiguous sizeof in hlua_load_per_thread

As pointed out by chipitsine in GH #1879, coverity complains
about a sizeof with char ** type where it should be char *.

This was introduced in 'MINOR: hlua: Allow argument on
lua-lod(-per-thread) directives' (ae6b568)

Luckily this had no effect so far because on most platforms
sizeof(char **) == sizeof(char *), but this can not be safely
assumed for portability reasons.

The fix simply changes the argument to sizeof so that it refers to
'*per_thread_load[len]' instead of 'per_thread_load[len]'.

No backport needed.

2 years agoMEDIUM: mworker/cli: keep the connection of the FD that ask for a reload
William Lallemand [Thu, 22 Sep 2022 15:26:23 +0000 (17:26 +0200)] 
MEDIUM: mworker/cli: keep the connection of the FD that ask for a reload

When using the "reload" command over the master CLI, all connections to
the master CLI were cut, this was unfortunate because it could have been
used to implement a synchronous reload command.

This patch implements an architecture to keep the connection alive after
the reload.

The master CLI is now equipped with a listener which uses a socketpair,
the 2 FDs of this socketpair are stored in the mworker_proc of the
master, which the master keeps via the environment variable.

ipc_fd[1] is used as a listener for the master CLI. During the "reload"
command, the CLI will send the FD of the current session over ipc_fd[0],
then the reload is achieved, so the master won't handle the recv of the
FD. Once reloaded, ipc_fd[1] receives the FD of the session, so the
connection is preserved. Of course it is a new context, so everything
like the "prompt mode" are lost.

Only the FD which performs the reload is kept.

2 years agoBUG/MINOR: anon: memory illegal accesses in tools.c with hash_anon and hash_ipanon
Erwan Le Goas [Wed, 21 Sep 2022 14:24:23 +0000 (16:24 +0200)] 
BUG/MINOR: anon: memory illegal accesses in tools.c with hash_anon and hash_ipanon

chipitsine reported in github issue #1872 that in function hash_anon and
hash_ipanon, index_hash can be equal to NB_L_HASH_WORD and can reach an
inexisting line table, the table is initialized hash_word[NB_L_HASH_WORD][20];
so hash_word[NB_L_HASH_WORD] doesn't exist.

No backport needed, except if anonymization mechanism is backported.

2 years agoMINOR: hlua: Allow argument on lua-lod(-per-thread) directives
Thierry Fournier [Mon, 19 Sep 2022 07:04:16 +0000 (09:04 +0200)] 
MINOR: hlua: Allow argument on lua-lod(-per-thread) directives

Allow per-lua file argument which makes multiples configuration
easier to handle

This patch fixes issue #1609.

2 years agoBUG/MINOR: hlua: Remove \n in Lua error message built with memprintf
Thierry Fournier [Mon, 19 Sep 2022 07:01:53 +0000 (09:01 +0200)] 
BUG/MINOR: hlua: Remove \n in Lua error message built with memprintf

Because memprintf return an error to the caller and not on screen.
the function which perform display of message on the right output
is in charge of adding \n if it is necessary.

This patch may be backported.

2 years agoCLEANUP: list: Fix mt_list_for_each_entry_safe indentation
Christopher Faulet [Wed, 21 Sep 2022 13:44:54 +0000 (15:44 +0200)] 
CLEANUP: list: Fix mt_list_for_each_entry_safe indentation

It makes the macro easier to read.

2 years agoMINOR: list: documenting mt_list_for_each_entry_safe() macro
Aurelien DARRAGON [Wed, 21 Sep 2022 08:43:15 +0000 (10:43 +0200)] 
MINOR: list: documenting mt_list_for_each_entry_safe() macro

- Adding some comments in mt_list_for_each_entry_safe() macro to make it
  somehow understandable.
  The macro is performing critical stuff but was not documented at all.
  Moreover, nested loops with conditional tricks are used,
  making it even harder to understand the steps performed in it.

- Updating mt_list_for_each_entry_safe usage example.

- Added a "FIXME:" comment in a specific condition that seems to
  never be reached even when deeply stress-testing mt_lists
  (using test_list binary provided in the repository).

2 years agoBUG/MINOR: smtpchk: SMTP Service check should gracefully close SMTP transaction
wrightlaw [Thu, 8 Sep 2022 15:10:48 +0000 (16:10 +0100)] 
BUG/MINOR: smtpchk: SMTP Service check should gracefully close SMTP transaction

At present option smtpchk closes the TCP connection abruptly on completion of service checking,
even if successful. This can result in a very high volume of errors in backend SMTP server logs.
This patch ensures an SMTP QUIT is sent and a positive 2xx response is received from the SMTP
server prior to disconnection.

This patch depends on the following one:

 * MINOR: smtpchk: Update expect rule to fully match replies to EHLO commands

This patch should fix the issue #1812. It may be backported as far as 2.2
with the commit above On the 2.2, proxy_parse_smtpchk_opt() function is
located in src/check.c

[cf: I updated reg-tests script accordingly]

2 years agoMINOR: smtpchk: Update expect rule to fully match replies to EHLO commands
Christopher Faulet [Wed, 21 Sep 2022 12:42:47 +0000 (14:42 +0200)] 
MINOR: smtpchk: Update expect rule to fully match replies to EHLO commands

The response to EHLO command is a multiline reply. However the corresponding
expect rule only match on the first line. For now, it is not an issue. But
to be able to send the QUIT command and gracefully close the connection, we
must be sure to consume the full EHLO reply first.

To do so, the regex has been updated to match all 2xx lines at a time.

2 years agoREGTESTS: 4be_1srv_smtpchk_httpchk_layer47errors: Return valid SMTP replies
Christopher Faulet [Wed, 21 Sep 2022 13:05:42 +0000 (15:05 +0200)] 
REGTESTS: 4be_1srv_smtpchk_httpchk_layer47errors: Return valid SMTP replies

The s1 server is acting like a SMTP server. But it sends two CRLF at the end of
each line, while only one CRLF must be returned. It only works becaue both CRLF
are received at the same time.

2 years agoMINOR: clock: do not update the global date too often
Willy Tarreau [Wed, 21 Sep 2022 06:21:45 +0000 (08:21 +0200)] 
MINOR: clock: do not update the global date too often

Tests with forced wakeups on a 24c/48t machine showed that we're caping
at 7.3M loops/s, which means 6.6 microseconds of loop delay without
having anything to do.

This is caused by two factors:
  - the load and update of the now_offset variable
  - the update of the global_now variable

What is happening is that threads are not running within the one-
microsecond time precision provided by gettimeofday(), so each thread
waking up sees a slightly different date and causes undesired updates
to global_now. But worse, these undesired updates mean that we then
have to adjust the now_offset to match that, and adds significant noise
to this variable, which then needs to be updated upon each call.

By only allowing sightly less precision we can completely eliminate
that contention. Here we're ignoring the 5 lowest bits of the usec
part, meaning that the global_now variable may be off by up to 31 us
(16 on avg). The variable is only used to correct the time drift some
threads might be observing in environments where CPU clocks are not
synchronized, and it's used by freq counters. In both cases we don't
need that level of precision and even one millisecond would be pretty
fine. We're just 30 times better at almost no cost since the global_now
and now_offset variables now only need to be updated 30000 times a
second in the worst case, which is unnoticeable.

After this change, the wakeup rate jumped from 7.3M/s to 66M/s, meaning
that the loop delay went from 6.6us to 0.73us, that's a 9x improvement
when under load! With real tasks we're seeing a boost from 28M to 52M
wakeups/s. The clock_update_global_date() function now only takes
1.6%, it's good enough so that we don't need to go further.

2 years agoMINOR: pollers: only update the local date during busy polling
Willy Tarreau [Wed, 21 Sep 2022 06:11:38 +0000 (08:11 +0200)] 
MINOR: pollers: only update the local date during busy polling

This patch modifies epoll, kqueue and evports (the 3 pollers that support
busy polling) to only update the local date in the inner polling loop,
the global one being done when leaving the loop. Testing with epoll on
a 24c/48t machine showed a boost from 53M to 352M loops/s, indicating
that the loop was spending 85% of its time updating the global date or
causing side effects (which was confirmed with perf top showing 67% in
clock_update_global_date() alone).

2 years agoMINOR: clock: split local and global date updates
Willy Tarreau [Wed, 21 Sep 2022 05:37:27 +0000 (07:37 +0200)] 
MINOR: clock: split local and global date updates

Pollers that support busy polling spend a lot of time (and cause
contention) updating the global date when they're looping over themselves
while it serves no purpose: what's needed is only an update on the local
date to know when to stop looping.

This patch splits clock_pudate_date() into a pair of local and global
update functions, so that pollers can be easily improved.

2 years agoCLEANUP: tools: removing escape_chunk() function
Aurelien DARRAGON [Tue, 20 Sep 2022 12:41:38 +0000 (14:41 +0200)] 
CLEANUP: tools: removing escape_chunk() function

    Func is not used anymore. See e3bde807d.

2 years agoBUG/MINOR: log: improper behavior when escaping log data
Aurelien DARRAGON [Tue, 20 Sep 2022 12:33:18 +0000 (14:33 +0200)] 
BUG/MINOR: log: improper behavior when escaping log data

Patrick Hemmer reported an improper log behavior when using
log-format to escape log data (+E option):
Some bytes were truncated from the output:

- escape_string() function now takes an extra parameter that
  allow the caller to specify input string stop pointer in
  case the input string is not guaranteed to be zero-terminated.
- Minors checks were added into lf_text_len() to make sure dst
  string will not overflow.
- lf_text_len() now makes proper use of escape_string() function.

This should be backported as far as 1.8.

2 years agoBUG/MINOR: mux-h1: Account consumed output data on synchronous connection error
Christopher Faulet [Thu, 15 Sep 2022 14:21:55 +0000 (16:21 +0200)] 
BUG/MINOR: mux-h1: Account consumed output data on synchronous connection error

The commit 372b38f935 ("BUG/MEDIUM: mux-h1: Handle connection error after a
synchronous send") introduced a bug. In h1_snd_buf(), consumed data are not
properly accounted if a connection error is detected. Indeed, data are
consumed when the output buffer is filled. But, on connection error, we exit
from the loop without incremented total variable accordingly.

When this happens, this leaves the channel buffer in an inconsistent
state. The buffer may be empty with some output at the channel level.
Because an error is reported, it is harmless. But it is safer to fix this
bug now to avoid any regression in future.

This patch must be backported as far as 2.2.

2 years agoBUG/MEDIUM: mux-quic: properly trim HTX buffer on snd_buf reset
Amaury Denoyelle [Tue, 20 Sep 2022 12:46:40 +0000 (14:46 +0200)] 
BUG/MEDIUM: mux-quic: properly trim HTX buffer on snd_buf reset

MUX QUIC snd_buf operation whill return early if a qcs instance is
resetted. In this case, HTX is left untouched and the callback returns
the whole bufer size. This lead to an undefined behavior as the stream
layer is notified about a transfer but does not see its HTX buffer
emptied. In the end, the transfer may stall which will lead to a leak on
session.

To fix this, HTX buffer is now resetted when snd_buf is short-circuited.
This should fix the issue as now the stream layer can continue the
transfer until its completion.

This patch has already been tested by Tristan and is reported to solve
the github issue #1801.

This should be backported up to 2.6.

2 years agoMINOR: mux-quic: refactor snd_buf
Amaury Denoyelle [Mon, 19 Sep 2022 15:14:27 +0000 (17:14 +0200)] 
MINOR: mux-quic: refactor snd_buf

Factorize common code between h3 and hq-interop snd_buf operation. This
is inserted in MUX QUIC snd_buf own callback.

The h3/hq-interop API has been adjusted to directly receive a HTX
message instead of a plain buf. This led to extracting part of MUX QUIC
snd_buf in qmux_http module.

This should be backported up to 2.6.

2 years agoREORG: mux-quic: export HTTP related function in a dedicated file
Amaury Denoyelle [Mon, 19 Sep 2022 15:02:28 +0000 (17:02 +0200)] 
REORG: mux-quic: export HTTP related function in a dedicated file

Extract function dealing with HTX outside of MUX QUIC. For the moment,
only rcv_buf stream operation is concerned.

The main objective is to be able to support both TCP and HTTP proxy mode
with a common base and add specialized modules on top of it.

This should be backported up to 2.6.

2 years agoREORG: mux-quic: extract traces in a dedicated source file
Amaury Denoyelle [Mon, 19 Sep 2022 14:12:38 +0000 (16:12 +0200)] 
REORG: mux-quic: extract traces in a dedicated source file

QUIC MUX implements several APIs to interface with stream, quic-conn and
app-ops layers. It is planified to better separate this roles, possibly
by using several files.

The first step is to extract QUIC MUX traces in a dedicated source
files. This will allow to reuse traces in multiple files.

The main objective is to be
able to support both TCP and HTTP proxy mode with a common base and add
specialized modules on top of it.

This should be backported up to 2.6.

2 years agoBUG/MINOR: mux-quic: do not keep detached qcs with empty Tx buffers
Amaury Denoyelle [Tue, 13 Sep 2022 14:49:21 +0000 (16:49 +0200)] 
BUG/MINOR: mux-quic: do not keep detached qcs with empty Tx buffers

A qcs instance free may be postponed in stream detach operation if the
stream is not locally closed. This condition is there to achieve
transfering data still present in Tx buffer. Once all data have been
emitted to quic-conn layer, qcs instance can be released.

However, the stream is only closed locally if HTX EOM has been seen or
it has been resetted. In case the transfer finished without EOM, a
detached qcs won't be freed even if there is no more activity on it.

This bug was not reproduced but was found on code analysis. Its precise
impact is unknown but it should not cause any leak as all qcs instances
are freed with its parent qcc connection : this should eventually happen
on MUX timeout or QUIC idle timeout.

To adjust this, condition to mark a stream as locally closed has been
extended. On qcc_streams_sent_done() notification, if its Tx buffer has
been fully transmitted, it will be closed if either FIN STREAM was set
or the stream is detached.

This must be backported up to 2.6.

2 years agoOPTIM: hpack-huff: reduce the cache footprint of the huffman decoder
Willy Tarreau [Tue, 20 Sep 2022 05:27:15 +0000 (07:27 +0200)] 
OPTIM: hpack-huff: reduce the cache footprint of the huffman decoder

Some tables are currently used to decode bit blocks and lengths. We do
see such lookups in perf top. We have 4 512-byte tables and one 64-byte
one. Looking closer, the second half of the table (length) has so few
variations that most of the time it will be computed in a single "if",
and never more than 3. This alone allows to cut the tables in half. In
addition, one table (bits 15-11) is only 32-element long, while another
one (bits 11-4) starts at 0x60, so we can merge the two as they do not
overlap, and further save size. We're now down to 4 256-entries tables.

This is visible in h3 and h2 where the max request rate is slightly higher
(e.g. +1.6% for h2). The huff_dec() function got slightly larger but the
overall code size shrunk:

  $ nm --size haproxy-before | grep huff_dec
  000000000000029e T huff_dec
  $ nm --size haproxy-after | grep huff_dec
  0000000000000345 T huff_dec
  $ size haproxy-before haproxy-after
     text    data     bss     dec     hex filename
  7591126  569268 2761348 10921742         a6a70e haproxy-before
  7591082  568180 2761348 10920610         a6a2a2 haproxy-after

2 years agoMINOR: httpclient: enabled the use of SNI presets
Miroslav Zagorac [Mon, 19 Sep 2022 10:20:29 +0000 (12:20 +0200)] 
MINOR: httpclient: enabled the use of SNI presets

This commit allows setting SNI outside http_client.c code.

2 years agoCLEANUP: httpclient: deleted unused variables
Miroslav Zagorac [Mon, 19 Sep 2022 10:18:53 +0000 (12:18 +0200)] 
CLEANUP: httpclient: deleted unused variables

The locally defined static variables 'httpclient_srv_raw' and
'httpclient_srv_ssl' are not used anywhere in the source code,
except that they are set in the httpclient_precheck() function.

2 years agoBUG/MEDIUM: mux-quic: fix nb_hreq decrement
Amaury Denoyelle [Mon, 19 Sep 2022 09:58:24 +0000 (11:58 +0200)] 
BUG/MEDIUM: mux-quic: fix nb_hreq decrement

nb_hreq is a counter on qcc for active HTTP requests. It is incremented
for each qcs where a full HTTP request was received. It is decremented
when the stream is closed locally :
- on HTTP response fully transmitted
- on stream reset

A bug will occur if a stream is resetted without having processed a full
HTTP request. nb_hreq will be decremented whereas it was not
incremented. This will lead to a crash when building with
DEBUG_STRICT=2. If BUG_ON_HOT are not active, nb_hreq counter will wrap
which may break the timeout logic for the connection.

This bug was triggered on haproxy.org. It can be reproduced by
simulating the reception of a STOP_SENDING frame instead of a STREAM one
by patching qc_handle_strm_frm() :

+       if (quic_stream_is_bidi(strm_frm->id))
+               qcc_recv_stop_sending(qc->qcc, strm_frm->id, 0);
+       //ret = qcc_recv(qc->qcc, strm_frm->id, strm_frm->len,
+       //               strm_frm->offset.key, strm_frm->fin,
+       //               (char *)strm_frm->data);

To fix this bug, a qcs is now flagged with a new QC_SF_HREQ_RECV. This
is set when the full HTTP request is received. When the stream is closed
locally, nb_hreq will be decremented only if this flag was set.

This must be backported up to 2.6.