]> git.ipfire.org Git - thirdparty/haproxy.git/log
thirdparty/haproxy.git
3 years agoBUG/MEDIUM: mux_h2: Handle others remaining read0 cases on partial frames
Christopher Faulet [Fri, 8 Oct 2021 06:56:00 +0000 (08:56 +0200)] 
BUG/MEDIUM: mux_h2: Handle others remaining read0 cases on partial frames

We've found others places where the read0 is ignored because of an
incomplete frame parsing. This time, it happens during parsing of
CONTINUATION frames.

When frames are parsed, incomplete frames are properly handled and
H2_CF_DEM_SHORT_READ flag is set. It is also true for HEADERS
frames. However, for CONTINUATION frames, there is an exception. Besides
parsing the current frame, we try to peek header of the next one to merge
payload of both frames, the current one and the next one. Idea is to create
a sole HEADERS frame before parsing the payload. However, in this case, it
is possible to have an incomplete frame too, not the current one but the
next one. From the demux point of view, the current frame is complete. We
must go to the internal function h2c_decode_headers() to detect an
incomplete frame. And this case was not identified and fixed when
H2_CF_DEM_SHORT_READ flag was introduced in the commit b5f7b5296
("BUG/MEDIUM: mux-h2: Handle remaining read0 cases on partial frames")

This bug was reported in a comment of the issue #1362. The patch must be
backported as far as 2.0.

3 years agoBUG/MAJOR: quic: remove qc from receiver cids tree on free
Amaury Denoyelle [Thu, 30 Sep 2021 09:03:28 +0000 (11:03 +0200)] 
BUG/MAJOR: quic: remove qc from receiver cids tree on free

Remove the quic_conn from the receiver connection_ids tree on
quic_conn_free. This fixes a crash due to dangling references in the
tree after a quic connection release.

This operation must be conducted under the listener lock. For this
reason, the quic_conn now contains a reference to its attached listener.

3 years agoMINOR: mux-quic: release connection if no more bidir streams
Amaury Denoyelle [Fri, 24 Sep 2021 08:05:30 +0000 (10:05 +0200)] 
MINOR: mux-quic: release connection if no more bidir streams

Use the count of bidirectional streams to call qc_release in qc_detach.
We cannot inspect the by_id tree because uni-streams are never removed
from it. This allows the connection to be properly freed.

3 years agoBUG/MAJOR: xprt-quic: do not queue qc timer if not set
Amaury Denoyelle [Tue, 5 Oct 2021 12:42:25 +0000 (14:42 +0200)] 
BUG/MAJOR: xprt-quic: do not queue qc timer if not set

Do not queue the pto/loss-detection timer if set to TICK_ETERNITY. This
usage is invalid with the scheduler and cause a BUG_ON trigger.

3 years agoBUG/MEDIUM: mux-quic: reinsert all streams in by_id tree
Amaury Denoyelle [Fri, 24 Sep 2021 08:03:16 +0000 (10:03 +0200)] 
BUG/MEDIUM: mux-quic: reinsert all streams in by_id tree

It is required that all qcs streams are in the by_id tree for the xprt
to function correctly. Without this, some ACKs are not properly emitted
by xprt.

Note that this change breaks the free of the connection because the
condition eb_is_empty in qc_detach is always true. This will be fixed in
a following patch.

3 years agoMINOR: quic: Fix SSL error issues (do not use ssl_bio_and_sess_init())
Frédéric Lécaille [Tue, 28 Sep 2021 07:51:23 +0000 (09:51 +0200)] 
MINOR: quic: Fix SSL error issues (do not use ssl_bio_and_sess_init())

It seems it was a bad idea to use the same function as for TCP ssl sockets
to initialize the SSL session objects for QUIC with ssl_bio_and_sess_init().
Indeed, this had as very bad side effects to generate SSL errors due
to the fact that such BIOs initialized for QUIC could not finally be controlled
via the BIO_ctrl*() API, especially BIO_ctrl() function used by very much other
internal OpenSSL functions (BIO_push(), BIO_pop() etc).
Others OpenSSL base QUIC implementation do not use at all BIOs to configure
QUIC connections. So, we decided to proceed the same way as ngtcp2 for instance:
only initialize an SSL object and call SSL_set_quic_method() to set its
underlying method. Note that calling this function silently disable this option:
SSL_OP_ENABLE_MIDDLEBOX_COMPAT.
We implement qc_ssl_sess_init() to initialize SSL sessions for QUIC connections
to do so with a retry in case of allocation failure as this is done by
ssl_bio_and_sess_init(). We also modify the code part for haproxy servers.

3 years agoMINOR: quic: BUG_ON() SSL errors.
Frédéric Lécaille [Tue, 28 Sep 2021 07:05:59 +0000 (09:05 +0200)] 
MINOR: quic: BUG_ON() SSL errors.

As this QUIC implementation is still experimental, let's BUG_ON()
very important SSL handshake errors.
Also dump the SSL errors before BUG_ON().

3 years agoMINOR: quic: Add a function to dump SSL stack errors
Frédéric Lécaille [Tue, 28 Sep 2021 07:04:12 +0000 (09:04 +0200)] 
MINOR: quic: Add a function to dump SSL stack errors

This has been very helpful to fix SSL related issues.

3 years agoMINOR: quic: Distinguish packet and SSL read enc. level in traces
Frédéric Lécaille [Thu, 23 Sep 2021 16:10:56 +0000 (18:10 +0200)] 
MINOR: quic: Distinguish packet and SSL read enc. level in traces

This is only to distinguish the encryption level of packet traces from
the TLS stack current read encryption level.

3 years agoMINOR: pools: report the amount used by thread caches in "show pools"
Willy Tarreau [Thu, 7 Oct 2021 14:29:31 +0000 (16:29 +0200)] 
MINOR: pools: report the amount used by thread caches in "show pools"

The "show pools" command provides some "allocated" and "used" estimates
on the pools objects, but this applies to the shared pool and the "used"
includes what is currently assigned to thread-local caches. It's possible
to know how much each thread uses, so let's dump the total size allocated
by thread caches as an estimate. It's only done when pools are enabled,
which explains why the patch adds quite a lot of ifdefs.

