]> git.ipfire.org Git - thirdparty/haproxy.git/log
thirdparty/haproxy.git
5 years agoMINOR: debug: add a new DEBUG_FD build option
Willy Tarreau [Tue, 23 Jun 2020 08:04:54 +0000 (10:04 +0200)] 
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.

5 years agoBUG/MINOR: http_ana: clarify connection pointer check on L7 retry
Willy Tarreau [Tue, 23 Jun 2020 03:58:20 +0000 (05:58 +0200)] 
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.

5 years agoMINOR: cfgparse: Warn on truncated lines / files
Tim Duesterhus [Mon, 22 Jun 2020 20:57:45 +0000 (22:57 +0200)] 
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).

5 years agoBUG/MINOR: cfgparse: Support configurations without newline at EOF
Tim Duesterhus [Mon, 22 Jun 2020 20:57:44 +0000 (22:57 +0200)] 
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
9e1758efbd68c8b1d27e17e2abe4444e110f3ebe which is 2.2 specific. No backport
needed.

5 years agoBUG/MINOR: spoe: correction of setting bits for analyzer
Miroslav Zagorac [Fri, 19 Jun 2020 20:17:17 +0000 (22:17 +0200)] 
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.

5 years agoBUG/MEDIUM: fcgi-app: Resolve the sink if a fcgi-app logs in a ring buffer
Christopher Faulet [Mon, 22 Jun 2020 09:07:18 +0000 (11:07 +0200)] 
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.

5 years agoREGTEST: Add a simple script to tests errorfile directives in proxy sections
Christopher Faulet [Thu, 11 Jun 2020 11:43:20 +0000 (13:43 +0200)] 
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.

