]> git.ipfire.org Git - thirdparty/haproxy.git/log
thirdparty/haproxy.git
2 years agoMINOR: event_hdl: add event handler base api
Aurelien DARRAGON [Wed, 16 Nov 2022 17:06:28 +0000 (18:06 +0100)] 
MINOR: event_hdl: add event handler base api

Adding base code to provide subscribe/publish API for internal
events processing.

event_hdl provides two complementary APIs, both are implemented
in src/event_hdl.c and include/haproxy/event_hdl{-t.h,.h}:

One API targeting developers that want to register event handlers
that will be notified on specific events.
(SUBSCRIBE)

One API targeting developers that want to notify registered handlers
about an event.
(PUBLISH)

This feature is being considered to address the following scenarios:
- mailers code refactoring (getting rid of deprecated
tcp-check ruleset implementation)
- server events from lua code (registering user defined
lua function that is executed with relevant data when a
server is dynamically added/removed or on server state change)
- providing a stable and easy to use API for upcoming
developments that rely on specific events to perform actions.
(e.g: ressource cleanup when a server is deleted from haproxy)

At this time though, we don't have much use cases in mind in addition to
server events handling, but the API is aimed at being multipurpose
so that new event families, with their own particularities, can be
easily implemented afterwards (and hopefully) without requiring breaking
changes to the API.

Moreover, you should know that the API was not designed to cope well
with high rate event publishing.
Mostly because publishing means iterating over unsorted subscriber list.
So it won't scale well as subscriber list increases, but it is intended in
order to keep the code simple and versatile.

Instead, it is assumed that events implemented using this API
should be periodic events, and that events related to critical
io/networking processing should be handled using
dedicated facilities anyway.
(After all, this is meant to be a general purpose event API)

Apart from being easily extensible, one of the main goals of this API is
to make subscriber code as simple and safe as possible.

This is done by offering multiple event handling modes:
- SYNC mode:
publishing code directly
leverages handler code (callback function)
and handler code has a direct access to "live" event data
(pointers mostly, alongside with lock hints/context
so that accessing data pointers can be done properly)
- normal ASYNC mode:
handler is executed in a backward compatible way with sync mode,
so that it is easy to switch from and to SYNC/ASYNC mode.
Only here the handler has access to "offline" event data, and
not "live" data (ptrs) so that data consistency is guaranteed.
By offline, you should understand "snapshot" of relevant data
at the time of the event, so that the handler can consume it
later (even if associated ressource is not valid anymore)
- advanced ASYNC mode
same as normal ASYNC mode, but here handler is not a function
that is executed with event data passed as argument: handler is a
user defined tasklet that is notified when event occurs.
The tasklet may consume pending events and associated data
through its own message queue.

