Commit Graph

511 Commits

Author SHA1 Message Date
Aurelien DARRAGON
f2629ebd4e MINOR: proxy: add free_server_rules() helper function
Take the px->server_rules freeing part out of free_proxy() and make it
a dedicated helper function so that it becomes possible to use it from
anywhere.
2023-11-24 16:27:55 +01:00
Aurelien DARRAGON
481e9317e3 MINOR: proxy: add free_logformat_list() helper function
There are multiple places inside free_proxy() where we need to perform
the exact same operation: freeing a logformat list which includes freeing
every member.

To prevent code duplication, we add the free_logformat_list() function
that takes such list as parameter and does all the freeing job on its
own.
2023-11-24 16:27:55 +01:00
Amaury Denoyelle
55e78ff7e1 MINOR: rhttp: large renaming to use rhttp prefix
Previous commit renames 'proto_reverse_connect' module to 'proto_rhttp'.
This commits follows this by replacing various custom prefix by 'rhttp_'
to make the code uniform.

Note that 'reverse_' prefix was kept in connection module. This is
because if a new reversable protocol not based on HTTP is implemented,
it may be necessary to reused the same connection function which are
protocol agnostic.
2023-11-23 17:40:01 +01:00
Willy Tarreau
53da8bfcb6 BUG/MINOR: server: do not leak default-server in defaults sections
When a default-server directive is used in a defaults section, it's never
freed and the "defaults" proxy gets reset without freeing the fields from
that default-server. Normally there are no allocation there, except for
the config file location stored in srv->conf.file form an strdup() since
commit 9394a9444 ("REORG: server: move alert traces in parse_server")
that appeared in 2.4. In addition, if a "default-server" directive
appears multiple times in a defaults section, one more entry will be
leaked per call.

This commit addresses this by checking that we don't overwrite the file
upon multiple calls, and by clearing it when resetting the default proxy.
This should be backported to 2.4.
2023-11-23 14:32:55 +01:00
Aurelien DARRAGON
4b2616f784 MINOR: log/backend: prevent stick table and stick rules with LOG mode
Report a warning and prevent errors if user tries to declare a stick table
or use stick rules within a log backend.
2023-11-18 11:16:21 +01:00
Aurelien DARRAGON
6a29888f60 MINOR: log/backend: ensure log exclusive params are not used in other modes
add proxy_cfg_ensure_no_log() function (similar to
proxy_cfg_ensure_no_http()) to ensure at the end of proxy parsing that
no log exclusive options are found if the proxy is not in log mode.
2023-11-18 11:16:21 +01:00
Aurelien DARRAGON
b6e1e9ec8b Revert "MINOR: proxy: report a warning for max_ka_queue in proxy_cfg_ensure_no_http()"
This reverts commit 3934901 since it makes no sense to report a warning
in this case given that max-keepalive-queue will also work with TCP
backends.
2023-11-18 11:16:21 +01:00
Aurelien DARRAGON
2c4943c18b BUG/MINOR: proxy/stktable: missing frees on proxy cleanup
In 1b8e68e ("MEDIUM: stick-table: Stop handling stick-tables as proxies.")
we forgot to free the table pointer which is now dynamically allocated.