3 years agoBUG/MINOR: task: fix missing include with DEBUG_TASK
Amaury Denoyelle [Thu, 7 Oct 2021 14:37:42 +0000 (16:37 +0200)] 
BUG/MINOR: task: fix missing include with DEBUG_TASK

Following include reorganzation, there is some missing include files for
task.h when compiling with DEBUG_TASK :
- activity.h for task_profiling_mask
- time.h for now_mono_time()

This is present since the following commit
  d8b325c74826bdb02759f62c41a00455dbae3431
  REORG: task: uninline the loop time measurement code

No need to backport this.

3 years agoDOC: configuration: add clarification on escaping in keyword arguments
Thayne McCombs [Mon, 4 Oct 2021 07:02:58 +0000 (01:02 -0600)] 
DOC: configuration: add clarification on escaping in keyword arguments

Add a more precise description on how backslash escaping is different
than the top-level parser, and give examples of how to handle single
quotes inside arguments.

3 years agoCLEANUP: thread: uninline ha_tkill/ha_tkillall/ha_cpu_relax()
Willy Tarreau [Wed, 6 Oct 2021 21:33:20 +0000 (23:33 +0200)] 
CLEANUP: thread: uninline ha_tkill/ha_tkillall/ha_cpu_relax()

These ones are rarely used or only to waste CPU cycles waiting, and are
the last ones requiring system includes in thread.h. Let's uninline them
and move them to thread.c.

3 years agoMINOR: thread: use a dedicated static pthread_t array in thread.c
Willy Tarreau [Wed, 6 Oct 2021 20:53:51 +0000 (22:53 +0200)] 
MINOR: thread: use a dedicated static pthread_t array in thread.c

This removes the thread identifiers from struct thread_info and moves
them only in static array in thread.c since it's now the only file that
needs to touch it. It's also the only file that needs to include
pthread.h, beyond haproxy.c which needs it to start the poll loop. As
a result, much less system includes are needed and the LoC reduced by
around 3%.

3 years agoREORG: thread: move ha_get_pthread_id() to thread.c
Willy Tarreau [Wed, 6 Oct 2021 20:44:28 +0000 (22:44 +0200)] 
REORG: thread: move ha_get_pthread_id() to thread.c

It's the last function which directly accesses the pthread_t, let's move
it to thread.c and leave a static inline for non-thread.

3 years agoREORG: thread: move the thread init/affinity/stop to thread.c
Willy Tarreau [Wed, 6 Oct 2021 20:22:40 +0000 (22:22 +0200)] 
REORG: thread: move the thread init/affinity/stop to thread.c

haproxy.c still has to deal with pthread-specific low-level stuff that
is OS-dependent. We should not have to deal with this there, and we do
not need to access pthread anywhere else.

Let's move these 3 functions to thread.c and keep empty inline ones for
when threads are disabled.

3 years agoCLENAUP: wdt: use ha_tkill() instead of accessing pthread directly
Willy Tarreau [Wed, 6 Oct 2021 19:43:38 +0000 (21:43 +0200)] 
CLENAUP: wdt: use ha_tkill() instead of accessing pthread directly

Instead of calling pthread_kill() directly on the pthread_t let's
call ha_tkill() which does the same by itself. This will help isolate
pthread_t.

3 years agoREORG: fd: uninline compute_poll_timeout()
Willy Tarreau [Wed, 6 Oct 2021 17:55:29 +0000 (19:55 +0200)] 
REORG: fd: uninline compute_poll_timeout()

It's not needed to inline it at all (one call per loop) and it introduces
dependencies, let's move it to fd.c.

Removing the few remaining includes that came with it further reduced
by ~0.2% the LoC and the build time is now below 6s.

3 years agoCLEANUP: fd: do not include time.h
Willy Tarreau [Wed, 6 Oct 2021 17:44:15 +0000 (19:44 +0200)] 
CLEANUP: fd: do not include time.h

It's not needed at all here.

3 years agoCLEANUP: time: move a few configurable defines to defaults.h
Willy Tarreau [Wed, 6 Oct 2021 17:36:47 +0000 (19:36 +0200)] 
CLEANUP: time: move a few configurable defines to defaults.h

TV_ETERNITY, TV_ETERNITY_MS and MAX_DELAY_MS may be configured and
ought to be in defaults.h so that they can be inherited from everywhere
without including time.h and could also be redefined if neede
(particularly for MAX_DELAY_MS).

3 years agoREORG: task: uninline the loop time measurement code
Willy Tarreau [Wed, 6 Oct 2021 17:25:38 +0000 (19:25 +0200)] 
REORG: task: uninline the loop time measurement code

It's pointless to inline this, it's called exactly once per poll loop,
and it depends on time.h which is quite deep. Let's move that to task.c
along with sched_report_idle().

3 years agoREORG: connection: uninline the rest of the alloc/free stuff
Willy Tarreau [Wed, 6 Oct 2021 17:11:10 +0000 (19:11 +0200)] 
REORG: connection: uninline the rest of the alloc/free stuff

The remaining large functions are those allocating/initializing and
occasionally freeing connections, conn_streams and sockaddr. Let's
move them to connection.c. In fact, cs_free() is the only one-liner
but let's move it along with the other ones since a call will be
small compared to the rest of the work done there.

3 years agoCLEANUP: connection: remove unneeded tcpcheck-t.h and use only session-t.h
Willy Tarreau [Wed, 6 Oct 2021 17:03:12 +0000 (19:03 +0200)] 
CLEANUP: connection: remove unneeded tcpcheck-t.h and use only session-t.h

No need to include the full session stuff, we only need the type. Also,
remove the unneeded tcpcheck types.

3 years agoCLEANUP: connection: do not include http_ana!
Willy Tarreau [Wed, 6 Oct 2021 16:57:44 +0000 (18:57 +0200)] 
CLEANUP: connection: do not include http_ana!

It makes no sense to have http_ana here, that's used at higher levels.

3 years agoREORG: connection: move the largest inlines from connection.h to connection.c
Willy Tarreau [Wed, 6 Oct 2021 16:48:28 +0000 (18:48 +0200)] 
REORG: connection: move the largest inlines from connection.h to connection.c