5 years ago[RELEASE] Released version 2.2-dev10 v2.2-dev10
Willy Tarreau [Fri, 19 Jun 2020 19:43:26 +0000 (21:43 +0200)] 
[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

5 years agoDOC: configuration: http-check send was missing from matrix
Peter Gervai [Thu, 11 Jun 2020 16:26:36 +0000 (18:26 +0200)] 
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.

5 years agoDOC: configuration: Unindent non-code sentences in the protobuf example
Peter Gervai [Thu, 11 Jun 2020 16:05:11 +0000 (18:05 +0200)] 
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.

5 years agoBUG/MINOR: systemd: Wait for network to be online
Ryan O'Hara [Mon, 15 Jun 2020 16:34:54 +0000 (11:34 -0500)] 
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>
5 years agoMINOR: mux_h1: Set H1_F_CO_MSG_MORE if we know we have more to send.
Olivier Houchard [Fri, 19 Jun 2020 14:15:05 +0000 (16:15 +0200)] 
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.

5 years agoBUG/MEDIUM: stream-int: fix loss of CO_SFL_MSG_MORE flag in forwarding
Willy Tarreau [Fri, 19 Jun 2020 15:07:06 +0000 (17:07 +0200)] 
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.

5 years agoMEDIUM: map: make the "clear map" operation yield
Willy Tarreau [Fri, 20 Dec 2019 17:22:02 +0000 (18:22 +0100)] 
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.

5 years agoBUG/MAJOR: init: properly compute the default global.maxpipes value
Willy Tarreau [Fri, 19 Jun 2020 14:20:59 +0000 (16:20 +0200)] 
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.

5 years agoREGTEST: http-rules: test spaces in ACLs with master CLI
William Lallemand [Thu, 18 Jun 2020 16:56:44 +0000 (18:56 +0200)] 
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.

5 years agoREGTEST: http-rules: test spaces in ACLs
William Lallemand [Thu, 18 Jun 2020 16:52:18 +0000 (18:52 +0200)] 
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.

5 years agoBUG/MINOR: mworker/cli: fix semicolon escaping in master CLI
William Lallemand [Thu, 18 Jun 2020 16:45:04 +0000 (18:45 +0200)] 
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.

5 years agoBUG/MINOR: mworker/cli: fix the escaping in the master CLI
William Lallemand [Thu, 18 Jun 2020 16:03:57 +0000 (18:03 +0200)] 
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.

5 years agoBUG/MINOR: cli: allow space escaping on the CLI
Yves Lafon [Mon, 8 Jun 2020 14:08:06 +0000 (16:08 +0200)] 
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.

5 years agoMEDIUM: tasks: also process late wakeups in process_runnable_tasks()
Willy Tarreau [Fri, 19 Jun 2020 10:17:55 +0000 (12:17 +0200)] 
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.

5 years agoMEDIUM: tasks: clean up the front side of the wait queue in wake_expired_tasks()
Willy Tarreau [Fri, 19 Jun 2020 09:50:27 +0000 (11:50 +0200)] 
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.

5 years agoMINOR: haproxy: process signals before runnable tasks
Willy Tarreau [Fri, 19 Jun 2020 10:06:34 +0000 (12:06 +0200)] 
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.

5 years agoCONTRIB: debug: add the missing flags CO_FL_SAFE_LIST and CO_FL_IDLE_LIST
Willy Tarreau [Fri, 19 Jun 2020 06:47:30 +0000 (08:47 +0200)] 
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.

5 years agoMEDIUM: fd: add experimental support for edge-triggered polling
Willy Tarreau [Thu, 18 Jun 2020 06:58:47 +0000 (08:58 +0200)] 
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.

5 years agoMEDIUM: peers: add the "localpeer" global option
Dragan Dosen [Thu, 18 Jun 2020 16:24:05 +0000 (18:24 +0200)] 
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.

5 years agoMINOR: peers: do not use localpeer as an array anymore
Dragan Dosen [Thu, 18 Jun 2020 14:56:47 +0000 (16:56 +0200)] 
MINOR: peers: do not use localpeer as an array anymore

It is now dynamically allocated by using strdup().

5 years agoMINOR: fd: factorize the fd_takeover() exit path to make it safer
Willy Tarreau [Thu, 18 Jun 2020 06:14:59 +0000 (08:14 +0200)] 
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.

5 years agoMINOR: fd: slightly optimize the fd_takeover double-CAS loop
Willy Tarreau [Thu, 18 Jun 2020 06:05:15 +0000 (08:05 +0200)] 
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.

5 years agoMEDIUM: fd: refine the fd_takeover() migration lock
Willy Tarreau [Thu, 18 Jun 2020 05:28:09 +0000 (07:28 +0200)] 
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.

5 years agoMINOR: activity: rename the "stream" field to "stream_calls"
Willy Tarreau [Wed, 17 Jun 2020 18:49:49 +0000 (20:49 +0200)] 
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.

5 years agoMINOR: activity: group the per-loop counters at the top
Willy Tarreau [Wed, 17 Jun 2020 18:44:28 +0000 (20:44 +0200)] 
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.

5 years agoBUG/MEDIUM: fd: Call fd_stop_recv() when we just got a fd.
Olivier Houchard [Wed, 17 Jun 2020 18:34:05 +0000 (20:34 +0200)] 
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.

5 years agoBUG/MEDIUM: fd: Don't fd_stop_recv() a fd we don't own.
Olivier Houchard [Wed, 17 Jun 2020 18:32:34 +0000 (20:32 +0200)] 
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.

5 years agoMINOR: fd: Fix a typo in a coment.
Olivier Houchard [Wed, 17 Jun 2020 18:28:27 +0000 (20:28 +0200)] 
MINOR: fd: Fix a typo in a coment.

The function si called fd_takeover, not fd_takeother.

5 years agoMINOR: activity: rename confusing poll_* fields in the output
Willy Tarreau [Wed, 17 Jun 2020 18:35:33 +0000 (20:35 +0200)] 
MINOR: activity: rename confusing poll_* fields in the output

We have poll_drop, poll_dead and poll_skip which are confusingly named
like their poll_io and poll_exp counterparts except that they are not
per poll() call but per-fd. This patch renames them to poll_drop_fd(),
poll_dead_fd() and poll_skip_fd() for this reason.

5 years agoMINOR: activity: report the number of times poll() reports I/O
Willy Tarreau [Wed, 17 Jun 2020 18:25:18 +0000 (20:25 +0200)] 
MINOR: activity: report the number of times poll() reports I/O

The "show activity" output mentions a number of indicators to explain
wake up reasons but doesn't have the number of times poll() sees some
I/O. And given that multiple events can happen simultaneously, it's
not always possible to deduce this metric by subtracting.

This patch adds a new "poll_io" counter that allows one to see how
often poll() returns with at least one active FD. This should help
detect stuck events and measure various ratios of poll sub-metrics.

5 years agoDOC: fd: make it clear that some fields ordering must absolutely be respected
Willy Tarreau [Wed, 17 Jun 2020 17:58:37 +0000 (19:58 +0200)] 
DOC: fd: make it clear that some fields ordering must absolutely be respected

fd_set_running() and fd_takeover() may both use a double-word CAS on the
(running_mask, thread_mask) couple and as such they expect the fields to
be exactly arranged like this. It's critical not to reorder them, so add
a comment to avoid such a potential mistake later.

5 years agoCLEANUP: activity: remove unused counter fd_lock
Willy Tarreau [Wed, 17 Jun 2020 17:12:43 +0000 (19:12 +0200)] 
CLEANUP: activity: remove unused counter fd_lock

Since 2.1-dev2, with commit 305d5ab46 ("MAJOR: fd: Get rid of the fd cache.")
we don't have the fd_lock anymore and as such its acitvity counter is always
zero. Let's remove it from the struct and from "show activity" output, as
there are already plenty of indicators to look at.

The cache line comment in the struct activity was updated to reflect
reality as it looks like another one already got removed in the past.

5 years agoBUG/MAJOR: connection: always disable ready events once reported
Willy Tarreau [Wed, 17 Jun 2020 14:26:22 +0000 (16:26 +0200)] 
BUG/MAJOR: connection: always disable ready events once reported

This effectively reverts the two following commits:

  6f95f6e11 ("OPTIM: connection: disable receiving on disabled events when the run queue is too high")
  065a02561 ("MEDIUM: connection: don't stop receiving events in the FD handler")

The problem as reported in issue #662 is that when the events signals
the readiness of input data that has to be forwarded over a congested
stream, the mux will read data and wake the stream up to forward them,
but the buffer full condition makes this impossible immediately, then
nobody in the chain will be able to disable the event after it was
first reported. And given we don't know at the connection level whether
an event was already reported or not, we can't decide anymore to
forcefully stop it if for any reason its processing gets delayed.

The problem is magnified in issue #662 by the fact that a shutdown is
reported with pending data occupying the buffer. The shutdown will
strike in loops and cause the upper layer stream to be notified until
it's handled, but with a buffer full it's not possible to call cs_recv()
hence to purge the event.

All this can only be handled optimally by implementing a lower layer,
direct mux-to-mux forwarding that will not require any scheduling. This
was no wake up will be needed and the event will be instantly handled
or paused for a long time.

For now let's simply revert these optimizations. Running a 1 MB transfer
test over H2 using 8 connections having each 32 streams with a limited
link of 320 Mbps shows the following profile before this fix:

   calls  syscall       (100% CPU)
  ------  -------
  259878  epoll_wait
  519759  clock_gettime
   17277  sendto
   17129  recvfrom
     672  epoll_ctl

And the following one after the fix:

   calls  syscall       (2-3% CPU)
  ------  -------
   17201  sendto
   17357  recvfrom
    2304  epoll_wait
    4609  clock_gettime
    1200  epoll_ctl

Thus the behavior is much better.

No backport is needed as these patches were only in 2.2-dev.

Many thanks to William Dauchy for reporting a lot of details around this
difficult issue.

5 years agoBUILD: Fix build by including haproxy/global.h
Olivier Houchard [Tue, 16 Jun 2020 21:35:00 +0000 (23:35 +0200)] 
BUILD: Fix build by including haproxy/global.h

In srv/version.c, fix build by including haproxy/global.h, so that
REGISTER_BUILD_OPTS is properly defined.

5 years agoMINOR: version: report the presence of the compiler's address sanitizer
Willy Tarreau [Tue, 16 Jun 2020 17:14:19 +0000 (19:14 +0200)] 
MINOR: version: report the presence of the compiler's address sanitizer

Since we've seen clang emit bad code when the address sanitizer is enabled
at -O2, better clearly report it in the version output. It is detected both
for clang and gcc (both tested with and without).

5 years agoMINOR: compiler: always define __has_feature()
Willy Tarreau [Tue, 16 Jun 2020 17:13:24 +0000 (19:13 +0200)] 
MINOR: compiler: always define __has_feature()

This macro is provided by clang but gcc lacks it. Not having it makes
it painful to test features on both compilers. Better define it to zero
when not available so that __has_feature(foo) never errors.

5 years agoMINOR: version: put the compiler version output into version.c not haproxy.c
Willy Tarreau [Tue, 16 Jun 2020 17:11:11 +0000 (19:11 +0200)] 
MINOR: version: put the compiler version output into version.c not haproxy.c

For an unknown reason in commit bb1b63c079 I placed the compiler version
output in haproxy.c instead of version.c. Better have it in version.c which
is more suitable to this sort of things.

5 years agoBUG/MINOR: spoe: add missing key length check before checking key names
Willy Tarreau [Tue, 16 Jun 2020 15:58:14 +0000 (17:58 +0200)] 
BUG/MINOR: spoe: add missing key length check before checking key names

The spoe parser fails to check that the decoded key length is large
enough to match a given key but it uses the returned length in memcmp().
So returning "ver" could match "version" for example. In addition this
makes clang 10's ASAN complain because the second argument to memcmp()
is the static key which is shorter than the decoded buffer size, which
in practice has no impact.

I'm still not 100% sure the parser is entirely correct because even
with this fix it cannot parse a key whose name matches the beginning
of another one, but in practice this does not happen. Ideally a
preliminary length check before the comparison would be safer.

This needs to be backported as far as 1.7.

5 years agoBUG/MINOR: cfgparse: Add missing fatal++ in PARSE_ERR_HEX case
Tim Duesterhus [Tue, 16 Jun 2020 16:14:21 +0000 (18:14 +0200)] 
BUG/MINOR: cfgparse: Add missing fatal++ in PARSE_ERR_HEX case

This fixes up commit 32234e751320b60a3879f274d4a4753d7570e757. This
patch should be backported whereever that commit is backported.

5 years agoMINOR: http: do not close connections anymore after internal responses
Willy Tarreau [Tue, 16 Jun 2020 15:36:36 +0000 (17:36 +0200)] 
MINOR: http: do not close connections anymore after internal responses

Since we dropped support for legacy mode, it's not the stream which
deals with the connection but the mux, and there's no point in closing
the client connection after most internal status codes. For example if
the client gets a 401 or a 503 because a server doesn't respond, it
makes no sense forcing the connection to close after reporting this
status, because it's already done by the mux if the client asks for it
or is not compatible with keep-alive. This current state was inherited
from the early days but is still limiting the amount of client-side
connection reuse in a number of circumstances (typically server-side
errors). This change was planned for 2.1 but forgotten.

The status codes for which the connection is not closed anymore are those
that do not depend on the client side connection itself, which are all
except 400 and 408. This could be backported to 2.1 but not further, in
order to make sure legacy and HTX behave strictly similarly.

5 years agoBUG/MEDIUM: cfgparse: stop after a reasonable amount of fatal error
Willy Tarreau [Tue, 16 Jun 2020 15:14:33 +0000 (17:14 +0200)] 
BUG/MEDIUM: cfgparse: stop after a reasonable amount of fatal error

One issue with the config parser is that while it tries to report as many
errors as possible at once, it's actually unbounded. Thus, when calling
haproxy on a wrong file, it can take ages to process, such as here on
half a gigabyte of map file instead of config file:

  $ time ./haproxy -c -f large.map 2>&1 |wc -l
  16777220

  real    0m31.324s
  user    0m22.595s
  sys     0m28.909s

This patch modifies readcfgfile() to stop reading the config file after a
reasonable amount of fatal errors. This threshold is set to 50, which seems
more than enough to spot a recurrent issue with a bit of context in a terminal
to address several issues at once, without filling logs nor taking time to
parse the file. The difference is clear now:

  $ time ./haproxy -c -f large.map 2>&1 |wc -l
  55

  real    0m0.005s
  user    0m0.004s
  sys     0m0.003s

This may be backported to older versions without causing too many
difficulties. However the patch will not apply as-is, it will require
to increment the "fatal" count for each place where ERR_FATAL is set
in the parsing loop.

5 years agoBUG/MEDIUM: cfgparse: use parse_line() to expand/unquote/unescape config lines
Willy Tarreau [Tue, 16 Jun 2020 14:32:59 +0000 (16:32 +0200)] 
BUG/MEDIUM: cfgparse: use parse_line() to expand/unquote/unescape config lines

Issue 22689 in oss-fuzz shows that specially crafted config files can take
a long time to process. This happens when variable expansion, backslash
escaping or unquoting causes calls to memmove() and possibly to realloc()
resulting in O(N^2) complexity with N following the line size.

By using parse_line() we now have a safe parser that remains in O(N)
regardless of the type of operation. Error reporting changed a little bit
since the errors are not reported anymore from the deepest parsing level.
As such we now report the beginning of the error. One benefit is that for
many invalid character sequences, the original line is shown and the first
bad char or sequence is designated with a caret ('^'), which tends to be
visually easier to spot, for example:

  [ALERT] 167/170507 (14633) : parsing [mini5.cfg:19]: unmatched brace in environment variable name below:
    "${VAR"}
      ^
or:

  [ALERT] 167/170645 (14640) : parsing [mini5.cfg:18]: unmatched quote below:
    timeout client 10s'
                      ^

In case the target buffer is too short for the new line, the output buffer
is grown in 1kB chunks and kept till the end, so that it should not happen
too often.

Before this patch a test like below involving a 4 MB long line would take
138s to process, 98% of which were spent in __memmove_avx_unaligned_erms(),
and now it takes only 65 milliseconds:

  $ perl -e 'print "\"\$A\""x1000000,"\n"' | ./haproxy -c -f /dev/stdin 2>/dev/null

This may be backported to stable versions after a long period of
observation to be sure nothing broke. It relies on patch "MINOR: tools:
add a new configurable line parse, parse_line()".

5 years agoMINOR: tools: add a new configurable line parse, parse_line()
Willy Tarreau [Tue, 16 Jun 2020 14:27:26 +0000 (16:27 +0200)] 
MINOR: tools: add a new configurable line parse, parse_line()

