]> git.ipfire.org Git - thirdparty/haproxy.git/log
thirdparty/haproxy.git
4 years agoMINOR: tasks: also compute the tasklet latency when DEBUG_TASK is set
Willy Tarreau [Thu, 25 Feb 2021 07:39:07 +0000 (08:39 +0100)] 
MINOR: tasks: also compute the tasklet latency when DEBUG_TASK is set

It is extremely useful to be able to observe the wakeup latency of some
important I/O operations, so let's accept to inflate the tasklet struct
by 8 extra bytes when DEBUG_TASK is set. With just this we have enough
to get live reports like this:

  $ socat - /tmp/sock1 <<< "show profiling"
  Per-task CPU profiling              : on      # set profiling tasks {on|auto|off}
  Tasks activity:
    function                      calls   cpu_tot   cpu_avg   lat_tot   lat_avg
    si_cs_io_cb                 8099492   4.833s    596.0ns   8.974m    66.48us
    h1_io_cb                    7460365   11.55s    1.548us   2.477m    19.92us
    process_stream              7383828   22.79s    3.086us   18.39m    149.5us
    h1_timeout_task                4157      -         -      348.4ms   83.81us
    srv_cleanup_toremove_connections751   39.70ms   52.86us   10.54ms   14.04us
    srv_cleanup_idle_connections     21   1.405ms   66.89us   30.82us   1.467us
    task_run_applet                  16   1.058ms   66.13us   446.2us   27.89us
    accept_queue_process              7   34.53us   4.933us   333.1us   47.58us

4 years agoMINOR: task: make grq_total atomic to move it outside of the grq_lock
Willy Tarreau [Thu, 25 Feb 2021 06:51:18 +0000 (07:51 +0100)] 
MINOR: task: make grq_total atomic to move it outside of the grq_lock

Instead of decrementing grq_total once per task picked from the global
run queue, let's do it at once after the loop like we do for other
counters. This simplifies the code everywhere. It is not expected to
bring noticeable improvements however, since global tasks tend to be
less common nowadays.

4 years agoCLEANUP: task: re-merge __task_unlink_rq() with task_unlink_rq()
Willy Tarreau [Thu, 25 Feb 2021 06:47:03 +0000 (07:47 +0100)] 
CLEANUP: task: re-merge __task_unlink_rq() with task_unlink_rq()

There's no point keeping the two separate anymore, some tests are
duplicated for no reason.

4 years agoMINOR: task: don't decrement then increment the local run queue
Willy Tarreau [Thu, 25 Feb 2021 06:19:45 +0000 (07:19 +0100)] 
MINOR: task: don't decrement then increment the local run queue

Now we don't need to decrement rq_total when we pick a tack in the tree
to immediately increment it again after installing it into the local
list. Instead, we simply add to the local queue count the number of
globally picked tasks. Avoiding this shows ~0.5% performance gains at
1Mreq/s (2M task switches/s).

4 years agoMINOR: task: do not use __task_unlink_rq() from process_runnable_tasks()
Willy Tarreau [Thu, 25 Feb 2021 06:14:58 +0000 (07:14 +0100)] 
MINOR: task: do not use __task_unlink_rq() from process_runnable_tasks()

As indicated in previous commit, this function tries to guess which tree
the task is in to figure what counters to update, while we already have
that info in the caller. Let's just pick the relevant parts to place them
in the caller.

4 years agoMINOR: task: split the counts of local and global tasks picked
Willy Tarreau [Thu, 25 Feb 2021 06:09:08 +0000 (07:09 +0100)] 
MINOR: task: split the counts of local and global tasks picked

In process_runnable_tasks() we're still calling __task_unlink_rq() to
pick a task, and this function tries to guess where to pick the task
from and which counter to update while the caller's context already
has everything. Worse, the number of local tasks is decremented then
recredited, doubling the operations. In order to avoid this we first
need to keep separate counters for local and global tasks that were
picked. This is what this patch does.

4 years agoBUG/MEDIUM: contrib/prometheus-exporter: fix segfault in listener name dump
William Dauchy [Wed, 24 Feb 2021 23:53:13 +0000 (00:53 +0100)] 
BUG/MEDIUM: contrib/prometheus-exporter: fix segfault in listener name dump

We need to check whether listener is empty before doing anything; in
that case, we were trying to dump listerner name while name is null. So
simply move the counters check above, which validate all possible cases
when the listener is empty. This is very similar to what is done in
stats.c

see also the trace:

  Thread 1 "haproxy" received signal SIGSEGV, Segmentation fault.
  __strlen_sse2 () at ../sysdeps/x86_64/multiarch/../strlen.S:120
  120     ../sysdeps/x86_64/multiarch/../strlen.S: No such file or directory.
  (gdb) bt
  #0  __strlen_sse2 () at ../sysdeps/x86_64/multiarch/../strlen.S:120
  #1  0x00005555555b716b in promex_dump_listener_metrics (htx=0x5555558fadf0, appctx=0x555555926070) at contrib/prometheus-exporter/service-prometheus.c:722
  #2  promex_dump_metrics (htx=0x5555558fadf0, si=0x555555925920, appctx=0x555555926070) at contrib/prometheus-exporter/service-prometheus.c:1200
  #3  promex_appctx_handle_io (appctx=0x555555926070) at contrib/prometheus-exporter/service-prometheus.c:1477
  #4  0x00005555556f0c94 in task_run_applet (t=0x555555926180, context=0x555555926070, state=<optimized out>) at src/applet.c:88
  #5  0x00005555556bc6d8 in run_tasks_from_lists (budgets=budgets@entry=0x7fffffffe374) at src/task.c:548
  #6  0x00005555556bd1a0 in process_runnable_tasks () at src/task.c:750
  #7  0x0000555555696cdd in run_poll_loop () at src/haproxy.c:2870
  #8  0x0000555555697025 in run_thread_poll_loop (data=data@entry=0x0) at src/haproxy.c:3035
  #9  0x0000555555596c90 in main (argc=<optimized out>, argv=0x7fffffffe818) at src/haproxy.c:3723
  quit)