The following inlined functions are particularly large (and probably not
inlined at all by the compiler), and together represent roughly half of
the file, while they're used at most once per connection. They were moved
to connection.c.

  conn_upgrade_mux_fe, conn_install_mux_fe, conn_install_mux_be,
  conn_install_mux_chk, conn_delete_from_tree, conn_init, conn_new,
  conn_free

3 years agoCLEANUP: tree-wide: only include ebtree-t from type files
Willy Tarreau [Wed, 6 Oct 2021 16:31:48 +0000 (18:31 +0200)] 
CLEANUP: tree-wide: only include ebtree-t from type files

No need to include the full tree management code, type files only
need the definitions. Doing so reduces the whole code size by around
3.6% and the build time is down to just 6s.

3 years agoREORG: ebtree: split structures into their own file ebtree-t.h
Willy Tarreau [Wed, 6 Oct 2021 15:55:45 +0000 (17:55 +0200)] 
REORG: ebtree: split structures into their own file ebtree-t.h

ebtree is one piece using a lot of inlines and each tree root or node
definition needed by many of our structures requires to parse and
compile all these includes, which is large and painfully slow. Let's
move the very basic definitions to their own file and include it from
ebtree.h.

3 years agoREORG: server: uninline the idle conns management functions
Willy Tarreau [Wed, 6 Oct 2021 16:30:04 +0000 (18:30 +0200)] 
REORG: server: uninline the idle conns management functions

The following functions are quite heavy and have no reason to be kept
inlined:

   srv_release_conn, srv_lookup_conn, srv_lookup_conn_next,
   srv_add_to_idle_list

They were moved to server.c. It's worth noting that they're a bit
at the edge between server and connection and that maybe we could
create an idle-conn file for these in the near future.

3 years agoREORG: connection: uninline conn_notify_mux() and conn_delete_from_tree()
Willy Tarreau [Wed, 6 Oct 2021 16:27:28 +0000 (18:27 +0200)] 
REORG: connection: uninline conn_notify_mux() and conn_delete_from_tree()

The former is far too huge to be inlined and the second is the only
one requiring an ebmb tree through all includes, let's move them to
connection.c.

3 years agoREORG: connection: move the hash-related stuff to connection.c
Willy Tarreau [Wed, 6 Oct 2021 15:14:49 +0000 (17:14 +0200)] 
REORG: connection: move the hash-related stuff to connection.c

We do not really need to have them inlined, and having xxhash.h included
by connection.h results in this 4700-lines file being processed 101 times
over the whole project, which accounts for 13.5% of the total size!
Additionally, half of the functions are only needed from connection.c.
Let's move the functions there and get rid of the painful include.

The build time is now down to 6.2s just due to this.

3 years agoMINOR: connection: use uint64_t for the hashes
Willy Tarreau [Wed, 6 Oct 2021 15:09:41 +0000 (17:09 +0200)] 
MINOR: connection: use uint64_t for the hashes

The hash type stored everywhere is XXH64_hash_t, which annoyingly forces
everyone to include the huge xxhash file. We know it's an uint64_t because
that's its purpose and the type is only made to abstract it on machines
where uint64_t is not availble. Let's switch the type to uint64_t
everywhere and avoid including xxhash from the type file.

3 years agoCLEANUP: stick-table: no need to include socket nor in.h
Willy Tarreau [Wed, 6 Oct 2021 14:43:14 +0000 (16:43 +0200)] 
CLEANUP: stick-table: no need to include socket nor in.h

The types provided by these are never present in stick_table-t.h,
let's drop them.

3 years agoCLEANUP: stream: remove many unneeded includes from stream-t.h
Willy Tarreau [Wed, 6 Oct 2021 14:39:28 +0000 (16:39 +0200)] 
CLEANUP: stream: remove many unneeded includes from stream-t.h

Plenty of includes were present there only for struct pointers resulting
in them being used from many other places. The LoC reduced again by more
than 1% by cleaning this.

3 years agoREORG: acitvity: uninline sched_activity_entry()
Willy Tarreau [Wed, 6 Oct 2021 14:26:33 +0000 (16:26 +0200)] 
REORG: acitvity: uninline sched_activity_entry()

This one is expensive in code size because it comes with xxhash.h at a
low level of dependency that's inherited at plenty of places, and for
a function does doesn't benefit from inlining and could possibly even
benefit from not being inline given that it's large and called from the
scheduler.

Moving it to activity.c reduces the LoC by 1.2% and the binary size by
~1kB.

3 years agoREORG: activity: uninline activity_count_runtime()
Willy Tarreau [Wed, 6 Oct 2021 14:22:09 +0000 (16:22 +0200)] 
REORG: activity: uninline activity_count_runtime()

This function has no reason for being inlined, it's called from non
critical places (once in pollers), is quite large and comes with
dependencies (time and freq_ctr). Let's move it to acitvity.c. That's
another 0.4% less LoC to build.

3 years agoCLEANUP: tree-wide: remove unneeded include time.h in ~20 files
Willy Tarreau [Wed, 6 Oct 2021 14:18:40 +0000 (16:18 +0200)] 
CLEANUP: tree-wide: remove unneeded include time.h in ~20 files

20 files used to have haproxy/time.h included only for now_ms, and two
were missing it for other things but used to inherit from it via other
files.

3 years agoREORG: time/ticks: move now_ms and global_now_ms definitions to ticks.h
Willy Tarreau [Wed, 6 Oct 2021 14:03:19 +0000 (16:03 +0200)] 
REORG: time/ticks: move now_ms and global_now_ms definitions to ticks.h

These are ticks, not timeval, and they're a cause for plenty of files
including time.h just to access now_ms that's only used with ticks
functions. Let's move them over there.

3 years agoREORG: sched: moved samp_time and idle_time to task.c as well
Willy Tarreau [Wed, 6 Oct 2021 13:58:46 +0000 (15:58 +0200)] 
REORG: sched: moved samp_time and idle_time to task.c as well

The idle time calculation stuff was moved to task.h by commit 6dfab112e
("REORG: sched: move idle time calculation from time.h to task.h") but
these two variables that are only maintained by task.{c,h} were still
left in time.{c,h}. They have to move as well.

3 years agoREORG: sample: move the crypto samples to ssl_sample.c
Willy Tarreau [Wed, 6 Oct 2021 13:37:17 +0000 (15:37 +0200)] 
REORG: sample: move the crypto samples to ssl_sample.c

