]> git.ipfire.org Git - thirdparty/haproxy.git/log
thirdparty/haproxy.git
4 years agoMEDIUM: log: slightly refine the output format of alerts/warnings/etc
Willy Tarreau [Fri, 7 May 2021 06:42:39 +0000 (08:42 +0200)] 
MEDIUM: log: slightly refine the output format of alerts/warnings/etc

For about 20 years we've been emitting cryptic messages on warnings and
alerts, that nobody knows how to parse:

  [NOTICE] 126/080118 (3115) : haproxy version is 2.4-dev18-0b7c78-49
  [NOTICE] 126/080118 (3115) : path to executable is ./haproxy
  [WARNING] 126/080119 (3115) : Server default/srv1 is DOWN via static/srv1. 0 active and 0 backup servers left. 0 sessions active, 0 requeued, 0 remaining in queue.
  [ALERT] 126/080119 (3115) : backend 'default' has no server available!

Hint: the first 3-digit number is the day of year, and the 6 digits
after it represent the time of day in format HHMMSS, then the pid in
parenthesis. These are not quite user-friendly and such cryptic into
are not useful at all.

This patch slightly adjusts the output by performing these minimal changes:
  - removing the date/time, as they were added very early when haproxy
    was meant to be used in foreground as a debugging tool, and they're
    provided in more details in logs nowadays ;

  - better aligning the fields by padding the severity tag to 10 chars.
    The diag output was renamed to "DIAG" only.

Now the output provides this:

  [NOTICE]   (4563) : haproxy version is 2.4-dev18-75a428-51
  [NOTICE]   (4563) : path to executable is ./haproxy
  [WARNING]  (4563) : Server default/srv1 is DOWN via static/srv1. 0 active and 0 backup servers left. 0 sessions active, 0 requeued, 0 remaining in queue.
  [ALERT]    (4563) : backend 'default' has no server available!

The useless space before the colon was kept so as not to confuse any
possible output parser.

The few entries in the doc referring to this format were adjusted to
reflect the new one.

The change was tagged "MEDIUM" as it may have visible consequences on
home-grown monitoring tools, though it is extremely unlikely due to the
limited extent of these changes.

4 years agoBUG/MINOR: stream: properly clear the previous error mask on L7 retries
Willy Tarreau [Fri, 7 May 2021 06:19:30 +0000 (08:19 +0200)] 
BUG/MINOR: stream: properly clear the previous error mask on L7 retries

The cleanup of the previous error was incorrect on L7 retries, it would
OR two values while they're part of an enum, leaving some bits set.
Depending on the errors it was possible to occasionally see an internal
error ("I" flag) being logged.

This should be backported as far as 2.0, though the do_l7_retry() function
in in proto_htx.c in older versions.

4 years agoBUG/MINOR: activity: use the new pointer to calculate the new size in realloc()
Willy Tarreau [Fri, 7 May 2021 06:01:35 +0000 (08:01 +0200)] 
BUG/MINOR: activity: use the new pointer to calculate the new size in realloc()

When memory profiling is enabled, realloc() can occasionally get the area
size wrong due to the wrong pointer being used to check the new size. When
the old area gets unmapped in the operation, this may even result in a
crash. There's no impact without memory profiling though.

No backport is needed as this is exclusively 2.4-dev.

4 years agoMINOR: config: add predicates "version_atleast" and "version_before" to cond blocks
Willy Tarreau [Thu, 6 May 2021 14:53:26 +0000 (16:53 +0200)] 
MINOR: config: add predicates "version_atleast" and "version_before" to cond blocks

These predicates respectively verify that the current version is at least
a given version or is before a specific one. The syntax is exactly the one
reported by "haproxy -v", though each component is optional, so both "1.5"
and "2.4-dev18-88910-48" are supported. Missing components equal zero, and
"dev" is below "pre" or "rc", which are both inferior to no such mention
(i.e. they are negative). Thus "2.4-dev18" is older than "2.4-rc1" which
is older than "2.4".

4 years agoMINOR: config: add predicate "feature" to detect certain built-in features
Willy Tarreau [Thu, 6 May 2021 14:34:23 +0000 (16:34 +0200)] 
MINOR: config: add predicate "feature" to detect certain built-in features

The "feature(name)" predicate will return true if <name> corresponds to
a name listed after a '+' in the features list, that is it was enabled at
build time with USE_<name>=1. Typical use cases will include OPENSSL, LUA
and LINUX_SPLICE. But maybe it will also be convenient to use with optional
addons such as PROMEX and the device detection modules to help keeping the
same configs across various deployments.

4 years agoMINOR: config: add predicates "streq()" and "strneq()" to conditional expressions
Willy Tarreau [Thu, 6 May 2021 14:10:09 +0000 (16:10 +0200)] 
MINOR: config: add predicates "streq()" and "strneq()" to conditional expressions

"streq(str1,str2)" will return true if the two strings match while
"strneq(str1,str2)" will return true only if they differ. This is
convenient to match an environment variable against a predefined value.

4 years agoMINOR: config: add predicate "defined()" to conditional expression blocks
Willy Tarreau [Thu, 6 May 2021 13:55:14 +0000 (15:55 +0200)] 
MINOR: config: add predicate "defined()" to conditional expression blocks

"defined(name)" will return true if <name> is a defined environment variable
otherwise false, regardless of its contents.

4 years agoMINOR: config: make cfg_eval_condition() support predicates with arguments
Willy Tarreau [Thu, 6 May 2021 13:49:04 +0000 (15:49 +0200)] 
MINOR: config: make cfg_eval_condition() support predicates with arguments

Now we can look up a list of known predicates and pre-parse their
arguments. For now the list is empty. The code needed to be arranged with
a common exit point to release all arguments because there's no default
argument freeing function (it likely only used to exist in the deinit
code). Since we only support simple arguments for now it's no big deal,
only a 2-liner loop.

4 years agoMINOR: config: improve .if condition error reporting
Willy Tarreau [Thu, 6 May 2021 13:07:10 +0000 (15:07 +0200)] 
MINOR: config: improve .if condition error reporting

Let's return the position of the first unparsable character on error,
so that instead of just saying "unparsable conditional expression blah"
we can have:

  [ALERT] 125/150618 (13995) : parsing [test-conds2.cfg:1]: unparsable conditional expression '12/blah' in '.if' at position 1:
    .if 12/blah
        ^
This is important because conditions will be made from environment
variables or later from more complex expressions where the error will
not always be easy to locate.

4 years agoMINOR: global: add version comparison functions
Willy Tarreau [Thu, 6 May 2021 05:43:35 +0000 (07:43 +0200)] 
MINOR: global: add version comparison functions

The new function split_version() converts a parsable haproxy version to
an array of integers. The function compare_current_version() compares an
arbitrary version to the current one. These two functions were written
by Thierry Fournier in 2013, and are still usable as-is. They will be
used to write config language predicates.

4 years agoMINOR: global: export the build features string list
Willy Tarreau [Thu, 6 May 2021 14:30:32 +0000 (16:30 +0200)] 
MINOR: global: export the build features string list

Till now it was only presented in the version output but could not be
consulted outside of haproxy.c, let's export it as a variable, and set
it to an empty string if not defined.

4 years agoMINOR: arg: improve the error message on missing closing parenthesis
Willy Tarreau [Thu, 6 May 2021 12:50:30 +0000 (14:50 +0200)] 
MINOR: arg: improve the error message on missing closing parenthesis

When the closing brace is missing after an argument (acl, ...), the
error may report something like "expected ')' before ''". Let's just
drop "before ''" when the final word is empty to make the message a
bit clearer.

4 years agoBUILD: activity: do not include malloc.h
Willy Tarreau [Thu, 6 May 2021 09:37:53 +0000 (11:37 +0200)] 
BUILD: activity: do not include malloc.h

It doesn't exist on MacOS and broke the build. We don't need it as it's
already included by compat.h when relevant. No backport is needed.

4 years agoMINOR: config: support some pseudo-variables for file/line/section
Willy Tarreau [Thu, 6 May 2021 08:25:11 +0000 (10:25 +0200)] 
MINOR: config: support some pseudo-variables for file/line/section

