Commit Graph

12310 Commits

Author SHA1 Message Date
Willy Tarreau
ba48d5c8f9 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.
2020-06-24 12:21:26 +02:00
Willy Tarreau
49f90bf148 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().
2020-06-24 12:21:26 +02:00
Willy Tarreau
c0a08ba2df 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.
2020-06-24 12:21:26 +02:00
Willy Tarreau
3ef7a190b0 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.
2020-06-24 12:21:26 +02:00
William Lallemand
b24086923c 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().
2020-06-24 09:57:18 +02:00
William Lallemand
9ab8f8d1b6 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.
2020-06-24 01:00:52 +02:00
William Lallemand
82d877dd02 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
2020-06-23 20:25:07 +02:00
William Lallemand
ee8530c65e 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.
2020-06-23 20:07:50 +02:00
William Lallemand
6a3168ae84 MINOR: ssl: free bind_conf_node in crtlist_free()
Free the list of bind_conf using a crt-list in crtlist_free()
2020-06-23 20:07:32 +02:00
William Lallemand
7df5c2dc3c 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.
2020-06-23 20:06:55 +02:00
Tim Duesterhus
c17a5fac57 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 9e1758efbd which
is 2.2 only. No backport needed.
2020-06-23 19:03:20 +02:00
Tim Duesterhus
f3d2c6d706 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 9e1758efbd which
is 2.2 only. No backport needed.
2020-06-23 19:03:20 +02:00
Willy Tarreau
5bd73063ab 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.
2020-06-23 16:45:28 +02:00
Willy Tarreau
116ef223d2 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.
2020-06-23 16:35:38 +02:00
Willy Tarreau
0c0c85ed9d 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.
2020-06-23 11:34:40 +02:00
Willy Tarreau
38e8a1c7b8 MINOR: debug: add a new DEBUG_FD build option
When DEBUG_FD is set at build time, we'll keep a counter of per-FD events
in the fdtab. This counter is reported in "show fd" even for closed FDs if
not zero. The purpose is to help spot situations where an apparently closed
FD continues to be reported in loops, or where some events are dismissed.
2020-06-23 10:04:54 +02:00
Willy Tarreau
ee99aaf1f9 BUG/MINOR: http_ana: clarify connection pointer check on L7 retry
Coverity reports a possible null deref in issue #703. It seems this
cannot happen as in order to have a CF_READ_ERROR we'd need to have
attempted a recv() which implies a conn_stream, thus conn cannot be
NULL anymore. But at least one line tests for conn and the other one
not, which is confusing. So let's add a check for conn before
dereferencing it.

This needs to be backported to 2.1 and 2.0. Note that in 2.0 it's
in proto_htx.c.
2020-06-23 05:58:20 +02:00
Tim Duesterhus
2fd5bdb439 MINOR: cfgparse: Warn on truncated lines / files
As discussed on the list: https://www.mail-archive.com/haproxy@formilux.org/msg37698.html

This patch adds warnings to the configuration parser that detect the
following situations:

- A line being truncated by a null byte in the middle.
- A file not ending in a new line (and possibly being truncated).
2020-06-23 05:14:59 +02:00
Tim Duesterhus
70f58997f4 BUG/MINOR: cfgparse: Support configurations without newline at EOF
Fix parsing of configurations if the configuration file does not end with
an LF.

This patch fixes GitHub issue #704. It's a regression in
9e1758efbd which is 2.2 specific. No backport
needed.
2020-06-23 05:14:36 +02:00
Miroslav Zagorac
88403266e5 BUG/MINOR: spoe: correction of setting bits for analyzer
When a SPOE filter starts the response analyze, the wrong flag is tested on the
pre_analyzers bit field. AN_RES_INSPECT must be tested instead of
SPOE_EV_ON_TCP_RSP.

This patch must be backported to all versions with the SPOE support, i.e as far
as 1.7.
2020-06-22 11:52:04 +02:00
Christopher Faulet
c97406f790 BUG/MEDIUM: fcgi-app: Resolve the sink if a fcgi-app logs in a ring buffer
If a fcgi application 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.

