]> git.ipfire.org Git - thirdparty/haproxy.git/log
thirdparty/haproxy.git
5 years agoREORG: include: move common/buffer.h to haproxy/dynbuf{,-t}.h
Willy Tarreau [Tue, 2 Jun 2020 09:28:02 +0000 (11:28 +0200)] 
REORG: include: move common/buffer.h to haproxy/dynbuf{,-t}.h

The pretty confusing "buffer.h" was in fact not the place to look for
the definition of "struct buffer" but the one responsible for dynamic
buffer allocation. As such it defines the struct buffer_wait and the
few functions to allocate a buffer or wait for one.

This patch moves it renaming it to dynbuf.h. The type definition was
moved to its own file since it's included in a number of other structs.

Doing this cleanup revealed that a significant number of files used to
rely on this one to inherit struct buffer through it but didn't need
anything from this file at all.

5 years agoREORG: include: move activity to haproxy/
Willy Tarreau [Tue, 2 Jun 2020 08:29:48 +0000 (10:29 +0200)] 
REORG: include: move activity to haproxy/

This moves types/activity.h to haproxy/activity-t.h and
proto/activity.h to haproxy/activity.h.

The macros defining the bit field values for the profiling variable
were moved to the type file to be more future-proof.

5 years agoREORG: include: move common/chunk.h to haproxy/chunk.h
Willy Tarreau [Tue, 2 Jun 2020 08:22:45 +0000 (10:22 +0200)] 
REORG: include: move common/chunk.h to haproxy/chunk.h

No change was necessary, it was already properly split.

5 years agoREORG: include: move common/memory.h to haproxy/pool.h
Willy Tarreau [Tue, 2 Jun 2020 07:38:52 +0000 (09:38 +0200)] 
REORG: include: move common/memory.h to haproxy/pool.h

Now the file is ready to be stored into its final destination. A few
minor reorderings were performed to keep the file properly organized,
making the various sections more visible (cache & lockless).

In addition and to stay consistent, memory.c was renamed to pool.c.

5 years agoMEDIUM: memory: make local pools independent on lockless pools
Willy Tarreau [Mon, 1 Jun 2020 17:00:28 +0000 (19:00 +0200)] 
MEDIUM: memory: make local pools independent on lockless pools

Till now the local pool caches were implemented only when lockless pools
were in use. This was mainly due to the difficulties to disentangle the
code parts. However the locked pools would further benefit from the local
cache, and having this would reduce the variants in the code.

This patch does just this. It adds a new debug macro DEBUG_NO_LOCAL_POOLS
to forcefully disable local pool caches, and makes sure that the high
level functions are now strictly the same between locked and lockless
(pool_alloc(), pool_alloc_dirty(), pool_free(), pool_get_first()). The
pool index calculation was moved inside the CONFIG_HAP_LOCAL_POOLS guards.
This allowed to move them out of the giant #ifdef and to significantly
reduce the code duplication.

A quick perf test shows that with locked pools the performance increases
by roughly 10% on 8 threads and gets closer to the lockless one.

5 years agoMINOR: memory: move pool-specific path of the locked pool_free() to __pool_free()
Willy Tarreau [Mon, 1 Jun 2020 16:35:24 +0000 (18:35 +0200)] 
MINOR: memory: move pool-specific path of the locked pool_free() to __pool_free()

pool_free() was not identical between locked and lockless pools. The
different was the call to __pool_free() in one case versus open-coded
accesses in the other, and the poisoning brought by commit da52035a45
("MINOR: memory: also poison the area on freeing") which unfortunately
did if only for the lockless path.

Let's now have __pool_free() to work on the global pool also in the
locked case so that the code is architected similarly.

5 years agoMEDIUM: memory: don't let pool_put_to_cache() free the objects itself
Willy Tarreau [Mon, 1 Jun 2020 16:16:57 +0000 (18:16 +0200)] 
MEDIUM: memory: don't let pool_put_to_cache() free the objects itself

Just as for the allocation path, the release path was not symmetrical.
It was not logical to have pool_put_to_cache() free the objects itself,
it was pool_free's job. In addition, just because of a variable export
issue, it the insertion of the object to free back into the local cache
couldn't be inlined while it was very cheap.

This patch just slightly reorganizes this code path by making pool_free()
decide whether or not to put the object back into the cache via
pool_put_to_cache() otherwise place it back to the global pool using
__pool_free().  Then pool_put_to_cache() adds the item to the local
cache and only calls pool_evict_from_cache() if the cache is too big.

5 years agoMINOR: memory: don't let __pool_get_first() pick from the cache
Willy Tarreau [Mon, 1 Jun 2020 15:51:05 +0000 (17:51 +0200)] 
MINOR: memory: don't let __pool_get_first() pick from the cache

When building with the local cache support, we have an asymmetry in
the allocation path which is that __pool_get_first() picks from the
cache while when no cache support is used, this one directly accesses
the shared area. It looks like it was done this way only to centralize
the call to __pool_get_from_cache() but this was not a good idea as it
complicates the splitting the code.

Let's move the cache access to the upper layer so thatt __pool_get_first()
remains agnostic to the cache support.

The call tree now looks like this with the cache enabled :

    pool_get_first()
      __pool_get_from_cache() // if cache enabled
      __pool_get_first()

    pool_alloc()
      pool_alloc_dirty()
        __pool_get_from_cache() // if cache enabled
        __pool_get_first()
        __pool_refill_alloc()

    __pool_free()
      pool_free_area()

    pool_put_to_cache()
      __pool_free()
      __pool_put_to_cache()

    pool_free()
      pool_put_to_cache()

With cache disabled, the pool_free() path still differs:

    pool_free()
      __pool_free_area()
      __pool_put_to_cache()

5 years agoREORG: memory: move the OS-level allocator to haproxy/pool-os.h
Willy Tarreau [Sat, 30 May 2020 16:56:17 +0000 (18:56 +0200)] 
REORG: memory: move the OS-level allocator to haproxy/pool-os.h

The memory.h file is particularly complex due to the combination of
debugging options. This patch extracts the OS-level interface and
places it into a new file: pool-os.h.

Doing this also moves pool_alloc_area() and pool_free_area() out of
the #ifndef CONFIG_HAP_LOCKLESS_POOLS, making them usable from
__pool_refill_alloc(), pool_free(), pool_flush() and pool_gc() instead
of having direct calls to malloc/free there that are hard to wrap for
debugging purposes.

5 years agoREORG: memory: move the pool type definitions to haproxy/pool-t.h
Willy Tarreau [Sat, 30 May 2020 16:27:03 +0000 (18:27 +0200)] 
REORG: memory: move the pool type definitions to haproxy/pool-t.h

This is the beginning of the move and cleanup of memory.h. This first
step only extracts type definitions and basic macros that are needed
by the files which reference a pool. They're moved to pool-t.h (since
"pool" is more obvious than "memory" when looking for pool-related
stuff). 3 files which didn't need to include the whole memory.h were
updated.

5 years agoCLEANUP: pool: include freq_ctr.h and remove locally duplicated functions
Willy Tarreau [Mon, 1 Jun 2020 10:35:03 +0000 (12:35 +0200)] 
CLEANUP: pool: include freq_ctr.h and remove locally duplicated functions

In memory.h we had to reimplement the swrate* functions just because of
a broken circular dependency around freq_ctr.h. Now that this one is
solved, let's get rid of this copy and use the original ones instead.

5 years agoREORG: include: move freq_ctr to haproxy/
Willy Tarreau [Mon, 1 Jun 2020 10:18:08 +0000 (12:18 +0200)] 
REORG: include: move freq_ctr to haproxy/

types/freq_ctr.h was moved to haproxy/freq_ctr-t.h and proto/freq_ctr.h
was moved to haproxy/freq_ctr.h. Files were updated accordingly, no other
change was applied.