The new pseudo-variables ".FILE", ".LINE" and ".SECTION" will be resolved
on the fly by the config parser and will respectively retrieve the current
configuration file name, the current line number and the current section
being parsed. This may help emit logs, errors, and debugging information
(e.g. which rule matched).

The '.' in the first char was reserved for such pseudo-variables and no
other variable is permitted. This will allow to add support for new ones
in the future if they prove to be useful (e.g. randoms/uuid for secret
keying or automatic naming of configuration objects).

4 years agoMINOR: config: keep up-to-date current file/line/section in the global struct
Willy Tarreau [Thu, 6 May 2021 08:04:45 +0000 (10:04 +0200)] 
MINOR: config: keep up-to-date current file/line/section in the global struct

Let's add a few fields to the global struct to store information about
the current file being processed, the current line number and the current
section. This will be used to retrieve them using special variables.

4 years agoMINOR: config: centralize the ".if"/".elif" condition parser and evaluator
Willy Tarreau [Thu, 6 May 2021 06:19:48 +0000 (08:19 +0200)] 
MINOR: config: centralize the ".if"/".elif" condition parser and evaluator

Instead of duplicating the condition evaluations, let's have a single
function cfg_eval_condition() that returns true/false/error. It takes
less code and will ease its extension.

4 years agoBUG/MINOR: config: .if/.elif should also accept negative integers
Willy Tarreau [Thu, 6 May 2021 05:52:50 +0000 (07:52 +0200)] 
BUG/MINOR: config: .if/.elif should also accept negative integers

The doc about .if/.elif config block conditions says:

  a non-nul integer (e.g. '1'), always returns "true"

So we must accept negative integers as well. The test was made on
atoi() > 0.

No backport is needed, this is only 2.4.

4 years agoBUG/MINOR: config: add a missing "ELIF_TAKE" test for ".elif" condition evaluator
Willy Tarreau [Thu, 6 May 2021 06:48:09 +0000 (08:48 +0200)] 
BUG/MINOR: config: add a missing "ELIF_TAKE" test for ".elif" condition evaluator

This missing state was causing a second elif condition to be evaluated
after a first one succeeded after a .if failed. For example in the test
below the else would be executed:

     .if    0
     .elif  1
     .elif  0
     .else
     .endif

No backport is needed, this is 2.4-only.

4 years agoBUG/MINOR: config: fix uninitialized initial state in ".if" block evaluator
Willy Tarreau [Thu, 6 May 2021 06:46:11 +0000 (08:46 +0200)] 
BUG/MINOR: config: fix uninitialized initial state in ".if" block evaluator

The condition to skip the block in the ".if" evaluator forgot to check
that the level was high enough, resulting in rare cases where a random
value matched one of the 5 values that cause the block to be skipped.

No backport is needed as it's 2.4-only.

4 years agoBUG/MINOR: stream: Decrement server current session counter on L7 retry
Christopher Faulet [Wed, 5 May 2021 16:23:59 +0000 (18:23 +0200)] 
BUG/MINOR: stream: Decrement server current session counter on L7 retry

When a L7 retry is performed, we must not forget to decrement the current
session counter of the assigned server. Of course, it must only be done if
the current session is already counted on the server, thus if SF_CURR_SESS
flag is set on the stream.

This patch is related to the issue #1003. It must be backported as far as
2.0.

4 years agoMINOR: mux-h1: Manage processing blocking flags on the H1 stream
Christopher Faulet [Wed, 7 Apr 2021 12:18:11 +0000 (14:18 +0200)] 
MINOR: mux-h1: Manage processing blocking flags on the H1 stream

Because H1C_F_RX_BLK and H1C_F_TX_BLK flags now only concerns data
processing, at the H1 stream level, there is no reason to still manage them
on the H1 connection. Thus, these flags are now set on the H1 stream.

4 years agoCLEANUP: mux-h1: rename WAIT_INPUT/WAIT_OUTPUT flags
Christopher Faulet [Wed, 7 Apr 2021 09:50:26 +0000 (11:50 +0200)] 
CLEANUP: mux-h1: rename WAIT_INPUT/WAIT_OUTPUT flags

These flags are used to block, respectively, the output and the input
processing. Thus, to be more explicit, H1C_F_WAIT_INPUT is renamed to
H1C_F_TX_BLK and H1C_F_WAIT_OUTPUT is renamed to H1C_F_RX_BLK.

4 years agoMEDIUM: mux-h1: Wake H1 stream when both sides a synchronized
Christopher Faulet [Fri, 9 Apr 2021 10:31:48 +0000 (12:31 +0200)] 
MEDIUM: mux-h1: Wake H1 stream when both sides a synchronized

Instead of subscribing for reads or sends to restart data processing, when
both sides are synchronized, the H1 stream is woken up. This happens when
H1C_F_WAIT_INPUT or H1C_F_WAIT_OUTPUT flags are removed, Indeed, these flags
block the data processing and not raw data sending or receiving.

4 years agoMINOR: mux-h1: Always subscribe for reads when splicing is disabled
Christopher Faulet [Fri, 9 Apr 2021 09:58:49 +0000 (11:58 +0200)] 
MINOR: mux-h1: Always subscribe for reads when splicing is disabled

In h1_rcv_pipe(), when the splicing is not possible or disabled at the end
of the fnuction, we make sure to subscribe for reads. It is not a bug but it
avoid an extra call to h1_rcv_pipe() to handle the subscription in some
cases (end of message, end of chunk or read0).

In addition, the condition to detect end of splicing has been simplified. We
now only rely on H1C_F_WANT_SPLICE flags.

4 years agoMINOR: mux-h1: Subscribe for sends if output buffer is not empty in h1_snd_pipe
Christopher Faulet [Tue, 6 Apr 2021 15:27:32 +0000 (17:27 +0200)] 
MINOR: mux-h1: Subscribe for sends if output buffer is not empty in h1_snd_pipe

In h1_snd_pipe(), before sending spliced data, we take care to flush the
output buffer by subscribing for sends. However, the condition to do so is
not accurate. We test data remaining in the pipe. It works but it also
unnecessarily subscribes H1C for sends when the output buffer is empty if we
are unable to send all spliced data in one time. Instead, H1C is now
subscribed for sends if output buffer is not empty.

4 years agoMINOR: mux-h1: clean up conditions to enabled and disabled splicing
Christopher Faulet [Tue, 6 Apr 2021 15:24:39 +0000 (17:24 +0200)] 
MINOR: mux-h1: clean up conditions to enabled and disabled splicing

First, there is no reason to announce the splicing support at the
conn-stream level when it is created, at least for now. GTUNE_USE_SPLICE
option is already handled at the stream level.

Second, in h1_rcv_buf(), there is no reason to test the message state to
switch the H1C in splicing mode (via H1C_F_WANT_SPLICE flag).
h1_process_input() already takes care to set CS_FL_MAY_SPLICE flag on the
conn-stream when appropriate. Thus, in h1_rcv_buf(), we can rely on this
flag to change the H1C state.

Finally, if h1_rcv_pipe() is called, it means the H1C is already in the
splicing mode. H1C_F_WANT_SPLICE flag is necessarily already set. Thus no
reason to force it.

4 years agoREGTESTS: Add script to test abortonclose option
Christopher Faulet [Wed, 7 Apr 2021 12:37:07 +0000 (14:37 +0200)] 
REGTESTS: Add script to test abortonclose option

This script test abortonclose option for HTTP/1 client only. It may be
backported as far as 2.0. But on the 2.2 and prior, the syslog part must be
adapted to catch log messages emitted by proxy during HAProxy
startup. Following lines must be added :

    recv
    expect ~ "[^:\\[ ]\\[${h1_pid}\\]: Proxy fe1 started."
    recv
    expect ~ "[^:\\[ ]\\[${h1_pid}\\]: Proxy fe2 started."

4 years agoBUG/MEDIUM: mux-h1: Properly report client close if abortonclose option is set
Christopher Faulet [Thu, 8 Apr 2021 16:42:59 +0000 (18:42 +0200)] 
BUG/MEDIUM: mux-h1: Properly report client close if abortonclose option is set

On client side, if CO_RFL_KEEP_RECV flags is set when h1_rcv_buf() is
called, we force subscription for reads to be able to catch read0. This way,
the event will be reported to upper layer to let the stream abort the
request.