No need to backport.
2020-06-22 11:35:55 +02:00
Christopher Faulet
60837d340c REGTEST: Add a simple script to tests errorfile directives in proxy sections
This script is compatible with all HAProxy versions. It does not depend on 2.2
features.
2020-06-22 10:35:38 +02:00
Willy Tarreau
dc0936c255 [RELEASE] Released version 2.2-dev10
Released version 2.2-dev10 with the following main changes :
    - BUILD: include: add sys/types before netinet/tcp.h
    - BUG/MEDIUM: log: don't hold the log lock during writev() on a file descriptor
    - BUILD: Remove nowarn for warnings that do not trigger
    - BUG/MEDIUM: pattern: fix thread safety of pattern matching
    - BUILD: Re-enable -Wimplicit-fallthrough
    - BUG/MINOR: ssl: fix ssl-{min,max}-ver with openssl < 1.1.0
    - BUILD: thread: add parenthesis around values of locking macros
    - BUILD: proto_uxst: shut up yet another gcc's absurd warning
    - BUG/MEDIUM: checks: Fix off-by-one in allocation of SMTP greeting cmd
    - CI: travis-ci: use "-O1" for clang builds
    - MINOR: haproxy: Add void deinit_and_exit(int)
    - MINOR: haproxy: Make use of deinit_and_exit() for clean exits
    - BUG/MINOR: haproxy: Free rule->arg.vars.expr during deinit_act_rules
    - BUILD: compression: make gcc 10 happy with free_zlib()
    - BUILD: atomic: add string.h for memcpy() on ARM64
    - BUG/MINOR: http: make smp_fetch_body() report that the contents may change
    - BUG/MINOR: tcp-rules: tcp-response must check the buffer's fullness
    - BUILD: haproxy: mark deinit_and_exit() as noreturn
    - BUG/MAJOR: vars: Fix bogus free() during deinit() for http-request rules
    - BUG/MEDIUM: ebtree: use a byte-per-byte memcmp() to compare memory blocks
    - MINOR: tools: add a new configurable line parse, parse_line()
    - BUG/MEDIUM: cfgparse: use parse_line() to expand/unquote/unescape config lines
    - BUG/MEDIUM: cfgparse: stop after a reasonable amount of fatal error
    - MINOR: http: do not close connections anymore after internal responses
    - BUG/MINOR: cfgparse: Add missing fatal++ in PARSE_ERR_HEX case
    - BUG/MINOR: spoe: add missing key length check before checking key names
    - MINOR: version: put the compiler version output into version.c not haproxy.c
    - MINOR: compiler: always define __has_feature()
    - MINOR: version: report the presence of the compiler's address sanitizer
    - BUILD: Fix build by including haproxy/global.h
    - BUG/MAJOR: connection: always disable ready events once reported
    - CLEANUP: activity: remove unused counter fd_lock
    - DOC: fd: make it clear that some fields ordering must absolutely be respected
    - MINOR: activity: report the number of times poll() reports I/O
    - MINOR: activity: rename confusing poll_* fields in the output
    - MINOR: fd: Fix a typo in a coment.
    - BUG/MEDIUM: fd: Don't fd_stop_recv() a fd we don't own.
    - BUG/MEDIUM: fd: Call fd_stop_recv() when we just got a fd.
    - MINOR: activity: group the per-loop counters at the top
    - MINOR: activity: rename the "stream" field to "stream_calls"
    - MEDIUM: fd: refine the fd_takeover() migration lock
    - MINOR: fd: slightly optimize the fd_takeover double-CAS loop
    - MINOR: fd: factorize the fd_takeover() exit path to make it safer
    - MINOR: peers: do not use localpeer as an array anymore
    - MEDIUM: peers: add the "localpeer" global option
    - MEDIUM: fd: add experimental support for edge-triggered polling
    - CONTRIB: debug: add the missing flags CO_FL_SAFE_LIST and CO_FL_IDLE_LIST
    - MINOR: haproxy: process signals before runnable tasks
    - MEDIUM: tasks: clean up the front side of the wait queue in wake_expired_tasks()
    - MEDIUM: tasks: also process late wakeups in process_runnable_tasks()
    - BUG/MINOR: cli: allow space escaping on the CLI
    - BUG/MINOR: mworker/cli: fix the escaping in the master CLI
    - BUG/MINOR: mworker/cli: fix semicolon escaping in master CLI
    - REGTEST: http-rules: test spaces in ACLs
    - REGTEST: http-rules: test spaces in ACLs with master CLI
    - BUG/MAJOR: init: properly compute the default global.maxpipes value
    - MEDIUM: map: make the "clear map" operation yield
    - BUG/MEDIUM: stream-int: fix loss of CO_SFL_MSG_MORE flag in forwarding
    - MINOR: mux_h1: Set H1_F_CO_MSG_MORE if we know we have more to send.
    - BUG/MINOR: systemd: Wait for network to be online
    - DOC: configuration: Unindent non-code sentences in the protobuf example
    - DOC: configuration: http-check send was missing from matrix