5 years agoCLEANUP: include: remove excessive includes of common/standard.h
Willy Tarreau [Mon, 1 Jun 2020 10:09:26 +0000 (12:09 +0200)] 
CLEANUP: include: remove excessive includes of common/standard.h

Some of them were simply removed as unused (possibly some leftovers
from an older cleanup session), some were turned to haproxy/bitops.h
and a few had to be added (hlua.c and stick-table.h need standard.h
for parse_time_err; htx.h requires chunk.h but used to get it through
standard.h).

5 years agoREORG: include: move integer manipulation functions from standard.h to intops.h
Willy Tarreau [Mon, 1 Jun 2020 09:48:35 +0000 (11:48 +0200)] 
REORG: include: move integer manipulation functions from standard.h to intops.h

There are quite a number of integer manipulation functions defined in
standard.h, which is one of the reasons why standard.h is included from
many places and participates to the dependencies loop.

Let's just have a new file, intops.h to place all these operations.
These are a few bitops, 32/64 bit mul/div/rotate, integer parsing and
encoding (including varints), the full avalanche hash function, and
the my_htonll/my_ntohll functions.

For now no new C file was created for these yet.

5 years agoREORG: include: move time.h from common/ to haproxy/
Willy Tarreau [Mon, 1 Jun 2020 09:05:15 +0000 (11:05 +0200)] 
REORG: include: move time.h from common/ to haproxy/

This one is included almost everywhere and used to rely on a few other
.h that are not needed (unistd, stdlib, standard.h). It could possibly
make sense to split it into multiple parts to distinguish operations
performed on timers and the internal time accounting, but at this point
it does not appear much important.

5 years agoCLEANUP: thread: rename __decl_hathreads() to __decl_thread()
Willy Tarreau [Fri, 5 Jun 2020 06:40:51 +0000 (08:40 +0200)] 
CLEANUP: thread: rename __decl_hathreads() to __decl_thread()

I can never figure whether it takes an "s" or not, and in the end it's
better if it matches the file's naming, so let's call it "__decl_thread".

5 years agoREORG: include: split hathreads into haproxy/thread.h and haproxy/thread-t.h
Willy Tarreau [Thu, 28 May 2020 13:29:19 +0000 (15:29 +0200)] 
REORG: include: split hathreads into haproxy/thread.h and haproxy/thread-t.h

This splits the hathreads.h file into types+macros and functions. Given
that most users of this file used to include it only to get the definition
of THREAD_LOCAL and MAXTHREADS, the bare minimum was placed into thread-t.h
(i.e. types and macros).

All the thread management was left to haproxy/thread.h. It's worth noting
the drop of the trailing "s" in the name, to remove the permanent confusion
that arises between this one and the system implementation (no "s") and the
makefile's option (no "s").

For consistency, src/hathreads.c was also renamed thread.c.

A number of files were updated to only include thread-t which is the one
they really needed.

Some future improvements are possible like replacing empty inlined
functions with macros for the thread-less case, as building at -O0 disables
inlining and causes these ones to be emitted. But this really is cosmetic.

5 years agoCLEANUP: threads: remove a few needless includes of hathreads.h
Willy Tarreau [Thu, 28 May 2020 15:00:53 +0000 (17:00 +0200)] 
CLEANUP: threads: remove a few needless includes of hathreads.h

A few files were including it while not needing it (anymore). Some
only required access to the atomic ops and got haproxy/atomic.h in
exchange. Others didn't need it at all. A significant number of
files still include it only for THREAD_LOCAL definition.

5 years agoREORG: threads: extract atomic ops from hathreads.h
Willy Tarreau [Thu, 28 May 2020 13:29:33 +0000 (15:29 +0200)] 
REORG: threads: extract atomic ops from hathreads.h

The hathreads.h file has quickly become a total mess because it contains
thread definitions, atomic operations and locking operations, all this for
multiple combinations of threads, debugging and architectures, and all this
done with random ordering!

This first patch extracts all the atomic ops code from hathreads.h to move
it to haproxy/atomic.h. The code there still contains several sections
based on non-thread vs thread, and GCC versions in the latter case. Each
section was arranged in the exact same order to ease finding.

The redundant HA_BARRIER() which was the same as __ha_compiler_barrier()
was dropped in favor of the latter which follows the naming convention
of all other barriers. It was only used in freq_ctr.c which was updated.
Additionally, __ha_compiler_barrier() was defined inconditionally but
used only for thread-related operations, so it was made thread-only like
HA_BARRIER() used to be. We'd still need to have two types of compiler
barriers, one for the general case (e.g. signals) and another one for
concurrency, but this was not addressed here.

Some comments were added at the beginning of each section to inform about
the use case and warn about the traps to avoid.

Some files which continue to include hathreads.h solely for atomic ops
should now be updated.

5 years agoREORG: include: split mini-clist into haproxy/list and list-t.h
Willy Tarreau [Wed, 27 May 2020 16:01:47 +0000 (18:01 +0200)] 
REORG: include: split mini-clist into haproxy/list and list-t.h

Half of the users of this include only need the type definitions and
not the manipulation macros nor the inline functions. Moves the various
types into mini-clist-t.h makes the files cleaner. The other one had all
its includes grouped at the top. A few files continued to reference it
without using it and were cleaned.

In addition it was about time that we'd rename that file, it's not
"mini" anymore and contains a bit more than just circular lists.

5 years agoREORG: include: move istbuf.h to haproxy/
Willy Tarreau [Wed, 27 May 2020 15:32:53 +0000 (17:32 +0200)] 
REORG: include: move istbuf.h to haproxy/

This one now relies on two files that were already cleaned up and is
only used by buffer.h.

5 years agoREORG: include: split buf.h into haproxy/buf-t.h and haproxy/buf.h
Willy Tarreau [Wed, 27 May 2020 15:22:10 +0000 (17:22 +0200)] 
REORG: include: split buf.h into haproxy/buf-t.h and haproxy/buf.h

File buf.h is one common cause of pain in the dependencies. Many files in
the code need it to get the struct buffer definition, and a few also need
the inlined functions to manipulate a buffer, but the file used to depend
on a long chain only for BUG_ON() (addressed by last commit).

Now buf.h is split into buf-t.h which only contains the type definitions,
and buf.h for all inlined functions. Callers who don't care can continue
to use buf.h but files in types/ must only use buf-t.h. sys/types.h had
to be added to buf.h to get ssize_t as used by b_move(). It's worth noting
that ssize_t is only supposed to be a size_t supporting -1, so b_move()
ought to be rethought regarding this.

The files were moved to haproxy/ and all their users were updated
accordingly. A dependency issue was addressed on fcgi whose C file didn't
include buf.h.

5 years agoCLEANUP: debug: drop unused function p_malloc()
Willy Tarreau [Wed, 27 May 2020 15:10:05 +0000 (17:10 +0200)] 
CLEANUP: debug: drop unused function p_malloc()

This one was introduced 5 years ago for debugging and never really used.
It is the one which used to cause circular dependencies issues. Let's drop
it instead of starting to split the debug include in two.

5 years agoREORG: include: move debug.h from common/ to haproxy/
Willy Tarreau [Wed, 27 May 2020 14:58:08 +0000 (16:58 +0200)] 
REORG: include: move debug.h from common/ to haproxy/

The debug file is cleaner now and does not depend on much anymore.

5 years agoREORG: include: move the BUG_ON() code to haproxy/bug.h
Willy Tarreau [Wed, 27 May 2020 14:51:33 +0000 (16:51 +0200)] 
REORG: include: move the BUG_ON() code to haproxy/bug.h

