]> git.ipfire.org Git - thirdparty/haproxy.git/log
thirdparty/haproxy.git
2 years agoBUG/MEDIUM: peers: limit reconnect attempts of the old process on reload
Christopher Faulet [Tue, 26 Jul 2022 17:19:18 +0000 (19:19 +0200)] 
BUG/MEDIUM: peers: limit reconnect attempts of the old process on reload

When peers are configured and HAProxy is reloaded or restarted, a
synchronization is performed between the old process and the new one. To do
so, the old process connects on the new one. If the synchronization fails,
it retries. However, there is no delay and reconnect attempts are not
bounded. Thus, it may loop for a while, consuming all the CPU. Of course, it
is unexpected, but it is possible. For instance, if the local peer is
misconfigured, an infinite loop can be observed if the connection succeeds
but not the synchronization. This prevents the old process to exit, except
if "hard-stop-after" option is set.

To fix the bug, the reconnect is delayed. The local peer already has a
expiration date to delay the reconnects. But it was not used on stopping
mode. So we use it not. Thanks to the previous fix, the reconnect timeout is
shorter in this case (500ms against 5s on running mode). In addition, we
also use the peers resync expiration date to not infinitely retries. It is
accurate because the new process, on its side, use this timeout to switch
from a local resync to a remote resync.

This patch depends on "MINOR: peers: Use a dedicated reconnect timeout when
stopping the local peer". It fixes the issue #1799. It should be backported
as far as 2.0.

2 years agoMINOR: peers: Use a dedicated reconnect timeout when stopping the local peer
Christopher Faulet [Tue, 26 Jul 2022 17:14:36 +0000 (19:14 +0200)] 
MINOR: peers: Use a dedicated reconnect timeout when stopping the local peer

When a process is stopped or reload, a dedicated reconnect timeout is now
used. For now, this timeout is not used because the current code retries
immediately to reconnect to perform the local synchronization with the new
local peer, if any.

This patch is required to fix the issue #1799. It should be backported as
far as 2.0 with next fixes.

2 years agoMINOR: peers: Add a warning about incompatible SSL config for the local peer
Christopher Faulet [Tue, 26 Jul 2022 17:03:51 +0000 (19:03 +0200)] 
MINOR: peers: Add a warning about incompatible SSL config for the local peer

In peers section, it is possible to enable SSL for the local peer. In this
case, the bind line and the server line should both be configured. A
"default-server" directive may also be used to configure the SSL on the
server side. However there is no test to be sure the SSL is enabled on both
sides. It is an problem because the local resync performed during a reload
will be impossible and it is probably not the expected behavior.

So, it is now checked during the configuration validation. A warning message
is displayed if the SSL is not properly configured for the local peer.

This patch is related to issue #1799. It should probably be backported to 2.6.

2 years agoMEDIUM: mux-quic: implement http-keep-alive timeout
Amaury Denoyelle [Mon, 25 Jul 2022 09:53:18 +0000 (11:53 +0200)] 
MEDIUM: mux-quic: implement http-keep-alive timeout

Complete QUIC MUX timeout refresh function by using http-keep-alive
timeout. It is used when the connection is idle after having handle at
least one request.

To implement this a new member <idle_start> has been defined in qcc
structure. This is used as timestamp for when the connection became idle
and is used as base time for http keep-alive timeout

2 years agoMINOR: mux-quic: count in-progress requests
Amaury Denoyelle [Mon, 25 Jul 2022 09:21:46 +0000 (11:21 +0200)] 
MINOR: mux-quic: count in-progress requests

Add a new qcc member named <nb_hreq>. Its purpose is close to <nb_sc>
which represents the number of attached stream connectors. Both are
incremented inside qc_attach_sc().

The difference is on the decrement operation. While <nb_cs> is
decremented on sedesc detach callback, <nb_hreq> is decremented when the
qcs is locally closed.

In most cases, <nb_hreq> will be decremented before <nb_cs>. However, it
will be the reverse if a stream must be kept alive after detach callback.

The main purpose of this field is to implement http-keep-alive timeout.
Both <nb_sc> and <nb_hreq> must be null to activate the http-keep-alive
timeout.

2 years agoMEDIUM: mux-quic: adjust timeout refresh
Amaury Denoyelle [Mon, 25 Jul 2022 12:58:48 +0000 (14:58 +0200)] 
MEDIUM: mux-quic: adjust timeout refresh

Implement a new internal function qcc_refresh_timeout(). Its role will be
to reset QUIC MUX timeout depending if there is requests in progress or
not.

qcc_update_timeout() does not set a timeout if there is still attached
streams as in this case the upper layer is responsible to manage it.
Else it will activate the timeout depending on the connection current
status.

Timeout is refreshed on several locations : on stream detach and in I/O
handler and wake callback.

For the moment, only the default timeout is used (client or server). The
function may be expanded in the future to support more specific ones :
* http-keep-alive if connection is idle
* http-request when waiting for incomplete HTTP requests
* client/server-fin for graceful shutdown

2 years agoMINOR: mux-quic: use timeout server for backend conns
Amaury Denoyelle [Mon, 25 Jul 2022 12:51:56 +0000 (14:51 +0200)] 
MINOR: mux-quic: use timeout server for backend conns

Use timeout server in qcc_init() as default timeout for backend
connections. No impact for the moment as QUIC backend support is not
implemented.

2 years agoMINOR: mux-quic: save proxy instance into qcc
Amaury Denoyelle [Fri, 22 Jul 2022 14:16:03 +0000 (16:16 +0200)] 
MINOR: mux-quic: save proxy instance into qcc

Store a reference to proxy in the qcc structure. This will be useful to
access to proxy members outside of qcc_init().

Most notably, this change is required to implement timeout refreshing by
using the various timeouts configured at the proxy level.

2 years agoBUG/MINOR: mux-quic: do not free conn if attached streams
Amaury Denoyelle [Wed, 27 Jul 2022 09:39:01 +0000 (11:39 +0200)] 
BUG/MINOR: mux-quic: do not free conn if attached streams

Ensure via qcc_is_dead() that a connection is not released instance
until all of qcs streams are detached by the upper layer, even if an
error has been reported or the timeout has fired.

On the other side, as qc_detach() always check the connection status,
this should ensure that we do not keep a connection if not necessary.

Without this patch, a qcc instance may be freed with some of its qcs
streams not detached. This is an incorrect behavior and will lead to a
BUG_ON fault. Note however that no occurence of this bug has been
produced currently. This patch is mainly a safety against future
occurences.

This should be backported up to 2.6.

2 years agoCLEANUP: mux-quic: remove useless app_ops is_active callback
Amaury Denoyelle [Mon, 1 Aug 2022 09:42:48 +0000 (11:42 +0200)] 
CLEANUP: mux-quic: remove useless app_ops is_active callback

Timeout in QUIC MUX has evolved from the simple first implementation. At
the beginning, a connection was considered dead unless bidirectional
streams were opened. This was abstracted through an app callback
is_active().

Now this paradigm has been reversed and a connection is considered alive
by default, unless an error has been reported or a timeout has already
been fired. The callback is_active() is thus not used anymore and can be
safely removed to simplify qcc_is_dead().

This commit should be backported to 2.6.

2 years agoBUG/MINOR: mux-quic: prevent crash if conn released during IO callback
Amaury Denoyelle [Mon, 25 Jul 2022 12:56:54 +0000 (14:56 +0200)] 
BUG/MINOR: mux-quic: prevent crash if conn released during IO callback

A qcc instance may be freed in the middle of qc_io_cb() if all streams
were purged. This will lead to a crash as qcc instance is reused after
this step. Jump directly to the end of the function to avoid this.

Note that this bug has not been triggered for the moment. This is a
safety fix to prevent it.

This must be backported up to 2.6.

2 years agoBUG/MEDIUM: pattern: only visit equivalent nodes when skipping versions
Willy Tarreau [Mon, 1 Aug 2022 09:46:27 +0000 (11:46 +0200)] 
BUG/MEDIUM: pattern: only visit equivalent nodes when skipping versions

Miroslav reported in issue #1802 a problem that affects atomic map/acl
updates. During an update, incorrect versions are properly skipped, but
in order to do so, we rely on ebmb_next() instead of ebmb_next_dup().
This means that if a new matching entry is in the process of being
added and is the first one to succeed in the lookup, we'll skip it due
to its version and use the next entry regardless of its value provided
that it has the correct version. For IP addresses and string prefixes
it's particularly visible because a lookup may match a new longer prefix
that's not yet committed (e.g. 11.0.0.1 would match 11/8 when 10/7 was
the only committed one), and skipping it could end up on 12/8 for
example. As soon as a commit for the last version happens, the issue
disappears.

This problem only affects tree-based matches: the "str", "ip", and "beg"
matches.

Here we replace the ebmb_next() values with ebmb_next_dup() for exact
string matches, and with ebmb_lookup_shorter() for longest matches,
which will first visit duplicates, then look for shorter prefixes. This
relies on previous commit:

    MINOR: ebtree: add ebmb_lookup_shorter() to pursue lookups

Both need to be backported to 2.4, where the generation ID was added.

Note that nowadays a simpler and more efficient approach might be employed,
by having a single version in the current tree, and a list of trees per
version. Manipulations would look up the tree version and work (and lock)
only in the relevant trees, while normal operations would be performed on
the current tree only. Committing would just be a matter of swapping tree
roots and deleting old trees contents.

2 years agoMINOR: ebtree: add ebmb_lookup_shorter() to pursue lookups
Willy Tarreau [Mon, 1 Aug 2022 08:37:29 +0000 (10:37 +0200)] 
MINOR: ebtree: add ebmb_lookup_shorter() to pursue lookups

This function is designed to enlarge the scope of a lookup performed
by a caller via ebmb_lookup_longest() that was not satisfied with the
result. It will first visit next duplicates, and if none are found,
it will go up in the tree to visit similar keys with shorter prefixes
and will return them if they match. We only use the starting point's
value to perform the comparison since it was expected to be valid for
the looked up key, hence it has all bits in common with its own length.

The algorithm is a bit complex because when going up we may visit nodes
that are located beneath the level we just come from. However it is
guaranteed that keys having a shorter prefix will be present above the
current location, though they may be attached to the left branch of a
cover node, so we just visit all nodes as long as their prefix is too
large, possibly go down along the left branch on cover nodes, and stop
when either there's a match, or there's a non-matching prefix anymore.