This function takes on input a string to tokenize, an output storage
(which may be the same) and a number of options indicating how to handle
certain characters (single & double quote support, backslash support,
end of line on '#', environment variables etc). On output it will provide
a list of pointers to individual words after having possibly unescaped
some character sequences, handled quotes and resolved environment
variables, and it will also indicate a status made of:
  - a list of failures (overlap between src/dst, wrong quote etc)
  - the pointer to the first sequence in error
  - the required output length (a-la snprintf()).

This allows a caller to freely unescape/unquote a string by using a
pre-allocated temporary buffer and expand it as necessary. It takes
extreme care at avoiding expensive operations and intentionally does
not use memmove() when removing escapes, hence the reason for the
different input and output buffers. The goal is to use it as the basis
for the config parser.

5 years agoBUG/MEDIUM: ebtree: use a byte-per-byte memcmp() to compare memory blocks
Willy Tarreau [Tue, 16 Jun 2020 09:10:53 +0000 (11:10 +0200)] 
BUG/MEDIUM: ebtree: use a byte-per-byte memcmp() to compare memory blocks

As reported in issue #689, there is a subtle bug in the ebtree code used
to compared memory blocks. It stems from the platform-dependent memcmp()
implementation. Original implementations used to perform a byte-per-byte
comparison and to stop at the first non-matching byte, as in this old
example:

   https://www.retro11.de/ouxr/211bsd/usr/src/lib/libc/compat-sys5/memcmp.c.html

The ebtree code has been relying on this to detect the first non-matching
byte when comparing keys. This is made so that a zero-terminated string
can fail to match against a longer string.

Over time, especially with large busses and SIMD instruction sets,
multi-byte comparisons have appeared, making the processor fetch bytes
past the first different byte, which could possibly be a trailing zero.
This means that it's possible to read past the allocated area for a
string if it was allocated by strdup().

This is not correct and definitely confuses address sanitizers. In real
life the problem doesn't have visible consequences. Indeed, multi-byte
comparisons are implemented so that aligned words are loaded (e.g. 512
bits at once to process a cache line at a time). So there is no way such
a multi-byte access will cross a page boundary and end up reading from
an unallocated zone. This is why it was never noticed before.

This patch addresses this by implementing a one-byte-at-a-time memcmp()
variant for ebtree, called eb_memcmp(). It's optimized for both small and
long strings and guarantees to stop after the first non-matching byte. It
only needs 5 instructions in the loop and was measured to be 3.2 times
faster than the glibc's AVX2-optimized memcmp() on short strings (1 to
257 bytes), since that latter one comes with a significant setup cost.
The break-even seems to be at 512 bytes where both version perform
equally, which is way longer than what's used in general here.

This fix should be backported to stable versions and reintegrated into
the ebtree code.

5 years agoBUG/MAJOR: vars: Fix bogus free() during deinit() for http-request rules
Tim Duesterhus [Sun, 14 Jun 2020 15:27:36 +0000 (17:27 +0200)] 
BUG/MAJOR: vars: Fix bogus free() during deinit() for http-request rules

We cannot simply `release_sample_expr(rule->arg.vars.expr)` for a
`struct act_rule`, because `rule->arg` is a union that might not
contain valid `vars`. This leads to a crash on a configuration using
`http-request redirect` and possibly others:

    frontend http
     mode http
     bind 127.0.0.1:80
     http-request redirect scheme https

Instead a `struct act_rule` has a `release_ptr` that must be used
to properly free any additional storage allocated.

This patch fixes a regression in commit ff78fcdd7f15c8626c7e70add7a935221ee2920c.
It must be backported to whereever that patch is backported.

It has be verified that the configuration above no longer crashes.
It has also been verified that the configuration in ff78fcdd7f15c8626c7e70add7a935221ee2920c
does not leak.

5 years agoBUILD: haproxy: mark deinit_and_exit() as noreturn
Willy Tarreau [Mon, 15 Jun 2020 16:43:46 +0000 (18:43 +0200)] 
BUILD: haproxy: mark deinit_and_exit() as noreturn

Commit 0a3b43d9c ("MINOR: haproxy: Make use of deinit_and_exit() for
clean exits") introduced this build warning:

  src/haproxy.c: In function 'main':
  src/haproxy.c:3775:1: warning: control reaches end of non-void function [-Wreturn-type]
   }
   ^

This is because the new deinit_and_exit() is not marked as "noreturn"
so depending on the optimizations, the noreturn attribute of exit() will
either leak through it and silence the warning or not and confuse the
compiler. Let's just add the attribute to fix this.

No backport is needed, this is purely 2.2.

5 years agoBUG/MINOR: tcp-rules: tcp-response must check the buffer's fullness
Willy Tarreau [Mon, 15 Jun 2020 16:08:07 +0000 (18:08 +0200)] 
BUG/MINOR: tcp-rules: tcp-response must check the buffer's fullness

