]> git.ipfire.org Git - thirdparty/haproxy.git/log
thirdparty/haproxy.git
5 years agoBUG/MAJOR: sched: make sure task_kill() always queues the task
Willy Tarreau [Thu, 2 Jul 2020 12:14:00 +0000 (14:14 +0200)] 
BUG/MAJOR: sched: make sure task_kill() always queues the task

task_kill() may fail to queue a task if this task has never ever run,
because its equivalent (tasklet->list) member has never been "emptied"
since it didn't pass through the LIST_DEL_INIT() that's performed by
run_tasks_from_lists(). This results in these tasks to never be freed.

It happens during the mux takeover since the target task usually is
the timeout task which, by definition, has never run yet.

This fixes commit eb8c2c69f ("MEDIUM: sched: implement task_kill() to
kill a task") which was introduced after 2.2-dev11 and doesn't need to
be backported.

5 years agoBUILD: debug: avoid build warnings with DEBUG_MEM_STATS
Willy Tarreau [Thu, 2 Jul 2020 08:25:01 +0000 (10:25 +0200)] 
BUILD: debug: avoid build warnings with DEBUG_MEM_STATS

Some libcs define strdup() as a macro and cause redefine warnings to
be emitted, so let's first undefine all functions we redefine.

5 years agoMINOR: log-format: allow to preserve spacing in log format strings
Dragan Dosen [Tue, 23 Jun 2020 16:16:44 +0000 (18:16 +0200)] 
MINOR: log-format: allow to preserve spacing in log format strings

Now it's possible to preserve spacing everywhere except in "log-format",
"log-format-sd" and "unique-id-format" directives, where spaces are
delimiters and are merged. That may be useful when the response payload
is specified as a log format string by "lf-file" or "lf-string", or even
for headers or anything else.

In order to merge spaces, a new option LOG_OPT_MERGE_SPACES is applied
exclusively on options passed to function parse_logformat_string().

This patch fixes an issue #701 ("http-request return log-format file
evaluation altering spacing of ASCII output/art").

5 years agoMINOR: debug: add a new "debug dev memstats" command
Willy Tarreau [Thu, 2 Jul 2020 07:14:48 +0000 (09:14 +0200)] 
MINOR: debug: add a new "debug dev memstats" command

Now when building with -DDEBUG_MEM_STATS, some malloc/calloc/strdup/realloc
stats are kept per file+line number and may be displayed and even reset on
the CLI using "debug dev memstats". This allows to easily track potential
leakers or abnormal usages.

5 years agoMINOR: 51d: silence a warning about null pointer dereference
Dragan Dosen [Wed, 1 Jul 2020 17:58:32 +0000 (19:58 +0200)] 
MINOR: 51d: silence a warning about null pointer dereference

This is due to issue #713, that reports null pointer dereference
suspected by coverity.

5 years agoMINOR: config: add a new tune.idle-pool.shared global setting.
Willy Tarreau [Wed, 1 Jul 2020 16:49:24 +0000 (18:49 +0200)] 
MINOR: config: add a new tune.idle-pool.shared global setting.

Enables ('on') or disables ('off') sharing of idle connection pools between
threads for a same server. The default is to share them between threads in
order to minimize the number of persistent connections to a server, and to
optimize the connection reuse rate. But to help with debugging or when
suspecting a bug in HAProxy around connection reuse, it can be convenient to
forcefully disable this idle pool sharing between multiple threads, and force
this option to "off". The default is on.

This could have been nice to have during the idle connections debugging,
but it's not too late to add it!

5 years agoDOC: configuration: fix alphabetical ordering for tune.pool-{high,low}-fd-ratio
Willy Tarreau [Wed, 1 Jul 2020 16:30:16 +0000 (18:30 +0200)] 
DOC: configuration: fix alphabetical ordering for tune.pool-{high,low}-fd-ratio

In addition they were in the wrong alphabetical order in the doc. They
were added in 2.0 by commit 88698d966 ("MEDIUM: connections: Add a way
to control the number of idling connections.") so this must be backported
to 2.0.

5 years agoDOC: configuration: add missing index entries for tune.pool-{low,high}-fd-ratio
Willy Tarreau [Wed, 1 Jul 2020 16:27:16 +0000 (18:27 +0200)] 
DOC: configuration: add missing index entries for tune.pool-{low,high}-fd-ratio

These two keywords didn't have an entry in the index. They were added in
2.0 by commit 88698d966 ("MEDIUM: connections: Add a way to control the
number of idling connections.") so this must be backported to 2.0.

5 years agoMEDIUM: connections: Don't use a lock when moving connections to remove.
Olivier Houchard [Mon, 29 Jun 2020 18:15:59 +0000 (20:15 +0200)] 
MEDIUM: connections: Don't use a lock when moving connections to remove.

Make it so we don't have to take a lock while moving a connection from
the idle list to the toremove_list by taking advantage of the MT_LIST.

5 years agoCLEANUP: connections: rename the toremove_lock to takeover_lock
Olivier Houchard [Mon, 29 Jun 2020 18:15:59 +0000 (20:15 +0200)] 
CLEANUP: connections: rename the toremove_lock to takeover_lock

This lock was misnamed and a bit confusing. It's only used for takeover
so let's call it takeover_lock.

5 years agoMINOR: list: Add MT_LIST_DEL_SAFE_NOINIT() and MT_LIST_ADDQ_NOCHECK()
Olivier Houchard [Mon, 29 Jun 2020 18:14:28 +0000 (20:14 +0200)] 
MINOR: list: Add MT_LIST_DEL_SAFE_NOINIT() and MT_LIST_ADDQ_NOCHECK()

Add two new macros, MT_LIST_DEL_SAFE_NOINIT makes sure we remove the
element from the list, without reinitializing its next and prev, and
MT_LIST_ADDQ_NOCHECK is similar to MT_LIST_ADDQ(), except it doesn't check
if the element is already in a list.
The goal is to be able to move an element from a list we're currently
parsing to another, keeping it locked in the meanwhile.

5 years agoMEDIUM: mux-fcgi: use task_kill() during fcgi_takeover() instead of task_wakeup()
Willy Tarreau [Wed, 1 Jul 2020 14:39:33 +0000 (16:39 +0200)] 
MEDIUM: mux-fcgi: use task_kill() during fcgi_takeover() instead of task_wakeup()

task_wakeup() passes the task through the global run queue under the
global RQ lock, which is expensive when dealing with large amounts of
fcgi_takeover() calls. Let's use the new task_kill() instead to kill the
task.

5 years agoMEDIUM: mux-h2: use task_kill() during h2_takeover() instead of task_wakeup()
Willy Tarreau [Wed, 1 Jul 2020 14:39:33 +0000 (16:39 +0200)] 
MEDIUM: mux-h2: use task_kill() during h2_takeover() instead of task_wakeup()

task_wakeup() passes the task through the global run queue under the
global RQ lock, which is expensive when dealing with large amounts of
h2_takeover() calls. Let's use the new task_kill() instead to kill the
task.

5 years agoMEDIUM: mux-h1: use task_kill() during h1_takeover() instead of task_wakeup()
Willy Tarreau [Wed, 1 Jul 2020 14:39:33 +0000 (16:39 +0200)] 
MEDIUM: mux-h1: use task_kill() during h1_takeover() instead of task_wakeup()

task_wakeup() passes the task through the global run queue under the
global RQ lock, which is expensive when dealing with large amounts of
h1_takeover() calls. Let's use the new task_kill() instead to kill the
task.

By doing so, a scenario involving approximately 130k takeover/s running on
16 threads gained almost 3% performance from 319k req/s to 328k.

5 years agoMEDIUM: sched: implement task_kill() to kill a task
Willy Tarreau [Tue, 30 Jun 2020 09:48:48 +0000 (11:48 +0200)] 
MEDIUM: sched: implement task_kill() to kill a task

task_kill() may be used by any thread to kill any task with less overhead
than a regular wakeup. In order to achieve this, it bypasses the priority
tree and inserts the task directly into the shared tasklets list, cast as
a tasklet. The task_list_size is updated to make sure it is properly
decremented after execution of this task. The task will thus be picked by
process_runnable_tasks() after checking the tree and sent to the TL_URGENT
list, where it will be processed and killed.

If the task is bound to more than one thread, its first thread will be the
one notified.

If the task was already queued or running, nothing is done, only the flag
is added so that it gets killed before or after execution. Of course it's
the caller's responsibility to make sur any resources allocated by this
task were already cleaned up or taken over.

5 years agoMEDIUM: sched: create a new TASK_KILLED task flag
Willy Tarreau [Tue, 30 Jun 2020 09:48:48 +0000 (11:48 +0200)] 
MEDIUM: sched: create a new TASK_KILLED task flag

This flag, when set, will be used to indicate that the task must die.
At the moment this may only be placed by the task itself or by the
scheduler when placing it into the TL_NORMAL queue.

5 years agoMINOR: sched: make sched->task_list_size atomic
Willy Tarreau [Tue, 30 Jun 2020 09:48:48 +0000 (11:48 +0200)] 
MINOR: sched: make sched->task_list_size atomic

We'll need to update it from foreign threads in order to throw killed
tasks and maintain correct accounting, so let's make it atomic.

5 years agoMINOR: backend: don't always takeover from the same threads
Willy Tarreau [Wed, 1 Jul 2020 13:55:30 +0000 (15:55 +0200)] 
MINOR: backend: don't always takeover from the same threads

The next thread walking algorithm in commit 566df309c ("MEDIUM:
connections: Attempt to get idle connections from other threads.")
proved to be sufficient for most cases, but it still has some rough
edges when threads are unevenly loaded. If one thread wakes up with
10 streams to process in a burst, it will mainly take over connections
from the next one until it doesn't have anymore.

This patch implements a rotating index that is stored into the server
list and that any thread taking over a connection is responsible for
updating. This way it starts mostly random and avoids always picking
from the same place. This results in a smoother distribution overall
and a slightly lower takeover rate.

5 years agoBUG/MEDIUM: backend: always search in the safe list after failing on the idle one
Willy Tarreau [Wed, 1 Jul 2020 13:04:38 +0000 (15:04 +0200)] 
BUG/MEDIUM: backend: always search in the safe list after failing on the idle one

There's a tricky behavior that was lost when the idle connections were
made sharable between thread in commit 566df309c ("MEDIUM: connections:
Attempt to get idle connections from other threads."), it is the ability
to retry from the safe list when looking for any type of idle connection
and not finding one in the idle list.

It is already important when dealing with long-lived connections since
they ultimately all become safe, but that case is already covered by
the fact that safe conns not being used end up closing and are not
looked up anymore since connect_server() sees there are none.

But it's even more important when using server-side connections which
periodically close, because the new connections may spend half of their
time in safe state and the other half in the idle state, and failing
to grab one such connection from the right list results in establishing
a new connection.

This patch makes sure that a failure to find an idle connection results
in a new attempt at finding one from the safe list if available. In order
to avoid locking twice, connections are attempted alternatively from the
idle then safe list when picking from siblings. Tests have shown a ~2%
performance increase by avoiding to lock twice.

A typical test with 10000 connections over 16 threads with 210 servers
having a 1 millisecond response time and closing every 5 requests shows
a degrading performance starting at 120k req/s down to 60-90k and an
average reuse rate of 44%. After the fix, the reuse rate raises to 79%
and the performance becomes stable at 254k req/s. Similarly the previous
test with full keep-alive has now increased from 96% reuse rate to 99%
and from 352k to 375k req/s.

No backport is needed as this is 2.2-only.

5 years agoMEDIUM: server: add a new pool-low-conn server setting
Willy Tarreau [Wed, 1 Jul 2020 05:43:51 +0000 (07:43 +0200)] 
MEDIUM: server: add a new pool-low-conn server setting

The problem with the way idle connections currently work is that it's
easy for a thread to steal all of its siblings' connections, then release
them, then it's done by another one, etc. This happens even more easily
due to scheduling latencies, or merged events inside the same pool loop,
which, when dealing with a fast server responding in sub-millisecond
delays, can really result in one thread being fully at work at a time.

In such a case, we perform a huge amount of takeover() which consumes
CPU and requires quite some locking, sometimes resulting in lower
performance than expected.

In order to fight against this problem, this patch introduces a new server
setting "pool-low-conn", whose purpose is to dictate when it is allowed to
steal connections from a sibling. As long as the number of idle connections
remains at least as high as this value, it is permitted to take over another
connection. When the idle connection count becomes lower, a thread may only
use its own connections or create a new one. By proceeding like this even
with a low number (typically 2*nbthreads), we quickly end up in a situation
where all active threads have a few connections. It then becomes possible
to connect to a server without bothering other threads the vast majority
of the time, while still being able to use these connections when the
number of available FDs becomes low.

We also use this threshold instead of global.nbthread in the connection
release logic, allowing to keep more extra connections if needed.

A test performed with 10000 concurrent HTTP/1 connections, 16 threads
and 210 servers with 1 millisecond of server response time showed the
following numbers:

   haproxy 2.1.7:           185000 requests per second
   haproxy 2.2:             314000 requests per second
   haproxy 2.2 lowconn 32:  352000 requests per second

The takeover rate goes down from 300k/s to 13k/s. The difference is
further amplified as the response time shrinks.

5 years agoBUG/MINOR: server: fix the connection release logic regarding nearly full conditions
Willy Tarreau [Wed, 1 Jul 2020 11:57:49 +0000 (13:57 +0200)] 
BUG/MINOR: server: fix the connection release logic regarding nearly full conditions

There was a logic bug in commit ddfe0743d ("MEDIUM: server: use the two
thresholds for the connection release algorithm"): instead of keeping
only our first idle connection when FDs become scarce, the condition was
inverted resulting in enforcing this constraint unless FDs are scarce.
This results in less idle connections than permitted to be kept under
normal condition.

No backport needed.

5 years agoMINOR: server: skip servers with no idle conns earlier
Willy Tarreau [Wed, 1 Jul 2020 06:24:44 +0000 (08:24 +0200)] 
MINOR: server: skip servers with no idle conns earlier

In conn_backend_get() we can avoid locking other servers when trying
to steal their connections when we know for sure they will not have
one, so let's do it to lower the contention on the lock.

5 years agoMINOR: cli/proxy: add a new "show servers conn" command
Willy Tarreau [Wed, 1 Jul 2020 05:00:59 +0000 (07:00 +0200)] 
MINOR: cli/proxy: add a new "show servers conn" command

This command reuses the existing "show servers state" to also dump the
state of active and idle connections. The main use is to serve as a
debugging tool to troubleshot connection reuse issues.

5 years agoBUG/MINOR: proxy: always initialize the trash in show servers state
Willy Tarreau [Wed, 1 Jul 2020 05:09:39 +0000 (07:09 +0200)] 
BUG/MINOR: proxy: always initialize the trash in show servers state

Actually the cleanup in commit 6ff8143f7 ("BUG/MINOR: proxy: fix
dump_server_state()'s misuse of the trash") allowed to spot that the
trash is never reset when dumping a servers state. I couldn't manage
to make it dump garbage even with large setups but didn't find either
where it's cleared between successive calls while other handlers do
explicitly invoke chunk_reset(), so it seems to happen a bit by luck.

Let's use chunk_printf() here for each turn, it makes things clearer.

This could be backported along with previous patch, especially if any
user reports occasional garbage appearing in the show servers output.

5 years agoBUG/MINOR: proxy: fix dump_server_state()'s misuse of the trash
Willy Tarreau [Wed, 1 Jul 2020 05:02:42 +0000 (07:02 +0200)] 
BUG/MINOR: proxy: fix dump_server_state()'s misuse of the trash

dump_server_state() claims to dump into a buffer but instead it writes
into a buffer then dumps the trash into the channel, so it only supports
being called with buf=&trash and doesn't need this buffer. There doesn't
seem to be any current impact of this mistake since the function is called
from one location only.

A backport may be performed if it helps fixing other bugs but it will not
fix an existing bug by itself.

5 years agoBUG/MEDIUM: log-format: fix possible endless loop in parse_logformat_string()
Dragan Dosen [Tue, 30 Jun 2020 19:16:43 +0000 (21:16 +0200)] 
BUG/MEDIUM: log-format: fix possible endless loop in parse_logformat_string()

This patch adds a missing break to end the loop in case when '%[' is not
properly closed with ']'.

The issue has been introduced with commit cd0d2ed ("MEDIUM: log-format:
make the LF parser aware of sample expressions' end").

5 years agoBUG/MEDIUM: pattern: Add a trailing \0 to match strings only if possible
Christopher Faulet [Tue, 30 Jun 2020 16:52:32 +0000 (18:52 +0200)] 
BUG/MEDIUM: pattern: Add a trailing \0 to match strings only if possible

In pat_match_str() and pat_math_beg() functions, a trailing zero is
systematically added at the end of the string, even if the buffer is not large
enough to accommodate it. It is a possible buffer overflow. For instance, when
the alpn is matched against a list of strings, the sample fetch is filled with a
non-null terminated string returned by the SSL library. No trailing zero must be
added at the end of this string, because it is outside the buffer.

So, to fix the bug, a trailing zero is added only if the buffer is large enough
to accommodate it. Otherwise, the sample fetch is duplicated. smp_dup() function
adds a trailing zero to the duplicated string, truncating it if it is too long.

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

5 years agoDOC: ssl: add "allow-0rtt" and "ciphersuites" in crt-list
William Lallemand [Tue, 30 Jun 2020 14:11:36 +0000 (16:11 +0200)] 
DOC: ssl: add "allow-0rtt" and "ciphersuites" in crt-list

Support for "allow-0rtt" and "ciphersuites" exists for crt-list.

Fix issue #721.

Should be backported as far as 1.8.

5 years agoMINOR: pools: increase MAX_BASE_POOLS to 64
Willy Tarreau [Tue, 30 Jun 2020 12:29:02 +0000 (14:29 +0200)] 
MINOR: pools: increase MAX_BASE_POOLS to 64

When not sharing pools (i.e. when building with -DDEBUG_DONT_SHARE_POOLS)
we have about 47 pools right now, while MAX_BASE_POOLS is only 32, meaning
that only the first 32 ones will benefit from a per-thread cache entry.
This totally kills performance when pools are not shared (roughly -20%).

Let's double the limit to gain some margin, and make it possible to set
it as a build option.

It might be useful to backport this to stable versions as they're likely
to be affected as well.

5 years agoMINOR: mux-fcgi: avoid taking the toremove_lock in on dying tasks
Willy Tarreau [Tue, 30 Jun 2020 09:19:23 +0000 (11:19 +0200)] 
MINOR: mux-fcgi: avoid taking the toremove_lock in on dying tasks

If the owning task is already dying (context was destroyed by fcgi_takeover)
there's no point taking the lock then removing it later since all the code
in between is conditionned by a non-null context. Let's simplify this.

5 years agoMINOR: mux-h2: avoid taking the toremove_lock in on dying tasks
Willy Tarreau [Tue, 30 Jun 2020 09:19:23 +0000 (11:19 +0200)] 
MINOR: mux-h2: avoid taking the toremove_lock in on dying tasks

If the owning task is already dying (context was destroyed by h2_takeover)
there's no point taking the lock then removing it later since all the code
in between is conditionned by a non-null context. Let's simplify this.

5 years agoMINOR: mux-h1: avoid taking the toremove_lock in on dying tasks
Willy Tarreau [Tue, 30 Jun 2020 09:19:23 +0000 (11:19 +0200)] 
MINOR: mux-h1: avoid taking the toremove_lock in on dying tasks

If the owning task is already dying (context was destroyed by h1_takeover)
there's no point taking the lock then removing it later since all the code
in between is conditionned by a non-null context. Let's simplify this.

5 years agoBUG/MINOR: sched: properly cover for a rare MT_LIST_ADDQ() race
Willy Tarreau [Tue, 30 Jun 2020 11:46:21 +0000 (13:46 +0200)] 
BUG/MINOR: sched: properly cover for a rare MT_LIST_ADDQ() race

In commit 3ef7a190b ("MEDIUM: tasks: apply a fair CPU distribution
between tasklet classes") we compute a total weight to be used to
split the CPU time between queues. There is a mention that the
total cannot be null, wihch is based on the fact that we only get
there if thread_has_task() returns non-zero. But there is a very
small race which can break this assumption: if two threads conflict
on MT_LIST_ADDQ() on an empty shared list and both roll back before
trying again, there is the possibility that a first call to
MT_LIST_ISEMPTY() sees the first thread install itself, then the
second call will see the list empty when both roll back. Thus we
could proceed with the queue while it's temporarily empty and
compute max lengths using a divide by zero. This case is very
hard to trigger, it seldom happens on 16 threads at 400k req/s.

Let's simply test for max_total and leave the loop when we've not
found any work.

No backport is needed, that's 2.2-only.

5 years agoBUG/MINOR: http-rules: Fix ACLs parsing for http deny rules
Christopher Faulet [Tue, 30 Jun 2020 07:32:01 +0000 (09:32 +0200)] 
BUG/MINOR: http-rules: Fix ACLs parsing for http deny rules

The parsing of http deny rules with no argument or only the deny_status argument
is buggy if followed by an ACLs expression (starting with "if" or "unless"
keyword). Instead of using the proxy errorfiles, a dummy error is used. To fix
the bug, the parsing function must also check for "if" or "unless" keyword in
such cases.

This patch should fix the issue #720. No backport is needed.

5 years agoMEDIUM: server: use the two thresholds for the connection release algorithm
Willy Tarreau [Mon, 29 Jun 2020 18:55:53 +0000 (20:55 +0200)] 
MEDIUM: server: use the two thresholds for the connection release algorithm

The algorithm improvement in bdb86bd ("MEDIUM: server: improve estimate
of the need for idle connections") is still not enough because there's
a hard limit between below and above the FD count, so it continues to
end up with many killed connections.

Here we're proceeding differently. Given that there are two configured
limits, a low and a high one, what we do is that we drop connections
when the high limit is reached (what's already done by the killing task
anyway), when we're between the low and the high threshold, we only keep
the connection if our idle entries are empty (with a preference for safe
ones), and below the low threshold, we keep any connection so as to give
them a chance of being reused or taken over by another thread.

Proceeding like this results in much less dropped connections, we
typically see a 99.3% reuse rate (76k conns for 10M requests over 200
servers and 4 threads, with 335k takeovers or 3%), and much less CPU
usage variations because there are no more bursts to try to kill extra
connections.

It should be possible to further improve this by counting the number
of threads exploiting a server and trying to optimize the amount of
per-thread idle connections so that it is approximately balanced among
the threads.

5 years agoBUG/MINOR: server: always count one idle slot for current thread
Willy Tarreau [Mon, 29 Jun 2020 18:37:29 +0000 (20:37 +0200)] 
BUG/MINOR: server: always count one idle slot for current thread

The idle server connection estimates brought in commit bdb86bd ("MEDIUM:
server: improve estimate of the need for idle connections") were committed
without the minimum of 1 idle conn needed for the current thread. The net
effect is that there are bursts of dropped connections when the load varies
because there's no provision for the last connection.

No backport needed, this is 2.2-dev.

5 years agoBUG/MINOR: haproxy: don't wake already stopping threads on exit
Willy Tarreau [Mon, 29 Jun 2020 17:23:19 +0000 (19:23 +0200)] 
BUG/MINOR: haproxy: don't wake already stopping threads on exit

Commit d645574 ("MINOR: soft-stop: let the first stopper only signal
other threads") introduced a minor mistake which is that when a stopping
thread signals all other threads, it also signals itself. When
single-threaded, the process constantly wakes up while waiting for
last connections to exit. Let's reintroduce the lost mask to avoid
this.

No backport is needed, this is 2.2-dev only.

5 years agoRevert "BUG/MEDIUM: lists: Lock the element while we check if it is in a list."
Willy Tarreau [Mon, 29 Jun 2020 19:37:57 +0000 (21:37 +0200)] 
Revert "BUG/MEDIUM: lists: Lock the element while we check if it is in a list."

This reverts previous commit 347bbf79d20e1cff57075a8a378355dfac2475e2i.

The original code was correct. This patch resulted from a mistaken analysis
and breaks the scheduler:

 ########################## Starting vtest ##########################
 Testing with haproxy version: 2.2-dev11-90b7d9-23
 #    top  TEST reg-tests/lua/close_wait_lf.vtc TIMED OUT (kill -9)
 #    top  TEST reg-tests/lua/close_wait_lf.vtc FAILED (10.008) signal=9
 1 tests failed, 0 tests skipped, 88 tests passed

 Program terminated with signal SIGABRT, Aborted.
 [Current thread is 1 (Thread 0x7fb0dac2c700 (LWP 11292))]
 (gdb) bt
 #0  0x00007fb0e7c143f8 in raise () from /lib64/libc.so.6
 #1  0x00007fb0e7c15ffa in abort () from /lib64/libc.so.6
 #2  0x000000000053f5d6 in ha_panic () at src/debug.c:269
 #3  0x00000000005a6248 in wdt_handler (sig=14, si=<optimized out>, arg=<optimized out>) at src/wdt.c:119
 #4  <signal handler called>
 #5  0x00000000004fbccd in tasklet_wakeup (tl=0x1b5abc0) at include/haproxy/task.h:351
 #6  listener_accept (fd=<optimized out>) at src/listener.c:999
 #7  0x00000000004262df in fd_update_events (evts=<optimized out>, fd=6) at include/haproxy/fd.h:418
 #8  _do_poll (p=<optimized out>, exp=<optimized out>, wake=<optimized out>) at src/ev_epoll.c:251
 #9  0x0000000000548d0f in run_poll_loop () at src/haproxy.c:2949
 #10 0x000000000054908b in run_thread_poll_loop (data=<optimized out>) at src/haproxy.c:3067
 #11 0x00007fb0e902b684 in start_thread () from /lib64/libpthread.so.0
 #12 0x00007fb0e7ce5eed in clone () from /lib64/libc.so.6
 (gdb) up
 #5  0x00000000004fbccd in tasklet_wakeup (tl=0x1b5abc0) at include/haproxy/task.h:351
 351                     if (MT_LIST_ADDQ(&task_per_thread[tl->tid].shared_tasklet_list, (struct mt_list *)&tl->list) == 1) {

If the commit above is ever backported, this one must be as well!

5 years agoBUG/MEDIUM: lists: Lock the element while we check if it is in a list.
Olivier Houchard [Mon, 29 Jun 2020 17:52:01 +0000 (19:52 +0200)] 
BUG/MEDIUM: lists: Lock the element while we check if it is in a list.

In MT_LIST_ADDQ() and MT_LIST_ADD() we can't just check if the element is
already in a list, because there's a small race condition, it could be added
between the time we checked, and the time we actually set its next and prev.
So we have to lock it first.

This should be backported to 2.1.

5 years agoBUG/MINOR: threads: Don't forget to init each thread toremove_lock.
Olivier Houchard [Mon, 29 Jun 2020 15:48:27 +0000 (17:48 +0200)] 
BUG/MINOR: threads: Don't forget to init each thread toremove_lock.

Don't forget to use HA_SPIN_INIT() on each toremove_lock, or DEBUG_THREAD may
not work reliably with it.

This should be backported to 2.1 and 2.0.

5 years agoMINOR: stats: add the estimated need of concurrent connections per server
Willy Tarreau [Mon, 29 Jun 2020 13:38:53 +0000 (15:38 +0200)] 
MINOR: stats: add the estimated need of concurrent connections per server

The max_used_conns value is used as an estimate of the needed number of
connections on a server to know how many to keep open. But this one is
not reported, making it hard to troubleshoot reuse issues. Let's export
it in the sessions/current column.

5 years agoMEDIUM: server: improve estimate of the need for idle connections
Willy Tarreau [Mon, 29 Jun 2020 13:56:35 +0000 (15:56 +0200)] 
MEDIUM: server: improve estimate of the need for idle connections

Starting with commit 079cb9a ("MEDIUM: connections: Revamp the way idle
connections are killed") we started to improve the way to compute the
need for idle connections. But the condition to keep a connection idle
or drop it when releasing it was not updated. This often results in
storms of close when certain thresholds are met, and long series of
takeover() when there aren't enough connections left for a thread on
a server.

This patch tries to improve the situation this way:
  - it keeps an estimate of the number of connections needed for a server.
    This estimate is a copy of the max over previous purge period, or is a
    max of what is seen over current period; it differs from max_used_conns
    in that this one is a counter that's reset on each purge period ;

  - when releasing, if the number of current idle+used connections is
    lower than this last estimate, then we'll keep the connection;

  - when releasing, if the current thread's idle conns head is empty,
    and we don't exceed the estimate by the number of threads, then
    we'll keep the connection.

  - when cleaning up connections, we consider the max of the last two
    periods to avoid killing too many idle conns when facing bursty
    traffic.

Thanks to this we can better converge towards a situation where, provided
there are enough FDs, each active server keeps at least one idle connection
per thread all the time, with a total number close to what was needed over
the previous measurement period (as defined by pool-purge-delay).

On tests with large numbers of concurrent connections (30k) and many
servers (200), this has quite smoothed the CPU usage pattern, increased
the reuse rate and roughly halved the takeover rate.

5 years agoBUG/MINOR: server: start cleaning idle connections from various points
Willy Tarreau [Mon, 29 Jun 2020 12:43:16 +0000 (14:43 +0200)] 
BUG/MINOR: server: start cleaning idle connections from various points

There's a minor glitch with the way idle connections start to be evicted.
The lookup always goes from thread 0 to thread N-1. This causes depletion
of connections on the first threads and abundance on the last ones. This
is visible with the takeover() stats below:

 $ socat - /tmp/sock1 <<< "show activity"|grep ^fd ; \
   sleep 10 ; \
   socat -/tmp/sock1 <<< "show activity"|grep ^fd
 fd_takeover: 300144 [ 91887 84029 66254 57974 ]
 fd_takeover: 359631 [ 111369 99699 79145 69418 ]

There are respectively 19k, 15k, 13k and 11k takeovers for only 4 threads,
indicating that the first thread needs a foreign FD twice more often than
the 4th one.

This patch changes this si that all threads are scanned in round robin
starting with the current one. The takeovers now happen in a much more
distributed way (about 4 times 9k) :

  fd_takeover: 1420081 [ 359562 359453 346586 354480 ]
  fd_takeover: 1457044 [ 368779 368429 355990 363846 ]

There is no need to backport this, as this happened along a few patches
that were merged during 2.2 development.

5 years agoMINOR: activity: add per-thread statistics on FD takeover
Willy Tarreau [Mon, 29 Jun 2020 12:17:59 +0000 (14:17 +0200)] 
MINOR: activity: add per-thread statistics on FD takeover

The FD takeover operation might have certain impacts explaining
unexpected activities, so it's important to report such a counter
there. We thus count the number of times a thread has stolen an
FD from another thread.

5 years agoMINOR: stats: add 3 new output values for the per-server idle conn state
Willy Tarreau [Mon, 29 Jun 2020 11:51:05 +0000 (13:51 +0200)] 
MINOR: stats: add 3 new output values for the per-server idle conn state

The servers have internal states describing the status of idle connections,
unfortunately these were not exported in the stats. This patch adds the 3
following gauges:

 - idle_conn_cur : Current number of unsafe idle connections
 - safe_conn_cur : Current number of safe idle connections
 - used_conn_cur : Current number of connections in use

5 years agoBUG/MINOR: debug: fix "show fd" null-deref when built with DEBUG_FD
Willy Tarreau [Mon, 29 Jun 2020 12:23:31 +0000 (14:23 +0200)] 
BUG/MINOR: debug: fix "show fd" null-deref when built with DEBUG_FD

DEBUG_FD was added by commit 38e8a1c in 2.2-dev, and "show fd" was
slightly modified to still allow to print orphaned/closed FDs if their
count is non-null. But bypassing the existing test made it possible
to dereference fdt.owner which can be null. Let's adjust the condition
to avoid this.

No backport is needed.

5 years agoMINOR: pools: move the LRU cache heads to thread_info
Willy Tarreau [Sat, 27 Jun 2020 22:54:27 +0000 (00:54 +0200)] 
MINOR: pools: move the LRU cache heads to thread_info

The LRU cache head was an array of list, which causes false sharing
between 4 to 8 threads in the same cache line. Let's move it to the
thread_info structure instead. There's no need to do the same for the
pool_cache[] array since it's already quite large (32 pointers each).

By doing this the request rate increased by 1% on a 16-thread machine.

5 years agoCLEANUP: pool: only include the type files from types
Willy Tarreau [Mon, 29 Jun 2020 08:11:24 +0000 (10:11 +0200)] 
CLEANUP: pool: only include the type files from types

pool-t.h was mistakenly including the full-blown includes for threads,
lists and api instead of the types, and as such, CONFIG_HAP_LOCAL_POOLS
and CONFIG_HAP_LOCKLESS_POOLS were not visible everywhere.

5 years agoREORG: includes: create tinfo.h for the thread_info struct
Willy Tarreau [Mon, 29 Jun 2020 07:57:23 +0000 (09:57 +0200)] 
REORG: includes: create tinfo.h for the thread_info struct

The thread_info struct is convenient to store various per-thread info
without having to resort to a painful thread_local storage which is
slow and painful to initialize.

The problem is, by having this one in thread.h it's very difficult to
add more entries there because everyone already includes thread.h so
conversely thread.h cannot reference certain types.

There's no point in having this there, instead let's create a new pair
of files, tinfo{,-t}.h, which declare the structure. This way it will
become possible to extend them with other includes and have certain
files store their own types there.

5 years agoREORG: buffer: rename buffer.c to dynbuf.c
Willy Tarreau [Mon, 29 Jun 2020 07:26:59 +0000 (09:26 +0200)] 
REORG: buffer: rename buffer.c to dynbuf.c

The include part was renamed by commit 2741c8c but I somehow missed
the renaming of the C file, whose name didn't match the H file anymore.

5 years agoBUG/MEDIUM: checks: Increment the server's curr_used_conns
Olivier Houchard [Sun, 28 Jun 2020 14:23:09 +0000 (16:23 +0200)] 
BUG/MEDIUM: checks: Increment the server's curr_used_conns

In tcpcheck_eval_connect(), if we're targetting a server, increase its
curr_used_conns when creating a new connection, as the counter will be
decreased later when the connection is destroyed and conn_free() is called.

5 years agoBUG/MEDIUM: connections: Don't increase curr_used_conns for shared connections.
Olivier Houchard [Sun, 28 Jun 2020 14:14:09 +0000 (16:14 +0200)] 
BUG/MEDIUM: connections: Don't increase curr_used_conns for shared connections.

In connect_server(), we want to increase curr_used_conns only if the
connection is new, or if it comes from an idle_pool, otherwise it means
the connection is already used by at least one another stream, and it is
already accounted for.

5 years agoCONTRIB: debug: add missing flags SI_FL_L7_RETRY & SI_FL_D_L7_RETRY
Willy Tarreau [Sun, 28 Jun 2020 14:05:39 +0000 (16:05 +0200)] 
CONTRIB: debug: add missing flags SI_FL_L7_RETRY & SI_FL_D_L7_RETRY

These ones were missing from debug/flags.

5 years agoMINOR: connection: align toremove_{lock,connections} and cleanup into idle_conns
Willy Tarreau [Sat, 27 Jun 2020 22:19:17 +0000 (00:19 +0200)] 
MINOR: connection: align toremove_{lock,connections} and cleanup into idle_conns

We used to have 3 thread-based arrays for toremove_lock, idle_cleanup,
and toremove_connections. The problem is that these items are small,
and that this creates false sharing between threads since it's possible
to pack up to 8-16 of these values into a single cache line. This can
cause real damage where there is contention on the lock.

This patch creates a new array of struct "idle_conns" that is aligned
on a cache line and which contains all three members above. This way
each thread has access to its variables without hindering the other
ones. Just doing this increased the HTTP/1 request rate by 5% on a
16-thread machine.

The definition was moved to connection.{c,h} since it appeared a more
natural evolution of the ongoing changes given that there was already
one of them declared in connection.h previously.

5 years agoBUG/MEDIUM: buffers: always allocate from the local cache first
Willy Tarreau [Sun, 28 Jun 2020 08:31:15 +0000 (10:31 +0200)] 
BUG/MEDIUM: buffers: always allocate from the local cache first

It looked strange to see pool_evict_from_cache() always very present
on "perf top", but there was actually a reason to this: while b_free()
uses pool_free() which properly disposes the buffer into the local cache
and b_alloc_fast() allocates using pool_get_first() which considers the
local cache, b_alloc_margin() does not consider the local cache as it
only uses __pool_get_first() which only allocates from the shared pools.

The impact is that basically everywhere a buffer is allocated (muxes,
streams, applets), it's always picked from the shared pool (hence
involves locking) and is released to the local one and makes it grow
until it's required to trigger a flush using pool_evict_from_cache().
Buffers usage are thus not thread-local at all, and cause eviction of
a lot of possibly useful objects from the local caches.

Just fixing this results in a 10% request rate increase in an HTTP/1 test
on a 16-thread machine.

This bug was caused by recent commit ed891fd ("MEDIUM: memory: make local
pools independent on lockless pools") merged into 2.2-dev9, so not backport
is needed.

5 years agoCLEANUP: buffers: remove unused buffer_wq_lock lock
Willy Tarreau [Sun, 28 Jun 2020 05:31:42 +0000 (07:31 +0200)] 
CLEANUP: buffers: remove unused buffer_wq_lock lock

Commit 2104659 ("MEDIUM: buffer: remove the buffer_wq lock") removed
usage of the lock but not the lock itself. It's totally unused, let's
remove it.

5 years agoMINOR: cli: make "show sess" stop at the last known session
Willy Tarreau [Sat, 27 Jun 2020 23:24:12 +0000 (01:24 +0200)] 
MINOR: cli: make "show sess" stop at the last known session

"show sess" and particularly "show sess all" can be very slow when dumping
lots of information, and while dumping, new sessions might appear, making
the output really endless. When threads are used, this causes a double
problem:
  - all threads are paused during the dump, so an overly long dump degrades
    the quality of service ;

  - since all threads are paused, more events get postponed, possibly
    resulting in more streams to be dumped on next invocation of the dump
    function.

This patch addresses this long-lasting issue by doing something simple:
the CLI's stream is moved at the end of the steams list, serving as an
identifiable marker to end the dump, because all entries past it were
added after the command was entered. As a result, the CLI's stream always
appears as the last one.

It may make sense to backport this to stable branches where dumping live
streams is difficult as well.

5 years agoBUG/MINOR: mux_h2: don't lose the leaving trace in h2_io_cb()
Willy Tarreau [Sat, 27 Jun 2020 22:31:13 +0000 (00:31 +0200)] 
BUG/MINOR: mux_h2: don't lose the leaving trace in h2_io_cb()

Commit cd4159f ("MEDIUM: mux_h2: Implement the takeover() method.")
added a return in the middle of the function, and as usual with such
stray return statements, some unrolling was lost. Here it's only the
TRACE_LEAVE() call, so it's mostly harmless. That's 2.2 only, no
backport is needed.

5 years ago[RELEASE] Released version 2.2-dev11 v2.2-dev11
Willy Tarreau [Fri, 26 Jun 2020 20:01:04 +0000 (22:01 +0200)] 
[RELEASE] Released version 2.2-dev11

Released version 2.2-dev11 with the following main changes :
    - REGTEST: Add a simple script to tests errorfile directives in proxy sections
    - BUG/MEDIUM: fcgi-app: Resolve the sink if a fcgi-app logs in a ring buffer
    - BUG/MINOR: spoe: correction of setting bits for analyzer
    - BUG/MINOR: cfgparse: Support configurations without newline at EOF
    - MINOR: cfgparse: Warn on truncated lines / files
    - BUG/MINOR: http_ana: clarify connection pointer check on L7 retry
    - MINOR: debug: add a new DEBUG_FD build option
    - BUG/MINOR: tasks: make sure never to exceed max_processed
    - MINOR: task: add a new pointer to current tasklet queue
    - BUG/MEDIUM: task: be careful not to run too many tasks at TL_URGENT
    - BUG/MINOR: cfgparse: Fix argument reference in PARSE_ERR_TOOMANY message
    - BUG/MINOR: cfgparse: Fix calculation of position for PARSE_ERR_TOOMANY message
    - BUG/MEDIUM: ssl: fix ssl_bind_conf double free
    - MINOR: ssl: free bind_conf_node in crtlist_free()
    - MINOR: ssl: free the crtlist and the ckch during the deinit()
    - BUG/MINOR: ssl: fix build with ckch_deinit() and crtlist_deinit()
    - BUG/MINOR: ssl/cli: certs added from the CLI can't be deleted
    - MINOR: ssl: move the ckch/crtlist deinit to ssl_sock.c
    - MEDIUM: tasks: apply a fair CPU distribution between tasklet classes
    - MINOR: tasks: make current_queue an index instead of a pointer
    - MINOR: tasks: add a mask of the queues with active tasklets
    - MINOR: tasks: pass the queue index to run_task_from_list()
    - MINOR: tasks: make run_tasks_from_lists() scan the queues itself
    - MEDIUM: tasks: add a tune.sched.low-latency option
    - BUG/MEDIUM: ssl/cli: 'commit ssl cert' crashes when no private key
    - BUG/MINOR: cfgparse: don't increment linenum on incomplete lines
    - MINOR: tools: make parse_line() always terminate the args list
    - BUG/MINOR: cfgparse: report extraneous args *after* the string is allocated
    - MINOR: cfgparse: sanitize the output a little bit
    - MINOR: cli/ssl: handle trailing slashes in crt-list commands
    - MINOR: ssl: add the ssl_s_* sample fetches for server side certificate
    - BUG/MEDIUM: http-ana: Don't loop trying to generate a malformed 500 response
    - BUG/MINOR: stream-int: Don't wait to send truncated HTTP messages
    - BUG/MINOR: http-ana: Set CF_EOI on response channel for generated responses
    - BUG/MINOR: http-ana: Don't wait to send 1xx responses generated by HAProxy
    - MINOR: spoe: Don't systematically create new applets if processing rate is low
    - DOC: fix some typos in the ssl_s_{s|i}_dn documentation
    - BUILD: fix ssl_sample.c when building against BoringSSL
    - CI: travis-ci: switch BoringSSL builds to ninja
    - CI: extend spellchecker whitelist
    - DOC: assorted typo fixes in the documentation
    - CLEANUP: assorted typo fixes in the code and comments
    - MINOR: http: Add support for http 413 status
    - REGTEST: ssl: tests the ssl_f_* sample fetches
    - REGTEST: ssl: add some ssl_c_* sample fetches test
    - DOC: ssl: update the documentation of "commit ssl cert"
    - BUG/MINOR: cfgparse: correctly deal with empty lines
    - BUG/MEDIUM: fetch: Fix hdr_ip misparsing IPv4 addresses due to missing NUL

5 years agoBUG/MEDIUM: fetch: Fix hdr_ip misparsing IPv4 addresses due to missing NUL
Tim Duesterhus [Fri, 26 Jun 2020 13:44:48 +0000 (15:44 +0200)] 
BUG/MEDIUM: fetch: Fix hdr_ip misparsing IPv4 addresses due to missing NUL

The IPv4 code did not take into account that the header value might not
contain the trailing NUL byte, possibly reading stray data after the header
value, failing the parse and testing the IPv6 branch. That one adds the
missing NUL, but fails to parse IPv4 addresses.

Fix this issue by always adding the trailing NUL.

The bug was reported on GitHub as issue #715.

It's not entirely clear when this bug started appearing, possibly earlier
versions of smp_fetch_hdr guaranteed the NUL termination. However the
addition of the NUL in the IPv6 case was added together with IPv6 support,
hinting that at that point in time the NUL was not guaranteed.

The commit that added IPv6 support was 69fa99292e689e355080d83ab19db4698b7c502b
which first appeared in HAProxy 1.5. This patch should be backported to
1.5+, taking into account the various buffer / chunk changes and the movement
across different files.

5 years agoBUG/MINOR: cfgparse: correctly deal with empty lines
Willy Tarreau [Fri, 26 Jun 2020 15:24:54 +0000 (17:24 +0200)] 
BUG/MINOR: cfgparse: correctly deal with empty lines

Issue 23653 in oss-fuzz reports a heap overflow bug which is in fact a
bug introduced by commit 9e1758efb ("BUG/MEDIUM: cfgparse: use
parse_line() to expand/unquote/unescape config lines") to address
oss-fuzz issue 22689, which was only partially fixed by commit 70f58997f
("BUG/MINOR: cfgparse: Support configurations without newline at EOF").

Actually on an empty line, end == line so we cannot dereference end-1
to check for a trailing LF without first being sure that end is greater
than line.

No backport is needed, this is 2.2 only.

5 years agoDOC: ssl: update the documentation of "commit ssl cert"
William Lallemand [Fri, 26 Jun 2020 13:39:57 +0000 (15:39 +0200)] 
DOC: ssl: update the documentation of "commit ssl cert"

Update the documentation of "commit ssl cert" in management.txt to
explain the behavior with new certificates.

5 years agoREGTEST: ssl: add some ssl_c_* sample fetches test
William Lallemand [Fri, 26 Jun 2020 10:03:45 +0000 (12:03 +0200)] 
REGTEST: ssl: add some ssl_c_* sample fetches test

Test the following ssl sample fetches:

ssl_c_der, ssl_c_sha1,hex, ssl_c_notafter, ssl_c_notbefore,
ssl_c_sig_alg, ssl_c_i_dn, ssl_c_s_dn, ssl_c_serial,hex, ssl_c_key_alg,
ssl_c_version

This reg-test could be used as far as haproxy 1.6.

5 years agoREGTEST: ssl: tests the ssl_f_* sample fetches
William Lallemand [Fri, 26 Jun 2020 09:29:43 +0000 (11:29 +0200)] 
REGTEST: ssl: tests the ssl_f_* sample fetches

Test the following ssl sample fetches:

ssl_f_der, ssl_f_sha1,hex, ssl_f_notafter, ssl_f_notbefore,
ssl_f_sig_alg, ssl_f_i_dn, ssl_f_s_dn, ssl_f_serial,hex, ssl_f_key_alg,
ssl_f_version

This reg-test could be used as far as haproxy 1.5.

5 years agoMINOR: http: Add support for http 413 status
Anthonin Bonnefoy [Mon, 22 Jun 2020 07:17:01 +0000 (09:17 +0200)] 
MINOR: http: Add support for http 413 status

Add 413 http "payload too large" status code. This will allow 413 to be
used in deny_status and errorfile.

5 years agoCLEANUP: assorted typo fixes in the code and comments
Ilya Shipitsin [Sun, 21 Jun 2020 16:42:57 +0000 (21:42 +0500)] 
CLEANUP: assorted typo fixes in the code and comments

This is 10th iteration of typo fixes

5 years agoDOC: assorted typo fixes in the documentation
Ilya Shipitsin [Sun, 21 Jun 2020 16:18:27 +0000 (21:18 +0500)] 
DOC: assorted typo fixes in the documentation

this is 10th iteration of typo fixes

5 years agoCI: extend spellchecker whitelist
Ilya Shipitsin [Sun, 21 Jun 2020 16:06:55 +0000 (21:06 +0500)] 
CI: extend spellchecker whitelist

let us ignore *.pem files

5 years agoCI: travis-ci: switch BoringSSL builds to ninja
Ilya Shipitsin [Sun, 21 Jun 2020 11:39:30 +0000 (16:39 +0500)] 
CI: travis-ci: switch BoringSSL builds to ninja

using ninja instead of make speed up build by 40 sec

5 years agoBUILD: fix ssl_sample.c when building against BoringSSL
Ilya Shipitsin [Sat, 20 Jun 2020 18:38:37 +0000 (23:38 +0500)] 
BUILD: fix ssl_sample.c when building against BoringSSL

BoringSSL does not have X509_get_X509_PUBKEY
let our emulation level define that for BoringSSL as well

Build log:

src/ssl_sample.o: In function `smp_fetch_ssl_x_key_alg':
/home/travis/build/haproxy/haproxy/src/ssl_sample.c:592: undefined reference to `X509_get_X509_PUBKEY'
clang-7: error: linker command failed with exit code 1 (use -v to see invocation)
Makefile:860: recipe for target 'haproxy' failed
make: *** [haproxy] Error 1

travis-ci: https://travis-ci.com/github/haproxy/haproxy/jobs/351670996

5 years agoDOC: fix some typos in the ssl_s_{s|i}_dn documentation
William Lallemand [Fri, 26 Jun 2020 07:55:06 +0000 (09:55 +0200)] 
DOC: fix some typos in the ssl_s_{s|i}_dn documentation

Fix some typos in the ssl_s_{s|i}_dn documentation.

5 years agoMINOR: spoe: Don't systematically create new applets if processing rate is low
Christopher Faulet [Mon, 22 Jun 2020 13:32:14 +0000 (15:32 +0200)] 
MINOR: spoe: Don't systematically create new applets if processing rate is low

When an event must be processed, we decide to create a new SPOE applet if there
is no idle applet at all or if the processing rate is lower than the number of
waiting events. But when the processing rate is very low (< 1 event/second), a
new applet is created independently of the number of idle applets.

Now, when there is at least one idle applet when there is only one event to
process, no new applet is created.

This patch is related to the issue #690.

5 years agoBUG/MINOR: http-ana: Don't wait to send 1xx responses generated by HAProxy
Christopher Faulet [Thu, 25 Jun 2020 13:55:11 +0000 (15:55 +0200)] 
BUG/MINOR: http-ana: Don't wait to send 1xx responses generated by HAProxy

When an informational response (1xx) is returned by HAProxy, we must be sure to
send it ASAP. To do so, CF_SEND_DONTWAIT flag must be set on the response
channel to instruct the stream-interface to not set the CO_SFL_MSG_MORE flag on
the transport layer. Otherwise the response delivery may be delayed, because of
the commit 8945bb6c0 ("BUG/MEDIUM: stream-int: fix loss of CO_SFL_MSG_MORE flag
in forwarding").

This patch may be backported as far as 1.9, for HTX part only. But this part has
changed in the 2.2, so it may be a bit tricky. Note it does not fix any known
bug on previous versions because the CO_SFL_MSG_MORE flag is ignored by the h1
mux.

5 years agoBUG/MINOR: http-ana: Set CF_EOI on response channel for generated responses
Christopher Faulet [Thu, 25 Jun 2020 13:36:45 +0000 (15:36 +0200)] 
BUG/MINOR: http-ana: Set CF_EOI on response channel for generated responses

To be consistent with other processings on the channels, when HAProxy generates
a final response, the CF_EOI flag must be set on the response channel. This flag
is used to know that a full message was pushed into the channel (HTX messages
with an EOM block). It is used in conjunction with other channel's flags in
stream-interface functions. Especially when si_cs_send() is called, to know if
we must set or not the CO_SFL_MSG_MORE flag. Without CF_EOI, the CO_SFL_MSG_MORE
flag is always set and the message forwarding is delayed.

This patch may be backported as far as 1.9, for HTX part only. But this part has
changed in the 2.2, so it may be a bit tricky. Note it does not fix any known
bug on previous versions because the CO_SFL_MSG_MORE flag is ignored by the h1
mux.

5 years agoBUG/MINOR: stream-int: Don't wait to send truncated HTTP messages
Christopher Faulet [Thu, 25 Jun 2020 14:11:20 +0000 (16:11 +0200)] 
BUG/MINOR: stream-int: Don't wait to send truncated HTTP messages

In HTX, since the commit 8945bb6c0 ("BUG/MEDIUM: stream-int: fix loss of
CO_SFL_MSG_MORE flag in forwarding"), the CO_SFL_MSG_MORE flag is set on the
transport layer if the end of the HTTP message is not reached, to delay the data
forwarding. To do so, the CF_EOI flag is tested and must not be set on the
output channel.

But the CO_SFL_MSG_MORE flag is also added if the message was truncated. Only
CF_SHUTR is set if this case. So the forwarding may be delayed to wait more data
that will never come. So, in HTX, the CO_SFL_MSG_MORE flag must not be set if
the message is finished (full or truncated).

No backport is needed.

5 years agoBUG/MEDIUM: http-ana: Don't loop trying to generate a malformed 500 response
Christopher Faulet [Thu, 25 Jun 2020 14:04:50 +0000 (16:04 +0200)] 
BUG/MEDIUM: http-ana: Don't loop trying to generate a malformed 500 response

When HAProxy generates a 500 response, if the formatting failed, for instance
because the message is larger than a buffer, it retries to format it in loop. To
fix the bug, we must stop trying to send a response if it is a non-rewritable
response (TX_CONST_REPLY flag is set on the HTTP transaction).

Because this part is not trivial, some comments have been added.

No backport is needed.

5 years agoMINOR: ssl: add the ssl_s_* sample fetches for server side certificate
William Lallemand [Thu, 25 Jun 2020 18:07:18 +0000 (20:07 +0200)] 
MINOR: ssl: add the ssl_s_* sample fetches for server side certificate

This commit adds some sample fetches that were lacking on the server
side:

ssl_s_key_alg, ssl_s_notafter, ssl_s_notbefore, ssl_s_sig_alg,
ssl_s_i_dn, ssl_s_s_dn, ssl_s_serial, ssl_s_sha1, ssl_s_der,
ssl_s_version

5 years agoMINOR: cli/ssl: handle trailing slashes in crt-list commands
William Lallemand [Thu, 25 Jun 2020 13:19:51 +0000 (15:19 +0200)] 
MINOR: cli/ssl: handle trailing slashes in crt-list commands

Trailing slashes were not handled in crt-list commands on CLI which can
be useful when you use the commands with a directory.

Strip the slashes before looking for the crtlist in the tree.

5 years agoMINOR: cfgparse: sanitize the output a little bit
Willy Tarreau [Thu, 25 Jun 2020 07:15:40 +0000 (09:15 +0200)] 
MINOR: cfgparse: sanitize the output a little bit

With the rework of the config line parser, we've started to emit a dump
of the initial line underlined by a caret character indicating the error
location. But with extremely large lines it starts to take time and can
even cause trouble to slow terminals (e.g. over ssh), and this becomes
useless. In addition, control characters could be dumped as-is which is
bad, especially when the input file is accidently wrong (an executable).

This patch adds a string sanitization function which isolates an area
around the error position in order to report only that area if the string
is too large. The limit was set to 80 characters, which will result in
roughly 40 chars around the error being reported only, prefixed and suffixed
with "..." as needed. In addition, non-printable characters in the line are
now replaced with '?' so as not to corrupt the terminal. This way invalid
variable names, unmatched quotes etc will be easier to spot.

A typical output is now:

  [ALERT] 176/092336 (23852) : parsing [bad.cfg:8]: forbidden first char in environment variable name at position 811957:
    ...c$PATH$PATH$d(xlc`%?$PATH$PATH$dgc?T$%$P?AH?$PATH$PATH$d(?$PATH$PATH$dgc?%...
                                            ^

5 years agoBUG/MINOR: cfgparse: report extraneous args *after* the string is allocated
Willy Tarreau [Thu, 25 Jun 2020 05:41:22 +0000 (07:41 +0200)] 
BUG/MINOR: cfgparse: report extraneous args *after* the string is allocated

The config parser change in commit 9e1758efb ("BUG/MEDIUM: cfgparse: use
parse_line() to expand/unquote/unescape config lines") is wrong when
displaying the last parsed word, because it doesn't verify that the output
string was properly allocated. This may fail in two cases:
  - very first line (outline is NULL, as in oss-fuzz issue 23657)
  - much longer line than previous ones, requiring a realloc(), in which
    case the final 0 is out of the allocated space.

This patch moves the reporting after the allocation check to fix this.

No backport is needed, this is 2.2 only.

5 years agoMINOR: tools: make parse_line() always terminate the args list
Willy Tarreau [Thu, 25 Jun 2020 05:35:42 +0000 (07:35 +0200)] 
MINOR: tools: make parse_line() always terminate the args list

parse_line() as added in commit c8d167bcf ("MINOR: tools: add a new
configurable line parse, parse_line()") presents an difficult usage
because it's up to the caller to determine the last written argument
based on what was passed to it. In practice the only way to safely
use it is for the caller to always pass nbarg-1 and make that last
entry point to the last arg + its strlen. This is annoying because
it makes it as painful to use as the infamous strncpy() while it has
all the information the caller needs.

This patch changes its behavior so that it guarantees that at least
one argument will point to the trailing zero at the end of the output
string, as long as there is at least one argument. The caller just
has to pass +1 to the arg count to make sure at least a last one is
empty.

5 years agoBUG/MINOR: cfgparse: don't increment linenum on incomplete lines
Willy Tarreau [Thu, 25 Jun 2020 07:37:54 +0000 (09:37 +0200)] 
BUG/MINOR: cfgparse: don't increment linenum on incomplete lines

When fgets() returns an incomplete line we must not increment linenum
otherwise line numbers become incorrect. This may happen when parsing
files with extremely long lines which require a realloc().

The bug has been present since unbounded line length was supported, so
the fix should be backported to older branches.

5 years agoBUG/MEDIUM: ssl/cli: 'commit ssl cert' crashes when no private key
William Lallemand [Wed, 24 Jun 2020 14:26:41 +0000 (16:26 +0200)] 
BUG/MEDIUM: ssl/cli: 'commit ssl cert' crashes when no private key

A crash was reported in issue #707 because the private key was not
uploaded correctly with "set ssl cert".

The bug is provoked by X509_check_private_key() being called when there
is no private key, which can lead to a segfault.

This patch adds a check and return an error is the private key is not
present.

This must be backported in 2.1.

5 years agoMEDIUM: tasks: add a tune.sched.low-latency option
Willy Tarreau [Wed, 24 Jun 2020 09:11:02 +0000 (11:11 +0200)] 
MEDIUM: tasks: add a tune.sched.low-latency option

Now that all tasklet queues are scanned at once by run_tasks_from_lists(),
it becomes possible to always check for lower priority classes and jump
back to them when they exist.

This patch adds tune.sched.low-latency global setting to enable this
behavior. What it does is stick to the lowest ranked priority list in
which tasks are still present with an available budget, and leave the
loop to refill the tasklet lists if the trees got new tasks or if new
work arrived into the shared urgent queue.

Doing so allows to cut the latency in half when running with extremely
deep run queues (10k-100k), thus allowing forwarding of small and large
objects to coexist better. It remains off by default since it does have
a small impact on large traffic by default (shorter batches).

5 years agoMINOR: tasks: make run_tasks_from_lists() scan the queues itself
Willy Tarreau [Wed, 24 Jun 2020 08:17:29 +0000 (10:17 +0200)] 
MINOR: tasks: make run_tasks_from_lists() scan the queues itself

Now process_runnable_tasks is responsible for calculating the budgets
for each queue, dequeuing from the tree, and calling run_tasks_from_lists().
This latter one scans the queues, picking tasks there and respecting budgets.
Note that its name was updated with a plural "s" for this reason.

5 years agoMINOR: tasks: pass the queue index to run_task_from_list()
Willy Tarreau [Wed, 24 Jun 2020 07:54:24 +0000 (09:54 +0200)] 
MINOR: tasks: pass the queue index to run_task_from_list()

Instead of passing it a pointer to the queue, pass it the queue's index
so that it can perform all the work around current_queue and tl_class_mask.

5 years agoMINOR: tasks: add a mask of the queues with active tasklets
Willy Tarreau [Wed, 24 Jun 2020 07:39:48 +0000 (09:39 +0200)] 
MINOR: tasks: add a mask of the queues with active tasklets

It is neither convenient nor scalable to check each and every tasklet
queue to figure whether it's empty or not while we often need to check
them all at once. This patch introduces a tasklet class mask which gets
a bit 1 set for each queue representing one class of service. A single
test on the mask allows to figure whether there's still some work to be
done. It will later be usable to better factor the runqueue code.

Bits are set when tasklets are queued. They're cleared when queues are
emptied. It is possible that a queue is empty but has a bit if a tasklet
was added then removed, but this is not a problem as this is properly
checked for in run_tasks_from_list().

5 years agoMINOR: tasks: make current_queue an index instead of a pointer
Willy Tarreau [Wed, 24 Jun 2020 07:19:50 +0000 (09:19 +0200)] 
MINOR: tasks: make current_queue an index instead of a pointer

It will be convenient to have the tasklet queue number soon, better make
current_queue an index rather than a pointer to the queue. When not currently
running (e.g. from I/O), the index is -1.

5 years agoMEDIUM: tasks: apply a fair CPU distribution between tasklet classes
Willy Tarreau [Wed, 24 Jun 2020 05:21:08 +0000 (07:21 +0200)] 
MEDIUM: tasks: apply a fair CPU distribution between tasklet classes

Till now in process_runnable_tasks() we used to reserve a fixed portion
of max_processed to urgent tasks, then a portion of what remains for
normal tasks, then what remains for bulk tasks. This causes two issues:

  - the current budget for processed tasks could be drained once for
    all by higher level tasks so that they couldn't have enough left
    for the next run. For example, if bulk tasklets cause task wakeups,
    the required share to run them could be eaten by other bulk tasklets.

  - it forces the urgent tasks to be run before scanning the tree so that
    we know how many tasks to pick from the tree, and this isn't very
    efficient cache-wise.

This patch changes this so that we compute upfront how max_processed will
be shared between classes that require so. We can then decide in advance
to pick a certain number of tasks from the tree, then execute all tasklets
in turn. When reaching the end, if there's still some budget, we can go
back and do the same thing again, improving chances to pick new work
before the global budget is depleted.

The default weights have been set to 50% for urgent tasklets, 37% for
normal ones and 13% for the bulk ones. In practice, there are not that
many urgent tasklets but when they appear they are cheap and must be
processed in as large batches as possible. Every time there is nothing
to pick there, the unused budget is shared between normal and bulk and
this allows bulk tasklets to still have quite some CPU to run on.

5 years agoMINOR: ssl: move the ckch/crtlist deinit to ssl_sock.c
William Lallemand [Wed, 24 Jun 2020 07:54:29 +0000 (09:54 +0200)] 
MINOR: ssl: move the ckch/crtlist deinit to ssl_sock.c

Move the ckch_deinit() and crtlist_deinit() call to ssl_sock.c,
also unlink the SNI from the ckch_inst because they are free'd before in
ssl_sock_free_all_ctx().

5 years agoBUG/MINOR: ssl/cli: certs added from the CLI can't be deleted
William Lallemand [Tue, 23 Jun 2020 23:00:52 +0000 (01:00 +0200)] 
BUG/MINOR: ssl/cli: certs added from the CLI can't be deleted

In ticket #706 it was reported that a certificate which was added from
the CLI can't be removed with 'del ssl cert' and is marked as 'Used'.

The problem is that the certificate instances are not added to the
created crtlist_entry, so they can't be deleted upon a 'del ssl
crt-list', and the store can't never be marked 'Unused' because of this.

This patch fixes the issue by adding the instances to the crtlist_entry,
which is enough to fix the issue.

5 years agoBUG/MINOR: ssl: fix build with ckch_deinit() and crtlist_deinit()
William Lallemand [Tue, 23 Jun 2020 18:25:07 +0000 (20:25 +0200)] 
BUG/MINOR: ssl: fix build with ckch_deinit() and crtlist_deinit()

ee8530c ("MINOR: ssl: free the crtlist and the ckch during the
deinit()") introduced a build problem because it lacks the right
includes in haproxy.c

5 years agoMINOR: ssl: free the crtlist and the ckch during the deinit()
William Lallemand [Tue, 23 Jun 2020 16:19:42 +0000 (18:19 +0200)] 
MINOR: ssl: free the crtlist and the ckch during the deinit()

Add some functions to deinit the whole crtlist and ckch architecture.

It will free all crtlist, crtlist_entry, ckch_store, ckch_inst and their
associated SNI, ssl_conf and SSL_CTX.

The SSL_CTX in the default_ctx and initial_ctx still needs to be free'd
separately.

5 years agoMINOR: ssl: free bind_conf_node in crtlist_free()
William Lallemand [Tue, 23 Jun 2020 09:43:35 +0000 (11:43 +0200)] 
MINOR: ssl: free bind_conf_node in crtlist_free()

Free the list of bind_conf using a crt-list in crtlist_free()

5 years agoBUG/MEDIUM: ssl: fix ssl_bind_conf double free
William Lallemand [Tue, 23 Jun 2020 09:02:17 +0000 (11:02 +0200)] 
BUG/MEDIUM: ssl: fix ssl_bind_conf double free

Since commit 2954c47 ("MEDIUM: ssl: allow crt-list caching"), the
ssl_bind_conf is allocated directly in the crt-list, and the crt-list
can be shared between several bind_conf. The deinit() code wasn't
changed to handle that.

This patch fixes the issue by removing the free of the ssl_conf in
ssl_sock_free_all_ctx().

It should be completed with a patch that free the ssl_conf and the
crt-list.

Fix issue #700.

5 years agoBUG/MINOR: cfgparse: Fix calculation of position for PARSE_ERR_TOOMANY message
Tim Duesterhus [Tue, 23 Jun 2020 16:56:10 +0000 (18:56 +0200)] 
BUG/MINOR: cfgparse: Fix calculation of position for PARSE_ERR_TOOMANY message

The arguments are relative to the outline, not relative to the input line.

This patch fixes up commit 9e1758efbd68c8b1d27e17e2abe4444e110f3ebe which
is 2.2 only. No backport needed.

5 years agoBUG/MINOR: cfgparse: Fix argument reference in PARSE_ERR_TOOMANY message
Tim Duesterhus [Tue, 23 Jun 2020 16:56:09 +0000 (18:56 +0200)] 
BUG/MINOR: cfgparse: Fix argument reference in PARSE_ERR_TOOMANY message

The returned `arg` value is the number of arguments found, but in case
of the error message it's not a valid argument index.

Because we know how many arguments we allowed (MAX_LINE_ARGS) we know
what to print in the error message, so do just that.

Consider a configuration like this:

    listen foo
      1 2 3 [...] 64 65

Then running a configuration check within valgrind reports the following:

    ==18265== Conditional jump or move depends on uninitialised value(s)
    ==18265==    at 0x56E8B83: vfprintf (vfprintf.c:1631)
    ==18265==    by 0x57B1895: __vsnprintf_chk (vsnprintf_chk.c:63)
    ==18265==    by 0x4A8642: vsnprintf (stdio2.h:77)
    ==18265==    by 0x4A8642: memvprintf (tools.c:3647)
    ==18265==    by 0x4CB8A4: print_message (log.c:1085)
    ==18265==    by 0x4CE0AC: ha_alert (log.c:1128)
    ==18265==    by 0x459E41: readcfgfile (cfgparse.c:1978)
    ==18265==    by 0x507CB5: init (haproxy.c:2029)
    ==18265==    by 0x4182A2: main (haproxy.c:3137)
    ==18265==
    ==18265== Use of uninitialised value of size 8
    ==18265==    at 0x56E576B: _itoa_word (_itoa.c:179)
    ==18265==    by 0x56E912C: vfprintf (vfprintf.c:1631)
    ==18265==    by 0x57B1895: __vsnprintf_chk (vsnprintf_chk.c:63)
    ==18265==    by 0x4A8642: vsnprintf (stdio2.h:77)
    ==18265==    by 0x4A8642: memvprintf (tools.c:3647)
    ==18265==    by 0x4CB8A4: print_message (log.c:1085)
    ==18265==    by 0x4CE0AC: ha_alert (log.c:1128)
    ==18265==    by 0x459E41: readcfgfile (cfgparse.c:1978)
    ==18265==    by 0x507CB5: init (haproxy.c:2029)
    ==18265==    by 0x4182A2: main (haproxy.c:3137)
    ==18265==
    ==18265== Conditional jump or move depends on uninitialised value(s)
    ==18265==    at 0x56E5775: _itoa_word (_itoa.c:179)
    ==18265==    by 0x56E912C: vfprintf (vfprintf.c:1631)
    ==18265==    by 0x57B1895: __vsnprintf_chk (vsnprintf_chk.c:63)
    ==18265==    by 0x4A8642: vsnprintf (stdio2.h:77)
    ==18265==    by 0x4A8642: memvprintf (tools.c:3647)
    ==18265==    by 0x4CB8A4: print_message (log.c:1085)
    ==18265==    by 0x4CE0AC: ha_alert (log.c:1128)
    ==18265==    by 0x459E41: readcfgfile (cfgparse.c:1978)
    ==18265==    by 0x507CB5: init (haproxy.c:2029)
    ==18265==    by 0x4182A2: main (haproxy.c:3137)
    ==18265==
    ==18265== Conditional jump or move depends on uninitialised value(s)
    ==18265==    at 0x56E91AF: vfprintf (vfprintf.c:1631)
    ==18265==    by 0x57B1895: __vsnprintf_chk (vsnprintf_chk.c:63)
    ==18265==    by 0x4A8642: vsnprintf (stdio2.h:77)
    ==18265==    by 0x4A8642: memvprintf (tools.c:3647)
    ==18265==    by 0x4CB8A4: print_message (log.c:1085)
    ==18265==    by 0x4CE0AC: ha_alert (log.c:1128)
    ==18265==    by 0x459E41: readcfgfile (cfgparse.c:1978)
    ==18265==    by 0x507CB5: init (haproxy.c:2029)
    ==18265==    by 0x4182A2: main (haproxy.c:3137)
    ==18265==
    ==18265== Conditional jump or move depends on uninitialised value(s)
    ==18265==    at 0x56E8C59: vfprintf (vfprintf.c:1631)
    ==18265==    by 0x57B1895: __vsnprintf_chk (vsnprintf_chk.c:63)
    ==18265==    by 0x4A8642: vsnprintf (stdio2.h:77)
    ==18265==    by 0x4A8642: memvprintf (tools.c:3647)
    ==18265==    by 0x4CB8A4: print_message (log.c:1085)
    ==18265==    by 0x4CE0AC: ha_alert (log.c:1128)
    ==18265==    by 0x459E41: readcfgfile (cfgparse.c:1978)
    ==18265==    by 0x507CB5: init (haproxy.c:2029)
    ==18265==    by 0x4182A2: main (haproxy.c:3137)
    ==18265==
    ==18265== Conditional jump or move depends on uninitialised value(s)
    ==18265==    at 0x56E941A: vfprintf (vfprintf.c:1631)
    ==18265==    by 0x57B1895: __vsnprintf_chk (vsnprintf_chk.c:63)
    ==18265==    by 0x4A8642: vsnprintf (stdio2.h:77)
    ==18265==    by 0x4A8642: memvprintf (tools.c:3647)
    ==18265==    by 0x4CB8A4: print_message (log.c:1085)
    ==18265==    by 0x4CE0AC: ha_alert (log.c:1128)
    ==18265==    by 0x459E41: readcfgfile (cfgparse.c:1978)
    ==18265==    by 0x507CB5: init (haproxy.c:2029)
    ==18265==    by 0x4182A2: main (haproxy.c:3137)
    ==18265==
    ==18265== Conditional jump or move depends on uninitialised value(s)
    ==18265==    at 0x56E8CAB: vfprintf (vfprintf.c:1631)
    ==18265==    by 0x57B1895: __vsnprintf_chk (vsnprintf_chk.c:63)
    ==18265==    by 0x4A8642: vsnprintf (stdio2.h:77)
    ==18265==    by 0x4A8642: memvprintf (tools.c:3647)
    ==18265==    by 0x4CB8A4: print_message (log.c:1085)
    ==18265==    by 0x4CE0AC: ha_alert (log.c:1128)
    ==18265==    by 0x459E41: readcfgfile (cfgparse.c:1978)
    ==18265==    by 0x507CB5: init (haproxy.c:2029)
    ==18265==    by 0x4182A2: main (haproxy.c:3137)
    ==18265==
    ==18265== Conditional jump or move depends on uninitialised value(s)
    ==18265==    at 0x56E8CE2: vfprintf (vfprintf.c:1631)
    ==18265==    by 0x57B1895: __vsnprintf_chk (vsnprintf_chk.c:63)
    ==18265==    by 0x4A8642: vsnprintf (stdio2.h:77)
    ==18265==    by 0x4A8642: memvprintf (tools.c:3647)
    ==18265==    by 0x4CB8A4: print_message (log.c:1085)
    ==18265==    by 0x4CE0AC: ha_alert (log.c:1128)
    ==18265==    by 0x459E41: readcfgfile (cfgparse.c:1978)
    ==18265==    by 0x507CB5: init (haproxy.c:2029)
    ==18265==    by 0x4182A2: main (haproxy.c:3137)
    ==18265==
    ==18265== Conditional jump or move depends on uninitialised value(s)
    ==18265==    at 0x56EA2DB: vfprintf (vfprintf.c:1632)
    ==18265==    by 0x57B1895: __vsnprintf_chk (vsnprintf_chk.c:63)
    ==18265==    by 0x4A8642: vsnprintf (stdio2.h:77)
    ==18265==    by 0x4A8642: memvprintf (tools.c:3647)
    ==18265==    by 0x4CB8A4: print_message (log.c:1085)
    ==18265==    by 0x4CE0AC: ha_alert (log.c:1128)
    ==18265==    by 0x459E41: readcfgfile (cfgparse.c:1978)
    ==18265==    by 0x507CB5: init (haproxy.c:2029)
    ==18265==    by 0x4182A2: main (haproxy.c:3137)
    ==18265==
    [ALERT] 174/165720 (18265) : parsing [./config.cfg:2]: too many words, truncating at word 65, position -95900735: <(null)>.
    [ALERT] 174/165720 (18265) : Error(s) found in configuration file : ./config.cfg
    [ALERT] 174/165720 (18265) : Fatal errors found in configuration.

Valgrind reports conditional jumps relying on an undefined value and the
error message clearly shows incorrect stuff.

After this patch is applied the relying on undefined values is gone and
the <(null)> will actually show the argument. However the position value
still is incorrect. This will be fixed in a follow up patch.

This patch fixes up commit 9e1758efbd68c8b1d27e17e2abe4444e110f3ebe which
is 2.2 only. No backport needed.

5 years agoBUG/MEDIUM: task: be careful not to run too many tasks at TL_URGENT
Willy Tarreau [Tue, 23 Jun 2020 14:36:36 +0000 (16:36 +0200)] 
BUG/MEDIUM: task: be careful not to run too many tasks at TL_URGENT

A test on large objects revealed a big performance loss from 2.1. The
cause was found to be related to cache locality between scheduled
operations that are batched using tasklets. It happens that we now
have several layers of tasklets and that queuing all these operations
leaves time to let memory objects cool down in the CPU cache, effectively
resulting in halving the performance.

A quick test consisting in putting most unknown tasklets into the BULK
queue almost fixed the performance regression, but this is a wrong
approach as it can also slow down some low-latency transfers or access
to applets like the CLI.

What this patch does instead is to queue unknown tasklets into the same
queue as the current one when tasklet_wakeup() is itself called from a
task/tasklet, otherwise it uses urgent for real I/O (when sched->current
is NULL). This results in the called tasklet being woken up much sooner,
often at the end of the current batch of tasklets.

By doing so, a test on 2 cores 4 threads with 256 concurrent H1 conns
transferring 16m objects with 256kB buffers jumped from 55 to 88 Gbps.
It's even possible to go as high as 101 Gbps by evaluating the URGENT
queue after the BULK one, though this was not done as considered
dangerous for latency sensitive operations.

This reinforces the importance of getting back the CPU transfer
mechanisms based on tasklet_wakeup_after() to work at the tasklet level
by supporting an immediate wakeup in certain cases.

No backport is needed, this is strictly 2.2.

5 years agoMINOR: task: add a new pointer to current tasklet queue
Willy Tarreau [Tue, 23 Jun 2020 14:35:38 +0000 (16:35 +0200)] 
MINOR: task: add a new pointer to current tasklet queue

In task_per_thread[] we now have current_queue which is a pointer to
the current tasklet_list entry being evaluated. This will be used to
know the class under which the current task/tasklet is currently
running.

5 years agoBUG/MINOR: tasks: make sure never to exceed max_processed
Willy Tarreau [Tue, 23 Jun 2020 09:32:35 +0000 (11:32 +0200)] 
BUG/MINOR: tasks: make sure never to exceed max_processed

We want to be sure not to exceed max_processed. It can actually go
slightly negative due to the rounding applied to ratios, but we must
refrain from processing too many tasks if it's already low.

This became particularly relevant since recent commit 5c8be272c ("MEDIUM:
tasks: also process late wakeups in process_runnable_tasks()") which was
merged into 2.2-dev10. No backport is needed.