ASYNC mode should be considered first if you don't rely on live event
data and you wan't to make sure that your code has the lowest impact
possible on publisher code. (ie: you don't want to break stuff)

Internal API documentation will follow:
You will find more details about the notions we roughly approached here.

2 years agoLICENSE: wurfl: clarify the dummy library license.
scientiamobile [Thu, 1 Dec 2022 16:21:10 +0000 (17:21 +0100)] 
LICENSE: wurfl: clarify the dummy library license.

This clarifies that LGPL is also permitted for the wurfl.h dummy file.
Should be backported where relevant.

Signed-off-by: Luca Passani <luca.passani@scientiamobile.com>
2 years agoMINOR: debug: add a balance of alloc - free at the end of the memstats dump
Willy Tarreau [Wed, 30 Nov 2022 16:16:51 +0000 (17:16 +0100)] 
MINOR: debug: add a balance of alloc - free at the end of the memstats dump

When digging into suspected memory leaks, it's cumbersome to count the
number of allocations and free calls. Here we're adding a summary at the
end of the sum of allocs minus the sum of frees, excluding realloc since
we can't know how much it releases upon each call. This means that when
doing many realloc+free the count may be negative but in practice there
are very few reallocs so that's not a problem. Also the size/call is signed
and corresponds to the average size allocated (e.g. leaked) per call.

It seems to work reasonably well for now:

  > debug dev memstats match buf
  quic_conn.c:2978       P_FREE  size:   1239547904  calls:     75656  size/call:  16384 buffer
  quic_conn.c:2960      P_ALLOC  size:   1239547904  calls:     75656  size/call:  16384 buffer
  mux_quic.c:393        P_ALLOC  size:   9112780800  calls:    556200  size/call:  16384 buffer
  mux_quic.c:383        P_ALLOC  size:  17783193600  calls:   1085400  size/call:  16384 buffer
  mux_quic.c:159         P_FREE  size:   8935833600  calls:    545400  size/call:  16384 buffer
  mux_quic.c:142         P_FREE  size:   9112780800  calls:    556200  size/call:  16384 buffer
  h3.c:776              P_ALLOC  size:   8935833600  calls:    545400  size/call:  16384 buffer
  quic_stream.c:166      P_FREE  size:    975241216  calls:     59524  size/call:  16384 buffer
  quic_stream.c:127      P_FREE  size:   7960592384  calls:    485876  size/call:  16384 buffer
  stream.c:772           P_FREE  size:      8798208  calls:       537  size/call:  16384 buffer
  stream.c:768           P_FREE  size:      2424832  calls:       148  size/call:  16384 buffer
  stream.c:751          P_ALLOC  size:   8852062208  calls:    540287  size/call:  16384 buffer
  stream.c:641           P_FREE  size:   8849162240  calls:    540110  size/call:  16384 buffer
  stream.c:640           P_FREE  size:   8847360000  calls:    540000  size/call:  16384 buffer
  channel.h:850         P_ALLOC  size:      2441216  calls:       149  size/call:  16384 buffer
  channel.h:850         P_ALLOC  size:      5914624  calls:       361  size/call:  16384 buffer
  dynbuf.c:55            P_FREE  size:        32768  calls:         2  size/call:  16384 buffer
  Total                 BALANCE  size:            0  calls:   5606906  size/call:      0 (excl. realloc)

Let's see how useful this becomes over time.

2 years agoMINOR: debug: support pool filtering on "debug dev memstats"
Willy Tarreau [Wed, 30 Nov 2022 15:42:54 +0000 (16:42 +0100)] 
MINOR: debug: support pool filtering on "debug dev memstats"

Sometimes when debugging it's convenient to be able to focus only on
certain pools. Just like we did for "show pools", let's add a filter
based on a prefix on "debug dev memstats match <prefix>".

2 years agoMEDIUM: 51d: add support for 51Degrees V4 with Hash algorithm
Dragan Dosen [Mon, 14 Feb 2022 12:05:45 +0000 (13:05 +0100)] 
MEDIUM: 51d: add support for 51Degrees V4 with Hash algorithm

This patch also adds a set of new global options:

- 51degrees-use-performance-graph { on | off }
- 51degrees-use-predictive-graph { on | off }
- 51degrees-drift <number>
- 51degrees-difference <number>
- 51degrees-allow-unmatched { on | off }

To build using the latest 51Degrees V4 engine with Hash algorithm, set
USE_51DEGREES_V4=1.

Other supported build options are 51DEGREES_INC, 51DEGREES_LIB and
51DEGREES_SRC which needs to be set to the directory that contains
headers and C files. For example:

make TARGET=<target> USE_51DEGREES_V4=1 51DEGREES_SRC='51D_REPO_PATH'/src

2 years ago[RELEASE] Released version 2.8-dev0 v2.8-dev0
Willy Tarreau [Thu, 1 Dec 2022 14:25:34 +0000 (15:25 +0100)] 
[RELEASE] Released version 2.8-dev0

Released version 2.8-dev0 with the following main changes :
    - MINOR: version: mention that it's development again

2 years agoMINOR: version: mention that it's development again
Willy Tarreau [Thu, 1 Dec 2022 14:23:12 +0000 (15:23 +0100)] 
MINOR: version: mention that it's development again

This essentially reverts d705b85a4a43e9be5c99796e2b83197b42d6918e.

2 years ago[RELEASE] Released version 2.7.0 v2.7.0
Willy Tarreau [Thu, 1 Dec 2022 14:16:46 +0000 (15:16 +0100)] 
[RELEASE] Released version 2.7.0

Released version 2.7.0 with the following main changes :
    - MINOR: ssl: forgotten newline in error messages on ca-file
    - BUG/MINOR: ssl: shut the ca-file errors emitted during httpclient init
    - DOC: config: provide some configuration hints for "http-reuse"
    - DOC: config: refer to section about quoting in the "add_item" converter
    - DOC: halog: explain how to use -ac and -ad in the help message
    - DOC: config: clarify the fact that SNI should not be used in HTTP scenarios
    - DOC: config: mention that a single monitor-uri rule is supported
    - DOC: config: explain how default matching method for ACL works
    - DOC: config: clarify the fact that "retries" is not just for connections
    - BUILD: halog: fix missing double-quote at end of help line
    - DOC: config: clarify the -m dir and -m dom pattern matching methods
    - MINOR: activity: report uptime in "show activity"
    - REORG: activity/cli: move the "show activity" handler to activity.c
    - DEV: poll: add support for epoll
    - DEV: tcploop: centralize the polling code into wait_for_fd()
    - DEV: tcploop: add support for POLLRDHUP when supported
    - DEV: tcploop: do not report an error on POLLERR
    - DEV: tcploop: add optional support for epoll
    - SCRIPTS: announce-release: add a link to the data plane API
    - CLEANUP: stick-table: fill alignment holes in the stktable struct
    - MINOR: stick-table: store a per-table hash seed and use it
    - MINOR: stick-table: show the shard number in each entry's "show table" output
    - CLEANUP: ncbuf: remove ncb_blk args by value
    - CLEANUP: ncbuf: inline small functions
    - CLEANUP: ncbuf: use standard BUG_ON with DEBUG_STRICT
    - BUG/MINOR: quic: Endless loop during retransmissions
    - MINOR: mux-h2: add the expire task and its expiration date in "show fd"
    - BUG/MINOR: peers: always initialize the stksess shard value
    - REGTESTS: fix peers-related regtests regarding "show table"
    - BUG/MEDIUM: mux-h1: Close client H1C on EOS when there is no output data
    - MINOR: stick-table: change the API of the function used to calculate the shard
    - CLEANUP: peers: factor out the key len calculation in received updates
    - BUG/MINOR: peers: always update the stksess shard number on incoming updates
    - CLEANUP: assorted typo fixes in the code and comments
    - MINOR: mux-h1: add the expire task and its expiration date in "show fd"
    - MINOR: debug: improve error handling on the memstats command parser
    - BUILD: quic: allow build with USE_QUIC and USE_OPENSSL_WOLFSSL
    - CLEANUP: anon: clarify the help message on "debug dev hash"
    - MINOR: debug: relax access restrictions on "debug dev hash" and "memstats"
    - SCRIPTS: run-regtests: add a version check
    - MINOR: version: mention that it's stable now

2 years agoMINOR: version: mention that it's stable now
Willy Tarreau [Thu, 1 Dec 2022 14:15:24 +0000 (15:15 +0100)] 
MINOR: version: mention that it's stable now

This version will be maintained up to around Q1 2024. The INSTALL file
also mentions it.

2 years agoSCRIPTS: run-regtests: add a version check
Willy Tarreau [Wed, 30 Nov 2022 17:44:33 +0000 (18:44 +0100)] 
SCRIPTS: run-regtests: add a version check

It happens from time to time while switching between branches and/or
updating after someone else's changes that regtests are run by accident
on the wrong binary, typically the one the tests were run on during
development and not with the latest adaptations. And obviously it's
when this happens that we break the CI. There are various causes to
this but they all come down to humans context-switching a lot, and
there's no real fix for this that doesn't add even more burden hence
increases the overhead. However we can help the human detect such
mistakes very easily.

This change here will compare the version of the haproxy binary to
the version of the tree, and will emit a warning in the regtest output
if they do not match, regardless of the outcome of the test. This is
sufficient in case of failures because these are quickly glanced over,
and is sufficient as well in case of accidental success because the
warning is the last message. E.g:

  ########################## Starting vtest ##########################
  Testing with haproxy version: 2.7-dev10-cfcdbc-38
  Warning: version does not match the current tree (2.7-dev10-111c78-39)
  0 tests failed, 0 tests skipped, 182 tests passed

This should not affect builds made out of a git tree because the version
is retrieved using "make version", or exactly the same way as it's passd
to the haproxy binary. We just need to know what "make" command to run,
so $MAKE is used primarily, falling back to "make" then to "gmake". In
case all of these fail, we just ignore the version check. This should be
sufficient to catch human mistakes without affecting the CI.

2 years agoMINOR: debug: relax access restrictions on "debug dev hash" and "memstats"
Willy Tarreau [Wed, 30 Nov 2022 16:51:47 +0000 (17:51 +0100)] 
MINOR: debug: relax access restrictions on "debug dev hash" and "memstats"

These two have absolutely zero impact on the process and do not need to
be restricted to the expert mode. The first one calculates a string hash
that can be used by anyone when checking a dump; the second one may be
used by anyone tracking a memory leak, and is cumbersome to use due to
the "expert-mode on" that needs to be prepended. In addition this gives
bad habits to users and needlessly taints the process. So let's drop
this restriction for these two commands.

2 years agoCLEANUP: anon: clarify the help message on "debug dev hash"
Willy Tarreau [Wed, 30 Nov 2022 16:47:08 +0000 (17:47 +0100)] 
CLEANUP: anon: clarify the help message on "debug dev hash"

This command is used to hash a section name using the current anon key,
it was brought in 2.7 by commit 54966dffd ("MINOR: anon: store the
anonymizing key in the CLI's appctx"). However the help message only
says "return msg hashed" which is misleading because if anon mode is
not enabled, it returns the string as-is. Let's just mention this
condition in the help message, and also fix the alphabetical ordering
and alignment on the line.

2 years agoBUILD: quic: allow build with USE_QUIC and USE_OPENSSL_WOLFSSL
Stefan Eissing [Wed, 30 Nov 2022 14:16:38 +0000 (15:16 +0100)] 
BUILD: quic: allow build with USE_QUIC and USE_OPENSSL_WOLFSSL

WolfSSL does not implement the TLS1_3_CK_AES_128_CCM_SHA256 cipher as
well as the SSL_ERROR_WANT_ASYNC, SSL_ERROR_WANT_ASYNC_JOB and
SSL_ERROR_WANT_CLIENT_HELLO_CB error codes.

This patch disables them for WolfSSL.

Signed-off-by: William Lallemand <wlallemand@haproxy.org>
2 years agoMINOR: debug: improve error handling on the memstats command parser
Willy Tarreau [Wed, 30 Nov 2022 15:50:48 +0000 (16:50 +0100)] 
MINOR: debug: improve error handling on the memstats command parser

"debug dev memstats" supports various options but silently ignores the
unknown ones. Let's make sure it returns indications about what it
expects, as the help message is quite limited otherwise.

2 years agoMINOR: mux-h1: add the expire task and its expiration date in "show fd"
Christopher Faulet [Wed, 30 Nov 2022 13:49:56 +0000 (14:49 +0100)] 
MINOR: mux-h1: add the expire task and its expiration date in "show fd"

Just like for the H2 multiplexer, info about the H1 connection task is now
displayed in "show fd" output. The task pointer is displayed and, if not
null, its expiration date.

It may be useful to backport it.

2 years agoCLEANUP: assorted typo fixes in the code and comments
Ilya Shipitsin [Wed, 30 Nov 2022 11:22:42 +0000 (16:22 +0500)] 
CLEANUP: assorted typo fixes in the code and comments

This is 33rd iteration of typo fixes

2 years agoBUG/MINOR: peers: always update the stksess shard number on incoming updates
Willy Tarreau [Tue, 29 Nov 2022 17:01:28 +0000 (18:01 +0100)] 
BUG/MINOR: peers: always update the stksess shard number on incoming updates

If shards are in use, we must fill the shard number on incoming updates,
otherwise some entries are assigned shard number zero, and may be broadcast
everywhere once updated, instead of being sent only to the peers having the
same shard number.

This fixes commit 36d156564 ("MINOR: peers: Support for peer shards"). No
backport is needed.

2 years agoCLEANUP: peers: factor out the key len calculation in received updates
Willy Tarreau [Tue, 29 Nov 2022 16:43:10 +0000 (17:43 +0100)] 
CLEANUP: peers: factor out the key len calculation in received updates

In peer_treat_updatemsg(), the lower layers of the stick-table code are
reimplemented, and the key length is never really known for an entry
being processed, it depends on the type being parsed and the moment
where it's done. This makes it quite difficult to stuff some shard
number calculation there.

This patch adds a keylen local variable that is always set to the length
of the current key depending on its type. It takes this opportunity for
reducing redudant expressions involving this length and always using the
new variable instead, limiting the risk of errors. Arguably that code
would have been way simpler by creating a dummy stktable_key and passing
it to stksess_new() as done anywhere else, but let's not change all that
a few days before the release.

2 years agoMINOR: stick-table: change the API of the function used to calculate the shard
Willy Tarreau [Tue, 29 Nov 2022 16:36:44 +0000 (17:36 +0100)] 
MINOR: stick-table: change the API of the function used to calculate the shard

The function used to calculate the shard number currently requires a
stktable_key on input for this. Unfortunately, it happens that peers
currently miss this calculation and they do not provide stktable_key
at all, instead they're open-coding all the low-level stick-table work
(hence why it's missing). Thus we'll need to be able to calculate the
shard number in keys coming from peers as well but the current API does
not make it possible.

This commit addresses this by inverting the order where the length and
the shard number are used. Now the low-level function is independent on
stksess and stktable_key, it takes a table, pointer and length and does
all the job. The upper function takes care of the type and key to get
the its length, and is for use only from stick-table code.

This doesn't change anything except that the low-level one will be usable
from outside (hence why it's exported now).

2 years agoBUG/MEDIUM: mux-h1: Close client H1C on EOS when there is no output data
Christopher Faulet [Tue, 29 Nov 2022 16:16:30 +0000 (17:16 +0100)] 
BUG/MEDIUM: mux-h1: Close client H1C on EOS when there is no output data

If the client closes the connection while there is no pending outgoing data,
the H1 connection must be released. However, it was switched to CLOSING
state instead. Thus the client connection was closed on client timeout.

It is side effect of the commif d1b573059a ("MINOR: mux-h1: Avoid useless
call to h1_send() if no error is sent"). Before, the extra call to h1_send()
was able to fix the H1C state.

To fix the bug and make switch to close state (CLOSING or CLOSED) less
errorprone, h1_close() helper function is systematically used.

It is a 2.7-specific bug. No backport needed.

2 years agoREGTESTS: fix peers-related regtests regarding "show table"
Willy Tarreau [Tue, 29 Nov 2022 15:29:12 +0000 (16:29 +0100)] 
REGTESTS: fix peers-related regtests regarding "show table"

When I added commit 16b282f4b ("MINOR: stick-table: show the shard
number in each entry's "show table" output"), I don't know how but
I managed to mess up my reg tests since everything worked fine,
most likely by running it on a binary built in the wrong branch.
Several reg tests include some table outputs that were upset by the
new "shard=" field. This test added them and revealed at the same
time that entries learned over peers are not properly initialized,
which will be fixed in a future series of fixes.

This commit requires previous fix "BUG/MINOR: peers: always
initialize the stksess shard value" so as not to trip on entries
learned from peers.

2 years agoBUG/MINOR: peers: always initialize the stksess shard value
Willy Tarreau [Tue, 29 Nov 2022 15:08:35 +0000 (16:08 +0100)] 
BUG/MINOR: peers: always initialize the stksess shard value

We need to initialize the shard value in __stksess_init() because there is
not necessarily a key to make it happen later, resulting in an uninitialized
shard value appearing in the entry, typically when entries are learned from
peers. This fixes commit 36d156564 ("MINOR: peers: Support for peer shards"),
no backport is needed.

Note however that it is not sufficient to completely fix the peers code, the
shard value remains zero because the setting of the key was open-coded in
the peers code and these parts were not identified when adding support for
shards.

2 years agoMINOR: mux-h2: add the expire task and its expiration date in "show fd"
Willy Tarreau [Tue, 29 Nov 2022 14:26:43 +0000 (15:26 +0100)] 
MINOR: mux-h2: add the expire task and its expiration date in "show fd"

Some issues such as #1929 seem to involve a task without timeout but we
can't find the condition to reproduce this in the code. However, not having
this info in the output doesn't help, so this patch adds the task pointer
and its timeout (when the task is non-null). It may be useful to backport
it.

2 years agoBUG/MINOR: quic: Endless loop during retransmissions
Frédéric Lécaille [Mon, 28 Nov 2022 16:21:45 +0000 (17:21 +0100)] 
BUG/MINOR: quic: Endless loop during retransmissions

qc_dgrams_retransmit() could reuse the same local list and could splice it two
times to the packet number space list of frame to be send/resend. This creates a
loop in this list and makes qc_build_frms() possibly endlessly loop when trying
to build frames from the packet number space list of frames. Then haproxy aborts.

This issue could be easily reproduced patching qc_build_frms() function to set <dlen>
variable value to 0 after having built at least 10 CRYPTO frames and using ngtcp2
as client with 30% packet loss in both direction.

Thank you to @gabrieltz for having reported this issue in GH #1903.

Must be backported to 2.6.

2 years agoCLEANUP: ncbuf: use standard BUG_ON with DEBUG_STRICT
Amaury Denoyelle [Mon, 28 Nov 2022 14:10:30 +0000 (15:10 +0100)] 
CLEANUP: ncbuf: use standard BUG_ON with DEBUG_STRICT

ncbuf can be compiled for haproxy or standalone to run unit test suite.
For the latest mode, BUG_ON() macro has been re-implemented in a simple
version.

The inclusion of the default or the redefined macro relied on DEBUG_DEV.
Change this to now rely on DEBUG_STRICT as this is activated for the
default build.

This change is safe as only BUG_ON_HOT() macro is used in ncbuf code,
which is activated only with the default value DEBUG_STRICT=2.

This should be backported up to 2.6.

2 years agoCLEANUP: ncbuf: inline small functions
Amaury Denoyelle [Mon, 28 Nov 2022 14:08:14 +0000 (15:08 +0100)] 
CLEANUP: ncbuf: inline small functions

ncbuf API relies on lot of small functions. Mark these functions as
inline to reduce call invocations and facilitate compiler optimizations
to reduce code size.

This should be backported up to 2.6.

2 years agoCLEANUP: ncbuf: remove ncb_blk args by value
Amaury Denoyelle [Mon, 28 Nov 2022 10:06:42 +0000 (11:06 +0100)] 
CLEANUP: ncbuf: remove ncb_blk args by value

ncb_blk structure is used to represent a block of data or a gap in a
non-contiguous buffer. This is used in several functions for ncbuf
implementation. Before this patch, ncb_blk was passed by value, which is
sub-optimal. Replace this by const pointer arguments.

This has the side-effect of suppressing a compiler warning reported in
older GCC version :
  CC      src/http_conv.o
  src/ncbuf.c: In function 'ncb_blk_next':
  src/ncbuf.c:170: warning: 'blk.end' may be used uninitialized in this function

This should be backported up to 2.6.

2 years agoMINOR: stick-table: show the shard number in each entry's "show table" output
Willy Tarreau [Tue, 29 Nov 2022 10:55:18 +0000 (11:55 +0100)] 
MINOR: stick-table: show the shard number in each entry's "show table" output

Stick-tables support sharding to multiple peers but there was no way to
know to what shard an entry was going to be sent. Let's display this in
the "show table" output to ease debugging.

2 years agoMINOR: stick-table: store a per-table hash seed and use it
Willy Tarreau [Mon, 28 Nov 2022 17:53:06 +0000 (18:53 +0100)] 
MINOR: stick-table: store a per-table hash seed and use it

Instead of using memcpy() to concatenate the table's name to the key when
allocating an stksess, let's compute once for all a per-table seed at boot
time and use it to calculate the key's hash. This saves two memcpy() and
the usage of a chunk, it's always nice in a fast path.

When tested under extreme conditions with a 80-byte long table name, it
showed a 1% performance increase.

2 years agoCLEANUP: stick-table: fill alignment holes in the stktable struct
Willy Tarreau [Mon, 28 Nov 2022 17:48:11 +0000 (18:48 +0100)] 
CLEANUP: stick-table: fill alignment holes in the stktable struct

There were two 32-bit holes in the stktable struct surrounding 32-bit
words, so let's just reorder them a little bit to address the issue.

2 years agoSCRIPTS: announce-release: add a link to the data plane API
Willy Tarreau [Mon, 28 Nov 2022 06:22:46 +0000 (07:22 +0100)] 
SCRIPTS: announce-release: add a link to the data plane API

Since Marko announced at HAProxyConf 2022 that the data plane API is
mostly complete and will now follow the same release cycle as haproxy
starting with 2.7, it's probably the right moment to encourage users
to start trying it so that we can hope to migrate all the painful
discovery stuff there in a not too distant future.

Let's just point to the latest release for now. We'll see in the future
if we need to adapt the link depending on the branch.

2 years agoDEV: tcploop: add optional support for epoll
Willy Tarreau [Fri, 25 Nov 2022 16:05:05 +0000 (17:05 +0100)] 
DEV: tcploop: add optional support for epoll

When -e is passed, epoll is used instead of poll. The FD is added
then removed around the call to epoll_wait() so that we don't need
to track it. The only purpose is to compare events reported by each
syscall.

2 years agoDEV: tcploop: do not report an error on POLLERR
Willy Tarreau [Fri, 25 Nov 2022 16:04:05 +0000 (17:04 +0100)] 
DEV: tcploop: do not report an error on POLLERR

Actually this breaks certain client tests where the server closes with
RST, it prevents from reading the final data so better not abort on
that.

2 years agoDEV: tcploop: add support for POLLRDHUP when supported
Willy Tarreau [Fri, 25 Nov 2022 15:15:20 +0000 (16:15 +0100)] 
DEV: tcploop: add support for POLLRDHUP when supported

This is just in order to closer match what haproxy does.

2 years agoDEV: tcploop: centralize the polling code into wait_for_fd()
Willy Tarreau [Fri, 25 Nov 2022 15:05:46 +0000 (16:05 +0100)] 
DEV: tcploop: centralize the polling code into wait_for_fd()

There are multiple call places for poll(), let's first centralize them
to make it easier to enhance it. All callers now use the wait_for_fd()
function which was extended to take a timeout and which can return the
indication that an error was seen.

2 years agoDEV: poll: add support for epoll
Willy Tarreau [Fri, 25 Nov 2022 15:34:11 +0000 (16:34 +0100)] 
DEV: poll: add support for epoll

When called with -e, epoll is used instead of poll. The poller does
very little in this code (just checks for any event without waiting) but
that's sufficient to see return values.

2 years agoREORG: activity/cli: move the "show activity" handler to activity.c
Willy Tarreau [Fri, 25 Nov 2022 14:32:38 +0000 (15:32 +0100)] 
REORG: activity/cli: move the "show activity" handler to activity.c

Initially the code was placed into cli.c to keep activity.c small and
independent of the cli stuff, but that's no longer the case anyway and
keeping that code over there makes it harder to find. Let's move it to
its more natural place now.

2 years agoMINOR: activity: report uptime in "show activity"
Willy Tarreau [Fri, 25 Nov 2022 14:36:48 +0000 (15:36 +0100)] 
MINOR: activity: report uptime in "show activity"

It happened a few times that it was difficult to figure if a counter was
normal or not in "show activity" based on the uptime. Let's just emit the
uptime value along with the date.

2 years agoDOC: config: clarify the -m dir and -m dom pattern matching methods
Willy Tarreau [Fri, 25 Nov 2022 11:02:25 +0000 (12:02 +0100)] 
DOC: config: clarify the -m dir and -m dom pattern matching methods

There's regularly some confusion about them (do they match at the
beginning, end ? do they support multiple components etc). Tim
suggested to improve the doc in issue #61, it's never too late, so
let's do it now wih a few examples.

2 years agoBUILD: halog: fix missing double-quote at end of help line
Willy Tarreau [Fri, 25 Nov 2022 10:10:19 +0000 (11:10 +0100)] 
BUILD: halog: fix missing double-quote at end of help line

This will tell me to change the line format after testing :-(
This was introduced with commit 286199c24 ("DOC: halog: explain how to
use -ac and -ad in the help message"), no backport is needed unless it's
backported as well.

2 years agoDOC: config: clarify the fact that "retries" is not just for connections
Willy Tarreau [Fri, 25 Nov 2022 10:06:20 +0000 (11:06 +0100)] 
DOC: config: clarify the fact that "retries" is not just for connections

In issue #412 it was rightfully reported that the wording in "retries"
still exclusively speaks about connection attempts, while since L7
retries with "retry-on" it's no longer a limitation. Let's update the
text.

2 years agoDOC: config: explain how default matching method for ACL works
Willy Tarreau [Fri, 25 Nov 2022 09:49:41 +0000 (10:49 +0100)] 
DOC: config: explain how default matching method for ACL works

In issue #698, it's made apparent that the default matching method for
ACL keywords can be confusing when a converter is applied, because
depending on the converters used, users may think that the default
matching method from the sample fetch name might apply to the whole
expression. It's easier to understand that this doesn't make sense
when thinking about converters turning to completely different types
(e.g. hdr_beg(host),do_resolve() returns an IP, thus it's obvious
that _beg makes no sense at all).  This patch states this in the
doc to avoid future confusion.

2 years agoDOC: config: mention that a single monitor-uri rule is supported
Willy Tarreau [Fri, 25 Nov 2022 09:24:44 +0000 (10:24 +0100)] 
DOC: config: mention that a single monitor-uri rule is supported

It was reported in issue #1059 that when multiple monitor-uri rules are
specified, only the last one is used. While this was done on purpose
since a single URI is used, it was not clearly mentioned in the doc,
possibly leading to confusion or wasted time trying to establish a
working setup. Let's clarify this point.

2 years agoDOC: config: clarify the fact that SNI should not be used in HTTP scenarios
Willy Tarreau [Fri, 25 Nov 2022 09:12:12 +0000 (10:12 +0100)] 
DOC: config: clarify the fact that SNI should not be used in HTTP scenarios

As reported by Tim in issue #1373 some warnings are deserved to explain
why using the frontend SNI for routing or connecting to a server is
usually not correct, especially since it can be tempting and used to
make sense in pure TCP scenarios.

2 years agoDOC: halog: explain how to use -ac and -ad in the help message
Willy Tarreau [Fri, 25 Nov 2022 08:40:06 +0000 (09:40 +0100)] 
DOC: halog: explain how to use -ac and -ad in the help message

Tim reported in issue #1435 that halog options -ac/-ad were poorly
documented. They're indeed used to spot infrastructure outages between
the clients and haproxy by detecting abnormal periods of silence followed
by bursts, either affecting the network itself, or also a single machine
(e.g. swapping on an edge client or proxy can cause such patterns).

2 years agoDOC: config: refer to section about quoting in the "add_item" converter
Willy Tarreau [Fri, 25 Nov 2022 08:27:15 +0000 (09:27 +0100)] 
DOC: config: refer to section about quoting in the "add_item" converter

As requested by Nick in issue #1719, let's add a reference to the section
about quoting there, since add_item() will often be used with commas and
it's easy to mess up.

2 years agoDOC: config: provide some configuration hints for "http-reuse"
Willy Tarreau [Fri, 25 Nov 2022 08:17:18 +0000 (09:17 +0100)] 
DOC: config: provide some configuration hints for "http-reuse"

This adds some configuration hints regarding various workloads that do
not manage to achieve high reuse rates due to too low a global maxconn
or thread groups.

This fixes github issue #1472.

2 years agoBUG/MINOR: ssl: shut the ca-file errors emitted during httpclient init
William Lallemand [Thu, 24 Nov 2022 18:14:19 +0000 (19:14 +0100)] 
BUG/MINOR: ssl: shut the ca-file errors emitted during httpclient init

With an OpenSSL library which use the wrong OPENSSLDIR, HAProxy tries to
load the OPENSSLDIR/certs/ into @system-ca, but emits a warning when it
can't.

This patch fixes the issue by allowing to shut the error when the SSL
configuration for the httpclient is not explicit.

Must be backported in 2.6.

2 years agoMINOR: ssl: forgotten newline in error messages on ca-file
William Lallemand [Thu, 24 Nov 2022 17:45:28 +0000 (18:45 +0100)] 
MINOR: ssl: forgotten newline in error messages on ca-file

Add forgotten newlines in ssl_store_load_ca_from_buf() error messages.

2 years ago[RELEASE] Released version 2.7-dev10 v2.7-dev10
Willy Tarreau [Thu, 24 Nov 2022 16:13:05 +0000 (17:13 +0100)] 
[RELEASE] Released version 2.7-dev10

Released version 2.7-dev10 with the following main changes :
    - MEDIUM: tcp-act: add parameter rst-ttl to silent-drop
    - BUG/MAJOR: quic: Crash upon retransmission of dgrams with several packets
    - MINOR: cli: print parsed command when not found
    - BUG/MAJOR: quic: Crash after discarding packet number spaces
    - CLEANUP: quic: replace "choosen" with "chosen" all over the code
    - MINOR: cli/pools: store "show pools" results into a temporary array
    - MINOR: cli/pools: add sorting capabilities to "show pools"
    - MINOR: cli/pools: add pool name filtering capability to "show pools"
    - DOC: configuration: fix quic prefix typo
    - MINOR: quic: report error if force-retry without cluster-secret
    - MINOR: global: generate random cluster.secret if not defined
    - BUG/MINOR: resolvers: do not run the timeout task when there's no resolution
    - BUG/MINOR: server/idle: at least use atomic stores when updating max_used_conns
    - MINOR: server/idle: make the next_takeover index per-tgroup
    - BUILD: listener: fix build warning on global_listener_rwlock without threads
    - BUG/MAJOR: sched: protect task during removal from wait queue
    - BUILD: sched: fix build with DEBUG_THREAD with the previous commit
    - DOC: quic: add note on performance issue with listener contention
    - BUG/MINOR: cfgparse-listen: fix ebpt_next_dup pointer dereference on proxy "from" inheritance
    - BUG/MINOR: log: fix parse_log_message rfc5424 size check
    - CLEANUP: arg: remove extra check in make_arg_list arg escaping
    - CLEANUP: tools: extra check in utoa_pad
    - MINOR: h1: Consider empty port as invalid in authority for CONNECT
    - MINOR: http: Considere empty ports as valid default ports
    - BUG/MINOR: http-htx: Normalized absolute URIs with an empty port
    - BUG/MINOR: h1: Replace authority validation to conform RFC3986
    - REG-TESTS: http: Add more tests about authority/host matching
    - BUG/MINOR: http-htx: Don't consider an URI as normalized after a set-uri action
    - BUG/MEDIUM: mux-h1: Don't release H1C on timeout if there is a SC attached
    - BUG/MEDIUM: mux-h1: Subscribe for reads on error on sending path
    - BUILD: http-htx: Silent build error about a possible NULL start-line
    - DOC: configuration.txt: add default_value for table_idle signature
    - BUILD: ssl-sock: Silent error about NULL deref in ssl_sock_bind_verifycbk()
    - BUG/MEDIUM: mux-h1: Remove H1C_F_WAIT_NEXT_REQ flag on a next request
    - BUG/MINOR: mux-h1: Fix handling of 408-Request-Time-Out
    - MINOR: mux-h1: Remove H1C_F_WAIT_NEXT_REQ in functions handling errors
    - MINOR: mux-h1: Avoid useless call to h1_send() if no error is sent
    - DOC: configuration.txt: fix typo in table_idle signature
    - BUILD: stick-tables: fix build breakage in xxhash on older compilers
    - BUILD: compiler: include compiler's definitions before ours
    - BUILD: quic: global.h is needed in cfgparse-quic
    - CLEANUP: tools: do not needlessly include xxhash nor cli from tools.h
    - BUILD: flags: really restrict the cases where flags are exposed
    - BUILD: makefile: minor reordering of objects by build time
    - BUILD: quic: silence two invalid build warnings at -O1 with gcc-6.5
    - BUILD: quic: use openssl-compat.h instead of openssl/ssl.h
    - MEDIUM: ssl: add minimal WolfSSL support with OpenSSL compatibility mode
    - MINOR: sample: make the rand() sample fetch function use the statistical_prng
    - MINOR: auth: silence null dereference warning in check_user()
    - CLEANUP: peers: fix format string for status messages (int signedness)
    - CLEANUP: qpack: fix format string in debugging code (int signedness)
    - CLEANUP: qpack: properly use the QPACK macros not HPACK ones in debug code
    - BUG/MEDIUM: quic: fix datagram dropping on queueing failed

2 years agoBUG/MEDIUM: quic: fix datagram dropping on queueing failed
Amaury Denoyelle [Thu, 24 Nov 2022 14:24:38 +0000 (15:24 +0100)] 
BUG/MEDIUM: quic: fix datagram dropping on queueing failed

After reading a datagram, it is enqueud for the thread attached to the
DCID. This is done via quic_lstnr_dgram_dispatch() function. If this
step fails, we remove the datagram from the buffer of quic_receiver_buf.

This step is faulty because we use b_del() instead of b_sub(). If
quic_receiver_buf was not empty, we will remove content from another
datagram while leaving the content of the last read datagram. This
probably produces valid datagram dropping and may even result in crash.

As stated, this bug can only happen if qc_lstnr_dgram_dispatch() fails
which happen on two occaions :
* on quic_dgram allocation failure, which should be pretty rare
* on datagram labelled as invalid for QUIC protocol. This may happen
  more frequently depending on the network conditions. Thus, this bug
  has been labelled as a medium one.

This should be backported up to 2.6.

2 years agoCLEANUP: qpack: properly use the QPACK macros not HPACK ones in debug code
Willy Tarreau [Thu, 24 Nov 2022 14:38:26 +0000 (15:38 +0100)] 
CLEANUP: qpack: properly use the QPACK macros not HPACK ones in debug code

There were a few leftovers of DEBUG_HPACK and HPACK_SHT_SIZE instead of
their QPACK equivalent in the QPACK debug code. There's no harm anyway,
but it could lead to confusing results if the tables are not sized
equally.

2 years agoCLEANUP: qpack: fix format string in debugging code (int signedness)
Willy Tarreau [Thu, 24 Nov 2022 14:35:17 +0000 (15:35 +0100)] 
CLEANUP: qpack: fix format string in debugging code (int signedness)

In issue #1939, Ilya mentions that cppchecks warned about use of "%d" to
report the QPACK table's index that's locally stored as an unsigned int.
While technically valid, this will never cause any trouble since indexes
are always small positive values, but better use %u anyway to silence
this warning.

2 years agoCLEANUP: peers: fix format string for status messages (int signedness)
Willy Tarreau [Thu, 24 Nov 2022 14:32:20 +0000 (15:32 +0100)] 
CLEANUP: peers: fix format string for status messages (int signedness)

In issue #1939, Ilya mentions that cppchecks warned about use of "%d" to
report the status state that's locally stored as an unsigned int. While
technically valid, this will never cause any trouble since in the end
what we store there are the applet's states (just a few enum values).
Better use %u anyway to silence this warning.

2 years agoMINOR: auth: silence null dereference warning in check_user()
Aurelien DARRAGON [Thu, 24 Nov 2022 07:37:13 +0000 (08:37 +0100)] 
MINOR: auth: silence null dereference warning in check_user()

In GH issue #1940 cppcheck warns about a possible null-dereference in
check_user() when DEBUG_AUTH is enabled. Indeed, <ep> may potentially
be NULL because upon error crypt_r() and crypt() may return a null
pointer. However it's not directly derefenced, it is only passed to
printf() with '%s' fmt. While it is in practice fine with the printf
implementations we care about (that check strings against null before
printing them), it is undefined behavior according to the spec, hence
the warning.

Let's check <ep> before passing it to printf. This should partly
solve GH #1940.

2 years agoMINOR: sample: make the rand() sample fetch function use the statistical_prng
Willy Tarreau [Thu, 24 Nov 2022 13:54:29 +0000 (14:54 +0100)] 
MINOR: sample: make the rand() sample fetch function use the statistical_prng

Emeric noticed that producing many randoms to fill a stick table was
saturating on the rand_lock. Since 2.4 we have the statistical PRNG
for low-quality randoms like this one, there is no point in using the
one that was originally implemented for the purpose of creating safe
UUIDs, since the doc itself clearly states that these randoms are not
secure and they have not been in the past either. With this change,
locking contention is completely gone.

2 years agoMEDIUM: ssl: add minimal WolfSSL support with OpenSSL compatibility mode
Uriah Pollock [Wed, 23 Nov 2022 15:41:25 +0000 (16:41 +0100)] 
MEDIUM: ssl: add minimal WolfSSL support with OpenSSL compatibility mode

This adds a USE_OPENSSL_WOLFSSL option, wolfSSL must be used with the
OpenSSL compatibility layer. This must be used with USE_OPENSSL=1.

WolfSSL build options:

   ./configure --prefix=/opt/wolfssl --enable-haproxy

HAProxy build options:

  USE_OPENSSL=1 USE_OPENSSL_WOLFSSL=1 WOLFSSL_INC=/opt/wolfssl/include/ WOLFSSL_LIB=/opt/wolfssl/lib/ ADDLIB='-Wl,-rpath=/opt/wolfssl/lib'

Using at least the commit 54466b6 ("Merge pull request #5810 from
Uriah-wolfSSL/haproxy-integration") from WolfSSL. (2022-11-23).

This is still to be improved, reg-tests are not supported yet, and more
tests are to be done.

Signed-off-by: William Lallemand <wlallemand@haproxy.org>
2 years agoBUILD: quic: use openssl-compat.h instead of openssl/ssl.h
Uriah Pollock [Wed, 23 Nov 2022 16:10:07 +0000 (17:10 +0100)] 
BUILD: quic: use openssl-compat.h instead of openssl/ssl.h

Replace the include of openssl/ssl.h by openssl-compat.h.

Signed-off-by: William Lallemand <wlallemand@haproxy.org>
2 years agoBUILD: quic: silence two invalid build warnings at -O1 with gcc-6.5
Willy Tarreau [Thu, 24 Nov 2022 08:16:41 +0000 (09:16 +0100)] 
BUILD: quic: silence two invalid build warnings at -O1 with gcc-6.5

Gcc 6.5 is now well known for triggering plenty of false "may be used
uninitialized", particularly at -O1, and two of them happen in quic,
quic_tp and quic_conn. Both of them were reviewed and easily confirmed
as wrong (gcc seems to ignore the control flow after the function
returns and believes error conditions are not met). Let's just preset
the variables that bothers it. In quic_tp the initialization was moved
out of the loop since there's no point inflating the code just to
silence a stupid warning.

2 years agoBUILD: makefile: minor reordering of objects by build time
Willy Tarreau [Thu, 24 Nov 2022 07:57:13 +0000 (08:57 +0100)] 
BUILD: makefile: minor reordering of objects by build time

This time the current ordering of common objects remained mostly
unchanged, except for flt_bwlim that was added. However, the SSL
and QUIC build order still had not been handled and were extremely
imbalanced, so they were adjusted. It's even possible to start
building QUIC before openssl to save a little bit more but more
likely that a few large quic files will get split again over time.

2 years agoBUILD: flags: really restrict the cases where flags are exposed
Willy Tarreau [Thu, 24 Nov 2022 07:22:40 +0000 (08:22 +0100)] 
BUILD: flags: really restrict the cases where flags are exposed

A number of internal flags started to be exposed to external programs
at the location of their definition since commit 77acaf5af ("MINOR:
flags: add a new file to host flag dumping macros"). This allowed the
"flags" utility to decode many more of them and always correctly. The
condition to expose them was to rely on the preliminary definition of
EOF that indicates that stdio is already included. But this was a
wrong approach. It only guarantees that snprintf() can safely be used
but still causes large functions to be built. But stdio is often
included before some of these includes, so these heavy inline functions
actually have to be compiled in many cases. The result is that the
build time significantly increased, especially with fast compilers
like gcc -O0 which took +50% or TCC which took +100%!

This patch addresses the problem by instead relying on an explicit
macro HA_EXPOSE_FLAGS that the calling program must explicitly define
before including these files. flags.c does this and that's all. The
previous build time is now restored with a speed up of 20 to 50%
depending on the build options.

2 years agoCLEANUP: tools: do not needlessly include xxhash nor cli from tools.h
Willy Tarreau [Thu, 24 Nov 2022 07:09:12 +0000 (08:09 +0100)] 
CLEANUP: tools: do not needlessly include xxhash nor cli from tools.h

These includes brought by commit 9c76637ff ("MINOR: anon: add new macros
and functions to anonymize contents") resulted in an increase of exactly
20% of the number of lines to build. These include are not needed there,
only tools.c needs xxhash.h.

2 years agoBUILD: quic: global.h is needed in cfgparse-quic
Willy Tarreau [Thu, 24 Nov 2022 07:28:43 +0000 (08:28 +0100)] 
BUILD: quic: global.h is needed in cfgparse-quic

cfgparse-quic accesses some members of the "global" struct but only
includes global-t.h. It actually used to work via tools.h due to a
long dependency chain that brought it, but it will be fixed and will
break cfgparse-quic, so let's fix it first.

2 years agoBUILD: compiler: include compiler's definitions before ours
Willy Tarreau [Thu, 24 Nov 2022 06:51:57 +0000 (07:51 +0100)] 
BUILD: compiler: include compiler's definitions before ours

Building with TCC caused a warning on __attribute__() being redefined,
because we do define it on compilers that don't have it, but we didn't
include the compiler's definitions first to leave it a chance to expose
its definitions. The correct way to do this would be to include
sys/cdefs.h but we currently don't include it explicitly and a few
reports on the net mention some platforms where it could be missing
by default. Let's use inttypes.h instead, it always causes it (or its
equivalent) to be included and we know it's present on supported
platforms since we already depend on it.

No backport is needed.

2 years agoBUILD: stick-tables: fix build breakage in xxhash on older compilers
Willy Tarreau [Thu, 24 Nov 2022 06:35:17 +0000 (07:35 +0100)] 
BUILD: stick-tables: fix build breakage in xxhash on older compilers

Commit 36d156564 ("MINOR: peers: Support for peer shards") reintroduced
a direct dependency on import/xxhash.h which was previously dropped by
commit d5fc8fcb8 ("CLEANUP: Add haproxy/xxhash.h to avoid modifying
import/xxhash.h"). This results in xxhash.h being included twice, which
breaks the build on older compilers which do not like seeing XXH32_hash_t
being defined twice.

Let's just use include/haproxy/xxhash.h instead.

No backport is needed.

2 years agoDOC: configuration.txt: fix typo in table_idle signature
Aurelien DARRAGON [Wed, 23 Nov 2022 13:35:06 +0000 (14:35 +0100)] 
DOC: configuration.txt: fix typo in table_idle signature

An extra ',' was mistakenly added in table_idle converter signature
with commit ed36968 ("DOC: configuration.txt: add default_value for
table_idle signature").

2 years agoMINOR: mux-h1: Avoid useless call to h1_send() if no error is sent
Christopher Faulet [Wed, 23 Nov 2022 16:13:12 +0000 (17:13 +0100)] 
MINOR: mux-h1: Avoid useless call to h1_send() if no error is sent

If we choose to not send an error to the client, there is no reason to call
h1_send() for nothing. This happens because functions handling errors return
1 instead of 0 when nothing is performed.

2 years agoMINOR: mux-h1: Remove H1C_F_WAIT_NEXT_REQ in functions handling errors
Christopher Faulet [Wed, 23 Nov 2022 16:07:48 +0000 (17:07 +0100)] 
MINOR: mux-h1: Remove H1C_F_WAIT_NEXT_REQ in functions handling errors

If is cleaner to remove this flag in the internal functions handling errors,
responsible to update the H1 connection state, instead to do so in calling
functions. This will hopefully avoid bugs in future.

2 years agoBUG/MINOR: mux-h1: Fix handling of 408-Request-Time-Out
Christopher Faulet [Wed, 23 Nov 2022 15:58:22 +0000 (16:58 +0100)] 
BUG/MINOR: mux-h1: Fix handling of 408-Request-Time-Out

When a timeout is detected waiting for the request, a 408-Request-Time-Out
response is sent. However, an error was introduced by commit 6858d76cd3
("BUG/MINOR: mux-h1: Obey dontlognull option for empty requests"). Instead
of inhibiting the log message, the option was stopping the error sending.

Of course, we must do the opposite.

This patch must be backported as far as 2.4.

2 years agoBUG/MEDIUM: mux-h1: Remove H1C_F_WAIT_NEXT_REQ flag on a next request
Christopher Faulet [Wed, 23 Nov 2022 14:58:59 +0000 (15:58 +0100)] 
BUG/MEDIUM: mux-h1: Remove H1C_F_WAIT_NEXT_REQ flag on a next request

When an idle H1 connection starts to process an new request, we must take
care to remove H1C_F_WAIT_NEXT_REQ flag. This flag is used to know an idle
H1 connection has already processed at least one request and is waiting for
a next one, but nothing was received yet.

Keeping this flag leads to a crash because some running H1 connections may
be erroneously released on a soft-stop. Indeed, only idle or closed
connections must be released.

This bug was reported into #1903. It is 2.7-specific. No backport needed.

2 years agoBUILD: ssl-sock: Silent error about NULL deref in ssl_sock_bind_verifycbk()
Christopher Faulet [Wed, 23 Nov 2022 08:27:13 +0000 (09:27 +0100)] 
BUILD: ssl-sock: Silent error about NULL deref in ssl_sock_bind_verifycbk()

In ssl_sock_bind_verifycbk(), when compiled without QUIC support, the
compiler may report an error during compilation about a possible NULL
dereference:

src/ssl_sock.c: In function ‘ssl_sock_bind_verifycbk’:
src/ssl_sock.c:1738:12: error: potential null pointer dereference [-Werror=null-dereference]
 1738 |         ctx->xprt_st |= SSL_SOCK_ST_FL_VERIFY_DONE;
      |         ~~~^~~~~~~~~

A BUG_ON() was addeded because it must never happen. But when compiled
without DEBUG_STRICT, there is nothing to help the compiler. Thus
ALREADY_CHECKED() macro is used. The ssl-sock context and the bind config
are concerned.

This patch must be backported to 2.6.

2 years agoDOC: configuration.txt: add default_value for table_idle signature
Aurelien DARRAGON [Tue, 22 Nov 2022 17:55:35 +0000 (18:55 +0100)] 
DOC: configuration.txt: add default_value for table_idle signature

table_idle converter takes optional default_value argument.
The documentation correctly describes this usage but <default_value> was
missing in the converter signature.

It should be backported with bbeec37b3 ("MINOR: stick-table:
Add table_expire() and table_idle() new converters")

2 years agoBUILD: http-htx: Silent build error about a possible NULL start-line
Christopher Faulet [Tue, 22 Nov 2022 17:02:00 +0000 (18:02 +0100)] 
BUILD: http-htx: Silent build error about a possible NULL start-line

In http_replace_req_uri(), if the URI was successfully replaced, it means
the start-line exists. However, the compiler reports an error about a
possible NULL pointer dereference:

src/http_htx.c: In function ‘http_replace_req_uri’:
src/http_htx.c:392:19: error: potential null pointer dereference [-Werror=null-dereference]
  392 |         sl->flags &= ~HTX_SL_F_NORMALIZED_URI;

So, ALREADY_CHECKED() macro is used to silent the build error. This patch
must be backported with 84cdbe478a ("BUG/MINOR: http-htx: Don't consider an
URI as normalized after a set-uri action").

2 years agoBUG/MEDIUM: mux-h1: Subscribe for reads on error on sending path
Christopher Faulet [Tue, 22 Nov 2022 16:16:22 +0000 (17:16 +0100)] 
BUG/MEDIUM: mux-h1: Subscribe for reads on error on sending path

The recent refactoring about errors handling in the H1 multiplexer
introduced a bug on abort when the client wait for the server response. The
bug only exists if abortonclose option is not enabled. Indeed, in this case,
when the end of the message is reached, the mux stops to receive data
because these data are part of the next request. However, error on the
sending path are no longer fatal. An error on the reading path must be
caught to do so. So, in case of a client abort, the error is not reported to
the upper layer and the H1 connection cannot be closed if some pending data
are blocked (remember, an error on sending path was detected, blocking
outgoing data).

To be sure to have a chance to detect the abort in the case, when an error
is detected on the sending path, we force the subscription for reads.

This patch, with the previous one, should fix the issue #1943. It is
2.7-specific, no backport is needed.

2 years agoBUG/MEDIUM: mux-h1: Don't release H1C on timeout if there is a SC attached
Christopher Faulet [Tue, 22 Nov 2022 16:06:13 +0000 (17:06 +0100)] 
BUG/MEDIUM: mux-h1: Don't release H1C on timeout if there is a SC attached

When the H1 task timed out, we must be careful to not release the H1
conneciton if there is still a H1 stream with a stream-connector
attached. In this case, we must wait. There are some tests to prevent it
happens. But the last one only tests the UPGRADING state while there is also
the CLOSING state with a SC attached. But, in the end, it is safer to test
if there is a H1 stream with a SC attached.

This patch should partially fix the issue #1943. However, it only prevent
the segfault. There is another bug under the hood. BTW, this one is
2.7-specific. Not backport is needed.

2 years agoBUG/MINOR: http-htx: Don't consider an URI as normalized after a set-uri action
Christopher Faulet [Tue, 22 Nov 2022 14:41:48 +0000 (15:41 +0100)] 
BUG/MINOR: http-htx: Don't consider an URI as normalized after a set-uri action

An abosulte URI is marked as normalized if it comes from an H2 client. This
way, we know we can send a relative URI to an H1 server. But, after a
set-uri action, the URI must no longer be considered as normalized.
Otherwise there is no way to send an absolute URI on the server side.

If it is important to update a normalized absolute URI without altering this
property, the host, path and/or query-string must be set separatly.

This patch should fix the issue #1938. It should be backported as far as
2.4.

2 years agoREG-TESTS: http: Add more tests about authority/host matching
Christopher Faulet [Tue, 22 Nov 2022 14:39:12 +0000 (15:39 +0100)] 
REG-TESTS: http: Add more tests about authority/host matching

More tests were added to h1_host_normalization.vtc to be sure we are RF3986
compliant. Among other things, some tests with empty ports were added.

2 years agoBUG/MINOR: h1: Replace authority validation to conform RFC3986
Christopher Faulet [Tue, 22 Nov 2022 09:04:16 +0000 (10:04 +0100)] 
BUG/MINOR: h1: Replace authority validation to conform RFC3986

Except for CONNECT method, where a normalization is performed, we expected
to have an exact match between the authority and the host header value.
However it was too strict. Indeed, default port must be handled and the
matching must respect the RFC3986.

There is already a scheme based normalizeation performed on the URI later,
on the HTX message. And we cannot normalize the URI during H1 parsing to be
able to report proper errors on the original raw buffer. And a systematic
read-only normalization to validate the authority will consume CPU for only
few requests. At the end, we decided to perform extra-checks when the exact
match fails. Now, following authority/host are considered as equivalent:

  http:  domain.com <=> domain.com:80  <=> domain.com:
  https: domain.com <=> domain.com:443 <=> domain.com:

This patch depends on:

  * MINOR: h1: Consider empty port as invalid in authority for CONNECT
  * MINOR: http: Considere empty ports as valid default ports

It is a bug regarding the RFC3986. Technically, I may be backported as far
as 2.4. However, this must be discussed first. If it is backported, the
commits above must be backported too.

2 years agoBUG/MINOR: http-htx: Normalized absolute URIs with an empty port
Christopher Faulet [Mon, 21 Nov 2022 18:20:20 +0000 (19:20 +0100)] 
BUG/MINOR: http-htx: Normalized absolute URIs with an empty port

Thanks to the previous commit ("MINOR: http: Considere empty ports as valid
default ports"), empty ports are now considered as valid default ports. Thus,
absolute URIs with empty port should be normalized.

So now, the following URIs are normalized:

 http://example.com:/  --> http://example.com/
 https://example.com:/ --> https://example.com/

This patch depend on:

   * MINOR: h1: Consider empty port as invalid in authority for CONNECT
   * MINOR: http: Considere empty ports as valid default ports

It is a bug regarding the RFC3986. Technically, I may be backported as far
as 2.4. However, this must be discussed first. If backported, the commits
above must be backported too.

2 years agoMINOR: http: Considere empty ports as valid default ports
Christopher Faulet [Mon, 21 Nov 2022 17:57:49 +0000 (18:57 +0100)] 
MINOR: http: Considere empty ports as valid default ports

In RFC3986#6.2.3, following URIs are considered as equivalent:

      http://example.com
      http://example.com/
      http://example.com:/
      http://example.com:80/

The third one is interristing because the port is empty and it is still
considered as a default port. Thus, http_get_host_port() does no longer
return IST_NULL when the port is empty. Now, a ist is returned, it points on
the first character after the colon (':') with a length of 0. In addition,
http_is_default_port() now considers an empty port as a default port,
regardless the scheme.

This patch must not be backported, if so, without the previous one ("MINOR:
h1: Consider empty port as invalid in authority for CONNECT").

2 years agoMINOR: h1: Consider empty port as invalid in authority for CONNECT
Christopher Faulet [Tue, 22 Nov 2022 09:27:54 +0000 (10:27 +0100)] 
MINOR: h1: Consider empty port as invalid in authority for CONNECT

For now, this change is useless because http_get_host_port() returns
IST_NULL when the port is empty. But this will change. For other methods,
empty ports are valid. But not for CONNECT method. To still return a
400-Bad-Request if a CONNECT is performed with an empty port, istlen() is
used to test the port, instead of isttest().

2 years agoCLEANUP: tools: extra check in utoa_pad
Aurelien DARRAGON [Tue, 22 Nov 2022 10:42:07 +0000 (11:42 +0100)] 
CLEANUP: tools: extra check in utoa_pad

Removing useless check in utoa_pad().

This was reported by Ilya with the help of cppcheck.

2 years agoCLEANUP: arg: remove extra check in make_arg_list arg escaping
Aurelien DARRAGON [Tue, 22 Nov 2022 10:48:12 +0000 (11:48 +0100)] 
CLEANUP: arg: remove extra check in make_arg_list arg escaping

Len cannot be equal to 1 when entering in escape handling code.
But yet, an extra "len == 1" check was performed.

Removing this useless check.

This was reported by Ilya with the help of cppcheck.

2 years agoBUG/MINOR: log: fix parse_log_message rfc5424 size check
Aurelien DARRAGON [Tue, 22 Nov 2022 10:17:11 +0000 (11:17 +0100)] 
BUG/MINOR: log: fix parse_log_message rfc5424 size check

In parse_log_message(), if log is rfc5424 compliant, p pointer
is incremented and size is not. However size is still used in further
checks as if p pointer was not incremented.

This could lead to logic error or buffer overflow if input buf is not
null-terminated.

Fixing this by making sure size is up to date where it is needed.

It could be backported up to 2.4.

2 years agoBUG/MINOR: cfgparse-listen: fix ebpt_next_dup pointer dereference on proxy "from...
Aurelien DARRAGON [Mon, 21 Nov 2022 16:01:11 +0000 (17:01 +0100)] 
BUG/MINOR: cfgparse-listen: fix ebpt_next_dup pointer dereference on proxy "from" inheritance

ebpt_next_dup() was used 2 times in a row but only the first call was
checked against NULL, probably assuming that the 2 calls always yield the
same result here.

gcc is not OK with that, and it should be safer to store the result of
the first call in a temporary var to dereference it once checked against NULL.

This should fix GH #1869.
Thanks to Ilya for reporting this issue.

It may be backported up to 2.4.

2 years agoDOC: quic: add note on performance issue with listener contention
Amaury Denoyelle [Tue, 22 Nov 2022 10:26:16 +0000 (11:26 +0100)] 
DOC: quic: add note on performance issue with listener contention

Complete quic4/quic6 bind lines by a note on performance issues due to
receiver socket contention. Suggest to use sharding to improve the
situation.

This should be backported up to 2.6.

2 years agoBUILD: sched: fix build with DEBUG_THREAD with the previous commit
Willy Tarreau [Tue, 22 Nov 2022 09:24:07 +0000 (10:24 +0100)] 
BUILD: sched: fix build with DEBUG_THREAD with the previous commit

The build with DEBUG_THREAD was broken by commit fc50b9dd1 ("BUG/MAJOR:
sched: protect task during removal from wait queue"). It took me a while
to figure how to declare and aligned and initialized rwlock that wasn't
static, but it turns out that __decl_aligned_rwlock() does exactly this,
so that we don't have to assign an integer value when a struct is expected
in case of debugging.

No backport is needed.

2 years agoBUG/MAJOR: sched: protect task during removal from wait queue
Willy Tarreau [Tue, 22 Nov 2022 06:05:44 +0000 (07:05 +0100)] 
BUG/MAJOR: sched: protect task during removal from wait queue

The issue addressed by commit fbb934da9 ("BUG/MEDIUM: stick-table: fix
a race condition when updating the expiration task") is still present
when thread groups are enabled, but this time it lies in the scheduler.

What happens is that a task configured to run anywhere might already
have been queued into one group's wait queue. When updating a stick
table entry, sometimes the task will have to be dequeued and requeued.

For this a lock is taken on the current thread group's wait queue lock,
but while this is necessary for the queuing, it's not sufficient for
dequeuing since another thread might be in the process of expiring this
task under its own group's lock which is different. This is easy to test
using 3 stick tables with 1ms expiration, 3 track-sc rules and 4 thread
groups. The process crashes almost instantly under heavy traffic.

One approach could consist in storing the group number the task was
queued under in its descriptor (we don't need 32 bits to store the
thread id, it's possible to use one short for the tid and another
one for the tgrp). Sadly, no safe way to do this was figured, because
the race remains at the moment the thread group number is checked, as
it might be in the process of being changed by another thread. It seems
that a working approach could consist in always having it associated
with one group, and only allowing to change it under this group's lock,
so that any code trying to change it would have to iterately read it
and lock its group until the value matches, confirming it really holds
the correct lock. But this seems a bit complicated, particularly with
wait_expired_tasks() which already uses upgradable locks to switch from
read state to a write state.

Given that the shared tasks are not that common (stick-table expirations,
rate-limited listeners, maybe resolvers), it doesn't seem worth the extra
complexity for now. This patch takes a simpler and safer approach
consisting in switching back to a single wq_lock, but still keeping
separate wait queues. Given that shared wait queues are almost always
empty and that otherwise they're scanned under a read lock, the
contention remains manageable and most of the time the lock doesn't
even need to be taken since such tasks are not present in a group's
queue. In essence, this patch reverts half of the aforementionned
patch. This was tested and confirmed to work fine, without observing
any performance degradation under any workload. The performance with
8 groups on an EPYC 74F3 and 3 tables remains twice the one of a
single group, with the contention remaining on the table's lock first.

No backport is needed.

2 years agoBUILD: listener: fix build warning on global_listener_rwlock without threads
Willy Tarreau [Tue, 22 Nov 2022 08:08:23 +0000 (09:08 +0100)] 
BUILD: listener: fix build warning on global_listener_rwlock without threads

The global_listener_rwlock was introduced by recent commit 13e86d947
("BUG/MEDIUM: listener: Fix race condition when updating the global mngmt
task"), but it's declared static and is not used when threads are disabled,
thus causing a warning to be emitted in this case. Let's just condition it
to thread usage to shut the warning.

This will need to be backported where the patch above is backported.

2 years agoMINOR: server/idle: make the next_takeover index per-tgroup
Willy Tarreau [Mon, 21 Nov 2022 13:14:06 +0000 (14:14 +0100)] 
MINOR: server/idle: make the next_takeover index per-tgroup

In order to evenly pick idle connections from other threads, there is
a "next_takeover" index in the server, that is incremented each time
a connection is picked from another thread, and indicates which one to
start from next time.

With thread groups this doesn't work well because the index is the same
regardless of the group, and if a group has more threads than another,
there's even a risk to reintroduce an imbalance.

This patch introduces a new per-tgroup storage in servers which, for now,
only contains an instance of this next_takeover index. This way each
thread will now only manipulate the index specific to its own group, and
the takeover will become fair again. More entries may come soon.

2 years agoBUG/MINOR: server/idle: at least use atomic stores when updating max_used_conns
Willy Tarreau [Mon, 21 Nov 2022 13:32:33 +0000 (14:32 +0100)] 
BUG/MINOR: server/idle: at least use atomic stores when updating max_used_conns

In 2.2, some idle conns usage metrics were added by commit cf612a045
("MINOR: servers: Add a counter for the number of currently used
connections."), which mentioned that the operation doesn't need to be
atomic since we're not seeking exact values. This is true but at least
we should use atomic stores to make sure not to cause invalid values
to appear on archs that wouldn't guarantee atomicity when writing an
int, such as writing two 16-bit words. This is pretty unlikely on our
targets but better keep the code safe against this.

This may be backported as far as 2.2.

2 years agoBUG/MINOR: resolvers: do not run the timeout task when there's no resolution
Willy Tarreau [Mon, 21 Nov 2022 18:15:22 +0000 (19:15 +0100)] 
BUG/MINOR: resolvers: do not run the timeout task when there's no resolution

The function resolv_update_resolvers_timeout() always schedules a wakeup
of the process_resolvers() task based on the "timeout resolve" setting,
regardless of the presence of an ongoing resolution or not. This is causing
one wakeup every second by default even when there's no resolvers section
(due to the default one), and can even be worse: creating a section with
"timeout resolve 1" bombs the process with 1000 wakeups per second.

Let's condition the setting to the presence of a resolution to address
this.

This issue has been there forever, but it doesn't cause that much trouble,
and given how fragile and tricky this code is, it's probably wise to
refrain from backporting it until it's reported to really cause trouble.

2 years agoMINOR: global: generate random cluster.secret if not defined
Amaury Denoyelle [Mon, 14 Nov 2022 15:18:46 +0000 (16:18 +0100)] 
MINOR: global: generate random cluster.secret if not defined

If no cluster-secret is defined by the user, a random one is silently
generated.

This ensures that at least QUIC Retry tokens are generated if abnormal
conditions are detected. However, it is advisable to specify it in the
configuration for tokens to be valid even after a reload or across LBs
instances in the same cluster.

This should be backported up to 2.6.

2 years agoMINOR: quic: report error if force-retry without cluster-secret
Amaury Denoyelle [Mon, 14 Nov 2022 15:17:13 +0000 (16:17 +0100)] 
MINOR: quic: report error if force-retry without cluster-secret

QUIC Retry generation relies on global cluster-secret to produce token
valid even after a process restart and across several LBs instances.

Before this patch, Retry is automatically deactivated if no
cluster-secret is provided. This is the case even if a user has
configured a QUIC listener with quic-force-retry. Change this behavior
by now returning an error during configuration parsing. The user must
provide a cluster-secret if quic-force-retry is used.

This shoud be backported up to 2.6.

2 years agoDOC: configuration: fix quic prefix typo
Amaury Denoyelle [Mon, 14 Nov 2022 16:14:41 +0000 (17:14 +0100)] 
DOC: configuration: fix quic prefix typo

Replace quicv4/quicv6 -> quic4/quic6 as prefix for bind lines of QUIC
listeners.

This should be backported up to 2.6.

2 years agoMINOR: cli/pools: add pool name filtering capability to "show pools"
Willy Tarreau [Mon, 21 Nov 2022 09:02:29 +0000 (10:02 +0100)] 
MINOR: cli/pools: add pool name filtering capability to "show pools"

Now it becomes possible to match a pool name's prefix, for example:

  $ socat - /tmp/haproxy.sock <<< "show pools match quic byusage"
  Dumping pools usage. Use SIGQUIT to flush them.
    - Pool quic_conn_r (65560 bytes) : 1337 allocated (87653720 bytes), ...
    - Pool quic_crypto (1048 bytes) : 6685 allocated (7005880 bytes), ...
    - Pool quic_conn (4056 bytes) : 1337 allocated (5422872 bytes), ...
    - Pool quic_rxbuf (262168 bytes) : 8 allocated (2097344 bytes), ...
    - Pool quic_connne (184 bytes) : 9359 allocated (1722056 bytes), ...
    - Pool quic_frame (184 bytes) : 7938 allocated (1460592 bytes), ...
    - Pool quic_tx_pac (152 bytes) : 6454 allocated (981008 bytes), ...
    - Pool quic_tls_ke (56 bytes) : 12033 allocated (673848 bytes), ...
    - Pool quic_rx_pac (408 bytes) : 1596 allocated (651168 bytes), ...
    - Pool quic_tls_se (88 bytes) : 6685 allocated (588280 bytes), ...
    - Pool quic_cstrea (88 bytes) : 4011 allocated (352968 bytes), ...
    - Pool quic_tls_iv (24 bytes) : 12033 allocated (288792 bytes), ...
    - Pool quic_dgram (344 bytes) : 732 allocated (251808 bytes), ...
    - Pool quic_arng (56 bytes) : 4011 allocated (224616 bytes), ...
    - Pool quic_conn_c (152 bytes) : 1337 allocated (203224 bytes), ...
  Total: 15 pools, 109578176 bytes allocated, 109578176 used ...

In this case the reported total only concerns the dumped ones.

2 years agoMINOR: cli/pools: add sorting capabilities to "show pools"
Willy Tarreau [Mon, 21 Nov 2022 08:34:02 +0000 (09:34 +0100)] 
MINOR: cli/pools: add sorting capabilities to "show pools"

The "show pools" command is used a lot for debugging but didn't get much
love over the years. This patch brings new capabilities:
  - sorting the output by pool names to ese their finding ("byname").
  - sorting the output by reverse item size to spot the biggest ones("bysize")
  - sorting the output by reverse number of allocated bytes ("byusage")

The last one (byusage) also omits displaying the ones with zero allocation.

In addition, an optional max number of output entries may be passed so as
to dump only the N most relevant ones.

2 years agoMINOR: cli/pools: store "show pools" results into a temporary array
Willy Tarreau [Mon, 21 Nov 2022 08:02:41 +0000 (09:02 +0100)] 
MINOR: cli/pools: store "show pools" results into a temporary array

This will permit sorting and filtering that are currently not possible.

2 years agoCLEANUP: quic: replace "choosen" with "chosen" all over the code
Ilya Shipitsin [Tue, 1 Nov 2022 10:46:39 +0000 (15:46 +0500)] 
CLEANUP: quic: replace "choosen" with "chosen" all over the code

Some variables were set as "choosen" instead of "chosen", this is dedicated
spelling fix

2 years agoBUG/MAJOR: quic: Crash after discarding packet number spaces
Frédéric Lécaille [Sun, 20 Nov 2022 17:35:35 +0000 (18:35 +0100)] 
BUG/MAJOR: quic: Crash after discarding packet number spaces

This previous patch was not sufficient to prevent haproxy from
crashing when some Handshake packets had to be inspected before being possibly
retransmitted:

     "BUG/MAJOR: quic: Crash upon retransmission of dgrams with several packets"

This patch introduced another issue: access to packets which have been
released because still attached to others (in the same datagram). This was
the case for instance when discarding the Initial packet number space before
inspecting an Handshake packet in the same datagram through its ->prev or
member in our case.

This patch implements quic_tx_packet_dgram_detach() which detaches a packet
from the adjacent ones in the same datagram to be called when ackwowledging
a packet (as done in the previous commit) and when releasing its memory. This
was, we are sure the released packets will not be accessed during retransmissions.

Thank you to @gabrieltz for having reported this issue in GH #1903.

Must be backported to 2.6.