This patch fixes the abortonclose option for H1 connections. It depends on
following patches :

  * MEDIUM: mux-h1: Don't block reads when waiting for the other side
  * MINOR: conn-stream: Force mux to wait for read events if abortonclose is set

But to be sure the event is handled by the stream, the following patches are
also required :

  * BUG/MINOR: stream-int: Don't block reads in si_update_rx() if chn may receive
  * MINOR: channel: Rely on HTX version if appropriate in channel_may_recv()

All the series must be backported with caution as far as 2.0, and only after
a period of observation to be sure nothing broke.

4 years agoMEDIUM: mux-h1: Don't block reads when waiting for the other side
Christopher Faulet [Thu, 8 Apr 2021 16:34:30 +0000 (18:34 +0200)] 
MEDIUM: mux-h1: Don't block reads when waiting for the other side

When we are waiting for the other side to read more data, or to read the
next request, we must only stop the processing of input data and not the
data receipt. This patch don't change anything on the subscribes for
reads. So it should not change anything. The only difference is that the H1
connection will try to read data if it is woken up for an I/O event and if
it was subscribed for reads.

This patch is required to fix abortonclose option for H1 client connections.

4 years agoMINOR: conn-stream: Force mux to wait for read events if abortonclose is set
Christopher Faulet [Thu, 8 Apr 2021 16:13:25 +0000 (18:13 +0200)] 
MINOR: conn-stream: Force mux to wait for read events if abortonclose is set

When the abortonclose option is enabled, to be sure to be immediately
notified when a shutdown is received from the client, the frontend
conn-stream must be sure the mux will wait for read events. To do so, the
CO_RFL_KEEP_RECV flag is set when mux->rcv_buf() is called. This new flag
instructs the mux to wait for read events, regardless its internal state.

This patch is required to fix abortonclose option for H1 client connections.

4 years agoBUG/MINOR: stream-int: Don't block reads in si_update_rx() if chn may receive
Christopher Faulet [Wed, 7 Apr 2021 06:45:05 +0000 (08:45 +0200)] 
BUG/MINOR: stream-int: Don't block reads in si_update_rx() if chn may receive

In si_update_rx() function, the reads may be blocked because we explicitly
don't want to read or because of a lack of room in the input buffer. The
first condition is valid. However the second one only test if the channel is
empty or not. It means the reads are blocked if there are still some output
data in the input channel, in its buffer or its pipe. This condition is not
accurate. The reads must not be blocked if the channel can still receive
data. Thus instead of relying on channel_is_empty() function, we now call
channel_may_recv().

This patch is especially useful to be able to catch read0 on client side
when we are waiting for a connection to the server, when abortonclose option
is enabled. Otherwise, the client abort is not detected.

This patch depends on "MINOR: channel: Rely on HTX version if appropriate in
channel_may_recv()". Both must be backported as far as 2.0 after a period of
observation to be sure nothing broke.

4 years agoMINOR: channel: Rely on HTX version if appropriate in channel_may_recv()
Christopher Faulet [Wed, 7 Apr 2021 06:10:41 +0000 (08:10 +0200)] 
MINOR: channel: Rely on HTX version if appropriate in channel_may_recv()

When channel_may_recv() is called for an HTX stream, the HTX version,
channel_htx_may_recv() is called. This patch is mandatory to fix a bug
related to the abortonclose option.

4 years agoBUILD: makefile: add new option USE_MEMORY_PROFILING
Willy Tarreau [Wed, 5 May 2021 16:17:39 +0000 (18:17 +0200)] 
BUILD: makefile: add new option USE_MEMORY_PROFILING

It is not enabled by default, and may only work on linux-glibc for now,
though maybe other platforms could adopt it, possibly with certain
restrictions.

4 years agoMINOR: activity: add the profiling.memory global setting
Willy Tarreau [Wed, 5 May 2021 16:33:19 +0000 (18:33 +0200)] 
MINOR: activity: add the profiling.memory global setting

This allows to enable/disable memory usage profiling very early, which
can be convenient to trace the memory usage in maps, certificates, Lua
etc.

4 years agoMINOR: activity: make "show profiling" also dump the memoery usage
Willy Tarreau [Wed, 5 May 2021 16:07:02 +0000 (18:07 +0200)] 
MINOR: activity: make "show profiling" also dump the memoery usage

Now the memory usage stats are dumped. They are first sorted by total
alloc+free so that the first ones are always the most relevant, and
that most symmetric alloc/free pairs appear next to each other. This
way it becomes convenient to only show a small part of them such as:

    show profiling memory 20

It's worth noting that the sorting is performed upon each call to the
iohandler so it is technically possible that an entry could appear
twice or be dropped if the ordering changes between two calls. In
practice it is not an issue but it's worth being mentioned.

4 years agoMINOR: activity: make "show profiling" support a few arguments
Willy Tarreau [Wed, 5 May 2021 15:48:13 +0000 (17:48 +0200)] 
MINOR: activity: make "show profiling" support a few arguments

These ones allow to limit the output to only certain sections and/or
a number of lines per dump.

4 years agoMINOR: activity: clean up the show profiling io_handler a little bit
Willy Tarreau [Wed, 5 May 2021 15:33:27 +0000 (17:33 +0200)] 
MINOR: activity: clean up the show profiling io_handler a little bit