Let's take this opportunity to also fix a missing free in the table itself
(the table expire task wasn't properly destroyed)

This patch depends on:
 - "MINOR: stktable: add sktable_deinit function"

It should be backported in every stable versions.
2023-11-18 11:16:21 +01:00
Aurelien DARRAGON
e10cf61099 MINOR: stktable: add stktable_deinit function
Adding sktable_deinit() helper function to properly cleanup a sticktable
that was initialized using stktable_init().
2023-11-18 11:16:21 +01:00
Frédéric Lécaille
888d1dc3dc MINOR: quic: Rename "handshake" timeout to "client-hs"
Use a more specific name for this timeout to distinguish it from a possible
future one on the server side.
Also update the documentation.
2023-11-17 18:09:41 +01:00
Frédéric Lécaille
b33eacc523 MINOR: proxy: Add "handshake" new timeout (frontend side)
Add a new timeout for the handshake, on the frontend side only. Such a hanshake
will be typically used for TLS hanshakes during client connections to TLS/TCP or
QUIC frontends.
2023-11-17 17:31:42 +01:00
Willy Tarreau
cf07cb96be BUG/MEDIUM: proxy: always initialize the default settings after init
The proxy's initialization is rather odd. First, init_new_proxy() is
called to zero all the lists and certain values, except those that can
come from defaults, which are initialized by proxy_preset_defaults().
The default server settings are also only set there.

This results in these settings not to be set for a number of internal
proxies that do not explicitly call proxy_preset_defaults() after
allocation, such as sink and log forwarders.

This was revealed by last commit 79aa63823 ("MINOR: server: always
initialize pp_tlvs for default servers") which crashes in log parsers
when applied to certain proxies which did not initialize their default
servers.

In theory this should be backported, however it would be desirable to
wait a bit before backporting it, in case certain parts would rely on
these elements not being initialized.
2023-11-13 09:17:05 +01:00
Willy Tarreau
79aa638238 MINOR: server: always initialize pp_tlvs for default servers
In commit 6f4bfed3a ("MINOR: server: Add parser support for
set-proxy-v2-tlv-fmt") a suspicious check for a NULL srv_tlv was placed
in the list_for_each_entry(), that should not be needed. In practice,
it's caused by the list head not being initialized, hence the first
element is NULL, as shown by Alexander's reproducer below which crashes
if the test in the loop is removed:

  backend dummy
    default-server send-proxy-v2 set-proxy-v2-tlv-fmt(0xE1) %[fc_pp_tlv(0xE1)]
    server dummy_server 127.0.0.1:2319

The right place to initialize this field is proxy_preset_defaults().
We'd really need a function to initialize a server :-/

The check in the loop was removed. No backport is needed.
2023-11-13 08:53:28 +01:00
Aurelien DARRAGON
b9c0b039c8 MINOR: proxy/stktable: add resolve_stick_rule helper function
Simplify stick and store sticktable proxy rules postparsing by adding
a sticking rule entry resolve (postparsing) function.

This will ease code maintenance.
2023-11-03 17:30:30 +01:00
Aurelien DARRAGON
9a74a6cb17 MAJOR: log: introduce log backends
Using "mode log" in a backend section turns the proxy in a log backend
which can be used to log-balance logs between multiple log targets
(udp or tcp servers)

log backends can be used as regular log targets using the log directive
with "backend@be_name" prefix, like so:

  | log backend@mybackend local0

A log backend will distribute log messages to servers according to the
log load-balancing algorithm that can be set using the "log-balance"
option from the log backend section. For now, only the roundrobin
algorithm is supported and set by default.
2023-10-13 10:05:06 +02:00
Aurelien DARRAGON
18da35c123 MEDIUM: tree-wide: logsrv struct becomes logger
When 'log' directive was implemented, the internal representation was
named 'struct logsrv', because the 'log' directive would directly point
to the log target, which used to be a (UDP) log server exclusively at
that time, hence the name.

But things have become more complex, since today 'log' directive can point
to ring targets (implicit, or named) for example.

Indeed, a 'log' directive does no longer reference the "final" server to
which the log will be sent, but instead it describes which log API and
parameters to use for transporting the log messages to the proper log
destination.

So now the term 'logsrv' is rather confusing and prevents us from
introducing a new level of abstraction because they would be mixed
with logsrv.

So in order to better designate this 'log' directive, and make it more
generic, we chose the word 'logger' which now replaces logsrv everywhere
it was used in the code (including related comments).

This is internal rewording, so no functional change should be expected
on user-side.
2023-10-13 10:05:06 +02:00
Aurelien DARRAGON
3934901e51 MINOR: proxy: report a warning for max_ka_queue in proxy_cfg_ensure_no_http()
Display a warning when max_ka_queue is set (it is the case when
"max-keep-alive-queue" directive is used within a proxy section) to inform
the user that this directives depends on the "http" mode to work and thus
will safely be ignored.
2023-10-06 15:34:30 +02:00
Aurelien DARRAGON
403fdee6a4 MINOR: proxy: dynamic-cookie CLIs require TCP or HTTP mode
Prevent the use of "dynamic-cookie" related CLI commands if the backend
is not in TCP or HTTP mode.
2023-10-06 15:34:30 +02:00
Aurelien DARRAGON
969e212c66 MINOR: log: add dup_logsrv() helper function
ease code maintenance by introducing dup_logsrv() helper function to
properly duplicate an existing logsrv struct.
2023-09-06 16:06:39 +02:00
Amaury Denoyelle
47f502df5e MEDIUM: proto_reverse_connect: bootstrap active reverse connection
Implement active reverse connection initialization. This is done through
a new task stored in the receiver structure. This task is instantiated
via bind callback and first woken up via enable callback.

Task handler is separated into two halves. On the first step, a new
connection is allocated and stored in <pend_conn> member of the
receiver. This new client connection will proceed to connect using the
server instance referenced in the bind_conf.

When connect has successfully been executed and HTTP/2 connection is
ready for exchange after SETTINGS, reverse_connect task is woken up. As
<pend_conn> is still set, the second halve is executed which only
execute listener_accept(). This will in turn execute accept_conn
callback which is defined to return the pending connection.

The task is automatically requeued inside accept_conn callback if bind
maxconn is not yet reached. This allows to specify how many connection
should be opened. Each connection is instantiated and reversed serially
one by one until maxconn is reached.

conn_free() has been modified to handle failure if a reverse connection
fails before being accepted. In this case, no session exists to notify
about the failure. Instead, reverse_connect task is requeud with a 1
second delay, giving time to fix a possible network issue. This will
allow to attempt a new connection reverse.

Note that for the moment connection rebinding after accept is disabled
for simplicity. Extra operations are required to migrate an existing
connection and its stack to a new thread which will be implemented
later.
2023-08-24 17:03:06 +02:00
Amaury Denoyelle
0747e493a0 MINOR: proto_reverse_connect: parse rev@ addresses for bind
Implement parsing for "rev@" addresses on bind line. On config parsing,
server name is stored on the bind_conf.

Several new callbacks are defined on reverse_connect protocol to
complete parsing. listen callback is used to retrieve the server
instance from the bind_conf server name. If found, the server instance
is stored on the receiver. Checks are implemented to ensure HTTP/2
protocol only is used by the server.
2023-08-24 17:02:37 +02:00
Amaury Denoyelle
fbe35afaa4 MINOR: proxy: simplify parsing 'backend/server'
Several CLI handlers use a server argument specified with the format
'<backend>/<server>'. The parsing of this arguement is done in two
steps, first splitting the string with '/' delimiter and then use
get_backend_server() to retrieve the server instance.

Refactor this code sections with the following changes :
* splitting is reimplented using ist API
* get_backend_server() is removed. Instead use the already existing
  proxy_be_by_name() then server_find_by_name() which contains
  duplicated code with the now removed function.

No functional change occurs with this commit. However, it will be useful
to add new configuration options reusing the same '<backend>/<server>'
for reverse connect.
2023-08-24 14:44:33 +02:00
Aurelien DARRAGON
b7f8af3ca9 BUG/MINOR: proxy/server: free default-server on deinit
proxy default-server is a specific type of server that is not allocated
using new_server(): it is directly stored within the parent proxy
structure. However, since it may contain some default config options that
may be inherited by regular servers, it is also subject to dynamic members
(strings, structures..) that needs to be deallocated when the parent proxy
is cleaned up.

Unfortunately, srv_drop() may not be used directly from p->defsrv since
this function is meant to be used on regular servers only (those created
using new_server()).

To circumvent this, we're splitting srv_drop() to make a new function
called srv_free_params() that takes care of the member cleaning which
originally takes place in srv_drop(). This function is exposed through
server.h, so it may be called from outside server.c.

Thanks to this, calling srv_free_params(&p->defsrv) from free_proxy()
prevents any memory leaks due to dynamic parameters allocated when
parsing a default-server line from a proxy section.

This partially fixes GH #2173 and may be backported to 2.8.

[While it could also be relevant for other stable versions, the patch
won't apply due to architectural changes / name changes between 2.4 => 2.6
and then 2.6 => 2.8. Considering this is a minor fix that only makes
memory analyzers happy during deinit paths (at least for <= 2.8), it might
not be worth the trouble to backport them any further?]
2023-06-06 15:15:17 +02:00
Aurelien DARRAGON
9be9225ef2 BUG/MINOR: proxy: add missing interface bind free in free_proxy
bind->settings.interface hint is allocated when "interface" keyword
is specified on a bind line, but the string isn't explicitly freed in
proxy_free, resulting in minor memory leak on deinit paths when the
keyword is being used.

It partially fixes GH #2173 and may be backported to all stable versions.

[in 2.2 free_proxy did not exist so the patch must be applied directly
in deinit() function from haproxy.c]
2023-06-06 15:15:17 +02:00
William Lallemand
7f95469163 MEDIUM: proxy: stop emitting logs for internal proxies when stopping
The HTTPCLIENT and the OCSP-UPDATE proxies are internal proxies, we
don't need to display logs of them stopping during the stopping of the
process.

This patch checks if a proxy has the flag PR_CAP_INT so it doesn't
display annoying messages.
2023-05-15 10:38:09 +02:00
Aurelien DARRAGON
c610095258 MINOR: tree-wide: use free_acl_cond() where relevant
Now that we have free_acl_cond(cond) function that does cond prune then
frees cond, replace all occurences of this pattern:

   | prune_acl_cond(cond)
   | free(cond)

with:

   | free_acl_cond(cond)
2023-05-11 15:37:04 +02:00
Aurelien DARRAGON
7abc9224a6 MINOR: proxy: add http_free_redirect_rule() function
Adding http_free_redirect_rule() function to free a single redirect rule
since it may be required to free rules outside of free_proxy() function.

This patch is required for an upcoming bugfix.

[for 2.2, free_proxy function did not exist (first seen in 2.4), thus
http_free_redirect_rule() needs to be deducted from haproxy.c deinit()
function if the patch is required]
2023-05-11 15:37:04 +02:00
Aurelien DARRAGON
8dfc2491d2 BUG/MINOR: proxy: missing free in free_proxy for redirect rules
cookie_str from struct redirect, which may be allocated through
http_parse_redirect_rule() function is not properly freed on proxy
cleanup within free_proxy().

This could be backported to all stable versions.

[for 2.2, free_proxy() did not exist so the fix needs to be performed
directly in deinit() function from haproxy.c]
2023-05-11 15:37:04 +02:00
Willy Tarreau
862588a4b5 BUG/MINOR: config: make compression work again in defaults section
When commit ead43fe4f ("MEDIUM: compression: Make it so we can compress
requests as well.") added the test for the direction flags to select the
compression, it implicitly broke compression defined in defaults sections
because the flags from the default proxy were not recopied, hence the
compression was enabled but in no direction.

No backport is needed, that's 2.8 only.
2023-05-10 16:41:21 +02:00
Christopher Faulet
7b3d38a633 MEDIUM: tree-wide: Change sc API to specify required free space to progress
sc_need_room() now takes the required free space to receive more data as
parameter. All calls to this function are updated accordingly. For now, this
value is set but not used. When we are waiting for a buffer, 0 is used. So
we expect to be unblocked ASAP. However this must be reviewed because
SC_FL_NEED_BUF is probably enough in this case and this flag is already set
if the input buffer allocation fails.
2023-05-05 15:44:23 +02:00
Willy Tarreau
69530f59ae MEDIUM: clock: replace timeval "now" with integer "now_ns"
This puts an end to the occasional confusion between the "now" date
that is internal, monotonic and not synchronized with the system's
date, and "date" which is the system's date and not necessarily
monotonic. Variable "now" was removed and replaced with a 64-bit
integer "now_ns" which is a counter of nanoseconds. It wraps every
585 years, so if all goes well (i.e. if humanity does not need
haproxy anymore in 500 years), it will just never wrap. This implies
that now_ns is never nul and that the zero value can reliably be used
as "not set yet" for a timestamp if needed. This will also simplify
date checks where it becomes possible again to do "date1<date2".

All occurrences of "tv_to_ns(&now)" were simply replaced by "now_ns".
Due to the intricacies between now, global_now and now_offset, all 3
had to be turned to nanoseconds at once. It's not a problem since all
of them were solely used in 3 functions in clock.c, but they make the
patch look bigger than it really  is.

The clock_update_local_date() and clock_update_global_date() functions
are now much simpler as there's no need anymore to perform conversions
nor to round the timeval up or down.

The wrapping continues to happen by presetting the internal offset in
the short future so that the 32-bit now_ms continues to wrap 20 seconds
after boot.

The start_time used to calculate uptime can still be turned to
nanoseconds now. One interrogation concerns global_now_ms which is used
only for the freq counters. It's unclear whether there's more value in
using two variables that need to be synchronized sequentially like today
or to just use global_now_ns divided by 1 million. Both approaches will
work equally well on modern systems, the difference might come from
smaller ones. Better not change anyhting for now.

One benefit of the new approach is that we now have an internal date
with a resolution of the nanosecond and the precision of the microsecond,
which can be useful to extend some measurements given that timestamps
also have this resolution.
2023-04-28 16:08:08 +02:00
Willy Tarreau
eed5da1037 MINOR: clock: do not use now.tv_sec anymore
Instead we're using ns_to_sec(tv_to_ns(&now)) which allows the tv_sec
part to disappear. At this point, "now" is only used as a timeval in
clock.c where it is updated.
2023-04-28 16:08:08 +02:00
Christopher Faulet
208c712b40 MINOR: stconn: Rename SC_FL_SHUTW in SC_FL_SHUT_DONE
Here again, it is just a flag renaming. In SC flags, there is no longer
shutdown for writes but shutdowns.
2023-04-14 15:01:21 +02:00
Olivier Houchard
db573e9c58 MINOR: compression: Store algo and type for both request and response
Make provision for being able to store both compression algorithms and
content-types to compress for both requests and responses. For now only
the responses one are used.
2023-04-07 00:46:59 +02:00
Aurelien DARRAGON
4e5e26641d MINOR: proxy: add findserver_unique_id() and findserver_unique_name()
Adding alternative findserver() functions to be able to perform an
unique match based on name or puid and by leveraging revision id (rid)
to make sure the function won't match with a new server reusing the
same name or puid of the "potentially deleted" server we were initially
looking for.

For example, if you were in the position of finding a server based on
a given name provided to you by a different context:

Since dynamic servers were implemented, between the time the name was
picked and the time you will perform the findserver() call some dynamic
server deletion/additions could've been performed in the mean time.

In such cases, findserver() could return a new server that re-uses the
name of a previously deleted server. Depending on your needs, it could
be perfectly fine, but there are some cases where you want to lookup
the original server that was provided to you (if it still exists).
2023-04-05 08:58:17 +02:00
Christopher Faulet
7faac7cf34 MINOR: tree-wide: Simplifiy some tests on SHUT flags by accessing SCs directly
At many places, we simplify the tests on SHUT flags to remove calls to
chn_prod() or chn_cons() function because the corresponding SC is available.
2023-04-05 08:57:06 +02:00
Christopher Faulet
87633c3a11 MEDIUM: tree-wide: Move flags about shut from the channel to the SC
The purpose of this patch is only a one-to-one replacement, as far as
possible.

CF_SHUTR(_NOW) and CF_SHUTW(_NOW) flags are now carried by the
stream-connecter. CF_ prefix is replaced by SC_FL_ one. Of course, it is not
so simple because at many places, we were testing if a channel was shut for
reads and writes in same time. To do the same, shut for reads must be tested
on one side on the SC and shut for writes on the other side on the opposite
SC. A special care was taken with process_stream(). flags of SCs must be
saved to be able to detect changes, just like for the channels.
2023-04-05 08:57:06 +02:00
Christopher Faulet
68ef218a72 MINOR: stconn/channel: Move CF_NEVER_WAIT into the SC and rename it
The channel flag CF_NEVER_WAIT is renamed to SC_FL_SND_NEVERWAIT and moved
into the stream-connector.
2023-04-05 08:57:05 +02:00
Aurelien DARRAGON
7f01f0a8ef BUG/MEDIUM: proxy/sktable: prevent watchdog trigger on soft-stop
During soft-stop, manage_proxy() (p->task) will try to purge
trashable (expired and not referenced) sticktable entries,
effectively releasing the process memory to leave some space
for new processes.

This is done by calling stktable_trash_oldest(), immediately
followed by a pool_gc() to give the memory back to the OS.

As already mentioned in dfe7925 ("BUG/MEDIUM: stick-table:
limit the time spent purging old entries"), calling
stktable_trash_oldest() with a huge batch can result in the function
spending too much time searching and purging entries, and ultimately
triggering the watchdog.

Lately, an internal issue was reported in which we could see
that the watchdog is being triggered in stktable_trash_oldest()
on soft-stop (thus initiated by manage_proxy())

According to the report, the crash seems to only occur since 5938021
("BUG/MEDIUM: stick-table: do not leave entries in end of window during purge")

This could be the result of stktable_trash_oldest() now working
as expected, and thus spending a large amount of time purging
entries when called with a large enough <to_batch>.

Instead of adding new checks in stktable_trash_oldest(), here we
chose to address the issue directly in manage_proxy().

Since the stktable_trash_oldest() function is called with
<to_batch> == <p->table->current>, it's pretty obvious that it could
cause some issues during soft-stop if a large table, assuming it is
full prior to the soft-stop, suddenly sees most of its entries
becoming trashable because of the soft-stop.

Moreover, we should note that the call to stktable_trash_oldest() is
immediately followed by a call to pool_gc():

We know for sure that pool_gc(), as it involves malloc_trim() on
glibc, is rather expensive, and the more memory to reclaim,
the longer the call.

We need to ensure that both stktable_trash_oldest() + consequent
pool_gc() call both theoretically fit in a single task execution window
to avoid contention, and thus prevent the watchdog from being triggered.

To do this, we now allocate a "budget" for each purging attempt.
budget is maxed out to 32K, it means that each sticktable cleanup
attempt will trash at most 32K entries.

32K value is quite arbitrary here, and might need to be adjusted or
even deducted from other parameters if this fails to properly address
the issue without introducing new side-effects.
The goal is to find a good balance between the max duration of each
cleanup batch and the frequency of (expensive) pool_gc() calls.

If most of the budget is actually spent trashing entries, then the task
will immediately be rescheduled to continue the purge.
This way, the purge is effectively batched over multiple task runs.

This may be slowly backported to all stable versions.
[Please note that this commit depends on 6e1fe25 ("MINOR: proxy/pool:
prevent unnecessary calls to pool_gc()")]
2023-03-31 07:05:08 +02:00
Aurelien DARRAGON
2c5b9ded9b CLEANUP: proxy: remove stop_time related dead code
Since eb77824 ("MEDIUM: proxy: remove the deprecated "grace" keyword"),
stop_time is never set, so the related code in manage_proxy() is not
relevant anymore.

Removing code that refers to p->stop_time, since it was probably
overlooked.
2023-03-28 20:26:47 +02:00
Aurelien DARRAGON
6e1fe253b7 MINOR: proxy/pool: prevent unnecessary calls to pool_gc()
Under certain soft-stopping conditions (ie: sticktable attached to proxy
and in-progress connections to the proxy that prevent haproxy from
exiting), manage_proxy() (p->task) will wake up every second to perform
a cleanup attempt on the proxy sticktable (to purge unused entries).

However, as reported by TimWolla in GH #2091, it was found that a
systematic call to pool_gc() could cause some CPU waste, mainly
because malloc_trim() (which is rather expensive) is being called
for each pool_gc() invocation.

As a result, such soft-stopping process could be spending a significant
amount of time in the malloc_trim->madvise() syscall for nothing.

Example "strace -c -f -p `pidof haproxy`" output (taken from
Tim's report):

% time     seconds  usecs/call     calls    errors syscall
------ ----------- ----------- --------- --------- ----------------
 46.77    1.840549        3941       467         1 epoll_wait
 43.82    1.724708          13    128509           sched_yield
  8.82    0.346968          11     29696           madvise
  0.58    0.023011          24       951           clock_gettime
  0.01    0.000257          10        25         7 recvfrom
  0.00    0.000033          11         3           sendto
  0.00    0.000021          21         1           rt_sigreturn
  0.00    0.000021          21         1           timer_settime
------ ----------- ----------- --------- --------- ----------------
100.00    3.935568          24    159653         8 total

To prevent this, we now only call pool_gc() when some memory is
really expected to be reclaimed as a direct result of the previous
stick table cleanup.
This is pretty straightforward since stktable_trash_oldest() returns
the number of trashed sticky sessions.

This may be backported to every stable versions.
2023-03-28 20:26:38 +02:00
Christopher Faulet
48678e483f BUG/MEDIUM: proxy: properly stop backends on soft-stop
On soft-stop, we must properlu stop backends and not only proxies with at
least a listener. This is mandatory in order to stop the health checks. A
previous fix was provided to do so (ba29687bc1 "BUG/MEDIUM: proxy: properly
stop backends"). However, only stop_proxy() function was fixed. When HAproxy
is stopped, this function is no longer used. So the same kind of fix must be
done on do_soft_stop_now().

This patch partially fixes the issue #1874. It must be backported as far as
2.4.
2023-03-14 15:23:55 +01:00
Aurelien DARRAGON
d3ffba4512 MINOR: listener: pause_listener() becomes suspend_listener()
We are simply renaming pause_listener() to suspend_listener() to prevent
confusion around listener pausing.

A suspended listener can be in two differents valid states:
 - LI_PAUSED: the listener is effectively paused, it will unpause on
   resume_listener()
 - LI_ASSIGNED (not bound): the listener does not support the LI_PAUSED
   state, so it was unbound to satisfy the suspend request, it will
   correcly re-bind on resume_listener()

Besides that, we add the LI_F_SUSPENDED flag to mark suspended listeners in
suspend_listener() and unmark them in resume_listener().

We're also adding li_suspend proxy variable to track the number of currently
suspended listeners:
That is, the number of listeners that were suspended through suspend_listener()
and that are either in LI_PAUSED or LI_ASSIGNED state.

Counter is increased on successful suspend in suspend_listener() and it is
decreased on successful resume in resume_listener()

--
Backport notes:

-> 2.4 only, as "MINOR: proxy/listener: support for additional PAUSED state"
was not backported:

Replace this:

    |                /* PROXY_LOCK is require
    |                proxy_cond_resume(px);

By this:

    |                ha_warning("Resumed %s %s.\n", proxy_cap_str(px->cap), px->id);
    |                send_log(px, LOG_WARNING, "Resumed %s %s.\n", proxy_cap_str(px->cap), px->id);

-> 2.6 and 2.7 only, as "MINOR: listener: make sure we don't pause/resume" was
custom patched:

Replace this:

    |@@ -253,6 +253,7 @@ struct listener {
    |
    | /* listener flags (16 bits) */
    | #define LI_F_FINALIZED           0x0001  /* listener made it to the READY||LIMITED||FULL state at least once, may be suspended/resumed safely */
    |+#define LI_F_SUSPENDED           0x0002  /* listener has been suspended using suspend_listener(), it is either is LI_PAUSED or LI_ASSIGNED state */
    |
    | /* Descriptor for a "bind" keyword. The ->parse() function returns 0 in case of
    |  * success, or a combination of ERR_* flags if an error is encountered. The

By this:

    |@@ -222,6 +222,7 @@ struct li_per_thread {
    |
    | #define LI_F_QUIC_LISTENER       0x00000001  /* listener uses proto quic */
    | #define LI_F_FINALIZED           0x00000002  /* listener made it to the READY||LIMITED||FULL state at least once, may be suspended/resumed safely */
    |+#define LI_F_SUSPENDED           0x00000004  /* listener has been suspended using suspend_listener(), it is either is LI_PAUSED or LI_ASSIGNED state */
    |
    | /* The listener will be directly referenced by the fdtab[] which holds its
    |  * socket. The listener provides the protocol-specific accept() function to
2023-02-23 15:05:05 +01:00
Aurelien DARRAGON
f5d98938ad MINOR: listener: workaround for closing a tiny race between resume_listener() and stopping
This is an alternative fix that tries to address the same issue as
d1ebee177 ("BUG/MINOR: listener: close tiny race between
resume_listener() and stopping") while allowing resume_listener() to be
more versatile.

Indeed, because of the previous fix, resume_listener() is not able to
rebind stopped listeners, and this breaks the original behavior that is
documented in the function description:

 "If the listener was only in the assigned
  state, it's totally rebound. This can happen if a pause() has completely
  stopped it. If the resume fails, 0 is returned and an error might be
  displayed."

With relax_listener(), we now make sure to check l->state under the
listener lock so we don't call resume_listener() when the conditions are not
met.

As such, concurrently stopped listeners may not be rebound using
relax_listener().

Note: the documented race can't happen since 1b927eb3c ("MEDIUM: proto: stop
protocols under thread isolation during soft stop"), but older versions are
concerned as 1b927eb3c was not marked for backports.
Moreover, the patch also prevents the race between protocol_pause_all() and
resuming from LIMITED or FULL states.

This commit depends on:
  - "MINOR: listener: add relax_listener() function"

This should be backported with d1ebee177 up to 2.4
(d1ebee177 is marked to be backported for all stable versions but the current
patch does not apply for versions < 2.4)
2023-02-23 15:05:05 +01:00
Aurelien DARRAGON
4059e094db MINOR: listener/api: add lli hint to listener functions
Add listener lock hint (AKA lli) to (stop/resume/pause)_listener() functions.
All these functions implicitely take the listener lock when they are called:
It could be useful to be able to call them while already holding the lock, so
we're adding lli hint to make them take the lock only when it is missing.

This should only be backported if explicitly required by another commit
--

-> 2.4 and 2.5 common backport notes:

These 2 commits need to be backported first:
 - 187396e34 "CLEANUP: listener: function comment typo in stop_listener()"
 - a57786e87 "BUG/MINOR: listener: null pointer dereference suspected by
   coverity"

-> 2.4 special backport notes:

In addition to the previously mentionned dependencies, the patch needs to be
slightly adapted to match the corresponding contextual lines:

Replace this:

    |@@ -471,7 +474,8 @@ int pause_listener(struct listener *l, int lpx)
    |        if (!lpx && px)
    |                HA_RWLOCK_WRLOCK(PROXY_LOCK, &px->lock);
    |
    |-       HA_RWLOCK_WRLOCK(LISTENER_LOCK, &l->lock);
    |+       if (!lli)
    |+               HA_RWLOCK_WRLOCK(LISTENER_LOCK, &l->lock);
    |
    |        if (l->state <= LI_PAUSED)
    |                goto end;

By this:

    |@@ -471,7 +474,8 @@ int pause_listener(struct listener *l, int lpx)
    |        if (!lpx && px)
    |                HA_RWLOCK_WRLOCK(PROXY_LOCK, &px->lock);
    |
    |-       HA_RWLOCK_WRLOCK(LISTENER_LOCK, &l->lock);
    |+       if (!lli)
    |+               HA_RWLOCK_WRLOCK(LISTENER_LOCK, &l->lock);
    |
    |        if ((global.mode & (MODE_DAEMON | MODE_MWORKER)) &&
    |            !(proc_mask(l->rx.settings->bind_proc) & pid_bit))

Replace this:

    |@@ -169,7 +169,7 @@ void protocol_stop_now(void)
    |        HA_SPIN_LOCK(PROTO_LOCK, &proto_lock);
    |        list_for_each_entry(proto, &protocols, list) {
    |                list_for_each_entry_safe(listener, lback, &proto->receivers, rx.proto_list)
    |-                       stop_listener(listener, 0, 1);
    |+                       stop_listener(listener, 0, 1, 0);
    |        }
    |        HA_SPIN_UNLOCK(PROTO_LOCK, &proto_lock);
    | }

By this:

    |@@ -169,7 +169,7 @@ void protocol_stop_now(void)
    |        HA_SPIN_LOCK(PROTO_LOCK, &proto_lock);
    |        list_for_each_entry(proto, &protocols, list) {
    |                list_for_each_entry_safe(listener, lback, &proto->receivers, rx.proto_list)
    |                        if (!listener->bind_conf->frontend->grace)
    |-                               stop_listener(listener, 0, 1);
    |+                               stop_listener(listener, 0, 1, 0);
    |        }
    |        HA_SPIN_UNLOCK(PROTO_LOCK, &proto_lock);

Replace this:

    |@@ -2315,7 +2315,7 @@ void stop_proxy(struct proxy *p)
    |        HA_RWLOCK_WRLOCK(PROXY_LOCK, &p->lock);
    |
    |        list_for_each_entry(l, &p->conf.listeners, by_fe)
    |-               stop_listener(l, 1, 0);
    |+               stop_listener(l, 1, 0, 0);
    |
    |        if (!(p->flags & (PR_FL_DISABLED|PR_FL_STOPPED)) && !p->li_ready) {
    |                /* might be just a backend */

By this:

    |@@ -2315,7 +2315,7 @@ void stop_proxy(struct proxy *p)
    |        HA_RWLOCK_WRLOCK(PROXY_LOCK, &p->lock);
    |
    |        list_for_each_entry(l, &p->conf.listeners, by_fe)
    |-               stop_listener(l, 1, 0);
    |+               stop_listener(l, 1, 0, 0);
    |
    |        if (!p->disabled && !p->li_ready) {
    |                /* might be just a backend */
2023-02-23 15:05:05 +01:00
Christopher Faulet
be5cc766b0 MINOR: stconn: Remove half-closed timeout
The half-closed timeout is now directly retrieved from the proxy
settings. There is no longer usage for the .hcto field in the stconn
structure. So let's remove it.
2023-02-22 15:59:16 +01:00
Willy Tarreau
7866e8e50d MEDIUM: listener: move the analysers mask to the bind_conf
When bind_conf were created, some elements such as the analysers mask
ought to have moved there but that wasn't the case. Now that it's
getting clearer that bind_conf provides all binding parameters and
the listener is essentially a listener on an address, it's starting
to get really confusing to keep such parameters in the listener, so
let's move the mask to the bind_conf. We also take this opportunity
for pre-setting the mask to the frontend's upon initalization. Now
several loops have one less argument to take care of.
2023-02-03 18:00:20 +01:00
Aurelien DARRAGON
b2e2ec51b3 MEDIUM: proxy/http_ext: implement dynamic http_ext
proxy http-only options implemented in http_ext were statically stored
within proxy struct.

We're making some changes so that http_ext are now stored in a dynamically
allocated structs.
http_ext related structs are only allocated when needed to save some space
whenever possible, and they are automatically freed upon proxy deletion.

Related PX_O_HTTP{7239,XFF,XOT) option flags were removed because we're now
considering an http_ext option as 'active' if it is allocated (ptr is not NULL)

A few checks (and BUG_ON) were added to make these changes safe because
it adds some (acceptable) complexity to the previous design.

Also, proxy.http was renamed to proxy.http_ext to make things more explicit.
2023-01-27 15:18:59 +01:00
Aurelien DARRAGON
f958341610 MINOR: proxy: move 'originalto' option to http_ext
Just like forwarded (7239) header and forwardfor header, move parsing,
logic and management of 'originalto' option into http_ext dedicated class.

We're only doing this to standardize proxy http options management.
Existing behavior remains untouched.
2023-01-27 15:18:59 +01:00
Aurelien DARRAGON
730b9836a6 MINOR: proxy: move 'forwardfor' option to http_ext
Just like forwarded (7239) header, move parsing, logic and management
of 'forwardfor' option into http_ext dedicated class.

We're only doing this to standardize proxy http options management.
Existing behavior remains untouched.
2023-01-27 15:18:59 +01:00