this bug was introduced by commit
e3f7bd5ae9e969cbfe87e4130d06bff7a3e814c6 ("MEDIUM:
contrib/prometheus-exporter: add listen stats"), which is present for
2.4 only, so no backport needed.

Signed-off-by: William Dauchy <wdauchy@gmail.com>
4 years agoDOC: Update the filters guide
Christopher Faulet [Wed, 24 Feb 2021 20:58:43 +0000 (21:58 +0100)] 
DOC: Update the filters guide

The filters guide was totally outdated. Callbacks to filter payload were
changed, especially the HTTP one because of the HTX. All the HTTP legacy
part is removed. This new guide now reflects the reality.

This patch may be backported as far as 2.2.

4 years agoDOC: Update the HTX API documentation
Christopher Faulet [Wed, 24 Feb 2021 10:33:21 +0000 (11:33 +0100)] 
DOC: Update the HTX API documentation

Missing functions have been added. And because the EOM block was removed,
some parts have been adapted to better explain how the end of the message
may be detected.

4 years agoMINOR: htx: Add function to reserve the max possible size for an HTX DATA block
Christopher Faulet [Wed, 3 Feb 2021 11:11:31 +0000 (12:11 +0100)] 
MINOR: htx: Add function to reserve the max possible size for an HTX DATA block

The function htx_reserve_max_data() should be used to get an HTX DATA block
with the max possible size. A current block may be extended or a new one
created, depending on the HTX message state. But the idea is to let the
caller to copy a bunch of data without requesting many new blocks. It is its
responsibility to resize the block at the end, to set the final block size.

This function will be used to parse messages with small chunks. Indeed, we
can have more than 2700 1-byte chunks in a 16Kb of input data. So it is easy
to understand how this function may help to improve the parsing of chunk
messages.

4 years agoDOC: Update the module list in MAINTAINERS file
Christopher Faulet [Tue, 23 Feb 2021 16:51:02 +0000 (17:51 +0100)] 
DOC: Update the module list in MAINTAINERS file

Some missing modules have been added and some others have been updated. The
list is now sorted. It is a bit easier to find something. In addition the
path of files have been updated to reflect recent changes.

4 years agoBUG/MEDIUM: resolvers: Reset address for unresolved servers
Christopher Faulet [Tue, 23 Feb 2021 11:33:17 +0000 (12:33 +0100)] 
BUG/MEDIUM: resolvers: Reset address for unresolved servers

If the DNS resolution failed for a server, its ip address must be
removed. Otherwise, the server is stopped but keeps its ip. This may be
confusing when the servers state are retrieved on the CLI and it may lead to
undefined behavior if HAproxy is configured to load its servers state from a
file.

This patch should be backported as far as 2.0.

4 years agoBUG/MEDIUM: resolvers: Reset server address and port for obselete SRV records
Christopher Faulet [Tue, 23 Feb 2021 11:24:09 +0000 (12:24 +0100)] 
BUG/MEDIUM: resolvers: Reset server address and port for obselete SRV records

When a SRV record expires, the ip/port assigned to the associated server are
now removed. Otherwise, the server is stopped but keeps its ip/port while
the server hostname is removed. It is confusing when the servers state are
retrieve on the CLI and may be a problem if saved in a server-state
file. Because the reload may fail because of this inconsistency.

Here is an example:

 * Declare a server template in a backend, using the resolver <dns>

server-template test 2 _http._tcp.example.com resolvers dns check

 * 2 SRV records are announced with the corresponding additional
   records. Thus, 2 servers are filled. Here is the "show servers state"
   output :

2 frt 1 test1 192.168.1.1 2 64 0 1 2 15 3 4 6 0 0 0 http1.example.com 8001 _http._tcp.example.com 0 0 - - 0
2 frt 2 test2 192.168.1.2 2 64 0 1 1 15 3 4 6 0 0 0 http2.example.com 8002 _http._tcp.example.com 0 0 - - 0

 * Then, one additional record is removed (or a SRV record is removed, the
   result is the same). Here is the new "show servers state" output :

2 frt 1 test1 192.168.1.1 2 64 0 1 38 15 3 4 6 0 0 0 http1.example.com 8001 _http._tcp.example.com 0 0 - - 0
2 frt 2 test2 192.168.1.2 0 96 0 1 19 15 3 0 14 0 0 0 - 8002 _http._tcp.example.com 0 0 - - 0

On reload, if a server-state file is used, this leads to undefined behaviors
depending on the configuration.

This patch should be backported as far as 2.0.

4 years agoBUG/MINOR: resolvers: new callback to properly handle SRV record errors
Baptiste Assmann [Thu, 19 Nov 2020 21:38:33 +0000 (22:38 +0100)] 
BUG/MINOR: resolvers: new callback to properly handle SRV record errors

When a SRV record was created, it used to register the regular server name
resolution callbacks. That said, SRV records and regular server name
resolution don't work the same way, furthermore on error management.

This patch introduces a new call back to manage DNS errors related to
the SRV queries.

this fixes github issue #50.

Backport status: 2.3, 2.2, 2.1, 2.0

4 years agoBUG/MINOR: resolvers: Only renew TTL for SRV records with an additional record
Christopher Faulet [Tue, 23 Feb 2021 11:22:29 +0000 (12:22 +0100)] 
BUG/MINOR: resolvers: Only renew TTL for SRV records with an additional record

If no additional record is associated to a SRV record, its TTL must not be
renewed. Otherwise the entry never expires. Thus once announced a first
time, the entry remains blocked on the same IP/port except if a new announce
replaces the old one.

Now, the TTL is updated if a SRV record is received while a matching
existing one is found with an additional record or when an new additional
record is assigned to an existing SRV record.

This patch should be backported as far as 2.2.

4 years agoBUG/MINOR: resolvers: Fix condition to release received ARs if not assigned
Christopher Faulet [Tue, 23 Feb 2021 10:59:19 +0000 (11:59 +0100)] 
BUG/MINOR: resolvers: Fix condition to release received ARs if not assigned

At the end of resolv_validate_dns_response(), if a received additionnal
record is not assigned to an existing server record, it is released. But the
condition to do so is buggy. If "answer_record" (the received AR) is not
assigned, "tmp_record" is not a valid record object. It is just a dummy
record "representing" the head of the record list.

Now, the condition is far cleaner. This patch must be backported as far as
2.2.

4 years agoBUG/MINOR: fd: properly wait for !running_mask in fd_set_running_excl()
Willy Tarreau [Wed, 24 Feb 2021 18:40:49 +0000 (19:40 +0100)] 
BUG/MINOR: fd: properly wait for !running_mask in fd_set_running_excl()

In fd_set_running_excl() we don't reset the old mask in the CAS loop,
so if we fail on the first round, we'll forcefully take the FD on the
next one.

In practice it's used bu fd_insert() and fd_delete() only, none of which
is supposed to be passed an FD which is still in use since in practice,
given that for now only listeners may be enabled on multiple threads at
once.

This can be backported to 2.2 but shouldn't result in fixing any user
visible bug for now.

4 years agoCLEANUP: task: split the large tasklet_wakeup_on() function in two
Willy Tarreau [Wed, 24 Feb 2021 16:51:38 +0000 (17:51 +0100)] 
CLEANUP: task: split the large tasklet_wakeup_on() function in two

This function has become large with the multi-queue scheduler. We need
to keep the fast path and the debugging parts inlined, but the rest now
moves to task.c just like was done for task_wakeup(). This has reduced
the code size by 6kB due to less inlining of large parts that are always
context-dependent, and as a side effect, has increased the overall
performance by 1%.

4 years agoMINOR: task: move the allocated tasks counter to the per-thread struct
Willy Tarreau [Wed, 24 Feb 2021 16:03:30 +0000 (17:03 +0100)] 
MINOR: task: move the allocated tasks counter to the per-thread struct

The nb_tasks counter was still global and gets incremented and decremented
for each task_new()/task_free(), and was read in process_runnable_tasks().
But it's only used for stats reporting, so doing this this often is
pointless and expensive. Let's move it to the task_per_thread struct and
have the stats sum it when needed.

4 years agoMINOR: task: limit the remote thread wakeup to the global runqueue only
Willy Tarreau [Wed, 24 Feb 2021 15:44:51 +0000 (16:44 +0100)] 
MINOR: task: limit the remote thread wakeup to the global runqueue only

The test in __task_wakeup() to figure if the remote threads are sleeping
doesn't make sense outside of the global runqueue test, since there are
only two possibilities here: local runqueue or global runqueue, hence a
sleeping thread is another one and can only happen when sending to the
global run queue. Let's move the test inside the "if" block.

4 years agoCLEANUP: task: move the tree root detection from __task_wakeup() to task_wakeup()
Willy Tarreau [Wed, 24 Feb 2021 15:41:11 +0000 (16:41 +0100)] 
CLEANUP: task: move the tree root detection from __task_wakeup() to task_wakeup()

Historically we used to call __task_wakeup() with a known tree root but
this is not the case and the code has remained needlessly complicated
with the root calculation in task_wakeup() passed in argument to
__task_wakeup() which compares it again.

Let's get rid of this and just move the detection code there. This
eliminates some ifdefs and allows to simplify the test conditions quite
a bit.

4 years agoCLEANUP: tasks: use a less confusing name for task_list_size
Willy Tarreau [Wed, 24 Feb 2021 13:13:40 +0000 (14:13 +0100)] 
CLEANUP: tasks: use a less confusing name for task_list_size

This one is systematically misunderstood due to its unclear name. It
is in fact the number of tasks in the local tasklet list. Let's call
it "tasks_in_list" to remove some of the confusion.

4 years agoMINOR: tasks: do not maintain the rqueue_size counter anymore
Willy Tarreau [Wed, 24 Feb 2021 15:13:03 +0000 (16:13 +0100)] 
MINOR: tasks: do not maintain the rqueue_size counter anymore

This one is exclusively used as a boolean nowadays and is non-zero only
when the thread-local run queue is not empty. Better check the root tree's
pointer and avoid updating this counter all the time.

4 years agoMEDIUM: task: remove the tasks_run_queue counter and have one per thread
Willy Tarreau [Wed, 24 Feb 2021 14:10:07 +0000 (15:10 +0100)] 
MEDIUM: task: remove the tasks_run_queue counter and have one per thread

This counter is solely used for reporting in the stats and is the hottest
thread contention point to date. Moving it to the scheduler and having a
separate one for the global run queue dramatically improves the performance,
showing a 12% boost on the request rate on 16 threads!

In addition, the thread debugging output which used to rely on rqueue_size
was not totally accurate as it would only report task counts. Now we can
return the exact thread's run queue length.

It is also interesting to note that there are still a few other task/tasklet
counters in the scheduler that are not efficiently updated because some cover
a single area and others cover multiple areas. It looks like having a distinct
counter for each of the following entries would help and would keep the code
a bit cleaner:
  - global run queue (tree)
  - per-thread run queue (tree)
  - per-thread shared tasklets list
  - per-thread local lists

Maybe even splitting the shared tasklets lists between pure tasklets and
tasks instead of having the whole and tasks would simplify the code because
there remain a number of places where several counters have to be updated.

4 years agoBUILD: dns: avoid a build warning when threads are disabled (dss unused)
Willy Tarreau [Wed, 24 Feb 2021 16:38:46 +0000 (17:38 +0100)] 
BUILD: dns: avoid a build warning when threads are disabled (dss unused)

dns_session_release() only uses its struct dns_stream_server to access
the lock, so a warning is emitted when threads are disabled. Let's mark
it __maybe_unused.

4 years agoMEDIUM: streams: do not use the streams lock anymore
Willy Tarreau [Wed, 24 Feb 2021 12:46:12 +0000 (13:46 +0100)] 
MEDIUM: streams: do not use the streams lock anymore

The lock was still used exclusively to deal with the concurrency between
the "show sess" release handler and a stream_new() or stream_free() on
another thread. All other accesses made by "show sess" are already done
under thread isolation. The release handler only requires to unlink its
node when stopping in the middle of a dump (error, timeout etc). Let's
just isolate the thread to deal with this case so that it's compatible
with the dump conditions, and remove all remaining locking on the streams.

This effectively kills the streams lock. The measured gain here is around
1.6% with 4 threads (374krps -> 380k).

4 years agoMINOR: streams: use one list per stream instead of a global one
Willy Tarreau [Wed, 24 Feb 2021 09:37:01 +0000 (10:37 +0100)] 
MINOR: streams: use one list per stream instead of a global one

The global streams list is exclusively used for "show sess", to look up
a stream to shut down, and for the hard-stop. Having all of them in a
single list is extremely expensive in terms of locking when using threads,
with performance losses as high as 7% having been observed just due to
this.

This patch makes the list per-thread, since there's no need to have a
global one in this situation. All call places just iterate over all
threads. The most "invasive" changes was in "show sess" where the end
of list needs to go back to the beginning of next thread's list until
the last thread is seen. For now the lock was maintained to keep the
code auditable but a next commit should get rid of it.

The observed performance gain here with only 4 threads is already 7%
(350krps -> 374krps).

4 years agoMINOR: cli/streams: make "show sess" dump all streams till the new epoch
Willy Tarreau [Wed, 24 Feb 2021 10:53:17 +0000 (11:53 +0100)] 
MINOR: cli/streams: make "show sess" dump all streams till the new epoch

Instead of placing the current stream at the end of the stream list when
issuing a "show sess" on the CLI as was done in 2.2 with commit c6e7a1b8e
("MINOR: cli: make "show sess" stop at the last known session"), now we
compare the listed stream's epoch with the dumping stream's and stop on
more recent ones.

This way we're certain to always only dump known streams at the moment we
issue the dump command without having to modify the list. In theory we
could miss some streams if more than 2^31 "show sess" requests are issued
while an old stream remains present, but that's 68 years at 1 "show sess"
per second and it's unlikely we'll keep a process, let alone a stream, that
long.

It could be verified that the count of dumped streams still matches the
one before this change.

4 years agoMINOR: stream: add an "epoch" to figure which streams appeared when
Willy Tarreau [Wed, 24 Feb 2021 10:29:51 +0000 (11:29 +0100)] 
MINOR: stream: add an "epoch" to figure which streams appeared when

The "show sess" CLI command currently lists all streams and needs to
stop at a given position to avoid dumping forever. Since 2.2 with
commit c6e7a1b8e ("MINOR: cli: make "show sess" stop at the last known
session"), a hack consists in unlinking the stream running the applet
and linking it again at the current end of the list, in order to serve
as a delimiter. But this forces the stream list to be global, which
affects scalability.

This patch introduces an epoch, which is a global 32-bit counter that
is incremented by the "show sess" command, and which is copied by newly
created streams. This way any stream can know whether any other one is
newer or older than itself.

For now it's only stored and not exploited.

4 years agoBUG/MINOR: proxy: wake up all threads when sending the hard-stop signal
Willy Tarreau [Wed, 24 Feb 2021 10:13:59 +0000 (11:13 +0100)] 
BUG/MINOR: proxy: wake up all threads when sending the hard-stop signal

The hard-stop event didn't wake threads up. In the past it wasn't an issue
as the poll timeout was limited to 1 second, but since commit 4f59d3861
("MINOR: time: increase the minimum wakeup interval to 60s") it has become
a problem because old processes can remain live for up to one minute after
the hard-stop-after delay. Let's just wake them up.

This may be backported to older releases, though before 2.4 the extra
delay was only one second.

4 years agoBUG/MEDIUM: cli/shutdown sessions: make it thread-safe
Willy Tarreau [Wed, 24 Feb 2021 10:11:06 +0000 (11:11 +0100)] 
BUG/MEDIUM: cli/shutdown sessions: make it thread-safe

There's no locking around the lookup of a stream nor its shutdown
when issuing "shutdown sessions" over the CLI so the risk of crashing
the process is particularly high.

Let's use a thread_isolate() there which is suitable for this task, and
there are not that many alternatives.

This must be backported to 1.8.

4 years agoBUG/MEDIUM: proxy: use thread-safe stream killing on hard-stop
Willy Tarreau [Wed, 24 Feb 2021 10:08:56 +0000 (11:08 +0100)] 
BUG/MEDIUM: proxy: use thread-safe stream killing on hard-stop

When setting hard-stop-after, hard_stop() is called at the end to kill
last pending streams. Unfortunately there's no locking there while
walking over the streams list nor when shutting them down, so it's
very likely that some old processes have been crashing or gone wild
due to this. Let's use a thread_isolate() call for this as we don't
have much other choice (and it happens once in the process' life,
that's OK).

This must be backported to 1.8.

4 years agoDOC: muxes: add a diagram of the exchanges between muxes and outer world
Willy Tarreau [Wed, 24 Feb 2021 08:07:52 +0000 (09:07 +0100)] 
DOC: muxes: add a diagram of the exchanges between muxes and outer world

Since the muxes API is far from being obvious, let's show a stream being
forwarded between two sides through muxes with their buffers and the
transport layers. The diagram is provided in .fig, .svg, .png, and .pdf.

4 years agoCLEANUP: vars: make smp_fetch_var() to reuse vars_get_by_desc()
Dragan Dosen [Mon, 22 Feb 2021 16:39:02 +0000 (17:39 +0100)] 
CLEANUP: vars: make smp_fetch_var() to reuse vars_get_by_desc()

They both do the same thing, so let's remove unneeded code duplication.

4 years agoBUG/MEDIUM: vars: make functions vars_get_by_{name,desc} thread-safe
Dragan Dosen [Mon, 22 Feb 2021 16:20:01 +0000 (17:20 +0100)] 
BUG/MEDIUM: vars: make functions vars_get_by_{name,desc} thread-safe

This patch adds a lock to functions vars_get_by_name() and
vars_get_by_desc() to protect accesses to the list of variables.

After the variable is fetched, a sample data is duplicated by using
smp_dup() because the variable may be modified by another thread.

This should be backported to all versions supporting vars along with
"BUG/MINOR: sample: secure convs that accept base64 string and var name
as args" which this patch depends on.

4 years agoBUG/MINOR: sample: secure convs that accept base64 string and var name as args
Dragan Dosen [Mon, 22 Feb 2021 09:03:53 +0000 (10:03 +0100)] 
BUG/MINOR: sample: secure convs that accept base64 string and var name as args

This patch adds a few improvements in order to secure the use of
converters that accept base64 string and variable name as arguments.

The first change is within related function sample_conv_var2smp_str()
which now flags the sample as SMP_F_CONST if the argument is of type
ARGT_STR. This makes the sample more safe for later use.

A new function sample_check_arg_base64() is added. It checks an argument
and fills it with a variable type if the argument string contains a
valid variable name. If failed, it tries to perform a base64 decode
operation on a non-empty string, and fills the argument with the decoded
content which can be used later, without any additional base64dec()
function calls during runtime. This means that haproxy configuration
check may fail if variable lookup fails and an invalid base64 encoded
string is specified as an argument for such converters.

Both converters, "aes_gcm_dec" and "hmac", now use alloc_trash_chunk()
in order to allocate additional buffers for various conversions, and
avoid the use of a pre-allocated trash chunks directly (usually returned
by get_trash_chunk()). The function sample_check_arg_base64() is used
for both converters in order to check their arguments specified within
the haproxy configuration.

This patch should be backported as far as 2.0. However, it is important
to keep in mind a few things. The "hmac" converter is only available
starting with 2.2. In versions prior to 2.2, the "aes_gcm_dec" converter
and sample_conv_var2smp_str() are implemented in src/ssl_sock.c. Thus
the patch will have to be adapted on these versions.

Note that this patch is required for a subsequent, more important fix.

4 years agoBUG/MINOR: ssl/cli: potential null pointer dereference in "set ssl cert"
William Lallemand [Tue, 23 Feb 2021 13:45:45 +0000 (14:45 +0100)] 
BUG/MINOR: ssl/cli: potential null pointer dereference in "set ssl cert"

A potential null pointer dereference was reported with an old gcc
version (6.5)

    src/ssl_ckch.c: In function 'cli_parse_set_cert':
    src/ssl_ckch.c:838:7: error: potential null pointer dereference [-Werror=null-dereference]
      if (!ssl_sock_copy_cert_key_and_chain(src->ckch, dst->ckch))
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    src/ssl_ckch.c:838:7: error: potential null pointer dereference [-Werror=null-dereference]
    src/ssl_ckch.c: In function 'ckchs_dup':
    src/ssl_ckch.c:838:7: error: potential null pointer dereference [-Werror=null-dereference]
      if (!ssl_sock_copy_cert_key_and_chain(src->ckch, dst->ckch))
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    src/ssl_ckch.c:838:7: error: potential null pointer dereference [-Werror=null-dereference]
    cc1: all warnings being treated as errors

This case does not actually happen but it's better to fix the ckch API
with a NULL check.

Could be backported as far as 2.1.

4 years agoMINOR: Configure the `cpp` userdiff driver for *.[ch] in .gitattributes
Tim Duesterhus [Sat, 20 Feb 2021 18:21:35 +0000 (19:21 +0100)] 
MINOR: Configure the `cpp` userdiff driver for *.[ch] in .gitattributes

This might improve the output of `git diff` in certain cases. Especially
`git diff --word-diff` will be much more useful.

Does not affect the generated code, may be backported for consistency if
desired.

4 years agoBUILD: SSL: introduce fine guard for RAND_keep_random_devices_open
Ilya Shipitsin [Fri, 19 Feb 2021 18:42:53 +0000 (23:42 +0500)] 
BUILD: SSL: introduce fine guard for RAND_keep_random_devices_open

RAND_keep_random_devices_open is OpenSSL specific function, not
implemented in LibreSSL and BoringSSL. Let us define guard
HAVE_SSL_RAND_KEEP_RANDOM_DEVICES_OPEN in include/haproxy/openssl-compat.h
That guard does not depend anymore on HA_OPENSSL_VERSION

4 years ago[RELEASE] Released version 2.4-dev9 v2.4-dev9
Willy Tarreau [Sat, 20 Feb 2021 12:30:31 +0000 (13:30 +0100)] 
[RELEASE] Released version 2.4-dev9

Released version 2.4-dev9 with the following main changes :
    - BUG/MINOR: server: Remove RMAINT from admin state when loading server state
    - CLEANUP: check: fix get_check_status_info declaration
    - CLEANUP: contrib/prometheus-exporter: align for with srv status case
    - MEDIUM: stats: allow to select one field in `stats_fill_li_stats`
    - MINOR: stats: add helper to get status string
    - MEDIUM: contrib/prometheus-exporter: add listen stats
    - BUG/MINOR: dns: add test on result getting value from buffer into ring.
    - BUG/MINOR: dns: dns_connect_server must return -1 unsupported nameserver's type
    - BUG/MINOR: dns: missing test writing in output channel in session handler
    - BUG/MINOR: dns: fix ring attach control on dns_session_new
    - BUG/MEDIUM: dns: fix multiple double close on fd in dns.c
    - BUG/MAJOR: connection: prevent double free if conn selected for removal
    - BUG/MINOR: session: atomically increment the tracked sessions counter
    - REGTESTS: fix http_reuse_conn_hash proxy test
    - BUG/MINOR: backend: do not call smp_make_safe for sni conn hash
    - MINOR: connection: remove pointers for prehash in conn_hash_params
    - BUG/MINOR: checks: properly handle wrapping time in __health_adjust()
    - BUG/MEDIUM: checks: don't needlessly take the server lock in health_adjust()
    - DEBUG: thread: add 5 extra lock labels for statistics and debugging
    - OPTIM: server: switch the actconn list to an mt-list
    - Revert "MINOR: threads: change lock_t to an unsigned int"
    - MINOR: lb/api: let callers of take_conn/drop_conn tell if they have the lock
    - OPTIM: lb-first: do not take the server lock on take_conn/drop_conn
    - OPTIM: lb-leastconn: do not take the server lock on take_conn/drop_conn
    - OPTIM: lb-leastconn: do not unlink the server if it did not change
    - MINOR: tasks: add DEBUG_TASK to report caller info in a task
    - MINOR: tasks/debug: add some extra controls of use-after-free in DEBUG_TASK
    - BUG/MINOR: sample: Always consider zero size string samples as unsafe
    - MINOR: cli: add missing agent commands for set server
    - BUILD/MEDIUM: da Adding pcre2 support.
    - BUILD: ssl: introduce fine guard for OpenSSL specific SCTL functions
    - REGTESTS: reorder reuse conn proxy protocol test
    - DOC: explain the relation between pool-low-conn and tune.idle-pool.shared
    - MINOR: tasks: refine the default run queue depth
    - MINOR: listener: refine the default MAX_ACCEPT from 64 to 4
    - MINOR: mux_h2: do not try to remove front conn from idle trees
    - REGTESTS: workaround for a crash with recent libressl on http-reuse sni
    - BUG/MEDIUM: lists: Avoid an infinite loop in MT_LIST_TRY_ADDQ().
    - MINOR: connection: allocate dynamically hash node for backend conns
    - DOC: DeviceAtlas documentation typo fix.
    - BUG/MEDIUM: spoe: Resolve the sink if a SPOE logs in a ring buffer
    - BUG/MINOR: http-rules: Always replace the response status on a return action
    - BUG/MINOR: server: Init params before parsing a new server-state line
    - BUG/MINOR: server: Be sure to cut the last parsed field of a server-state line
    - MEDIUM: server: Don't introduce a new server-state file version
    - DOC: contrib/prometheus-exporter: remove htx reference
    - REGTESTS: contrib/prometheus-exporter: test NaN values
    - REGTESTS: contrib/prometheus-exporter: test well known labels
    - CI: github actions: switch to stable LibreSSL release
    - BUG/MINOR: server: Fix test on number of fields allowed in a server-state line
    - MINOR: dynbuf: make the buffer wait queue per thread
    - MINOR: dynbuf: use regular lists instead of mt_lists for buffer_wait
    - MINOR: dynbuf: pass offer_buffers() the number of buffers instead of a threshold
    - MINOR: sched: have one runqueue ticks counter per thread

4 years agoMINOR: sched: have one runqueue ticks counter per thread
Willy Tarreau [Sat, 20 Feb 2021 11:49:54 +0000 (12:49 +0100)] 
MINOR: sched: have one runqueue ticks counter per thread

The runqueue_ticks counts the number of task wakeups and is used to
position new tasks in the run queue, but since we've had per-thread
run queues, the values there are not very relevant anymore and the
nice value doesn't apply well if some threads are more loaded than
others. In addition, letting all threads compete over a shared counter
is not smart as this may cause some excessive contention.

Let's move this index close to the run queues themselves, i.e. one per
thread and a global one. In addition to improving fairness, this has
increased global performance by 2% on 16 threads thanks to the lower
contention on rqueue_ticks.

Fairness issues were not observed, but if any were to be, this patch
could be backported as far as 2.0 to address them.

4 years agoMINOR: dynbuf: pass offer_buffers() the number of buffers instead of a threshold
Willy Tarreau [Sat, 20 Feb 2021 11:02:46 +0000 (12:02 +0100)] 
MINOR: dynbuf: pass offer_buffers() the number of buffers instead of a threshold

Historically this function would try to wake the most accurate number of
process_stream() waiters. But since the introduction of filters which could
also require buffers (e.g. for compression), things started not to be as
accurate anymore. Nowadays muxes and transport layers also use buffers, so
the runqueue size has nothing to do anymore with the number of supposed
users to come.

In addition to this, the threshold was compared to the number of free buffer
calculated as allocated minus used, but this didn't work anymore with local
pools since these counts are not updated upon alloc/free!

Let's clean this up and pass the number of released buffers instead, and
consider that each waiter successfully called counts as one buffer. This
is not rocket science and will not suddenly fix everything, but at least
it cannot be as wrong as it is today.

This could have been marked as a bug given that the current situation is
totally broken regarding this, but this probably doesn't completely fix
it, it only goes in a better direction. It is possible however that it
makes sense in the future to backport this as part of a larger series if
the situation significantly improves.

4 years agoMINOR: dynbuf: use regular lists instead of mt_lists for buffer_wait
Willy Tarreau [Sat, 20 Feb 2021 10:49:49 +0000 (11:49 +0100)] 
MINOR: dynbuf: use regular lists instead of mt_lists for buffer_wait

There's no point anymore in keeping mt_lists for the buffer_wait and
buffer_wq since it's thread-local now.

4 years agoMINOR: dynbuf: make the buffer wait queue per thread
Willy Tarreau [Sat, 20 Feb 2021 10:38:56 +0000 (11:38 +0100)] 
MINOR: dynbuf: make the buffer wait queue per thread

The buffer wait queue used to be global historically but this doest not
make any sense anymore given that the most common use case is to have
thread-local pools. Thus there's no point waking up waiters of other
threads after releasing an entry, as they won't benefit from it.

Let's move the queue head to the thread_info structure and use
ti->buffer_wq from now on.

4 years agoBUG/MINOR: server: Fix test on number of fields allowed in a server-state line
Christopher Faulet [Sat, 20 Feb 2021 11:15:22 +0000 (12:15 +0100)] 
BUG/MINOR: server: Fix test on number of fields allowed in a server-state line

When a server-state line is parsed, a test is performed to be sure there is
enough but not too much fields. However the test is buggy. The bug was
introduced in the commit ea2cdf55e ("MEDIUM: server: Don't introduce a new
server-state file version").

No backport needed.

4 years agoCI: github actions: switch to stable LibreSSL release
Ilya Shipitsin [Fri, 19 Feb 2021 07:09:31 +0000 (12:09 +0500)] 
CI: github actions: switch to stable LibreSSL release

LibreSSL-3.3.0 appeared to have its own bugs, it is development release,
let us switch to stable LibreSSL-3.2.4 instead

4 years agoREGTESTS: contrib/prometheus-exporter: test well known labels
William Dauchy [Thu, 18 Feb 2021 22:05:34 +0000 (23:05 +0100)] 
REGTESTS: contrib/prometheus-exporter: test well known labels

as we previously briefly broke labels handling, test them to make sure
we don't introduce regressions in the future.
see also commit 040b1195f70d6a24204ede081451fd1dd71e6a34 ("BUG/MINOR:
contrib/prometheus-exporter: Restart labels dump at the right pos") for
reference

Signed-off-by: William Dauchy <wdauchy@gmail.com>
4 years agoREGTESTS: contrib/prometheus-exporter: test NaN values
William Dauchy [Thu, 18 Feb 2021 22:05:33 +0000 (23:05 +0100)] 
REGTESTS: contrib/prometheus-exporter: test NaN values

In order to make sure we detect when we change default behaviour for
some metrics, test the NaN value when it is expected.

Those metrics were listed since our last rework as their default value
changed, unless the appropriate config is set.

Signed-off-by: William Dauchy <wdauchy@gmail.com>
4 years agoDOC: contrib/prometheus-exporter: remove htx reference
William Dauchy [Thu, 18 Feb 2021 22:05:32 +0000 (23:05 +0100)] 
DOC: contrib/prometheus-exporter: remove htx reference

now that htx is the default everywhere, we can remove the need to put
htx as a mandatory option to setup prometheus.

Signed-off-by: William Dauchy <wdauchy@gmail.com>
4 years agoMEDIUM: server: Don't introduce a new server-state file version
Christopher Faulet [Fri, 19 Feb 2021 11:10:36 +0000 (12:10 +0100)] 
MEDIUM: server: Don't introduce a new server-state file version

This revert the commit 63e6cba12 ("MEDIUM: server: add server-states version
2"), but keeping all recent features added to the server-sate file. Instead
of adding a 2nd version for the server-state file format to handle the 5 new
fields added during the 2.4 development, these fields are considered as
optionnal during the parsing. So it is possible to load a server-state file
from HAProxy 2.3. However, from 2.4, these new fields are always dumped in
the server-state file. But it should not be a problem to load it on the 2.3.

This patch seems a bit huge but the diff ignoring the space is much smaller.

The version 2 of the server-state file format is reserved for a real
refactoring to address all issues of the current format.

4 years agoBUG/MINOR: server: Be sure to cut the last parsed field of a server-state line
Christopher Faulet [Fri, 19 Feb 2021 15:57:20 +0000 (16:57 +0100)] 
BUG/MINOR: server: Be sure to cut the last parsed field of a server-state line

If a line of a server-state file has too many fields, the last one is not
cut on the first following space, as all other fileds. It contains all the
end of the line. It is not the expected behavior. So, now, we cut it on the
next following space, if any. The parsing loop was slighly rewritten.

Note that for now there is no error reported if the line is too long.

This patch may be backported at least as far as 2.1. On 2.0 and prior the
code is not the same. The line parsing is inlined in apply_server_state()
function.

4 years agoBUG/MINOR: server: Init params before parsing a new server-state line
Christopher Faulet [Fri, 19 Feb 2021 15:47:11 +0000 (16:47 +0100)] 
BUG/MINOR: server: Init params before parsing a new server-state line

Same static arrays of parameters are used to parse all server-state
lines. Thus it is important to reinit them to be sure to not get params from
the previous line, eventually from the previous loaded file.

This patch should be backported to all stable branches. However, in 2.0 and
prior, the parsing of server-state lines are inlined in apply_server_state()
function. Thus the patch will have to be adapted on these versions.

4 years agoBUG/MINOR: http-rules: Always replace the response status on a return action
Christopher Faulet [Fri, 19 Feb 2021 10:41:01 +0000 (11:41 +0100)] 
BUG/MINOR: http-rules: Always replace the response status on a return action

When a HTTP return action is triggered, HAProxy is responsible to return the
response, based on the configured status code. On the request side, there is
no problem because there is no server response to replace. But on the
response side, we must take care to override the server response status
code, if any, to be sure to use the rigth status code to get the http reply
message.

In short, we must always set the configured status code of the HTTP return
action before returning the http reply to be sure to get the right reply,
the one base on the http return action status code and not a reply based on
the server response status code..

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

4 years agoBUG/MEDIUM: spoe: Resolve the sink if a SPOE logs in a ring buffer
Christopher Faulet [Fri, 19 Feb 2021 09:56:41 +0000 (10:56 +0100)] 
BUG/MEDIUM: spoe: Resolve the sink if a SPOE logs in a ring buffer

If a SPOE filter is configured to send its logs to a ring buffer, the
corresponding sink must be resolved during the configuration post
parsing. Otherwise, the sink is undefined when a log message is emitted,
crashing HAProxy.

This patch must be backported as far as 2.2.

4 years agoDOC: DeviceAtlas documentation typo fix.
David Carlier [Fri, 19 Feb 2021 12:01:38 +0000 (12:01 +0000)] 
DOC: DeviceAtlas documentation typo fix.

The USE_PCRE syntax was incorrect. No backport needed.

4 years agoMINOR: connection: allocate dynamically hash node for backend conns
Amaury Denoyelle [Fri, 19 Feb 2021 14:29:16 +0000 (15:29 +0100)] 
MINOR: connection: allocate dynamically hash node for backend conns

Remove ebmb_node entry from struct connection and create a dedicated
struct conn_hash_node. struct connection contains now only a pointer to
a conn_hash_node, allocated only for connections where target is of type
OBJ_TYPE_SERVER. This will reduce memory footprints for every
connections that does not need http-reuse such as frontend connections.

4 years agoBUG/MEDIUM: lists: Avoid an infinite loop in MT_LIST_TRY_ADDQ().
Olivier Houchard [Thu, 18 Feb 2021 22:55:30 +0000 (23:55 +0100)] 
BUG/MEDIUM: lists: Avoid an infinite loop in MT_LIST_TRY_ADDQ().

In MT_LIST_TRY_ADDQ(), deal with the "prev" field of the element before the
"next". If the element is the first in the list, then its next will
already have been locked when we locked list->prev->next, so locking it
again will fail, and we'll start over and over.

This should be backported to 2.3.

4 years agoREGTESTS: workaround for a crash with recent libressl on http-reuse sni
Amaury Denoyelle [Fri, 19 Feb 2021 14:37:40 +0000 (15:37 +0100)] 
REGTESTS: workaround for a crash with recent libressl on http-reuse sni

Disable the ssl-reuse for the sni test on http_reuse_conn_hash vtc. This
seems to be the origin of a crash with libressl environment from 3.2.2
up to 3.3.1 included.

For now, it is not determined if the root cause is in haproxy or
libressl.

Please look for the github issue #1115 for all the details.

4 years agoMINOR: mux_h2: do not try to remove front conn from idle trees
Amaury Denoyelle [Fri, 19 Feb 2021 14:37:38 +0000 (15:37 +0100)] 
MINOR: mux_h2: do not try to remove front conn from idle trees

In h2_process there was two parts where the connection was removed from
the idle trees, without first checking if the connection is a backend
side.

This should not produce a crash as the node is properly zeroed on
conn_init. However, it is better to explicit the test as it is done on
all other places. Besides it will be mandatory if the node part is
dynamically allocated only for backend connections.

4 years agoMINOR: listener: refine the default MAX_ACCEPT from 64 to 4
Willy Tarreau [Fri, 19 Feb 2021 14:50:27 +0000 (15:50 +0100)] 
MINOR: listener: refine the default MAX_ACCEPT from 64 to 4

The maximum number of connections accepted at once by a thread for a single
listener used to default to 64 divided by the number of processes but the
tasklet-based model is much more scalable and benefits from smaller values.
Experimentation has shown that 4 gives the highest accept rate for all
thread values, and that 3 and 5 come very close, as shown below (HTTP/1
connections forwarded per second at multi-accept 4 and 64):

 ac\thr|    1     2    4     8     16
 ------+------------------------------
      4|   80k  106k  168k  270k  336k
     64|   63k   89k  145k  230k  274k

Some tests were also conducted on SSL and absolutely no change was observed.

The value was placed into a define because it used to be spread all over the
code.

It might be useful at some point to backport this to 2.3 and 2.2 to help
those who observed some performance regressions from 1.6.

4 years agoMINOR: tasks: refine the default run queue depth
Willy Tarreau [Fri, 19 Feb 2021 14:11:55 +0000 (15:11 +0100)] 
MINOR: tasks: refine the default run queue depth

Since a lot of internal callbacks were turned to tasklets, the runqueue
depth had not been readjusted from the default 200 which was initially
used to favor batched processing. But nowadays it appears too large
already based on the following tests conducted on a 8c16t machine with
a simple config involving "balance leastconn" and one server. The setup
always involved the two threads of a same CPU core except for 1 thread,
and the client was running over 1000 concurrent H1 connections. The
number of requests per second is reported for each (runqueue-depth,
nbthread) couple:

 rq\thr|    1     2     4     8    16
 ------+------------------------------
     32|  120k  159k  276k  477k  698k
     40|  122k  160k  276k  478k  722k
     48|  121k  159k  274k  482k  720k
     64|  121k  160k  274k  469k  710k
    200|  114k  150k  247k  415k  613k  <-- default

It's possible to save up to about 18% performance by lowering the
default value to 40. One possible explanation to this is that checking
I/Os more frequently allows to flush buffers faster and to smooth the
I/O wait time over multiple operations instead of alternating phases
of processing, waiting for locks and waiting for new I/Os.

The total round trip time also fell from 1.62ms to 1.40ms on average,
among which at least 0.5ms is attributed to the testing tools since
this is the minimum attainable on the loopback.

After some observation it would be nice to backport this to 2.3 and
2.2 which observe similar improvements, since some users have already
observed some perf regressions between 1.6 and 2.2.

4 years agoDOC: explain the relation between pool-low-conn and tune.idle-pool.shared
Willy Tarreau [Fri, 19 Feb 2021 10:45:22 +0000 (11:45 +0100)] 
DOC: explain the relation between pool-low-conn and tune.idle-pool.shared

Disabling idle-pool sharing can result in awful performance in presence
of a not so high number of threads, because the number of available idle
connections will be shared among threads, resulting in most of them
abandonning their connections after a request is done if there are already
enough total available. This is a case where pool-low-conn ought to be
used to preserve a number of connections for each thread, but this relation
isn't obvious as is. Let's add mentions about this with both keywords.

4 years agoREGTESTS: reorder reuse conn proxy protocol test
Amaury Denoyelle [Thu, 18 Feb 2021 15:03:37 +0000 (16:03 +0100)] 
REGTESTS: reorder reuse conn proxy protocol test

Try to fix http_reuse_conn_hash proxy protocol for both single and
multi-thread environment. Schedule a new set of requests to be sure that
takeover will be functional even with pool-low-count set to 2.

4 years agoBUILD: ssl: introduce fine guard for OpenSSL specific SCTL functions
Ilya Shipitsin [Sat, 13 Feb 2021 06:45:33 +0000 (11:45 +0500)] 
BUILD: ssl: introduce fine guard for OpenSSL specific SCTL functions

SCTL (signed certificate timestamp list) specified in RFC6962
was implemented in c74ce24cd22e8c683ba0e5353c0762f8616e597d, let
us introduce macro HAVE_SSL_SCTL for the HAVE_SSL_SCTL sake,
which in turn is based on SN_ct_cert_scts, which comes in the same commit

4 years agoBUILD/MEDIUM: da Adding pcre2 support.
David Carlier [Tue, 16 Feb 2021 11:37:45 +0000 (11:37 +0000)] 
BUILD/MEDIUM: da Adding pcre2 support.

The DeviceAtlas Detection API now supports also the pcre2 library,
 and some users wish to have exclusively this version in their
environment.
Also, there is no longer new development happening in the legacy
 pcre(1) counterpart.
Simple check in the build process as the mutual exclusivity check between the
 two are already taking care of early on. Moving the check to the part
only when we build haproxy + the API from source as the other case the API is
 already built with the chosen regex library separately.

4 years agoMINOR: cli: add missing agent commands for set server
William Dauchy [Mon, 15 Feb 2021 16:22:16 +0000 (17:22 +0100)] 
MINOR: cli: add missing agent commands for set server

we previously forgot to add `agent-*` commands.
Take this opportunity to rewrite the help string in a simpler way for
readability (mainly removing simple quotes)

Signed-off-by: William Dauchy <wdauchy@gmail.com>
4 years agoBUG/MINOR: sample: Always consider zero size string samples as unsafe
Christopher Faulet [Thu, 18 Feb 2021 09:22:48 +0000 (10:22 +0100)] 
BUG/MINOR: sample: Always consider zero size string samples as unsafe

smp_is_safe() function is used to be sure a sample may be safely
modified. For string samples, a test is performed to verify if there is a
null-terminated byte. If not, one is added, if possible. It means if the
sample is not const and if there is some free space in the buffer, after
data. However, we must not try to read the null-terminated byte if the
string sample is too long (data >= size) or if the size is equal to
zero. This last test was not performed. Thus it was possible to consider a
string sample as safe by testing a byte outside the buffer.

Now, a zero size string sample is always considered as unsafe and is
duplicated when smp_make_safe() is called.

This patch must be backported in all stable versions.

4 years agoMINOR: tasks/debug: add some extra controls of use-after-free in DEBUG_TASK
Willy Tarreau [Thu, 18 Feb 2021 13:38:49 +0000 (14:38 +0100)] 
MINOR: tasks/debug: add some extra controls of use-after-free in DEBUG_TASK

It's pretty easy to pre-initialize the index, change it on free() and check
it during the wakeup, so let's do this to ease detection of any accidental
task_wakeup() after a task_free() or tasklet_wakeup() after a tasklet_free().
If this would ever happen we'd then get a backtrace and a core now. The
index's parity is respected so that the call history remains exploitable.

4 years agoMINOR: tasks: add DEBUG_TASK to report caller info in a task
Willy Tarreau [Tue, 16 Feb 2021 17:44:00 +0000 (18:44 +0100)] 
MINOR: tasks: add DEBUG_TASK to report caller info in a task

The idea is to know who woke a task up, by recording the last two
callers in a rotating mode. For now it's trivial with task_wakeup()
but tasklet_wakeup_on() will require quite some more changes.

This typically gives this from the debugger:

  (gdb) p t->debug
  $2 = {
    caller_file = {0x0, 0x8c0d80 "src/task.c"},
    caller_line = {0, 260},
    caller_idx = 1
  }

or this:

  (gdb) p t->debug
  $6 = {
    caller_file = {0x7fffe40329e0 "", 0x885feb "src/stream.c"},
    caller_line = {284, 284},
    caller_idx = 1
  }

But it also provides a trivial macro allowing to simply place a call in
a task/tasklet handler that needs to be observed:

   DEBUG_TASK_PRINT_CALLER(t);

Then starting haproxy this way would trivially yield such info:

  $ ./haproxy -db -f test.cfg | sort | uniq -c | sort -nr
   199992 h1_io_cb woken up from src/sock.c:797
    51764 h1_io_cb woken up from src/mux_h1.c:3634
       65 h1_io_cb woken up from src/connection.c:169
       45 h1_io_cb woken up from src/sock.c:777

4 years agoOPTIM: lb-leastconn: do not unlink the server if it did not change
Willy Tarreau [Wed, 17 Feb 2021 15:26:55 +0000 (16:26 +0100)] 
OPTIM: lb-leastconn: do not unlink the server if it did not change

Due to the two-phase server reservation, there are 3 calls to
fwlc_srv_reposition() per request, one during assign_server() to reserve
the slot, one in connect_server() to commit it, and one in process_stream()
to release it. However only one of the first two will change the key, so
it's needlessly costly to take the lock, remove a server and insert it
again at the same place when we can already figure we ought not to even
have taken the lock.

Furthermore, even when the server needs to move, there can be quite some
contention on the lbprm lock forcing the thread to wait. During this time
the served and nbpend server values might have changed, just like the
lb_node.key itself. Thus we measure the values again under the lock
before updating the tree. Measurements have shown that under contention
with 16 servers and 16 threads, 50% of the updates can be avoided there.

This patch makes the function compute the new key and compare it to
the current one before deciding to move the entry (and does it again
under the lock forthe second test).

This removes between 40 and 50% of the tree updates depending on the
thread contention and the number of servers. The performance gain due
to this (on 16 threads) was:

   16 servers:  415 krps -> 440 krps (6%, contention on lbprm)
    4 servers:  554 krps -> 714 krps (+29%, little contention)

One point worth thinking about is that it's not logic to update the
tree 2-3 times per request while it's only read once. half to 2/3 of
these updates are not needed. An experiment consisting in subscribing
the server to a list and letting the readers reinsert them on the fly
showed further degradation instead of an improvement.

A better approach would probably consist in avoinding writes to shared
cache lines by having the leastconn nodes distinct from the servers,
with one node per value, and having them hold an mt-list with all the
servers having that number of connections. The connection count tree
would then be read-mostly instead of facing heavy writes, and most
write operations would be performed on 1-3 list heads which are way
cheaper to migrate than a tree node, and do not require updating the
last two updated neighbors' cache lines.

4 years agoOPTIM: lb-leastconn: do not take the server lock on take_conn/drop_conn
Willy Tarreau [Wed, 17 Feb 2021 15:14:00 +0000 (16:14 +0100)] 
OPTIM: lb-leastconn: do not take the server lock on take_conn/drop_conn

The operations are only an insert and a delete into the LB tree, which
doesn't require the server's lock at all as the lbprm lock is already
held. Let's drop it. Just for the sake of cleanness, given that the
served and nbpend values used to be atomically updated, we'll use an
atomic load to read them.

4 years agoOPTIM: lb-first: do not take the server lock on take_conn/drop_conn
Willy Tarreau [Wed, 17 Feb 2021 15:15:23 +0000 (16:15 +0100)] 
OPTIM: lb-first: do not take the server lock on take_conn/drop_conn

The operations are only an insert and a delete into the LB tree, which
doesn't require the server's lock at all as the lbprm lock is already
held. Let's drop it.

4 years agoMINOR: lb/api: let callers of take_conn/drop_conn tell if they have the lock
Willy Tarreau [Wed, 17 Feb 2021 15:01:37 +0000 (16:01 +0100)] 
MINOR: lb/api: let callers of take_conn/drop_conn tell if they have the lock

The two algos defining these functions (first and leastconn) do not need the
server's lock. However it's already present in pendconn_process_next_strm()
so the API must be updated so that the functions may take it if needed and
that the callers indicate whether they already own it.

As such, the call places (backend.c and stream.c) now do not take it
anymore, queue.c was unchanged since it's already held, and both "first"
and "leastconn" were updated to take it if not already held.

A quick test on the "first" algo showed a jump from 432 to 565k rps by
just dropping the lock in stream.c!

4 years agoRevert "MINOR: threads: change lock_t to an unsigned int"
Willy Tarreau [Wed, 17 Feb 2021 14:45:01 +0000 (15:45 +0100)] 
Revert "MINOR: threads: change lock_t to an unsigned int"

This reverts commit 8f1f177ed0bbdb6c10e61e83994df113452d434f.

Repeated tests have shown a small perforamnce degradation of ~1.8%
caused by this patch at high request rates on 16 threads. The exact
cause is not yet perfectly known but it probably stems in slower
accesses for non-64-bit aligned atomic accesses.

4 years agoOPTIM: server: switch the actconn list to an mt-list
Willy Tarreau [Wed, 17 Feb 2021 12:33:24 +0000 (13:33 +0100)] 
OPTIM: server: switch the actconn list to an mt-list

The remaining contention on the server lock solely comes from
sess_change_server() which takes the lock to add and remove a
stream from the server's actconn list. This is both expensive
and pointless since we have mt-lists, and this list is only
used by the CLI's "shutdown server sessions" command!

Let's migrate to an mt-list and remove the need for this costly
lock. By doing so, the request rate increased by ~1.8%.

4 years agoDEBUG: thread: add 5 extra lock labels for statistics and debugging
Willy Tarreau [Wed, 17 Feb 2021 13:33:58 +0000 (14:33 +0100)] 
DEBUG: thread: add 5 extra lock labels for statistics and debugging

Since OTHER_LOCK is commonly used it's become much more difficult to
profile lock contention by temporarily changing a lock label. Let's
add DEBUG1..5 to serve only for debugging. These ones must not be
used in committed code. We could decide to only define them when
DEBUG_THREAD is set but that would complicate attempts at measuring
performance with debugging turned off.

4 years agoBUG/MEDIUM: checks: don't needlessly take the server lock in health_adjust()
Willy Tarreau [Wed, 17 Feb 2021 14:20:19 +0000 (15:20 +0100)] 
BUG/MEDIUM: checks: don't needlessly take the server lock in health_adjust()

The server lock was taken preventively for anything in health_adjust(),
including the static config checks needed to detect that the lock was not
needed, while the function is always called on the response path to update
a server's status. This was responsible for huge contention causing a
performance drop of about 17% on 16 threads. Let's move the lock only
where it should be, i.e. inside the function around the critical sections
only. By doing this, a 16-thread process jumped back from 575 to 675 krps.

This should be backported to 2.3 as the situation degraded there, and
maybe later to 2.2.

4 years agoBUG/MINOR: checks: properly handle wrapping time in __health_adjust()
Willy Tarreau [Wed, 17 Feb 2021 14:15:15 +0000 (15:15 +0100)] 
BUG/MINOR: checks: properly handle wrapping time in __health_adjust()

There's an issue when a server state changes, we use an integer comparison
to decide whether or not to reschedule a test instead of using a wrapping
timer comparison. This will cause some health-checks not to be immediately
triggered half of the time, and some unneeded calls to task_queue() to be
performed in other cases.

This bug has always been there as it was introduced with the commit that
added the feature, 97f07b832 ("[MEDIUM] Decrease server health based on
http responses / events, version 3"). This may be backported everywhere.

4 years agoMINOR: connection: remove pointers for prehash in conn_hash_params
Amaury Denoyelle [Wed, 17 Feb 2021 15:25:31 +0000 (16:25 +0100)] 
MINOR: connection: remove pointers for prehash in conn_hash_params

Replace unneeded pointers for sni/proxy prehash by plain data type.
The code is slightly cleaner.

4 years agoBUG/MINOR: backend: do not call smp_make_safe for sni conn hash
Amaury Denoyelle [Wed, 17 Feb 2021 14:59:02 +0000 (15:59 +0100)] 
BUG/MINOR: backend: do not call smp_make_safe for sni conn hash

conn_hash_prehash does not need a nul-terminated string, thus it is only
needed to test if the sni sample is not null before using it as
connection hash input.

Moreover, a bug could be introduced between smp_make_safe and
ssl_sock_set_servername call. Indeed, smp_make_safe may call smp_dup
which duplicates the sample in the trash buffer. If another function
manipulates the trash buffer before the call to ssl_sock_set_servername,
the sni sample might be erased. Currently, no function seems to do that
except make_proxy_line in case proxy protocol is used simultaneously
with the sni on the server.

This does not need to be backported.

4 years agoREGTESTS: fix http_reuse_conn_hash proxy test
Amaury Denoyelle [Tue, 16 Feb 2021 16:08:32 +0000 (17:08 +0100)] 
REGTESTS: fix http_reuse_conn_hash proxy test

Define pool-low-conn to 2 to force to have at least 2 idle connections
on the server side for the purpose of the test.

4 years agoBUG/MINOR: session: atomically increment the tracked sessions counter
Willy Tarreau [Tue, 16 Feb 2021 17:08:12 +0000 (18:08 +0100)] 
BUG/MINOR: session: atomically increment the tracked sessions counter

In session_count_new() the tracked counter was still incremented with
a "++" outside of any lock, resulting in occasional slightly off values
such as the following:

    # table: foo, type: string, size:1000, used:1
    0xb2a398: key=127.1.2.3 use=0 exp=86398318 sess_cnt=999959 http_req_cnt=1000004

Now with the correct atomic increment:

    # table: foo, type: string, size:1000, used:1
    0x7f82a4026d38: key=127.1.2.3 use=0 exp=86399294 sess_cnt=1000004 http_req_cnt=1000004

This can be backported to 1.8.

4 years agoBUG/MAJOR: connection: prevent double free if conn selected for removal
Amaury Denoyelle [Tue, 16 Feb 2021 14:16:17 +0000 (15:16 +0100)] 
BUG/MAJOR: connection: prevent double free if conn selected for removal

Always try to remove a connexion from its toremove_list in conn_free.
This prevents a double-free in case the connection is freed but was
already added in toremove_list.

This bug was easily reproduced by running 4-5 runs of inject on a
single-thread instance of haproxy :

$ inject -u 10000 -d 10 -G 127.0.0.1:20080

A crash would soon be triggered in srv_cleanup_toremove_connections.

This does not need to be backported.

4 years agoBUG/MEDIUM: dns: fix multiple double close on fd in dns.c
Emeric Brun [Mon, 15 Feb 2021 14:20:19 +0000 (15:20 +0100)] 
BUG/MEDIUM: dns: fix multiple double close on fd in dns.c

It seems that fd_delete perform the close of the file descriptor
Se we must not close the fd once again after that.

This should fix issues #1128, #1130 and #1131

4 years agoBUG/MINOR: dns: fix ring attach control on dns_session_new
Emeric Brun [Mon, 15 Feb 2021 14:13:31 +0000 (15:13 +0100)] 
BUG/MINOR: dns: fix ring attach control on dns_session_new

Ths patch adds a control on ring_attach which can not currently fail
since we are the first to try to attach.

This should fix issue #1126

4 years agoBUG/MINOR: dns: missing test writing in output channel in session handler
Emeric Brun [Mon, 15 Feb 2021 13:12:06 +0000 (14:12 +0100)] 
BUG/MINOR: dns: missing test writing in output channel in session handler

This patch fix a case which should never happen writing
in output channel since we check available room before

This patch should fix github issue #1132

4 years agoBUG/MINOR: dns: dns_connect_server must return -1 unsupported nameserver's type
Emeric Brun [Mon, 15 Feb 2021 13:28:27 +0000 (14:28 +0100)] 
BUG/MINOR: dns: dns_connect_server must return -1 unsupported nameserver's type

This patch fix returns code in case of dns_connect_server is called
on unsupported type (which should not happen). Doing this we have
the warranty that after a return 0 the fd is never -1.

This patch should fix github issues #1127, #1128 and #1130

4 years agoBUG/MINOR: dns: add test on result getting value from buffer into ring.
Emeric Brun [Mon, 15 Feb 2021 12:58:06 +0000 (13:58 +0100)] 
BUG/MINOR: dns: add test on result getting value from buffer into ring.

This patch adds a missing test in dns_session_io_handler, getting
the query id from the buffer of the ring. An error should never
happen since messages are completely added atomically.

This bug should fix github issue #1133

4 years agoMEDIUM: contrib/prometheus-exporter: add listen stats
William Dauchy [Sun, 14 Feb 2021 22:22:56 +0000 (23:22 +0100)] 
MEDIUM: contrib/prometheus-exporter: add listen stats

this was a missing piece for a while now even though it was planned. This
patch adds listen stats.
Nothing in particular but we make use of the status helper previously
added.  `promex_st_metrics` diff also looks scary, but I had to realign
all lines.

Signed-off-by: William Dauchy <wdauchy@gmail.com>
4 years agoMINOR: stats: add helper to get status string
William Dauchy [Sun, 14 Feb 2021 22:22:55 +0000 (23:22 +0100)] 
MINOR: stats: add helper to get status string

move listen status to a helper, defining both status enum and string
definition.
this will be helpful to be reused in prometheus code. It also removes
this hard-to-read nested ternary.

Signed-off-by: William Dauchy <wdauchy@gmail.com>
4 years agoMEDIUM: stats: allow to select one field in `stats_fill_li_stats`
William Dauchy [Sun, 14 Feb 2021 22:22:54 +0000 (23:22 +0100)] 
MEDIUM: stats: allow to select one field in `stats_fill_li_stats`

prometheus approach requires to output all values for a given metric
name; meaning we iterate through all metrics, and then iterate in the
inner loop on all objects for this metric.
In order to allow more code reuse, adapt the stats API to be able to
select one field or fill them all otherwise.
From this patch it should be possible to add support for listen stats in
prometheus.

Signed-off-by: William Dauchy <wdauchy@gmail.com>
4 years agoCLEANUP: contrib/prometheus-exporter: align for with srv status case
William Dauchy [Sun, 14 Feb 2021 21:26:24 +0000 (22:26 +0100)] 
CLEANUP: contrib/prometheus-exporter: align for with srv status case

the for loop was wrongly indented following our recent rework

Signed-off-by: William Dauchy <wdauchy@gmail.com>
4 years agoCLEANUP: check: fix get_check_status_info declaration
William Dauchy [Sun, 14 Feb 2021 21:26:23 +0000 (22:26 +0100)] 
CLEANUP: check: fix get_check_status_info declaration

we always put a \n between function name and `{`

Signed-off-by: William Dauchy <wdauchy@gmail.com>
4 years agoBUG/MINOR: server: Remove RMAINT from admin state when loading server state
Christopher Faulet [Fri, 12 Feb 2021 16:36:08 +0000 (17:36 +0100)] 
BUG/MINOR: server: Remove RMAINT from admin state when loading server state

The RMAINT admin state is dynamic and should be remove from the
srv_admin_state parameter when a server state is loaded from a server-state
file. Otherwise an erorr is reported, the server-state line is ignored and
the server state is not updated.

This patch should fix the issue #576. It must be backported as far as 1.8.

4 years ago[RELEASE] Released version 2.4-dev8 v2.4-dev8
Willy Tarreau [Sat, 13 Feb 2021 09:17:27 +0000 (10:17 +0100)] 
[RELEASE] Released version 2.4-dev8

Released version 2.4-dev8 with the following main changes :
    - BUILD: ssl: fix typo in HAVE_SSL_CTX_ADD_SERVER_CUSTOM_EXT macro
    - BUILD: ssl: guard SSL_CTX_add_server_custom_ext with special macro
    - BUG/MINOR: mux-h1: Don't emit extra CRLF for empty chunked messages
    - MINOR: contrib/prometheus-exporter: use stats desc when possible followup
    - MEDIUM: contrib/prometheus-exporter: export base stick table stats
    - CLEANUP: assorted typo fixes in the code and comments
    - CLEANUP: check: fix some typo in comments
    - CLEANUP: tools: typo in `strl2irc` mention
    - BUILD: ssl: guard SSL_CTX_set_msg_callback with SSL_CTRL_SET_MSG_CALLBACK macro
    - MEDIUM: ssl: add a rwlock for SSL server session cache
    - BUG/MINOR: intops: fix mul32hi()'s off-by-one
    - BUG/MINOR: freq_ctr: fix a wrong delay calculation in next_event_delay()
    - MINOR: stick-tables/counters: add http_fail_cnt and http_fail_rate data types
    - MINOR: ssl: add SSL_SERVER_LOCK label in threads.h
    - BUG/MINOR: mux-h1: Don't increment HTTP error counter for 408/500/501 errors
    - BUG/MINOR: http-ana: Don't increment HTTP error counter on internal errors
    - BUG/MEDIUM: mux-h1: Always set CS_FL_EOI for response in MSG_DONE state
    - BUG/MINOR: mux-h1: Fix data skipping for bodyless responses
    - BUG/MINOR: mux-h1: Don't blindly skip EOT block for non-chunked messages
    - BUG/MEDIUM: mux-h2: Add EOT block when EOM flag is set on an empty HTX message
    - MINOR: mux-h1: Be sure EOM flag is set when processing end of outgoing message
    - REGTESTS: Add a script to test payload skipping for bodyless HTTP responses
    - BUG/MINOR: server: re-align state file fields number
    - CLEANUP: muxes: Remove useless calls to b_realign_if_empty()
    - BUG/MINOR: tools: Fix a memory leak on error path in parse_dotted_uints()
    - CLEANUP: remove unused variable assigned found by Coverity
    - CLEANUP: queue: Remove useless tests on p or pp in pendconn_process_next_strm()
    - BUG/MINOR: backend: hold correctly lock when killing idle conn
    - MEDIUM: connection: protect idle conn lists with locks
    - MEDIUM: connection: replace idle conn lists by eb trees
    - MINOR: backend: search conn in idle/safe trees after available
    - MINOR: backend: search conn in idle tree after safe on always reuse
    - MINOR: connection: prepare hash calcul for server conns
    - MINOR: connection: use the srv pointer for the srv conn hash
    - MINOR: backend: compare conn hash for session conn reuse
    - MINOR: connection: use sni as parameter for srv conn hash
    - MINOR: reg-tests: test http-reuse with sni
    - MINOR: backend: rewrite alloc of stream target address
    - MINOR: connection: use dst addr as parameter for srv conn hash
    - MINOR: reg-test: test http-reuse with specific dst addr
    - MINOR: backend: rewrite alloc of connection src address
    - MINOR: connection: use src addr as parameter for srv conn hash
    - MINOR: connection: use proxy protocol as parameter for srv conn hash
    - MINOR: reg-tests: test http-reuse with proxy protocol
    - MINOR: doc: update http reuse for new eligilible connections
    - BUG/MINOR: backend: fix compilation without ssl
    - REGTESTS: adjust http_reuse_conn_hash requirements
    - REGTESTS: deactivate a failed test on CI in http_reuse_conn_hash
    - REGTESTS: fix sni used in http_reuse_conn_hash for libressl 3.3.0
    - CI: cirrus: update FreeBSD image to 12.2
    - MEDIUM: cli: add check-addr command
    - MEDIUM: cli: add agent-port command
    - MEDIUM: server: add server-states version 2
    - MEDIUM: server: support {check,agent}_addr, agent_port in server state
    - MINOR: server: enhance error precision when applying server state
    - BUG/MINOR: server: Fix server-state-file-name directive
    - CLEANUP: deinit: release global and per-proxy server-state variables on deinit
    - BUG/MEDIUM: config: don't pick unset values from last defaults section
    - BUG/MINOR: stats: revert the change on ST_CONVDONE
    - BUG/MINOR: cfgparse: do not mention "addr:port" as supported on proxy lines
    - BUG/MINOR: http-htx: defpx must be a const in proxy_dup_default_conf_errors()
    - BUG/MINOR: tcpheck: the source list must be a const in dup_tcpcheck_var()
    - BUILD: proxy: add missing compression-t.h to proxy-t.h
    - REORG: move init_default_instance() to proxy.c and pass it the defproxy pointer
    - REORG: proxy: centralize the proxy allocation code into alloc_new_proxy()
    - MEDIUM: proxy: only take defaults when a default proxy is passed.
    - MINOR: proxy: move the defproxy freeing code to proxy.c
    - MINOR: proxy: always properly reset the just freed default instance pointers
    - BUG/MINOR: extcheck: proxy_parse_extcheck() must take a const for the defproxy
    - BUG/MINOR: tcpcheck: proxy_parse_*check*() must take a const for the defproxy
    - BUG/MINOR: server: parse_server() must take a const for the defproxy
    - MINOR: cfgparse: move defproxy to cfgparse-listen as a static
    - MINOR: proxy: add a new capability PR_CAP_DEF
    - MINOR: cfgparse: check PR_CAP_DEF instead of comparing poiner against defproxy
    - MINOR: cfgparse: use a pointer to the current default proxy
    - MINOR: proxy: also store the name for a defaults section
    - MINOR: proxy: support storing defaults sections into their own tree
    - MEDIUM: proxy: store the default proxies in a tree by name
    - MEDIUM: cfgparse: allow a proxy to designate the defaults section to use
    - MINOR: http: add baseq sample fetch
    - CLEANUP: tcpcheck: Remove a useless test on port variable
    - BUG/MINOR: server: Don't call fopen() with server-state filepath set to NULL
    - CLEANUP: server: Remove useless "filepath" variable in apply_server_state()
    - MINOR: peers/cli: do not dump the peers dictionaries by default on "show peers"
    - MINOR: cfgparse: implement a simple if/elif/else/endif macro block handler
    - DOC: tune: explain the origin of block size for ssl.cachesize
    - MINOR: tcp: add support for defer-accept on FreeBSD.
    - MINOR: ring: adds new ring_init function.
    - CLEANUP: channel: fix comment in ci_putblk.
    - BUG/MINOR: dns: add missing sent counter and parent id to dns counters.
    - BUG/MINOR: resolvers: fix attribute packed struct for dns
    - MINOR: resolvers: renames some resolvers internal types and removes dns prefix
    - MINOR: resolvers: renames type dns_resolvers to resolvers.
    - MINOR: resolvers: renames some resolvers specific types to not use dns prefix
    - MINOR: resolvers: renames some dns prefixed types using resolv prefix.
    - MINOR: resolvers: renames resolvers DNS_RESP_* errcodes RSLV_RESP_*
    - MINOR: resolvers: renames resolvers DNS_UPD_* returncodes to RSLV_UPD_*
    - MINOR: resolvers: rework prototype suffixes to split resolving and dns.
    - MEDIUM: resolvers: move resolvers section parsing from cfgparse.c to dns.c
    - MINOR: resolvers: replace nameserver's resolver ref by generic parent pointer
    - MINOR: resolvers: rework dns stats prototype because specific to resolvers
    - MEDIUM: resolvers: split resolving and dns message exchange layers.
    - MEDIUM: resolvers/dns: split dns.c into dns.c and resolvers.c
    - MEDIUM: dns: adds code to support pipelined DNS requests over TCP.
    - MEDIUM: resolvers: add supports of TCP nameservers in resolvers.

4 years agoMEDIUM: resolvers: add supports of TCP nameservers in resolvers.
Emeric Brun [Fri, 12 Feb 2021 19:05:45 +0000 (20:05 +0100)] 
MEDIUM: resolvers: add supports of TCP nameservers in resolvers.

This patch introduce the new line "server" to set a TCP
nameserver in a "resolvers" section:

server <name> <address> [param*]
  Used to configure a DNS TCP or stream server. This supports for all
  "server" parameters found in 5.2 paragraph. Some of these parameters
  are irrelevant for DNS resolving. Note: currently 4 queries are pipelined
  on the same connections. A batch of idle connections are removed every
  5 seconds. "maxconn" can be configured to limit the amount of those
  concurrent connections and TLS should also usable if the server supports
. The current implementation limits to 4 pipelined

The name of the line in configuration is open to discussion
and could be changed before the next release.

4 years agoMEDIUM: dns: adds code to support pipelined DNS requests over TCP.
Emeric Brun [Fri, 12 Feb 2021 19:03:38 +0000 (20:03 +0100)] 
MEDIUM: dns: adds code to support pipelined DNS requests over TCP.

This patch introduce the "dns_stream_nameserver" to use DNS over
TCP on strict nameservers.  For the upper layer it is analog to
the api used with udp nameservers except that the user que switch
the name server in "stream" mode at the init using "dns_stream_init".

The fallback from UDP to TCP is not handled and this is not the
purpose of this feature. This is done to choose the transport layer
during the initialization.

Currently there is a hardcoded limit of 4 pipelined transactions
per TCP connections. A batch of idle connections is expired every 5s.
This code is designed to support a maximum DNS message size on TCP: 64k.

Note: this code won't perform retry on unanswered queries this
should be handled by the upper layer

4 years agoMEDIUM: resolvers/dns: split dns.c into dns.c and resolvers.c
Emeric Brun [Fri, 12 Feb 2021 18:42:55 +0000 (19:42 +0100)] 
MEDIUM: resolvers/dns: split dns.c into dns.c and resolvers.c

This patch splits current dns.c into two files:

The first dns.c contains code related to DNS message exchange over UDP
and in future other TCP. We try to remove depencies to resolving
to make it usable by other stuff as DNS load balancing.

The new resolvers.c inherit of the code specific to the actual
resolvers.

Note:
It was really difficult to obtain a clean diff dur to the amount
of moved code.

Note2:
Counters and stuff related to stats is not cleany separated because
currently counters for both layers are merged and hard to separate
for now.

4 years agoMEDIUM: resolvers: split resolving and dns message exchange layers.
Emeric Brun [Mon, 4 Jan 2021 12:32:20 +0000 (13:32 +0100)] 
MEDIUM: resolvers: split resolving and dns message exchange layers.

This patch splits recv and send functions in two layers. the
lowest is responsible of DNS message transactions over
the network. Doing this we could use DNS message layer
for something else than resolving. Load balancing for instance.

This patch also re-works the way to init a nameserver and
introduce the new struct dns_dgram_server to prepare the arrival
of dns_stream_server and the support of DNS over TCP.

The way to retry a send failure of a request because of EAGAIN
was re-worked. Previously there was no control and all "pending"
queries were re-played each time it reaches a EAGAIN. This
patch introduce a ring to stack messages in case of sent
failure. This patch is emptied if poller shows that the
socket is ready again to push messages.

4 years agoMINOR: resolvers: rework dns stats prototype because specific to resolvers
Emeric Brun [Wed, 6 Jan 2021 15:01:02 +0000 (16:01 +0100)] 
MINOR: resolvers: rework dns stats prototype because specific to resolvers

Counters are currently stored into lowlevel nameservers struct but
most of them are resolving layer data and increased in the upper layer
So this patch renames the prototype used to allocate/dump them with prefix
'resolv' waiting for a clean split.