This one used to be stored into debug.h but the debug tools got larger
and require a lot of other includes, which can't use BUG_ON() anymore
because of this. It does not make sense and instead this macro should
be placed into the lower includes and given its omnipresence, the best
solution is to create a new bug.h with the few surrounding macros needed
to trigger bugs and place assertions anywhere.

Another benefit is that it won't be required to add include <debug.h>
anymore to use BUG_ON, it will automatically be covered by api.h. No
less than 32 occurrences were dropped.

The FSM_PRINTF macro was dropped since not used at all anymore (probably
since 1.6 or so).

5 years agoREORG: include: move ist.h from common/ to import/
Willy Tarreau [Wed, 27 May 2020 14:21:26 +0000 (16:21 +0200)] 
REORG: include: move ist.h from common/ to import/

Fortunately that file wasn't made dependent upon haproxy since it was
integrated, better isolate it before it's too late. Its dependency on
api.h was the result of the change from config.h, which in turn wasn't
correct. It was changed back to stddef.h for size_t and sys/types.h for
ssize_t. The recently added reference to MAX() was changed as it was
placed only to avoid a zero length in the non-free-standing version and
was causing a build warning in the hpack encoder.

5 years agoREORG: include: move openssl-compat.h from common/ to haproxy/
Willy Tarreau [Wed, 27 May 2020 14:26:00 +0000 (16:26 +0200)] 
REORG: include: move openssl-compat.h from common/ to haproxy/

This file is to openssl what compat.h is to the libc, so it makes sense
to move it to haproxy/. It could almost be part of api.h but given the
amount of openssl stuff that gets loaded I fear it could increase the
build time.

Note that this file contains lots of inlined functions. But since it
does not depend on anything else in haproxy, it remains safe to keep
all that together.

5 years agoREORG: include: move base64.h, errors.h and hash.h from common to to haproxy/
Willy Tarreau [Wed, 27 May 2020 14:10:29 +0000 (16:10 +0200)] 
REORG: include: move base64.h, errors.h and hash.h from common to to haproxy/

These ones do not depend on any other file. One used to include
haproxy/api.h but that was solely for stddef.h.

5 years agoREORG: include: move version.h to haproxy/
Willy Tarreau [Wed, 27 May 2020 13:59:00 +0000 (15:59 +0200)] 
REORG: include: move version.h to haproxy/

Few files were affected. The release scripts was updated.

5 years agoREORG: include: move the base files from common/ to haproxy/
Willy Tarreau [Wed, 27 May 2020 12:38:20 +0000 (14:38 +0200)] 
REORG: include: move the base files from common/ to haproxy/

The files currently covered by api-t.h and api.h (compat, compiler,
defaults, initcall) are now located inside haproxy/.

5 years agoCLEANUP: include: remove unused common/tools.h
Willy Tarreau [Wed, 3 Jun 2020 15:50:30 +0000 (17:50 +0200)] 
CLEANUP: include: remove unused common/tools.h

Let's definitely get rid of this old file.

5 years agoREORG: include: move SWAP/MID_RANGE/MAX_RANGE from tools.h to standard.h
Willy Tarreau [Wed, 3 Jun 2020 15:49:00 +0000 (17:49 +0200)] 
REORG: include: move SWAP/MID_RANGE/MAX_RANGE from tools.h to standard.h

Tools.h doesn't make sense for these 3 macros alone anymore, let's move
them to standard.h which will ultimately become again tools.h once moved.

5 years agoREORG: include: move MIN/MAX from tools.h to compat.h
Willy Tarreau [Wed, 3 Jun 2020 15:47:08 +0000 (17:47 +0200)] 
REORG: include: move MIN/MAX from tools.h to compat.h

Given that these macros are usually provided by sys/param.h, better move
them to compat.h.

5 years agoCLEANUP: include: remove unused template.h
Willy Tarreau [Wed, 27 May 2020 14:01:15 +0000 (16:01 +0200)] 
CLEANUP: include: remove unused template.h

There is one "template.h" per include subdirectory to show how to create
a new file but in practice nobody knows they're here so they're useless.
Let's simply remove them.

5 years agoCLEANUP: include: remove common/config.h
Willy Tarreau [Wed, 27 May 2020 12:30:40 +0000 (14:30 +0200)] 
CLEANUP: include: remove common/config.h

It was already an indirection to load other files, it's not used
anymore.

5 years agoREORG: include: update all files to use haproxy/api.h or api-t.h if needed
Willy Tarreau [Wed, 27 May 2020 10:58:42 +0000 (12:58 +0200)] 
REORG: include: update all files to use haproxy/api.h or api-t.h if needed

All files that were including one of the following include files have
been updated to only include haproxy/api.h or haproxy/api-t.h once instead:

  - common/config.h
  - common/compat.h
  - common/compiler.h
  - common/defaults.h
  - common/initcall.h
  - common/tools.h

The choice is simple: if the file only requires type definitions, it includes
api-t.h, otherwise it includes the full api.h.

In addition, in these files, explicit includes for inttypes.h and limits.h
were dropped since these are now covered by api.h and api-t.h.

No other change was performed, given that this patch is large and
affects 201 files. At least one (tools.h) was already freestanding and
didn't get the new one added.

5 years agoREORG: include: create new file haproxy/api.h
Willy Tarreau [Wed, 27 May 2020 10:00:13 +0000 (12:00 +0200)] 
REORG: include: create new file haproxy/api.h

This file includes everything that must be guaranteed to be available to
any buildable file in the project (including the contrib/ subdirs). For
now it includes <haproxy/api-t.h> so that standard integer types and
compiler macros are known, <common/initcall.h> to ease dynamic registration
of init functions, and <common/tools.h> for a few MIN/MAX macros.

version.h should probably also be added, though at the moment it doesn't
bring a great value.

All files which currently include the ones above should now switch to
haproxy/api.h or haproxy/api-t.h instead. This should also reduce build
time by having a single guard for several files at once.

5 years agoREORG: include: create new file haproxy/api-t.h
Willy Tarreau [Wed, 3 Jun 2020 14:31:41 +0000 (16:31 +0200)] 
REORG: include: create new file haproxy/api-t.h

This file is at the lowest level of the include tree. Its purpose is to
make sure that common types are known pretty much everywhere, particularly
in structure declarations. It will essentially cover integer types such as
uintXX_t via inttypes.h, "size_t" and "ptrdiff_t" via stddef.h, and various
type modifiers such as __maybe_unused or ALIGN() via compiler.h, compat.h
and defaults.h.

It could be enhanced later if required, for example if some macros used
to compute array sizes are needed.

5 years agoREORG: ebtree: clean up remains of the ebtree/ directory
Willy Tarreau [Wed, 27 May 2020 09:05:33 +0000 (11:05 +0200)] 
REORG: ebtree: clean up remains of the ebtree/ directory

The only leftovers were the unused compiler.h file and the LICENSE file
which is already mentioned in each and every ebtree file header.

A few build paths were updated in the contrib/ directory not to mention
this directory anymore, and all its occurrences were dropped from the
main makefile. From now on no other include path but include/ will be
needed anymore to build any file.

5 years agoREORG: ebtree: move the include files from ebtree to include/import/
Willy Tarreau [Wed, 27 May 2020 08:58:19 +0000 (10:58 +0200)] 
REORG: ebtree: move the include files from ebtree to include/import/

This is where other imported components are located. All files which
used to directly include ebtree were touched to update their include
path so that "import/" is now prefixed before the ebtree-related files.

The ebtree.h file was slightly adjusted to read compiler.h from the
common/ subdirectory (this is the only change).

A build issue was encountered when eb32sctree.h is loaded before
eb32tree.h because only the former checks for the latter before
defining type u32. This was addressed by adding the reverse ifdef
in eb32tree.h.

No further cleanup was done yet in order to keep changes minimal.