These ones require openssl and are only built when it's enabled. There's
no point keeping them in sample.c when ssl_sample.c already deals with this
and the required includes. This also allows to remove openssl-compat.h
from sample.c and to further reduce the number of inclusions of openssl
includes, and the build time is now down to under 8 seconds.

3 years agoREORG: ssl-sock: move the sslconns/totalsslconns counters to global
Willy Tarreau [Wed, 6 Oct 2021 10:15:18 +0000 (12:15 +0200)] 
REORG: ssl-sock: move the sslconns/totalsslconns counters to global

These two counters were the only ones not in the global struct, while
the SSL freq counters or the req counts are already in it, this forces
stats.c to include ssl_sock just to know about them. Let's move them
over there with their friends. This reduces from 408 to 384 the number
of includes of opensslconf.h.

3 years agoCLEANUP: ssl/server: move ssl_sock_set_srv() to srv_set_ssl() in server.c
Willy Tarreau [Wed, 6 Oct 2021 09:48:34 +0000 (11:48 +0200)] 
CLEANUP: ssl/server: move ssl_sock_set_srv() to srv_set_ssl() in server.c

This one has nothing to do with ssl_sock as it manipulates the struct
server only. Let's move it to server.c and remove unneeded dependencies
on ssl_sock.h. This further reduces by 10% the number of includes of
opensslconf.h and by 0.5% the number of compiled lines.

3 years agoCLEANUP: mux_fcgi: remove dependency on ssl_sock
Willy Tarreau [Wed, 6 Oct 2021 09:40:11 +0000 (11:40 +0200)] 
CLEANUP: mux_fcgi: remove dependency on ssl_sock

It's not needed anymore (used to be needed for ssl_sock_is_ssl()).

3 years agoREORG: ssl: move ssl_sock_is_ssl() to connection.h and rename it
Willy Tarreau [Wed, 6 Oct 2021 09:38:44 +0000 (11:38 +0200)] 
REORG: ssl: move ssl_sock_is_ssl() to connection.h and rename it

This one doesn't use anything from an SSL context, it only checks the
type of the transport layer of a connection, thus it belongs to
connection.h. This is particularly visible due to all the ifdefs
around it in various call places.

3 years agoCLEANUP: servers: do not include openssl-compat
Willy Tarreau [Wed, 6 Oct 2021 09:23:32 +0000 (11:23 +0200)] 
CLEANUP: servers: do not include openssl-compat

This is exactly the same as for listeners, servers only include
openssl-compat to provide the SSL_CTX type to use as two pointers to
contexts, and to detect if NPN, ALPN, and cipher suites are supported,
and save up to 5 pointers in the ssl_ctx struct if not supported. This
is pointless, as these ones have all been supported for about a decade,
and including this file comes with a long dependency chain that impacts
lots of other files. The ctx was made a void*.

Now the build time was significantly reduced, from 9.2 to 8.1 seconds,
thanks to opensslconf.h being included "only" 456 times instead of 2424
previously!

The total number of lines of code compiled was reduced by 15%.

3 years agoCLEANUP: listeners: do not include openssl-compat
Willy Tarreau [Wed, 6 Oct 2021 09:16:02 +0000 (11:16 +0200)] 
CLEANUP: listeners: do not include openssl-compat

Listeners only include openssl-compat to provide the SSL_CTX type to
use as two pointers to contexts, and to detect if NPN, ALPN, and cipher
suites are supported, and save up to 5 pointers in the ssl_bind_conf
struct if not supported. This is pointless, as these ones have all been
supported for about a decade, and including this file comes with a long
dependency chain that impacts lots of other files. The initial_ctx and
default_ctx can perfectly remain void* instead of SSL_CTX*.

3 years agoREORG: listener: move bind_conf_alloc() and listener_state_str() to listener.c
Willy Tarreau [Wed, 6 Oct 2021 07:05:08 +0000 (09:05 +0200)] 
REORG: listener: move bind_conf_alloc() and listener_state_str() to listener.c

These functions have no reason for being inlined, and they require some
includes with long dependencies. Let's move them to listener.c and trim
unused includes in listener.h.

3 years agoCLEANUP: remove some unneeded includes from applet-t.h
Willy Tarreau [Wed, 6 Oct 2021 06:54:05 +0000 (08:54 +0200)] 
CLEANUP: remove some unneeded includes from applet-t.h

This file includes streams, proxies, Lua just for some definitions of
structures for which we only have a pointer. Let's drop this. That's
responsible for 0.2% of all the lines of code.

3 years agoMINOR: thread/debug: replace nsec_now() with now_mono_time()
Willy Tarreau [Tue, 5 Oct 2021 16:48:23 +0000 (18:48 +0200)] 
MINOR: thread/debug: replace nsec_now() with now_mono_time()

The two functions do exactly the same except that the second one
is already provided by time.h and still defined if not available.

3 years agoREORG: thread: uninline the lock-debugging code
Willy Tarreau [Tue, 5 Oct 2021 16:39:27 +0000 (18:39 +0200)] 
REORG: thread: uninline the lock-debugging code

The lock-debugging code in thread.h has no reason to be inlined. the
functions are quite fat and perform a lot of operations so there's no
saving keeping them inlined. Worse, most of them are in fact not
inlined, resulting in a significantly bigger executable.

This patch moves all this part from thread.h to thread.c. The functions
are still exported in thread.h of course. This results in ~166kB less
code:

     text    data     bss     dec     hex filename
  3165938   99424  897376 4162738  3f84b2 haproxy-before
  2991987   99424  897376 3988787  3cdd33 haproxy-after

In addition the build time with thread debugging enabled has shrunk
from 19.2 to 17.7s thanks to much less code to be parsed in thread.h
that is included virtually everywhere.

3 years agoREORG: pools: uninline the UAF allocator and force-inline the rest
Willy Tarreau [Tue, 5 Oct 2021 16:14:11 +0000 (18:14 +0200)] 
REORG: pools: uninline the UAF allocator and force-inline the rest

pool-os.h relies on a number of includes solely because the
pool_alloc_area() function was inlined, and this only because we want
the normal version to be inlined so that we can track the calling
places for the memory profiler. It's worth noting that it already
does not work at -O0, and that when UAF is enabled we don't care a
dime about profiling.

