]> git.ipfire.org Git - thirdparty/haproxy.git/log
thirdparty/haproxy.git
3 years agoBUG/MEDIUM: mux-h1: Handle connection error after a synchronous send
Christopher Faulet [Fri, 8 Jul 2022 13:20:31 +0000 (15:20 +0200)] 
BUG/MEDIUM: mux-h1: Handle connection error after a synchronous send

Since commit d1480cc8 ("BUG/MEDIUM: stream-int: do not rely on the
connection error once established"), connection errors are not handled
anymore by the stream-connector once established. But it is a problem for
the H1 mux when an error occurred during a synchronous send in
h1_snd_buf(). Because in this case, the connction error is just missed. It
leads to a session leak until a timeout is reached (client or server).

To fix the bug, the connection flags are now checked in h1_snd_buf(). If
there is an error, it is reported to the stconn level by setting SF_FL_ERROR
flags. But only if there is no pending data in the input buffer.

This patch should solve the issue #1774. It must be backported as far as
2.2.

3 years agoBUG/MEDIUM: http-ana: Don't wait to have an empty buf to switch in TUNNEL state
Christopher Faulet [Fri, 8 Jul 2022 07:22:17 +0000 (09:22 +0200)] 
BUG/MEDIUM: http-ana: Don't wait to have an empty buf to switch in TUNNEL state

When we want to establish a tunnel on a side, we wait to have flush all data
from the buffer. At this stage the other side is at least in DONE state. But
there is no real reason to wait. We are already in DONE state on its
side. So all the HTTP message was already forwarded or planned to be
forwarded. Depending on the scheduling if the mux already started to
transfer tunneled data, these data may block the switch in TUNNEL state and
thus block these data infinitly.

This bug exists since the early days of HTX. May it was mandatory but today
it seems useless. But I honestly don't remember why this prerequisite was
added. So be careful during the backports.

This patch should be backported with caution. At least as far as 2.4. For
2.2 and 2.0, it seems to be mandatory too. But a review must be performed.

3 years agoBUG/MINOR: mux-h1: Be sure to commit htx changes in the demux buffer
Christopher Faulet [Fri, 8 Jul 2022 07:02:59 +0000 (09:02 +0200)] 
BUG/MINOR: mux-h1: Be sure to commit htx changes in the demux buffer

When a buffer area is casted to an htx message, depending on the method
used, the underlying buffer may be updated or not. The htxbuf() function
does not change the buffer state. We assume the buffer was already prepared
to store an htx message. htx_from_buf() on its side, updates the
buffer. With the first function, we only need to commit changes to the
underlying buffer if the htx message is changed.  With last one, we must
always commit the changes. The idea is to be sure to keep non-empty HTX
messages while an empty message must be lead to an empty buffer after
commit.

All that said because in h1_process_demux(), the changes is not always
committed as expected. When the demux is blocked, we just leave the
function. So it is possible to have an empty htx message stored in a buffer
that appears non-empty. It is not critical, but the buffer cannot be
released in this state. And we should always release an empty buffer.

This patch must be backported as far as 2.4.

3 years agoMEDIUM: mworker/systemd: send STATUS over sd_notify
William Lallemand [Thu, 7 Jul 2022 12:00:36 +0000 (14:00 +0200)] 
MEDIUM: mworker/systemd: send STATUS over sd_notify

The sd_notify API is not able to change the "Active:" line in "systemcl
status". However a message can still be displayed on a "Status: " line,
even if the service is still green and "active (running)".

When startup succeed the Status will be set to "Ready.", upon a reload
it will be set to "Reloading Configuration." If the configuration
succeed "Ready." again. However if the reload failed, it will be set to
"Reload failed!".

Keep in mind that the "Active:" line won't change upon a reload failure,
and will still be green.

3 years agoREGTEESTS: filters: Fix CONNECT request in random-forwarding script
Christopher Faulet [Thu, 7 Jul 2022 07:52:54 +0000 (09:52 +0200)] 
REGTEESTS: filters: Fix CONNECT request in random-forwarding script

An invalid CONNECT request was used and make the script failed because of a
recent fix.

3 years agoBUG/MEDIUM: http-fetch: Don't fetch the method if there is no stream
Christopher Faulet [Wed, 6 Jul 2022 15:53:02 +0000 (17:53 +0200)] 
BUG/MEDIUM: http-fetch: Don't fetch the method if there is no stream

The "method" sample fetch does not perform any check on the stream existence
before using it. However, for errors triggered at the mux level, there is no
stream. When the log message is formatted, this sample fetch must fail. It
must also fail when it is called from a health-check.

This patch must be backported as far as 2.4.

3 years agoMINOR: http-htx: Use new HTTP functions for the scheme based normalization
Christopher Faulet [Tue, 5 Jul 2022 08:24:52 +0000 (10:24 +0200)] 
MINOR: http-htx: Use new HTTP functions for the scheme based normalization

Use http_get_host_port() and http_is_default_port() functions to perform the
scheme based normalization.

3 years agoBUG/MEDIUM: h1: Improve authority validation for CONNCET request
Christopher Faulet [Tue, 5 Jul 2022 12:50:17 +0000 (14:50 +0200)] 
BUG/MEDIUM: h1: Improve authority validation for CONNCET request

From time to time, users complain to get 400-Bad-request responses for
totally valid CONNECT requests. After analysis, it is due to the H1 parser
performs an exact match between the authority and the host header value. For
non-CONNECT requests, it is valid. But for CONNECT requests the authority
must contain a port while it is often omitted from the host header value
(for default ports).

So, to be sure to not reject valid CONNECT requests, a basic authority
validation is now performed during the message parsing. In addition, the
host header value is normalized. It means the default port is removed if
possible.

This patch should solve the issue #1761. It must be backported to 2.6 and
probably as far as 2.4.

3 years agoMINOR: http: Add function to detect default port
Christopher Faulet [Tue, 5 Jul 2022 07:53:37 +0000 (09:53 +0200)] 
MINOR: http: Add function to detect default port

http_is_default_port() can be used to test if a port is a default HTTP/HTTPS
port. A scheme may be specified. In this case, it is used to detect defaults
ports, 80 for "http://" and 443 for "https://". Otherwise, with no scheme, both
are considered as default ports.

3 years agoMINOR: http: Add function to get port part of a host
Christopher Faulet [Tue, 5 Jul 2022 07:48:39 +0000 (09:48 +0200)] 
MINOR: http: Add function to get port part of a host

http_get_host_port() function can be used to get the port part of a host. It
will be used to get the port of an uri authority or a host header
value. This function only look for a port starting from the end of the
host. It is the caller responsibility to call it with a valid host value. An
indirect string is returned.

3 years agoBUG/MINOR: http-htx: Fix scheme based normalization for URIs wih userinfo
Christopher Faulet [Wed, 6 Jul 2022 15:41:31 +0000 (17:41 +0200)] 
BUG/MINOR: http-htx: Fix scheme based normalization for URIs wih userinfo

The scheme based normalization is not properly handled the URI's userinfo,
if any. First, the authority parser is not called with "no_userinfo"
parameter set. Then it is skipped from the URI normalization.

This patch must be backported as far as 2.4.

3 years agoBUG/MINOR: peers: fix possible NULL dereferences at config parsing
William Lallemand [Wed, 6 Jul 2022 12:30:23 +0000 (14:30 +0200)] 
BUG/MINOR: peers: fix possible NULL dereferences at config parsing

Patch 49f6f4b ("BUG/MEDIUM: peers: fix segfault using multiple bind on
peers sections") introduced possible NULL dereferences when parsing the
peers configuration.

Fix the issue by checking the return value of bind_conf_uniq_alloc().

This patch should be backported as far as 2.0.

3 years agoCLEANUP: thread: also remove a thread's bit from stopping_threads on stop
Willy Tarreau [Wed, 6 Jul 2022 08:17:21 +0000 (10:17 +0200)] 
CLEANUP: thread: also remove a thread's bit from stopping_threads on stop

As much as possible we should take care of not leaving bits from stopped
threads in shared thread masks. It can avoid issues like the previous
fix and will also make debugging less confusing.

3 years agoBUG/MEDIUM: thread: mask stopping_threads with threads_enabled when checking it
Willy Tarreau [Wed, 6 Jul 2022 08:13:05 +0000 (10:13 +0200)] 
BUG/MEDIUM: thread: mask stopping_threads with threads_enabled when checking it

When soft-stopping, there's a comparison between stopping_threads and
threads_enabled to make sure all threads are stopped, but this is not
correct and is racy since the threads_enabled bit is removed when a
thread is stopped but not its stopping_threads bit. The consequence is
that depending on timing, when stopping, if the first stopping thread
is fast enough to remove its bit from threads_enabled, the other threads
will see that stopping_threads doesn't match threads_enabled anymore and
will wait forever. As such the mask must be applied to stopping_threads
during the test. This issue was introduced in recent commit ef422ced9
("MEDIUM: thread: make stopping_threads per-group and add stopping_tgroups"),
no backport is needed.

3 years agoBUG/MINOR: http-act: Properly generate 103 responses when several rules are used
Christopher Faulet [Tue, 5 Jul 2022 14:24:15 +0000 (16:24 +0200)] 
BUG/MINOR: http-act: Properly generate 103 responses when several rules are used

When several "early-hint" rules are used, we try, as far as possible, to
merge links into the same 103-early-hints response. However, it only works
if there is no ACLs. If a "early-hint" rule is not executed an invalid
response is generated. the EOH block or the start-line may be missing,
depending on the rule order.

To fix the bug, we use the transaction status code. It is unused at this
stage. Thus, it is set to 103 when a 103-early-hints response is in
progress. And it is reset when the response is forwarded. In addition, the
response is forwarded if the next rule is an "early-hint" rule with an
ACL. This way, the response is always valid.

This patch must be backported as far as 2.2.

3 years agoBUG/MINOR: http-check: Preserve headers if not redefined by an implicit rule
Christopher Faulet [Tue, 5 Jul 2022 13:33:53 +0000 (15:33 +0200)] 
BUG/MINOR: http-check: Preserve headers if not redefined by an implicit rule

When an explicit "http-check send" rule is used, if it is the first one, it
is merge with the implicit rule created by "option httpchk" statement. The
opposite is also true. Idea is to have only one send rule with the merged
info. It means info defined in the second rule override those defined in the
first one. However, if an element is not defined in the second rule, it must
be ignored, keeping this way info from the first rule. It works as expected
for the method, the uri and the request version. But it is not true for the
header list.

For instance, with the following statements, a x-forwarded-proto header is
added to healthcheck requests:

  option httpchk
  http-check send meth GET hdr x-forwarded-proto https

while by inverting the statements, no extra headers are added:

  http-check send meth GET hdr x-forwarded-proto https
  option httpchk

Now the old header list is overriden if the new one is not empty.

This patch should fix the issue #1772. It must be backported as far as 2.2.

3 years agoCLEANUP: bwlim: Set pointers to NULL when memory is released
Christopher Faulet [Fri, 24 Jun 2022 12:52:18 +0000 (14:52 +0200)] 
CLEANUP: bwlim: Set pointers to NULL when memory is released

Calls to free() are replaced by ha_free(). And otherwise, the pointers are
explicitly set to NULL after a release. There is no issue here but it could
help debugging sessions.

3 years agoBUG/MEDIUM: peers/config: properly set the thread mask
Willy Tarreau [Tue, 5 Jul 2022 14:00:56 +0000 (16:00 +0200)] 
BUG/MEDIUM: peers/config: properly set the thread mask

The peers didn't have their bind_conf thread mask nor group set, because
they're still not part of the global proxy list. Till 2.6 it seems it does
not have any visible impact, since most listener-oriented operations pass
through thread_mask() which detects null masks and turns them to
all_threads_mask. But starting with 2.7 it becomes a problem as won't
permit these null masks anymore.

This patch duplicates (yes, sorry) the loop that applies to the frontend's
bind_conf, though it is simplified (no sharding, etc).

As the code is right now, it simply seems impossible to trigger the second
(and largest) part of the check when leaving thread_resolve_group_mask()
on success, so it looks like it might be removed.

No backport is needed, unless a report in 2.6 or earlier mentions an issue
with a null thread_mask.

3 years agoBUG/MINOR: peers/config: always fill the bind_conf's argument
Willy Tarreau [Tue, 5 Jul 2022 13:54:09 +0000 (15:54 +0200)] 
BUG/MINOR: peers/config: always fill the bind_conf's argument

Some generic frontend errors mention the bind_conf by its name as
"bind '%s'", but if this is used on peers "bind" lines it shows
"(null)" because the argument is set to NULL in the call to
bind_conf_uniq_alloc() instead of passing the argument. Fortunately
that's trivial to fix.

This may be backported to older versions.

3 years agoMINOR: mux-quic: emit FINAL_SIZE_ERROR on invalid STREAM size
Amaury Denoyelle [Mon, 4 Jul 2022 08:02:04 +0000 (10:02 +0200)] 
MINOR: mux-quic: emit FINAL_SIZE_ERROR on invalid STREAM size

Add a check on stream size when the stream is in state Size Known. In
this case, a STREAM frame cannot change the stream size. If this is not
respected, a CONNECTION_CLOSE with FINAL_SIZE_ERROR will be emitted as
specified in the RFC 9000.

3 years agoMINOR: mux-quic: rename qcs flag FIN_RECV to SIZE_KNOWN
Amaury Denoyelle [Fri, 1 Jul 2022 14:11:03 +0000 (16:11 +0200)] 
MINOR: mux-quic: rename qcs flag FIN_RECV to SIZE_KNOWN

Rename QC_SF_FIN_RECV to the more generic name QC_SF_SIZE_KNOWN. This
better align with the QUIC RFC 9000 which uses the "Size Known" state
definition. This change is purely cosmetic.

3 years agoMEDIUM: mux-quic: refactor streams opening
Amaury Denoyelle [Mon, 4 Jul 2022 13:50:33 +0000 (15:50 +0200)] 
MEDIUM: mux-quic: refactor streams opening

Review the whole API used to access/instantiate qcs.

A public function qcc_open_stream_local() is available to the
application protocol layer. It allows to easily opening a local stream.
The ID is automatically attributed to the next one available.

For remote streams, qcc_open_stream_remote() has been implemented. It
will automatically take care of allocating streams in a linear way
according to the ID. This function is called via qcc_get_qcs() which can
be used for each qcc_recv*() operations. For the moment, it is only used
for STREAM frames via qcc_recv(), but soon it will be implemented for
other frames types which can also be used to open a new stream.

qcs_new() and qcs_free() has been restricted to the MUX QUIC only as
they are now reserved for internal usage.

This change is a pure refactoring and should not have any noticeable
impact. It clarifies the developer intent and help to ensure that a
stream is not automatically opened when not desired.

3 years agoMINOR: mux-quic: implement accessor for sedesc
Amaury Denoyelle [Mon, 4 Jul 2022 09:42:27 +0000 (11:42 +0200)] 
MINOR: mux-quic: implement accessor for sedesc

Implement a function <qcs_sc> to easily access to the stconn associated
with a QCS. This takes care of qcs.sd which may be NULL, for example for
unidirectional streams.

It is expected that in the future when implementing
STOP_SENDING/RESET_STREAM, stconn must be notify about the event. This
accessor will allow to easily test if the stconn is instantiated or not.

3 years agoREORG: mux-quic: reorganize flow-control fields
Amaury Denoyelle [Mon, 4 Jul 2022 13:31:43 +0000 (15:31 +0200)] 
REORG: mux-quic: reorganize flow-control fields

<qcc.cl_bidi_r> is used to implement STREAM ID flow control enforcement.
Move it with all fields related to this operation and separated from MAX
STREAM DATA calcul.

3 years agoCLEANUP: mux-quic: do not export qc_get_ncbuf
Amaury Denoyelle [Mon, 4 Jul 2022 13:48:57 +0000 (15:48 +0200)] 
CLEANUP: mux-quic: do not export qc_get_ncbuf

qc_get_ncbuf() is only used internally : thus its prototype in QUIC MUX
include is not required.

3 years agoCLEANUP: mworker: rename mworker_pipe to mworker_sockpair
William Lallemand [Tue, 5 Jul 2022 07:04:03 +0000 (09:04 +0200)] 
CLEANUP: mworker: rename mworker_pipe to mworker_sockpair

The function mworker_pipe_register_per_thread() is called this way
because the master first used pipes instead of socketpairs.

Rename mworker_pipe_register_per_thread() to
mworker_sockpair_register_per_thread() in order to be more consistent.

Also update a comment inside the function.

3 years agoMINOR: fd: Add BUG_ON checks on fd_insert()
Emeric Brun [Fri, 1 Jul 2022 11:57:39 +0000 (04:57 -0700)] 
MINOR: fd: Add BUG_ON checks on fd_insert()

This patch adds two BUG_ON on fd_insert() into the fdtab checking
if the fd has been correctly re-initialized into the fdtab
before a new insert.

It will raise a BUG if we try to insert the same fd multiple times
without an intermediate fd_delete().

First one checks that the owner for this fd in fdtab was reset to NULL.

Second one checks that the state flags for this fd in fdtab was reset
to 0.

This patch could be backported on version >= 2.4

3 years agoMEDIUM: mworker: set the iocb of the socketpair without using fd_insert()
William Lallemand [Mon, 4 Jul 2022 22:55:09 +0000 (00:55 +0200)] 
MEDIUM: mworker: set the iocb of the socketpair without using fd_insert()

The worker was previously changing the iocb of the socketpair in the
worker by mworker_accept_wrapper(). However, it was done using
fd_insert() instead of changing directly the callback in the
fdtab[].iocb pointer.

This patch cleans up this by part by removing fd_insert().

It also stops setting tid_bit on the thread mask, the socketpair will be
handled by any thread from now.

3 years agoCI: re-enable gcc asan builds
Ilya Shipitsin [Sat, 2 Jul 2022 05:30:28 +0000 (10:30 +0500)] 
CI: re-enable gcc asan builds

for some unclear reasons asan builds were limited to clang only. let us
enable them for gcc as well

3 years agoBUILD: Makefile: Add Lua 5.4 autodetect
Christian Ruppert [Tue, 28 Jun 2022 08:03:00 +0000 (10:03 +0200)] 
BUILD: Makefile: Add Lua 5.4 autodetect

This patch is based on:
https://www.mail-archive.com/haproxy@formilux.org/msg39689.html
Thanks to Callum Farmer!

Signed-off-by: Christian Ruppert <idl0r@qasl.de>
3 years agoMINOR: proxy: use tg->threads_enabled in hard_stop() to detect stopped threads
Willy Tarreau [Mon, 4 Jul 2022 11:33:13 +0000 (13:33 +0200)] 
MINOR: proxy: use tg->threads_enabled in hard_stop() to detect stopped threads

Let's rely on tg->threads_enabled there to detect running threads. We
should probably have a dedicated function for this in order to simplify
the code and avoid the risk of using the wrong group ID.

3 years agoBUG/MEDIUM: thread: check stopping thread against local bit and not global one
Willy Tarreau [Mon, 4 Jul 2022 12:07:29 +0000 (14:07 +0200)] 
BUG/MEDIUM: thread: check stopping thread against local bit and not global one

Commit ef422ced9 ("MEDIUM: thread: make stopping_threads per-group and add
stopping_tgroups") moved the stopping_threads mask to per-group, but one
test in the loop preserved its global value instead, resulting in stopping
threads never sleeping on stop and eating 100% CPU until all were stopped.

No backport is needed.

3 years agoBUG/MEDIUM: threads: fix incorrect thread group being used on soft-stop
Willy Tarreau [Mon, 4 Jul 2022 11:36:16 +0000 (13:36 +0200)] 
BUG/MEDIUM: threads: fix incorrect thread group being used on soft-stop

Commit 377e37a80 ("MINOR: tinfo: add the mask of enabled threads in each
group") forgot -1 on the tgid, thus the groups was not always correctly
tested, which is visible only when running with more than one group. No
backport is needed.

3 years agoBUILD: debug: re-export thread_dump_state
Willy Tarreau [Fri, 1 Jul 2022 19:18:03 +0000 (21:18 +0200)] 
BUILD: debug: re-export thread_dump_state

Building with threads and without thread dump (e.g. macos, freebsd)
warns that thread_dump_state is unused. This happened in fact with
recentcommit 1229ef312 ("MINOR: wdt: do not rely on threads_to_dump
anymore"). The solution would be to mark it unused, but after a
second thought, it can be convenient to keep it exported to help
debug crashes, so let's export it again. It's just not referenced in
include files since it's not needed outside.

3 years agoBUILD: debug: fix build issue on clang with previous commit
Willy Tarreau [Fri, 1 Jul 2022 17:37:42 +0000 (19:37 +0200)] 
BUILD: debug: fix build issue on clang with previous commit

Since the thread_dump_state type changed to uint, the old value in the
CAS needs to be the same as well.

3 years agoMEDIUM: debug: make the thread dumper not rely on a thread mask anymore
Willy Tarreau [Fri, 1 Jul 2022 17:08:56 +0000 (19:08 +0200)] 
MEDIUM: debug: make the thread dumper not rely on a thread mask anymore

The thread mask is too short to dump more than 64 bits. Thus here we're
using a different approach with two counters, one for the next thread ID
to dump (which always exists, as it's looked up), and the second one for
the number of threads done dumping. This allows to dump threads in ascending
order then to let them wait for all others to be done, then to leave without
the risk of an overlapping dump until the done count is null again.

This allows to remove threads_to_dump which was the last non-FD variable
using a global thread mask.

3 years agoMINOR: wdt: do not rely on threads_to_dump anymore
Willy Tarreau [Fri, 1 Jul 2022 15:26:15 +0000 (17:26 +0200)] 
MINOR: wdt: do not rely on threads_to_dump anymore

This flag is not needed anymore as we're already marking the waiting
threads as harmless, thus the thread's bit is already covered by this
information. The variable was unexported.

3 years agoMINOR: debug: mark oneself harmless while waiting for threads to finish
Willy Tarreau [Fri, 1 Jul 2022 15:19:14 +0000 (17:19 +0200)] 
MINOR: debug: mark oneself harmless while waiting for threads to finish

The debug_handler() function waits for other threads to join, but does
not mark itself as harmless, so if at the same time another thread tries
to isolate, this may deadlock. In practice this does not happen as the
signal is received during epoll_wait() hence under harmless mode, but
it can possibly arrive under other conditions.

In order to improve this, while waiting for other threads to join, we're
now marking the current thread as harmless, as it's doing nothing but
waiting for the other ones. This way another harmless waiter will be able
to proceed. It's valid to do this since we're not doing anything else in
this loop.

One improvement could be to also check for the thread being idle and
marking it idle in addition to harmless, so that it can even release a
full isolation requester. But that really doesn't look worth it.

3 years agoMINOR: thread: add is_thread_harmless() to know if a thread already is harmless
Willy Tarreau [Fri, 1 Jul 2022 15:11:03 +0000 (17:11 +0200)] 
MINOR: thread: add is_thread_harmless() to know if a thread already is harmless

The harmless status is not re-entrant, so sometimes for signal handling
it can be useful to know if we're already harmless or not. Let's add a
function doing that, and make the debugger use it instead of manipulating
the harmless mask.

3 years agoMAJOR: threads: change thread_isolate to support inter-group synchronization
Willy Tarreau [Fri, 1 Jul 2022 13:08:37 +0000 (15:08 +0200)] 
MAJOR: threads: change thread_isolate to support inter-group synchronization

thread_isolate() and thread_isolate_full() were relying on a set of thread
masks for all threads in different states (rdv, harmless, idle). This cannot
work anymore when the number of threads increases beyond LONGBITS so we need
to change the mechanism.

What is done here is to have a counter of requesters and the number of the
current isolated thread. Threads which want to isolate themselves increment
the request counter and wait for all threads to be marked harmless (or idle)
by scanning all groups and watching the respective masks. This is possible
because threads cannot escape once they discover this counter, unless they
also want to isolate and possibly pass first. Once all threads are harmless,
the requesting thread tries to self-assign the isolated thread number, and
if it fails it loops back to checking all threads. If it wins it's guaranted
to be alone, and can drop its harmless bit, so that other competing threads
go back to the loop waiting for all threads to be harmless. The benefit of
proceeding this way is that there's very little write contention on the
thread number (none during work), hence no cache line moves between caches,
thus frozen threads do not slow down the isolated one.

Once it's done, the isolated thread resets the thread number (hence lets
another thread take the place) and decrements the requester count, thus
possibly releasing all harmless threads.

With this change there's no more need for any global mask to synchronize
any thread, and we only need to loop over a number of groups to check
64 threads at a time per iteration. As such, tinfo's threads_want_rdv
could be dropped.

This was tested with 64 threads spread into 2 groups, running 64 tasks
(from the debug dev command), 20 "show sess" (thread_isolate()), 20
"add server blah/blah" (thread_isolate()), and 20 "del server blah/blah"
(thread_isolate_full()). The load remained very low (limited by external
socat forks) and no stuck nor starved thread was found.

3 years agoMEDIUM: thread: make stopping_threads per-group and add stopping_tgroups
Willy Tarreau [Tue, 28 Jun 2022 17:29:29 +0000 (19:29 +0200)] 
MEDIUM: thread: make stopping_threads per-group and add stopping_tgroups

Stopping threads need a mask to figure who's still there without scanning
everything in the poll loop. This means this will have to be per-group.
And we also need to have a global stopping groups mask to know what groups
were already signaled. This is used both to figure what thread is the first
one to catch the event, and which one is the first one to detect the end of
the last job. The logic isn't changed, though a loop is required in the
slow path to make sure all threads are aware of the end.

Note that for now the soft-stop still takes time for group IDs > 1 as the
poller is not yet started on these threads and needs to expire its timeout
as there's no way to wake it up. But all threads are eventually stopped.

3 years agoMEDIUM: tinfo: add a dynamic thread-group context
Willy Tarreau [Mon, 27 Jun 2022 14:02:24 +0000 (16:02 +0200)] 
MEDIUM: tinfo: add a dynamic thread-group context

The thread group info is not sufficient to represent a thread group's
current state as it's read-only. We also need something comparable to
the thread context to represent the aggregate state of the threads in
that group. This patch introduces ha_tgroup_ctx[] and tg_ctx for this.
It's indexed on the group id and must be cache-line aligned. The thread
masks that were global and that do not need to remain global were moved
there (want_rdv, harmless, idle).

Given that all the masks placed there now become group-specific, the
associated thread mask (tid_bit) now switches to the thread's local
bit (ltid_bit). Both are the same for nbtgroups 1 but will differ for
other values.

There's also a tg_ctx pointer in the thread so that it can be reached
from other threads.

3 years agoCLEANUP: thread: remove thread_sync_release() and thread_sync_mask
Willy Tarreau [Mon, 27 Jun 2022 13:05:44 +0000 (15:05 +0200)] 
CLEANUP: thread: remove thread_sync_release() and thread_sync_mask

This function was added in 2.0 when reworking the thread isolation
mechanism to make it more reliable. However it if fundamentally
incompatible with the full isolation mechanism provided by
thread_isolate_full() since that one will wait for all threads to
become idle while the former will wait for all threads to finish
waiting, causing a deadlock.

Given that it's not used, let's just drop it entirely before it gets
used by accident.

3 years agoMINOR: thread: add a new all_tgroups_mask variable to know about active tgroups
Willy Tarreau [Fri, 24 Jun 2022 13:55:11 +0000 (15:55 +0200)] 
MINOR: thread: add a new all_tgroups_mask variable to know about active tgroups

In order to kill all_threads_mask we'll need to have an equivalent for
the thread groups. The all_tgroups_mask does just this, it keeps one bit
set per enabled group.

3 years agoMINOR: thread: use ltid_bit in ha_tkillall()
Willy Tarreau [Mon, 27 Jun 2022 14:23:44 +0000 (16:23 +0200)] 
MINOR: thread: use ltid_bit in ha_tkillall()

Since commit cc7a11ee3 ("MINOR: threads: set the tid, ltid and their bit
in thread_cfg") we ought not use (1UL << thr) to get the group mask for
thread <thr>, but (ha_thread_info[thr].ltid_bit). ha_tkillall() needs
this.

3 years agoMINOR: clock: use ltid_bit in clock_report_idle()
Willy Tarreau [Mon, 27 Jun 2022 14:22:22 +0000 (16:22 +0200)] 
MINOR: clock: use ltid_bit in clock_report_idle()

Since commit cc7a11ee3 ("MINOR: threads: set the tid, ltid and their bit
in thread_cfg") we ought not use (1UL << thr) to get the group mask for
thread <thr>, but (ha_thread_info[thr].ltid_bit). clock_report_idle()
needs this.

This also implies not using all_threads_mask anymore but taking the mask
from the tgroup since it becomes relative now.

3 years agoMINOR: wdt: use ltid_bit in wdt_handler()
Willy Tarreau [Mon, 27 Jun 2022 14:20:13 +0000 (16:20 +0200)] 
MINOR: wdt: use ltid_bit in wdt_handler()

Since commit cc7a11ee3 ("MINOR: threads: set the tid, ltid and their bit
in thread_cfg") we ought not use (1UL << thr) to get the group mask for
thread <thr>, but (ha_thread_info[thr].ltid_bit). wdt_handler() needs
this.

3 years agoMINOR: debug: use ltid_bit in ha_thread_dump()
Willy Tarreau [Mon, 27 Jun 2022 14:13:50 +0000 (16:13 +0200)] 
MINOR: debug: use ltid_bit in ha_thread_dump()

Since commit cc7a11ee3 ("MINOR: threads: set the tid, ltid and their bit
in thread_cfg") we ought not use (1UL << thr) to get the group mask for
thread <thr>, but (ha_thread_info[thr].ltid_bit). ha_thread_dump() needs
this.

3 years agoMINOR: tinfo: add the mask of enabled threads in each group
Willy Tarreau [Fri, 24 Jun 2022 13:18:49 +0000 (15:18 +0200)] 
MINOR: tinfo: add the mask of enabled threads in each group

In order to replace the global "all_threads_mask" we'll need to have an
equivalent per group. Take this opportunity for calling it threads_enabled
and make sure which ones are counted there (in case in the future we allow
to stop some).

3 years agoMINOR: tinfo: replace the tgid with tgid_bit in tgroup_info
Willy Tarreau [Tue, 28 Jun 2022 15:48:07 +0000 (17:48 +0200)] 
MINOR: tinfo: replace the tgid with tgid_bit in tgroup_info

Now that the tgid is accessible from the thread, it's pointless to have
it in the group, and it was only set but never used. However we'll soon
frequently need the mask corresponding to the group ID and the risk of
getting it wrong with the +1 or to shift 1 instead of 1UL is important,
so let's store the tgid_bit there.

3 years agoMINOR: tinfo: add the tgid to the thread_info struct
Willy Tarreau [Tue, 28 Jun 2022 08:49:57 +0000 (10:49 +0200)] 
MINOR: tinfo: add the tgid to the thread_info struct

At several places we're dereferencing the thread group just to catch
the group number, and this will become even more required once we start
to use per-group contexts. Let's just add the tgid in the thread_info
struct to make this easier.

3 years agoMEDIUM: tasks/fd: replace sleeping_thread_mask with a TH_FL_SLEEPING flag
Willy Tarreau [Mon, 20 Jun 2022 07:23:24 +0000 (09:23 +0200)] 
MEDIUM: tasks/fd: replace sleeping_thread_mask with a TH_FL_SLEEPING flag

Every single place where sleeping_thread_mask was still used was to test
or set a single thread. We can now add a per-thread flag to indicate a
thread is sleeping, and remove this shared mask.

The wake_thread() function now always performs an atomic fetch-and-or
instead of a first load then an atomic OR. That's cleaner and more
reliable.

This is not easy to test, as broadcast FD events are rare. The good
way to test for this is to run a very low rate-limited frontend with
a listener that listens to the fewest possible threads (2), and to
send it only 1 connection at a time. The listener will periodically
pause and the wakeup task will sometimes wake up on a random thread
and will call wake_thread():

   frontend test
        bind :8888 maxconn 10 thread 1-2
        rate-limit sessions 5

Alternately, disabling/enabling a frontend in loops via the CLI also
broadcasts such events, but they're more difficult to observe since
this is causing connection failures.

3 years agoMEDIUM: thread: add a new per-thread flag TH_FL_NOTIFIED to remember wakeups
Willy Tarreau [Wed, 22 Jun 2022 13:38:38 +0000 (15:38 +0200)] 
MEDIUM: thread: add a new per-thread flag TH_FL_NOTIFIED to remember wakeups

Right now when an inter-thread wakeup happens, we preliminary check if the
thread was asleep, and if so we wake the poller up and remove its bit from
the sleeping mask. That's not very clean since the sleeping mask cannot be
entirely trusted since a thread that's about to wake up will already have
its sleeping bit removed.

This patch adds a new per-thread flag (TH_FL_NOTIFIED) to remember that a
thread was notified to wake up. It's cleared before checking the task lists
last, so that new wakeups can be considered again (since wake_thread() is
only used to notify about task wakeups and FD polling changes). This way
we do not need to modify a remote thread's sleeping mask anymore. As such
wake_thread() now only tests and sets the TH_FL_NOTIFIED flag but doesn't
clear sleeping anymore.

3 years agoMINOR: poller: update_fd_polling: wake a random other thread
Willy Tarreau [Thu, 23 Jun 2022 16:31:08 +0000 (18:31 +0200)] 
MINOR: poller: update_fd_polling: wake a random other thread

When enabling an FD that's only bound to another thread, instead of
always picking the first one, let's pick a random one. This is rarely
used (enabling a frontend, or session rate-limiting period ending),
and has greater chances of avoiding that some obscure corner cases
could degenerate into a poorly distributed load.

3 years agoMEDIUM: polling: make update_fd_polling() not care about sleeping threads
Willy Tarreau [Thu, 23 Jun 2022 16:38:06 +0000 (18:38 +0200)] 
MEDIUM: polling: make update_fd_polling() not care about sleeping threads

Till now, update_fd_polling() used to check if all the target threads were
sleeping, and only then would wake an owning thread up. This causes several
problems among which the need for the sleeping_thread_mask and the fact that
by the time we wake one thread up, it has changed.

This commit changes this by leaving it to wake_thread() to perform this
check on the selected thread, since wake_thread() is already capable of
doing this now. Concretely speaking, for updt_fd_polling() it will mean
performing one computation of an ffsl() before knowing the sleeping status
on a global FD state change (which is very rare and not important here,
as it basically happens after relaxing a rate-limit (i.e. once a second
at beast) or after enabling a frontend from the CLI); thus we don't care.

3 years agoMINOR: poller: centralize poll return handling
Willy Tarreau [Wed, 22 Jun 2022 13:21:34 +0000 (15:21 +0200)] 
MINOR: poller: centralize poll return handling

When returning from the polling syscall, all pollers have a certain
dance to follow, made of wall clock updates, thread harmless updates,
idle time management and sleeping mask updates. Let's have a centralized
function to deal with all of this boring stuff: fd_leaving_poll(), and
make all the pollers use it.

3 years agoMINOR: thread: only use atomic ops to touch the flags
Willy Tarreau [Wed, 22 Jun 2022 07:19:46 +0000 (09:19 +0200)] 
MINOR: thread: only use atomic ops to touch the flags

The thread flags are touched a little bit by other threads, e.g. the STUCK
flag may be set by other ones, and they're watched a little bit. As such
we need to use atomic ops only to manipulate them. Most places were already
using them, but here we generalize the practice. Only ha_thread_dump() does
not change because it's run under isolation.

3 years agoMINOR: thread: move the flags to the shared cache line
Willy Tarreau [Wed, 22 Jun 2022 07:00:08 +0000 (09:00 +0200)] 
MINOR: thread: move the flags to the shared cache line

The thread flags were once believed to be local to the thread, but as
it stands, even the STUCK flag is shared since it's looked at by the
watchdog. As such we'll need to use atomic ops to manipulate them, and
likely to move them into the shared area.

This patch only moves the flag into the shared area so that we can later
decide whether it's best to leave them there or to move them back to the
local area. Interestingly, some tests have shown a 3% better performance
on dequeuing with this, while they're not used by other threads yet, so
there are definitely alignment effects that might change over time.

3 years agoMINOR: thread: make wake_thread() take care of the sleeping threads mask
Willy Tarreau [Mon, 20 Jun 2022 07:14:40 +0000 (09:14 +0200)] 
MINOR: thread: make wake_thread() take care of the sleeping threads mask

Almost every call place of wake_thread() checks for sleeping threads and
clears the sleeping mask itself, while the function is solely used for
there. Let's move the check and the clearing of the bit inside the function
itself. Note that updt_fd_polling() still performs the check because its
rules are a bit different.

3 years agoMEDIUM: queue: revert to regular inter-task wakeups
Willy Tarreau [Thu, 16 Jun 2022 14:10:05 +0000 (16:10 +0200)] 
MEDIUM: queue: revert to regular inter-task wakeups

Now that the inter-task wakeups are cheap, there's no point in using
task_instant_wakeup() anymore when dequeueing tasks. The use of the
regular task_wakeup() is sufficient there and will preserve a better
fairness: the test that went from 40k to 570k RPS now gives 580k RPS
(down from 585k RPS with previous commit). This essentially reverts
commit 27fab1dcb ("MEDIUM: queue: use tasklet_instant_wakeup() to
wake tasks").

3 years agoMEDIUM: task: use regular eb32 trees for the run queues
Willy Tarreau [Thu, 16 Jun 2022 14:28:01 +0000 (16:28 +0200)] 
MEDIUM: task: use regular eb32 trees for the run queues

Since we don't mix tasks from different threads in the run queues
anymore, we don't need to use the eb32sc_ trees and we can switch
to the regular eb32 ones. This uses cheaper lookup and insert code,
and a 16-thread test on the queues shows a performance increase
from 570k RPS to 585k RPS.

3 years agoMINOR: task: replace global_tasks_mask with a check for tree's emptiness
Willy Tarreau [Thu, 16 Jun 2022 13:59:36 +0000 (15:59 +0200)] 
MINOR: task: replace global_tasks_mask with a check for tree's emptiness

This bit field used to be a per-thread cache of the result of the last
lookup of the presence of a task for each thread in the shared cache.
Since we now know that each thread has its own shared cache, a test of
emptiness is now sufficient to decide whether or not the shared tree
has a task for the current thread. Let's just remove this mask.

3 years agoMINOR: task: remove grq_total and use rq_total instead
Willy Tarreau [Thu, 16 Jun 2022 13:52:49 +0000 (15:52 +0200)] 
MINOR: task: remove grq_total and use rq_total instead

grq_total was only used to know how many tasks were being queued in the
global runqueue for stats purposes, and that was transferred to the per
thread rq_total counter once assigned. We don't need this anymore since
we know where they are, so let's just directly update rq_total and drop
that one.

3 years agoMEDIUM: task: replace the global rq_lock with a per-rq one
Willy Tarreau [Thu, 16 Jun 2022 14:58:17 +0000 (16:58 +0200)] 
MEDIUM: task: replace the global rq_lock with a per-rq one

There's no point having a global rq_lock now that we have one shared RQ
per thread, let's have one lock per runqueue instead.

3 years agoMEDIUM: task: move the shared runqueue to one per thread
Willy Tarreau [Thu, 16 Jun 2022 13:30:50 +0000 (15:30 +0200)] 
MEDIUM: task: move the shared runqueue to one per thread

Since we only use the shared runqueue to put tasks only assigned to
known threads, let's move that runqueue to each of these threads. The
goal will be to arrange an N*(N-1) mesh instead of a central contention
point.

The global_rqueue_ticks had to be dropped (for good) since we'll now
use the per-thread rqueue_ticks counter for both trees.

A few points to note:
  - the rq_lock stlil remains the global one for now so there should not
    be any gain in doing this, but should this trigger any regression, it
    is important to detect whether it's related to the lock or to the tree.

  - there's no more reason for using the scope-based version of the ebtree
    now, we could switch back to the regular eb32_tree.

  - it's worth checking if we still need TASK_GLOBAL (probably only to
    delete a task in one's own shared queue maybe).

3 years agoMINOR: task: make rqueue_ticks atomic
Willy Tarreau [Thu, 16 Jun 2022 13:44:35 +0000 (15:44 +0200)] 
MINOR: task: make rqueue_ticks atomic

The runqueue ticks counter is per-thread and wasn't initially meant to
be shared. We'll soon have to share it so let's make it atomic. It's
only updated when waking up a task, and no performance difference was
observed. It was moved in the thread_ctx struct so that it doesn't
pollute the local cache line when it's later updated by other threads.

3 years agoCLEANUP: task: remove the now unused TASK_GLOBAL flag
Willy Tarreau [Thu, 16 Jun 2022 14:05:02 +0000 (16:05 +0200)] 
CLEANUP: task: remove the now unused TASK_GLOBAL flag

TASK_GLOBAL was exclusively used by task_unlink_rq(), as such it can be
dropped.

3 years agoCLEANUP: task: remove the unused task_unlink_rq()
Willy Tarreau [Thu, 16 Jun 2022 13:36:58 +0000 (15:36 +0200)] 
CLEANUP: task: remove the unused task_unlink_rq()

This function stopped being used before 2.4 because either the task is
dequeued by the scheduler itself and it knows where to find it, or it's
killed by any thread, and task_kill() must be used for this as only this
one is safe.

It's difficult to say whether task_unlink_rq() is still safe, but once
the lock moves to a thread declared in the task itself, it will be even
more difficult to keep it safe.

Let's just remove it now before someone reuses it and causes trouble.

3 years agoMINOR: task: replace task_set_affinity() with task_set_thread()
Willy Tarreau [Wed, 15 Jun 2022 15:20:16 +0000 (17:20 +0200)] 
MINOR: task: replace task_set_affinity() with task_set_thread()

The latter passes a thread ID instead of a mask, making the code simpler.

3 years agoMEDIUM: task: remove TASK_SHARED_WQ and only use t->tid
Willy Tarreau [Wed, 15 Jun 2022 14:48:45 +0000 (16:48 +0200)] 
MEDIUM: task: remove TASK_SHARED_WQ and only use t->tid

TASK_SHARED_WQ was set upon task creation and never changed afterwards.
Thus if a task was created to run anywhere (e.g. a check or a Lua task),
all its timers would always pass through the shared timers queue with a
lock. Now we know that tid<0 indicates a shared task, so we can use that
to decide whether or not to use the shared queue. The task might be
migrated using task_set_affinity() but it's always dequeued first so
the check will still be valid.

Not only this removes a flag that's difficult to keep synchronized with
the thread ID, but it should significantly lower the load on systems with
many checks. A quick test with 5000 servers and fast checks that were
saturating the CPU shows that the check rate increased by 20% (hence the
CPU usage dropped by 17%). It's worth noting that run_task_lists() almost
no longer appears in perf top now.

3 years agoMINOR: applet: always use task_new_on() on applet creation
Willy Tarreau [Wed, 15 Jun 2022 14:40:58 +0000 (16:40 +0200)] 
MINOR: applet: always use task_new_on() on applet creation

Now that task_new_on() supports negative numbers, there's no need for
checking the thread value nor falling back to task_new_anywhere().

3 years agoMEDIUM: task: only keep task_new_*() and drop task_new()
Willy Tarreau [Wed, 15 Jun 2022 14:37:33 +0000 (16:37 +0200)] 
MEDIUM: task: only keep task_new_*() and drop task_new()

As previously advertised in comments, the mask-based task_new() is now
gone. The low-level function now is task_new_on() which takes a thread
number or a negative value for "any thread", which is turned to zero
for thread-less builds since there's no shared WQ in thiscase. The
task_new_here() and task_new_anywhere() functions were adjusted
accordingly.

3 years agoMEDIUM: applet: only keep appctx_new_*() and drop appctx_new()
Willy Tarreau [Wed, 15 Jun 2022 14:35:51 +0000 (16:35 +0200)] 
MEDIUM: applet: only keep appctx_new_*() and drop appctx_new()

This removes the mask-based variant so that from now on the low-level
function becomes appctx_new_on() and it takes either a thread number or
a negative value for "any thread". This way we can use task_new_on() and
task_new_anywhere() instead of task_new() which will soon disappear.

3 years agoCLEANUP: task: remove thread_mask from the struct task
Willy Tarreau [Wed, 15 Jun 2022 13:57:53 +0000 (15:57 +0200)] 
CLEANUP: task: remove thread_mask from the struct task

It was not used anymore since everything moved to ->tid, so let's
remove it.

3 years agoMAJOR: task: replace t->thread_mask with 1<<t->tid when thread mask is needed
Willy Tarreau [Wed, 15 Jun 2022 13:44:00 +0000 (15:44 +0200)] 
MAJOR: task: replace t->thread_mask with 1<<t->tid when thread mask is needed

At a few places where the task's thread mask. Now we know that it's always
either one bit or all bits of all_threads_mask, so we can replace it with
either 1<<tid or all_threads_mask depending on what's expected.

It's worth noting that the global_tasks_mask is still set this way and
that it's reaching its limits. Similarly, the task_new() API would deserve
an update to stop using a thread mask and use a thread number instead.
Similarly, task_set_affinity() should be updated to directly take a
thread number.

At this point the task's thread mask is not used anymore.

3 years agoMAJOR: task: use t->tid instead of ffsl(t->thread_mask) to take the thread ID
Willy Tarreau [Wed, 15 Jun 2022 12:31:38 +0000 (14:31 +0200)] 
MAJOR: task: use t->tid instead of ffsl(t->thread_mask) to take the thread ID

At several places we need to figure the ID of the first thread allowed
to run a task. Till now this was performed using my_ffsl(t->thread_mask)
but since we now have the thread ID stored into the task, let's use it
instead. This is tagged major because it starts to assume that tid<0 is
strictly equivalent to atleast2(thread_mask), and that as such, among
the allowed threads are the current one.

3 years agoMEDIUM: task/debug: move the ->thread_mask integrity checks to ->tid
Willy Tarreau [Wed, 15 Jun 2022 12:24:57 +0000 (14:24 +0200)] 
MEDIUM: task/debug: move the ->thread_mask integrity checks to ->tid

Let's make sure the new ->tid field is always correct instead of checking
the thread mask.

3 years agoMEDIUM: task: add and preset a thread ID in the task struct
Willy Tarreau [Wed, 15 Jun 2022 12:19:48 +0000 (14:19 +0200)] 
MEDIUM: task: add and preset a thread ID in the task struct

The tasks currently rely on a mask but do not have an assigned thread ID,
contrary to tasklets. However, in practice they're either running on a
single thread or on any thread, so that it will be worth simplifying all
this in order to ease the transition to the thread groups.

This patch introduces a "tid" field in the task struct, that's either
the number of the thread the task is attached to, or a negative value
if the task is not bound to a thread, (i.e. its mask is all_threads_mask).

The new ID is only set and updated but not used yet.

3 years agoMINOR: debug: remove mask support from "debug dev sched"
Willy Tarreau [Wed, 15 Jun 2022 14:32:41 +0000 (16:32 +0200)] 
MINOR: debug: remove mask support from "debug dev sched"

The thread mask will not be used anymore, instead the thread id only
is used. Interestingly it was already implemented in the parsing but
not used. The single/multi thread argument is not needed anymore since
it's sufficient to pass tid<0 to get a multi-threaded task/tasklet.

This is in preparation for the removal of the thread_mask in tasks as
only this debug code was using it!

3 years agoCLEANUP: config: remove unused proc_mask()
Willy Tarreau [Fri, 24 Jun 2022 07:26:19 +0000 (09:26 +0200)] 
CLEANUP: config: remove unused proc_mask()

The function was used to return the mask of enabled processes, but it
now always returns 1 and is not called anymore. Let's drop it.

3 years agoMINOR: tinfo: make tid temporarily still reflect global ID
Willy Tarreau [Tue, 14 Jun 2022 08:43:01 +0000 (10:43 +0200)] 
MINOR: tinfo: make tid temporarily still reflect global ID

For now we still set tid_bit to (1UL << tid) because FDs will not
work with more than one group without this, but once FDs start to
adopt local masks this must change to thr->ltid_bit.

3 years agoBUG/MEDIUM: ssl/fd: unexpected fd close using async engine
Emeric Brun [Fri, 1 Jul 2022 15:36:50 +0000 (17:36 +0200)] 
BUG/MEDIUM: ssl/fd: unexpected fd close using async engine

Before 2.3, after an async crypto processing or on session close, the engine
async file's descriptors were removed from the fdtab but not closed because
it is the engine which has created the file descriptor, and it is responsible
for closing it. In 2.3 the fd_remove() call was replaced by fd_stop_both()
which stops the polling but does not remove the fd from the fdtab and the
fd remains indefinitively in the fdtab.

A simple replacement by fd_delete() is not a valid fix because fd_delete()
removes the fd from the fdtab but also closes the fd. And the fd will be
closed twice: by the haproxy's core and by the engine itself.

Instead, let's set FD_DISOWN on the FD before calling fd_delete() which will
take care of not closing it.

This patch must be backported on branches >= 2.3, and it relies on this
previous patch:

   MINOR: fd: add a new FD_DISOWN flag to prevent from closing a deleted FD

As mentioned in the patch above, a different flag will be needed in 2.3.

3 years agoMINOR: fd: add a new FD_DISOWN flag to prevent from closing a deleted FD
Emeric Brun [Fri, 1 Jul 2022 15:31:25 +0000 (17:31 +0200)] 
MINOR: fd: add a new FD_DISOWN flag to prevent from closing a deleted FD

Some FDs might be offered to some external code (external libraries)
which will deal with them until they close them. As such we must not
close them upon fd_delete() but we need to delete them anyway so that
they do not appear anymore in the fdtab. This used to be handled by
fd_remove() before 2.3 but we don't have this anymore.

This patch introduces a new flag FD_DISOWN to let fd_delete() know that
the core doesn't own the fd and it must not be closed upon removal from
the fd_tab. This way it's totally unregistered from the poller but still
open.

This patch must be backported on branches >= 2.3 because it will be
needed to fix a bug affecting SSL async. it should be adapted on 2.3
because state flags were stored in a different way (via bits in the
structure).

3 years agoBUG/MINOR: mux-quic: do not signal FIN if gap in buffer
Amaury Denoyelle [Fri, 1 Jul 2022 09:26:04 +0000 (11:26 +0200)] 
BUG/MINOR: mux-quic: do not signal FIN if gap in buffer

Adjust FIN signal on Rx path for the application layer : ensure that the
receive buffer has no gap.

Without this extra condition, FIN was signalled as soon as the STREAM
frame with FIN was received, even if we were still waiting to receive
missing offsets.

This bug could have lead to incomplete requests read from the
application protocol. However, in practice this bug has very little
chance to happen as the application layer ensures that the demuxed frame
length is equivalent to the buffer data size. The only way to happen is
if to receive the FIN STREAM as the H3 demuxer is still processing on a
frame which is not the last one of the stream.

This must be backported up to 2.6. The previous patch on ncbuf is
required for the newly defined function ncb_is_fragmented().
  MINOR: ncbuf: implement ncb_is_fragmented()

3 years agoMINOR: ncbuf: implement ncb_is_fragmented()
Amaury Denoyelle [Fri, 1 Jul 2022 12:45:41 +0000 (14:45 +0200)] 
MINOR: ncbuf: implement ncb_is_fragmented()

Implement a new status function for ncbuf. It allows to quickly report
if a buffer contains data in a fragmented way, i.e. with gaps in between
or at start of the buffer.

To summarize, a buffer is considered as non-fragmented in the following
cases :
- a null or empty buffer
- a full buffer
- a buffer containing exactly one data block at the beginning, following
  by a gap until the end.

3 years agoCLEANUP: mux-quic: adjust comment on qcs_consume()
Amaury Denoyelle [Fri, 1 Jul 2022 09:25:40 +0000 (11:25 +0200)] 
CLEANUP: mux-quic: adjust comment on qcs_consume()

Since a previous refactoring, application protocol layer is not require
anymore to call qcs_consume(). This function is now automatically used
by the MUX itself.

3 years agoMINOR: quic: Increase the QUIC connections RX buffer size (upto 64Kb)
Frédéric Lécaille [Thu, 30 Jun 2022 10:04:38 +0000 (12:04 +0200)] 
MINOR: quic: Increase the QUIC connections RX buffer size (upto 64Kb)

With ~1500 bytes QUIC datagrams, we can handle less than 200 datagrams
which is less than the default maxpollevents value. This should reduce
the chances of fulfilling the connections RX buffers as reported by
Tristan in GH #1737.

Must be backported to 2.6.

3 years agoCLEANUP: h2: Typo fix in h2_unsubcribe() traces
Frédéric Lécaille [Thu, 30 Jun 2022 10:01:54 +0000 (12:01 +0200)] 
CLEANUP: h2: Typo fix in h2_unsubcribe() traces

Very minor modification for the traces of this function.

3 years agoMINOR: quic: Improvements for the datagrams receipt
Frédéric Lécaille [Thu, 30 Jun 2022 09:28:56 +0000 (11:28 +0200)] 
MINOR: quic: Improvements for the datagrams receipt

First we add a loop around recfrom() into the most low level I/O handler
quic_sock_fd_iocb() to collect as most as possible datagrams before during
its tasklet wakeup with a limit: we recvfrom() at most "maxpollevents"
datagrams. Furthermore we add a local task list into the datagram handler
quic_lstnr_dghdlr() which is passed to the first datagrams parser qc_lstnr_pkt_rcv().
This latter parser only identifies the connection associated to the datagrams then
wakeup the highest level packet parser I/O handlers (quic_conn.*io_cb()) after
it is done, thanks to the call to tasklet_wakeup_after() which replaces from now on
the call to tasklet_wakeup(). This should reduce drastically the latency and the
chances to fulfil the RX buffers at the QUIC connections level as reported in
GH #1737 by Tritan.

These modifications depend on this commit:
    "MINOR: task: Add tasklet_wakeup_after()"

Must be backported to 2.6 with the previous commit.

3 years agoMINOR: quic: Duplicated QUIC_RX_BUFSZ definition
Frédéric Lécaille [Thu, 30 Jun 2022 09:25:09 +0000 (11:25 +0200)] 
MINOR: quic: Duplicated QUIC_RX_BUFSZ definition

This macro is already defined in src/quic_sock.c which is the correct place.

Must be backported to 2.6

3 years agoMINOR: quic: Add new stats counter to diagnose RX buffer overrun
Frédéric Lécaille [Wed, 29 Jun 2022 10:03:34 +0000 (12:03 +0200)] 
MINOR: quic: Add new stats counter to diagnose RX buffer overrun

Remove the call to qc_list_all_rx_pkts() which print messages on stderr
during RX buffer overruns and add a new counter for the number of dropped packets
because of such events.

Must be backported to 2.6

3 years agoBUG/MINOR: quic: Dropped packets not counted (with RX buffers full)
Frédéric Lécaille [Wed, 29 Jun 2022 09:24:35 +0000 (11:24 +0200)] 
BUG/MINOR: quic: Dropped packets not counted (with RX buffers full)

When the connection RX buffer is full, the received packets are dropped.
Some of them were not taken into an account by the ->dropped_pkt counter.
This very simple patch has no impact at all on the packet handling workflow.

Must be backported to 2.6.

3 years agoMINOR: task: Add tasklet_wakeup_after()
Frédéric Lécaille [Wed, 29 Jun 2022 08:53:03 +0000 (10:53 +0200)] 
MINOR: task: Add tasklet_wakeup_after()

We want to be able to schedule a tasklet onto a thread after the current tasklet
is done. What we have to do is to insert this tasklet at the head of the thread
task list. Furthermore, we would like to serialize the tasklets. They must be
run in the same order as the order in which they have been scheduled. This is
implemented passing a list of tasklet as parameter (see <head> parameters) which
must be reused for subsequent calls.
_tasklet_wakeup_after_on() is implemented to accomplish this job.
tasklet_wakeup_after_on() and tasklet_wake_after() are only wrapper macros around
_tasklet_wakeup_after_on(). tasklet_wakeup_after_on() does exactly the same thing
as _tasklet_wakeup_after_on() without having to pass the filename and line in the
filename as parameters (usefull when DEBUG_TASK is enabled).
tasklet_wakeup_after() hides also the usage of the thread parameter which is
<tl> tasklet thread ID.

3 years agoMINOR: qpack: properly handle invalid dynamic table references
Amaury Denoyelle [Thu, 30 Jun 2022 07:30:23 +0000 (09:30 +0200)] 
MINOR: qpack: properly handle invalid dynamic table references

Return QPACK_DECOMPRESSION_FAILED error code when dealing with dynamic
table references. This is justified as for now haproxy does not
implement dynamic table support and advertizes a zero-sized table.

The H3 calling function will thus reuse this code in a CONNECTION_CLOSE
frame, in conformance with the QPACK RFC.

This proper error management allows to remove obsolete ABORT_NOW guards.

3 years agoBUG/MINOR: qpack: abort on dynamic index field line decoding
Amaury Denoyelle [Thu, 30 Jun 2022 07:31:24 +0000 (09:31 +0200)] 
BUG/MINOR: qpack: abort on dynamic index field line decoding

This is a complement to partial fix from commit
  debaa04f9e249f6bf75d40f38b34cfdcd7fc2047
  BUG/MINOR: qpack: abort on dynamic index field line decoding

The main objective is to fix coverity report about usage of
uninitialized variable when receiving dynamic table references. These
references are invalid as for the moment haproxy advertizes a 0-sized
dynamic table. An ABORT_NOW clause is present to catch this. A following
patch will clean up this in order to properly handle QPACK errors with
CONNECTION_CLOSE.

This should fix github issue #1753.

No need to backport as this was introduced in the current dev branch.

3 years agoMINOR: h3: handle errors on HEADERS parsing/QPACK decoding
Amaury Denoyelle [Thu, 30 Jun 2022 08:04:42 +0000 (10:04 +0200)] 
MINOR: h3: handle errors on HEADERS parsing/QPACK decoding

Emit a CONNECTION_CLOSE if HEADERS parsing function returns an error.
This is useful to remove previous ABORT_NOW guards.

For the moment, the whole connection is closed. In the future, it may be
justified to only reset the faulting stream in some cases. This requires
the implementation of RESET_STREAM emission.

3 years agoBUG/MINOR: qpack: fix build with QPACK_DEBUG
Amaury Denoyelle [Thu, 30 Jun 2022 07:28:50 +0000 (09:28 +0200)] 
BUG/MINOR: qpack: fix build with QPACK_DEBUG

The local variable 't' was renamed 'static_tbl'. Fix its name in the
qpack_debug_printf() statement which is activated only with QPACK_DEBUG
mode.

No need to backport as this was introduced in current dev branch.

3 years ago[RELEASE] Released version 2.7-dev1 v2.7-dev1
Willy Tarreau [Fri, 24 Jun 2022 20:09:05 +0000 (22:09 +0200)] 
[RELEASE] Released version 2.7-dev1

Released version 2.7-dev1 with the following main changes :
    - BUG/MINOR: ssl_ckch: Free error msg if commit changes on a cert entry fails
    - BUG/MINOR: ssl_ckch: Free error msg if commit changes on a CA/CRL entry fails
    - BUG/MEDIUM: ssl_ckch: Don't delete a cert entry if it is being modified
    - BUG/MEDIUM: ssl_ckch: Don't delete CA/CRL entry if it is being modified
    - BUG/MINOR: ssl_ckch: Don't duplicate path when replacing a cert entry
    - BUG/MINOR: ssl_ckch: Don't duplicate path when replacing a CA/CRL entry
    - BUG/MEDIUM: ssl_ckch: Rework 'commit ssl cert' to handle full buffer cases
    - BUG/MEDIUM: ssl_ckch: Rework 'commit ssl ca-file' to handle full buffer cases
    - BUG/MEDIUM: ssl/crt-list: Rework 'add ssl crt-list' to handle full buffer cases
    - BUG/MEDIUM: httpclient: Don't remove HTX header blocks before duplicating them
    - BUG/MEDIUM: httpclient: Rework CLI I/O handler to handle full buffer cases
    - MEDIUM: httpclient: Don't close CLI applet at the end of a response
    - MEDIUM: http-ana: Always report rewrite failures as PRXCOND in logs
    - CLEANUP: Re-apply xalloc_size.cocci (2)
    - REGTESTS: abortonclose: Add a barrier to not mix up log messages
    - REGTESTS: http_request_buffer: Increase client timeout to wait "slow" clients
    - CLEANUP: ssl_ckch: Use corresponding enum for commit_cacrlfile_ctx.cafile_type
    - MINOR: ssl_ckch: Simplify I/O handler to commit changes on CA/CRL entry
    - BUG/MINOR: ssl_ckch: Use right type for old entry in show_crlfile_ctx
    - BUG/MINOR: ssl_ckch: Dump CRL transaction only once if show command yield
    - BUG/MINOR: ssl_ckch: Dump CA transaction only once if show command yield
    - BUG/MINOR: ssl_ckch: Dump cert transaction only once if show command yield
    - BUG/MINOR: ssl_ckch: Init right field when parsing "commit ssl crl-file" cmd
    - CLEANUP: ssl_ckch: Remove unused field in commit_cacrlfile_ctx structure
    - MINOR: ssl_ckch: Simplify structure used to commit changes on CA/CRL entries
    - MINOR: ssl_ckch: Remove service context for "set ssl cert" command
    - MINOR: ssl_ckch: Remove service context for "set ssl ca-file" command
    - MINOR: ssl_ckch: Remove service context for "set ssl crl-file" command
    - BUG/MINOR: ssl_ckch: Fix possible uninitialized value in show_cert I/O handler
    - BUG/MINOR: ssl_ckch: Fix possible uninitialized value in show_cafile I/O handler
    - BUG/MINOR: ssl_ckch: Fix possible uninitialized value in show_crlfile I/O handler
    - BUILD: ssl_ckch: Fix build error about a possible uninitialized value
    - BUG/MINOR: ssl_ckch: Fix another possible uninitialized value
    - REGTESTS: http_abortonclose: Extend supported versions
    - REGTESTS: restrict_req_hdr_names: Extend supported versions
    - MINOR: connection: support HTTP/3.0 for smp_*_http_major fetch
    - MINOR: h3: add h3c pointer into h3s instance
    - MINOR: mux-quic: simplify decode_qcs API
    - MINOR: mux-quic/h3: adjust demuxing function return values
    - BUG/MINOR: h3: fix return value on decode_qcs on error
    - BUILD: quic: fix anonymous union for gcc-4.4
    - BUILD: compiler: implement unreachable for older compilers too
    - DEV: tcploop: reorder options in the usage message
    - DEV: tcploop: make the current address the default address
    - DEV: tcploop: make it possible to change the target address of a connect()
    - DEV: tcploop: factor out the socket creation
    - DEV: tcploop: permit port 0 to ease handling of default options
    - DEV: tcploop: add a new "bind" command to bind to ip/port.
    - DEV: tcploop: add minimal UDP support
    - BUG/MINOR: trace: Test server existence for health-checks to get proxy
    - BUG/MINOR: checks: Properly handle email alerts in trace messages
    - BUG/MEDIUM: mailers: Set the object type for check attached to an email alert
    - REGTESTS: healthcheckmail: Update the test to be functionnal again
    - REGTESTS: healthcheckmail: Relax health-check failure condition
    - BUG/MINOR: h3: fix incorrect BUG_ON assert on SETTINGS parsing
    - MEDIUM: mux-h2: try to coalesce outgoing WINDOW_UPDATE frames
    - OPTIM: mux-h2: increase h2_settings_initial_window_size default to 64k
    - BUG/MINOR: h3: fix frame type definition
    - BUG/MEDIUM: h3: fix SETTINGS parsing
    - BUG/MINOR: cli/stats: add missing trailing LF after JSON outputs
    - BUG/MINOR: server: do not enable DNS resolution on disabled proxies
    - BUG/MINOR: cli/stats: add missing trailing LF after "show info json"
    - DOC: design: update the notes on thread groups
    - BUG/MEDIUM: mux-quic: fix flow control connection Tx level
    - MINOR: mux-quic: complete BUG_ON on TX flow-control enforcing
    - BUG/MINOR: mux-quic: fix memleak on frames rejected by transport
    - BUG/MINOR: tcp-rules: Make action call final on read error and delay expiration
    - CLEANUP: check: Remove useless tests on check's stream-connector
    - BUG/MEDIUM: stconn: Don't wakeup applet for send if it won't consume data
    - BUG/MEDIUM: cli: Notify cli applet won't consume data during request processing
    - BUG/MEDIUM: mux-quic: fix segfault on flow-control frame cleanup
    - MINOR: task: move profiling bit to per-thread
    - CLEANUP: quic: use task_new_on() for single-threaded tasks
    - MINOR: tinfo: remove the global thread ID bit (tid_bit)
    - CLEANUP: hlua: check for at least 2 threads on a task
    - MINOR: thread: get rid of MAX_THREADS_MASK
    - OPTIM: task: do not consult shared WQ when we're already full
    - DOC: design: update the task vs thread affinity requirements
    - MINOR: qpack: add comments and remove a useless trace
    - MINOR: qpack: reduce dependencies on other modules
    - BUG/MINOR: qpack: support header litteral name decoding
    - MINOR: qpack: add ABORT_NOW on unimplemented decoding
    - BUG/MINOR: h3/qpack: deal with too many headers
    - MINOR: qpack: improve decoding function
    - MINOR: qpack: implement standalone decoder tool
    - BUG/BUILD: h3: fix wrong label name
    - BUG/MINOR: quic: Stop hardcoding Retry packet Version field
    - MINOR: quic: Add several nonce and key definitions for Retry tag
    - BUG/MINOR: quic: Wrong PTO calculation
    - MINOR: quic: Parse long packet version from qc_parse_hd_form()
    - CLEANUP: quid: QUIC draft-28 no more supported
    - MEDIUM: quic: Add QUIC v2 draft support
    - MINOR: quic: Released QUIC TLS extension for QUIC v2 draft
    - MEDIUM: quic: Compatible version negotiation implementation (draft-08)
    - CLEANUP: quic: Remove any reference to boringssl
    - BUG/MINOR: task: fix thread assignment in tasklet_kill()
    - BUG/MEDIUM: stream: Properly handle destructive client connection upgrades
    - MINOR: stream: Rely on stconn flags to abort stream destructive upgrade
    - CLEANUP: stconn: Don't expect to have no sedesc on detach
    - BUG/MINOR: log: Properly test connection retries to fix dontlog-normal option
    - MINOR: hlua: don't dump empty entries in hlua_traceback()
    - MINOR: hlua: add a new hlua_show_current_location() function
    - MEDIUM: debug: add a tainted flag when a shared library is loaded
    - MEDIUM: debug: detect redefinition of symbols upon dlopen()
    - BUILD: quic: Wrong HKDF label constant variable initializations
    - BUG/MINOR: quic: Unexpected half open connection counter wrapping
    - BUG/MINOR: quic_stats: Duplicate "quic_streams_data_blocked_bidi" field name
    - BUG/MINOR: quic: purge conn Rx packet list on release
    - BUG/MINOR: quic: free rejected Rx packets
    - BUG/MINOR: qpack: abort on dynamic index field line decoding
    - BUG/MEDIUM: ssl/cli: crash when crt inserted into a crt-list
    - REGTESTS: ssl: add the same cert for client/server
    - BUG/MINOR: quic: Acknowledgement must be forced during handshake
    - MINOR: quic: Dump version_information transport parameter
    - BUG/MEDIUM: mworker: use default maxconn in wait mode
    - MINOR: intops: add a function to return a valid bit position from a mask
    - TESTS: add a unit test for one_among_mask()
    - BUILD: ssl_ckch: fix "maybe-uninitialized" build error on gcc-9.4 + ARM
    - BUG/MINOR: ssl: Do not look for key in extra files if already in pem
    - BUG/MINOR: quic: Missing acknowledgments for trailing packets
    - BUG/MINOR: http-ana: Set method to HTTP_METH_OTHER when an HTTP txn is created
    - BUG/MINOR: http-fetch: Use integer value when possible in "method" sample fetch
    - MINOR: freq_ctr: Add a function to get events excess over the current period
    - BUG/MINOR: stream: only free the req/res captures when set
    - CLEANUP: pool/tree-wide: remove suffix "_pool" from certain pool names
    - MEDIUM: debug: improve DEBUG_MEM_STATS to also report pool alloc/free
    - BUG/MINOR: quic: Wrong reuse of fulfilled dgram RX buffer
    - BUG/MAJOR: quic: Big RX dgrams leak when fulfilling a buffer
    - BUG/MAJOR: quic: Big RX dgrams leak with POST requests
    - BUILD: quic+h3: 32-bit compilation errors fixes
    - MEDIUM: bwlim: Add support of bandwith limitation at the stream level

3 years agoMEDIUM: bwlim: Add support of bandwith limitation at the stream level
Christopher Faulet [Wed, 22 Jun 2022 14:55:04 +0000 (16:55 +0200)] 
MEDIUM: bwlim: Add support of bandwith limitation at the stream level

This patch adds a filter to limit bandwith at the stream level. Several
filters can be defined. A filter may limit incoming data (upload) or
outgoing data (download). The limit can be defined per-stream or shared via
a stick-table. For a given stream, the bandwith limitation filters can be
enabled using the "set-bandwidth-limit" action.

A bandwith limitation filter can be used indifferently for HTTP or TCP
stream. For HTTP stream, only the payload transfer is limited. The filter is
pretty simple for now. But it was designed to be extensible. The current
design tries, as far as possible, to never exceed the limit. There is no
burst.

3 years agoBUILD: quic+h3: 32-bit compilation errors fixes
Frédéric Lécaille [Fri, 24 Jun 2022 10:13:53 +0000 (12:13 +0200)] 
BUILD: quic+h3: 32-bit compilation errors fixes

In GH #1760 (which is marked as being a feature), there were compilation
errors on MacOS which could be reproduced in Linux when building 32-bit code
(-m32 gcc option). Most of them were due to variables types mixing in QUIC_MIN macro
or using size_t type in place of uint64_t type.

Must be backported to 2.6.