It's unclear why the buffer length wasn't considered when tcp-response
rules were added in 1.5-dev3 with commit 97679e790 ("[MEDIUM] Implement
tcp inspect response rules"). But it's impossible to write working
tcp-response content rules as they're always waiting for the expiration
and do not consider the fact that the buffer is full. It's likely that
tcp-response content rules were only used with HTTP traffic.

This may be backported to stable versions, though it's not very
important considering that that nobody reported this in 10 years.

5 years agoBUG/MINOR: http: make smp_fetch_body() report that the contents may change
Willy Tarreau [Mon, 15 Jun 2020 16:01:10 +0000 (18:01 +0200)] 
BUG/MINOR: http: make smp_fetch_body() report that the contents may change

The req_body and res_body sample fetch functions forgot to set the
SMP_F_MAY_CHANGE flag, making them unusable in tcp content rules. Now we
set the flag as long as the channel is not full and nothing indicates
the end was reached.

This is marked as a bug because it's unusual for a sample fetch function
to return a final verdict while data my change, but this results from a
limitation that was affecting the legacy mode where it was not possible
to know whether the end was reached without de-chunking the message. In
HTX there is no more reason to limit this. This fix could be backported
to 2.1, and to 2.0 if really needed, though it will only be doable for
HTX, and legacy cannot be fixed.

5 years agoBUILD: atomic: add string.h for memcpy() on ARM64
Willy Tarreau [Sun, 14 Jun 2020 06:06:55 +0000 (08:06 +0200)] 
BUILD: atomic: add string.h for memcpy() on ARM64

As reported in issue #686, ARM64 build fails since the include files
reorganization. This is caused by the lack of string.h while a memcpy()
is present in __ha_cas_dw().

5 years agoBUILD: compression: make gcc 10 happy with free_zlib()
Willy Tarreau [Sun, 14 Jun 2020 05:50:18 +0000 (07:50 +0200)] 
BUILD: compression: make gcc 10 happy with free_zlib()

In issue #685 gcc 10 seems to find a null pointer deref in free_zlib().
There is no such case because all possible pools are tested and there's
no other one in zlib, except if there's a bug or memory corruption
somewhere else. The code used to be written like this to make sure that
any such bug couldn't remain unnoticed.

Now gcc 10 sees this theorical code path and complains, so let's just
change the code to place an explicit crash in case of no match (which
must never happen).

This might be backported if other versions are affected.

5 years agoBUG/MINOR: haproxy: Free rule->arg.vars.expr during deinit_act_rules
Tim Duesterhus [Sat, 13 Jun 2020 22:37:43 +0000 (00:37 +0200)] 
BUG/MINOR: haproxy: Free rule->arg.vars.expr during deinit_act_rules

Given the following example configuration:

    frontend foo
     bind *:8080
     mode http
     http-request  set-var(txn.foo) str(bar)

Running a configuration check within valgrind reports:

    ==23665== Memcheck, a memory error detector
    ==23665== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
    ==23665== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
    ==23665== Command: ./haproxy -c -f ./crasher.cfg
    ==23665==
    [WARNING] 165/002941 (23665) : config : missing timeouts for frontend 'foo'.
       | While not properly invalid, you will certainly encounter various problems
       | with such a configuration. To fix this, please ensure that all following
       | timeouts are set to a non-zero value: 'client', 'connect', 'server'.
    Warnings were found.
    Configuration file is valid
    ==23665==
    ==23665== HEAP SUMMARY:
    ==23665==     in use at exit: 314,008 bytes in 87 blocks
    ==23665==   total heap usage: 160 allocs, 73 frees, 1,448,074 bytes allocated
    ==23665==
    ==23665== 132 (48 direct, 84 indirect) bytes in 1 blocks are definitely lost in loss record 15 of 28
    ==23665==    at 0x4C2FB55: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
    ==23665==    by 0x4A2612: sample_parse_expr (sample.c:876)
    ==23665==    by 0x54DF84: parse_store (vars.c:766)
    ==23665==    by 0x528BDF: parse_http_req_cond (http_rules.c:95)
    ==23665==    by 0x469F36: cfg_parse_listen (cfgparse-listen.c:1339)
    ==23665==    by 0x459E33: readcfgfile (cfgparse.c:2167)
    ==23665==    by 0x5074FD: init (haproxy.c:2021)
    ==23665==    by 0x418262: main (haproxy.c:3126)
    ==23665==
    ==23665== LEAK SUMMARY:
    ==23665==    definitely lost: 48 bytes in 1 blocks
    ==23665==    indirectly lost: 84 bytes in 2 blocks
    ==23665==      possibly lost: 0 bytes in 0 blocks
    ==23665==    still reachable: 313,876 bytes in 84 blocks
    ==23665==         suppressed: 0 bytes in 0 blocks
    ==23665== Reachable blocks (those to which a pointer was found) are not shown.
    ==23665== To see them, rerun with: --leak-check=full --show-leak-kinds=all
    ==23665==
    ==23665== For counts of detected and suppressed errors, rerun with: -v
    ==23665== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

After this patch is applied the leak is gone as expected.

This is a very minor leak that can only be observed if deinit() is called,
shortly before the OS will free all memory of the process anyway. No
backport needed.

5 years agoMINOR: haproxy: Make use of deinit_and_exit() for clean exits
Tim Duesterhus [Sat, 13 Jun 2020 22:37:42 +0000 (00:37 +0200)] 
MINOR: haproxy: Make use of deinit_and_exit() for clean exits

Particularly cleanly deinit() after a configuration check to clean up the
output of valgrind which reports "possible losses" without a deinit() and
does not with a deinit(), converting actual losses into proper hard losses
which makes the whole stuff easier to analyze.

As an example, given an example configuration of the following:

    frontend foo
     bind *:8080
     mode http

Running `haproxy -c -f cfg` within valgrind will report 4 possible losses:

    $ valgrind --leak-check=full ./haproxy -c -f ./example.cfg
    ==21219== Memcheck, a memory error detector
    ==21219== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
    ==21219== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
    ==21219== Command: ./haproxy -c -f ./example.cfg
    ==21219==
    [WARNING] 165/001100 (21219) : config : missing timeouts for frontend 'foo'.
       | While not properly invalid, you will certainly encounter various problems
       | with such a configuration. To fix this, please ensure that all following
       | timeouts are set to a non-zero value: 'client', 'connect', 'server'.
    Warnings were found.
    Configuration file is valid
    ==21219==
    ==21219== HEAP SUMMARY:
    ==21219==     in use at exit: 1,436,631 bytes in 130 blocks
    ==21219==   total heap usage: 153 allocs, 23 frees, 1,447,758 bytes allocated
    ==21219==
    ==21219== 7 bytes in 1 blocks are possibly lost in loss record 5 of 54
    ==21219==    at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
    ==21219==    by 0x5726489: strdup (strdup.c:42)
    ==21219==    by 0x468FD9: bind_conf_alloc (listener.h:158)
    ==21219==    by 0x468FD9: cfg_parse_listen (cfgparse-listen.c:557)
    ==21219==    by 0x459DF3: readcfgfile (cfgparse.c:2167)
    ==21219==    by 0x5056CD: init (haproxy.c:2021)
    ==21219==    by 0x418232: main (haproxy.c:3121)
    ==21219==
    ==21219== 14 bytes in 1 blocks are possibly lost in loss record 9 of 54
    ==21219==    at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
    ==21219==    by 0x5726489: strdup (strdup.c:42)
    ==21219==    by 0x468F9B: bind_conf_alloc (listener.h:154)
    ==21219==    by 0x468F9B: cfg_parse_listen (cfgparse-listen.c:557)
    ==21219==    by 0x459DF3: readcfgfile (cfgparse.c:2167)
    ==21219==    by 0x5056CD: init (haproxy.c:2021)
    ==21219==    by 0x418232: main (haproxy.c:3121)
    ==21219==
    ==21219== 128 bytes in 1 blocks are possibly lost in loss record 35 of 54
    ==21219==    at 0x4C2FB55: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
    ==21219==    by 0x468F90: bind_conf_alloc (listener.h:152)
    ==21219==    by 0x468F90: cfg_parse_listen (cfgparse-listen.c:557)
    ==21219==    by 0x459DF3: readcfgfile (cfgparse.c:2167)
    ==21219==    by 0x5056CD: init (haproxy.c:2021)
    ==21219==    by 0x418232: main (haproxy.c:3121)
    ==21219==
    ==21219== 608 bytes in 1 blocks are possibly lost in loss record 46 of 54
    ==21219==    at 0x4C2FB55: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
    ==21219==    by 0x4B953A: create_listeners (listener.c:576)
    ==21219==    by 0x4578F6: str2listener (cfgparse.c:192)
    ==21219==    by 0x469039: cfg_parse_listen (cfgparse-listen.c:568)
    ==21219==    by 0x459DF3: readcfgfile (cfgparse.c:2167)
    ==21219==    by 0x5056CD: init (haproxy.c:2021)
    ==21219==    by 0x418232: main (haproxy.c:3121)
    ==21219==
    ==21219== LEAK SUMMARY:
    ==21219==    definitely lost: 0 bytes in 0 blocks
    ==21219==    indirectly lost: 0 bytes in 0 blocks
    ==21219==      possibly lost: 757 bytes in 4 blocks
    ==21219==    still reachable: 1,435,874 bytes in 126 blocks
    ==21219==         suppressed: 0 bytes in 0 blocks
    ==21219== Reachable blocks (those to which a pointer was found) are not shown.
    ==21219== To see them, rerun with: --leak-check=full --show-leak-kinds=all
    ==21219==
    ==21219== For counts of detected and suppressed errors, rerun with: -v
    ==21219== ERROR SUMMARY: 4 errors from 4 contexts (suppressed: 0 from 0)

Re-running the same command with the patch applied will not report any
losses any more:

    $ valgrind --leak-check=full ./haproxy -c -f ./example.cfg
    ==22124== Memcheck, a memory error detector
    ==22124== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
    ==22124== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
    ==22124== Command: ./haproxy -c -f ./example.cfg
    ==22124==
    [WARNING] 165/001503 (22124) : config : missing timeouts for frontend 'foo'.
       | While not properly invalid, you will certainly encounter various problems
       | with such a configuration. To fix this, please ensure that all following
       | timeouts are set to a non-zero value: 'client', 'connect', 'server'.
    Warnings were found.
    Configuration file is valid
    ==22124==
    ==22124== HEAP SUMMARY:
    ==22124==     in use at exit: 313,864 bytes in 82 blocks
    ==22124==   total heap usage: 153 allocs, 71 frees, 1,447,758 bytes allocated
    ==22124==
    ==22124== LEAK SUMMARY:
    ==22124==    definitely lost: 0 bytes in 0 blocks
    ==22124==    indirectly lost: 0 bytes in 0 blocks
    ==22124==      possibly lost: 0 bytes in 0 blocks
    ==22124==    still reachable: 313,864 bytes in 82 blocks
    ==22124==         suppressed: 0 bytes in 0 blocks
    ==22124== Reachable blocks (those to which a pointer was found) are not shown.
    ==22124== To see them, rerun with: --leak-check=full --show-leak-kinds=all
    ==22124==
    ==22124== For counts of detected and suppressed errors, rerun with: -v
    ==22124== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

It might be worth investigating what exactly HAProxy does to lose pointers
to the start of those 4 memory areas and then to be able to still free them
during deinit(). If HAProxy is able to free them, they ideally should be
"still reachable" and not "possibly lost".

5 years agoMINOR: haproxy: Add void deinit_and_exit(int)
Tim Duesterhus [Sat, 13 Jun 2020 22:37:41 +0000 (00:37 +0200)] 
MINOR: haproxy: Add void deinit_and_exit(int)

This helper function calls deinit() and then exit() with the given status.

5 years agoCI: travis-ci: use "-O1" for clang builds
Ilya Shipitsin [Sat, 13 Jun 2020 20:08:37 +0000 (01:08 +0500)] 
CI: travis-ci: use "-O1" for clang builds

it turned out that clang asan fails with -O2 option

better solution yet to be found

ML: https://www.mail-archive.com/haproxy@formilux.org/msg37621.html

5 years agoBUG/MEDIUM: checks: Fix off-by-one in allocation of SMTP greeting cmd
Tim Duesterhus [Fri, 12 Jun 2020 13:58:48 +0000 (15:58 +0200)] 
BUG/MEDIUM: checks: Fix off-by-one in allocation of SMTP greeting cmd

The allocation did not account for either the trailing null byte or the
space, leading to a buffer overwrite.

This bug was detected by an assertion failure in the allocator. But can
also be easily detected using valgrind:

    ==25827== Invalid write of size 1
    ==25827==    at 0x6529759: __vsprintf_chk (vsprintf_chk.c:84)
    ==25827==    by 0x65296AC: __sprintf_chk (sprintf_chk.c:31)
    ==25827==    by 0x4D6AB7: sprintf (stdio2.h:33)
    ==25827==    by 0x4D6AB7: proxy_parse_smtpchk_opt (check.c:1799)
    ==25827==    by 0x4A7DDD: cfg_parse_listen (cfgparse-listen.c:2269)
    ==25827==    by 0x494AD3: readcfgfile (cfgparse.c:2167)
    ==25827==    by 0x542995: init (haproxy.c:2021)
    ==25827==    by 0x421DD2: main (haproxy.c:3121)
    ==25827==  Address 0x78712a8 is 0 bytes after a block of size 24 alloc'd
    ==25827==    at 0x4C2FB55: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
    ==25827==    by 0x4D6A8C: proxy_parse_smtpchk_opt (check.c:1797)
    ==25827==    by 0x4A7DDD: cfg_parse_listen (cfgparse-listen.c:2269)
    ==25827==    by 0x494AD3: readcfgfile (cfgparse.c:2167)
    ==25827==    by 0x542995: init (haproxy.c:2021)
    ==25827==    by 0x421DD2: main (haproxy.c:3121)

This patch fixes issue #681.

This bug was introduced in commit fbcc77c6baa7edee85be9c2384d12c55ef651a5a,
which first appeared in 2.2-dev7. No backport needed.

5 years agoBUILD: proto_uxst: shut up yet another gcc's absurd warning
Willy Tarreau [Fri, 12 Jun 2020 13:58:19 +0000 (15:58 +0200)] 
BUILD: proto_uxst: shut up yet another gcc's absurd warning

When building with gcc-8 -fsanitize=address, we get this warning once on
an strncpy() call in proto_uxst.c:

  src/proto_uxst.c:262:3: warning: 'strncpy' output may be truncated copying 107 bytes from a string of length 4095 [-Wstringop-truncation]
     strncpy(addr.sun_path, tempname, sizeof(addr.sun_path) - 1);
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

It happens despite the test on snprintf() at the top (since gcc's string
handling is totally empiric), and requires the strlen() test to be placed
"very close" to the strncpy() call (with "very close" yet to be determined).
There's no other way to shut this one except disabling it. Given there's
only one instance of this warning and the cost of dealing with it in the
code is not huge, let's decorate the code to make gcc happily believe it
is smart since it seems to have a mind of itself.

5 years agoBUILD: thread: add parenthesis around values of locking macros
Willy Tarreau [Fri, 12 Jun 2020 09:42:25 +0000 (11:42 +0200)] 
BUILD: thread: add parenthesis around values of locking macros

clang just failed on fd.c with this error:

  src/fd.c:491:9: error: logical not is only applied to the left hand side of this comparison [-Werror,-Wlogical-not-parentheses]
          while (HA_SPIN_TRYLOCK(OTHER_LOCK, &log_lock) != 0) {
                 ^                                      ~~
That's because this expands to this:

          while (!pl_try_s(&log_lock) != 0) {

Let's just add parenthesis in the TRYLOCK macros to avoid this.
This may need to be backported if commit df187875d ("BUG/MEDIUM: log:
don't hold the log lock during writev() on a file descriptor") is
backported as well as it seems to be the first one to trigger it.

5 years agoBUG/MINOR: ssl: fix ssl-{min,max}-ver with openssl < 1.1.0
William Lallemand [Thu, 11 Jun 2020 15:34:00 +0000 (17:34 +0200)] 
BUG/MINOR: ssl: fix ssl-{min,max}-ver with openssl < 1.1.0

In bug #676, it was reported that ssl-min-ver SSLv3 does not work in
Amazon environments with OpenSSL 1.0.2.

The reason for this is a patch of Amazon OpenSSL which sets
SSL_OP_NO_SSLv3 in SSL_CTX_new(). Which is kind of a problem with our
implementation of ssl-{min,max}-ver in old openSSL versions, because it
does not try to clear existing version flags.

This patch fixes the bug by cleaning versions flags known by HAProxy in
the SSL_CTX before applying the right ones.

Should be backported as far as 1.8.

5 years agoBUILD: Re-enable -Wimplicit-fallthrough
Tim Duesterhus [Fri, 29 May 2020 12:35:51 +0000 (14:35 +0200)] 
BUILD: Re-enable -Wimplicit-fallthrough

Getting rid of this warning is cleaner solved using a 'fall through' comment,
because it clarifies intent to a human reader.

This patch adjust a few places that cause -Wimplicit-fallthrough to trigger:

- Fix typos in the comment.
- Remove redundant 'no break' that trips up gcc from comment.
- Move the comment out of the block when the 'case' is completely surrounded
  by braces.
- Add comments where I could determine that the fall through was intentional.

Changes tested on

    gcc (Debian 9.3.0-13) 9.3.0
    Copyright (C) 2019 Free Software Foundation, Inc.
    This is free software; see the source for copying conditions.  There is NO
    warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

using

    make -j4 all TARGET=linux-glibc USE_OPENSSL=1 USE_LUA=1 USE_ZLIB=1 USE_PCRE2=1 USE_PCRE2_JIT=1 USE_GETADDRINFO=1

5 years agoBUG/MEDIUM: pattern: fix thread safety of pattern matching
Willy Tarreau [Thu, 11 Jun 2020 14:37:35 +0000 (16:37 +0200)] 
BUG/MEDIUM: pattern: fix thread safety of pattern matching

Commit b5997f740 ("MAJOR: threads/map: Make acls/maps thread safe")
introduced a subtle bug in the pattern matching code. In order to cope
with the possibility that another thread might be modifying the pattern's
sample_data while it's being used, we return a thread-local static
sample_data which is a copy of the one found in the matched pattern. The
copy is performed depending on the sample_data's type. But the switch
statement misses some breaks and doesn't set the new sample_data pointer
at the right place, resulting in the original sample_data being restored
at the end before returning.

The net effect overall is that the correct sample_data is returned (hence
functionally speaking the matching works fine) but it's not thread-safe
so any del_map() or set_map() action could modify the pattern on one
thread while it's being used on another one. It doesn't seem likely to
cause a crash but could result in corrupted data appearing where the
value is consumed (e.g. when appended in a header or when logged) or an
ACL occasionally not matching after a map lookup.

This fix should be backported as far as 1.8.

Thanks to Tim for reporting it and to Emeric for the analysis.

5 years agoBUILD: Remove nowarn for warnings that do not trigger
Tim Duesterhus [Fri, 29 May 2020 12:35:50 +0000 (14:35 +0200)] 
BUILD: Remove nowarn for warnings that do not trigger

Tested with

    make -j4 all TARGET=linux-glibc USE_OPENSSL=1 USE_LUA=1 USE_ZLIB=1 USE_PCRE2=1 USE_PCRE2_JIT=1 USE_GETADDRINFO=1

against

    gcc (Debian 9.3.0-13) 9.3.0
    Copyright (C) 2019 Free Software Foundation, Inc.
    This is free software; see the source for copying conditions.  There is NO
    warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

5 years agoBUG/MEDIUM: log: don't hold the log lock during writev() on a file descriptor
Willy Tarreau [Thu, 11 Jun 2020 12:25:47 +0000 (14:25 +0200)] 
BUG/MEDIUM: log: don't hold the log lock during writev() on a file descriptor

In issue #648 a second problem was reported, indicating that some users
mistakenly send the log to an FD mapped on a file. This situation doesn't
even enable O_NONBLOCK and results in huge access times in the order of
milliseconds with the lock held and other threads waiting till the
watchdog fires to unblock the situation.

The problem with files is that O_NONBLOCK is ignored, and we still need
to lock otherwise we can end up with interleaved log messages.

What this patch does is different. Instead of locking all writers, it
uses a trylock so that there's always at most one logger and that other
candidates can simply give up and report a failure, just as would happen
if writev() returned -1 due to a pipe full condition. This solution is
elegant because it gives back the control to haproxy to decide to give
up when it takes too much time, while previously it was the kernel that
used to block the syscall.

However at high log rates (500000 req/s) there was up to 50% dropped logs
due to the contention on the lock. In order to address this, we try to
grab the lock up to 200 times and call ha_thread_relax() on failure. This
results in almost no failure (no more than previously with O_NONBLOCK). A
typical test with 6 competing threads writing to stdout chained to a pipe
to a single process shows around 1000 drops for 10 million logs at 480000
lines per second.

Please note that this doesn't mean that writing to a blocking FD is a good
idea, and it might only be temporarily done on testing environments for
debugging. A file or a terminal will continue to block the writing thread
while others spin a little bit and lose their logs, but the writing thread
will still experience performance-killing latencies.

This patch should be backported to 2.1 and 2.0. The code is in log.c in
2.0, but the principle is the same.

5 years agoBUILD: include: add sys/types before netinet/tcp.h
Willy Tarreau [Thu, 11 Jun 2020 09:22:44 +0000 (11:22 +0200)] 
BUILD: include: add sys/types before netinet/tcp.h

Apparently Cygwin requires sys/types.h before netinet/tcp.h but doesn't
include it by itself, as shown here:
  https://github.com/haproxy/haproxy/actions/runs/131943890

This patch makes sure it's always present, which is in server.c and
the SPOA example.

5 years ago[RELEASE] Released version 2.2-dev9 v2.2-dev9
Willy Tarreau [Thu, 11 Jun 2020 08:22:10 +0000 (10:22 +0200)] 
[RELEASE] Released version 2.2-dev9

Released version 2.2-dev9 with the following main changes :
    - BUG/MINOR: http-htx: Don't forget to release the http reply in release function
    - BUG/MINOR: http-htx: Fix a leak on error path during http reply parsing
    - MINOR: checks: Remove dead code from process_chk_conn()
    - REGTESTS: checks: Fix tls_health_checks when IPv6 addresses are used
    - REGTESTS: Add missing OPENSSL to REQUIRE_OPTIONS for lua/txn_get_priv
    - MINOR: lua: Use vars_unset_by_name_ifexist()
    - CLEANUP: vars: Remove void vars_unset_by_name(const char*, size_t, struct sample*)
    - MINOR: vars: Make vars_(un|)set_by_name(_ifexist|) return a success value
    - MINOR: lua: Make `set_var()` and `unset_var()` return success
    - MEDIUM: lua: Add `ifexist` parameter to `set_var`
    - MEDIUM: ring: new section ring to declare custom ring buffers.
    - REGTESTS: Add missing OPENSSL to REQUIRE_OPTIONS for compression/lua_validation
    - REGTESTS: Require the version 2.2 to execute lua/set_var
    - BUG/MEDIUM: checks: Refresh the conn-stream and the connection after a connect
    - MINOR: checks: Remove useless tests on the connection and conn-stream
    - BUG/MEDIUM: contrib/spoa: do not register python3.8 if --embed fail
    - BUG/MEDIUM: connection: Ignore PP2 unique ID for stream-less connections
    - BUG/MINOR: connection: Always get the stream when available to send PP2 line
    - BUG/MEDIUM: backend: set the connection owner to the session when using alpn.
    - MINOR: pools: compute an estimate of each pool's average needed objects
    - MEDIUM: pools: directly free objects when pools are too much crowded
    - REGTEST: Add connection/proxy_protocol_send_unique_id_alpn
    - MINOR: http-ana: Make the function http_reply_to_htx() public
    - MINOR: http-ana: Use proxy's error replies to emit 401/407 responses
    - MINOR: http-rules: Use an action function to eval http-request auth rules
    - CLEANUP: http: Remove unused HTTP message templates
    - BUG/MEDIUM: checks: Don't blindly subscribe for receive if waiting for connect
    - MINOR: checks: I/O callback function only rely on the data layer wake callback
    - BUG/MINOR: lua: Add missing string length for lua sticktable lookup
    - BUG/MEDIUM: logs: fix trailing zeros on log message.
    - CI: cirrus-ci: skip reg-tests/connection/proxy_protocol_send_unique_id_alpn.vtc on CentOS 6
    - BUG/MINOR: nameservers: fix error handling in parsing of resolv.conf
    - BUG/MEDIUM: checks: Don't add a tcpcheck ruleset twice in the shared tree
    - MEDIUM: ssl: use TLSv1.2 as the minimum default on bind lines
    - CLEANUP: pools: use the regular lock for the flush operation on lockless pools
    - SCRIPTS: publish-release: pass -n to gzip to remove timestamp
    - MINOR: ring: re-work ring attach generic API.
    - BUG/MINOR: error on unknown statement in ring section.
    - MEDIUM: ring: add server statement to forward messages from a ring
    - MEDIUM: ring: add new srv statement to support octet counting forward
    - MINOR: ssl: set ssl-min-ver in ambiguous configurations
    - CLEANUP: ssl: remove comment from dump_crtlist_sslconf()
    - BUILD: sink: address build warning on 32-bit architectures
    - BUG/MINOR: peers: fix internal/network key type mapping.
    - CLEANUP: regex: remove outdated support for regex actions
    - Revert "MINOR: ssl: rework add cert chain to CTX to be libssl independent"
    - MINOR: mux-h1/proxy: Add a proxy option to disable clear h2 upgrade
    - BUG/MEDIUM: lua: Reset analyse expiration timeout before executing a lua action
    - DOC: add a line about comments in crt-list
    - BUG/MEDIUM: hlua: Lock pattern references to perform set/add/del operations
    - BUG/MINOR: checks: Fix test on http-check rulesets during config validity check
    - BUG/MEDIUM: contrib/prometheus-exporter: Properly set flags to dump metrics
    - BUG/MEDIUM: mworker: fix the copy of options in copy_argv()
    - BUG/MINOR: init: -x can have a parameter starting with a dash
    - BUG/MINOR: init: -S can have a parameter starting with a dash
    - BUG/MEDIUM: mworker: fix the reload with an -- option
    - BUG/MINOR: ssl: fix a trash buffer leak in some error cases
    - BUG/MINOR: mworker: fix a memleak when execvp() failed
    - MINOR: sample: Add secure_memcmp converter
    - REORG: ebtree: move the C files from ebtree/ to src/
    - REORG: ebtree: move the include files from ebtree to include/import/
    - REORG: ebtree: clean up remains of the ebtree/ directory
    - REORG: include: create new file haproxy/api-t.h
    - REORG: include: create new file haproxy/api.h
    - REORG: include: update all files to use haproxy/api.h or api-t.h if needed
    - CLEANUP: include: remove common/config.h
    - CLEANUP: include: remove unused template.h
    - REORG: include: move MIN/MAX from tools.h to compat.h
    - REORG: include: move SWAP/MID_RANGE/MAX_RANGE from tools.h to standard.h
    - CLEANUP: include: remove unused common/tools.h
    - REORG: include: move the base files from common/ to haproxy/
    - REORG: include: move version.h to haproxy/
    - REORG: include: move base64.h, errors.h and hash.h from common to to haproxy/
    - REORG: include: move openssl-compat.h from common/ to haproxy/
    - REORG: include: move ist.h from common/ to import/
    - REORG: include: move the BUG_ON() code to haproxy/bug.h
    - REORG: include: move debug.h from common/ to haproxy/
    - CLEANUP: debug: drop unused function p_malloc()
    - REORG: include: split buf.h into haproxy/buf-t.h and haproxy/buf.h
    - REORG: include: move istbuf.h to haproxy/
    - REORG: include: split mini-clist into haproxy/list and list-t.h
    - REORG: threads: extract atomic ops from hathreads.h
    - CLEANUP: threads: remove a few needless includes of hathreads.h
    - REORG: include: split hathreads into haproxy/thread.h and haproxy/thread-t.h
    - CLEANUP: thread: rename __decl_hathreads() to __decl_thread()
    - REORG: include: move time.h from common/ to haproxy/
    - REORG: include: move integer manipulation functions from standard.h to intops.h
    - CLEANUP: include: remove excessive includes of common/standard.h
    - REORG: include: move freq_ctr to haproxy/
    - CLEANUP: pool: include freq_ctr.h and remove locally duplicated functions
    - REORG: memory: move the pool type definitions to haproxy/pool-t.h
    - REORG: memory: move the OS-level allocator to haproxy/pool-os.h
    - MINOR: memory: don't let __pool_get_first() pick from the cache
    - MEDIUM: memory: don't let pool_put_to_cache() free the objects itself
    - MINOR: memory: move pool-specific path of the locked pool_free() to __pool_free()
    - MEDIUM: memory: make local pools independent on lockless pools
    - REORG: include: move common/memory.h to haproxy/pool.h
    - REORG: include: move common/chunk.h to haproxy/chunk.h
    - REORG: include: move activity to haproxy/
    - REORG: include: move common/buffer.h to haproxy/dynbuf{,-t}.h
    - REORG: include: move common/net_helper.h to haproxy/net_helper.h
    - REORG: include: move common/namespace.h to haproxy/namespace{,-t}.h
    - REORG: include: split common/regex.h into haproxy/regex{,-t}.h
    - REORG: include: split common/xref.h into haproxy/xref{,-t}.h
    - REORG: include: move common/ticks.h to haproxy/ticks.h
    - REORG: include: split common/http.h into haproxy/http{,-t}.h
    - REORG: include: split common/http-hdr.h into haproxy/http-hdr{,-t}.h
    - REORG: include: move common/h1.h to haproxy/h1.h
    - REORG: include: split common/htx.h into haproxy/htx{,-t}.h
    - REORG: include: move hpack*.h to haproxy/ and split hpack-tbl
    - REORG: include: move common/h2.h to haproxy/h2.h
    - REORG: include: move common/fcgi.h to haproxy/
    - REORG: include: move protocol.h to haproxy/protocol{,-t}.h
    - REORG: tools: split common/standard.h into haproxy/tools{,-t}.h
    - REORG: include: move dict.h to hparoxy/dict{,-t}.h
    - REORG: include: move shctx to haproxy/shctx{,-t}.h
    - REORG: include: move port_range.h to haproxy/port_range{,-t}.h
    - REORG: include: move fd.h to haproxy/fd{,-t}.h
    - REORG: include: move ring to haproxy/ring{,-t}.h
    - REORG: include: move sink.h to haproxy/sink{,-t}.h
    - REORG: include: move pipe.h to haproxy/pipe{,-t}.h
    - CLEANUP: include: remove empty raw_sock.h
    - REORG: include: move proto_udp.h to haproxy/proto_udp{,-t}.h
    - REORG: include: move proto/proto_sockpair.h to haproxy/proto_sockpair.h
    - REORG: include: move compression.h to haproxy/compression{,-t}.h
    - REORG: include: move h1_htx.h to haproxy/h1_htx.h
    - REORG: include: move http_htx.h to haproxy/http_htx{,-t}.h
    - REORG: include: move hlua.h to haproxy/hlua{,-t}.h
    - REORG: include: move hlua_fcn.h to haproxy/hlua_fcn.h
    - REORG: include: move action.h to haproxy/action{,-t}.h
    - REORG: include: move arg.h to haproxy/arg{,-t}.h
    - REORG: include: move auth.h to haproxy/auth{,-t}.h
    - REORG: include: move dns.h to haproxy/dns{,-t}.h
    - REORG: include: move flt_http_comp.h to haproxy/
    - REORG: include: move counters.h to haproxy/counters-t.h
    - REORG: include: split mailers.h into haproxy/mailers{,-t}.h
    - REORG: include: move capture.h to haproxy/capture{,-t}.h
    - REORG: include: move frontend.h to haproxy/frontend.h
    - REORG: include: move obj_type.h to haproxy/obj_type{,-t}.h
    - REORG: include: move http_rules.h to haproxy/http_rules.h
    - CLEANUP: include: remove unused mux_pt.h
    - REORG: include: move mworker.h to haproxy/mworker{,-t}.h
    - REORG: include: move ssl_utils.h to haproxy/ssl_utils.h
    - REORG: include: move ssl_ckch.h to haproxy/ssl_ckch{,-t}.h
    - REORG: move ssl_crtlist.h to haproxy/ssl_crtlist{,-t}.h
    - REORG: include: move lb_chash.h to haproxy/lb_chash{,-t}.h
    - REORG: include: move lb_fas.h to haproxy/lb_fas{,-t}.h
    - REORG: include: move lb_fwlc.h to haproxy/lb_fwlc{,-t}.h
    - REORG: include: move lb_fwrr.h to haproxy/lb_fwrr{,-t}.h
    - REORG: include: move listener.h to haproxy/listener{,-t}.h
    - REORG: include: move pattern.h to haproxy/pattern{,-t}.h
    - REORG: include: move map to haproxy/map{,-t}.h
    - REORG: include: move payload.h to haproxy/payload.h
    - REORG: include: move sample.h to haproxy/sample{,-t}.h
    - REORG: include: move protocol_buffers.h to haproxy/protobuf{,-t}.h
    - REORG: include: move vars.h to haproxy/vars{,-t}.h
    - REORG: include: split global.h into haproxy/global{,-t}.h
    - REORG: include: move task.h to haproxy/task{,-t}.h
    - REORG: include: move proto_tcp.h to haproxy/proto_tcp.h
    - REORG: include: move signal.h to haproxy/signal{,-t}.h
    - REORG: include: move tcp_rules.h to haproxy/tcp_rules.h
    - REORG: include: move connection.h to haproxy/connection{,-t}.h
    - REORG: include: move checks.h to haproxy/check{,-t}.h
    - REORG: include: move http_fetch.h to haproxy/http_fetch.h
    - REORG: include: move peers.h to haproxy/peers{,-t}.h
    - REORG: include: move stick_table.h to haproxy/stick_table{,-t}.h
    - REORG: include: move session.h to haproxy/session{,-t}.h
    - REORG: include: move trace.h to haproxy/trace{,-t}.h
    - REORG: include: move acl.h to haproxy/acl.h{,-t}.h
    - REORG: include: split common/uri_auth.h into haproxy/uri_auth{,-t}.h
    - REORG: move applet.h to haproxy/applet{,-t}.h
    - REORG: include: move stats.h to haproxy/stats{,-t}.h
    - REORG: include: move cli.h to haproxy/cli{,-t}.h
    - REORG: include: move lb_map.h to haproxy/lb_map{,-t}.h
    - REORG: include: move ssl_sock.h to haproxy/ssl_sock{,-t}.h
    - REORG: include: move stream_interface.h to haproxy/stream_interface{,-t}.h
    - REORG: include: move channel.h to haproxy/channel{,-t}.h
    - REORG: include: move http_ana.h to haproxy/http_ana{,-t}.h
    - REORG: include: move filters.h to haproxy/filters{,-t}.h
    - REORG: include: move fcgi-app.h to haproxy/fcgi-app{,-t}.h
    - REORG: include: move log.h to haproxy/log{,-t}.h
    - REORG: include: move proxy.h to haproxy/proxy{,-t}.h
    - REORG: include: move spoe.h to haproxy/spoe{,-t}.h
    - REORG: include: move backend.h to haproxy/backend{,-t}.h
    - REORG: include: move queue.h to haproxy/queue{,-t}.h
    - REORG: include: move server.h to haproxy/server{,-t}.h
    - REORG: include: move stream.h to haproxy/stream{,-t}.h
    - REORG: include: move cfgparse.h to haproxy/cfgparse.h
    - CLEANUP: hpack: export debug functions and move inlines to .h
    - REORG: check: move the e-mail alerting code to mailers.c
    - REORG: check: move tcpchecks away from check.c
    - REORG: check: move email_alert* from proxy-t.h to mailers-t.h
    - REORG: check: extract the external checks from check.{c,h}
    - CLEANUP: include: don't include stddef.h directly
    - CLEANUP: include: don't include proxy-t.h in global-t.h
    - CLEANUP: include: move sample_data out of sample-t.h
    - REORG: include: move the error reporting functions to from log.h to errors.h
    - BUILD: reorder objects in the Makefile for faster builds
    - CLEANUP: compiler: add a THREAD_ALIGNED macro and use it where appropriate
    - CLEANUP: include: make atomic.h part of the base API
    - REORG: include: move MAX_THREADS to defaults.h
    - REORG: include: move THREAD_LOCAL and __decl_thread() to compiler.h
    - CLEANUP: include: tree-wide alphabetical sort of include files
    - REORG: include: make list-t.h part of the base API
    - REORG: dgram: rename proto_udp to dgram

5 years agoREORG: dgram: rename proto_udp to dgram
Willy Tarreau [Thu, 11 Jun 2020 07:23:02 +0000 (09:23 +0200)] 
REORG: dgram: rename proto_udp to dgram

The set of files proto_udp.{c,h} were misleadingly named, as they do not
provide anything related to the UDP protocol but to datagram handling
instead, since currently all UDP processing is hard-coded where it's used
(dns, logs). They are to UDP what connection.{c,h} are to proto_tcp. This
was causing confusion about how to insert UDP socket management code,
so let's rename them right now to dgram.{c,h} which more accurately
matches what's inside since every function and type is already prefixed
with "dgram_".

5 years agoREORG: include: make list-t.h part of the base API
Willy Tarreau [Thu, 11 Jun 2020 07:14:49 +0000 (09:14 +0200)] 
REORG: include: make list-t.h part of the base API

There are list definitions everywhere in the code, let's drop the need
for including list-t.h to declare them. The rest of the list manipulation
is huge however and not needed everywhere so using the list walking macros
still requires to include list.h.

5 years agoCLEANUP: include: tree-wide alphabetical sort of include files
Willy Tarreau [Tue, 9 Jun 2020 07:07:15 +0000 (09:07 +0200)] 
CLEANUP: include: tree-wide alphabetical sort of include files

This patch fixes all the leftovers from the include cleanup campaign. There
were not that many (~400 entries in ~150 files) but it was definitely worth
doing it as it revealed a few duplicates.

5 years agoREORG: include: move THREAD_LOCAL and __decl_thread() to compiler.h
Willy Tarreau [Thu, 11 Jun 2020 06:33:02 +0000 (08:33 +0200)] 
REORG: include: move THREAD_LOCAL and __decl_thread() to compiler.h

Since these are used as type attributes or conditional clauses, they
are used about everywhere and should not require a dependency on
thread.h. Moving them to compiler.h along with other similar statements
like ALIGN() etc looks more logical; this way they become part of the
base API. This allowed to remove thread-t.h from ~12 files, one was
found to only require thread-t and not thread and dict.c was found to
require thread.h.

5 years agoREORG: include: move MAX_THREADS to defaults.h
Willy Tarreau [Thu, 11 Jun 2020 06:14:01 +0000 (08:14 +0200)] 
REORG: include: move MAX_THREADS to defaults.h

That's already where MAX_PROCS is set, and we already handle the case of
the default value so there is no reason for placing it in thread.h given
that most call places don't need the rest of the threads definitions. The
include was removed from global-t.h and activity.c.

5 years agoCLEANUP: include: make atomic.h part of the base API
Willy Tarreau [Thu, 11 Jun 2020 05:58:05 +0000 (07:58 +0200)] 
CLEANUP: include: make atomic.h part of the base API

Atomic ops are used about everywhere, let's make them part of the base
API by including atomic.h in api.h.

5 years agoCLEANUP: compiler: add a THREAD_ALIGNED macro and use it where appropriate
Willy Tarreau [Thu, 11 Jun 2020 06:22:01 +0000 (08:22 +0200)] 
CLEANUP: compiler: add a THREAD_ALIGNED macro and use it where appropriate

Sometimes we need to align a struct member or a struct's size only when
threads are enabled. This is the case on fdtab for example. Instead of
using ugly ifdefs in the code itself, let's have a THREAD_ALIGNED() macro
performing the alignment only when threads are enabled. For now this was
only applied to fd-t.h as it was the only place found.

5 years agoBUILD: reorder objects in the Makefile for faster builds
Willy Tarreau [Fri, 5 Jun 2020 18:04:36 +0000 (20:04 +0200)] 
BUILD: reorder objects in the Makefile for faster builds

Splitting large files and changing includes has changed the per-file
build time. After a careful reordering based on build time, we're now
down to 5.8s at -O0 on the PC at -j8 and 2.4-2.6s on the farm at -j120.
Some room for at least one file name was left on each line to ease
future additions.

5 years agoREORG: include: move the error reporting functions to from log.h to errors.h
Willy Tarreau [Fri, 5 Jun 2020 15:27:29 +0000 (17:27 +0200)] 
REORG: include: move the error reporting functions to from log.h to errors.h

Most of the files dealing with error reports have to include log.h in order
to access ha_alert(), ha_warning() etc. But while these functions don't
depend on anything, log.h depends on a lot of stuff because it deals with
log-formats and samples. As a result it's impossible not to embark long
dependencies when using ha_warning() or qfprintf().

This patch moves these low-level functions to errors.h, which already
defines the error codes used at the same places. About half of the users
of log.h could be adjusted, sometimes revealing other issues such as
missing tools.h. Interestingly the total preprocessed size shrunk by
4%.

5 years agoCLEANUP: include: move sample_data out of sample-t.h
Willy Tarreau [Fri, 5 Jun 2020 14:54:16 +0000 (16:54 +0200)] 
CLEANUP: include: move sample_data out of sample-t.h

The struct sample_data is used by pattern, map and vars, and currently
requires to include sample-t which comes with many other dependencies.
Let's move sample_data into its own file to shorten the dependency tree.
This revealed a number of issues in adjacent files which were hidden by
the fact that sample-t.h brought everything that was missing.

5 years agoCLEANUP: include: don't include proxy-t.h in global-t.h
Willy Tarreau [Fri, 5 Jun 2020 14:25:48 +0000 (16:25 +0200)] 
CLEANUP: include: don't include proxy-t.h in global-t.h

We only need a forward declaration here to avoid embarking lots of
files, and by just doing this we reduce the build size by 3.5%.

5 years agoCLEANUP: include: don't include stddef.h directly
Willy Tarreau [Fri, 5 Jun 2020 13:37:34 +0000 (15:37 +0200)] 
CLEANUP: include: don't include stddef.h directly

Directly including stddef.h in many files results in it being processed
multiple times while it can be centralized in api-t.h and be guarded
against multiple inclusions. Doing so reduces the number of preprocessed
lines by 1200!

5 years agoREORG: check: extract the external checks from check.{c,h}
Willy Tarreau [Fri, 5 Jun 2020 13:31:31 +0000 (15:31 +0200)] 
REORG: check: extract the external checks from check.{c,h}

The health check code is ugly enough, let's take the external checks
out of it to simplify the code and shrink the file a little bit.

5 years agoREORG: check: move email_alert* from proxy-t.h to mailers-t.h
Willy Tarreau [Fri, 5 Jun 2020 12:55:20 +0000 (14:55 +0200)] 
REORG: check: move email_alert* from proxy-t.h to mailers-t.h

These ones are specific to mailers and have nothing to do in proxy-t.h.

5 years agoREORG: check: move tcpchecks away from check.c
Willy Tarreau [Fri, 5 Jun 2020 10:25:38 +0000 (12:25 +0200)] 
REORG: check: move tcpchecks away from check.c

Checks.c remains one of the largest file of the project and it contains
too many things. The tcpchecks code represents half of this file, and
both parts are relatively isolated, so let's move it away into its own
file. We now have tcpcheck.c, tcpcheck{,-t}.h.

Doing so required to export quite a number of functions because check.c
has almost everything made static, which really doesn't help to split!

5 years agoREORG: check: move the e-mail alerting code to mailers.c
Willy Tarreau [Fri, 5 Jun 2020 09:40:38 +0000 (11:40 +0200)] 
REORG: check: move the e-mail alerting code to mailers.c

check.c is one of the largest file and contains too many things. The
e-mail alerting code is stored there while nothing is in mailers.c.
Let's move this code out. That's only 4% of the code but a good start.
In order to do so, a few tcp-check functions had to be exported.

5 years agoCLEANUP: hpack: export debug functions and move inlines to .h
Willy Tarreau [Fri, 5 Jun 2020 07:05:31 +0000 (09:05 +0200)] 
CLEANUP: hpack: export debug functions and move inlines to .h

When building contrib/hpack there is a warning about an unused static
function. Actually it makes no sense to make it static, instead it must
be regularly exported. Similarly there is hpack_dht_get_tail() which is
inlined in the C file and which would make more sense with all other ones
in the H file.

5 years agoREORG: include: move cfgparse.h to haproxy/cfgparse.h
Willy Tarreau [Thu, 4 Jun 2020 22:00:29 +0000 (00:00 +0200)] 
REORG: include: move cfgparse.h to haproxy/cfgparse.h

There's no point splitting the file in two since only cfgparse uses the
types defined there. A few call places were updated and cleaned up. All
of them were in C files which register keywords.

There is nothing left in common/ now so this directory must not be used
anymore.

5 years agoREORG: include: move stream.h to haproxy/stream{,-t}.h
Willy Tarreau [Thu, 4 Jun 2020 21:46:14 +0000 (23:46 +0200)] 
REORG: include: move stream.h to haproxy/stream{,-t}.h

This one was not easy because it was embarking many includes with it,
which other files would automatically find. At least global.h, arg.h
and tools.h were identified. 93 total locations were identified, 8
additional includes had to be added.

In the rare files where it was possible to finalize the sorting of
includes by adjusting only one or two extra lines, it was done. But
all files would need to be rechecked and cleaned up now.

It was the last set of files in types/ and proto/ and these directories
must not be reused anymore.

5 years agoREORG: include: move server.h to haproxy/server{,-t}.h
Willy Tarreau [Thu, 4 Jun 2020 21:20:13 +0000 (23:20 +0200)] 
REORG: include: move server.h to haproxy/server{,-t}.h

extern struct dict server_name_dict was moved from the type file to the
main file. A handful of inlined functions were moved at the bottom of
the file. Call places were updated to use server-t.h when relevant, or
to simply drop the entry when not needed.

5 years agoREORG: include: move queue.h to haproxy/queue{,-t}.h
Willy Tarreau [Thu, 4 Jun 2020 20:59:39 +0000 (22:59 +0200)] 
REORG: include: move queue.h to haproxy/queue{,-t}.h

Nothing outstanding here. A number of call places were not justified and
removed.

5 years agoREORG: include: move backend.h to haproxy/backend{,-t}.h
Willy Tarreau [Thu, 4 Jun 2020 20:50:02 +0000 (22:50 +0200)] 
REORG: include: move backend.h to haproxy/backend{,-t}.h

The files remained mostly unchanged since they were OK. However, half of
the users didn't need to include them, and about as many actually needed
to have it and used to find functions like srv_currently_usable() through
a long chain that broke when moving the file.

5 years agoREORG: include: move spoe.h to haproxy/spoe{,-t}.h
Willy Tarreau [Thu, 4 Jun 2020 20:35:49 +0000 (22:35 +0200)] 
REORG: include: move spoe.h to haproxy/spoe{,-t}.h

Only minor change was to make sure all defines were before the structs
in spoe-t.h, everything else went smoothly.

5 years agoREORG: include: move proxy.h to haproxy/proxy{,-t}.h
Willy Tarreau [Thu, 4 Jun 2020 20:29:18 +0000 (22:29 +0200)] 
REORG: include: move proxy.h to haproxy/proxy{,-t}.h

This one is particularly difficult to split because it provides all the
functions used to manipulate a proxy state and to retrieve names or IDs
for error reporting, and as such, it was included in 73 files (down to
68 after cleanup). It would deserve a small cleanup though the cut points
are not obvious at the moment given the number of structs involved in
the struct proxy itself.

5 years agoREORG: include: move log.h to haproxy/log{,-t}.h
Willy Tarreau [Thu, 4 Jun 2020 20:01:04 +0000 (22:01 +0200)] 
REORG: include: move log.h to haproxy/log{,-t}.h

The current state of the logging is a real mess. The main problem is
that almost all files include log.h just in order to have access to
the alert/warning functions like ha_alert() etc, and don't care about
logs. But log.h also deals with real logging as well as log-format and
depends on stream.h and various other things. As such it forces a few
heavy files like stream.h to be loaded early and to hide missing
dependencies depending where it's loaded. Among the missing ones is
syslog.h which was often automatically included resulting in no less
than 3 users missing it.

Among 76 users, only 5 could be removed, and probably 70 don't need the
full set of dependencies.

A good approach would consist in splitting that file in 3 parts:
  - one for error output ("errors" ?).
  - one for log_format processing
  - and one for actual logging.

5 years agoREORG: include: move fcgi-app.h to haproxy/fcgi-app{,-t}.h
Willy Tarreau [Thu, 4 Jun 2020 19:33:21 +0000 (21:33 +0200)] 
REORG: include: move fcgi-app.h to haproxy/fcgi-app{,-t}.h

Only arg-t.h was missing from the types to get arg_list.

5 years agoREORG: include: move filters.h to haproxy/filters{,-t}.h
Willy Tarreau [Thu, 4 Jun 2020 19:29:29 +0000 (21:29 +0200)] 
REORG: include: move filters.h to haproxy/filters{,-t}.h

Just a minor change, moved the macro definitions upwards. A few caller
files were updated since they didn't need to include it.

5 years agoREORG: include: move http_ana.h to haproxy/http_ana{,-t}.h
Willy Tarreau [Thu, 4 Jun 2020 19:21:03 +0000 (21:21 +0200)] 
REORG: include: move http_ana.h to haproxy/http_ana{,-t}.h

It was moved without any change, however many callers didn't need it at
all. This was a consequence of the split of proto_http.c into several
parts that resulted in many locations to still reference it.

5 years agoREORG: include: move channel.h to haproxy/channel{,-t}.h
Willy Tarreau [Thu, 4 Jun 2020 19:07:02 +0000 (21:07 +0200)] 
REORG: include: move channel.h to haproxy/channel{,-t}.h

The files were moved with no change. The callers were cleaned up a bit
and a few of them had channel.h removed since not needed.