]> git.ipfire.org Git - thirdparty/haproxy.git/log
thirdparty/haproxy.git
6 years agoBUILD: makefile: enable TFO on linux platforms
Willy Tarreau [Fri, 14 Jun 2019 14:57:42 +0000 (16:57 +0200)] 
BUILD: makefile: enable TFO on linux platforms

TCP Fast Open is supported on all supported Linux kernels and on all
kernels shipped in supported distros, except the older 2.6.32 that
comes with RHEL6. However the option is harmless, will not prevent
from building and smoothly falls back even if forcefully enabled, so
it makes sense to enable it by default. It's still possible to pass
"USE_TFO=" to force it disabled if really desired.

6 years agoBUILD: makefile: enable linux namespaces by default on linux
Willy Tarreau [Fri, 14 Jun 2019 14:54:51 +0000 (16:54 +0200)] 
BUILD: makefile: enable linux namespaces by default on linux

Oldest kernel found on a supported Linux distro (2.6.32 + backports on
RHEL6) supports network namespaces, so we have no reason not to enable
them by default on the linux-glibc target.

6 years agoBUILD: makefile: detect and reject recently removed linux targets
Willy Tarreau [Fri, 14 Jun 2019 14:44:49 +0000 (16:44 +0200)] 
BUILD: makefile: detect and reject recently removed linux targets

We've just removed old linux targets "linux22", "linux24", "linux24e",
"linux26" and "linux2628" and it's likely that many build scripts and
packages will still reference these. So let's have the makefile detect
these and reject with instructions instead of silently building with
incorrect options.

6 years agoDOC: update few references to the linux* targets and change them to linux-glibc
Willy Tarreau [Fri, 14 Jun 2019 16:40:48 +0000 (18:40 +0200)] 
DOC: update few references to the linux* targets and change them to linux-glibc

The INSTALL guide, the Lua doc and the Prometheus exporter's README all
used to reference "linux2628", "linux26" or even "linux". These were all
updated to consistently reflect "linux-glibc" instead. The default options
were updated there as well so that it should build cleanly on most distros.

6 years agoBUILD: travis-ci: switch to linux-glibc instead of linux2628
Willy Tarreau [Fri, 14 Jun 2019 16:32:34 +0000 (18:32 +0200)] 
BUILD: travis-ci: switch to linux-glibc instead of linux2628

The Linux target has changed and only linux-glibc is known now.

6 years agoBUILD: makefile: rename "linux2628" to "linux-glibc" and remove older targets
Willy Tarreau [Fri, 14 Jun 2019 14:32:09 +0000 (16:32 +0200)] 
BUILD: makefile: rename "linux2628" to "linux-glibc" and remove older targets

The linux targets have become more than confusing over time. We used to
have "linux2628" to match the features available in kernels 2.6.28 and
above, without consideration for the libc, and due to many new features
appearing later in kernels, some other options were added that are not
enabled by default in linux2628, so this target doesn't make any sense
anymore. The older ones (linux 2.2, linux 2.4, ...) do not make sense
either since these versions are not supported anymore. Let's clean things
up by creating a new "linux-glibc" target that matches what is available
by default on Linux kernels and glibc present on supported distros at the
time of release. Other libc implementation may use a custom or generic
target or be added later if needed.

All the older linux targets were removed.

6 years agoBUILD: makefile: further clarify the "help" output and list targets
Willy Tarreau [Fri, 14 Jun 2019 13:52:01 +0000 (15:52 +0200)] 
BUILD: makefile: further clarify the "help" output and list targets

When a target is not set we now also list the known ones. A minor
alignment issue in the output was also addressed.

6 years agoBUG/MEDIUM: mux-h2: properly account for the appended data in HTX
Willy Tarreau [Sat, 15 Jun 2019 09:34:41 +0000 (11:34 +0200)] 
BUG/MEDIUM: mux-h2: properly account for the appended data in HTX