This patch does two things at once:
  - force-inline the functions so that pool_alloc_area() is still
    inlined at -O0 to help track malloc() users ;

  - uninline the UAF version of these (that rely on mmap/munmap)
    and move them to pools.c so that we can remove all unneeded
    includes.

Doing so reduces by ~270kB or 0.15% the total build size.

3 years agoCLEANUP: pools: pools-t.h doesn't need to include thread-t.h
Willy Tarreau [Tue, 5 Oct 2021 16:04:48 +0000 (18:04 +0200)] 
CLEANUP: pools: pools-t.h doesn't need to include thread-t.h

This is probably a leftover from an older version to access MAX_THREADS.

3 years agoBUILD: compat: fix -Wundef on SO_REUSEADDR
Willy Tarreau [Wed, 6 Oct 2021 18:06:06 +0000 (20:06 +0200)] 
BUILD: compat: fix -Wundef on SO_REUSEADDR

If USE_NETFILTER is set and not SO_REUSEPORT, we evaluate SO_REUSEADDR,
let's fix that to check that it's defined.

3 years agoBUILD: tree-wide: add several missing activity.h
Willy Tarreau [Wed, 6 Oct 2021 17:54:09 +0000 (19:54 +0200)] 
BUILD: tree-wide: add several missing activity.h

A number of files currently access activity counters but rely on their
definitions to be inherited from other files (task.c, backend.c hlua.c,
sock.c, pool.c, stats.c, fd.c).

3 years agoBUILD: mworker: mworker-prog needs time.h for the 'now' variable
Willy Tarreau [Wed, 6 Oct 2021 17:31:06 +0000 (19:31 +0200)] 
BUILD: mworker: mworker-prog needs time.h for the 'now' variable

It wasn't included and it used to get them through other includes.

3 years agoBUILD: tcp_sample: include missing errors.h and session-t.h
Willy Tarreau [Wed, 6 Oct 2021 17:01:21 +0000 (19:01 +0200)] 
BUILD: tcp_sample: include missing errors.h and session-t.h

Both are used without being defined as they were inherited from other
files.

3 years agoBUILD: cfgparse-ssl: add missing errors.h
Willy Tarreau [Wed, 6 Oct 2021 17:00:49 +0000 (19:00 +0200)] 
BUILD: cfgparse-ssl: add missing errors.h

ha_warning(), ha_alert() and friends are in errors.h and it used
to be inherited via other files.

3 years agoBUILD: tree-wide: add missing http_ana.h from many places
Willy Tarreau [Wed, 6 Oct 2021 16:56:42 +0000 (18:56 +0200)] 
BUILD: tree-wide: add missing http_ana.h from many places

At least 6 files make use of s->txn without including http_ana which
defines it. They used to get it from other includes.

3 years agoBUILD: connection: connection.h needs list.h and server.h
Willy Tarreau [Wed, 6 Oct 2021 16:48:01 +0000 (18:48 +0200)] 
BUILD: connection: connection.h needs list.h and server.h

It manipulates lists and calls srv_add_conn().

3 years agoBUILD: idleconns: include missing ebmbtree.h at several places
Willy Tarreau [Wed, 6 Oct 2021 16:23:40 +0000 (18:23 +0200)] 
BUILD: idleconns: include missing ebmbtree.h at several places

backend.c, all muxes, backend.c started manipulating ebmb_nodes with
the introduction of idle conns but the types were inherited through
other includes. Let's add ebmbtree.h there.

3 years agoBUILD: compiler: add the container_of() and container_of_safe() macros
Willy Tarreau [Wed, 6 Oct 2021 16:11:38 +0000 (18:11 +0200)] 
BUILD: compiler: add the container_of() and container_of_safe() macros

These ones are called from a few places in the code and are only provided
by ebtree.h, which is not normal given that some callers do not even use
ebtree.

3 years agoBUILD: ssl_ckch: include ebpttree.h in ssl_ckch.c
Willy Tarreau [Wed, 6 Oct 2021 15:54:12 +0000 (17:54 +0200)] 
BUILD: ssl_ckch: include ebpttree.h in ssl_ckch.c

It's used but is only found through other includes.

3 years agoBUILD: peers: need to include eb{32/mb/pt}tree.h
Willy Tarreau [Wed, 6 Oct 2021 15:53:19 +0000 (17:53 +0200)] 
BUILD: peers: need to include eb{32/mb/pt}tree.h

peers.c uses them all and used to only find them through other includes.

3 years agoBUILD: vars: need to include xxhash
Willy Tarreau [Wed, 6 Oct 2021 15:11:51 +0000 (17:11 +0200)] 
BUILD: vars: need to include xxhash

It's needed for XXH3(), and it used to get it through other includes.

3 years agoBUILD: http_rules: requires http_ana-t.h for REDIRECT_*
Willy Tarreau [Wed, 6 Oct 2021 14:38:53 +0000 (16:38 +0200)] 
BUILD: http_rules: requires http_ana-t.h for REDIRECT_*

It used to inherit it through other includes.

3 years agoBUILD: http_ana: need to include proxy-t to get redirect_rule
Willy Tarreau [Wed, 6 Oct 2021 14:37:40 +0000 (16:37 +0200)] 
BUILD: http_ana: need to include proxy-t to get redirect_rule

The struct was only defined inside function arguments there and
inherited from other files.

3 years agoBUILD: sample: include openssl-compat
Willy Tarreau [Wed, 6 Oct 2021 09:29:03 +0000 (11:29 +0200)] 
BUILD: sample: include openssl-compat

It's needed for EVP_*.

3 years agoBUILD: httpclient: include missing ssl_sock-t
Willy Tarreau [Wed, 6 Oct 2021 09:28:24 +0000 (11:28 +0200)] 
BUILD: httpclient: include missing ssl_sock-t

It's needed for SSL_SOCK_VERIFY_NONE.

3 years agoBUILD: resolvers: define missing types in resolvers.h
Willy Tarreau [Wed, 6 Oct 2021 07:18:37 +0000 (09:18 +0200)] 
BUILD: resolvers: define missing types in resolvers.h

proxy, server, stream_interface and list were used but not defined. Let's
define them as well as act_rule and drop action-t.h.