2020-06-19 21:43:26 +02:00
Peter Gervai
8912ae6987 DOC: configuration: http-check send was missing from matrix
The new directive and its doc were added by commit 8acb1284b ("MINOR:
checks: Add a way to send custom headers and payload during http chekcs")
but the index was not updated.
2020-06-19 21:38:05 +02:00
Peter Gervai
df4c9d2a28 DOC: configuration: Unindent non-code sentences in the protobuf example
Unindent to make the explanation go back to text from code formatted
example in tyhe HTMLized version. Still it's not perfect since these
are not haproxy examples but protobuf config, but... way better.
2020-06-19 21:33:37 +02:00
Ryan O'Hara
f49a6049b8 BUG/MINOR: systemd: Wait for network to be online
Change systemd service file to wait for network to be completely
online. This solves two problems:

If haproxy is configured to bind to IP address(es) that are not yet
assigned, haproxy would previously fail. The workaround is to use
"option transparent".

If haproxy us configured to use a resolver to resolve servers via DNS,
haproxy would previously fail due to the fact that the network is not
fully online yet. This is the most compelling reason for this patch.

Signed-off-by: Ryan O'Hara <rohara@redhat.com>
Acked-by: Lukas Tribus <lukas@ltri.eu>
2020-06-19 21:31:10 +02:00
Olivier Houchard
c89a42feba MINOR: mux_h1: Set H1_F_CO_MSG_MORE if we know we have more to send.
In h1_snd_buf(), also set H1_F_CO_MSG_MORE if we know we still have more to
send, not just if the stream-interface told us to do so. This may happen if
the last block of a transfer doesn't fit in the buffer, it remains useful
for the transport layer to know that more data follows what's already in
the buffer.
2020-06-19 17:42:42 +02:00
Willy Tarreau
8945bb6c05 BUG/MEDIUM: stream-int: fix loss of CO_SFL_MSG_MORE flag in forwarding
In 2.2-dev1, a change was made by commit 46230363a ("MINOR: mux-h1: Inherit
send flags from the upper layer"). The purpose was to accurately set the
CO_SFL_MSG_MORE flag on the transport layer because previously it as only
set based on the buffer full condition, which does not accurately indicate
that there are more data to follow.

The problem is that the stream-interface never sets this flag anymore in
HTX mode due to the channel's to_forward always being set to infinity.
Because of this, HTX transfers are always performed without the MSG_MORE
flag and experience a severe performance degradation on large transfers.

This patch addresses this by making the stream-interface aware of HTX and
having it check for CF_EOI to check if more contents are expected or not.
With this change, the single-threaded forwarding performance on 10 MB
objects jumped from 29 to 40 Gbps.

No backport is needed.
2020-06-19 17:42:42 +02:00
Willy Tarreau
d1d005d7f6 MEDIUM: map: make the "clear map" operation yield
As reported in issue #419, a "clear map" operation on a very large map
can take a lot of time and freeze the entire process for several seconds.

This patch makes sure that pat_ref_prune() can regularly yield after
clearing some entries so that the rest of the process continues to work.
The first part, the removal of the patterns, can take quite some time
by itself in one run but it's still relatively fast. It may block for
up to 100ms for 16M IP addresses in a tree typically. This change needed
to declare an I/O handler for the clear operation so that we can get
back to it after yielding.

The second part can be much slower because it deconstructs the elements
and its users, but it iterates progressively so we can yield less often
here.

The patch was tested with traffic in parallel sollicitating the map being
released and showed no problem. Some traffic will definitely notice an
incomplete map but the filling is already not atomic anyway thus this is
not different.

It may be backported to stable versions once sufficiently tested for side
effects, at least as far as 2.0 in order to avoid the watchdog triggering
when the process is frozen there. For a better behaviour, all these
prune_* functions should support yielding so that the callers have a
chance to continue also yield in turn.
2020-06-19 16:57:51 +02:00
Willy Tarreau
a4818db0a9 BUG/MAJOR: init: properly compute the default global.maxpipes value
Initial default settings for maxconn/maxsock/maxpipes were rearranged
in commit a409f30d0 ("MINOR: init: move the maxsock calculation code
to compute_ideal_maxsock()") but as a side effect, the calculated
maxpipes value was not stored anymore into global.maxpipes. This
resulted in splicing being disabled unless there is an explicit
maxpipes setting in the global section.

This patch just stores the calculated ideal value as planned in the
computation and as was done before the patch above.

This is strictly 2.2, no backport is needed.
2020-06-19 16:23:36 +02:00
William Lallemand
5bb21b1d29 REGTEST: http-rules: test spaces in ACLs with master CLI
Do the tests for spaces on the CLI with the master CLI.

Could be backported as far as 2.0 once the required patches are applied.
2020-06-19 14:32:55 +02:00
William Lallemand
398c5f39ee REGTEST: http-rules: test spaces in ACLs
This reg-test tests the spaces in an ACL file, it tries to add new
entries with spaces from the CLI

This reg-test could backported in all stable branches if the fix for
spaces on the CLI was backported.
2020-06-19 14:32:55 +02:00
William Lallemand
02c255e64b BUG/MINOR: mworker/cli: fix semicolon escaping in master CLI
Fix the semicolon escaping which must be handled in the master CLI,
the commands were wrongly splitted and could be forwarded partially to
the target CLI.
2020-06-19 14:32:55 +02:00
William Lallemand
fe249c3df5 BUG/MINOR: mworker/cli: fix the escaping in the master CLI
The master CLI must not do the escaping since it forwards the commands
to another CLI. It should be able to split into words by taking care of
the escaping, but must not remove the forwarded backslashes.

This fix do the same thing as the previous patch applied to the
cli_parse_request() function, by taking care of the escaping during the
word split, but it also remove the part which was removing the
backslashes from the forwarded command.
2020-06-19 14:32:55 +02:00
Yves Lafon
b08c6d06e7 BUG/MINOR: cli: allow space escaping on the CLI
It was not possible to escape spaces over the CLI, making impossible the
insertion of new ACL entries with spaces from the CLI.

This patch fixes the escaping of spaces over the CLI.

It is now possible to launch "add acl agents.acl My\ User\ Agent" over
the CLI.

Could be backported in all stable branches.

Should fix issue #400.
2020-06-19 14:32:55 +02:00
Willy Tarreau
5c8be272c7 MEDIUM: tasks: also process late wakeups in process_runnable_tasks()
Since version 1.8, we've started to use tasks and tasklets more
extensively to defer I/O processing. Originally with the simple
scheduler, a task waking another one up using task_wakeup() would
have caused it to be processed right after the list of runnable ones.

With the introduction of tasklets, we've started to spill running
tasks from the run queues to the tasklet queues, so if a task wakes
another one up, it will only be executed on the next call to
process_runnable_task(), which means after yet another round of
polling loop.

This is particularly visible with I/Os hitting muxes: poll() reports
a read event, the connection layer performs a tasklet_wakeup() on the
mux subscribed to this I/O, and this mux in turn signals the upper
layer stream using task_wakeup(). The process goes back to poll() with
a null timeout since there's one active task, then back to checking all
possibly expired events, and finally back to process_runnable_tasks()
again. Worse, when there is high I/O activity, doing so will make the
task's execution further apart from the tasklet and will both increase
the total processing latency and reduce the cache hit ratio.

This patch brings back to the original spirit of process_runnable_tasks()
which is to execute runnable tasks as long as the execution budget is not
exhausted. By doing so, we're immediately cutting in half the number of
calls to all functions called by run_poll_loop(), and halving the number
of calls to poll(). Furthermore, calling poll() less often also means
purging FD updates less often and offering more chances to merge them.

This also has the nice effect of making tune.runqueue-depth effective
again, as in the past it used to be quickly bounded by this artificial
event horizon which was preventing from executing remaining tasks. On
certain workloads we can see a 2-3% performance increase.
2020-06-19 14:21:46 +02:00
Willy Tarreau
77015abe0b MEDIUM: tasks: clean up the front side of the wait queue in wake_expired_tasks()
Due to the way the wait queue works, some tasks might be postponed but not
requeued. However when we exit wake_expired_tasks() on a not-yet-expired
task and leave it in this situation, the next call to next_timer_expiry()
will use this first task's key in the tree as an expiration date, but this
date might be totally off and cause needless wakeups just to reposition it.

This patch makes sure that we leave wake_expired_tasks with a clean state
of frontside tasks and that their tree's key matches their expiration date.
Doing so we can already observe a ~15% reduction of the number of wakeups
when dealing with large numbers of health checks.

The patch looks large because the code was rearranged but the real change
is to take the wakeup/requeue decision on the task's expiration date instead
of the tree node's key, the rest is unchanged.
2020-06-19 14:21:46 +02:00
Willy Tarreau
a7ad4aed60 MINOR: haproxy: process signals before runnable tasks
Nowadays signals cause tasks to be woken up. The historic code still
processes signals after tasks, which forces a second round in the loop
before they can effectively be processed. Let's move the signal queue
handling between wake_expired_tasks() and process_runnable_tasks() where
it makes much more sense.
2020-06-19 14:21:46 +02:00
Willy Tarreau
54067e9d38 CONTRIB: debug: add the missing flags CO_FL_SAFE_LIST and CO_FL_IDLE_LIST
As often when flags are added they're not updated here. These ones were
missing. They're 2.2 only so no backport is needed.
2020-06-19 14:21:46 +02:00
Willy Tarreau
bc52bec163 MEDIUM: fd: add experimental support for edge-triggered polling
Some of the recent optimizations around the polling to save a few
epoll_ctl() calls have shown that they could also cause some trouble.
However, over time our code base has become totally asynchronous with
I/Os always attempted from the upper layers and only retried at the
bottom, making it look like we're getting closer to EPOLLET support.

There are showstoppers there such as the listeners which cannot support
this. But given that most of the epoll_ctl() dance comes from the
connections, we can try to enable edge-triggered polling on connections.

What this patch does is to add a new global tunable "tune.fd.edge-triggered",
that makes fd_insert() automatically set an et_possible bit on the fd if
the I/O callback is conn_fd_handler. When the epoll code sees an update
for such an FD, it immediately registers it in both directions the first
time and doesn't update it anymore.

On a few tests it proved quite useful with a 14% request rate increase in
a H2->H1 scenario, reducing the epoll_ctl() calls from 2 per request to
2 per connection.

The option is obviously disabled by default as bugs are still expected,
particularly around the subscribe() code where it is possible that some
layers do not always re-attempt reading data after being woken up.
2020-06-19 14:21:46 +02:00
Dragan Dosen
13cd54c08b MEDIUM: peers: add the "localpeer" global option
localpeer <name>
  Sets the local instance's peer name. It will be ignored if the "-L"
  command line argument is specified or if used after "peers" section
  definitions. In such cases, a warning message will be emitted during
  the configuration parsing.

  This option will also set the HAPROXY_LOCALPEER environment variable.
  See also "-L" in the management guide and "peers" section in the
  configuration manual.
2020-06-19 11:37:30 +02:00
Dragan Dosen
4f01415d3b MINOR: peers: do not use localpeer as an array anymore
It is now dynamically allocated by using strdup().
2020-06-19 11:37:11 +02:00
Willy Tarreau
f1cad38281 MINOR: fd: factorize the fd_takeover() exit path to make it safer
Since there was a risk of leaving fd_takeover() without properly
stopping the fd, let's take this opportunity for factoring the code
around a commont exit point that's common to both double-cas and locked
modes. This means using the "ret" variable inside the double-CAS code,
and inverting the loop to first test the old values. Doing do also
produces cleaner code because the compiler cannot factorize common
exit paths using asm statements that are present in some atomic ops.
2020-06-18 08:25:42 +02:00
Willy Tarreau
4297363de3 MINOR: fd: slightly optimize the fd_takeover double-CAS loop
The loop in fd_takeover() around the double-CAS is conditionned on
a previous value of old_masks[0] that always matches tid_bit on the
first iteration because it does not result from the atomic op but
from a pre-loaded value. Let's set the result of the atomic op there
instead so that the conflict between threads can be detected earlier
and before performing the double-word CAS.
2020-06-18 08:08:50 +02:00
Willy Tarreau
c460c91633 MEDIUM: fd: refine the fd_takeover() migration lock
When haproxy is compiled without double-word CAS, we use a migration lock
in fd_takeover(). This lock was covering the atomic OR on the running_mask
before checking its value, while it is not needed since this atomic op
already returns the result. Let's just refine the code to avoid grabbing
the lock in the event another thread has already stolen the FD, this may
reduce contention in high reuse rate scenarios.
2020-06-18 07:28:09 +02:00
Willy Tarreau
7af4fa9a48 MINOR: activity: rename the "stream" field to "stream_calls"
This one was confusingly called, I thought it was the cumulated number
of streams but it's the number of calls to process_stream(). Let's make
this clearer.
2020-06-17 20:52:29 +02:00
Willy Tarreau
a00cf9bbaf MINOR: activity: group the per-loop counters at the top
empty_rq and long_rq are per-loop so it makes sense to group them
together with the loop count. In addition since ctxsw and tasksw
apply in the context of these counters, let's move them as well.
More precisely the difference between wake_tasks and long_rq should
roughly correspond to the number of inter-task messages. Visually
it's much easier to spot ratios of wakeup causes now.
2020-06-17 20:52:29 +02:00
Olivier Houchard
ddc874c46c BUG/MEDIUM: fd: Call fd_stop_recv() when we just got a fd.
In fd_takeover(), when a double-width compare-and-swap is implemented,
make sure, if we managed to get the fd, to call fd_stop_recv() on it, so
that the thread that used to own it will know it has to stop polling it.
2020-06-17 20:36:28 +02:00
Olivier Houchard
8d7b517824 BUG/MEDIUM: fd: Don't fd_stop_recv() a fd we don't own.
In fd_takeover(), if we failed to grab the fd, when a double-width
compare-and-swap is not implemented, do not call fd_stop_recv() on the
fd, it is not ours and may be used by another thread.
2020-06-17 20:36:28 +02:00
Olivier Houchard
f86a106f68 MINOR: fd: Fix a typo in a coment.
The function si called fd_takeover, not fd_takeother.
2020-06-17 20:36:28 +02:00