When commit 0350b90e3 ("MEDIUM: htx: make htx_add_data() never defragment
the buffer") was introduced, it made htx_add_data() actually be able to
add less data than it was asked for, and the callers must use the returned
value to know how much was added. The H2 code used to rely on the frame
length instead of the return value. A version of the code doing this was
written but is obviously not the one that got merged, resulting in breaking
large uploads or downloads when HTX would have instead defragmented the
buffer because the HTX side sees less contents than what the H2 side sees.

This patch fixes this again. No backport is needed.

6 years agoBUG/MEDIUM: connections: Don't try to send early data if we have no mux.
Olivier Houchard [Fri, 14 Jun 2019 22:14:05 +0000 (00:14 +0200)] 
BUG/MEDIUM: connections: Don't try to send early data if we have no mux.

In connect_server(), if we don't yet have a mux, because we're choosing
one depending on the ALPN, don't attempt to send early data. We can't do
it because those data would depend on the mux, that will only be determined
by the handshake.

This should be backported to 1.9.

6 years agoBUG/MEDIUM: connections: Don't use ALPN to pick mux when in mode TCP.
Olivier Houchard [Fri, 14 Jun 2019 22:13:15 +0000 (00:13 +0200)] 
BUG/MEDIUM: connections: Don't use ALPN to pick mux when in mode TCP.

In connect_server(), don't wait until we negociate the ALPN to choose the
mux, the only mux we want to use is the mux_pt anyway.

This should be backported to 1.9.

6 years agoBUG/MEDIUM: mux-h2: fix early close with option abortonclose
Willy Tarreau [Sat, 15 Jun 2019 07:55:50 +0000 (09:55 +0200)] 
BUG/MEDIUM: mux-h2: fix early close with option abortonclose

Olivier found that commit 99ad1b3e8 ("MINOR: mux-h2: stop relying on
CS_FL_REOS") managed to break abortonclose again with H2. What happens
is that while the CS_FL_REOS flag was set on some transitions to the
HREM state, it's not set on all and is in fact only set when the low
level connection is closed. So making the replacement condition match
the HREM and ERROR states is not correct and causes completely correct
requests to send advertise an early close of the connection layer while
only the stream's input is closed.

In order to avoid this, we now properly split the checks for the CLOSED
state and for the closed connection. This way there is no risk to set
the EOS flag too early on the connection.

No backport is needed.

6 years agoBUG: tasks: fix bug introduced by latest scheduler cleanup
Willy Tarreau [Fri, 14 Jun 2019 16:05:54 +0000 (18:05 +0200)] 
BUG: tasks: fix bug introduced by latest scheduler cleanup

In commit 86eded6c6 ("CLEANUP: tasks: rename task_remove_from_tasklet_list()
to tasklet_remove_*") which consisted in removing the casts between tasks
and tasklet, I was a bit too fast to believe that we only saw tasklets in
this function since process_runnable_tasks() also uses it with tasks under
a cast. So removing the bookkeeping on task_list_size was not appropriate.
Bah, the joy of casts which hide the real thing...

This patch does two things at once to address this mess once for all:
  - it restores the decrement of task_list_size when it's a real task,
    but moves it to process_runnable_task() since it's the only place
    where it's allowed to call it with a task

  - it moves the increment there as well and renames
    task_insert_into_tasklet_list() to tasklet_insert_into_tasklet_list()
    of obvious consistency reasons.

This way the increment/decrement of task_list_size is made at the only
places where the cast is enforced, so it has less risks to be missed.
The comments on top of these functions were updated to reflect that they
are only supposed to be used with tasklets and that the caller is responsible
for keeping task_list_size up to date if it decides to enforce a task there.

Now we don't have to worry anymore about how these functions work outside
of the scheduler, which is better longterm-wise. Thanks to Christopher for
spotting this mistake.

No backport is needed.

6 years agoBUG/MINOR: mux-h1: Wake busy mux for I/O when message is fully sent
Christopher Faulet [Fri, 14 Jun 2019 14:54:15 +0000 (16:54 +0200)] 
BUG/MINOR: mux-h1: Wake busy mux for I/O when message is fully sent

If a mux is in busy mode when the outgoing EOM is consummed, it is important to
wake it up for I/O. Because in busy mode, the mux is not subscribed for
receive. Otherwise, it depends on the applicative layer to shutdown the H1
stream. Wake it up allows the mux to catch the read0 as soon as possible.

This patch must be backported to 1.9.

6 years agoBUILD: makefile: clarify the "help" output and list options
Willy Tarreau [Fri, 14 Jun 2019 13:52:01 +0000 (15:52 +0200)] 
BUILD: makefile: clarify the "help" output and list options

The list of enable and disabled build options now appears separately
at the end of "make help". This is convenient to know what is enabled
by default on a given target. For example :

  $ make help TARGET=linux2628
  Enabled features for TARGET 'linux2628' (disable with 'USE_xxx=') :
    EPOLL NETFILTER POLL THREAD TPROXY LINUX_TPROXY LINUX_SPLICE LIBCRYPT
    CRYPT_H FUTEX ACCEPT4 CPU_AFFINITY DL RT PRCTL THREAD_DUMP

  Disabled features for TARGET 'linux2628' (enable with 'USE_xxx=1') :
    KQUEUE MY_EPOLL MY_SPLICE PCRE PCRE_JIT PCRE2 PCRE2_JIT PRIVATE_CACHE
    PTHREAD_PSHARED REGPARM STATIC_PCRE STATIC_PCRE2 VSYSCALL GETADDRINFO
    OPENSSL LUA MY_ACCEPT4 ZLIB SLZ TFO NS DEVICEATLAS 51DEGREES WURFL
    SYSTEMD OBSOLETE_LINKER EVPORTS

6 years agoDOC: add some environment variables in section 2.3
William Lallemand [Fri, 14 Jun 2019 13:35:37 +0000 (15:35 +0200)] 
DOC: add some environment variables in section 2.3

Add the missing environment variable in the 2.3 section.

6 years agoBUG/MEDIUM: connections: Don't call shutdown() if we want to disable linger.
Olivier Houchard [Fri, 14 Jun 2019 13:26:06 +0000 (15:26 +0200)] 
BUG/MEDIUM: connections: Don't call shutdown() if we want to disable linger.

In conn_sock_shutw(), avoid calling shutdown() if linger_risk is set. Not
doing so will result in getting sockets in TIME_WAIT for some time.
This is particularly observable with health checks.

This should be backported to 1.9.

6 years agoCLEANUP: tasks: rename task_remove_from_tasklet_list() to tasklet_remove_*
Willy Tarreau [Fri, 14 Jun 2019 12:47:49 +0000 (14:47 +0200)] 
CLEANUP: tasks: rename task_remove_from_tasklet_list() to tasklet_remove_*

The function really only operates on tasklets, its arguments are always
tasklets cast as tasks to match the function's type, to be cast back to
a struct tasklet. Let's rename it to tasklet_remove_from_tasklet_list(),
take a struct tasklet, and get rid of the undesired task casts.

6 years agoCLEANUP: connection: rename the wait_event.task field to .tasklet
Willy Tarreau [Fri, 14 Jun 2019 12:42:29 +0000 (14:42 +0200)] 
CLEANUP: connection: rename the wait_event.task field to .tasklet

It's really confusing to call it a task because it's a tasklet and used
in places where tasks and tasklets are used together. Let's rename it
to tasklet to remove this confusion.

6 years agoMEDIUM: server: server-state only rely on server name
Baptiste Assmann [Tue, 11 Jun 2019 12:51:49 +0000 (14:51 +0200)] 
MEDIUM: server: server-state only rely on server name

Since h7da71293e431b5ebb3d6289a55b0102331788ee6as has been added, the
server name (srv->id in the code) is now unique per backend, which
means it can reliabely be used to identify a server recovered from the
server-state file.

This patch cleans up the parsing of server-state file and ensure we use
only the server name as a reliable key.

6 years agoMINOR: mux-h2: Forward clients scheme to servers checking start-line flags
Christopher Faulet [Fri, 14 Jun 2019 08:46:51 +0000 (10:46 +0200)] 
MINOR: mux-h2: Forward clients scheme to servers checking start-line flags

By default, the scheme "https" is always used. But when an explicit scheme was
defined and when this scheme is "http", we use it in the request sent to the
server. This is done by checking flags of the start-line. If the flag
HTX_SL_F_HAS_SCHM is set, it means an explicit scheme was defined on the client
side. And if the flag HTX_SL_F_SCHM_HTTP is set, it means the scheme "http" was
used.

6 years agoMINOR: mux-h1: Set flags about the request's scheme on the start-line
Christopher Faulet [Fri, 14 Jun 2019 08:31:25 +0000 (10:31 +0200)] 
MINOR: mux-h1: Set flags about the request's scheme on the start-line

We first try to figure out if the URI of the start-line is absolute or not. So,
if it does not start by a slash ("/"), it means the URI is an absolute one and
the flag HTX_SL_F_HAS_SCHM is set. Then checks are performed to know if the
scheme is "http" or "https" and the corresponding flag is set,
HTX_SL_F_SCHM_HTTP or HTX_SL_F_SCHM_HTTPS. Other schemes, for instance ftp, are
ignored.

6 years agoMINOR: h2: Set flags about the request's scheme on the start-line
Christopher Faulet [Fri, 14 Jun 2019 08:25:47 +0000 (10:25 +0200)] 
MINOR: h2: Set flags about the request's scheme on the start-line

The flag HTX_SL_F_HAS_SCHM is always set because H2 requests have always an
explicit scheme. Then, the pseudo-header ":scheme" is tested. If it is set to
"http", the flag HTX_SL_F_SCHM_HTTP is set. Otherwise, for all other cases, the
flag HTX_SL_F_SCHM_HTTPS is set. For now, it seems reasonable to have a fallback
on the scheme "https".

6 years agoMINOR: htx: Add 3 flags on the start-line to deal with the request schemes
Christopher Faulet [Fri, 14 Jun 2019 08:08:13 +0000 (10:08 +0200)] 
MINOR: htx: Add 3 flags on the start-line to deal with the request schemes

The first one, HTX_SL_F_HAS_SCHM, will be used to know the request has an
explicit scheme. So, in H2, it is always true because the pseudo-header
":scheme" is mandatory. In H1, it is only true when an absolute URI is found on
the start-line. The other flags, HTX_SL_F_SCHM_HTTP and HTX_SL_F_SCHM_HTTPS,
will be used to know which scheme the request have. For now, other protocols are
not handled.

The aim of these flags is to pass this information to the backend side in
general, and to the H2 mux in particular. So the multiplexer will have a chance
to use this information to send the right scheme to the server.

6 years agoBUG/MEDIUM: proto_htx: Introduce the state ENDING during forwarding
Christopher Faulet [Thu, 13 Jun 2019 14:43:22 +0000 (16:43 +0200)] 
BUG/MEDIUM: proto_htx: Introduce the state ENDING during forwarding

This state is used in the legacy HTTP when everything was received from an
endpoint but a filter doesn't forward all the data. It is used to not report a
client or a server abort, depending on channels flags.

The same must be done on HTX streams. Otherwise, the message may be
truncated. For instance, it may happen with the filter trace with the random
forwarding enabled on the response channel.

This patch must be backported to 1.9.

6 years agoCLEANUP: channel: Remove channel_htx_fwd_payload() and channel_htx_fwd_all()
Christopher Faulet [Thu, 13 Jun 2019 09:31:24 +0000 (11:31 +0200)] 
CLEANUP: channel: Remove channel_htx_fwd_payload() and channel_htx_fwd_all()

These functions are unused now. No backport needed.

6 years agoBUG/MEDIUM: htx: Don't change position of the first block during HTX analysis
Christopher Faulet [Thu, 13 Jun 2019 09:16:45 +0000 (11:16 +0200)] 
BUG/MEDIUM: htx: Don't change position of the first block during HTX analysis

In the HTX structure, the field <first> is used to know where to (re)start the
analysis. It may differ from the message's head. It is especially important to
update it to handle 1xx messages, to be sure to restart the analysis on the next
message (another 1xx message or the final one). It is also updated when some
data are forwarded (the headers or part of the body). But this update is an
error and must never be done at the analysis level. It is a bug, because some
sample fetches may be used after the data forwarding (but before the first send
of course). At this stage, if the first block position does not point on the
start-line, most of HTTP sample fetches fail.

So now, when something is forwarding by HTX analyzers, the first block position
is not update anymore.

This issue was reported on Github. See #119. No backport needed.

6 years agoBUG/MINOR: htx: Detect when tail_addr meet end_addr to maximize free rooms
Christopher Faulet [Wed, 12 Jun 2019 09:08:11 +0000 (11:08 +0200)] 
BUG/MINOR: htx: Detect when tail_addr meet end_addr to maximize free rooms

When a block's payload is moved during an expansion or when the whole block is
removed, the addresses of free spaces are updated accordingly. We must be
careful to reset them when <tail_addr> becomes equal to <end_addr>. In this
situation, we can maximize the free space between the blocks and their payload
and set the other one to 0. It is also important to be sure to never have
<end_addr> greater than <tail_addr>.

6 years agoBUG/MINOR: http: Use the global value to limit the number of parsed headers
Christopher Faulet [Tue, 11 Jun 2019 13:05:37 +0000 (15:05 +0200)] 
BUG/MINOR: http: Use the global value to limit the number of parsed headers

Instead of using the macro MAX_HTTP_HDR to limit the number of headers parsed
before throwing an error, we now use the custom global variable
global.tune.max_http_hdr.

This patch must be backported to 1.9.

6 years agoBUG/MINOR: channel/htx: Call channel_htx_full() from channel_full()
Christopher Faulet [Tue, 11 Jun 2019 12:14:49 +0000 (14:14 +0200)] 
BUG/MINOR: channel/htx: Call channel_htx_full() from channel_full()

When channel_full() is called for an HTX stream, we fall back on the HTX
version. This function is called, among other, from tcp_inspect_request(). With
this patch, the inspect delay is respected again.

This patch must be backported to 1.9.

6 years agoBUG/MINOR: fl_trace/htx: Be sure to always forward trailers and EOM
Christopher Faulet [Wed, 12 Jun 2019 14:07:48 +0000 (16:07 +0200)] 
BUG/MINOR: fl_trace/htx: Be sure to always forward trailers and EOM

Previous fix about the random forwarding on the message body was not enough to
fix the bug in all cases. Among others, when there is no data but only the EOM,
we must forward everything.

This patch must be backported to 1.9 if the patch 0bdeeaacb ("BUG/MINOR:
flt_trace/htx: Only apply the random forwarding on the message body.") is also
backported.

6 years agoBUG/MINOR: task: prevent schedulable tasks from starving under high I/O activity
Willy Tarreau [Fri, 14 Jun 2019 06:30:10 +0000 (08:30 +0200)] 
BUG/MINOR: task: prevent schedulable tasks from starving under high I/O activity

With both I/O and tasks in the same tasklet list, we now have a very
smooth and responsive scheduler, providing a good fairness between I/O
activities. With the lower layers relying on tasklet a lot (I/O wakeup,
subscribe, etc), there may often be a large number of totally autonomous
tasklets doing their business such as forwarding data between two muxes.

But the task scheduler historically refrained from picking tasks from the
priority-ordered run queue to put them into the tasklet list until this
later had less than max_runqueue_depth entries. This was to make sure that
low-latency, high-priority tasks would have an opportunity to be dequeued
before others even if they arrive late. But the counter used for this is
still the tasklet list size, which contains countless I/O events. This
causes an unfairness between unbounded I/Os and bounded tasks, resulting
for example in the CLI responding slower when forwarding 40 Gbps of HTTP
traffic spread over a thousand of connections.

A good solution consists in sticking to the initial intent of
max_runqueue_depth which is to limit the number of tasks in the list
(to maintain fairness between them) and not to limit the number of these
tasks among tasklets. It just turns out that the task_list_size initially
was this task counter and changed over time to be a tasklet list size.
Let's simply refrain from updating it for pure tasklets so that it takes
back its original role of counting real tasks as its name implies. With
this change the CLI becomes instantly responsive under load again.

This patch may possibly be backported to 1.9 though it requires some
careful checks.

6 years agoBUG/MEDIUM: h1: Wait for the connection if the handshake didn't complete.
Olivier Houchard [Thu, 13 Jun 2019 15:54:33 +0000 (17:54 +0200)] 
BUG/MEDIUM: h1: Wait for the connection if the handshake didn't complete.

In h1_init(), also add the H1C_F_CS_WAIT_CONN flag if the handshake didn't
complete, otherwise we may end up letting the upper layer sending data too
soon.

6 years agoBUG/MEDIUM: h1: Don't wait for handshake if we had an error.
Olivier Houchard [Thu, 13 Jun 2019 15:37:00 +0000 (17:37 +0200)] 
BUG/MEDIUM: h1: Don't wait for handshake if we had an error.

In h1_process(), only wait for the handshake if we had no error on the
connection. If the handshake failed, we have to let the upper layer know.

6 years agoBUILD/MINOR: 51d: Updated build registration output to indicate thatif the library...
Ben51Degrees [Thu, 13 Jun 2019 15:51:59 +0000 (16:51 +0100)] 
BUILD/MINOR: 51d: Updated build registration output to indicate thatif the library is a dummy one or not.

When built with the dummy 51Degrees library for testing, the output will
include "(dummy library)" to ensure it is clear that this is this is not
the API.

6 years agoMINOR: doc: update the manpage and usage message about -S
William Lallemand [Thu, 13 Jun 2019 15:03:37 +0000 (17:03 +0200)] 
MINOR: doc: update the manpage and usage message about -S

Add -S in the manpage, and update the usage message.

Should be backported to 1.9.

6 years agoBUILD: travis-ci: add 51Degree device detection, update openssl to 1.1.1c
Ilya Shipitsin [Thu, 13 Jun 2019 15:00:05 +0000 (20:00 +0500)] 
BUILD: travis-ci: add 51Degree device detection, update openssl to 1.1.1c

6 years agoCLEANUP: 51d: move the 51d dummy lib to contrib/51d/src to match the real lib
Willy Tarreau [Thu, 13 Jun 2019 13:56:10 +0000 (15:56 +0200)] 
CLEANUP: 51d: move the 51d dummy lib to contrib/51d/src to match the real lib

This way the directory structure remains the same as with the real lib and
one can apply the same build options regardless of where the lib is stored,
removing any possible confusion.

6 years agoBUILD: Silence gcc warning about unused return value
Tim Duesterhus [Wed, 12 Jun 2019 18:47:30 +0000 (20:47 +0200)] 
BUILD: Silence gcc warning about unused return value

gcc (Ubuntu 5.4.0-6ubuntu1~16.04.11) 5.4.0 20160609
Copyright (C) 2015 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.

complains:

> src/debug.c: In function "ha_panic":
> src/debug.c:162:2: warning: ignoring return value of "write", declared with attribute warn_unused_result [-Wunused-result]
>  (void) write(2, trash.area, trash.data);
>    ^

6 years agoMINOR: doc: mention HAPROXY_LOCALPEER in the man
William Lallemand [Thu, 13 Jun 2019 13:39:48 +0000 (15:39 +0200)] 
MINOR: doc: mention HAPROXY_LOCALPEER in the man

vMention the HAPROXY_LOCALPEER environment variable in the -L argument
of the manpage.

Should be backported in 1.9.

6 years agoMINOR: doc: add master-worker in the man page
William Lallemand [Thu, 13 Jun 2019 09:51:09 +0000 (11:51 +0200)] 
MINOR: doc: add master-worker in the man page

Add some information about the master-worker in the man page.

Should be backported in every version since 1.8.

6 years agoMINOR: doc: Remove -Ds option in man page
Kazuo Yagi [Thu, 13 Jun 2019 08:14:57 +0000 (17:14 +0900)] 
MINOR: doc: Remove -Ds option in man page

Remove -Ds option in man page.

Should be backported in every version since 1.8.

6 years agoMINOR: mworker: add the HAProxy version in "show proc"
William Lallemand [Wed, 12 Jun 2019 17:11:33 +0000 (19:11 +0200)] 
MINOR: mworker: add the HAProxy version in "show proc"

Displays the HAProxy version so you can compare the version of old
processes and new ones.

6 years agoMINOR: mworker: change formatting in uptime field of "show proc"
William Lallemand [Wed, 12 Jun 2019 16:21:17 +0000 (18:21 +0200)] 
MINOR: mworker: change formatting in uptime field of "show proc"

Change the formatting of the uptime field in "show proc" so it's easier
to parse it. Remove the space between the day and the hour and align the
field on 15 characters.

6 years agoMINOR: 51d: Added dummy libraries for the 51Degrees module for testing.
Ben51Degrees [Wed, 12 Jun 2019 14:42:53 +0000 (15:42 +0100)] 
MINOR: 51d: Added dummy libraries for the 51Degrees module for testing.

These are intended for use by HAProxy developers to ensure any changes
did not affect the 51Degrees implementation. The 51Degrees module can be
enabled and used by using the source in contrib/51d. This will run
without breaking, but will not return any meaningful information.

This is ideal for testing HAProxy core code, and other modules alongside
51Degrees, but should never be used as an actual module as it does
nothing.

6 years agoBUG/MINOR: 51d/htx: The _51d_fetch method, and the methods it calls are now HTX aware.
Ben51Degrees [Wed, 12 Jun 2019 14:19:12 +0000 (15:19 +0100)] 
BUG/MINOR: 51d/htx: The _51d_fetch method, and the methods it calls are now HTX aware.

The _51d_fetch method, and the two methods it calls to fetch HTTP
headers (_51d_set_device_offsets, and _51d_set_headers), now support
both legacy and HTX operation.

This should be backported to 1.9.

6 years agoMINOR: http: add a new "http-request replace-uri" action
Willy Tarreau [Wed, 12 Jun 2019 15:44:02 +0000 (17:44 +0200)] 
MINOR: http: add a new "http-request replace-uri" action

This action is particularly convenient to replace some deprecated usees
of "reqrep". It takes a match and a format string including back-
references. The reqrep warning was updated to suggest it as well.

6 years agoDOC: mworker-prog: documentation for the program section
William Lallemand [Wed, 12 Jun 2019 14:32:11 +0000 (16:32 +0200)] 
DOC: mworker-prog: documentation for the program section

This patch documents the program feature.

6 years agoMINOR: fd: Don't use atomic operations when it's not needed.
Olivier Houchard [Wed, 12 Jun 2019 12:31:08 +0000 (14:31 +0200)] 
MINOR: fd: Don't use atomic operations when it's not needed.

In updt_fd_polling(), when updating fd_nbupdt, there's no need to use an
atomic operation, as it's a TLS variable.

6 years ago[RELEASE] Released version 2.0-dev7 v2.0-dev7
Willy Tarreau [Tue, 11 Jun 2019 17:28:00 +0000 (19:28 +0200)] 
[RELEASE] Released version 2.0-dev7

Released version 2.0-dev7 with the following main changes :
    - BUG/MEDIUM: mux-h2: make sure the connection timeout is always set
    - MINOR: tools: add new bitmap manipulation functions
    - MINOR: logs: use the new bitmap functions instead of fd_sets for encoding maps
    - MINOR: chunks: Make sure trash_size is only set once.
    - Revert "MINOR: chunks: Make sure trash_size is only set once."
    - MINOR: threads: serialize threads initialization
    - MINOR peers: data structure simplifications for server names dictionary cache.
    - DOC: peers: Update for dictionary cache entries for peers protocol.
    - MINOR: dict: Store the length of the dictionary entries.
    - MINOR: peers: A bit of optimization when encoding cached server names.
    - MINOR: peers: Optimization for dictionary cache lookup.
    - MEDIUM: tools: improve time format error detection
    - BUG/MEDIUM: H1: When upgrading, make sure we don't free the buffer too early.
    - BUG/MEDIUM: stream_interface: Make sure we call si_cs_process() if CS_FL_EOI.
    - MINOR: threads: avoid clearing harmless twice in thread_release()
    - MEDIUM: threads: add thread_sync_release() to synchronize steps
    - BUG/MEDIUM: init/threads: prevent initialized threads from starting before others
    - OPTIM/MINOR: init/threads: only call protocol_enable_all() on first thread
    - BUG/MINOR: dict: race condition fix when inserting dictionary entries.
    - MEDIUM: init/threads: don't use spinlocks during the init phase
    - BUG/MINOR: cache/htx: Fix the counting of data already sent by the cache applet
    - BUG/MEDIUM: compression/htx: Fix the adding of the last data block
    - MINOR: flt_trace: Don't scrash the original offset during the random forwarding
    - MAJOR: htx: Rework how free rooms are tracked in an HTX message
    - MINOR: htx: Add the function htx_move_blk_before()
    - Revert "BUG/MEDIUM: H1: When upgrading, make sure we don't free the buffer too early."
    - BUG/MINOR: http-rules: mention "deny_status" for "deny" in the error message
    - MINOR: http: turn default error files to HTTP/1.1
    - BUG/MEDIUM: h1: Don't try to subscribe if we had a connection error.
    - BUG/MEDIUM: h1: Don't consider we're connected if the handshake isn't done.
    - MINOR: contrib/spoa_server: Upgrade SPOP to 2.0
    - BUG/MEDIUM: contrib/spoa_server: Set FIN flag on agent frames
    - MINOR: contrib/spoa_server: Add random IP score
    - DOC/MINOR: contrib/spoa_server: Fix typo in README

6 years agoDOC/MINOR: contrib/spoa_server: Fix typo in README
Daniel Corbett [Tue, 11 Jun 2019 14:08:53 +0000 (10:08 -0400)] 
DOC/MINOR: contrib/spoa_server: Fix typo in README

Fix typo in README ps_pyhton.py -> ps_python.py

6 years agoMINOR: contrib/spoa_server: Add random IP score
Daniel Corbett [Tue, 11 Jun 2019 14:04:15 +0000 (10:04 -0400)] 
MINOR: contrib/spoa_server: Add random IP score

The example configuration uses sess.ip_score however this variable
is not referenced within the example scripts.  This patch adds support
for sess.ip_score to the python + lua scripts and generates a
random number between 1 and 100.

6 years agoBUG/MEDIUM: contrib/spoa_server: Set FIN flag on agent frames
Daniel Corbett [Tue, 11 Jun 2019 13:46:27 +0000 (09:46 -0400)] 
BUG/MEDIUM: contrib/spoa_server: Set FIN flag on agent frames

When communicating over SPOP the AGENT-HELLO, AGENT-DISCONNECT,
and ACK frames must have the FIN flag set.

6 years agoMINOR: contrib/spoa_server: Upgrade SPOP to 2.0
Daniel Corbett [Tue, 11 Jun 2019 01:01:32 +0000 (21:01 -0400)] 
MINOR: contrib/spoa_server: Upgrade SPOP to 2.0

Upgrade SPOP version to 2.0

6 years agoBUG/MEDIUM: h1: Don't consider we're connected if the handshake isn't done.
Olivier Houchard [Tue, 11 Jun 2019 14:37:24 +0000 (16:37 +0200)] 
BUG/MEDIUM: h1: Don't consider we're connected if the handshake isn't done.

In h1_process(), don't consider we're connected if we still have handshakes
pending. It used not to happen, because we would not be called if there
were any ongoing handshakes, but that changed now that the handshakes are
handled by a xprt, and not by conn_fd_handler() directly.

6 years agoBUG/MEDIUM: h1: Don't try to subscribe if we had a connection error.
Olivier Houchard [Tue, 11 Jun 2019 14:36:33 +0000 (16:36 +0200)] 
BUG/MEDIUM: h1: Don't try to subscribe if we had a connection error.

If the CO_FL_ERROR flag is set, and we weren't connected yet, don't attempt
to subscribe, as the underlying xprt may already have been destroyed.

6 years agoMINOR: http: turn default error files to HTTP/1.1
Willy Tarreau [Tue, 11 Jun 2019 14:08:25 +0000 (16:08 +0200)] 
MINOR: http: turn default error files to HTTP/1.1

For quite a long time we've been saying that the default error files
should produce HTTP/1.1 responses and since it's of low importance, it
always gets forgotten.

So here it finally comes. Each status code now properly contains a
content-length header so that the output is clean and doesn't force
upstream proxies to switch to chunked encoding or to close the connection
immediately after the response, which is particularly annoying for 401
or 407 for example. It's worth noting that the 3xx codes had already
been turned to HTTP/1.1.

This patch will obviously not change anything for user-provided error files.

6 years agoBUG/MINOR: http-rules: mention "deny_status" for "deny" in the error message
Willy Tarreau [Tue, 11 Jun 2019 14:01:56 +0000 (16:01 +0200)] 
BUG/MINOR: http-rules: mention "deny_status" for "deny" in the error message

The error message indicating an unknown keyword on an http-request rule
doesn't mention the "deny_status" option which comes with the "deny" rule,
this is particularly confusing.

This can be backported to all versions supporting this option.

6 years agoRevert "BUG/MEDIUM: H1: When upgrading, make sure we don't free the buffer too early."
Olivier Houchard [Tue, 11 Jun 2019 12:06:23 +0000 (14:06 +0200)] 
Revert "BUG/MEDIUM: H1: When upgrading, make sure we don't free the buffer too early."

This reverts commit 6c7fe5c3700fac7cc945b2b756df30874cbf77a6.

This patch was harmless, but not needed, conn_upgrade_mux_fe() already takes
care of setting the buffer to BUF_NULL.

6 years agoMINOR: htx: Add the function htx_move_blk_before()
Christopher Faulet [Tue, 11 Jun 2019 08:41:19 +0000 (10:41 +0200)] 
MINOR: htx: Add the function htx_move_blk_before()

The function htx_add_data_before() was removed because it was buggy. The
function htx_move_blk_before() may be used if necessary to do something
equivalent, except it just moves blocks. It doesn't handle the adding.

6 years agoMAJOR: htx: Rework how free rooms are tracked in an HTX message
Christopher Faulet [Tue, 11 Jun 2019 08:40:43 +0000 (10:40 +0200)] 
MAJOR: htx: Rework how free rooms are tracked in an HTX message

In an HTX message, it may have 2 available rooms to store a new block. The first
one is between the blocks and their payload. Blocks are added starting from the
end of the buffer and their payloads are added starting from the begining. So
the first free room is between these 2 edges. The second one is at the begining
of the buffer, when we start to wrap to add new payloads. Once we start to use
this one, the other one is ignored until the next defragmentation of the HTX
message.

In theory, there is no problem. But in practice, some lacks in the HTX structure
force us to defragment too often HTX messages to always be in a known state. The
second free room is not tracked as it should do and the first one may be easily
corrupted when rewrites happen.

So to fix the problem and avoid unecessary defragmentation, the HTX structure
has been refactored. The front (the block's position of the first payload before
the blocks) is no more stored. Instead we keep the relative addresses of 3 edges:

 * tail_addr : The start address of the free space in front of the the blocks
               table
 * head_addr : The start address of the free space at the beginning
 * end_addr  : The end address of the free space at the beginning

Here is the general view of the HTX message now:

           head_addr     end_addr    tail_addr
               |            |            |
               V            V            V
  +------------+------------+------------+------------+------------------+
  |            |            |            |            |                  |
  |  PAYLOAD   | Free space |  PAYLOAD   | Free space |    Blocks area   |
  |    ==>     |     1      |    ==>     |     2      |        <==       |
  +------------+------------+------------+------------+------------------+

<head_addr> is always lower or equal to <end_addr> and <tail_addr>. <end_addr>
is always lower or equal to <tail_addr>.

In addition;, to simplify everything, the blocks area are now contiguous. It
doesn't wrap anymore. So the head is always the block with the lowest position,
and the tail is always the one with the highest position.

6 years agoMINOR: flt_trace: Don't scrash the original offset during the random forwarding
Christopher Faulet [Tue, 11 Jun 2019 08:20:57 +0000 (10:20 +0200)] 
MINOR: flt_trace: Don't scrash the original offset during the random forwarding

There is no bug here, but this patch improves the debug message reported during
the random forwarding. The original offset is kept untouched so its value may be
used to format the message. Before, 0 was always reported.

6 years agoBUG/MEDIUM: compression/htx: Fix the adding of the last data block
Christopher Faulet [Tue, 11 Jun 2019 08:38:38 +0000 (10:38 +0200)] 
BUG/MEDIUM: compression/htx: Fix the adding of the last data block

The function htx_add_data_before() is buggy and cannot work. It first add a data
block and then move it before another one, passed in argument. The problem
happens when a defragmentation is done to add the new block. In this case, the
reference is no longer valid, because the blocks are rearranged. So, instead of
moving the new block before the reference, it is moved at the head of the HTX
message.

So this function has been removed. It was only used by the compression filter to
add a last data block before a TLR, EOT or EOM block. Now, the new function
htx_add_last_data() is used. It adds a last data block, after all others and
before any TLR, EOT or EOM block. Then, the next bock is get. It is the first
non-data block after data in the HTX message. The compression loop continues
with it.

This patch must be backported to 1.9.

6 years agoBUG/MINOR: cache/htx: Fix the counting of data already sent by the cache applet
Christopher Faulet [Tue, 11 Jun 2019 07:58:09 +0000 (09:58 +0200)] 
BUG/MINOR: cache/htx: Fix the counting of data already sent by the cache applet

Since the commit 8f3c256f7 ("MEDIUM: cache/htx: Always store info about HTX
blocks in the cache"), it is possible to read info about a data block without
sending anything. It is possible because we rely on the function htx_add_data(),
which will try to add data without any defragmentation. In such case, info about
the data block are skipped but don't count in data sent.

No need to backport this patch, expect if the commit 8f3c256f7 is backported
too.

6 years agoMEDIUM: init/threads: don't use spinlocks during the init phase
Willy Tarreau [Tue, 11 Jun 2019 07:16:41 +0000 (09:16 +0200)] 
MEDIUM: init/threads: don't use spinlocks during the init phase

PiBa-NL found some pathological cases where starting threads can hinder
each other and cause a measurable slow down. This problem is reproducible
with the following config (haproxy must be built with -DDEBUG_DEV) :

    global
stats socket /tmp/sock1 mode 666 level admin
nbthread 64

    backend stopme
timeout server  1s
option tcp-check
tcp-check send "debug dev exit\n"
server cli unix@/tmp/sock1 check

This will cause the process to be stopped once the checks are ready to
start. Binding all these to just a few cores magnifies the problem.
Starting them in loops shows a significant time difference among the
commits :

  # before startup serialization
  $ time for i in {1..20}; do taskset -c 0,1,2,3 ./haproxy-e186161 -db -f slow-init.cfg >/dev/null 2>&1; done

  real    0m1.581s
  user    0m0.621s
  sys     0m5.339s

  # after startup serialization
  $ time for i in {1..20}; do taskset -c 0,1,2,3 ./haproxy-e4d7c9dd -db -f slow-init.cfg >/dev/null 2>&1; done

  real    0m2.366s
  user    0m0.894s
  sys     0m8.238s

In order to address this, let's use plain mutexes and cond_wait during
the init phase. With this done, waiting threads now sleep and the problem
completely disappeared :

  $ time for i in {1..20}; do taskset -c 0,1,2,3 ./haproxy -db -f slow-init.cfg >/dev/null 2>&1; done

  real    0m0.161s
  user    0m0.079s
  sys     0m0.149s

6 years agoBUG/MINOR: dict: race condition fix when inserting dictionary entries.
Frédéric Lécaille [Tue, 11 Jun 2019 06:34:26 +0000 (08:34 +0200)] 
BUG/MINOR: dict: race condition fix when inserting dictionary entries.

When checking the result of an ebis_insert() call in an ebtree with unique keys,
if already present, in place of freeing() the old one and return the new one,
rather the correct way is to free the new one, and return the old one. For
this, the __dict_insert() function was folded into dict_insert() as this
significantly simplifies the test of duplicates.

Thanks to Olivier for having reported this bug which came with this one:
"MINOR: dict: Add dictionary new data structure".

6 years agoOPTIM/MINOR: init/threads: only call protocol_enable_all() on first thread
Willy Tarreau [Mon, 10 Jun 2019 08:14:52 +0000 (10:14 +0200)] 
OPTIM/MINOR: init/threads: only call protocol_enable_all() on first thread

There's no point in calling this on each and every thread since the first
thread passing there will enable the listeners, and the next ones will
simply scan all of them in turn to discover that they are already
initialized. Let's only initilize them on the first thread. This could
slightly speed up start up on very large configurations, eventhough most
of the time is still spent in the main thread binding the sockets.

A few measurements have constantly shown that this decreases the startup
time by ~0.1s for 150k listeners. Starting all of them in parallel doesn't
provide better results and can still expose some undesired races.

6 years agoBUG/MEDIUM: init/threads: prevent initialized threads from starting before others
Willy Tarreau [Mon, 10 Jun 2019 07:51:04 +0000 (09:51 +0200)] 
BUG/MEDIUM: init/threads: prevent initialized threads from starting before others

Since commit 6ec902a ("MINOR: threads: serialize threads initialization")
we now serialize threads initialization. But doing so has emphasized another
race which is that some threads may actually start the loop before others
are done initializing.

As soon as all threads enter the first thread_release() call, their rdv
bit is cleared and they're all waiting for all others' rdv to be cleared
as well, with their harmless bit set. The first one to notice the cleared
mask will progress through thread_isolate(), take rdv again preventing
most others from noticing its short pass to zero, and this first one will
be able to run all the way through the initialization till the last call
to thread_release() which it happily crosses, being the only one with the
rdv bit, leaving the room for one or a few others to do the same. This
results in some threads entering the loop before others are done with
their initialization, which is particularly bad. PiBa-NL reported that
some regtests fail for him due to this (which was impossible to reproduce
here, but races are racy by definition). However placing some printf()
in the initialization code definitely shows this unsychronized startup.

This patch takes a different approach in three steps :
  - first, we don't start with thread_release() anymore and we don't
    set the rdv mask anymore in the main call. This was initially done
    to let all threads start toghether, which we don't want. Instead
    we just start with thread_isolate(). Since all threads are harmful
    by default, they all wait for each other's readiness before starting.

  - second, we don't release with thread_release() but with
    thread_sync_release(), meaning that we don't leave the function until
    other ones have reached the point in the function where they decide
    to leave it as well.

  - third, it makes sure we don't start the listeners using
    protocol_enable_all() before all threads have allocated their local
    FD tables or have initialized their pollers, otherwise startup could
    be racy as well. It's worth noting that it is even possible to limit
    this call to thread #0 as it only needs to be performed once.

This now guarantees that all thread init calls start only after all threads
are ready, and that no thread enters the polling loop before all others have
completed their initialization.

Please check GH issues #111 and #117 for more context.

No backport is needed, though if some new init races are reported in
1.9 (or even 1.8) which do not affect 2.0, then it may make sense to
carefully backport this small series.

6 years agoMEDIUM: threads: add thread_sync_release() to synchronize steps
Willy Tarreau [Sun, 9 Jun 2019 10:20:02 +0000 (12:20 +0200)] 
MEDIUM: threads: add thread_sync_release() to synchronize steps

This function provides an alternate way to leave a critical section run
under thread_isolate(). Currently, a thread may remain in thread_release()
without having the time to notice that the rdv mask was released and taken
again by another thread entering thread_isolate() (often the same that just
released it). This is because threads wait in harmless mode in the loop,
which is compatible with the conditions to enter thread_isolate(). It's
not possible to make them wait with the harmless bit off or we cannot know
when the job is finished for the next thread to start in thread_isolate(),
and if we don't clear the rdv bit when going there, we create another
race on the start point of thread_isolate().

This new synchronous variant of thread_release() makes use of an extra
mask to indicate the threads that want to be synchronously released. In
this case, they will be marked harmless before releasing their sync bit,
and will wait for others to release their bit as well, guaranteeing that
thread_isolate() cannot be started by any of them before they all left
thread_sync_release(). This allows to construct synchronized blocks like
this :

     thread_isolate()
     /* optionally do something alone here */
     thread_sync_release()
     /* do something together here */
     thread_isolate()
     /* optionally do something alone here */
     thread_sync_release()

And so on. This is particularly useful during initialization where several
steps have to be respected and no thread must start a step before the
previous one is completed by other threads.

This one must not be placed after any call to thread_release() or it would
risk to block an earlier call to thread_isolate() which the current thread
managed to leave without waiting for others to complete, and end up here
with the thread's harmless bit cleared, blocking others. This might be
improved in the future.

6 years agoMINOR: threads: avoid clearing harmless twice in thread_release()
Willy Tarreau [Sun, 9 Jun 2019 06:44:19 +0000 (08:44 +0200)] 
MINOR: threads: avoid clearing harmless twice in thread_release()

thread_release() is to be called after thread_isolate(), i.e. when the
thread already has its harmless bit cleared. No need to clear it twice,
thus avoid calling thread_harmless_end() and directly check the rdv
bits then loop on them.

6 years agoBUG/MEDIUM: stream_interface: Make sure we call si_cs_process() if CS_FL_EOI.
Olivier Houchard [Fri, 7 Jun 2019 16:10:52 +0000 (18:10 +0200)] 
BUG/MEDIUM: stream_interface: Make sure we call si_cs_process() if CS_FL_EOI.

In si_cs_recv(), if we got the CS_FL_EOI flag on the conn_stream, make sure
we return 1, so that si_cs_process() will be called, and wake
process_stream() up, otherwise if we're unlucky the flag will never be
noticed, and the stream won't be woken up.

6 years agoBUG/MEDIUM: H1: When upgrading, make sure we don't free the buffer too early.
Olivier Houchard [Fri, 7 Jun 2019 16:08:17 +0000 (18:08 +0200)] 
BUG/MEDIUM: H1: When upgrading, make sure we don't free the buffer too early.

In h1_release(), when we want to upgrade the mux to h2, make sure we set
h1c->ibuf to BUF_NULL before calling conn_upgrade_mux_fe().
If the upgrade is successful, the buffer will be provided to the new mux,
h1_release() will be called recursively, it will so try to free h1c->ibuf,
and freeing the buffer we just provided to the new mux would be unfortunate.

6 years agoMEDIUM: tools: improve time format error detection
Willy Tarreau [Fri, 7 Jun 2019 17:00:37 +0000 (19:00 +0200)] 
MEDIUM: tools: improve time format error detection

As reported in GH issue #109 and in discourse issue
https://discourse.haproxy.org/t/haproxy-returns-408-or-504-error-when-timeout-client-value-is-every-25d
the time parser doesn't error on overflows nor underflows. This is a
recurring problem which additionally has the bad taste of taking a long
time before hitting the user.

This patch makes parse_time_err() return special error codes for overflows
and underflows, and adds the control in the call places to report suitable
errors depending on the requested unit. In practice, underflows are almost
never returned as the parsing function takes care of rounding values up,
so this might possibly happen on 64-bit overflows returning exactly zero
after rounding though. It is not really possible to cut the patch into
pieces as it changes the function's API, hence all callers.

Tests were run on about every relevant part (cookie maxlife/maxidle,
server inter, stats timeout, timeout*, cli's set timeout command,
tcp-request/response inspect-delay).

6 years agoMINOR: peers: Optimization for dictionary cache lookup.
Frédéric Lécaille [Fri, 7 Jun 2019 12:25:25 +0000 (14:25 +0200)] 
MINOR: peers: Optimization for dictionary cache lookup.

When we look up an dictionary entry in the cache used upon transmission
we store the last result in ->prev_lookup of struct dcache_tx so that
to compare it with the subsequent entries to look up and save performances.

6 years agoMINOR: peers: A bit of optimization when encoding cached server names.
Frédéric Lécaille [Fri, 7 Jun 2019 08:34:04 +0000 (10:34 +0200)] 
MINOR: peers: A bit of optimization when encoding cached server names.

When a server name is cached we only send its cache entry ID which has
an encoded length of 1 (because smaller than PEER_ENC_2BYTES_MIN).
So, in this case we only have to encode 1, the already known encoded length
of this ID before encoding it.

Furthermore we do not have to call strlen() to compute the lengths of server
name strings thanks to this commit: "MINOR: dict: Store the length of the
dictionary entries".

6 years agoMINOR: dict: Store the length of the dictionary entries.
Frédéric Lécaille [Fri, 7 Jun 2019 08:58:20 +0000 (10:58 +0200)] 
MINOR: dict: Store the length of the dictionary entries.

When allocating new dictionary entries we store the length of the strings.
May be useful so that not to have to call strlen() too much often at runing
time.

6 years agoDOC: peers: Update for dictionary cache entries for peers protocol.
Frédéric Lécaille [Thu, 6 Jun 2019 13:53:20 +0000 (15:53 +0200)] 
DOC: peers: Update for dictionary cache entries for peers protocol.

Add information about how the peers protocol send/receive entries of
LRU caches for literal dictionaries (e.g. server names in replacement
for server IDs).

6 years agoMINOR peers: data structure simplifications for server names dictionary cache.
Frédéric Lécaille [Thu, 6 Jun 2019 09:34:03 +0000 (11:34 +0200)] 
MINOR peers: data structure simplifications for server names dictionary cache.

We store pointers to server names dictionary entries in a pre-allocated array of
ebpt_node's (->entries member of struct dcache_tx) to cache those sent to remote
peers. Consequently the ID used to identify the server name dictionary entry is
also used as index for this array. There is no need to implement a lookup by key
for this dictionary cache.

6 years agoMINOR: threads: serialize threads initialization
Willy Tarreau [Fri, 7 Jun 2019 12:41:11 +0000 (14:41 +0200)] 
MINOR: threads: serialize threads initialization

There is no point in initializing threads in parallel when we know that
it's the moment where some global variables are turned to thread-local
ones, and/or that some global variables are updated (like global_now or
trash_size). Some FDs might be created/destroyed/reallocated and could
be tricky to follow as well (think about epoll_fd for example).

Instead of having to be extremely careful about all these, and to trigger
false positives in thread sanitizers, let's simply initialize one thread
at a time. The init step is very fast so nobody should even notice, and
we won't have any more doubts about what might have happened when
analysing a dump.

See GH issues #111 and #117 for some background on this.

6 years agoRevert "MINOR: chunks: Make sure trash_size is only set once."
Willy Tarreau [Fri, 7 Jun 2019 13:33:06 +0000 (15:33 +0200)] 
Revert "MINOR: chunks: Make sure trash_size is only set once."

This reverts commit 1c3b83242da96f1d94ef3df34c399c1b984668b6.

It was made only to silence the thread sanitizer but ends up creating a
bug. Indeed, if "tune.bufsize" is in the global section, the trash_size
value is not updated anymore and the trash becomes smaller than a buffer!

Let's stop trying to fix the thread sanitizer reports, they are invalid,
and trying to fix them actually introduces bugs where there were none.

See GH issue #117 for more context. No backport is needed.

6 years agoMINOR: chunks: Make sure trash_size is only set once.
Olivier Houchard [Fri, 7 Jun 2019 12:35:35 +0000 (14:35 +0200)] 
MINOR: chunks: Make sure trash_size is only set once.

The trash_size variable is shared by all threads, and is set by all threads,
when alloc_trash_buffers() is called. To make sure it's set only once,
to silence a harmless data race, use a CAS to set it, and only set it if it
was 0.

6 years agoMINOR: logs: use the new bitmap functions instead of fd_sets for encoding maps
Willy Tarreau [Fri, 7 Jun 2019 09:10:07 +0000 (11:10 +0200)] 
MINOR: logs: use the new bitmap functions instead of fd_sets for encoding maps

The fd_sets we've been using in the log encoding functions are not portable
and were shown to break at least under Cygwin. This patch gets rid of them
in favor of the new bitmap functions. It was verified with the config below
that the log output was exactly the same before and after the change :

    defaults
        mode http
        option httplog
        log stdout local0
        timeout client 1s
        timeout server 1s
        timeout connect 1s

    frontend foo
        bind :8001
        capture request header chars len 255

    backend bar
        option httpchk "GET" "/" "HTTP/1.0\r\nchars: \x01\x02\x03\x04\x05\x06\x07\x08\x09\x0b\x0c\x0e\x0f\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f\x20\x21\x22\x23\x24\x25\x26\x27\x28\x29\x2a\x2b\x2c\x2d\x2e\x2f\x30\x31\x32\x33\x34\x35\x36\x37\x38\x39\x3a\x3b\x3c\x3d\x3e\x3f\x40\x41\x42\x43\x44\x45\x46\x47\x48\x49\x4a\x4b\x4c\x4d\x4e\x4f\x50\x51\x52\x53\x54\x55\x56\x57\x58\x59\x5a\x5b\x5c\x5d\x5e\x5f\x60\x61\x62\x63\x64\x65\x66\x67\x68\x69\x6a\x6b\x6c\x6d\x6e\x6f\x70\x71\x72\x73\x74\x75\x76\x77\x78\x79\x7a\x7b\x7c\x7d\x7e\x7f\x80\x81\x82\x83\x84\x85\x86\x87\x88\x89\x8a\x8b\x8c\x8d\x8e\x8f\x90\x91\x92\x93\x94\x95\x96\x97\x98\x99\x9a\x9b\x9c\x9d\x9e\x9f\xa0\xa1\xa2\xa3\xa4\xa5\xa6\xa7\xa8\xa9\xaa\xab\xac\xad\xae\xaf\xb0\xb1\xb2\xb3\xb4\xb5\xb6\xb7\xb8\xb9\xba\xbb\xbc\xbd\xbe\xbf\xc0\xc1\xc2\xc3\xc4\xc5\xc6\xc7\xc8\xc9\xca\xcb\xcc\xcd\xce\xcf\xd0\xd1\xd2\xd3\xd4\xd5\xd6\xd7\xd8\xd9\xda\xdb\xdc\xdd\xde\xdf\xe0\xe1\xe2\xe3\xe4\xe5\xe6\xe7\xe8\xe9\xea\xeb\xec\xed\xee\xef\xf0\xf1\xf2\xf3\xf4\xf5\xf6\xf7\xf8\xf9\xfa\xfb\xfc\xfd\xfe\xff"
        server foo 127.0.0.1:8001 check

6 years agoMINOR: tools: add new bitmap manipulation functions
Willy Tarreau [Fri, 7 Jun 2019 08:42:43 +0000 (10:42 +0200)] 
MINOR: tools: add new bitmap manipulation functions

We now have ha_bit_{set,clr,flip,test} to manipulate bitfields made
of arrays of longs. The goal is to get rid of the remaining non-portable
FD_{SET,CLR,ISSET} that still exist at a few places.

6 years agoBUG/MEDIUM: mux-h2: make sure the connection timeout is always set
Willy Tarreau [Fri, 7 Jun 2019 06:20:46 +0000 (08:20 +0200)] 
BUG/MEDIUM: mux-h2: make sure the connection timeout is always set

There seems to be a tricky case in the H2 mux related to stream flow
control versus buffer a full situation : is a large response cannot
be entirely sent to the client due to the stream window being too
small, the stream is paused with the SFCTL flag. Then the upper
layer stream might get bored and expire this stream. It will then
shut it down first. But the shutdown operation might fail if the
mux buffer is full, resulting in the h2s being subscribed to the
deferred_shut event with the stream *not* added to the send_list
since it's blocked in SFCTL. In the mean time the upper layer completely
closes, calling h2_detach(). There we have a send_wait (the pending
shutw), the stream is marked with SFCTL so we orphan it.

Then if the client finally reads all the data that were clogging the
buffer, the send_list is run again, but our stream is not there. From
this point, the connection's stream list is not empty, the mux buffer
is empty, so the connection's timeout is not set. If the client
disappears without updating the stream's window, nothing will expire
the connection.

This patch makes sure we always keep the connection timeout updated.
There might be finer solutions, such as checking that there are still
living streams in the connection (i.e. streams not blocked in SFCTL
state), though this is not necessarily trivial nor useful, since the
client timeout is the same for the upper level stream and the connection
anyway.

This patch needs to be backported to 1.9 and 1.8 after some observation.

6 years ago[RELEASE] Released version 2.0-dev6 v2.0-dev6
Willy Tarreau [Fri, 7 Jun 2019 04:12:59 +0000 (06:12 +0200)] 
[RELEASE] Released version 2.0-dev6

Released version 2.0-dev6 with the following main changes :
    - BUG/MEDIUM: connection: fix multiple handshake polling issues
    - MINOR: connection: also stop receiving after a SOCKS4 response
    - MINOR: mux-h1: don't try to recv() before the connection is ready
    - BUG/MEDIUM: mux-h1: only check input data for the current stream, not next one
    - MEDIUM: mux-h1: don't use CS_FL_REOS anymore
    - CLEANUP: connection: remove the now unused CS_FL_REOS flag
    - CONTRIB: debug: add 4 missing connection/conn_stream flags
    - MEDIUM: stream: make a full process_stream() loop when completing I/O on exit
    - MINOR: server: increase the default pool-purge-delay to 5 seconds
    - BUILD: tools: do not use the weak attribute for trace() on obsolete linkers
    - BUG/MEDIUM: vars: make sure the scope is always valid when accessing vars
    - BUG/MEDIUM: vars: make the tcp/http unset-var() action support conditions
    - BUILD: task: fix a build warning when threads are disabled
    - CLEANUP: peers: Remove tabs characters.
    - CLEANUP: peers: Replace hard-coded values by macros.
    - BUG/MINOR: peers: Wrong stick-table update message building.
    - MINOR: dict: Add dictionary new data structure.
    - MINOR: peers: Add a LRU cache implementation for dictionaries.
    - MINOR: stick-table: Add "server_name" new data type.
    - MINOR: cfgparse: Space allocation for "server_name" stick-table data type.
    - MINOR: proxy: Add a "server by name" tree to proxy.
    - MINOR: server: Add a dictionary for server names.
    - MINOR: stream: Stickiness server lookup by name.
    - MINOR: peers: Make peers protocol support new "server_name" data type.
    - MINOR: stick-table: Make the CLI stick-table handler support dictionary entry data type.
    - REGTEST: Add a basic server by name stickiness reg test.
    - MINOR: peers: Add dictionary cache information to "show peers" CLI command.
    - MINOR: peers: Replace hard-coded for peer protocol 64-bits value encoding by macros.
    - MINOR: peers: Replace hard-coded values for peer protocol messaging by macros.
    - CLEANUP: ssl: remove unneeded defined(OPENSSL_IS_BORINGSSL)
    - BUILD: travis-ci improvements
    - MINOR: SSL: add client/server random sample fetches
    - BUG/MINOR: channel/htx: Don't alter channel during forward for empty HTX message
    - BUG/MINOR: contrib/prometheus-exporter: Add HTX data block in one time
    - BUG/MINOR: mux-h1: errflag must be set on H1S and not H1M during output processing
    - MEDIUM: mux-h1: refactor output processing
    - MINOR: mux-h1: Add the flag HAVE_O_CONN on h1s
    - MINOR: mux-h1: Add h1_eval_htx_hdrs_size() to estimate size of the HTX headers
    - MINOR: mux-h1: Don't count the EOM in the estimated size of headers
    - MEDIUM: cache/htx: Always store info about HTX blocks in the cache
    - MEDIUM: htx: Add the parsing of trailers of chunked messages
    - MINOR: htx: Don't use end-of-data blocks anymore
    - BUG/MINOR: mux-h1: Don't send more data than expected
    - BUG/MINOR: flt_trace/htx: Only apply the random forwarding on the message body.
    - BUG/MINOR: peers: Wrong "server_name" decoding.
    - BUG/MEDIUM: servers: Don't attempt to destroy idle connections if disabled.
    - MEDIUM: checks: Make sure we unsubscribe before calling cs_destroy().
    - MEDIUM: connections: Wake the upper layer even if sending/receiving is disabled.
    - MEDIUM: ssl: Handle subscribe by itself.
    - MINOR: ssl: Make ssl_sock_handshake() static.
    - MINOR: connections: Add a new xprt method, remove_xprt.
    - MINOR: connections: Add a new xprt method, add_xprt().
    - MEDIUM: connections: Introduce a handshake pseudo-XPRT.
    - MEDIUM: connections: Remove CONN_FL_SOCK*
    - BUG/MEDIUM: ssl: Don't forget to initialize ctx->send_recv and ctx->recv_wait.
    - BUG/MINOR: peers: Wrong server name parsing.
    - MINOR: server: really increase the pool-purge-delay default to 5 seconds
    - BUG/MINOR: stream: don't emit a send-name-header in conn error or disconnect states
    - MINOR: stream-int: use bit fields to match multiple stream-int states at once
    - MEDIUM: stream-int: remove dangerous interval checks for stream-int states
    - MEDIUM: stream-int: introduce a new state SI_ST_RDY
    - MAJOR: stream-int: switch from SI_ST_CON to SI_ST_RDY on I/O
    - MEDIUM: stream-int: make idle-conns switch to ST_RDY
    - MEDIUM: stream: re-arrange the connection setup status reporting
    - MINOR: stream-int: split si_update() into si_update_rx() and si_update_tx()
    - MINOR: stream-int: make si_sync_send() from the send code of si_update_both()
    - MEDIUM: stream: rearrange the events to remove the loop
    - MEDIUM: stream: only loop on flags relevant to the analysers
    - MEDIUM: stream: don't abusively loop back on changes on CF_SHUT*_NOW
    - BUILD: stream-int: avoid a build warning in dev mode in si_state_bit()
    - BUILD: peers: fix a build warning about an incorrect intiialization
    - BUG/MINOR: time: make sure only one thread sets global_now at boot
    - BUG/MEDIUM: tcp: Make sure we keep the polling consistent in tcp_probe_connect.

6 years agoBUG/MEDIUM: tcp: Make sure we keep the polling consistent in tcp_probe_connect.
Olivier Houchard [Thu, 6 Jun 2019 16:15:01 +0000 (18:15 +0200)] 
BUG/MEDIUM: tcp: Make sure we keep the polling consistent in tcp_probe_connect.

In tcp_probe_connect(), if the connection is still pending, do not disable
want_recv, we don't have any business to do so, but explicitely use
__conn_xprt_want_send(), otherwise the next time we'll reach tcp_probe_connect,
fd_send_ready() would return 0 and we would never flag the connection as
CO_FL_CONNECTED, which can lead to various problems, such as check not
completing because they consider it is not connected yet.

6 years agoBUG/MINOR: time: make sure only one thread sets global_now at boot
Willy Tarreau [Thu, 6 Jun 2019 14:50:39 +0000 (16:50 +0200)] 
BUG/MINOR: time: make sure only one thread sets global_now at boot

All threads call tv_update_date(-1) at boot to set their own local time
offset. While doing so they also overwrite global_now, which is not that
much of a problem except that it's not done using an atomic write and
that it will be overwritten by every there in parallel. We only need the
first thread to set it anyway, so let's simply set it if not set and do
it using a CAS. This should fix GH issue #111.

This may be backported to 1.9.

6 years agoBUILD: peers: fix a build warning about an incorrect intiialization
Willy Tarreau [Thu, 6 Jun 2019 14:40:43 +0000 (16:40 +0200)] 
BUILD: peers: fix a build warning about an incorrect intiialization

Just got this one :
src/peers.c:528:13: warning: missing braces around initializer [-Wmissing-braces]
src/peers.c:528:13: warning: (near initialization for 'cde.key') [-Wmissing-braces]

Indeed, this struct contains two structs so scalar zero is not a valid
value for the first field. Let's just leave it as an empty struct since
it was the purpose.

6 years agoBUILD: stream-int: avoid a build warning in dev mode in si_state_bit()
Willy Tarreau [Thu, 6 Jun 2019 14:38:40 +0000 (16:38 +0200)] 
BUILD: stream-int: avoid a build warning in dev mode in si_state_bit()

The BUG_ON() test emits a warning about an always-true comparison regarding
<state> which cannot be lower than zero. Let's get rid of it.

6 years agoMEDIUM: stream: don't abusively loop back on changes on CF_SHUT*_NOW
Willy Tarreau [Thu, 6 Jun 2019 12:45:26 +0000 (14:45 +0200)] 
MEDIUM: stream: don't abusively loop back on changes on CF_SHUT*_NOW

These flags are not used by analysers, only by the shut* functions, and
they were covered by CF_MASK_STATIC only because in the past the shut
functions were in the middle of the analysers. But here they are causing
excess loop backs which provide no value and increase processing cost.
Ideally the CF_MASK_STATIC bitfield should be revisited, but doing this
alone is enough to reduce by 30% the number of calls to si_sync_send().

6 years agoMEDIUM: stream: only loop on flags relevant to the analysers
Willy Tarreau [Thu, 6 Jun 2019 12:32:49 +0000 (14:32 +0200)] 
MEDIUM: stream: only loop on flags relevant to the analysers

In process_stream() we detect a number of conditions to decide to loop
back to the analysers. Some of them are excessive in that they perform
a strict comparison instead of filtering on the flags relevant to the
analysers as is done at other places, resulting in excess wakeups. One
of the effect is that after a successful WRITE_PARTIAL, a second send is
not possible, resulting in the loss of WRITE_PARTIAL, causing another
wakeup! Let's apply the same mask and verify the flags correctly.

6 years agoMEDIUM: stream: rearrange the events to remove the loop
Willy Tarreau [Thu, 6 Jun 2019 07:17:23 +0000 (09:17 +0200)] 
MEDIUM: stream: rearrange the events to remove the loop

The "goto redo" at the end of process_stream() to make the states converge
is still a big source of problems and mostly stems from the very late call
to the send() functions, whose results need to be considered, while it's
being done in si_update_both() when leaving.

This patch extracts the si_sync_send() calls from si_update_both(), and
places them at the relevant places in process_stream(), which are just
after the amount of data to forward is updated and before the shutw()
calls (which were also moved). The stream-interface resynchronization
needs to go slightly upper to take into account the transition from CON
to RDY that will happen consecutive to some successful send(), and that's
all.

By doing so we can now get rid of this loop and have si_update_both()
called only to update the stream interface and channel when leaving the
function, as it was initially designed to work.

It is worth noting that a number of the remaining conditions to perform
a goto resync_XXX still seem suboptimal and would benefit from being
refined to perform les resynchronization. But what matters at this stage
is that the code remains valid and efficient.

6 years agoMINOR: stream-int: make si_sync_send() from the send code of si_update_both()
Willy Tarreau [Thu, 6 Jun 2019 06:20:17 +0000 (08:20 +0200)] 
MINOR: stream-int: make si_sync_send() from the send code of si_update_both()

Just like we have a synchronous recv() function for the stream interface,
let's have a synchronous send function that we'll be able to call from
different places. For now this only moves the code, nothing more.

6 years agoMINOR: stream-int: split si_update() into si_update_rx() and si_update_tx()
Willy Tarreau [Thu, 6 Jun 2019 06:19:20 +0000 (08:19 +0200)] 
MINOR: stream-int: split si_update() into si_update_rx() and si_update_tx()

We should not update the two directions at once, in fact we should update
the Rx path after recv() and the Tx path after send(). Let's start by
splitting the update function in two for this.

6 years agoMEDIUM: stream: re-arrange the connection setup status reporting
Willy Tarreau [Wed, 5 Jun 2019 16:02:04 +0000 (18:02 +0200)] 
MEDIUM: stream: re-arrange the connection setup status reporting

Till now when a wakeup happens after a connection is attempted, we go
through sess_update_st_con_tcp() to deal with the various possible events,
then to sess_update_st_cer() to deal with a possible error detected by the
former, or to sess_establish() to complete the connection validation. There
are multiple issues in the way this is handled, which have accumulated over
time. One of them is that any spurious wakeup during SI_ST_CON would validate
the READ_ATTACHED flag and wake the analysers up. Another one is that nobody
feels responsible for clearing SI_FL_EXP if it happened at the same time as
a success (and it is present in all reports of loops to date). And another
issue is that aborts cannot happen after a clean connection setup with no
data transfer (since CF_WRITE_NULL is part of CF_WRITE_ACTIVITY). Last, the
flags cleanup work was hackish, added here and there to please the next
function (typically what had to be donne in commit 7a3367cca to work around
the url_param+reuse issue by moving READ_ATTACHED to CON).

This patch performs a significant lift up of this setup code. First, it
makes sure that the state handlers are the ones responsible for the cleanup
of the stuff they rely on. Typically sess_sestablish() will clean up the
SI_FL_EXP flag because if we decided to validate the connection it means
that we want to ignore this late timeout. Second, it splits the CON and
RDY state handlers because the former only has to deal with failures,
timeouts and non-events, while the latter has to deal with partial or
total successes. Third, everything related to connection success was
moved to sess_establish() since it's the only safe place to do so, and
this function is also called at a few places to deal with synchronous
connections, which are not seen by intermediary state handlers.

The code was made a bit more robust, for example by making sure we
always set SI_FL_NOLINGER when aborting a connection so that we don't
have any risk to leave a connection in SHUTW state in case it was
validated late. The useless return codes of some of these functions
were dropped so that callers only rely on the stream-int's state now
(which was already partially the case anyway).

The code is now a bit cleaner, could be further improved (and functions
renamed) but given the sensitivity of this part, better limit changes to
strictly necessary. It passes all reg tests.

6 years agoMEDIUM: stream-int: make idle-conns switch to ST_RDY
Willy Tarreau [Thu, 6 Jun 2019 07:17:15 +0000 (09:17 +0200)] 
MEDIUM: stream-int: make idle-conns switch to ST_RDY

The purpose of making idle-conns switch to SI_ST_CON was to make the
transition detectable and the operation retryable in case of connection
error. Now we have the RDY state for this which is much more suitable
since it indicates a validated connection on which we didn't necessarily
send anything yet. This will still lead to a transition to EST while not
requiring unnatural write polling nor connect timeouts.

6 years agoMAJOR: stream-int: switch from SI_ST_CON to SI_ST_RDY on I/O
Willy Tarreau [Wed, 5 Jun 2019 14:43:44 +0000 (16:43 +0200)] 
MAJOR: stream-int: switch from SI_ST_CON to SI_ST_RDY on I/O

Now whenever an I/O event succeeds during a connection attempt, we
switch the stream-int's state to SI_ST_RDY. This allows si_update()
to update R/W timeouts on the channel and end points to start to
consume outgoing data and to subscribe to lower layers in case of
failure. It also allows chk_rcv() to be performed on the other side
to enable data forwarding and make sure we don't fall into a situation
where no more events happen and nothing moves anymore.

6 years agoMEDIUM: stream-int: introduce a new state SI_ST_RDY
Willy Tarreau [Wed, 5 Jun 2019 12:34:03 +0000 (14:34 +0200)] 
MEDIUM: stream-int: introduce a new state SI_ST_RDY

The main reason for all the trouble we're facing with stream interface
error or timeout reports during the connection phase is that we currently
can't make the difference between a connection attempt and a validated
connection attempt. It is problematic because we tend to switch early
to SI_ST_EST but can't always do what we want in this state since it's
supposed to be set when we don't need to visit sess_establish() again.

This patch introduces a new state betwen SI_ST_CON and SI_ST_EST, which
is SI_ST_RDY. It indicates that we've verified that the connection is
ready. It's a transient state, like SI_ST_DIS, that cannot persist when
leaving process_stream(). For now it is not set, only verified in various
tests where SI_ST_CON was used or SI_ST_EST depending on the cases.

The stream-int state diagram was minimally updated to reflect the new
state, though it is largely obsolete and would need to be seriously
updated.

6 years agoMEDIUM: stream-int: remove dangerous interval checks for stream-int states
Willy Tarreau [Wed, 5 Jun 2019 12:53:22 +0000 (14:53 +0200)] 
MEDIUM: stream-int: remove dangerous interval checks for stream-int states

The stream interface state checks involving ranges were replaced with
checks on a set of states, already revealing some issues. No issue was
fixed, all was replaced in a one-to-one mapping for easier control. Some
checks involving a strict difference were also replaced with fields to
be clearer. At this stage, the result must be strictly equivalent. A few
tests were also turned to their bit-field equivalent for better readability
or in preparation for upcoming changes.

The test performed in the SPOE filter was swapped so that the closed and
error states are evicted first and that the established vs conn state is
tested second.

6 years agoMINOR: stream-int: use bit fields to match multiple stream-int states at once
Willy Tarreau [Wed, 5 Jun 2019 12:45:06 +0000 (14:45 +0200)] 
MINOR: stream-int: use bit fields to match multiple stream-int states at once

At some places we do check for ranges of stream-int states but those
are confusing as states ordering is not well known (e.g. it's not obvious
that CER is between CON and EST). Let's create a bit field from states so
that we can match multiple states at once instead. The new enum si_state_bit
contains SI_SB_* which are state bits instead of state values. The function
si_state_in() indicates if the state in argument is one of those represented
by the bit mask in second argument.

6 years agoBUG/MINOR: stream: don't emit a send-name-header in conn error or disconnect states
Willy Tarreau [Wed, 5 Jun 2019 13:29:38 +0000 (15:29 +0200)] 
BUG/MINOR: stream: don't emit a send-name-header in conn error or disconnect states

The test for the send-name-header field used to cover all states between
SI_ST_CON and SI_ST_CLO, which include SI_ST_CER and SI_ST_DIS. Trying to
send a header in these states makes no sense at all, so let's fix this.
This should have no visible impact so no backport is needed.