3 years agoBUILD: stats: define several missing structures in stats.h
Willy Tarreau [Wed, 6 Oct 2021 07:14:06 +0000 (09:14 +0200)] 
BUILD: stats: define several missing structures in stats.h

channel, stream_interface, appctx, buffer, proxy and htx ones are used
in function arguments and most of them are not defined but were inherited
from intermediary inclues. Let's define them here and drop the unneeded
includes.

3 years agoBUILD: hlua: needs to include stream-t.h
Willy Tarreau [Wed, 6 Oct 2021 07:12:44 +0000 (09:12 +0200)] 
BUILD: hlua: needs to include stream-t.h

It uses the SF_ERR_* error codes and currently gets them via
intermediary includes.

3 years agoBUILD: extcheck: needs to include stream-t.h
Willy Tarreau [Wed, 6 Oct 2021 07:11:54 +0000 (09:11 +0200)] 
BUILD: extcheck: needs to include stream-t.h

It uses the SF_ERR_* error codes and currently gets them via
intermediary includes.

3 years agoBUILD: action: add the relevant structures for function arguments
Willy Tarreau [Wed, 6 Oct 2021 07:09:01 +0000 (09:09 +0200)] 
BUILD: action: add the relevant structures for function arguments

Some structures are inherited via intermediary includes (e.g. dns_counters
comes from a long path). Let's define the missing ones and includes vars-t
that is needed in the structure.

3 years agoBUG/MEDIUM: sample: properly verify that variables cast to sample
Willy Tarreau [Wed, 6 Oct 2021 13:30:52 +0000 (15:30 +0200)] 
BUG/MEDIUM: sample: properly verify that variables cast to sample

The various variable-to-sample converters allow to turn a variable to
a sample of type string, sint or binary, but both the string one used
by strcmp() and the binary one used by secure_memcmp() are missing a
pointer check on the ability to the cast, making them crash if a
variable of type addr is used with strcmp(), or if an addr or bool is
used with secure_memcmp().

Let's rely on the new sample_conv_var2smp() function to run the proper
checks.

This will need to be backported to all supported version. It relies on
previous commits:

  CLEANUP: server: always include the storage for SSL settings
  CLEANUP: sample: rename sample_conv_var2smp() to *_sint
  CLEANUP: sample: uninline sample_conv_var2smp_str()
  MINOR: sample: provide a generic var-to-sample conversion function

For backports it's probably easier to check the sample_casts[] pointer
before calling it in sample_conv_strcmp() and sample_conv_secure_memcmp().

3 years agoMINOR: sample: provide a generic var-to-sample conversion function
Willy Tarreau [Wed, 6 Oct 2021 13:29:00 +0000 (15:29 +0200)] 
MINOR: sample: provide a generic var-to-sample conversion function

We're using variable-to-sample conversion at least 4 times in the code,
two of which are bogus. Let's introduce a generic conversion function
that performs the required checks.

3 years agoCLEANUP: sample: uninline sample_conv_var2smp_str()
Willy Tarreau [Wed, 6 Oct 2021 13:20:18 +0000 (15:20 +0200)] 
CLEANUP: sample: uninline sample_conv_var2smp_str()

There's no reason to limit this one to this file, it could be used in
other contexts.

3 years agoCLEANUP: sample: rename sample_conv_var2smp() to *_sint
Willy Tarreau [Wed, 6 Oct 2021 13:19:05 +0000 (15:19 +0200)] 
CLEANUP: sample: rename sample_conv_var2smp() to *_sint

This one only handles integers, contrary to its sibling with the suffix
_str that only handles strings. Let's rename it and uninline it since it
may well be used from outside.

3 years agoCLEANUP: server: always include the storage for SSL settings
Willy Tarreau [Wed, 6 Oct 2021 12:48:37 +0000 (14:48 +0200)] 
CLEANUP: server: always include the storage for SSL settings

The SSL stuff in struct server takes less than 3% of it and requires
lots of annoying ifdefs in the code just to take care of the cases
where the field is absent. Let's get rid of this and stop including
openssl-compat from server.c to detect NPN and ALPN capabilities.

This reduces the total LoC by another 0.4%.

3 years agoMINOR: httpclient/lua: supports headers via named arguments
William Lallemand [Wed, 6 Oct 2021 08:57:44 +0000 (10:57 +0200)] 
MINOR: httpclient/lua: supports headers via named arguments

Migrate the httpclient:get() method to named arguments so we can
specify optional arguments.

This allows to pass headers as an optional argument as an array.

The () in the method call must be replaced by {}:

local res = httpclient:get{url="http://127.0.0.1:9000/?s=99",
            headers= {["X-foo"]  = { "salt" }, ["X-bar"] = {"pepper" }}}

3 years agoBUG/MINOR: httpclient/lua: does not process headers when failed
William Lallemand [Tue, 5 Oct 2021 14:19:31 +0000 (16:19 +0200)] 
BUG/MINOR: httpclient/lua: does not process headers when failed

Do not try to process the header list when it is NULL. This case can
arrive when the request failed and did not return a response.

3 years agoMINOR: httpclient: destroy checks if a client was started but not stopped
William Lallemand [Tue, 5 Oct 2021 13:50:45 +0000 (15:50 +0200)] 
MINOR: httpclient: destroy checks if a client was started but not stopped

During httpclient_destroy, add a condition in the BUG_ON which checks
that the client was started before it has ended. A httpclient structure
could have been created without being started.

3 years agoBUG/MEDIUM: httpclient/lua: crash because of b_xfer and get_trash_chunk()
William Lallemand [Thu, 30 Sep 2021 08:07:57 +0000 (10:07 +0200)] 
BUG/MEDIUM: httpclient/lua: crash because of b_xfer and get_trash_chunk()

When using the lua httpclient, haproxy could crash because a b_xfer is
done in httpclient_xfer, which will do a zero-copy swap of the data in
the buffers. The ptr will then be free() by the pool.

However this can't work with a trash buffer, because the area was not
allocated from the pool buffer, so the pool is not suppose to free it
because it does not know this ptr, using -DDEBUG_MEMORY_POOLS will
result with a crash during the free.

Fix the problem by using b_force_xfer() instead of b_xfer which copy
the data instead. The problem still exist with the trash however, and
the trash API must be reworked.