The following tricky case now works fine and properly finds 10.0.0.0/7
when looking up 11.0.0.1 from tree version 1 though both belong to
different sub-trees:

  prepare map #1
    add map @1 #1 10.0.0.0/7 10.0.0.0/7
    add map @1 #1 10.0.0.0/7 10.0.0.0/7
  commit map @1 #1
  prepare map #1
    add map @2 #1 11.0.0.0/8 11.0.0.0/8
    add map @2 #1 11.0.0.0/8 11.0.0.0/8

  prepare map #1
    add map @1 #1 10.0.0.0/7 10.0.0.0/7
  commit map @1 #1
  prepare map #1
    add map @2 #1 10.0.0.0/7 10.0.0.0/7
    add map @2 #1 11.0.0.0/8 11.0.0.0/8
    add map @2 #1 11.0.0.0/8 11.0.0.0/8

2 years agoDEBUG: tools: provide a tree dump function for ebmbtrees as well
Willy Tarreau [Mon, 1 Aug 2022 09:55:57 +0000 (11:55 +0200)] 
DEBUG: tools: provide a tree dump function for ebmbtrees as well

It's convenient for debugging IP trees. However we're not dumping the
full keys, for the sake of simplicity, only the 4 first bytes are dumped
as a u32 hex value. In practice this is sufficient for debugging. As a
reminder since it seems difficult to recover the command each time it's
needed, the output is converted to an image using dot from Graphviz:

    dot -o a.png -Tpng dump.txt

2 years agoMINOR: thread: provide an alternative to pthread's rwlock
Willy Tarreau [Sun, 10 Jul 2022 08:58:57 +0000 (10:58 +0200)] 
MINOR: thread: provide an alternative to pthread's rwlock

Since version 1.1.0, OpenSSL's libcrypto ignores the provided locking
mechanism and uses pthread's rwlocks instead. The problem is that for
some code paths (e.g. async engines) this results in a huge amount of
syscalls on systems facing a bit of contention, to the point where more
than 80% of the CPU can be spent in the system dealing with spinlocks
just for futex_wake().

This patch provides an alternative by redefining the relevant pthread
rwlocks from the low-overhead version of the progressive rw locks. This
way there will be no more syscalls in case of contention, and CPU will
be burnt in userland. Doing this saves massive amounts of CPU, where
the locks only take 12-15% vs 80% before, which allows SSL to work much
faster on large thread counts (e.g. 24 or more).

The tryrdlock and trywrlock variants have been implemented using a CAS
since their goal is only to succeed on no contention and never to wait.
The pthread_rwlock API is complete except that the timed versions of
the rdlock and wrlock do not wait and simply fall back to trylock
versions.

Since the gains have only been observed with async engines for now,
this option remains disabled by default. It can be enabled at build
time using USE_PTHREAD_EMULATION=1.

2 years agoMAJOR: threads/plock: update the embedded library
Willy Tarreau [Fri, 29 Jul 2022 15:53:31 +0000 (17:53 +0200)] 
MAJOR: threads/plock: update the embedded library

The plock code hasn't been been updated since 2017 and didn't benefit
from the exponential back-off improvements that were added in 2018.
Simply updating the file shows a massive performance gain on large
thread count (>=48) with dequeuing going from 113k RPS to 300k RPS and
round robin from 229k RPS to 1020k RPS. It was about time to update.
In addition, some recent improvements to the code will be useful with
thread groups.

An interesting improvement concerns EPYC CPUs. This one alone increased
fairness and was sufficient to avoid crashes in process_srv_queue() there,
when hammering two servers with maxconn 200 under 1k connections.

2 years agoBUG/MEDIUM: queue/threads: limit the number of entries dequeued at once
Willy Tarreau [Sat, 30 Jul 2022 07:53:22 +0000 (09:53 +0200)] 
BUG/MEDIUM: queue/threads: limit the number of entries dequeued at once

When testing strong queue contention on a 48-thread machine, some crashes
would frequently happen due to process_srv_queue() never leaving and
processing pending requests forever. A dump showed more than 500000
loops at once. The problem is that other threads find it working so
they don't do anything and are free to process their pending requests.
Because of this, the dequeuing thread can be kept busy forever and does
not process its own requests anymore (fortunately the watchdog stops it).

This patch adds a limit to the number of rounds, it limits it to
maxpollevents, which is reasonable because it's also an indicator of
latency and batches size. However there's a catch. If all requests
are finished when the thread ends the loop, there might not be enough
anymore to restart processing the queue. Thus we tolerate to re-enter
the loop to process one request at a time when it doesn't have any
anymore. This way we're leaving more room for another thread to take
on this task, and we're sure to eventually end this loop.

Doing this has also improved the overall dequeuing performance by ~20%
in highly contended situations with 48 threads.

It should be backported at least to 2.4, maybe even 2.2 since issues
were faced in the past on machines having many cores.

2 years agoMINOR: quic: Send packets as much as possible from qc_send_app_pkts()
Frédéric Lécaille [Tue, 26 Jul 2022 07:17:19 +0000 (09:17 +0200)] 
MINOR: quic: Send packets as much as possible from qc_send_app_pkts()

Add a loop into this function to send more packets from this function
which is called by the mux. It is broken when we could not prepare
packet with qc_prep_app_pkts() due to missing available room in the
buffer used to send packets. This improves the throughput.

Must be backported to 2.6.

2 years agoBUG/MAJOR: quic: Useless resource intensive loop qc_ackrng_pkts()
Frédéric Lécaille [Fri, 22 Jul 2022 14:27:44 +0000 (16:27 +0200)] 
BUG/MAJOR: quic: Useless resource intensive loop qc_ackrng_pkts()

This usless loop should have been removed a long time ago. As it is CPU resource
intensive, it could trigger the watchdog.

Must be backported to 2.6.

2 years agoMINOR: quic: Stop looking for packet loss asap
Frédéric Lécaille [Thu, 21 Jul 2022 12:24:41 +0000 (14:24 +0200)] 
MINOR: quic: Stop looking for packet loss asap

As the TX packets are ordered by their packet number and always sent
in the same order. their TX timestamps are inspected from the older to
the newer values when we look for the packet loss. So we can stop
this search as soon as we found the first packet which has not been lost.

Must be backported to 2.6

2 years agoBUG/MINOR: quic: loss time limit variable computed but not used
Frédéric Lécaille [Thu, 21 Jul 2022 07:55:15 +0000 (09:55 +0200)] 
BUG/MINOR: quic: loss time limit variable computed but not used

<loss_time_limit> is the loss time limit computed from <time_sent> packet
transmission timestamps in qc_packet_loss_lookup() to identify the packets which
have been lost. This latter timestamp variable was used in place of
<loss_time_limit> to distinguish such packets from others (still in fly packets).

Must be backported to 2.6

2 years agoMINOR: quic: New "quic-cc-algo" bind keyword
Frédéric Lécaille [Mon, 11 Jul 2022 08:24:21 +0000 (10:24 +0200)] 
MINOR: quic: New "quic-cc-algo" bind keyword

As it could be interesting to be able to choose the QUIC control congestion
algorithm to be used by listener, add "quic-cc-algo" new keyword to do so.
Update the documentation consequently.

Must be backported to 2.6.

2 years agoMEDIUM: quic: Cubic congestion control algorithm implementation
Frédéric Lécaille [Wed, 1 Jun 2022 13:06:58 +0000 (15:06 +0200)] 
MEDIUM: quic: Cubic congestion control algorithm implementation

Cubic is the congestion control algorithm used by default by the Linux kernel
since 2.6.15 version. This algorithm is supposed to achieve good scalability and
fairness between flows using the same network path, it should also be used by QUIC
by default. This patch implements this algorithm and select it as default algorithm
for the congestion control.

Must be backported to 2.6.

2 years agoMINOR: quic: Congestion control architecture refactoring
Frédéric Lécaille [Tue, 31 May 2022 07:40:44 +0000 (09:40 +0200)] 
MINOR: quic: Congestion control architecture refactoring

Ease the integration of new congestion control algorithm to come.
Move the congestion controller state to a private array of uint32_t
to stop using a union. We do not want to continue using such long
paths cc->algo_state.<algo>.<var> to modify the internal state variable
for each algorithm.

Must be backported to 2.6

2 years agoBUG/MEDIUM: mux-quic: fix missing EOI flag to prevent streams leaks
Amaury Denoyelle [Fri, 29 Jul 2022 13:34:12 +0000 (15:34 +0200)] 
BUG/MEDIUM: mux-quic: fix missing EOI flag to prevent streams leaks

On H3 DATA frame transfer from the client, some streams are not properly
closed by the upper layer, despite all transfer operation completed.
Data integrity is not impacted but this will prevent the stream timeout
to fire and thus keep the owner session opened.

In most cases, sessions are closed on QUIC idle timeout, but it may stay
forever if a client emits PING frames at a regular interval to maintain
it.

This bug is caused by a missing EOI stream desc flag on certain
condition in qc_rcv_buf(). To be triggered, we have to use the
optimization when conn-stream buffer is empty and can be swapped with
qcs buffer. The problem is that it will skip the function body for
default copy but also a condition to check if EOI must be set. Thus this
bug does not happens for every H3 post requets : it requires that
conn-stream buffer is empty on last qc_rcv_buf() invocation.

This was reproduced more frequently when using ngtcp2 client with one or
multiple streams :
$ ngtcp2-client -m POST -d ~/infra/html/10K 127.0.0.1 20443 \
  http://127.0.0.1:20443/post

This may fix at least partially github issue #1801.

This must be backported up to 2.6.

2 years agoMINOR: cli: warning on _getsocks when socket were closed
William Lallemand [Thu, 28 Jul 2022 13:33:41 +0000 (15:33 +0200)] 
MINOR: cli: warning on _getsocks when socket were closed

The previous attempt was reverted because it would emit a warning when
the sockets are still in the process when a reload failed, so this was
an expected 2nd try.

This warning however, will be displayed if a new process successfully
get the previous sockets AND the sendable number of sockets is 0.

This way the user will be warned if he tried to get the sockets fromt
the wrong process.