5 years agoREORG: ebtree: move the C files from ebtree/ to src/
Willy Tarreau [Wed, 27 May 2020 08:43:24 +0000 (10:43 +0200)] 
REORG: ebtree: move the C files from ebtree/ to src/

As part of the include files cleanup, we're going to kill the ebtree
directory. For this we need to host its C files in a different location
and src/ is the right one.

5 years agoMINOR: sample: Add secure_memcmp converter
Tim Duesterhus [Tue, 9 Jun 2020 09:48:42 +0000 (11:48 +0200)] 
MINOR: sample: Add secure_memcmp converter

secure_memcmp compares two binary strings in constant time. It's only
available when haproxy is compiled with USE_OPENSSL.

5 years agoBUG/MINOR: mworker: fix a memleak when execvp() failed
William Lallemand [Mon, 8 Jun 2020 08:01:13 +0000 (10:01 +0200)] 
BUG/MINOR: mworker: fix a memleak when execvp() failed

Free next_argv when execvp() failed.

Must be backported as far as 1.8.

Should fix issue #668.

5 years agoBUG/MINOR: ssl: fix a trash buffer leak in some error cases
William Lallemand [Mon, 8 Jun 2020 07:40:37 +0000 (09:40 +0200)] 
BUG/MINOR: ssl: fix a trash buffer leak in some error cases

Fix a trash buffer leak when we can't take the lock of the ckch, or when
"set ssl cert" is wrongly used.

The bug was mentionned in this thread:
https://www.mail-archive.com/haproxy@formilux.org/msg37539.html