3 years agoMINOR: httpclient/lua: implement garbage collection
William Lallemand [Tue, 28 Sep 2021 17:10:38 +0000 (19:10 +0200)] 
MINOR: httpclient/lua: implement garbage collection

Implement the garbage collector of the lua httpclient.

This patch declares the __gc method of the httpclient object which only
does a httpclient_stop_and_destroy().

3 years agoMINOR: httpclient: test if started during stop_and_destroy()
William Lallemand [Tue, 28 Sep 2021 10:15:37 +0000 (12:15 +0200)] 
MINOR: httpclient: test if started during stop_and_destroy()

If the httpclient was never started, it is safe to destroy completely
the httpclient.

3 years agoMINOR: httpclient: stop_and_destroy() ask the applet to autokill
William Lallemand [Tue, 28 Sep 2021 09:00:43 +0000 (11:00 +0200)] 
MINOR: httpclient: stop_and_destroy() ask the applet to autokill

httpclient_stop_and_destroy() tries to destroy the httpclient structure
if the client was stopped.

In the case the client wasn't stopped, it ask the client to stop itself
and to destroy the httpclient structure itself during the release of the
applet.

3 years agoMINOR: httpclient: set HTTPCLIENT_F_ENDED only in release
William Lallemand [Tue, 28 Sep 2021 08:10:07 +0000 (10:10 +0200)] 
MINOR: httpclient: set HTTPCLIENT_F_ENDED only in release

Only set the HTTPCLIENT_F_ENDED flag in httpclient_applet_release()
function so we are sure that the appctx is not used anymore once the
flag is set.

3 years agoMINOR: httpclient: destroy() must free the headers and the ists
William Lallemand [Mon, 27 Sep 2021 13:17:47 +0000 (15:17 +0200)] 
MINOR: httpclient: destroy() must free the headers and the ists

httpclient_destroy() must free all the ist in the httpclient structure,
the URL in the request, the vsn and reason in the response.

It also must free the list of headers of the response.

3 years agoBUG/MEDIUM: http-ana: Clear request analyzers when applying redirect rule
Christopher Faulet [Mon, 4 Oct 2021 12:16:46 +0000 (14:16 +0200)] 
BUG/MEDIUM: http-ana: Clear request analyzers when applying redirect rule