2 years agoRevert "MINOR: cli: emit a warning when _getsocks was used more than once"
William Lallemand [Wed, 27 Jul 2022 11:55:54 +0000 (13:55 +0200)] 
Revert "MINOR: cli: emit a warning when _getsocks was used more than once"

This reverts commit 519cd2021bda11231d461f5974b4e321d0b4eb29.

This was reverted because it's still useful to have access to _getsosks
when the previous reload failed.

2 years agoBUG/MINOR: mworker: PROC_O_LEAVING used but not updated
William Lallemand [Wed, 27 Jul 2022 09:57:12 +0000 (11:57 +0200)] 
BUG/MINOR: mworker: PROC_O_LEAVING used but not updated

Since commit 2be557f ("MEDIUM: mworker: seamless reload use the internal
sockpair"), we are using the PROC_O_LEAVING flag to determine which
sockpair worker will be used with -x during the next reload.

However in mworker_reexec(), the PROC_O_LEAVING flag is not updated, it
is only updated at startup in mworker_env_to_proc_list().

This could be a problem when a remaining process is still in the list,
it could be selected as the current worker, and its socket will be used
even if _getsocks doesn't work anymore on it. (bug #1803)

This patch fixes the issue by updating the PROC_O_LEAVING flag in
mworker_proc_list_to_env() just before using it in mworker_reexec()

Must be backported to 2.6.

2 years agoMINOR: cli: emit a warning when _getsocks was used more than once
William Lallemand [Wed, 27 Jul 2022 09:48:54 +0000 (11:48 +0200)] 
MINOR: cli: emit a warning when _getsocks was used more than once

The _getsocks CLI command can be used only once, after that the sockets
are not available anymore.

Emit a warning when the command was already used once.

2 years agoBUG/MINOR: fd: always remove late updates when freeing fd_updt[]
Willy Tarreau [Tue, 26 Jul 2022 17:06:17 +0000 (19:06 +0200)] 
BUG/MINOR: fd: always remove late updates when freeing fd_updt[]

Christopher found that since commit 8e2c0fa8e ("MINOR: fd: delete unused
updates on close()") we may crash in a late stop due to an fd_delete()
in the main thread performed after all threads have deleted the fd_updt[]
array. Prior to that commit that didn't happen because we didn't touch
the updates on this path, but now it may happen. We don't care about these
ones anyway since the poller is stopped, so let's just wipe them by
resetting their counter before freeing the array.

No backport is needed as this is only 2.7.

2 years agoMEDIUM: resolvers: continue startup if network is unavailable
William Lallemand [Tue, 26 Jul 2022 08:50:09 +0000 (10:50 +0200)] 
MEDIUM: resolvers: continue startup if network is unavailable

When haproxy starts with a resolver section, and there is a default one
since 2.6 which use /etc/resolv.conf, it tries to do a connect() with the UDP
socket in order to check if the routes of the system allows to reach the
server.

This check is too much restrictive as it won't prevent any runtime
failure.

Relax the check by making it a warning instead of a fatal alert.

This must be backported in 2.6.

2 years agoDEBUG: fd: split the fd check
William Lallemand [Tue, 26 Jul 2022 08:35:24 +0000 (10:35 +0200)] 
DEBUG: fd: split the fd check

Split the BUG_ON(fd < 0 || fd >= global.maxsock) so it's easier to know
if it quits because of a -1.

2 years agoRevert "BUG/MINOR: peers: set the proxy's name to the peers section name"
Christopher Faulet [Mon, 25 Jul 2022 13:10:44 +0000 (15:10 +0200)] 
Revert "BUG/MINOR: peers: set the proxy's name to the peers section name"

This reverts commit 356866accefd16458f0e3c335d1b784e24e86d2d.

It seems that an undocumented expectation of peers is based on the peers
proxy name to determine if the local peer is fully configured or not. Thus
because of the commit above, we are no longer able to detect incomplete
peers sections.

On side effect of this bug is a segfault when HAProxy is stopped/reloaded if
we try to perform a local resync on a mis-configured local peer. So waiting
for a better solution, the patch is reverted.

This patch must be backported as far as 2.5.

2 years agoMINOR: sockpair: move send_fd_uxst() error message in caller
William Lallemand [Mon, 25 Jul 2022 14:04:38 +0000 (16:04 +0200)] 
MINOR: sockpair: move send_fd_uxst() error message in caller

Move the ha_alert() in send_fd_uxst() in the callers and add the FD
numbers in the message.

2 years agoBUG/MINOR: sockpair: wrong return value for fd_send_uxst()
William Lallemand [Mon, 25 Jul 2022 13:51:30 +0000 (15:51 +0200)] 
BUG/MINOR: sockpair: wrong return value for fd_send_uxst()

The fd_send_uxst() function which is used to send a socket over the
socketpair returns 1 upon error instead of -1, which means the error
case of the sendmsg() is never catched correctly.

Must be backported as far as 1.9.

2 years agoDEBUG: fd: detect possibly invalid tgid in fd_insert()
Willy Tarreau [Mon, 25 Jul 2022 13:42:41 +0000 (15:42 +0200)] 
DEBUG: fd: detect possibly invalid tgid in fd_insert()

Since the API is still a bit young, let's make sure nobody tries to
assign and FD to a group not strictly 1..MAX_TGROUPS as that would
indicate a bug.

Note: some of these might be relaxed to BUG_ON_HOT() in the future

2 years agoBUG/MAJOR: poller: drop FD's tgid when masks don't match
Willy Tarreau [Mon, 25 Jul 2022 13:39:21 +0000 (15:39 +0200)] 
BUG/MAJOR: poller: drop FD's tgid when masks don't match

A bug was introduced in 2.7-dev2 by commit 1f947cb39 ("MAJOR: poller:
only touch/inspect the update_mask under tgid protection"): once the
FD's tgid is held, we would forget to drop it in case the update mask
doesn't match, resulting in random watchdog panics of older processes
on successive reloads.

This should fix issue #1798. Thanks to Christian for the report and
to Christopher for the reproducer.

No backport is needed.

2 years agoBUG/MEDIUM: master: force the thread count earlier
Willy Tarreau [Fri, 22 Jul 2022 15:35:49 +0000 (17:35 +0200)] 
BUG/MEDIUM: master: force the thread count earlier

Christopher bisected that recent commit d0b73bca71 ("MEDIUM: listener:
switch bind_thread from global to group-local") broke the master socket
in that only the first out of the Nth initial connections would work,
where N is the number of threads, after which they all work.

The cause is that the master socket was bound to multiple threads,
despite global.nbthread being 1 there, so the incoming connection load
balancing would try to send incoming connections to non-existing threads,
however the bind_thread mask would nonetheless include multiple threads.

What happened is that in 1.9 we forced "nbthread" to 1 in the master's poll
loop with commit b3f2be338b ("MEDIUM: mworker: use the haproxy poll loop").

In 2.0, nbthread detection was enabled by default in commit 149ab779cc
("MAJOR: threads: enable one thread per CPU by default"). From this point
on, the operation above is unsafe because everything during startup is
performed with nbthread corresponding to the default value, then it
changes to one when starting the polling loop. But by then we weren't
using the wait mode except for reload errors, so even if it would have
happened nobody would have noticed.

In 2.5 with commit fab0fdce9 ("MEDIUM: mworker: reexec in waitpid mode
after successful loading") we started to rexecute all the time, not just
for errors, so as to release precious resources and to possibly spot bugs
that were rarely exposed in this mode. By then the incoming connection LB
was enforcing all_threads_mask on the listener's thread mask so that the
incorrect value was being corrected while using it.

Finally in 2.7 commit d0b73bca71 ("MEDIUM: listener: switch bind_thread
from global to group-local") replaces the all_threads_mask there with
the listener's bind_thread, but that one was never adjusted by the
starting master, whose thread group was filled to N threads by the
automatic detection during early setup.

The best approach here is to set nbthread to 1 very early in init()
when we're in the master in wait mode, so that we don't try to guess
the best value and don't end up with incorrect bindings anymore. This
patch does this and also sets nbtgroups to 1 in preparation for a
possible future where this will also be automatically calculated.

There is no need to backport this patch since no other versions were
affected, but if it were to be discovered that the incorrect bind mask
on some of the master's FDs could be responsible for any trouble in
older versions, then the backport should be safe (provided that
nbtgroups is dropped of course).

2 years agoBUG/MINOR: backend: Fallback on RR algo if balance on source is impossible
Christopher Faulet [Fri, 22 Jul 2022 15:07:32 +0000 (17:07 +0200)] 
BUG/MINOR: backend: Fallback on RR algo if balance on source is impossible

If the loadbalancing is performed on the source IP address, an internal
error was returned on error. So for an applet on the client side (for
instance an SPOE applet) or for a client connected to a unix socket, an
internal error is returned.

However, when other LB algos fail, a fallback on round-robin is
performed. There is no reson to not do the same here.

This patch should fix the issue #1797. It must be backported to all
supported versions.

2 years agoBUG/MEDIUM: stconn: Only reset connect expiration when processing backend side
Christopher Faulet [Wed, 20 Jul 2022 11:24:04 +0000 (13:24 +0200)] 
BUG/MEDIUM: stconn: Only reset connect expiration when processing backend side

Since commit ae024ced0 ("MEDIUM: stream-int/stream: Use connect expiration
instead of SI expiration"), the connect expiration date is per-stream. So
there is only one expiration date instead of one per side, front and
back. So when a stream-connector is processed, we must test if it is a
frontend or a backend stconn before updating the connect expiration
date. Indeed, the frontend stconn must not reset the connect expiration
date.

This bug may have several side effect. One known bug is about peer sessions
blocked because the frontend peer applet is in ST_CLO state and its backend
connection is in ST_TAR state but without connect expiration date.

This patch should fix the issue #1791 and #1792. It must be backported to
2.6.

2 years agoBUILD: add detection for unsupported compiler models
Willy Tarreau [Thu, 21 Jul 2022 07:55:22 +0000 (09:55 +0200)] 
BUILD: add detection for unsupported compiler models

As reported in github issue #1765, some people get trapped into building
haproxy and companion libraries on Windows using a compiler following the
LLP64 model. This has no chance to work, and definitely causes nasty bugs
everywhere when pointers are passed as longs. Let's save them time and
detect this at boot time.

The message and detection was factored with the existing one for -fwrapv
since we need the same info and actions.

This should be backported to all recent supported versions (the ones
that are likely to be tried on such platforms when people don't know).

2 years agoBUG/MEDIUM: mworker: proc_self incorrectly set crashes upon reload
William Lallemand [Wed, 20 Jul 2022 22:52:43 +0000 (00:52 +0200)] 
BUG/MEDIUM: mworker: proc_self incorrectly set crashes upon reload

When updating from 2.4 to 2.6, the child->reloads++ instruction changed
place, resulting in a former worker from the 2.4 process, still
identified as a current worker once in 2.6, because its reload counter
is still 0.

Unfortunately this counter is used to chose the mworker_proc structure
that will be used for the new worker.

What happens next, is that the mworker_proc structure of the previous
process is selected, and this one has ipc_fd[1] set to -1, because this
structure was supposed to be in the master.

The process then forks, and mworker_sockpair_register_per_thread() tries
to register ipc_fd[1] which is set to -1, instead of the fd of the new
socketpair.

This patch fixes the issue by checking if child->pid is equal to -1 when
selecting proc_self. This way we could be sure it wasn't a previous
process.

Should fix issue #1785.

This must be backported as far as 2.4 to fix the issue related to the
reload computation difference. However backporting it in every stable
branch will enforce the reload process.

2 years agoBUG/MAJOR: mux_quic: fix invalid PROTOCOL_VIOLATION on POST data overlap
Frédéric Lécaille [Mon, 4 Jul 2022 07:54:58 +0000 (09:54 +0200)] 
BUG/MAJOR: mux_quic: fix invalid PROTOCOL_VIOLATION on POST data overlap

Stream data reception is incorrect when dealing with a partially new
offset with some data already consumed out of the RX buffer. In this
case, data length is adjusted but not the data buffer. In most cases,
ncb_add() operation will be rejected as already stored data does not
correspond with the new inserted offset. This will result in an invalid
CONNECTION_CLOSE with PROTOCOL_VIOLATION.

To fix this, buffer pointer is advanced while the length is reduced.

This can be reproduced with a POST request and patching haproxy to call
qcc_recv() multiple times by copying a quic_stream frame with different
offsets.

Must be backported to 2.6.

2 years agoBUG/MINOR: mworker/cli: relative pid prefix not validated anymore
William Lallemand [Wed, 20 Jul 2022 12:30:56 +0000 (14:30 +0200)] 
BUG/MINOR: mworker/cli: relative pid prefix not validated anymore

Since e8422bf ("MEDIUM: global: remove the relative_pid from global and
mworker"), the relative pid prefix is not tested anymore on the master
CLI. Which means any value will fall into the "1" process.

Since we removed the nbproc, only the "1" and the "0" (master) value are
correct, any other value should return an error.

Fix issue #1793.

This must be backported as far as 2.5.

2 years agoMINOR: ssl: enhance ca-file error emitting
William Lallemand [Tue, 19 Jul 2022 16:03:16 +0000 (18:03 +0200)] 
MINOR: ssl: enhance ca-file error emitting

Enhance the errors and warnings when trying to load a ca-file with
ssl_store_load_locations_file().

Add errors from ERR_get_error() and strerror to give more information to
the user.

2 years agoMINOR: init: load OpenSSL error strings
William Lallemand [Tue, 19 Jul 2022 16:13:29 +0000 (18:13 +0200)] 
MINOR: init: load OpenSSL error strings

Load OpenSSL Error strings in order to be able to output reason strings.

This is mandatory to be able to use ERR_reason_error_string().

2 years agoBUG/MEDIUM: fd/threads: fix incorrect thread selection in wakeup broadcast
Willy Tarreau [Tue, 19 Jul 2022 13:58:00 +0000 (15:58 +0200)] 
BUG/MEDIUM: fd/threads: fix incorrect thread selection in wakeup broadcast

In commit cfdd20a0b ("MEDIUM: fd: support broadcasting updates for foreign
groups in updt_fd_polling") we decided to pick a random thread number among
a set of candidates for a wakeup in case we need an instant change. But the
thread count range was wrong (MAX_THREADS) instead of tg->count, resulting
in random crashes when thread groups are > 1 and MAX_THREADS > 64.

No backport is needed, this was introduced in 2.7-dev2.

2 years agoBUG/MINOR: fd: Properly init the fd state in fd_insert()
Christopher Faulet [Tue, 19 Jul 2022 10:04:18 +0000 (12:04 +0200)] 
BUG/MINOR: fd: Properly init the fd state in fd_insert()

When a new fd is inserted in the fdtab array, its state is initialized. The
"newstate" variable is used to compute the right state (0 by default, but
FD_ET_POSSIBLE flag is set if edge-triggered is supported for the fd).
However, this variable is never used and the fd state is always set to 0.

Now, the fd state is initialized with "newstate" variable.

This bug was introduced by commit ddedc1662 ("MEDIUM: fd: make
fd_insert/fd_delete atomically update fd.tgid"). No backport needed.

2 years agoBUILD: debug: Add braces to if statement calling only CHECK_IF()
Christopher Faulet [Tue, 19 Jul 2022 09:53:46 +0000 (11:53 +0200)] 
BUILD: debug: Add braces to if statement calling only CHECK_IF()

In src/ev_epoll.c, a CHECK_IF() is guarded by an if statement. So, when the
macro is empty, GCC (at least 11.3.1) is not happy because there is an if
statement with an empty body without braces... It is handled by
"-Wempty-body" option.

So, braces are added and GCC is now happy.

No backport needed.

2 years agoBUG/MINOR: quic: do not send CONNECTION_CLOSE_APP in initial/handshake
Amaury Denoyelle [Mon, 18 Jul 2022 12:11:50 +0000 (14:11 +0200)] 
BUG/MINOR: quic: do not send CONNECTION_CLOSE_APP in initial/handshake

As specified by RFC 9000, it is forbidden to send a CONNECTION_CLOSE of
type 0x1d (CONNECTION_CLOSE_APP) in an Initial or Handshake packet. It
must be converted to type 0x1c (CONNECTION_CLOSE) with APPLICATION_ERROR
code.

CONNECTION_CLOSE_APP are generated by QUIC MUX interaction. Thus,
special care must be taken when dealing with a 0-RTT packet, as this is
the only case where the MUX can be instantiated and quic-conn still on
the Initial or Handshake encryption level.

To enforce RFC 9000, xprt build packet function is now responsible to
translate a CONNECTION_CLOSE_APP if still on Initial/Handshake
encryption. This process is done in a dedicated function named
qc_build_cc_frm().

Without this patch, BUG_ON() statement in qc_build_frm() will be
triggered when building a CONNECTION_CLOSE_APP frame on Initial or
Handshake level. This is because QUIC_FT_CONNECTION_CLOSE_APP frame
builder mask does not allow these encryption levels, as opposed to
QUIC_FT_CONNECTION_CLOSE builder. This crash was reproduced by modifying
the H3 layer to force emission of a CONNECTION_CLOSE_APP on first frame
of a 0-RTT session.

Note however that CONNECTION_CLOSE emission during Handshake is a
complicated process for the server. For the moment, this is still
incomplete on haproxy side. RFC 9000 requires to emit it multiple times
in several packets under different encryption levels, depending on what
we know about the client encryption context.

This patch should be backported up to 2.6.

2 years agoBUG/MINOR: tools: fix statistical_prng_range()'s output range
Willy Tarreau [Mon, 18 Jul 2022 17:09:55 +0000 (19:09 +0200)] 
BUG/MINOR: tools: fix statistical_prng_range()'s output range

This function was added by commit 84ebfabf7 ("MINOR: tools: add
statistical_prng_range() to get a random number over a range") but it
contains a bug on the range, since mul32hi() covers the whole input
range, we must pass it range-1. For now it didn't have any impact, but
if used to find an array's index it will cause trouble.

This should be backported to 2.4.

2 years agoBUG/MINOR: ssl: allow duplicate certificates in ca-file directories
William Lallemand [Mon, 18 Jul 2022 16:42:52 +0000 (18:42 +0200)] 
BUG/MINOR: ssl: allow duplicate certificates in ca-file directories

It looks like OpenSSL 1.0.2 returns an error when trying to insert a
certificate whis is already present in a X509_STORE.

This patch simply ignores the X509_R_CERT_ALREADY_IN_HASH_TABLE error if
emitted.

Should fix part of issue #1780.

Must be backported in 2.6.

2 years agoBUG/MINOR: resolvers: shut off the warning for the default resolvers
William Lallemand [Mon, 18 Jul 2022 12:12:17 +0000 (14:12 +0200)] 
BUG/MINOR: resolvers: shut off the warning for the default resolvers

When the resolv.conf file is empty or there is no resolv.conf file, an
empty resolvers will be created, which emits a warning during the
postparsing step.

This patch fixes the problem by freeing the resolvers section if the
parsing failed or if the nameserver list is empty.

Must be backported in 2.6, the previous patch which introduces
resolvers_destroy() is also required.

2 years agoMINOR: resolvers: resolvers_destroy() deinit and free a resolver
William Lallemand [Mon, 18 Jul 2022 12:09:58 +0000 (14:09 +0200)] 
MINOR: resolvers: resolvers_destroy() deinit and free a resolver

Split the resolvers_deinit() function into resolvers_destroy() and
resolvers_deinit() in order to be able to free a unique resolvers
section.

2 years agoBUG/MEDIUM: tools: avoid calling dlsym() in static builds (try 2)
Willy Tarreau [Mon, 18 Jul 2022 11:58:17 +0000 (13:58 +0200)] 
BUG/MEDIUM: tools: avoid calling dlsym() in static builds (try 2)

The first approach in commit 288dc1d8e ("BUG/MEDIUM: tools: avoid calling
dlsym() in static builds") relied on dlopen() but on certain configs (at
least gcc-4.8+ld-2.27+glibc-2.17) it used to catch situations where it
ought not fail.

Let's have a second try on this using dladdr() instead. The variable was
renamed "build_is_static" as it's exactly what's being detected there.
We could even take it for reporting in -vv though that doesn't seem very
useful. At least the variable was made global to ease inspection via the
debugger, or in case it's useful later.

Now it properly detects a static build even with gcc-4.4+glibc-2.11.1 and
doesn't crash anymore.

2 years agoBUILD: makefile: Fix install(1) handling for OpenBSD/NetBSD/Solaris/AIX
Brad Smith [Sat, 16 Jul 2022 09:18:50 +0000 (05:18 -0400)] 
BUILD: makefile: Fix install(1) handling for OpenBSD/NetBSD/Solaris/AIX

Add a new INSTALL variable to allow overridiing the flags passed to
install(1). install(1) on OpenBSD/NetBSD/Solaris/AIX does not support
the -v flag. With the new INSTALL variable and handling only use the
-v flag with the Linux targets.

2 years ago[RELEASE] Released version 2.7-dev2 v2.7-dev2
Willy Tarreau [Sat, 16 Jul 2022 15:17:22 +0000 (17:17 +0200)] 
[RELEASE] Released version 2.7-dev2

Released version 2.7-dev2 with the following main changes :
    - BUG/MINOR: qpack: fix build with QPACK_DEBUG
    - MINOR: h3: handle errors on HEADERS parsing/QPACK decoding
    - BUG/MINOR: qpack: abort on dynamic index field line decoding
    - MINOR: qpack: properly handle invalid dynamic table references
    - MINOR: task: Add tasklet_wakeup_after()
    - BUG/MINOR: quic: Dropped packets not counted (with RX buffers full)
    - MINOR: quic: Add new stats counter to diagnose RX buffer overrun
    - MINOR: quic: Duplicated QUIC_RX_BUFSZ definition
    - MINOR: quic: Improvements for the datagrams receipt
    - CLEANUP: h2: Typo fix in h2_unsubcribe() traces
    - MINOR: quic: Increase the QUIC connections RX buffer size (upto 64Kb)
    - CLEANUP: mux-quic: adjust comment on qcs_consume()
    - MINOR: ncbuf: implement ncb_is_fragmented()
    - BUG/MINOR: mux-quic: do not signal FIN if gap in buffer
    - MINOR: fd: add a new FD_DISOWN flag to prevent from closing a deleted FD
    - BUG/MEDIUM: ssl/fd: unexpected fd close using async engine
    - MINOR: tinfo: make tid temporarily still reflect global ID
    - CLEANUP: config: remove unused proc_mask()
    - MINOR: debug: remove mask support from "debug dev sched"
    - MEDIUM: task: add and preset a thread ID in the task struct
    - MEDIUM: task/debug: move the ->thread_mask integrity checks to ->tid
    - MAJOR: task: use t->tid instead of ffsl(t->thread_mask) to take the thread ID
    - MAJOR: task: replace t->thread_mask with 1<<t->tid when thread mask is needed
    - CLEANUP: task: remove thread_mask from the struct task
    - MEDIUM: applet: only keep appctx_new_*() and drop appctx_new()
    - MEDIUM: task: only keep task_new_*() and drop task_new()
    - MINOR: applet: always use task_new_on() on applet creation
    - MEDIUM: task: remove TASK_SHARED_WQ and only use t->tid
    - MINOR: task: replace task_set_affinity() with task_set_thread()
    - CLEANUP: task: remove the unused task_unlink_rq()
    - CLEANUP: task: remove the now unused TASK_GLOBAL flag
    - MINOR: task: make rqueue_ticks atomic
    - MEDIUM: task: move the shared runqueue to one per thread
    - MEDIUM: task: replace the global rq_lock with a per-rq one
    - MINOR: task: remove grq_total and use rq_total instead
    - MINOR: task: replace global_tasks_mask with a check for tree's emptiness
    - MEDIUM: task: use regular eb32 trees for the run queues
    - MEDIUM: queue: revert to regular inter-task wakeups
    - MINOR: thread: make wake_thread() take care of the sleeping threads mask
    - MINOR: thread: move the flags to the shared cache line
    - MINOR: thread: only use atomic ops to touch the flags
    - MINOR: poller: centralize poll return handling
    - MEDIUM: polling: make update_fd_polling() not care about sleeping threads
    - MINOR: poller: update_fd_polling: wake a random other thread
    - MEDIUM: thread: add a new per-thread flag TH_FL_NOTIFIED to remember wakeups
    - MEDIUM: tasks/fd: replace sleeping_thread_mask with a TH_FL_SLEEPING flag
    - MINOR: tinfo: add the tgid to the thread_info struct
    - MINOR: tinfo: replace the tgid with tgid_bit in tgroup_info
    - MINOR: tinfo: add the mask of enabled threads in each group
    - MINOR: debug: use ltid_bit in ha_thread_dump()
    - MINOR: wdt: use ltid_bit in wdt_handler()
    - MINOR: clock: use ltid_bit in clock_report_idle()
    - MINOR: thread: use ltid_bit in ha_tkillall()
    - MINOR: thread: add a new all_tgroups_mask variable to know about active tgroups
    - CLEANUP: thread: remove thread_sync_release() and thread_sync_mask
    - MEDIUM: tinfo: add a dynamic thread-group context
    - MEDIUM: thread: make stopping_threads per-group and add stopping_tgroups
    - MAJOR: threads: change thread_isolate to support inter-group synchronization
    - MINOR: thread: add is_thread_harmless() to know if a thread already is harmless
    - MINOR: debug: mark oneself harmless while waiting for threads to finish
    - MINOR: wdt: do not rely on threads_to_dump anymore
    - MEDIUM: debug: make the thread dumper not rely on a thread mask anymore
    - BUILD: debug: fix build issue on clang with previous commit
    - BUILD: debug: re-export thread_dump_state
    - BUG/MEDIUM: threads: fix incorrect thread group being used on soft-stop
    - BUG/MEDIUM: thread: check stopping thread against local bit and not global one
    - MINOR: proxy: use tg->threads_enabled in hard_stop() to detect stopped threads
    - BUILD: Makefile: Add Lua 5.4 autodetect
    - CI: re-enable gcc asan builds
    - MEDIUM: mworker: set the iocb of the socketpair without using fd_insert()
    - MINOR: fd: Add BUG_ON checks on fd_insert()
    - CLEANUP: mworker: rename mworker_pipe to mworker_sockpair
    - CLEANUP: mux-quic: do not export qc_get_ncbuf
    - REORG: mux-quic: reorganize flow-control fields
    - MINOR: mux-quic: implement accessor for sedesc
    - MEDIUM: mux-quic: refactor streams opening
    - MINOR: mux-quic: rename qcs flag FIN_RECV to SIZE_KNOWN
    - MINOR: mux-quic: emit FINAL_SIZE_ERROR on invalid STREAM size
    - BUG/MINOR: peers/config: always fill the bind_conf's argument
    - BUG/MEDIUM: peers/config: properly set the thread mask
    - CLEANUP: bwlim: Set pointers to NULL when memory is released
    - BUG/MINOR: http-check: Preserve headers if not redefined by an implicit rule
    - BUG/MINOR: http-act: Properly generate 103 responses when several rules are used
    - BUG/MEDIUM: thread: mask stopping_threads with threads_enabled when checking it
    - CLEANUP: thread: also remove a thread's bit from stopping_threads on stop
    - BUG/MINOR: peers: fix possible NULL dereferences at config parsing
    - BUG/MINOR: http-htx: Fix scheme based normalization for URIs wih userinfo
    - MINOR: http: Add function to get port part of a host
    - MINOR: http: Add function to detect default port
    - BUG/MEDIUM: h1: Improve authority validation for CONNCET request
    - MINOR: http-htx: Use new HTTP functions for the scheme based normalization
    - BUG/MEDIUM: http-fetch: Don't fetch the method if there is no stream
    - REGTEESTS: filters: Fix CONNECT request in random-forwarding script
    - MEDIUM: mworker/systemd: send STATUS over sd_notify
    - BUG/MINOR: mux-h1: Be sure to commit htx changes in the demux buffer
    - BUG/MEDIUM: http-ana: Don't wait to have an empty buf to switch in TUNNEL state
    - BUG/MEDIUM: mux-h1: Handle connection error after a synchronous send
    - MEDIUM: epoll: don't synchronously delete migrated FDs
    - BUILD: debug: silence warning on gcc-5
    - BUILD: http: silence an uninitialized warning affecting gcc-5
    - BUG/MEDIUM: mux-quic: fix server chunked encoding response
    - REORG: mux-quic: rename stream initialization function
    - MINOR: mux-quic: rename stream purge function
    - MINOR: mux-quic: add traces on frame parsing functions
    - MINOR: mux-quic: implement qcs_alert()
    - MINOR: mux-quic: filter send/receive-only streams on frame parsing
    - MINOR: mux-quic: do not ack STREAM frames on unrecoverable error
    - MINOR: mux-quic: support stream opening via MAX_STREAM_DATA
    - MINOR: mux-quic: define basic stream states
    - MINOR: mux-quic: use stream states to mark as detached
    - MEDIUM: mux-quic: implement RESET_STREAM emission
    - MEDIUM: mux-quic: implement STOP_SENDING handling
    - BUG/MEDIUM: debug: fix possible hang when multiple threads dump at once
    - BUG/MINOR: quic: fix closing state on NO_ERROR code sent
    - CLEANUP: quic: clean up include on quic_frame-t.h
    - MINOR: quic: define a generic QUIC error type
    - MINOR: mux-quic: support app graceful shutdown
    - MINOR: mux-quic/h3: prepare CONNECTION_CLOSE on release
    - MEDIUM: quic: send CONNECTION_CLOSE on released MUX
    - CLEANUP: mux-quic: move qc_release()
    - MINOR: mux-quic: send one last time before release
    - MINOR: h3: store control stream in h3c
    - MINOR: h3: implement graceful shutdown with GOAWAY
    - BUG/MINOR: threads: produce correct global mask for tgroup > 1
    - BUG/MEDIUM: cli/threads: make "show threads" more robust on applets
    - BUG/MINOR: thread: use the correct thread's group in ha_tkillall()
    - BUG/MINOR: debug: enter ha_panic() only once
    - BUG/MEDIUM: debug: fix parallel thread dumps again
    - MINOR: cli/streams: show a stream's tgid next to its thread ID
    - DEBUG: cli: add a new "debug dev deadlock" expert command
    - MINOR: cli/activity: add a thread number argument to "show activity"
    - CLEANUP: applet: remove the obsolete command context from the appctx
    - MEDIUM: config: remove deprecated "bind-process" directives from frontends
    - MEDIUM: config: remove the "process" keyword on "bind" lines
    - MINOR: listener/config: make "thread" always support up to LONGBITS
    - CLEANUP: fd: get rid of the __GET_{NEXT,PREV} macros
    - MEDIUM: debug/threads: make the lock debugging take tgroups into account
    - MEDIUM: proto: stop protocols under thread isolation during soft stop
    - MEDIUM: poller: program the update in fd_update_events() for a migrated FD
    - MEDIUM: poller: disable thread-groups for poll() and select()
    - MINOR: thread: remove MAX_THREADS limitation
    - MEDIUM: cpu-map: replace the process number with the thread group number
    - MINOR: mworker/threads: limit the mworker sockets to group 1
    - MINOR: cli/threads: always bind CLI to thread group 1
    - MINOR: fd/thread: get rid of thread_mask()
    - MEDIUM: task/thread: move the task shared wait queues per thread group
    - MINOR: task: move the niced_tasks counter to the thread group context
    - DOC: design: add some thoughts about how to handle the update_list
    - MEDIUM: conn: make conn_backend_get always scan the same group
    - MAJOR: fd: remove pending updates upon real close
    - MEDIUM: fd/poller: make the update-list per-group
    - MINOR: fd: delete unused updates on close()
    - MINOR: fd: make fd_insert() apply the thread mask itself
    - MEDIUM: fd: add the tgid to the fd and pass it to fd_insert()
    - MINOR: cli/fd: show fd's tgid and refcount in "show fd"
    - MINOR: fd: add functions to manipulate the FD's tgid
    - MINOR: fd: add fd_get_running() to atomically return the running mask
    - MAJOR: fd: grab the tgid before manipulating running
    - MEDIUM: fd/poller: turn polled_mask to group-local IDs
    - MEDIUM: fd/poller: turn update_mask to group-local IDs
    - MEDIUM: fd/poller: turn running_mask to group-local IDs
    - MINOR: fd: make fd_clr_running() return the previous value instead
    - MEDIUM: fd: make thread_mask now represent group-local IDs
    - MEDIUM: fd: make fd_insert() take local thread masks
    - MEDIUM: fd: make fd_insert/fd_delete atomically update fd.tgid
    - MEDIUM: fd: quit fd_update_events() when FD is closed
    - MEDIUM: thread: change thread_resolve_group_mask() to return group-local values
    - MEDIUM: listener: switch bind_thread from global to group-local
    - MINOR: fd: add fd_reregister_all() to deal with boot-time FDs
    - MEDIUM: fd: support stopping FDs during starting
    - MAJOR: pollers: rely on fd_reregister_all() at boot time
    - MAJOR: poller: only touch/inspect the update_mask under tgid protection
    - MEDIUM: fd: support broadcasting updates for foreign groups in updt_fd_polling
    - CLEANUP: threads: remove the now unused all_threads_mask and tid_bit
    - MINOR: config: change default MAX_TGROUPS to 16
    - BUG/MEDIUM: tools: avoid calling dlsym() in static builds

2 years agoBUG/MEDIUM: tools: avoid calling dlsym() in static builds
Willy Tarreau [Sat, 16 Jul 2022 11:49:34 +0000 (13:49 +0200)] 
BUG/MEDIUM: tools: avoid calling dlsym() in static builds

Since 2.4 with commit 64192392c ("MINOR: tools: add functions to retrieve
the address of a symbol"), we can resolve symbols. However some old glibc
crash in dlsym() when the program is statically built.

Fortunately even on these old libs we can detect lack of support by
calling dlopen(NULL). Normally it returns a handle to the current
program, but on a static build it returns NULL. This is sufficient to
refrain from calling dlsym() (which will be of very limited use anyway),
so we check this once at boot and use the result when needed.

This may be backported to 2.4. On stable versions, be careful to place
the init code inside an if/endif guard that checks for DL support.

2 years agoMINOR: config: change default MAX_TGROUPS to 16
Willy Tarreau [Fri, 15 Jul 2022 19:46:55 +0000 (21:46 +0200)] 
MINOR: config: change default MAX_TGROUPS to 16

This will allows nbtgroups > 1 to be declared in the config without
recompiling. The theoretical limit is 64, though we'd rather not push
it too far for now as some structures might be enlarged to be indexed
per group. Let's start with 16 groups max, allowing to experiment with
dual-socket machines suffering from up to 8 loosely coupled L3 caches.
It's a good start and doesn't engage us too far.

2 years agoCLEANUP: threads: remove the now unused all_threads_mask and tid_bit
Willy Tarreau [Fri, 15 Jul 2022 15:28:11 +0000 (17:28 +0200)] 
CLEANUP: threads: remove the now unused all_threads_mask and tid_bit

Since these are not used anymore, let's now remove them. Given the
number of places where we're using ti->ldit_bit, maybe an equivalent
might be useful though.

2 years agoMEDIUM: fd: support broadcasting updates for foreign groups in updt_fd_polling
Willy Tarreau [Fri, 15 Jul 2022 18:12:31 +0000 (20:12 +0200)] 
MEDIUM: fd: support broadcasting updates for foreign groups in updt_fd_polling

We're still facing the situation where it's impossible to update an FD
for a foreign group. That's of particular concern when disabling/enabling
listeners (e.g. pause/resume on signals) since we don't decide which thread
gets the signal and it needs to process all listeners at once.

Fortunately, not that much is unprotected in FDs. This patch adds a test for
tgid's equality in updt_fd_polling() so that if a change is applied for a
foreing group, then it's detected and taken care of separately. The method
consists in forcing the update on all bound threads in this group, adding it
to the group's update_list, and sending a wake-up as would be done for a
remote thread in the local group, except that this is done by grabbing a
reference to the FD's tgid.

Thanks to this, SIGTTOU/SIGTTIN now work for nbtgroups > 1 (after that was
temporarily broken by "MEDIUM: fd/poller: make the update-list per-group").

2 years agoMAJOR: poller: only touch/inspect the update_mask under tgid protection
Willy Tarreau [Sat, 9 Jul 2022 21:55:43 +0000 (23:55 +0200)] 
MAJOR: poller: only touch/inspect the update_mask under tgid protection

With thread groups and group-local masks, the update_mask cannot be
touched nor even checked if it may change below us. In order to avoid
this, we have to grab a reference to the FD's tgid before checking the
update mask. The operations are cheap enough so that we don't notice it
in performance tests. This is expected because the risk of meeting a
reassigned FD during an update remains very low.

It's worth noting that the tgid cannot be trusted during startup nor
during soft-stop since that may come from anywhere at the moment. Since
soft-stop runs under thread isolation we use that hint to decide whether
or not to check that the FD's tgid matches the current one.

The modification is applied to the 3 thread-aware pollers, i.e. epoll,
kqueue, and evports. Also one poll_drop counter was missing for shared
updates, though it might be hard to trigger it.

With this change applied, thread groups are usable in benchmarks.

2 years agoMAJOR: pollers: rely on fd_reregister_all() at boot time
Willy Tarreau [Sat, 9 Jul 2022 21:23:50 +0000 (23:23 +0200)] 
MAJOR: pollers: rely on fd_reregister_all() at boot time

The poller-specific thread init code now uses that new function to
safely register boot events. This ensures that we don't register an
event for another group and that we properly deal with parallel
thread startup.

It's only done for thread-aware pollers, there's no point in using
that in poll/select though that should work as well.

2 years agoMEDIUM: fd: support stopping FDs during starting
Willy Tarreau [Fri, 15 Jul 2022 16:56:48 +0000 (18:56 +0200)] 
MEDIUM: fd: support stopping FDs during starting

There's a nasty case during boot, which is the master process. It stops
all listeners from the main thread, and as such we're seeing calls to
fd_delete() from a thread that doesn't match the FD's mask, but more
importantly from a group that doesn't match either. Fortunately this
happens in a process that doesn't see the threads creation, so the FDs
are left intact in the table and we can overwrite the tgid there.

The approach is ugly, it probably shows that we should use a dummy
value for the tgid during boot, that would be replaced once the FDs
migrate to their target, but we also need a way to make sure not to
miss them. Also that doesn't solve the possibility of closing a
listener at run time from the wrong thread group.

2 years agoMINOR: fd: add fd_reregister_all() to deal with boot-time FDs
Willy Tarreau [Sat, 9 Jul 2022 21:19:19 +0000 (23:19 +0200)] 
MINOR: fd: add fd_reregister_all() to deal with boot-time FDs

At boot the pollers are allocated for each thread and they need to
reprogram updates for all FDs they will manage. This code is not
trivial, especially when trying to respect thread groups, so we'd
rather avoid duplicating it.

Let's centralize this into fd.c with this function. It avoids closed
FDs, those whose thread mask doesn't match the requested one or whose
thread group doesn't match the requested one, and performs the update
if required under thread-group protection.

2 years agoMEDIUM: listener: switch bind_thread from global to group-local
Willy Tarreau [Tue, 28 Jun 2022 06:30:43 +0000 (08:30 +0200)] 
MEDIUM: listener: switch bind_thread from global to group-local

It requires to both adapt the parser and change the algorithm to
redispatch incoming traffic so that local threads IDs may always
be used.

The internal structures now only reference thread group IDs and
group-local masks which are compatible with those now used by the
FD layer and the rest of the code.

2 years agoMEDIUM: thread: change thread_resolve_group_mask() to return group-local values
Willy Tarreau [Tue, 28 Jun 2022 06:27:43 +0000 (08:27 +0200)] 
MEDIUM: thread: change thread_resolve_group_mask() to return group-local values

It used to turn group+local to global but now we're doing the exact
opposite as we want to stick to group-local masks. This means that
"thread 3-4" might very well emit what "thread 2/1-2" used to emit
till now for 2 groups and 4 threads. This is needed because we'll
have to support group-local thread masks in receivers.

However the rest of the code (receivers) is not ready yet for this,
so using this code with more than one thread group will definitely
break some bindings.

2 years agoMEDIUM: fd: quit fd_update_events() when FD is closed
Willy Tarreau [Fri, 8 Jul 2022 13:36:14 +0000 (15:36 +0200)] 
MEDIUM: fd: quit fd_update_events() when FD is closed

The IOCB might have closed the FD itself, so it's not an error to
have fd.tgid==0 or anything else, nor to have a null running_mask.

In fact there are different conditions under which we can leave the
IOCB, all of them have been enumerated in the code's comments (namely
FD still valid and used, hence has running bit, FD closed but not yet
reassigned thus running==0, FD closed and reassigned, hence different
tgid and running becomes irrelevant, just like all other masks). For
this reason we have no other solution but to try to grab the tgid on
return before checking the other bits. In practice it doesn't represent
a big cost, because if the FD was closed and reassigned, it's instantly
detected and the bit is immediately released without blocking other
threads, and if the FD wasn't closed this doesn't prevent it from
being migrated to another thread. In the worst case a close by another
thread after a migration will be postponed till the moment the running
bit is cleared, which is the same as before.

2 years agoMEDIUM: fd: make fd_insert/fd_delete atomically update fd.tgid
Willy Tarreau [Thu, 7 Jul 2022 13:05:55 +0000 (15:05 +0200)] 
MEDIUM: fd: make fd_insert/fd_delete atomically update fd.tgid

These functions need to set/reset the FD's tgid but when they're called
there may still be wakeups on other threads that discover late updates
and have to touch the tgid at the same time. As such, it is not possible
to just read/write the tgid there. It must only be done using operations
that are compatible with what other threads may be doing.

As we're using inc/dec on the refcount, it's safe to AND the area to zero
the lower part when resetting the value. However, in order to set the
value, there's no other choice but fd_claim_tgid() which will assign it
only if possible (via a CAS). This is convenient in the end because it
protects the FD's masks from being modified by late threads, so while
we hold this refcount we can safely reset the thread_mask and a few other
elements. A debug test for non-null masks was added to fd_insert() as it
must not be possible to face this situation thanks to the protection
offered by the tgid.

2 years agoMEDIUM: fd: make fd_insert() take local thread masks
Willy Tarreau [Thu, 7 Jul 2022 06:29:00 +0000 (08:29 +0200)] 
MEDIUM: fd: make fd_insert() take local thread masks

fd_insert() was already given a thread group ID and a global thread mask.
Now we're changing the few callers to take the group-local thread mask
instead. It's passed directly into the FD's thread mask. Just like for
previous commit, it must not change anything when a single group is
configured.

2 years agoMEDIUM: fd: make thread_mask now represent group-local IDs
Willy Tarreau [Thu, 7 Jul 2022 06:23:03 +0000 (08:23 +0200)] 
MEDIUM: fd: make thread_mask now represent group-local IDs

With the change that was started on other masks, the thread mask was
still not fully converted, sometimes being used as a global mask and
sometimes as a local one. This finishes the code modifications so that
the mask is always considered as a group-local mask. This doesn't
change anything as long as there's a single group, but is necessary
for groups 2 and above since it's used against running_mask and so on.

2 years agoMINOR: fd: make fd_clr_running() return the previous value instead
Willy Tarreau [Sat, 9 Jul 2022 13:57:17 +0000 (15:57 +0200)] 
MINOR: fd: make fd_clr_running() return the previous value instead

It's an AND so it destroys information and due to this there's a call
place where we have to perform two reads to know the previous value
then to change it. With a fetch-and-and instead, in a single operation
we can know if the bit was previously present, which is more efficient.

2 years agoMEDIUM: fd/poller: turn running_mask to group-local IDs
Willy Tarreau [Thu, 7 Jul 2022 06:16:08 +0000 (08:16 +0200)] 
MEDIUM: fd/poller: turn running_mask to group-local IDs

From now on, the FD's running_mask only refers to local thread IDs. However,
there remains a limitation, in updt_fd_polling(), we temporarily have to
check and set shared FDs against .thread_mask, which still contains global
ones. As such, nbtgroups > 1 may break (but this is not yet supported without
special build options).

2 years agoMEDIUM: fd/poller: turn update_mask to group-local IDs
Willy Tarreau [Tue, 5 Jul 2022 17:21:06 +0000 (19:21 +0200)] 
MEDIUM: fd/poller: turn update_mask to group-local IDs

From now on, the FD's update_mask only refers to local thread IDs. However,
there remains a limitation, in updt_fd_polling(), we temporarily have to
check and set shared FDs against .thread_mask, which still contains global
ones. As such, nbtgroups > 1 may break (but this is not yet supported without
special build options).

2 years agoMEDIUM: fd/poller: turn polled_mask to group-local IDs
Willy Tarreau [Wed, 6 Jul 2022 08:37:31 +0000 (10:37 +0200)] 
MEDIUM: fd/poller: turn polled_mask to group-local IDs

This changes the signification of each bit in the polled_mask so that
now each bit represents a local thread ID for the current group instead
of a global thread ID. As such, all tests now apply to ltid_bit instead
of tid_bit.

No particular check was made to verify that the FD's tgid matches the
current one because there should be no case where this is not true. A
check was added in epoll's __fd_clo() to confirm it never differs unless
expected (soft stop under thread isolation, or master in starting mode
going to exec mode), but that doesn't prevent from doing the job: it
only consists in checking in the group's threads those that are still
polling this FD and to remove them.

Some atomic loads were added at the various locations, and most repetitive
references to polled_mask[fd].xx were turned to a local copy instead making
the code much more clear.

2 years agoMAJOR: fd: grab the tgid before manipulating running
Willy Tarreau [Wed, 6 Jul 2022 16:47:38 +0000 (18:47 +0200)] 
MAJOR: fd: grab the tgid before manipulating running

We now grab a reference to the FD's tgid before manipulating the
running_mask so that we're certain it corresponds to our own group
(hence bits), and we drop it once we've set the bit. For now there's
no measurable performance impact in doing this, which is great. The
lock can be observed by perf top as taking a small share of the time
spent in fd_update_events(), itself taking no more than 0.28% of CPU
under 8 threads.

However due to the fact that the thread groups are not yet properly
spread across the pollers and the thread masks are still wrong, this
will trigger some BUG_ON() in fd_insert() after a few tens of thousands
of connections when threads other than those of group 1 are reached,
and this is expected.

2 years agoMINOR: fd: add fd_get_running() to atomically return the running mask
Willy Tarreau [Sat, 9 Jul 2022 12:09:35 +0000 (14:09 +0200)] 
MINOR: fd: add fd_get_running() to atomically return the running mask

The running mask is only valid if the tgid is the expected one. This
function takes a reference on the tgid before reading the running mask,
so that both are checked at once. It returns either the mask or zero if
the tgid differs, thus providing a simple way for a caller to check if
it still holds the FD.

2 years agoMINOR: fd: add functions to manipulate the FD's tgid
Willy Tarreau [Wed, 6 Jul 2022 16:27:13 +0000 (18:27 +0200)] 
MINOR: fd: add functions to manipulate the FD's tgid

The FD's tgid is refcounted and must be atomically manipulated. Function
fd_grab_tgid() will increase the refcount but only if the tgid matches the
one in argument (likely the current one). fd_claim_tgid() will be used to
self-assign the tgid after waiting for its refcount to reach zero.
fd_drop_tgid() will be used to drop a temporarily held tgid. All of these
are needed to prevent an FD from being reassigned to another group, either
when inspecting/modifying the running_mask, or when checking for updates,
in order to be certain that the mask being seen corresponds to the desired
group. Note that once at least one bit is set in the running mask of an
active FD, it cannot be closed, thus not migrated, thus the reference does
not need to be held long.

2 years agoMINOR: cli/fd: show fd's tgid and refcount in "show fd"
Willy Tarreau [Fri, 8 Jul 2022 08:23:01 +0000 (10:23 +0200)] 
MINOR: cli/fd: show fd's tgid and refcount in "show fd"

We really need to display these values now.

2 years agoMEDIUM: fd: add the tgid to the fd and pass it to fd_insert()
Willy Tarreau [Tue, 5 Jul 2022 03:16:13 +0000 (05:16 +0200)] 
MEDIUM: fd: add the tgid to the fd and pass it to fd_insert()

The file descriptors will need to know the thread group ID in addition
to the mask. This extends fd_insert() to take the tgid, and will store
it into the FD.

In the FD, the tgid is stored as a combination of tgid on the lower 16
bits and a refcount on the higher 16 bits. This allows to know when it's
really possible to trust the tgid and the running mask. If a refcount is
higher than 1 it indeed indicates another thread else might be in the
process of updating these values.

Since a closed FD must necessarily have a zero refcount, a test was
added to fd_insert() to make sure that it is the case.

2 years agoMINOR: fd: make fd_insert() apply the thread mask itself
Willy Tarreau [Wed, 6 Jul 2022 09:27:42 +0000 (11:27 +0200)] 
MINOR: fd: make fd_insert() apply the thread mask itself

It's a bit ugly to see that half of the callers of fd_insert() have to
apply all_threads_mask themselves to the bit field they're passing,
because usually it comes from a listener that may have other bits set.
Let's make the function apply the mask itself.

2 years agoMINOR: fd: delete unused updates on close()
Willy Tarreau [Wed, 6 Jul 2022 14:20:11 +0000 (16:20 +0200)] 
MINOR: fd: delete unused updates on close()

After a poller's ->clo() was called to completely terminate operations
on an FD, there's no reason for keeping updates on this FD, so if any
updates were already programmed it would be nice if we could delete them.

Tests show that __fd_clo() is called roughly half of the time with the
last FD from the local update list, which possibly makes sense if a close
has to appear after a polling change resulting from an incomplete read or
the end of a send().

We can detect this and remove the last entry, which gives less work to
do during the update() call, and eliminates most of the poll_drop_fd
event reports.

Note that while tempting, this must not be backported because it's only
safe to be done now that fd_delete_orphan() clears the update mask as
we need to be certain not to miss it:
  - if the update mask is kept up with no entry, we can miss future
    updates ;
  - if the update mask is cleared too fast, it may result in failure
    to add a shared event.

2 years agoMEDIUM: fd/poller: make the update-list per-group
Willy Tarreau [Fri, 8 Jul 2022 09:33:43 +0000 (11:33 +0200)] 
MEDIUM: fd/poller: make the update-list per-group

The update-list needs to be per-group because its inspection is based
on a mask and we need to be certain when scanning it if a mask is for
the same thread or another one. Once per-group there's no doubt about
it, even if the FD's polling changes, the entry remains valid. It will
be needed to check the tgid though.

Note that a soft-stop or pause/resume might not necessarily work here
with tgroups>1, because the operation might be delivered to a thread
that doesn't belong to the group and whoe update mask will not reflect
one that is interesting here. We can't do better at this stage.

2 years agoMAJOR: fd: remove pending updates upon real close
Willy Tarreau [Wed, 6 Jul 2022 14:23:41 +0000 (16:23 +0200)] 
MAJOR: fd: remove pending updates upon real close

Dealing with long-lasting updates that outlive a close() is always
going to be quite a problem, not because of the thread that will
discover such updates late, but mostly due to the shared update_list
that will have an entry on hold making it difficult to reuse it, and
requiring that the fd's tgid is changed and the update_mask reset
from a safe location.

After careful inspection, it turns out that all our pollers that support
automatic event removal upon close() do not need any extra bookkeeping,
and that poll and select that use an internal representation already
provide a poller->clo() callback that is already used to update the
local event. As such, it is already safe to reset the update mask and
to remove the event from the shared list just before the final close,
because nothing remains to be done with this FD by the poller.

Doing so considerably simplifies the handling of updates, which will
only have to be inspected by the pollers, while the writers can continue
to consider that the entries are always valid. Another benefit is that
it will be possible to reduce contention on the update_list by just
having one update_list per group (left to be done later if needed).

2 years agoMEDIUM: conn: make conn_backend_get always scan the same group
Willy Tarreau [Thu, 7 Jul 2022 07:12:45 +0000 (09:12 +0200)] 
MEDIUM: conn: make conn_backend_get always scan the same group

We don't want to pick idle connections from another thread group,
this would be very slow by forcing to share undesirable data.

This patch makes sure that we start seeking from the current thread
group's threads only and loops over that range exclusively.

It's worth noting that the next_takeover pointer remains per-server
and will bounce when multiple groups use it at the same time. But we
preserve the perturbation by applying a modulo when retrieving it,
so that when groups are of the same size (most common case), the
index will not even change. At this time it doesn't seem worth
storing one index per group in servers, but that might be an option
if any contention is detected later.

2 years agoDOC: design: add some thoughts about how to handle the update_list
Willy Tarreau [Wed, 6 Jul 2022 13:44:49 +0000 (15:44 +0200)] 
DOC: design: add some thoughts about how to handle the update_list

This one is a real problem as it outlives the closure of the FD, and
some subtle changes are required.

2 years agoMINOR: task: move the niced_tasks counter to the thread group context
Willy Tarreau [Thu, 7 Jul 2022 13:25:40 +0000 (15:25 +0200)] 
MINOR: task: move the niced_tasks counter to the thread group context

This one is only used as a hint to improve scheduling latency, so there
is no more point in keeping it global since each thread group handles
its own run q

2 years agoMEDIUM: task/thread: move the task shared wait queues per thread group
Willy Tarreau [Thu, 7 Jul 2022 13:22:55 +0000 (15:22 +0200)] 
MEDIUM: task/thread: move the task shared wait queues per thread group

Their migration was postponed for convenience only but now's time for
having the shared wait queues per thread group and not just per process,
otherwise the WQ lock uses a huge amount of CPU alone.

2 years agoMINOR: fd/thread: get rid of thread_mask()
Willy Tarreau [Wed, 6 Jul 2022 09:22:42 +0000 (11:22 +0200)] 
MINOR: fd/thread: get rid of thread_mask()

Since commit d2494e048 ("BUG/MEDIUM: peers/config: properly set the
thread mask") there must not remain any single case of a receiver that
is bound nowhere, so there's no need anymore for thread_mask().

We're adding a test in fd_insert() to make sure this doesn't happen by
accident though, but the function was removed and its rare uses were
replaced with the original value of the bind_thread msak.

2 years agoMINOR: cli/threads: always bind CLI to thread group 1
Willy Tarreau [Wed, 6 Jul 2022 09:52:24 +0000 (11:52 +0200)] 
MINOR: cli/threads: always bind CLI to thread group 1

When using multiple groups, the stats socket starts to emit errors and
it's not natural to have to touch the global section just to specify
"thread 1/all". Let's pre-attach these sockets to thread group 1. This
will cause errors when trying to change the group but this really is not
a problem for now as thread groups are not enabled by default. This will
make sure configs remain portable and may possibly be relaxed later.

2 years agoMINOR: mworker/threads: limit the mworker sockets to group 1
Willy Tarreau [Wed, 6 Jul 2022 09:46:34 +0000 (11:46 +0200)] 
MINOR: mworker/threads: limit the mworker sockets to group 1

As a side effect of commit 34aae2fd1 ("MEDIUM: mworker: set the iocb of
the socketpair without using fd_insert()"), a config may now refuse to
start if there are multiple groups configured because the default bind
mask may span over multiple groups, and it is not possible to force it
to work differently.

Let's just assign thread group 1 to the master<->worker sockets so that
the thread bindings automatically resolve to a single group. The same was
done for the master side of the socket even if it's not used. It will
avoid being forgotten in the future.

2 years agoMEDIUM: cpu-map: replace the process number with the thread group number
Willy Tarreau [Fri, 8 Jul 2022 07:38:30 +0000 (09:38 +0200)] 
MEDIUM: cpu-map: replace the process number with the thread group number

The principle remains the same, but instead of having a single process
and ignoring extra ones, now we set the affinity masks for the respective
threads of all groups.

The doc was updated with a few extra examples.

2 years agoMINOR: thread: remove MAX_THREADS limitation
Willy Tarreau [Thu, 7 Jul 2022 13:11:32 +0000 (15:11 +0200)] 
MINOR: thread: remove MAX_THREADS limitation

This one is now causing difficulties during the development phase and
it's going to disappear anyway, let's get rid of it.

2 years agoMEDIUM: poller: disable thread-groups for poll() and select()
Willy Tarreau [Sat, 9 Jul 2022 21:38:46 +0000 (23:38 +0200)] 
MEDIUM: poller: disable thread-groups for poll() and select()

These old legacy pollers are not designed for this. They're still
using a shared list of events for all threads, this will not scale at
all, so there's no point in enabling thread-groups there. Modern
systems have epoll, kqueue or event ports and do not need these ones.

We arrange for failing at boot time, only when thread-groups > 1 so
that existing setups will remain unaffected.

If there's a compelling reason for supporting thread groups with these
pollers in the future, the rework should not be too hard, it would just
consume a lot of memory to have an fd_evts[] array per thread, but that
is doable.

2 years agoMEDIUM: poller: program the update in fd_update_events() for a migrated FD
Willy Tarreau [Sat, 9 Jul 2022 16:55:37 +0000 (18:55 +0200)] 
MEDIUM: poller: program the update in fd_update_events() for a migrated FD

When an FD is migrated, all pollers program an update. That's useless
code duplication, and when thread groups will be supported, this will
require an extra round of locking just to verify the update_mask on
return. Let's just program the update direction from fd_update_events()
as it already does for closed FDs, this becomes more logical.

2 years agoMEDIUM: proto: stop protocols under thread isolation during soft stop
Willy Tarreau [Fri, 15 Jul 2022 17:15:02 +0000 (19:15 +0200)] 
MEDIUM: proto: stop protocols under thread isolation during soft stop

protocol_stop_now() is called from do_soft_stop_now() running on any
thread that received the signal. The problem is that it will call some
listener handlers to close the FD, resulting in an fd_delete() being
called from the wrong group. That's not clean and we cannot even rely
on the thread mask to show up.

One interesting long-term approach could be to have kill queues for
FDs, and maybe we'll need them in the long run. However that doesn't
work well for listeners in this situation.

Let's simply isolate ourselves during this instant. We know we'll be
alone dealing with the close and that the FD will be instantly deleted
since not in use by any other thread. It's not the cleanest solution
but it should last long enough without causing trouble.

2 years agoMEDIUM: debug/threads: make the lock debugging take tgroups into account
Willy Tarreau [Fri, 15 Jul 2022 15:53:10 +0000 (17:53 +0200)] 
MEDIUM: debug/threads: make the lock debugging take tgroups into account

Since we have to use masks to verify owners/waiters, we have no other
option but to have them per group. This definitely inflates the size
of the locks, but this is only used for extreme debugging anyway so
that's not dramatic.

Thus as of now, all masks in the lock stats are local bit masks, derived
from ti->ltid_bit. Since at boot ltid_bit might not be set, we just take
care of this situation (since some structs are initialized under look
during boot), and use bit 0 from group 0 only.

2 years agoCLEANUP: fd: get rid of the __GET_{NEXT,PREV} macros
Willy Tarreau [Wed, 6 Jul 2022 12:43:51 +0000 (14:43 +0200)] 
CLEANUP: fd: get rid of the __GET_{NEXT,PREV} macros

They were initially made to deal with both the cache and the update list
but there's no cache anymore and keeping them for the update list adds a
lot of obfuscation that is really not desired. Let's get rid of them now.

Their purpose was simply to get a pointer to fdtab[fd].update.{,next,prev}
in order to perform atomic tests and modifications. The offset passed in
argument to the functions (fd_add_to_fd_list() and fd_rm_from_fd_list())
was the offset of the ->update field in fdtab, and as it's not used anymore
it was removed. This also removes a number of casts, though those used by
the atomic ops have to remain since only scalars are supported.

2 years agoMINOR: listener/config: make "thread" always support up to LONGBITS
Willy Tarreau [Fri, 15 Jul 2022 15:18:23 +0000 (17:18 +0200)] 
MINOR: listener/config: make "thread" always support up to LONGBITS

The difference is subtle but in one place there was MAXTHREADS and this
will not work anymore once it goes over 64.

2 years agoMEDIUM: config: remove the "process" keyword on "bind" lines
Willy Tarreau [Fri, 15 Jul 2022 15:16:01 +0000 (17:16 +0200)] 
MEDIUM: config: remove the "process" keyword on "bind" lines

It was deprecated, marked for removal in 2.7 and was already emitting a
warning, let's get rid of it. Note that we've kept the keyword detection
to suggest to use "thread" instead.