Let's rearrange it to make it more configurable and allow to iterate
over multiple parts (header, tasks, memory etc), to restart from a
given line number (previously it didn't work, though fortunately it
didn't happen), and to support dumping only certain parts and a given
number of lines. A few entries from ctx.cli are now used to store a
restart point and the current step.

4 years agoMEDIUM: activity: collect memory allocator statistics with USE_MEMORY_PROFILING
Willy Tarreau [Wed, 5 May 2021 15:07:09 +0000 (17:07 +0200)] 
MEDIUM: activity: collect memory allocator statistics with USE_MEMORY_PROFILING

When built with USE_MEMORY_PROFILING the main memory allocation functions
are diverted to collect statistics per caller. It is a bit tricky because
the only way to call the original ones is to find their pointer, which
requires dlsym(), and which is not available everywhere.

Thus all functions are designed to call their fallback function (the
original one), which is preset to an initialization function that is
supposed to call dlsym() to resolve the missing symbols, and vanish.
This saves expensive tests in the critical path.

A second problem is that dlsym() calls calloc() to initialize some
error messages. After plenty of tests with posix_memalign(), valloc()
and friends, it turns out that returning NULL still makes it happy.
Thus we currently use a visit counter (in_memprof) to detect if we're
reentering, in which case all allocation functions return NULL.

In order to convert a return address to an entry in the stats, we
perform a cheap hash consisting in multiplying the pointer by a
balanced number (as many zeros as ones) and keeping the middle bits.
The hash is already pretty good like this, achieving to store up to
638 entries in a 2048-entry table without collision. But in order to
further refine this and improve the fill ratio of the table, in case
of collision we move up to 16 adjacent entries to find a free place.
This remains quite cheap and manages to store all of these inside a
1024-entries hash table with even less risk of collision.

Also, free(NULL) does not produce any stats. By doing so we reduce
from 638 to 208 the average number of entries needed for a basic
config using SSL. free(NULL) not only provides no information as it's
a NOP, but keeping it is pure pollution as it happens all the time.

When DEBUG_MEM_STATS is enabled, malloc/calloc/realloc are redefined as
macros, preventing the code from compiling. Thus, when this option is
detected, the macros are undefined as they are pointless there anyway.

The functions are optimized to quickly jump to the fallback and as such
become almost invisible in terms of processing time, execpt an extra
"if" on a read_mostly variable and a jump. Considering that this only
happens for pool misses and library routines, this remains acceptable.

Performance tests in SSL (the most stressful test) shows less than 1%
performance loss when profiling is enabled on 2c4t.

The code was written in a way to ease backporting to modern versions
(2.2+) if needed, so it keeps the long names for integers and doesn't
use the _INC version of the atomic ops.

4 years agoMINOR: activity: declare the storage for memory usage statistics
Willy Tarreau [Wed, 5 May 2021 14:50:40 +0000 (16:50 +0200)] 
MINOR: activity: declare the storage for memory usage statistics

We'll need to store for each call place, the pointer to the caller
(the return address to be more exact as with free() it's not uncommon
to see tail calls), the number of calls to alloc/free and the total
alloc/free bytes. realloc() will be counted either as alloc or free
depending on the balance of the size before vs after.

We store 1024+1 entries. The first ones are used as hashes and the
last one for collisions.

When profiling is enabled via the CLI, all the stats are reset.

4 years agoMINOR: activity: add a "memory" entry to "profiling"
Willy Tarreau [Wed, 5 May 2021 14:44:23 +0000 (16:44 +0200)] 
MINOR: activity: add a "memory" entry to "profiling"

This adds the necessary flags to permit run-time enabling/disabling of
memory profiling. For now this is disabled.

A few words were added to the management doc about it and recalling that
this is limited to certain OSes.

4 years agoCLEANUP: activity: mark the profiling and task_profiling_mask __read_mostly
Willy Tarreau [Wed, 5 May 2021 14:28:31 +0000 (16:28 +0200)] 
CLEANUP: activity: mark the profiling and task_profiling_mask __read_mostly

These ones are only read by the scheduler and occasionally written to
by the CLI parser, so let's move them to read_mostly so that they do
not risk to suffer from cache line pollution.

4 years agoMINOR: tools: add functions to retrieve the address of a symbol
Willy Tarreau [Wed, 5 May 2021 07:06:21 +0000 (09:06 +0200)] 
MINOR: tools: add functions to retrieve the address of a symbol

get_sym_curr_addr() will return the address of the first occurrence of
the given symbol while get_sym_next_addr() will return the address of
the next occurrence of the symbol. These ones return NULL on non-linux,
non-ELF, non-USE_DL.

4 years agoMEDIUM: connection: close front idling connection on soft-stop
Amaury Denoyelle [Mon, 3 May 2021 08:47:51 +0000 (10:47 +0200)] 
MEDIUM: connection: close front idling connection on soft-stop

Implement a safe mechanism to close front idling connection which
prevents the soft-stop to complete. Every h1/h2 front connection is
added in a new per-thread list instance. On shutdown, a new task is
waking up which calls wake mux operation on every connection still
present in the new list.

A new stopping_list attach point has been added in the connection
structure. As this member is only used for frontend connections, it
shared the same union as the session_list reserved for backend
connections.

4 years agoMEDIUM: mux_h1: release idling frontend conns on soft-stop
Amaury Denoyelle [Wed, 5 May 2021 09:11:11 +0000 (11:11 +0200)] 
MEDIUM: mux_h1: release idling frontend conns on soft-stop

In h1_process, if the proxy of a frontend connection is disabled,
release the connection.

This commit is in preparation to properly close idling front connections
on soft-stop. h1_process must still be called, this will be done via a
dedicated task which monitors the global variable stopping.

4 years agoMINOR: connection: move session_list member in a union
Amaury Denoyelle [Mon, 3 May 2021 12:28:30 +0000 (14:28 +0200)] 
MINOR: connection: move session_list member in a union

Move the session_list attach point in an anonymous union. This member is
only used for backend connections. This commit is in preparation for the
support of stopping frontend idling connections which will add another
member to the union.

This change means that a special care must be taken to be sure that only
backend connections manipulate the session_list. A few BUG_ON has been
added as special guard to prevent from misuse.

4 years agoMINOR: srv: close all idle connections on shutdown
Amaury Denoyelle [Thu, 29 Apr 2021 15:30:05 +0000 (17:30 +0200)] 
MINOR: srv: close all idle connections on shutdown

Implement a function to close all server idle connections. This function
is called via a global deinit server handler.

The main objective is to prevents from leaving sockets in TIME_WAIT
state. To limit the set of operations on shutdown and prevents
tasks rescheduling, only the ctrl stack closing is done.

4 years agoCI: Github Actions: switch to LibreSSL-3.3.3
Ilya Shipitsin [Wed, 5 May 2021 04:01:03 +0000 (09:01 +0500)] 
CI: Github Actions: switch to LibreSSL-3.3.3

stable LibreSSL-3.3.3 released, let us switch to it

4 years agoMINOR: pools/debug: slightly relax DEBUG_DONT_SHARE_POOLS
Willy Tarreau [Wed, 5 May 2021 05:29:01 +0000 (07:29 +0200)] 
MINOR: pools/debug: slightly relax DEBUG_DONT_SHARE_POOLS

The purpose of this debugging option was to prevent certain pools from
masking other ones when they were shared. For example, task, http_txn,
h2s, h1s, h1c, session, fcgi_strm, and connection are all 192 bytes and
would normally be mergedi, but not with this option. The problem is that
certain pools are declared multiple times with various parameters, which
are often very close, and due to the way the option works, they're not
shared either. Good examples of this are captures and stick tables. Some
configurations have large numbers of stick-tables of pretty similar types
and it's very common to end up with the following when the option is
enabled:

  $ socat - /tmp/sock1  <<< "show pools" | grep stick
    - Pool sticktables (160 bytes) : 0 allocated (0 bytes), 0 used, needed_avg 0, 0 failures, 1 users, @0x753800=56
    - Pool sticktables (160 bytes) : 0 allocated (0 bytes), 0 used, needed_avg 0, 0 failures, 1 users, @0x753880=57
    - Pool sticktables (160 bytes) : 0 allocated (0 bytes), 0 used, needed_avg 0, 0 failures, 1 users, @0x753900=58
    - Pool sticktables (160 bytes) : 0 allocated (0 bytes), 0 used, needed_avg 0, 0 failures, 1 users, @0x753980=59
    - Pool sticktables (160 bytes) : 0 allocated (0 bytes), 0 used, needed_avg 0, 0 failures, 1 users, @0x753a00=60
    - Pool sticktables (160 bytes) : 0 allocated (0 bytes), 0 used, needed_avg 0, 0 failures, 1 users, @0x753a80=61
    - Pool sticktables (160 bytes) : 0 allocated (0 bytes), 0 used, needed_avg 0, 0 failures, 1 users, @0x753b00=62
    - Pool sticktables (224 bytes) : 0 allocated (0 bytes), 0 used, needed_avg 0, 0 failures, 1 users, @0x753780=55

In addition to not being convenient, it can have important effects on the
memory usage because these pools will not share their entries, so one stick
table cannot allocate from another one's pool.

This patch solves this by going back to the initial goal which was not to
have different pools in the same list. Instead of masking the MAP_F_SHARED
flag, it simply adds a test on the pool's name, and disables pool sharing
if the names differ. This way pools are not shared unless they're of the
same name and size, which doesn't hinder debugging. The same test above
now returns this:

  $ socat - /tmp/sock1  <<< "show pools" | grep stick
    - Pool sticktables (160 bytes) : 0 allocated (0 bytes), 0 used, needed_avg 0, 0 failures, 7 users, @0x3fadb30 [SHARED]
    - Pool sticktables (224 bytes) : 0 allocated (0 bytes), 0 used, needed_avg 0, 0 failures, 1 users, @0x3facaa0 [SHARED]

This is much better. This should probably be backported, in order to limit
the side effects of DEBUG_DONT_SHARE_POOLS being enabled in production.

4 years agoMINOR: debug: add a new "debug dev sym" command in expert mode
Willy Tarreau [Tue, 4 May 2021 16:40:50 +0000 (18:40 +0200)] 
MINOR: debug: add a new "debug dev sym" command in expert mode

This command attempts to resolve a pointer to a symbol name. This is
convenient during development as it's easier to get such pointers live
than by issuing a debugger or calling addr2line.

4 years agoBUG/MINOR: ssl/cli: fix a lock leak when no memory available
William Lallemand [Tue, 4 May 2021 14:17:27 +0000 (16:17 +0200)] 
BUG/MINOR: ssl/cli: fix a lock leak when no memory available

This bug was introduced in e5ff4ad ("BUG/MINOR: ssl: fix a trash buffer
leak in some error cases").

When cli_parse_set_cert() returns because alloc_trash_chunk() failed, it
does not unlock the spinlock which can lead to a deadlock later.

Must be backported as far as 2.1 where e5ff4ad was backported.

4 years agoBUG/MEDIUM: cli: prevent memory leak on write errors
Willy Tarreau [Tue, 4 May 2021 14:27:45 +0000 (16:27 +0200)] 
BUG/MEDIUM: cli: prevent memory leak on write errors

Since the introduction of payload support on the CLI in 1.9-dev1 by
commit abbf60710 ("MEDIUM: cli: Add payload support"), a chunk is
temporarily allocated for the CLI to support defragmenting a payload
passed with a command. However it's only released when passing via
the CLI_ST_END state (i.e. on clean shutdown), but not on errors.
Something as trivial as:

  $ while :; do ncat --send-only -U /path/to/cli <<< "show stat"; done

with a few hundreds of servers is enough see the number of allocated
trash chunks go through the roof in "show pools".

This needs to be backported as far as 2.0.

4 years agoBUG/MINOR: hlua: Don't rely on top of the stack when using Lua buffers
Christopher Faulet [Mon, 3 May 2021 08:11:13 +0000 (10:11 +0200)] 
BUG/MINOR: hlua: Don't rely on top of the stack when using Lua buffers

When the lua buffers are used, a variable number of stack slots may be
used. Thus we cannot assume that we know where the top of the stack is. It
was not an issue for lua < 5.4.3 (at least for small buffers). But
'socket:receive()' now fails with lua 5.4.3 because a light userdata is
systematically pushed on the top of the stack when a buffer is initialized.

To fix the bug, in hlua_socket_receive(), we save the index of the top of
the stack before creating the buffer. This way, we can check the number of
arguments, regardless anything was pushed on the stack or not.

Note that the other buffer usages seem to be safe.

This patch should solve the issue #1240. It should be backport to all stable
branches.

4 years ago[RELEASE] Released version 2.4-dev18 v2.4-dev18
Willy Tarreau [Sat, 1 May 2021 06:25:15 +0000 (08:25 +0200)] 
[RELEASE] Released version 2.4-dev18

Released version 2.4-dev18 with the following main changes :
    - DOC: Fix indentation for `path-strip-dot` normalizer
    - DOC: Fix RFC reference for the percent-to-uppercase normalizer
    - DOC: Add RFC references for the path-strip-dot(dot)? normalizers
    - MINOR: uri_normalizer: Add a `percent-decode-unreserved` normalizer
    - BUG/MINOR: mux-fcgi: Don't send normalized uri to FCGI application
    - REORG: htx: Inline htx functions to add HTX blocks in a message
    - CLEANUP: assorted typo fixes in the code and comments
    - DOC: general: fix white spaces for HTML converter
    - BUG/MINOR: ssl: ssl_sock_prepare_ssl_ctx does not return an error code
    - BUG/MINOR: cpuset: move include guard at the very beginning
    - BUG/MAJOR: fix build on musl with cpu_set_t support
    - BUG/MEDIUM: cpuset: fix build on MacOS
    - BUG/MINOR: htx: Preserve HTX flags when draining data from an HTX message
    - MEDIUM: htx: Refactor htx_xfer_blks() to not rely on hdrs_bytes field
    - CLEANUP: htx: Remove unsued hdrs_bytes field from the HTX start-line
    - BUG/MINOR: mux-h2: Don't encroach on the reserve when decoding headers
    - MEDIUM: http-ana: handle read error on server side if waiting for response
    - MINOR: htx: Limit length of headers name/value when a HTX message is dumped
    - BUG/MINOR: applet: Notify the other side if data were consumed by an applet
    - BUG/MINOR: hlua: Don't consume headers when starting an HTTP lua service
    - BUG/MEDIUM: mux-h2: Handle EOM flag when sending a DATA frame with zero-copy
    - CLEANUP: channel: No longer notify the producer in co_skip()/co_htx_skip()
    - DOC: general: fix example in set-timeout
    - CLEANUP: cfgparse: de-uglify early file error handling in readcfgfile()
    - MINOR: config: add a new "default-path" global directive
    - BUG/MEDIUM: peers: initialize resync timer to get an initial full resync
    - BUG/MEDIUM: peers: register last acked value as origin receiving a resync req
    - BUG/MEDIUM: peers: stop considering ack messages teaching a full resync
    - BUG/MEDIUM: peers: reset starting point if peers appears longly disconnected
    - BUG/MEDIUM: peers: reset commitupdate value in new conns
    - BUG/MEDIUM: peers: re-work updates lookup during the sync on the fly
    - BUG/MEDIUM: peers: reset tables stage flags stages on new conns
    - MINOR: peers: add informative flags about resync process for debugging
    - BUG/MEDIUM: time: fix updating of global_now upon clock drift
    - CLEANUP: freq_ctr: make arguments of freq_ctr_total() const
    - CLEANUP: hlua: rename hlua_appctx* appctx to luactx
    - MINOR: server: fix doc/trace on lb algo for dynamic server creation
    - REGTESTS: server: fix cli_add_server due to previous trace update
    - REGTESTS: add minimal CLI "add map" tests
    - DOC: management: move "set var" to the proper place
    - CLEANUP: map: slightly reorder the add map function
    - MINOR: map: get rid of map_add_key_value()
    - MINOR: map: show the current and next pattern version in "show map"
    - MINOR: map/acl: add the possibility to specify the version in "show map/acl"
    - MINOR: pattern: support purging arbitrary ranges of generations
    - MINOR: map/acl: add the possibility to specify the version in "clear map/acl"
    - MINOR: map/acl: add the "prepare map/acl" CLI command
    - MINOR: map/acl: add the "commit map/acl" CLI command
    - MINOR: map/acl: make "add map/acl" support an optional version number
    - CLEANUP: map/cli: properly align the map/acl help
    - BUILD: compiler: do not use already defined __read_mostly on dragonfly

4 years agoBUILD: compiler: do not use already defined __read_mostly on dragonfly
Amaury Denoyelle [Fri, 23 Apr 2021 14:35:13 +0000 (16:35 +0200)] 
BUILD: compiler: do not use already defined __read_mostly on dragonfly

DragonflyBSD already has an attribute __read_mostly which serves the
same purpose as the one in compiler.h.

No need to be backported as it was added in the current 2.4-dev.

4 years agoCLEANUP: map/cli: properly align the map/acl help
Willy Tarreau [Fri, 30 Apr 2021 13:33:49 +0000 (15:33 +0200)] 
CLEANUP: map/cli: properly align the map/acl help

Due to extra options on some commands, the help started to become
a bit of a mess, so let's realign all the commands.

4 years agoMINOR: map/acl: make "add map/acl" support an optional version number
Willy Tarreau [Fri, 30 Apr 2021 13:23:36 +0000 (15:23 +0200)] 
MINOR: map/acl: make "add map/acl" support an optional version number

By passing a version number to "add map/acl", it becomes possible to
atomically replace maps and ACLs. The principle is that a new version
number is first retrieved by calling"prepare map/acl", and this version
number is used with "add map" and "add acl". Newly added entries then
remain invisible to the matching mechanism but are visible in "show
map/acl" when the version number is specified, or may be cleard with
"clear map/acl". Finally when the insertion is complete, a
"commit map/acl" command must be issued, and the version is atomically
updated so that there is no intermediate state with incomplete entries.

4 years agoMINOR: map/acl: add the "commit map/acl" CLI command
Willy Tarreau [Fri, 30 Apr 2021 13:10:01 +0000 (15:10 +0200)] 
MINOR: map/acl: add the "commit map/acl" CLI command

The command is used to atomically replace a map/acl with the pending
contents of the designated version. The new version must have been
allocated by "prepare map/acl" prior to this. At the moment it is not
possible to force the version when adding new entries, so this may only
be used to atomically clear an ACL/map.

4 years agoMINOR: map/acl: add the "prepare map/acl" CLI command
Willy Tarreau [Fri, 30 Apr 2021 12:57:03 +0000 (14:57 +0200)] 
MINOR: map/acl: add the "prepare map/acl" CLI command

This command allocates a new version for the map/acl, that will be usable
later to prepare the addition of new values to atomically replace existing
ones. Technically speaking the operation consists in atomically incrementing
the next version. There's no "undo" operation here, if a version is not
committed, it will automatically be trashed when committing a newer version.

4 years agoMINOR: map/acl: add the possibility to specify the version in "clear map/acl"
Willy Tarreau [Fri, 30 Apr 2021 11:31:43 +0000 (13:31 +0200)] 
MINOR: map/acl: add the possibility to specify the version in "clear map/acl"

This will ease maintenance of versionned maps by allowing to clear old or
failed updates instead of the current version. Nothing was done to allow
clearing everyhing, though if there was a need for this, implementing "@all"
or something equivalent wouldn't require more than 3 lines of code.

4 years agoMINOR: pattern: support purging arbitrary ranges of generations
Willy Tarreau [Fri, 30 Apr 2021 11:19:37 +0000 (13:19 +0200)] 
MINOR: pattern: support purging arbitrary ranges of generations

Instead of being able to purge only values older than a specific value,
let's support arbitrary ranges and make pat_ref_purge_older() just be
one special case of this one.

4 years agoMINOR: map/acl: add the possibility to specify the version in "show map/acl"
Willy Tarreau [Fri, 30 Apr 2021 10:09:54 +0000 (12:09 +0200)] 
MINOR: map/acl: add the possibility to specify the version in "show map/acl"

The maps and ACLs internally all have two versions, the "current" one,
which is the one being matched against, and the "next" one, the one being
filled during an atomic replacement. Till now the "show" commands only used
to show the current one but it can be convenient to be able to show other
ones as well, so let's add the ability to do this with "show map" and
"show acl". The method used here consists in passing the version number
as "@<ver>" before the map/acl name or ID. It would have been better after
it but that could create confusion with keys already using such a format.

4 years agoMINOR: map: show the current and next pattern version in "show map"
Willy Tarreau [Fri, 30 Apr 2021 08:55:53 +0000 (10:55 +0200)] 
MINOR: map: show the current and next pattern version in "show map"

The "show map" command wasn't updated when pattern generations were
added for atomic reloads, let's report them in the "show map" command
that lists all known maps. It will be useful for users.

4 years agoMINOR: map: get rid of map_add_key_value()
Willy Tarreau [Thu, 29 Apr 2021 14:55:17 +0000 (16:55 +0200)] 
MINOR: map: get rid of map_add_key_value()

This function was only used once in cli_parse_add_map(), and half of the
work it used to do was already known from the caller or testable outside
of the lock. Given that we'll need to modify it soon to pass a generation
number, let's remerge it in the caller instead, using pat_ref_load() which
is the one we'll need.

4 years agoCLEANUP: map: slightly reorder the add map function
Willy Tarreau [Thu, 29 Apr 2021 14:02:48 +0000 (16:02 +0200)] 
CLEANUP: map: slightly reorder the add map function

The function uses two distinct code paths for single the key/value pair
and multiple pairs inserted as payload, each with a copy-paste of the
error handling. Let's modify the loop to factor them out.

4 years agoDOC: management: move "set var" to the proper place
Willy Tarreau [Fri, 30 Apr 2021 12:45:53 +0000 (14:45 +0200)] 
DOC: management: move "set var" to the proper place

Commit b8bd1ee89 ("MEDIUM: cli: add a new experimental "set var" command")
added "get var" and "set var" but "set var" was misplaced in the doc,
breaking the alphabetic ordering.

4 years agoREGTESTS: add minimal CLI "add map" tests
Willy Tarreau [Thu, 29 Apr 2021 14:17:23 +0000 (16:17 +0200)] 
REGTESTS: add minimal CLI "add map" tests

The map_redirect test already tests for "show map", "del map" and
"clear map" but doesn't have any "add map" command. Let's add some
trivial ones involving one regular entry and two other ones added as
payload, checking they are properly returned.

4 years agoREGTESTS: server: fix cli_add_server due to previous trace update
Amaury Denoyelle [Thu, 29 Apr 2021 13:35:46 +0000 (15:35 +0200)] 
REGTESTS: server: fix cli_add_server due to previous trace update

Error output for dynamic server creation if invalid lb algo has changed
since previous commit :
MINOR: server: fix doc/trace on lb algo for dynamic server creation

The vtest regex should have been updated has well to match it.

4 years agoMINOR: server: fix doc/trace on lb algo for dynamic server creation
Amaury Denoyelle [Thu, 29 Apr 2021 12:59:42 +0000 (14:59 +0200)] 
MINOR: server: fix doc/trace on lb algo for dynamic server creation

The text mentionned that only backends with consistent hash method were
supported for dynamic servers. In fact, it is only required that the lb
algorith is dynamic.

4 years agoCLEANUP: hlua: rename hlua_appctx* appctx to luactx
Willy Tarreau [Wed, 28 Apr 2021 15:59:21 +0000 (17:59 +0200)] 
CLEANUP: hlua: rename hlua_appctx* appctx to luactx

There is some serious confusion in the lua interface code related to
sockets and services coming from the hlua_appctx structs being called
"appctx" everywhere, and where the real appctx is reached using
appctx->appctx. This part is a bit of a pain to debug so let's rename
all occurrences of this local variable to "luactx".

4 years agoCLEANUP: freq_ctr: make arguments of freq_ctr_total() const
Willy Tarreau [Wed, 28 Apr 2021 15:44:37 +0000 (17:44 +0200)] 
CLEANUP: freq_ctr: make arguments of freq_ctr_total() const

freq_ctr_total() doesn't modify the freq counters, it should take a
const argument.

4 years agoBUG/MEDIUM: time: fix updating of global_now upon clock drift
Willy Tarreau [Wed, 28 Apr 2021 15:31:22 +0000 (17:31 +0200)] 
BUG/MEDIUM: time: fix updating of global_now upon clock drift

During commit 7e4a557f6 ("MINOR: time: change the global timeval and the
the global tick at once") the approach made sure that the new now_ms was
always higher than or equal to global_now_ms, but by forgetting the old
value. This can cause the first update to global_now_ms to fail if it's
already out of sync, going back into the loop, and the subsequent call
would then succeed due to commit 4d01f3dcd ("MINOR: time: avoid
overwriting the same values of global_now").

And if it goes out of sync, it will fail to update forever, as observed
by Ashley Penney in github issue #1194, causing incorrect freq counters
calculations everywhere. One possible trigger for this issue is one thread
spinning for a few milliseconds while the other ones continue to work.

The issue really is that old_now_ms ought not to be modified in the loop
as it's used for the CAS. But we don't need to structurally guarantee that
global_now_ms grows monotonically as it's computed from the new global_now
which is already verified for this via the __tv_islt() test. Thus, dropping
any corrections on global_now_ms in the loop is the correct way to proceed
as long as this one is always updated to follow global_now.

No backport is needed, this is only for 2.4-dev.

4 years agoMINOR: peers: add informative flags about resync process for debugging
Emeric Brun [Wed, 28 Apr 2021 10:59:35 +0000 (12:59 +0200)] 
MINOR: peers: add informative flags about resync process for debugging

This patch adds miscellenous informative flags raised during the initial
full resync process performed during the reload for debugging purpose.

0x00000010: Timeout waiting for a full resync from a local node
0x00000020: Timeout waiting for a full resync from a remote node
0x00000040: Session aborted learning from a local node
0x00000080: Session aborted learning from a remote node
0x00000100: A local node teach us and was fully up to date
0x00000200: A remote node teach us and was fully up to date
0x00000400: A local node teach us but was partially up to date
0x00000800: A remote node teach us but was partially up to date
0x00001000: A local node was assigned for a full resync
0x00002000: A remote node was assigned for a full resync
0x00004000: A resync was explicitly requested

This patch could be backported on any supported branch

4 years agoBUG/MEDIUM: peers: reset tables stage flags stages on new conns
Emeric Brun [Tue, 20 Apr 2021 12:43:46 +0000 (14:43 +0200)] 
BUG/MEDIUM: peers: reset tables stage flags stages on new conns

Flags used as context to know current status of each table pushing a
full resync to a peer were correctly reset receiving a new resync
request or confirmation message but in case of local peer sync during
reload the resync request is implicit and those flags were not
correctly reset in this case.

This could result to a partial initial resync of some tables after reload
if the connection with the old process was broken and retried.

This patch reset those flags at the end of the handshake for all new
connections to be sure to push a entire full resync if needed.

This patch should be backported on all supported branches ( v >= 1.6 )

4 years agoBUG/MEDIUM: peers: re-work updates lookup during the sync on the fly
Emeric Brun [Wed, 28 Apr 2021 09:48:15 +0000 (11:48 +0200)] 
BUG/MEDIUM: peers: re-work updates lookup during the sync on the fly

Only entries between the opposite of the last 'local update' rotating
counter were considered to be pushed. This processing worked in most
cases because updates are continually pushed trying to reach this point
but it remains some cases where updates id are more far away in the past
and appearing in futur and the push of updates is stuck until the head
reach again the tail which could take a very long time.

This patch re-work the lookup to consider that all positions on the
rotating counter is considered in the past until we reach exactly
the 'local update' value. Doing this, the updates push won't be stuck
anymore.

This patch should be backported on all supported branches ( >= 1.6 )

4 years agoBUG/MEDIUM: peers: reset commitupdate value in new conns
Emeric Brun [Tue, 23 Feb 2021 16:08:08 +0000 (17:08 +0100)] 
BUG/MEDIUM: peers: reset commitupdate value in new conns

The commitupdate value of the table is used to check if the update
is still pending for a push for all peers. To be sure to not miss a
push we reset it just after a handshake success.

This patch should be backported on all supported branches ( >= 1.6 )

4 years agoBUG/MEDIUM: peers: reset starting point if peers appears longly disconnected
Emeric Brun [Tue, 23 Feb 2021 15:50:53 +0000 (16:50 +0100)] 
BUG/MEDIUM: peers: reset starting point if peers appears longly disconnected

If two peers are disconnected and during this period they continue to
process a large amount of local updates, after a reconnection they
may take a long time before restarting to push their updates. because
the last pushed update would appear internally in futur.

This patch fix this resetting the cursor on acked updates at the maximum
point considered in the past if it appears in futur but it means we
may lost some updates. A clean fix would be to update the protocol to
be able to signal a remote peer that is was not updated for a too long
period and needs a full resync but this is not yet supported by the
protocol.

This patch should be backported on all supported branches ( >= 1.6 )

4 years agoBUG/MEDIUM: peers: stop considering ack messages teaching a full resync
Emeric Brun [Thu, 4 Mar 2021 09:27:10 +0000 (10:27 +0100)] 
BUG/MEDIUM: peers: stop considering ack messages teaching a full resync

The re-con cursor was updated receiving any ack message
even if we are pushing a complete resync to a peer. This cursor
is reset at the end of the resync but if the connection is broken
during resync, we could re-start at an unwanted point.

With this patch, the peer stops to consider ack messages pushing
a resync since the resync process has is own acknowlegement and
is always restarted from the beginning in case of broken connection.

This patch should be backported on all supported branches ( >= 1.6 )

4 years agoBUG/MEDIUM: peers: register last acked value as origin receiving a resync req
Emeric Brun [Wed, 28 Apr 2021 07:49:33 +0000 (09:49 +0200)] 
BUG/MEDIUM: peers: register last acked value as origin receiving a resync req

Receiving a resync request, the origins to start the full sync and
to reset after the full resync are mistakenly computed based on
the last update on the table instead of computed based on the
the last update acked by the node requesting the resync.

It could result in disordered or missing updates pushing to the
requester

This patch sets correctly those origins.

This patch should be backported on all supported branches ( >= 1.6 )

4 years agoBUG/MEDIUM: peers: initialize resync timer to get an initial full resync
Emeric Brun [Wed, 21 Apr 2021 14:06:35 +0000 (16:06 +0200)] 
BUG/MEDIUM: peers: initialize resync timer to get an initial full resync

If a reload is performed and there is no incoming connections
from the old process to push a full resync, the new process
can be stuck waiting indefinitely for this conn and it never tries a
fallback requesting a full resync from a remote peer because the resync
timer was init to TICK_ETERNITY.

This patch forces a reset of the resync timer to default value (5 secs)
if we detect value is TICK_ETERNITY.

This patch should be backported on all supported branches ( >= 1.6 )

4 years agoMINOR: config: add a new "default-path" global directive
Willy Tarreau [Tue, 27 Apr 2021 18:29:11 +0000 (20:29 +0200)] 
MINOR: config: add a new "default-path" global directive

By default haproxy loads all files designated by a relative path from the
location the process is started in. In some circumstances it might be
desirable to force all relative paths to start from a different location
just as if the process was started from such locations. This is what this
directive is made for. Technically it will perform a temporary chdir() to
the designated location while processing each configuration file, and will
return to the original directory after processing each file. It takes an
argument indicating the policy to use when loading files whose path does
not start with a slash ('/').

A few options are offered, "current" (the default), "config" (files
relative to config file's dir), "parent" (files relative to config file's
parent dir), and "origin" with an absolute path.

This should address issue #1198.

4 years agoCLEANUP: cfgparse: de-uglify early file error handling in readcfgfile()
Willy Tarreau [Tue, 27 Apr 2021 16:30:28 +0000 (18:30 +0200)] 
CLEANUP: cfgparse: de-uglify early file error handling in readcfgfile()

In readcfgfile() when malloc() fails to allocate a buffer for the
config line, it currently says "parsing[<file>]: out of memory" while
the error is unrelated to the config file and may make one think it has
to do with the file's size. The second test (fopen() returning error)
needs to release the previously allocated line. Both directly return -1
which is not even documented as a valid error code for the function.

Let's simply make sure that the few variables freed at the end are
properly preset, and jump there upon error, after having displayed a
meaningful error message. Now at least we can get this:

  $ ./haproxy -f /dev/kmem
  [NOTICE] 116/191904 (23233) : haproxy version is 2.4-dev17-c3808c-13
  [NOTICE] 116/191904 (23233) : path to executable is ./haproxy
  [ALERT] 116/191904 (23233) : Could not open configuration file /dev/kmem : Permission denied

4 years agoDOC: general: fix example in set-timeout
Alex [Tue, 27 Apr 2021 10:57:07 +0000 (12:57 +0200)] 
DOC: general: fix example in set-timeout

The alternative arguments are always in curly brackets, let's fix it for
set-timeout.
The Example in set-timeout does not have the one of the required argument.

This commit makes the PR https://github.com/cbonte/haproxy-dconv/pull/34
obsolete.

4 years agoCLEANUP: channel: No longer notify the producer in co_skip()/co_htx_skip()
Christopher Faulet [Tue, 27 Apr 2021 21:06:20 +0000 (23:06 +0200)] 
CLEANUP: channel: No longer notify the producer in co_skip()/co_htx_skip()

Thanks to the commit "BUG/MINOR: applet: Notify the other side if data were
consumed by an applet", it is no longer necessary to notify the producer when an
applet skips output data. Now, it is the default applet handler responsibility
to take care of that.

4 years agoBUG/MEDIUM: mux-h2: Handle EOM flag when sending a DATA frame with zero-copy
Christopher Faulet [Tue, 27 Apr 2021 20:51:07 +0000 (22:51 +0200)] 
BUG/MEDIUM: mux-h2: Handle EOM flag when sending a DATA frame with zero-copy

When a DATA frame is sent, we must take care to properly detect the EOM flag
on the HTX message to set ES flag on the frame when necessary, to finish the
stream. But it is only done when data are copied from the HTX message to the
mux buffer and not when the frame are sent via a zero-copy. This patch fixes
this bug.

It is a 2.4-specific bug. No backport is needed.

4 years agoBUG/MINOR: hlua: Don't consume headers when starting an HTTP lua service
Christopher Faulet [Wed, 28 Apr 2021 08:50:21 +0000 (10:50 +0200)] 
BUG/MINOR: hlua: Don't consume headers when starting an HTTP lua service

When an HTTP lua service is started, headers are consumed before calling the
script. When it was initialized, the headers were stored in a lua array,
thus they can be removed from the HTX message because the lua service will
no longer access them. But it is a problem with bodyless messages because
the EOM flag is lost. Indeed, once the headers are consumed, the message is
empty and the buffer is reset, included the flags.

Now, the headers are not immediately consumed. We will skip them if
applet:receive() or applet:getline(). This way, the EOM flag is preserved.
At the end, when the script is finished, all output data are consumed, thus
this remains safe.

It is a 2.4-specific bug. No backport is needed.

4 years agoBUG/MINOR: applet: Notify the other side if data were consumed by an applet
Christopher Faulet [Tue, 27 Apr 2021 15:08:10 +0000 (17:08 +0200)] 
BUG/MINOR: applet: Notify the other side if data were consumed by an applet

If an applet consumed output data (the amount of output data has changed
between before and after the call to the applet), the producer is
notified. It means CF_WRITE_PARTIAL and CF_WROTE_DATA are set on the output
channel and the opposite stream interface is notified some room was made in
its input buffer. This way, it is no longer the applet responsibility to
take care of it. However, it doesn't matter if the applet does the same.

Said like that, it looks like an improvement not a bug. But it really fixes
a bug in the lua, for HTTP applets. Indeed, applet:receive() and
applet:getline() are buggy for HTTP applets. Data are consumed but the
producer is not notified. It means if the payload is not fully received in
one time, the applet may be blocked because the producer remains blocked (it
is time dependent).

This patch must be backported as far as 2.0 (only for the HTX part).

4 years agoMINOR: htx: Limit length of headers name/value when a HTX message is dumped
Christopher Faulet [Tue, 27 Apr 2021 09:29:00 +0000 (11:29 +0200)] 
MINOR: htx: Limit length of headers name/value when a HTX message is dumped

In htx_dump() function, we now limit the length of the headers name and the
value to not fully print huge headers.

4 years agoMEDIUM: http-ana: handle read error on server side if waiting for response
Christopher Faulet [Tue, 27 Apr 2021 08:56:28 +0000 (10:56 +0200)] 
MEDIUM: http-ana: handle read error on server side if waiting for response

A read error on the server side is also reported as a write error on the
client side. It means some times, a server side error is handled on the
client side. Among others, it is the case when the client side is waiting
for the response while the request processing is already finished. In this
case, the error is not handled as a server error. It is not accurate.

So now, when the request processing is finished but not the response
processing and if a read error was encountered on the server side, the error
is not immediatly processed on the client side, to let a chance to response
analysers to properly catch the error.

4 years agoBUG/MINOR: mux-h2: Don't encroach on the reserve when decoding headers
Christopher Faulet [Mon, 26 Apr 2021 15:46:13 +0000 (17:46 +0200)] 
BUG/MINOR: mux-h2: Don't encroach on the reserve when decoding headers

Since the input buffer is transferred to the stream when it is created,
there is no longer control on the request size to be sure the buffer's
reserve is still respected. It was automatically performed in h2_rcv_buf()
because the caller took care to provide the correct available space in the
buffer. The control is still there but it is no longer applied on the
request headers. Now, we should take care of the reserve when the headers
are decoded, before the stream creation.

The test is performed for the request and the response.

It is a 2.4-specific bug. No backport is needed.

4 years agoCLEANUP: htx: Remove unsued hdrs_bytes field from the HTX start-line
Christopher Faulet [Thu, 22 Apr 2021 07:50:14 +0000 (09:50 +0200)] 
CLEANUP: htx: Remove unsued hdrs_bytes field from the HTX start-line

Thanks to the htx_xfer_blks() refactoring, it is now possible to remove
hdrs_bytes field from the start-line because no function rely on it anymore.

4 years agoMEDIUM: htx: Refactor htx_xfer_blks() to not rely on hdrs_bytes field
Christopher Faulet [Thu, 22 Apr 2021 07:45:18 +0000 (09:45 +0200)] 
MEDIUM: htx: Refactor htx_xfer_blks() to not rely on hdrs_bytes field

It is the only function using the hdrs_bytes start-line field. Thus the
function has been refactored to no longer rely on it. To do so, we first
copy HTX blocks to the destination message, without removing them from the
source message. If the copy is interrupted on headers or trailers, we roll
back. Otherwise, data are drained from the source buffer.

Most of time, the copy will succeeds. So the roll back is only performed in
the worst but very rare case.

4 years agoBUG/MINOR: htx: Preserve HTX flags when draining data from an HTX message
Christopher Faulet [Thu, 22 Apr 2021 07:43:47 +0000 (09:43 +0200)] 
BUG/MINOR: htx: Preserve HTX flags when draining data from an HTX message

When all data of an HTX message are drained, we rely on htx_reset() to
reinit the message state. However, the flags must be preserved. It is, among
other things, important to preserve processing or parsing errors.

This patch must be backported as far as 2.0.

4 years agoBUG/MEDIUM: cpuset: fix build on MacOS
Amaury Denoyelle [Tue, 27 Apr 2021 14:45:29 +0000 (16:45 +0200)] 
BUG/MEDIUM: cpuset: fix build on MacOS

The compilation fails due to the following commit:
fc6ac53dca8391ba9c32bc716fb61267b475ba71
BUG/MAJOR: fix build on musl with cpu_set_t support

The new global variable cpu_map conflicted with a local variable of the
same name in the code path for the apple platform when setting the
process affinity.

This does not need to be backported.

4 years agoBUG/MAJOR: fix build on musl with cpu_set_t support
Amaury Denoyelle [Tue, 27 Apr 2021 08:46:36 +0000 (10:46 +0200)] 
BUG/MAJOR: fix build on musl with cpu_set_t support

Move cpu_map structure outside of the global struct to a global
variable defined in cpuset.c compilation unit. This allows to reorganize
the includes without having to define _GNU_SOURCE everywhere for the
support of the cpu_set_t.

This fixes the compilation with musl libc, most notably used for the
alpine based docker image.

This fixes the github issue #1235.

No need to backport as this feature is new in the current
2.4-dev.

4 years agoBUG/MINOR: cpuset: move include guard at the very beginning
Amaury Denoyelle [Tue, 27 Apr 2021 08:39:39 +0000 (10:39 +0200)] 
BUG/MINOR: cpuset: move include guard at the very beginning

The include guard in cpuset-t.h were misplaced and should be the first
directive of the file.

No need to backport.

4 years agoBUG/MINOR: ssl: ssl_sock_prepare_ssl_ctx does not return an error code
Remi Tricot-Le Breton [Wed, 21 Apr 2021 13:32:46 +0000 (15:32 +0200)] 
BUG/MINOR: ssl: ssl_sock_prepare_ssl_ctx does not return an error code

The return value check was wrongly based on error codes when the
function actually returns an error number.
This bug was introduced by f3eedfe19592ebcbaa5b97d8c68aa162e7f6f8fa
which is a feature not present before branch 2.4.

It does not need to be backported.

4 years agoDOC: general: fix white spaces for HTML converter
Alex [Sat, 24 Apr 2021 11:02:21 +0000 (13:02 +0200)] 
DOC: general: fix white spaces for HTML converter

The HTML converter expects some formats to recognize if a keyword is a
keyword.

4 years agoCLEANUP: assorted typo fixes in the code and comments
Ilya Shipitsin [Sat, 24 Apr 2021 08:25:42 +0000 (13:25 +0500)] 
CLEANUP: assorted typo fixes in the code and comments

This is 22nd iteration of typo fixes

4 years agoREORG: htx: Inline htx functions to add HTX blocks in a message
Christopher Faulet [Mon, 26 Apr 2021 08:18:16 +0000 (10:18 +0200)] 
REORG: htx: Inline htx functions to add HTX blocks in a message

The HTX functions used to add new HTX blocks in a message have been moved to
the header file to inline them in calling functions. These functions are
small enough.

4 years agoBUG/MINOR: mux-fcgi: Don't send normalized uri to FCGI application
Christopher Faulet [Mon, 26 Apr 2021 07:38:55 +0000 (09:38 +0200)] 
BUG/MINOR: mux-fcgi: Don't send normalized uri to FCGI application

A normalized URI is the internal term used to specify an URI is stored using
the absolute format (scheme + authority + path). For now, it is only used
for H2 clients. It is the default and recommended format for H2 request.
However, it is unusual for H1 servers to receive such URI. So in this case,
we only send the path of the absolute URI. It is performed for H1 servers,
but not for FCGI applications. This patch fixes the difference.

Note that it is not a real bug, because FCGI applications should support
abosolute URI.

Note also a normalized URI is only detected for H2 clients when a request is
received. There is no such test on the H1 side. It means an absolute URI
received from an H1 client will be sent without modification to an H1 server
or a FCGI application.

To make it possible, a dedicated function has been added to get the H1
URI. This function is called by the H1 and the FCGI multiplexer when a
request is sent to a server.

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