A bug was introduced by the commit 2d5650082 ("BUG/MEDIUM: http-ana: Reset
channels analysers when returning an error").

The request analyzers must be cleared when a redirect rule is applied. It is
not a problem if the redirect rule is inside an http-request ruleset because
the analyzer takes care to clear it. However, when it comes from a redirect
ruleset (via the "redirect ..."  directive), because of the above commit,
the request analyzers are no longer cleared. It means some HTTP request
analyzers may be called while the request channel was already flushed. It is
totally unexpected and may lead to crash.

Thanks to Yves Lafon for reporting the problem.

This patch must be backported everywhere the above commit was backported.

3 years agoBUG/MEDIUM: filters: Fix a typo when a filter is attached blocking the release
Christopher Faulet [Mon, 4 Oct 2021 05:50:13 +0000 (07:50 +0200)] 
BUG/MEDIUM: filters: Fix a typo when a filter is attached blocking the release

When a filter is attached to a stream, the wrong FLT_END analyzer is added
on the request channel. AN_REQ_FLT_END must be added instead of
AN_RES_FLT_END. Because of this bug, the stream may hang on the filter
release stage.

It seems to be ok for HTTP filters (cache & compression) in HTTP mode. But
when enabled on a TCP proxy, the stream is blocked until the client or the
server timeout expire because data forwarding is blocked. The stream is then
prematurely aborted.

This bug was introduced by commit 26eb5ea35 ("BUG/MINOR: filters: Always set
FLT_END analyser when CF_FLT_ANALYZE flag is set"). The patch must be
backported in all stable versions.

3 years agoREORG: sched: move the stolen CPU time detection to sched_entering_poll()
Willy Tarreau [Thu, 30 Sep 2021 16:20:30 +0000 (18:20 +0200)] 
REORG: sched: move the stolen CPU time detection to sched_entering_poll()

That's where that code initially was but it had been moved to
activity_count_runtime() for pure reasons of dependency loops. These
ones are no longer true so we can move that code back to the scheduler
and keep it where the information are updated and checked.

3 years agoREORG: sched: move idle time calculation from time.h to task.h
Willy Tarreau [Thu, 30 Sep 2021 15:53:22 +0000 (17:53 +0200)] 
REORG: sched: move idle time calculation from time.h to task.h

time.h is a horrible place to put activity calculation, it's a
historical mistake because the functions were there. We already have
most of the parts in sched.{c,h} and these ones make an exception in
the middle, forcing time.h to include some thread stuff and to access
the before/after_poll and idle_pct values.

Let's move these 3 functions to task.h with the other ones. They were
prefixed with "sched_" instead of the historical "tv_" which already
made no sense anymore.

3 years agoMINOR: time: uninline report_idle() and move it to task.c
Willy Tarreau [Thu, 30 Sep 2021 06:52:11 +0000 (08:52 +0200)] 
MINOR: time: uninline report_idle() and move it to task.c

I don't know why I inlined this one, this makes no sense given that it's
only used for stats, and it starts a circular dependency on tinfo.h which
can be problematic in the future. In addition, all the stuff related to
idle time calculation should be with the rest of the scheduler, which
currently is in task.{c,h}, so let's move it there.

3 years agoMINOR: task: provide 3 task_new_* wrappers to simplify the API
Willy Tarreau [Fri, 1 Oct 2021 16:23:30 +0000 (18:23 +0200)] 
MINOR: task: provide 3 task_new_* wrappers to simplify the API

We'll need to improve the API to pass other arguments in the future, so
let's start to adapt better to the current use cases. task_new() is used:
  - 18 times as task_new(tid_bit)
  - 18 times as task_new(MAX_THREADS_MASK)
  - 2 times with a single bit (in a loop)
  - 1 in the debug code that uses a mask

This patch provides 3 new functions to achieve this:
  - task_new_here()     to create a task on the calling thread
  - task_new_anywhere() to create a task to be run anywhere
  - task_new_on()       to create a task to run on a specific thread

The change is trivial and will allow us to later concentrate the
required adaptations to these 3 functions only. It's still possible
to call task_new() if needed but a comment was added to encourage the
use of the new ones instead. The debug code was not changed and still
uses it.

3 years agoCLEANUP: tasks: remove the long-unused work_lists
Willy Tarreau [Fri, 1 Oct 2021 16:30:14 +0000 (18:30 +0200)] 
CLEANUP: tasks: remove the long-unused work_lists

Work lists were a mechanism introduced in 1.8 to asynchronously delegate
some work to be performed on another thread via a dedicated task.
The only user was the listeners, to deal with the queue. Nowadays
the tasklets have made this much more convenient, and have replaced
work_lists in the listeners. It seems there will be no valid use case
of work lists anymore, so better get rid of them entirely and keep the
scheduler code cleaner.

3 years agoREGTESTS: ssl: wrong feature cmd in show_ssl_ocspresponse.vtc
William Lallemand [Thu, 30 Sep 2021 16:45:18 +0000 (18:45 +0200)] 
REGTESTS: ssl: wrong feature cmd in show_ssl_ocspresponse.vtc

The "feature cmd" needs to be separated in 2 parts to check the openssl
command.

3 years agoREGTESTS: ssl: show_ssl_ocspresponse w/ freebsd won't use base64
William Lallemand [Thu, 30 Sep 2021 15:57:04 +0000 (17:57 +0200)] 
REGTESTS: ssl: show_ssl_ocspresponse w/ freebsd won't use base64

The reg-test show_ssl_ocspresponse.vtc won't use the "base64" binary on
freebsd, replace it by a "openssl base64" which does the same thing.

3 years agoMINOR: tasks: catch TICK_ETERNITY with BUG_ON() in __task_queue()
Willy Tarreau [Thu, 30 Sep 2021 14:38:09 +0000 (16:38 +0200)] 
MINOR: tasks: catch TICK_ETERNITY with BUG_ON() in __task_queue()

__task_queue() must absolutely not be called with TICK_ETERNITY or it
will place a never-expiring node upfront in the timers queue, preventing
any timer from expiring until the process is restarted. Code was found
to cause this using "task_schedule(task, now_ms)" which does this one
millisecond every 49.7 days, so let's add a condition against this. It
must never trigger since any process susceptible to trigger it would
already accumulate tasks until it dies.

An extra test was added in wake_expired_tasks() to detect tasks whose
timeout would have been changed after being queued.

An improvement over this could be in the future to use a non-scalar
type (union/struct) for expiration dates so as to avoid the risk of
using them directly like this. But now_ms is already such a valid
time and this specific construct would still not be caught.

This could even be backported to stable versions to help detect other
occurrences if any.

3 years agoBUG/MINOR: tcp-rules: Stop content rules eval on read error and end-of-input
Christopher Faulet [Thu, 30 Sep 2021 12:56:30 +0000 (14:56 +0200)] 
BUG/MINOR: tcp-rules: Stop content rules eval on read error and end-of-input

For now, tcp-request and tcp-response content rules evaluation is
interrupted before the inspect-delay when the channel's buffer is full, the
RX path is blocked or when a shutdown for reads was received. To sum up, the
evaluation is interrupted when no more input data are expected. However, it
is not exhaustive. It also happens when end of input is reached (CF_EOI flag
set) or when a read error occurred (CF_READ_ERROR flag set).

Note that, AFAIK, it is only a problem on HAProy 2.3 and prior when a H1 to
H2 upgrade is performed. On newer versions, it works as expected because the
stream is not created at this stage.

This patch must be backported as far as 2.0.

3 years agoBUG/MINOR: tcpcheck: Don't use arg list for default proxies during parsing
Christopher Faulet [Thu, 30 Sep 2021 14:22:51 +0000 (16:22 +0200)] 
BUG/MINOR: tcpcheck: Don't use arg list for default proxies during parsing

During tcp/http check rules parsing, when a sample fetch or a log-format
string is parsed, the proxy's argument list used to track unresolved
argument is no longer passed for default proxies. It means it is no longer
possible to rely on sample fetches depending on the execution context (for
instance 'nbsrv').

It is important to avoid HAProxy crashes because these arguments are
resolved during the configuration validity check. But, default proxies are
not evaluated during this stage. Thus, these arguments remain unresolved.

It will probably be possible to relax this rule. But to ease backports, it
is forbidden for now.

This patch must be backported as far as 2.2. It depends on the commit
"MINOR: arg: Be able to forbid unresolved args when building an argument
list".  It must be adapted for the 2.3 because PR_CAP_DEF capability was
introduced in the 2.4. A solution may be to test The proxy's id agains NULL.

3 years agoMINOR: arg: Be able to forbid unresolved args when building an argument list
Christopher Faulet [Thu, 30 Sep 2021 06:48:56 +0000 (08:48 +0200)] 
MINOR: arg: Be able to forbid unresolved args when building an argument list

In make_arg_list() function, unresolved dependencies are pushed in an
argument list to be resolved later, during the configuration validity
check. It is now possible to forbid such unresolved dependencies by omitting
<al> parameter (setting it to NULL). It is usefull when the parsing context
is not the same than the running context or when the parsing context is lost
after the startup stage. For instance, an argument may be defined in
defaults section during parsing and executed in a frontend/backend section.

3 years agoBUG/MAJOR: lua: use task_wakeup() to properly run a task once
Willy Tarreau [Thu, 30 Sep 2021 14:17:37 +0000 (16:17 +0200)] 
BUG/MAJOR: lua: use task_wakeup() to properly run a task once

The Lua tasks registered vi core.register_task() use a dangerous
task_schedule(task, now_ms) to start them, that will most of the
time work by accident, except when the time wraps every 49.7 days,
if now_ms is 0, because it's not valid to queue a task with an
expiration date set to TICK_ETERNITY, as it will fail all wakeup
checks and prevent all subsequent timers from being seen as expired.
The only solution in this case is to restart the process.

Fortunately for the vast majority of users it is extremely unlikely
to ever be met (only one millisecond every 49.7 days is at risk), but
this can be systematic for a process dealing with 1000 req/s, hence
the major tag.

The bug was introduced in 1.6-dev with commit 24f335340 ("MEDIUM: lua:
add coroutine as tasks."), so the fix must be backported to all stable
branches.