The bug was introduced by commit bc6ca7c ("MINOR: ssl/cli: rework 'set
ssl cert' as 'set/commit'").

Must be backported in 2.1.

5 years agoBUG/MEDIUM: mworker: fix the reload with an -- option
William Lallemand [Fri, 5 Jun 2020 12:08:41 +0000 (14:08 +0200)] 
BUG/MEDIUM: mworker: fix the reload with an -- option

When HAProxy is started with a '--' option, all following parameters are
considered configuration files. You can't add new options after a '--'.

The current reload system of the master-worker adds extra options at the
end of the arguments list. Which is a problem if HAProxy was started wih
'--'.

This patch fixes the issue by copying the new option at the beginning of
the arguments list instead of the end.

This patch must be backported as far as 1.8.

5 years agoBUG/MINOR: init: -S can have a parameter starting with a dash
William Lallemand [Thu, 4 Jun 2020 21:49:20 +0000 (23:49 +0200)] 
BUG/MINOR: init: -S can have a parameter starting with a dash

There is no reason the -S option can't take an argument which starts with
a -. This limitation must only be used for options that take a
non-finite list of parameters (-sf/-st)

This can be backported only if the previous patch which fixes
copy_argv() is backported too.

Could be backported as far as 1.9.

5 years agoBUG/MINOR: init: -x can have a parameter starting with a dash
William Lallemand [Thu, 4 Jun 2020 21:41:29 +0000 (23:41 +0200)] 
BUG/MINOR: init: -x can have a parameter starting with a dash

There is no reason the -x option can't take an argument which starts with
a -. This limitation must only be used for options that take a
non-finite list of parameters (-sf/-st)

This can be backported only if the previous patch which fixes
copy_argv() is backported too.

Could be backported as far as 1.8.

5 years agoBUG/MEDIUM: mworker: fix the copy of options in copy_argv()
William Lallemand [Thu, 4 Jun 2020 15:40:23 +0000 (17:40 +0200)] 
BUG/MEDIUM: mworker: fix the copy of options in copy_argv()

The copy_argv() function, which is used to copy and remove some of the
arguments of the command line in order to re-exec() the master process,
is poorly implemented.

The function tries to remove the -x and the -sf/-st options but without
taking into account that some of the options could take a parameter
starting with a dash.

In issue #644, haproxy starts with "-L -xfoo" which is perfectly
correct. However, the re-exec is done without "-xfoo" because the master
tries to remove the "-x" option. Indeed, the copy_argv() function does
not know how much arguments an option can have, and just assume that
everything starting with a dash is an option. So haproxy is exec() with
"-L" but without a parameter, which is wrong and leads to the exit of
the master, with usage().

To fix this issue, copy_argv() must know how much parameters an option
takes, and copy or skip the parameters correctly.

This fix is a first step but it should evolve to a cleaner way of
declaring the options to avoid deduplication of the parsing code, so we
avoid new bugs.

Should be backported with care as far as 1.8, by removing the options
that does not exists in the previous versions.

5 years agoBUG/MEDIUM: contrib/prometheus-exporter: Properly set flags to dump metrics
Christopher Faulet [Fri, 5 Jun 2020 06:18:56 +0000 (08:18 +0200)] 
BUG/MEDIUM: contrib/prometheus-exporter: Properly set flags to dump metrics

When the metrics are dumped, in the main function promex_dump_metrics(), the
appctx flags are set before entering in a new scope, among others things to know
which metrics names and descriptions to use. But, those flags are not restored
when the dump is interrupted because of a full output buffer. If this happens
after the dump of global metrics, it may only lead to extra #TYPE and #HELP
lines. But if this happens during the dump of global metrics, the following
dumps of frontends, backends and servers metrics use names and descriptions of
global ones with the unmatching indexes. This first leads to unexisting metrics
names. For instance, "haproxy_frontend_nbproc". But also to out-of-bound
accesses to name and description arrays because there are more stats fields than
info fields.

It is easy to reproduce the bug using small buffers, setting tune.bufsize to
8192 for instance.

This patch should fix the issue #666. It must be backported as far as 2.0.

5 years agoBUG/MINOR: checks: Fix test on http-check rulesets during config validity check
Christopher Faulet [Wed, 3 Jun 2020 17:00:42 +0000 (19:00 +0200)] 
BUG/MINOR: checks: Fix test on http-check rulesets during config validity check

When checking the config validity of the http-check rulesets, the test on the
ruleset type is inverted. So a warning about ignored directive is emitted when
the config is valid and omitted when it should be reported.

No backport needed.

5 years agoBUG/MEDIUM: hlua: Lock pattern references to perform set/add/del operations
Christopher Faulet [Wed, 3 Jun 2020 16:39:16 +0000 (18:39 +0200)] 
BUG/MEDIUM: hlua: Lock pattern references to perform set/add/del operations

The pattern references lock must be hold to perform set/add/del
operations. Unfortunately, it is not true for the lua functions manipulating acl
and map files.

This patch should fix the issue #664. It must be backported as far as 1.8.

5 years agoDOC: add a line about comments in crt-list
William Lallemand [Wed, 3 Jun 2020 15:34:48 +0000 (17:34 +0200)] 
DOC: add a line about comments in crt-list

Add a line about comments in crt-list.

Fix issue #514.

5 years agoBUG/MEDIUM: lua: Reset analyse expiration timeout before executing a lua action
Christopher Faulet [Tue, 2 Jun 2020 16:46:07 +0000 (18:46 +0200)] 
BUG/MEDIUM: lua: Reset analyse expiration timeout before executing a lua action

Before executing a lua action, the analyse expiration timeout of the
corresponding channel must be reset. Otherwise, when it expires, for instance
because of a call to core.msleep(), if the action yields, an expired timeout
will be used for the stream's task, leading to a loop.

This patch should fix the issue #661. It must be backported in all versions
supporting the lua.

5 years agoMINOR: mux-h1/proxy: Add a proxy option to disable clear h2 upgrade
Christopher Faulet [Tue, 2 Jun 2020 15:33:56 +0000 (17:33 +0200)] 
MINOR: mux-h1/proxy: Add a proxy option to disable clear h2 upgrade

By default, HAProxy is able to implicitly upgrade an H1 client connection to an
H2 connection if the first request it receives from a given HTTP connection
matches the HTTP/2 connection preface. This way, it is possible to support H1
and H2 clients on a non-SSL connections. It could be a problem if for any
reason, the H2 upgrade is not acceptable. "option disable-h2-upgrade" may now be
used to disable it, per proxy. The main puprose of this option is to let an
admin to totally disable the H2 support for security reasons. Recently, a
critical issue in the HPACK decoder was fixed, forcing everyone to upgrade their
HAProxy version to fix the bug. It is possible to disable H2 for SSL
connections, but not on clear ones. This option would have been a viable
workaround.

5 years agoRevert "MINOR: ssl: rework add cert chain to CTX to be libssl independent"
William Lallemand [Tue, 2 Jun 2020 16:27:20 +0000 (18:27 +0200)] 
Revert "MINOR: ssl: rework add cert chain to CTX to be libssl independent"

This reverts commit 4fed93eb725b513dd3b2029daa888311db110851.

This commit was simplifying the certificate chain loading with
SSL_CTX_add_extra_chain_cert() which is available in all SSL libraries.
Unfortunately this function is not compatible with the
multi-certificates bundles, which have the effect of concatenating the
chains of all certificate types instead of creating a chain for each
type (RSA, ECDSA etc.)

Should fix issue #655.

5 years agoCLEANUP: regex: remove outdated support for regex actions
Willy Tarreau [Tue, 2 Jun 2020 15:17:13 +0000 (17:17 +0200)] 
CLEANUP: regex: remove outdated support for regex actions

The support for reqrep and friends was removed in 2.1 but the
chain_regex() function and the "action" field in the regex struct
was still there. This patch removes them.

One point worth mentioning though. There is a check_replace_string()
function whose purpose was to validate the replacement strings passed
to reqrep. It should also be used for other replacement regex, but is
never called. Callers of exp_replace() should be checked and a call to
this function should be added to detect the error early.

5 years agoBUG/MINOR: peers: fix internal/network key type mapping.
Emeric Brun [Tue, 2 Jun 2020 09:17:42 +0000 (11:17 +0200)] 
BUG/MINOR: peers: fix internal/network key type mapping.

Network types were directly and mistakenly mapped on sample types:

This patch fix the doc with values effectively used to keep backward
compatiblitiy on existing implementations.

In addition it adds an internal/network mapping for key types to avoid
further mistakes adding or modifying internals types.

This patch should be backported on all maintained branches,
particularly until v1.8 included for documentation part.

5 years agoBUILD: sink: address build warning on 32-bit architectures
Willy Tarreau [Tue, 2 Jun 2020 10:00:46 +0000 (12:00 +0200)] 
BUILD: sink: address build warning on 32-bit architectures

The recently added ring section post-processing added this bening
warning on 32-bit archs:

  src/sink.c: In function 'cfg_post_parse_ring':
  src/sink.c:994:15: warning: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'size_t {aka unsigned int}' [-Wformat=]
      ha_warning("ring '%s' event max length '%u' exceeds size, forced to size '%lu'.\n",
               ^
Let's just cast b_size() to unsigned long here.

5 years agoCLEANUP: ssl: remove comment from dump_crtlist_sslconf()
William Lallemand [Tue, 2 Jun 2020 09:54:46 +0000 (11:54 +0200)] 
CLEANUP: ssl: remove comment from dump_crtlist_sslconf()

Since 8177ad9 ("MINOR: ssl: split config and runtime variable for
ssl-{min,max}-ver"), the dump for ssl-min-ver and ssl-max-ver is fixed,
so we can remove the comment.

5 years agoMINOR: ssl: set ssl-min-ver in ambiguous configurations
William Lallemand [Tue, 2 Jun 2020 08:52:24 +0000 (10:52 +0200)] 
MINOR: ssl: set ssl-min-ver in ambiguous configurations

Using ssl-max-ver without ssl-min-ver is ambiguous.

When the ssl-min-ver is not configured, and ssl-max-ver is set to a
value lower than the default ssl-min-ver (which is TLSv1.2 currently),
set the ssl-min-ver to the value of ssl-max-ver, and emit a warning.

5 years agoMEDIUM: ring: add new srv statement to support octet counting forward
Emeric Brun [Fri, 29 May 2020 23:42:45 +0000 (01:42 +0200)] 
MEDIUM: ring: add new srv statement to support octet counting forward

log-proto <logproto>
  The "log-proto" specifies the protocol used to forward event messages to
  a server configured in a ring section. Possible values are "legacy"
  and "octet-count" corresponding respectively to "Non-transparent-framing"
  and "Octet counting" in rfc6587. "legacy" is the default.

Notes: a separated io_handler was created to avoid per messages test
and to prepare code to set different log protocols such as
request- response based ones.

5 years agoMEDIUM: ring: add server statement to forward messages from a ring
Emeric Brun [Thu, 28 May 2020 09:13:15 +0000 (11:13 +0200)] 
MEDIUM: ring: add server statement to forward messages from a ring

This patch adds new statement "server" into ring section, and the
related "timeout connect" and "timeout server".

server <name> <address> [param*]
  Used to configure a syslog tcp server to forward messages from ring buffer.
  This supports for all "server" parameters found in 5.2 paragraph.
  Some of these parameters are irrelevant for "ring" sections.

timeout connect <timeout>
  Set the maximum time to wait for a connection attempt to a server to succeed.

  Arguments :
    <timeout> is the timeout value specified in milliseconds by default, but
              can be in any other unit if the number is suffixed by the unit,
              as explained at the top of this document.

timeout server <timeout>
  Set the maximum time for pending data staying into output buffer.

  Arguments :
    <timeout> is the timeout value specified in milliseconds by default, but
              can be in any other unit if the number is suffixed by the unit,
              as explained at the top of this document.

  Example:
    global
        log ring@myring local7

    ring myring
        description "My local buffer"
        format rfc3164
        maxlen 1200
        size 32764
        timeout connect 5s
        timeout server 10s
        server mysyslogsrv 127.0.0.1:6514

5 years agoBUG/MINOR: error on unknown statement in ring section.
Emeric Brun [Fri, 29 May 2020 13:47:52 +0000 (15:47 +0200)] 
BUG/MINOR: error on unknown statement in ring section.

Previously unknown statements were silently ignored.

5 years agoMINOR: ring: re-work ring attach generic API.
Emeric Brun [Thu, 28 May 2020 12:39:30 +0000 (14:39 +0200)] 
MINOR: ring: re-work ring attach generic API.

Attach is now independent on appctx, which was unused anyway.

5 years agoSCRIPTS: publish-release: pass -n to gzip to remove timestamp
Willy Tarreau [Sat, 30 May 2020 04:59:07 +0000 (06:59 +0200)] 
SCRIPTS: publish-release: pass -n to gzip to remove timestamp

It just appeared that the tar.gz we put online are not reproducible
because a timestamp is put by default into the archive. Passing "-n"
to gzip is sufficient to remove this timestamp, so let's do it, and
also make the gzip command configurable for more flexibility. Now
issuing the commands multiple times finally results in the same
archives being produced.

This should be backported to supported stable branches.

5 years agoCLEANUP: pools: use the regular lock for the flush operation on lockless pools
Willy Tarreau [Fri, 29 May 2020 15:23:05 +0000 (17:23 +0200)] 
CLEANUP: pools: use the regular lock for the flush operation on lockless pools

Commit 04f5fe87d3d introduced an rwlock in the pools to deal with the risk
that pool_flush() dereferences an area being freed, and commit 899fb8abdcd
turned it into a spinlock. The pools already contain a spinlock in case of
locked pools, so let's use the same and simplify the code by removing ifdefs.

At this point I'm really suspecting that if pool_flush() would instead
rely on __pool_get_first() to pick entries from the pool, the concurrency
problem could never happen since only one user would get a given entry at
once, thus it could not be freed by another user. It's not certain this
would be faster however because of the number of atomic ops to retrieve
one entry compared to a locked batch.

5 years agoMEDIUM: ssl: use TLSv1.2 as the minimum default on bind lines
William Lallemand [Fri, 29 May 2020 06:54:33 +0000 (08:54 +0200)] 
MEDIUM: ssl: use TLSv1.2 as the minimum default on bind lines

Since HAProxy 1.8, the TLS default minimum version was set to TLSv1.0 to
avoid using the deprecated SSLv3.0. Since then, the standard changed and
the recommended TLS version is now TLSv1.2.

This patch changes the minimum default version to TLSv1.2 on bind lines.
If you need to use prior TLS version, this is still possible by
using the ssl-min-ver keyword.

5 years agoBUG/MEDIUM: checks: Don't add a tcpcheck ruleset twice in the shared tree
Christopher Faulet [Fri, 29 May 2020 06:10:50 +0000 (08:10 +0200)] 
BUG/MEDIUM: checks: Don't add a tcpcheck ruleset twice in the shared tree

When a tcpcheck ruleset is created, it is automatically inserted in a global
tree. Unfortunately for applicative health checks (redis, mysql...), the created
ruleset is inserted a second time during the directive parsing. The leads to a
infinite loop when haproxy is stopped when we try to scan the tree to release
all tcpcheck rulesets.

Now, only the function responsible to create the tcpcheck ruleset insert it into
the tree.

No backport needed.

5 years agoBUG/MINOR: nameservers: fix error handling in parsing of resolv.conf
Willy Tarreau [Thu, 28 May 2020 16:07:10 +0000 (18:07 +0200)] 
BUG/MINOR: nameservers: fix error handling in parsing of resolv.conf

In issue #657, Coverity found a bug in the "nameserver" parser for the
resolv.conf when "parse-resolv-conf" is set. What happens is that if an
unparsable address appears on a "nameserver" line, it will destroy the
previously allocated pointer before reporting the warning, then the next
"nameserver" line will dereference it again and wlil cause a crash. If
the faulty nameserver is the last one, it will only be a memory leak.
Let's just make sure we preserve the pointer when handling the error.
The patch also fixes a typo in the warning.

The bug was introduced in 1.9 with commit 44e609bfa ("MINOR: dns:
Implement `parse-resolv-conf` directive") so the fix needs to be backported
up to 1.9 or 2.0.

5 years agoCI: cirrus-ci: skip reg-tests/connection/proxy_protocol_send_unique_id_alpn.vtc on...
Ilya Shipitsin [Wed, 27 May 2020 20:50:57 +0000 (01:50 +0500)] 
CI: cirrus-ci: skip reg-tests/connection/proxy_protocol_send_unique_id_alpn.vtc on CentOS 6

as reg-tests/connection/proxy_protocol_send_unique_id_alpn.vtc requires ALPN, which is not
supported on CentOS 6, let us skip that test

5 years agoBUG/MEDIUM: logs: fix trailing zeros on log message.
Emeric Brun [Thu, 28 May 2020 12:21:33 +0000 (14:21 +0200)] 
BUG/MEDIUM: logs: fix trailing zeros on log message.

This patch removes all trailing LFs and Zeros from
log messages. Previously only the last LF was removed.

It's a regression from e8ea0ae6f6 "BUG/MINOR: logs:
prevent double line returns in some events."

This should fix github issue #654

5 years agoBUG/MINOR: lua: Add missing string length for lua sticktable lookup
Nathan Neulinger [Wed, 4 Mar 2020 02:32:47 +0000 (20:32 -0600)] 
BUG/MINOR: lua: Add missing string length for lua sticktable lookup

In hlua_stktable_lookup(), the key length is never set so all
stktable:lookup("key") calls return nil from lua.

This patch must be backported as far as 1.9.

[Cf: I slightly updated the patch to use lua_tolstring() instead of
     luaL_checkstring() + strlen()]

5 years agoMINOR: checks: I/O callback function only rely on the data layer wake callback
Christopher Faulet [Thu, 28 May 2020 12:34:02 +0000 (14:34 +0200)] 
MINOR: checks: I/O callback function only rely on the data layer wake callback

Most of code in event_srv_chk_io() function is inherited from the checks before
the recent refactoring. Now, it is enough to only call wake_srv_chk(). Since the
refactoring, the removed code is dead and never called. wake_srv_chk() may only
return 0 if tcpcheck_main() returns 0 and the check status is unknown
(CHK_RES_UNKNOWN). When this happens, nothing is performed in event_srv_chk_io().

5 years agoBUG/MEDIUM: checks: Don't blindly subscribe for receive if waiting for connect
Christopher Faulet [Thu, 28 May 2020 11:58:34 +0000 (13:58 +0200)] 
BUG/MEDIUM: checks: Don't blindly subscribe for receive if waiting for connect

When an health check is waiting for a connection establishment, it subscribe for
receive or send events, depending on the next rule to be evaluated. For
subscription for send events, there is no problem. It works as expected. For
subscription for receive events, It only works for HTTP checks because the
underlying multiplexer takes care to do a receive before subscribing again,
updating the fd state. For TCP checks, the PT multiplexer only forwards
subscriptions at the transport layer. So the check itself is woken up. This
leads to a subscribe/notify loop waiting the connection establishment or a
timeout, uselessly eating CPU.

Thus, when a check is waiting for a connect, instead of blindly resubscribe for
receive events when it is woken up, we now try to receive data.

This patch should fix the issue #635. No backport needed.

5 years agoCLEANUP: http: Remove unused HTTP message templates
Christopher Faulet [Wed, 27 May 2020 08:11:59 +0000 (10:11 +0200)] 
CLEANUP: http: Remove unused HTTP message templates

HTTP_1XX, HTTP_3XX and HTTP_4XX message templates are no longer used. Only
HTTP_302 and HTTP_303 are used during configuration parsing by "errorloc" family
directives. So these templates are removed from the generic http code. And
HTTP_302 and HTTP_303 templates are moved as static strings in the function
parsing "errorloc" directives.

5 years agoMINOR: http-rules: Use an action function to eval http-request auth rules
Christopher Faulet [Wed, 27 May 2020 13:26:43 +0000 (15:26 +0200)] 
MINOR: http-rules: Use an action function to eval http-request auth rules

Now http-request auth rules are evaluated in a dedicated function and no longer
handled "in place" during the HTTP rules evaluation. Thus the action name
ACT_HTTP_REQ_AUTH is removed. In additionn, http_reply_40x_unauthorized() is
also removed. This part is now handled in the new action_ptr callback function.

5 years agoMINOR: http-ana: Use proxy's error replies to emit 401/407 responses
Christopher Faulet [Wed, 27 May 2020 07:57:28 +0000 (09:57 +0200)] 
MINOR: http-ana: Use proxy's error replies to emit 401/407 responses

There is no reason to not use proxy's error replies to emit 401/407
responses. The function http_reply_40x_unauthorized(), responsible to emit those
responses, is not really complex. It only adds a
WWW-Authenticate/Proxy-Authenticate header to a generic message.

So now, error replies can be defined for 401 and 407 status codes, using
errorfile or http-error directives. When an http-request auth rule is evaluated,
the corresponding error reply is used. For 401 responses, all occurrences of the
WWW-Authenticate header are removed and replaced by a new one with a basic
authentication challenge for the configured realm. For 407 responses, the same
is done on the Proxy-Authenticate header. If the error reply must not be
altered, "http-request return" rule must be used instead.

5 years agoMINOR: http-ana: Make the function http_reply_to_htx() public
Christopher Faulet [Wed, 27 May 2020 13:24:22 +0000 (15:24 +0200)] 
MINOR: http-ana: Make the function http_reply_to_htx() public

This function may be used from anywhere to convert an HTTP reply to an HTX
message.

5 years agoREGTEST: Add connection/proxy_protocol_send_unique_id_alpn
Tim Duesterhus [Wed, 27 May 2020 10:58:43 +0000 (12:58 +0200)] 
REGTEST: Add connection/proxy_protocol_send_unique_id_alpn

This reg-test checks that sending unique IDs via PPv2 works for servers
with the `alpn` option specified (issue #640). As a side effect it also
checks that PPv2 works with ALPN (issue #651).

It has been verified that the test fails without the following commits
applied and succeeds with them applied.

   1f9a4ecea BUG/MEDIUM: backend: set the connection owner to the session when using alpn.
   083fd42d5 BUG/MEDIUM: connection: Ignore PP2 unique ID for stream-less connections
   eb9ba3cb2 BUG/MINOR: connection: Always get the stream when available to send PP2 line

Without the first two commits HAProxy crashes during execution of the
test. Without the last commit the test will fail, because no unique ID
is received.

5 years agoMEDIUM: pools: directly free objects when pools are too much crowded
Willy Tarreau [Fri, 8 May 2020 06:38:24 +0000 (08:38 +0200)] 
MEDIUM: pools: directly free objects when pools are too much crowded

During pool_free(), when the ->allocated value is 125% of needed_avg or
more, instead of putting the object back into the pool, it's immediately
freed using free(). By doing this we manage to significantly reduce the
amount of memory pinned in pools after transient traffic spikes.

During a test involving a constant load of 100 concurrent connections
each delivering 100 requests per second, the memory usage was a steady
21 MB RSS. Adding a 1 minute parallel load of 40k connections all looping
on 100kB objects made the memory usage climb to 938 MB before this patch.
With the patch it was only 660 MB. But when this parasit load stopped,
before the patch the RSS would remain at 938 MB while with the patch,
it went down to 480 then 180 MB after a few seconds, to stabilize around
69 MB after about 20 seconds.

This can be particularly important to improve reloads where the memory
has to be shared between the old and new process.

Another improvement would be welcome, we ought to have a periodic task
to check pools usage and continue to free up unused objects regardless
of any call to pool_free(), because the needed_avg value depends on the
past and will not cover recently refilled objects.

5 years agoMINOR: pools: compute an estimate of each pool's average needed objects
Willy Tarreau [Fri, 8 May 2020 06:31:56 +0000 (08:31 +0200)] 
MINOR: pools: compute an estimate of each pool's average needed objects

This adds a sliding estimate of the pools' usage. The goal is to be able
to use this to start to more aggressively free memory instead of keeping
lots of unused objects in pools. The average is calculated as a sliding
average over the last 1024 consecutive measures of ->used during calls to
pool_free(), and is bumped up for 1/4 of its history from ->allocated when
allocation from the pool fails and results in a call to malloc().

The result is a floating value between ->used and ->allocated, that tries
to react fast to under-estimates that result in expensive malloc() but
still maintains itself well in case of stable usage, and progressively
goes down if usage shrinks over time.

This new metric is reported as "needed_avg" in "show pools".

Sadly due to yet another include dependency hell, we couldn't reuse the
functions from freq_ctr.h so they were temporarily duplicated into memory.h.

5 years agoBUG/MEDIUM: backend: set the connection owner to the session when using alpn.
Olivier Houchard [Tue, 26 May 2020 23:26:06 +0000 (01:26 +0200)] 
BUG/MEDIUM: backend: set the connection owner to the session when using alpn.

In connect_server(), if we can't create the mux immediately because we have
to wait until the alpn is negociated, store the session as the connection's
owner. conn_create_mux() expects it to be set, and provides it to the mux
init() method. Failure to do so will result to crashes later if the
connection is private, and even if we didn't do so it would prevent connection
reuse for private connections.
This should fix github issue #651.

5 years agoBUG/MINOR: connection: Always get the stream when available to send PP2 line
Christopher Faulet [Tue, 26 May 2020 14:08:49 +0000 (16:08 +0200)] 
BUG/MINOR: connection: Always get the stream when available to send PP2 line

When a PROXY protocol line must be sent, it is important to always get the
stream if it exists. It is mandatory to send an unique ID when the unique-id
option is enabled. In conn_si_send_proxy(), to get the stream, we first retrieve
the conn-stream attached to the backend connection. Then if the conn-stream data
callback is si_conn_cb, it is possible to get the stream. But for now, it only
works for connections with a multiplexer. Thus, for mux-less connections, the
unique ID is never sent. This happens for all SSL connections relying on the
alpn to choose the good multiplexer. But it is possible to use the context of
such connections to get the conn-stream.

The bug was introduced by the commit cf6e0c8a8 ("MEDIUM: proxy_protocol: Support
sending unique IDs using PPv2"). Thus, this patch must be backported to the same
versions as the commit above.

5 years agoBUG/MEDIUM: connection: Ignore PP2 unique ID for stream-less connections
Christopher Faulet [Tue, 26 May 2020 13:16:01 +0000 (15:16 +0200)] 
BUG/MEDIUM: connection: Ignore PP2 unique ID for stream-less connections

It is possible to send a unique ID when the PROXY protocol v2 is used. It relies
on the stream to do so. So we must be sure to have a stream. Locally initiated
connections may not be linked to a stream. For instance, outgoing connections
created by health checks have no stream. Moreover, the stream is not retrieved
for mux-less connections (this bug will be fixed in another commit).

Unfortunately, in make_proxy_line_v2() function, the stream is not tested before
generating the unique-id.  This bug leads to a segfault when a health check is
performed for a server with the PROXY protocol v2 and the unique-id option
enabled. It also crashes for servers using SSL connections with alpn. The bug
was introduced by the commit cf6e0c8a8 ("MEDIUM: proxy_protocol: Support sending
unique IDs using PPv2")

This patch should fix the issue #640. It must be backported to the same versions
as the commit above.

5 years agoBUG/MEDIUM: contrib/spoa: do not register python3.8 if --embed fail
Bertrand Jacquin [Fri, 22 May 2020 16:03:46 +0000 (17:03 +0100)] 
BUG/MEDIUM: contrib/spoa: do not register python3.8 if --embed fail

spoa server fails to build when python3.8 is not available. If
python3-config --embed fails, the output of the command is registered in
check_python_config.  However when it's later used to define
PYTHON_DEFAULT_INC and PYTHON_DEFAULT_LIB it's content does not match
and fallback to python2.7

Content of check_python_config when building with python3.6:

  Usage: bin/python3-config --prefix|--exec-prefix|--includes|--libs|--cflags|--ldflags|--extension-suffix|--help|--abiflags|--configdir python3

As we are only looking for return code, this commit ensure we always
ignore the output of python3-config or hash commands.

5 years agoMINOR: checks: Remove useless tests on the connection and conn-stream
Christopher Faulet [Tue, 26 May 2020 10:14:22 +0000 (12:14 +0200)] 
MINOR: checks: Remove useless tests on the connection and conn-stream

During an health check execution, the conn-stream and the conncetion may only be
NULL before the evaluation of the first rule, which is always a connect, or if
the first conn-stream allocation failed. Thus, in tcpcheck_main(), useless tests
on the conn-stream or on the connection have been removed. A comment has been
added to make it clear.

No backport needed.

5 years agoBUG/MEDIUM: checks: Refresh the conn-stream and the connection after a connect
Christopher Faulet [Tue, 26 May 2020 09:14:50 +0000 (11:14 +0200)] 
BUG/MEDIUM: checks: Refresh the conn-stream and the connection after a connect

When a connect rule is evaluated, the conn-stream and the connection must be
refreshed in tcpcheck_main(). Otherwise, in case of synchronous connect, these
variables point on outdated values (NULL for the first connect or released
otherwise).

No backport needed.

5 years agoREGTESTS: Require the version 2.2 to execute lua/set_var
Christopher Faulet [Tue, 26 May 2020 09:11:23 +0000 (11:11 +0200)] 
REGTESTS: Require the version 2.2 to execute lua/set_var

This script depends on LUA changes introduced in HAProxy 2.2 and not backported.

5 years agoREGTESTS: Add missing OPENSSL to REQUIRE_OPTIONS for compression/lua_validation
Christopher Faulet [Tue, 26 May 2020 09:08:59 +0000 (11:08 +0200)] 
REGTESTS: Add missing OPENSSL to REQUIRE_OPTIONS for compression/lua_validation

The test uses the `ssl` keyword, add `OPENSSL` as a requirement.

Should be backported to all branches with that test.

5 years agoMEDIUM: ring: new section ring to declare custom ring buffers.
Emeric Brun [Mon, 25 May 2020 13:01:04 +0000 (15:01 +0200)] 
MEDIUM: ring: new section ring to declare custom ring buffers.

It is possible to globally declare ring-buffers, to be used as target for log
servers or traces.

ring <ringname>
  Creates a new ring-buffer with name <ringname>.

description <text>
  The descritpition is an optional description string of the ring. It will
  appear on CLI. By default, <name> is reused to fill this field.

format <format>
  Format used to store events into the ring buffer.

  Arguments:
    <format> is the log format used when generating syslog messages. It may be
             one of the following :

      iso     A message containing only the ISO date, followed by the text.
              The PID, process name and system name are omitted. This is
              designed to be used with a local log server.

      raw     A message containing only the text. The level, PID, date, time,
              process name and system name are omitted. This is designed to be
              used in containers or during development, where the severity
              only depends on the file descriptor used (stdout/stderr). This
              is the default.

      rfc3164 The RFC3164 syslog message format. This is the default.
              (https://tools.ietf.org/html/rfc3164)

      rfc5424 The RFC5424 syslog message format.
              (https://tools.ietf.org/html/rfc5424)

      short   A message containing only a level between angle brackets such as
              '<3>', followed by the text. The PID, date, time, process name
              and system name are omitted. This is designed to be used with a
              local log server. This format is compatible with what the systemd
              logger consumes.

      timed   A message containing only a level between angle brackets such as
              '<3>', followed by ISO date and by the text. The PID, process
              name and system name are omitted. This is designed to be
              used with a local log server.

maxlen <length>
  The maximum length of an event message stored into the ring,
  including formatted header. If an event message is longer than
  <length>, it will be truncated to this length.

size <size>
  This is the optional size in bytes for the ring-buffer. Default value is
  set to BUFSIZE.

  Example:
    global
        log ring@myring local7

    ring myring
        description "My local buffer"
        format rfc3164
        maxlen 1200

Note: ring names are resolved during post configuration processing.

5 years agoMEDIUM: lua: Add `ifexist` parameter to `set_var`
Tim Duesterhus [Tue, 19 May 2020 11:49:42 +0000 (13:49 +0200)] 
MEDIUM: lua: Add `ifexist` parameter to `set_var`

As discussed in GitHub issue #624 Lua scripts should not use
variables that are never going to be read, because the memory
for variable names is never going to be freed.

Add an optional `ifexist` parameter to the `set_var` function
that allows a Lua developer to set variables that are going to
be ignored if the variable name was not used elsewhere before.

Usually this mean that there is no `var()` sample fetch for the
variable in question within the configuration.

5 years agoMINOR: lua: Make `set_var()` and `unset_var()` return success
Tim Duesterhus [Tue, 19 May 2020 11:49:41 +0000 (13:49 +0200)] 
MINOR: lua: Make `set_var()` and `unset_var()` return success

This patch makes `set_var()` and `unset_var()` return a boolean indicating
success.

5 years agoMINOR: vars: Make vars_(un|)set_by_name(_ifexist|) return a success value
Tim Duesterhus [Tue, 19 May 2020 11:49:40 +0000 (13:49 +0200)] 
MINOR: vars: Make vars_(un|)set_by_name(_ifexist|) return a success value

Change the return type from `void` to `int` and return whether setting
the variable was successful.

5 years agoCLEANUP: vars: Remove void vars_unset_by_name(const char*, size_t, struct sample*)
Tim Duesterhus [Tue, 19 May 2020 11:49:39 +0000 (13:49 +0200)] 
CLEANUP: vars: Remove void vars_unset_by_name(const char*, size_t, struct sample*)

With "MINOR: lua: Use vars_unset_by_name_ifexist()" the last user was
removed and as outlined in that commit there is no good reason for this
function to exist.

May be backported together with the commit mentioned above.

5 years agoMINOR: lua: Use vars_unset_by_name_ifexist()
Tim Duesterhus [Tue, 19 May 2020 11:49:38 +0000 (13:49 +0200)] 
MINOR: lua: Use vars_unset_by_name_ifexist()

There is no good reason to register a variable name, just to unset
that value that could not even be set without the variable existing.

This change should be safe, may be backported if desired.

5 years agoREGTESTS: Add missing OPENSSL to REQUIRE_OPTIONS for lua/txn_get_priv
Tim Duesterhus [Tue, 19 May 2020 11:49:37 +0000 (13:49 +0200)] 
REGTESTS: Add missing OPENSSL to REQUIRE_OPTIONS for lua/txn_get_priv

The test uses the `ssl` keyword, add `OPENSSL` as a requirement.

Should be backported to all branches with that test.

5 years agoREGTESTS: checks: Fix tls_health_checks when IPv6 addresses are used
Christopher Faulet [Mon, 25 May 2020 05:59:59 +0000 (07:59 +0200)] 
REGTESTS: checks: Fix tls_health_checks when IPv6 addresses are used

In tls_health_checks.vtc, when IPv6 addresses are used, A config error is
reported because of the "addr" server parameter. Because there is no specified
port, the IPv6 address must be enclosed into brackets to be properly parsed. It
also works with IPv4 addresses. But instead, a dummy port is added to the addr
parameter. This way, we also check the port parameter, when specified, is used
in priority over the port found in the addr parameter.

This patch should fix the issue #646.

5 years agoMINOR: checks: Remove dead code from process_chk_conn()
Christopher Faulet [Mon, 25 May 2020 05:32:01 +0000 (07:32 +0200)] 
MINOR: checks: Remove dead code from process_chk_conn()

With the checks refactoring, all connections are performed into a tcp-check
ruleset. So, in process_chk_conn(), it is no longer required to check
synchronous error on the connect when an health check is started. This part is
now handled in tcpcheck_main(). And because it is impossible to start a health
check with a connection, all tests on the connection during the health check
startup can be safely removed.

This patch should fix the issue #636. No backport needed.

5 years agoBUG/MINOR: http-htx: Fix a leak on error path during http reply parsing
Christopher Faulet [Thu, 21 May 2020 08:10:41 +0000 (10:10 +0200)] 
BUG/MINOR: http-htx: Fix a leak on error path during http reply parsing

When "hdr" arguments of an http reply are parsed, the allocated header may leak
on error path. Adding it to the header list earlier fixes the issue.

This patch should partly fix the issue #645.

No backport needed.

5 years agoBUG/MINOR: http-htx: Don't forget to release the http reply in release function
Christopher Faulet [Thu, 21 May 2020 07:59:22 +0000 (09:59 +0200)] 
BUG/MINOR: http-htx: Don't forget to release the http reply in release function

The http reply must be released in the function responsible to release it. This
leak was introduced when the http return was refactored to use http reply.

This patch should partly fix the issue #645.

No backport needed.