haproxy-3.1.git
9 months agoBUG/MINOR: stats: fix segfault caused by uninitialized value in "show schema json"
Aurelien DARRAGON [Mon, 23 Dec 2024 08:35:15 +0000 (09:35 +0100)]
BUG/MINOR: stats: fix segfault caused by uninitialized value in "show schema json"

Since b3d5708 ("MINOR: stats: remove implicit static trash_chunk usage")
a segfault can occur when issuing "show schema json" on the stats socket.

Indeed, now the dumping functions don't rely on trash_chunk anymore, but
instead they rely on the appctx->chunk buffer. However, unlike other
stats dumping commands, the "show schema json" only have an io handler,
and no parse function. With other command, the parse function is
responsible for pre-setting some data, including applet ctx reservation.

Thus due to "show schema json" lacking parsing function, the applet ctx is
used uninitialized, which is a bug obviously.

To fix the issue we simply add a parse function for "show schema json",
although all it does for now is calling applet_reserve_svcctx() for the
current applet ctx.

This issue was reported by @dsuch in GH #2825. It must be backported up
to 3.0.

(cherry picked from commit ac1f413590932d4eb3b78ca5a536845f162b7866)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

9 months agoBUG/MEDIUM: queue: Make process_srv_queue return the number of streams
Olivier Houchard [Mon, 23 Dec 2024 14:17:25 +0000 (14:17 +0000)]
BUG/MEDIUM: queue: Make process_srv_queue return the number of streams

Make process_srv_queue() return the number of streams unqueued, as
pendconn_grab_from_px() did, as that number is used by
srv_update_status() to generate logs.

This should be backported up to 2.6 with
111ea83ed4e13ac3ab028ed5e95201a1b4aa82b8

(cherry picked from commit 5b8899b6ccc7dab3a54a51dcb8ba1512bd0c886c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

9 months agoMINOR: hlua: rename "tune.lua.preserve-smp-bool" to "tune.lua.bool-sample-conversion"
Aurelien DARRAGON [Fri, 20 Dec 2024 16:25:29 +0000 (17:25 +0100)]
MINOR: hlua: rename "tune.lua.preserve-smp-bool" to "tune.lua.bool-sample-conversion"

A better name was found for the option implemented in ec74438
("MINOR: hlua: add option to preserve bool type from smp to lua")

Indeed, "tune.lua.preserve-smp-bool {on | off}" wasn't explicit enough
nor did it encourage the adoption of the new "fixed" behavior (vs
historical behavior which is now considered as a bug).

Thus it becomes "tune.lua.bool-sample-conversion { normal | pre-3.1-bug }"
which actively encourage users to switch the new behavior after having
patched in-use Lua script if needed. From a technical point of view,
the logic remains the same, as the option currently defaults to
"pre-3.1-bug" to prevent script breakage, and a warning is emitted if
the option isn't set explicily and Lua is used.

Documentation and regtests were updated.

Must be backported in 3.1 with ec74438 and f2838f5 ("REGTESTS: fix
lua-based regtests using tune.lua.smp-preserve-bool")

(cherry picked from commit 29b6d8af16884bb8f0b5cd83a3752980df8548d0)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

9 months agoBUG/MINOR: h2/rhttp: fix HTTP2 conn counters on reverse
Amaury Denoyelle [Mon, 16 Dec 2024 10:00:23 +0000 (11:00 +0100)]
BUG/MINOR: h2/rhttp: fix HTTP2 conn counters on reverse

Dedicated HTTP/2 stats proxy counters are available for current and
total number of HTTP/2 connection on both frontend and backend sides.
Both counters are simply incremented into h2_init().

This causes issues when using reverse HTTP. First, increment is not
performed on the expected side, as it is triggered before
h2_conn_reverse() which switches a connection from frontend to backend
or vice versa. For example on active revers side, h2_total_connections
is incremented on the backend only even after connection is reversed and
attached to a listener for the remainder of its lifetime.

h2_open_connections suffers from a similar but arguably worst behavior
as it is also decremented. If increment and decrement operations are not
performed on the same proxy side, which happens for every connection
which has been successfully reversed, it causes an invalid counter
value, possibly with an integer overflow.

To fix this, delay increment operations on reverse HTTP from h2_init()
to h2_conn_reverse(). Both counters are updated only after reverse has
completed, thus using the expected frontend or backend side.

To prevent overflow on h2_open_connections, ensure h2_release()
decrement is not performed if a connection is freed before achieving its
reversal, as in this case it would not have been accounted by H2
counters.

This should be backported up to 2.9.

This should fix github issue #2821.

(cherry picked from commit 8633446337cd322f010ce7b3ca9c257d1ce7848c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

9 months agoCLEANUP: mux-quic: remove dead err label in qcc_build_frms()
Amaury Denoyelle [Thu, 19 Dec 2024 15:34:18 +0000 (16:34 +0100)]
CLEANUP: mux-quic: remove dead err label in qcc_build_frms()

STREAM frames emission in qcc_build_frms() has been splitted from
RESET_STREAM/STOP_SENDING into qcc_emit_rs_ss(). Now, the former cannot
fail, as such err label can be removed as it is unreachable.

This should be backported up to 3.1.

This should fix github issue #2824.

(cherry picked from commit 4490df57a6d6d603077c1ecbb69c67a07bc9e3d6)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

9 months agoBUG/MEDIUM: mux-quic: prevent BUG_ON() by refreshing frms on MAX_DATA
Amaury Denoyelle [Thu, 19 Dec 2024 13:17:37 +0000 (14:17 +0100)]
BUG/MEDIUM: mux-quic: prevent BUG_ON() by refreshing frms on MAX_DATA

QUIC MUX emission has been optimized recently by recycling STREAM frames
list between emission cycles. This is done via qcc frms list member. If
new data is available, frames list must be cleared before the next
emission to force the encoding of new STREAM frames.

If a refresh frames list is missed, it would lead to incomplete data
emission on the next transfer. In most cases, this is detected via a
BUG_ON() inside qcc_io_send(), as qcs instances remains in send_list
after a qcc_send_frames() full emission.

A bug was recently found which causes this BUG_ON() crash. This is
directly related to flow control. Indeed, when sending credit is
increased on the connection or a stream, frames list should be cleared
as new larger STREAM frames could be encoded. This was already performed
on MAX_DATA/MAX_STREAM_DATA reception but only if flow-control limit was
unblocked. However this is not the proper condition and it may lead to
insufficient frames refresh and thus this BUG_ON() crash.

Fix this by adjusting the condition for frames refresh on flow control
credit increase. Now, frames list is cleared if real offset is not
blocked and soft offset was equal or greater to the previous limit.
Indeed, this is the only case in which frames refreshing is necessary as
it would result in bigger encoded STREAM frames.

This bug was detected on QUIC interop with go-x-net client. It can also
be reproduced, albeit not systematically, using the following command :
  $ ngtcp2-client -q --no-quic-dump --no-http-dump \
    --exit-on-all-streams-close --max-data 10 \
    127.0.0.1 20443 -n10 "http://127.0.0.1:20443/?s=10k"

This bug appeared with the following patch. As it is scheduled for 3.1
backporting, the current fix should be backported with it.
  14710b5e6bf76834343d58db22e00b72590b16fe
  MEDIUM/OPTIM: mux-quic: do not rebuild frms list on every send

(cherry picked from commit 7edb2ffae7e899af58a6b4fe546ac0665f5d3c9d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

9 months agoREGTESTS: fix lua-based regtests using tune.lua.smp-preserve-bool
Aurelien DARRAGON [Thu, 19 Dec 2024 13:16:27 +0000 (14:16 +0100)]
REGTESTS: fix lua-based regtests using tune.lua.smp-preserve-bool

Because of the previous commit, configs making use of lua script without
setting "tune.lua.smp-preserve-bool" explicitly now raise a warning.

However, since 6f746af91 ("REGTESTS: use -dW by default on every
reg-tests"), regtests are not allowed to raise warnings anymore.

Because of this the CI now fails for every tests that relies on Lua.
To fix this, let's explicitly set the "tune.lua.smp-preserve-bool" for
all tests involving Lua. Here we set the value to "on" because we know
it is safe to do so, and this way it will be future-proof.

If ec7443827 ("MINOR: hlua: add option to preserve bool type from smp to
lua") is backported, then this patch must be backported with it (if it
is not trivial to backport, then simply follow this rule: grep for
"lua-load" in reg-tests directory, then for each match, make sure to set
the tune.smp-preserve-bool tunable in the global section.

(cherry picked from commit f2838f5172f163842c8b4d1bd2f55c9138e84573)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

9 months agoMINOR: hlua: add option to preserve bool type from smp to lua
Aurelien DARRAGON [Thu, 19 Dec 2024 11:32:10 +0000 (12:32 +0100)]
MINOR: hlua: add option to preserve bool type from smp to lua

As discussed in GH #2814, there is an ambiguity in hlua implementation
that causes haproxy smp boolean type to be pushed as an integer on the
Lua stack. On the other hand, when doing Lua to haproxy smp conversion,
the boolean type is properly perserved. Of course this situation is not
desirable and can lead to unexpected results. However we cannot simply
fix the behavior because in Lua boolean and integer types are not
are completely distinct types and cannot be used interchangeably. So in
order to prevent breaking existing scripts logic, in this patch we add a
dedicated lua tunable named "tune.lua.smp-preserve-bool" which can take
the following values:

  - "on" : when converting haproxy smp to lua, boolean type is preserved
  - "off": when converting haproxy smp to lua, boolean is converted to
           integer (legacy behavior)

For now, the tunable defaults to "off" to preserve historical behavior.
However, when the option isn't set explicitly and lua is used, a warning
will be emitted in order to raise user's awareness about this ambiguity.
It is expected that the tunable could default to "on" in future versions,
thus it is recommended to avoid setting it to "off" except when using
existing Lua scripts that still rely on the old behavior regarding boolean
smp to Lua conversion, and that they cannot be fixed easily.

This should solve issue GH #2814. It may be relevant to backport this in
haproxy 3.1.

(cherry picked from commit ec744382739d24681b7ccd08dd57ee9606870bc5)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

9 months agoDOC: config: add "tune.lua.burst-timeout" to the list of global parameters
Aurelien DARRAGON [Thu, 19 Dec 2024 08:25:21 +0000 (09:25 +0100)]
DOC: config: add "tune.lua.burst-timeout" to the list of global parameters

"tune.lua.burst-timeout" was properly defined but not listed in the list
of global parameters as it was overlooked in 58e36e5b1 ("MEDIUM: hlua:
introduce tune.lua.burst-timeout")

(cherry picked from commit 67e3270c59f4cb997f2078d395d365e0189326ba)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

9 months agoDOC: config: reorder "tune.lua.*" keywords by alphabetical order
Aurelien DARRAGON [Thu, 19 Dec 2024 08:08:01 +0000 (09:08 +0100)]
DOC: config: reorder "tune.lua.*" keywords by alphabetical order

Effort was made to properly organize "tune.*" keywords by alphabetical
order, but "tune.lua" keywords didn't follow that rule with care.

Let's fix that.

(cherry picked from commit 985a45d9c77568798b5c0c77cae7ddba219f3b63)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

9 months agoDOC: config: add example for server "track" keyword
Aurelien DARRAGON [Tue, 17 Dec 2024 10:54:06 +0000 (11:54 +0100)]
DOC: config: add example for server "track" keyword

As requested on GH #2325, "track" server keyword could benefit from a
simple config example to show how to make use of it.

That's what we're doing in this commit, thanks to GH user @HAkmiller
for the suggestion.

(cherry picked from commit 48545113f4db86ad43a99ba01742a9811410e6dc)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

9 months agoMINOR: mux-quic: hide traces when woken up on pacing only
Amaury Denoyelle [Thu, 12 Dec 2024 14:28:35 +0000 (15:28 +0100)]
MINOR: mux-quic: hide traces when woken up on pacing only

Previous commit aligned default and pacing emission. This is a cleaner
and more robust code. However, it may disrupt traces analysis when
pacing is rescheduled until timer expiration.

Hide traces when qcc_io_cb() is woken up only due to pacing and timer is
not yet expired. This is implemented by using special TASK_WOKEN_IO for
pacing.

This should be backported up to 3.1.

(cherry picked from commit 53db43aff231d83ea6f6bcaf99f5bdb5a0330e7b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

9 months agoMINOR: trace: implement tracing disabling API
Amaury Denoyelle [Tue, 17 Dec 2024 15:28:16 +0000 (16:28 +0100)]
MINOR: trace: implement tracing disabling API

Define a set of functions to temporarily disable/reactivate tracing for
the current thread. This could be useful when wanting to quickly remove
tracing output for some code parts.

The API relies on a disable/resume set of functions, with a thread-local
counter. This counter is tested under __trace_enabled(). It is a
cumulative value so that the same count of resume must be issued after
several disable usage. There is also the possibility to force reset the
counter to 0 before restoring the old value.

This should be backported up to 3.1.

(cherry picked from commit 9d155ca7064290d3280621c85443221fa13334b2)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

9 months agoMEDIUM: mux-quic: remove pacing specific code on qcc_io_cb
Amaury Denoyelle [Thu, 12 Dec 2024 13:53:49 +0000 (14:53 +0100)]
MEDIUM: mux-quic: remove pacing specific code on qcc_io_cb

Pacing was recently implemented by QUIC MUX. Its tasklet is rescheduled
until next emission timer is reached. To improve performance, an
alternate execution of qcc_io_cb was performed when rescheduled due to
pacing. This was implemented using TASK_F_USR1 flag.

However, this model is fragile, in particular when several events
happened alongside pacing scheduling. This has caused some issue
recently, most notably when MUX is subscribed on transport layer on
receive for handshake completion while pacing emission is performed in
parallel. MUX qcc_io_cb() would not execute the default code path, which
means the reception event is silently ignored.

Recent patches have reworked several parts of qcc_io_cb. The objective
was to improve performance with better algorithm on send and receive
part. Most notable, qcc frames list is only cleared when new data is
available for emission. With this, pacing alternative code is now mostly
unneeded. As such, this patch removes it. The following changes are
performed :

* TASK_F_USR1 is now not used by QUIC MUX. As such, tasklet_wakeup()
  default invokation can now replace obsolete wrappers
  qcc_wakeup/qcc_wakeup_pacing

* qcc_purge_sending is removed. On pacing rescheduling, all qcc_io_cb()
  is executed. This is less error-prone, in particular when pacing is
  mixed with other events like receive handling. This renders the code
  less fragile, as it completely solves the described issue above.

This should be backported up to 3.1.

(cherry picked from commit 41f0472d967b2deb095d5adc8a167da973fbee3d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

9 months agoMEDIUM/OPTIM: mux-quic: do not rebuild frms list on every send
Amaury Denoyelle [Thu, 12 Dec 2024 11:03:37 +0000 (12:03 +0100)]
MEDIUM/OPTIM: mux-quic: do not rebuild frms list on every send

A newly introduced frames list member has been defined into QCC instance
with pacing implementation. This allowed to preserve STREAM frames built
between different emission scheduled by pacing, without having to
regenerate it if no new QCS data is available.

Generalize this principle outside of pacing scheduling. Now, the frames
list will be reused accross several qcc_io_send() usage. Frames list is
only cleared when necessary. This will force its refreshing in the next
qcc_io_send() via qcc_build_frms_list().

Frames list refreshing is performed in the following cases :
* on successful transfer from stream snd_buf / done_ff / shut
* on stream reset or read abort
* on max_data/max_stream_data reception with window increase

Note that the two first cases are in fact covered directly due to
qcc_send_stream() usage when QCS is (re)inserted into the send_list.

The main objective of this patch will be to remove QUIC MUX pacing
specific code path. It could also provide better performance as emission
of large frames may often be rescheduled due to transport layer, either
on congestion or full socket buffer. When QUIC MUX is rescheduled, no
new data is available and frames list can be reuse as-is, avoiding an
unecessary loop over send_list.

This should be backported up to 3.1.

(cherry picked from commit 14710b5e6bf76834343d58db22e00b72590b16fe)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

9 months agoMINOR: mux-quic: split STREAM and RS/SS emission
Amaury Denoyelle [Fri, 13 Dec 2024 16:44:38 +0000 (17:44 +0100)]
MINOR: mux-quic: split STREAM and RS/SS emission

This commit is a follow-up of the previous one which defines function
qcc_build_frms(). This function implements looping over qcc send_list,
to both encode and send individually any STOP_SENDING and RESET_STREAM,
but also encode STREAM frames as a preparator step. STREAM frames were
then sent as a list outside of qcc_build_frms() via qcc_send_frames().

Extract STOP_SENDING/RESET_STREAM encoding and emission step into a new
function qcc_emit_rs_ss(). The code is thus cleaner. In particular it
highlights that an error during STOP_SENDING/RESET_STREAM emission stage
is fatal and prevent any STREAM frames processing.

This should be backported up to 3.1.

(cherry picked from commit 9ecc1a8e578c67e8653ba7d1e9c5441f5ad2eca8)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

9 months agoMINOR: mux-quic: extract code to build STREAM frames list
Amaury Denoyelle [Wed, 11 Dec 2024 16:53:29 +0000 (17:53 +0100)]
MINOR: mux-quic: extract code to build STREAM frames list

Extracts code responsible to generate STREAM, RESET_STREAM and
STOP_SENDING frames for each qcs instances registered in qcc send_list.
It is moved from qcc_io_send() to its owned new function
qcc_build_frms().

This commit does not bring functional change. It is a preparatory step
to adapt QUIC MUX send mechanism to allow reusing of qcc frms list
accross qcc_io_send() invokation.

As a side change, qcc_tx_frms_free() is renamed to qcc_clear_frms().
This better highlights its relationship with qcc_build_frms().

This should be bkacported up to 3.1.

(cherry picked from commit 244dc00b09868f52864d50fc14af8d981c81424b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

9 months agoMEDIUM/OPTIM: mux-quic: implement purg_list
Amaury Denoyelle [Tue, 17 Dec 2024 10:16:34 +0000 (11:16 +0100)]
MEDIUM/OPTIM: mux-quic: implement purg_list

This commit is part of the current serie which aims to refactor and
improve overall performance of QUIC MUX I/O handler.

qcc_io_process() is responsible to perform some internal operations on
QUIC MUX after I/O completion. It is notably called on every qcc_io_cb()
tasklet handler.

The most intensive work on it is the purging of QCS instances after
transfer completion. This was implemented by looping on QCC streams tree
and inspecting the state of every QCS. The purpose of this commit is to
optimize this processing.

A new purg_list QCC member is defined. It is responsible to list every
QCS instances whose transfer has been completed. It is thus safe to
reuse <el_send> QCS list attach point. Stream purging will thus only
loop on purg_list instead of every known QCS.

This should be backported up to 3.1.

(cherry picked from commit e296585ae9eb86f898614aed2b04929018340ee7)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

9 months agoMEDIUM/OPTIM: mux-quic: define a recv_list for demux resumption
Amaury Denoyelle [Thu, 5 Dec 2024 09:48:51 +0000 (10:48 +0100)]
MEDIUM/OPTIM: mux-quic: define a recv_list for demux resumption

This commit is part of the current serie which aims to refactor and
improve overall performance of QUIC MUX I/O handler.

Define a recv_list element into qcc structure. This is used to
registered every instance of qcs which are currently blocked on
demuxing, which happen on no more space in <rx.appbuf>.

The purpose of this patch is to reduce qcc_io_recv() CPU usage. Now,
only recv_list iteration is performed, instead of the previous looping
over every qcs instances. This is useful as qcc_io_recv() is called each
time qcc_io_cb() is scheduled, even if only sending condition was the
wakeup origin.

A qcs is not inserted into recv_list immediately after blocking on demux
full buffer. Instead, this is only done after unblocking via stream
rcv_buf callback, which ensure that new buffer space is available.

This should be backported up to 3.1.

(cherry picked from commit 4b42dd4ae0d65c62d1979791b50dadd0d42774bb)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

9 months agoMINOR: mux-quic: refactor wait-for-handshake support
Amaury Denoyelle [Fri, 29 Nov 2024 15:18:12 +0000 (16:18 +0100)]
MINOR: mux-quic: refactor wait-for-handshake support

This commit refactors wait-for-handshake support from QUIC MUX. The flag
logic QC_CF_WAIT_HS is inverted : it is now positionned only if MUX is
instantiated before handshake completion. When the handshake is
completed, the flag is removed.

The flag is now set directly on initialization via qmux_init(). Removal
via qcc_wait_for_hs() is moved from qcc_io_process() to qcc_io_recv().
This is deemed more logical as QUIC MUX is scheduled on RECV to be
notify by the transport layer about handshake termination. Moreover,
qcc_wait_for_hs() is now called if recv subscription is still active.

This commit is the first of a serie which aims to refactor QUIC MUX I/O
handler and improves its overall performance. The ultimate objective is
to be able to stream qcc_io_cb() by removing pacing specific code path
via qcc_purge_sending().

This should be backported up to 3.1.

(cherry picked from commit 0a53a008d032b69377869c8caaec38f81bdd5bd6)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

9 months agoMINOR: quic: add traces
Amaury Denoyelle [Thu, 28 Nov 2024 10:27:56 +0000 (11:27 +0100)]
MINOR: quic: add traces

Add some traces to better follow QUIC MUX scheduling, in particular with
pacing interaction.

This should be backported up to 3.1.

(cherry picked from commit 9dcd2369e210f617f738d68f6764ccc229309528)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

9 months agoCLEANUP: mux-quic: remove unused qcc member send_retry_list
Amaury Denoyelle [Thu, 12 Dec 2024 15:29:32 +0000 (16:29 +0100)]
CLEANUP: mux-quic: remove unused qcc member send_retry_list

Remove unused fields send_retry_list from qcc and its corresponding
attach element el from qcs.

This should be backported up to 3.1.

(cherry picked from commit 17bfe93768a1f579de35875cc4cf0e35dd7b6d35)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

9 months agoBUG/MEDIUM: mux-quic: do not mix qcc_io_send() return codes with pacing
Amaury Denoyelle [Wed, 11 Dec 2024 16:55:29 +0000 (17:55 +0100)]
BUG/MEDIUM: mux-quic: do not mix qcc_io_send() return codes with pacing

With pacing implementation, qcc_send_frames() return code has been
extended to report emission interruption due to pacing limitation. This
is used only in qcc_io_send().

However, its invokation may be skipped using 'sent_done' label. This
happens on emission failure of a STOP_SENDING or RESET_STREAM (either
memory allocation failure, or transport layer rejection). In this case,
return values are mixed as qcs_send() is wrongly compared against pacing
interruption condition. This value corresponds to the length of the last
built STREAM frames.

If by mischance the last frame was 1 byte long, qcs_send() return value
is equal to pacing interruption condition. This has several effects. If
pacing is activated, it may lead to unneeded wakeup on QUIC MUX. Worst,
if pacing is not used, a BUG_ON() crash will be triggered.

Fix this by using a different variable dedicated to qcc_send_frames()
return value. By default it is initialized to 0. This ensures that
pacing code won't be activated in case qcc_send_frames() is not used.

This must be backported up to 3.1.

(cherry picked from commit 2e3542bec6235482e4136ddd1e23e8f7cd4b5a26)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

9 months agoBUILD: debug: only dump/reset glitch counters when really defined
Willy Tarreau [Mon, 16 Dec 2024 08:31:27 +0000 (09:31 +0100)]
BUILD: debug: only dump/reset glitch counters when really defined

If neither DEBUG_GLITCHES nor DEBUG_STRICT is set, we end up with
no dbg_cnt section, resulting in debug_parse_cli_counters not
building due to __stop_dbg_cnt and __start_dbg_cnt not being defined.
Let's just condition the end of the function to these conditions.
An alternate approach (less elegant) is to always declare a dummy
entry of type DBG_COUNTER_TYPES in debug.c.

This must be backported to 3.1 since it was brought with glitches.

(cherry picked from commit 4710ab5604298eefddb36887098d5fc388dcd3cb)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

9 months agoBUG/MEDIUM: queues: Do not use pendconn_grab_from_px().
Olivier Houchard [Tue, 17 Dec 2024 14:39:21 +0000 (15:39 +0100)]
BUG/MEDIUM: queues: Do not use pendconn_grab_from_px().

pendconn_grab_from_px() was called when a server was brought back up, to
get some streams waiting in the proxy's queue and get them to run on the
newly available server. It is very similar to process_srv_queue(),
except it only goes through the proxy's queue, which can be a problem,
because there is a small race condition that could lead us to add more
streams to the server queue just as it's going down. If that happens,
the server would just be ignored when back up by new streams, as its
queue is not empty, and it would never try to process its queue.
The other problem with pendconn_grab_from_px() is that it is very
liberal with how it dequeues streams, and it is not very good at
enforcing maxconn, it could lead to having 3*maxconn connections.
For both those reasons, just get rid of pendconn_grab_from_px(), and
just use process_srv_queue().
Both problems are easy to reproduce, especially on a 64 threads machine,
set a maxconn to 100, inject in H2 with 1000 concurrent connections
containing up to 100 streams each, and after a few seconds/minutes the
max number of concurrent output streams will be much higher than
maxconn, and eventually the server will stop processing connections.

It may be related to github issue #2744. Note that it doesn't totally
fix the problem, we can occasionally see a few more connections than
maxconn, but the max that have been observed is 4 more connections, we
no longer get multiple times maxconn.

have more outgoing connections than maxconn,
This should be backported up to 2.6.

(cherry picked from commit 111ea83ed4e13ac3ab028ed5e95201a1b4aa82b8)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

9 months agoBUG/MEDIUM: queues: Make sure we call process_srv_queue() when leaving
Olivier Houchard [Fri, 13 Dec 2024 17:11:05 +0000 (17:11 +0000)]
BUG/MEDIUM: queues: Make sure we call process_srv_queue() when leaving

In stream_free(), make sure we call process_srv_queue() each time we
call sess_change_server(), otherwise a server may end up not dequeuing
any stream when it could do so. In some extreme cases it could lead to
an infinite loop, as the server would appear to be available, as its
"served" parameter would be < maxconn, but would end up not being used,
as there are elements still in its queue.

This should be backported up to 2.6.

(cherry picked from commit dc9ce9c26469e00ab71fe6387dbd13010d4930f0)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

9 months agoBUG/MEDIUM: stconn: Only consider I/O timers to update stream's expiration date
Christopher Faulet [Mon, 16 Dec 2024 10:04:53 +0000 (11:04 +0100)]
BUG/MEDIUM: stconn: Only consider I/O timers to update stream's expiration date

In sc_notify(), it remained a case where it was possible to set an
expiration date on the stream in the past, leading to a crash because of a
BUG_ON(). This must never happen of course.

In sc_notify(), The stream's expiration may be updated in case no wakeup
conditions are encoutered. In that case, we must take care to never set an
expiration date in the past. However, it appeared there was still a
condition to do so. This code is based on an implicit postulate: the
stream's expiration date must always be set when we leave
process_stream(). It was true since the 2.9. But in 3.0, the buffer
allocation mechanism was improved and on an alloc failure in
process_stream(), the stream is inserted in a wait-list and its expiration
date is set to TICK_ETERNITY. With the good timing, and an analysis
expiration date set on a channel, it is possible to set the stream's
expiration date in past.

After analysis, it appeared that the proper way to fix the issue is to only
evaluate I/O timers (read and write timeout) and not stream's timers
(analase_exp or conn_exp) because only I/O timers may have changed since the
last process_stream() call.

This patch must be backported as far as 3.0 to fix the issue. But it is
probably a good idea to also backported it as far as 2.8.

(cherry picked from commit 4f32d03360e42a3e7fe4ed71012ec90339b9731d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

9 months agoCLEANUP: quic: Rename some BBR functions in relation with bw probing
Frederic Lecaille [Fri, 13 Dec 2024 18:36:51 +0000 (19:36 +0100)]
CLEANUP: quic: Rename some BBR functions in relation with bw probing

Rename bbr_is_probing_bw() to bbr_is_in_a_probe_state() and
bbr_is_accelerating_probing_bw() to bbr_is_probing_bw() to match
the function names of the BBR v3 internet draft.

Must be backported to 3.1 to ease any further backport to come.

(cherry picked from commit 949bc18f660d860b9102360d2b98d0c96e60266c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

9 months agoBUG/MINOR: quic: missing Startup accelerating probing bw states
Frederic Lecaille [Fri, 13 Dec 2024 18:27:23 +0000 (19:27 +0100)]
BUG/MINOR: quic: missing Startup accelerating probing bw states

Startup state is also a probing with acceleration bandwidth state.
This modification should have come with this previous one:

  BUG/MINOR: quic: reduce packet losses at least during ProbeBW_CRUISE (BBR)

Must be backported to 3.1.

(cherry picked from commit 0dc0c890ea20f3af76ae4f4f7422718bcd8f6605)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

9 months agoREGTESTS: ssl: add a PEM with mix of LF and CRLF line endings
Valentine Krasnobaeva [Fri, 13 Dec 2024 11:02:14 +0000 (12:02 +0100)]
REGTESTS: ssl: add a PEM with mix of LF and CRLF line endings

User tried to update a PEM, generated automatically. Part of this PEM has LF
line endings, and another part (CA certificate), added by some API, has CRLF
line endings. This has revealed a bug in cli_snd_buf(), see more
details in issue GitHUB #2818. So, let's add an example of such PEM in our
SSL regtest.

(cherry picked from commit ea4a148a7d9ead2132757bffd4699146e80da2df)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

9 months agoBUG/MINOR: cli: cli_snd_buf: preserve \r\n for payload lines
Valentine Krasnobaeva [Thu, 12 Dec 2024 18:38:38 +0000 (19:38 +0100)]
BUG/MINOR: cli: cli_snd_buf: preserve \r\n for payload lines

cli_snd_buf() analyzez input line by line. Before this patch it has always
scanned a given line for the presence of '\r' followed by '\n'.

This is only needed for strings, that contain the commands itself like
"show ssl cert\n", "set ssl cert test.pem <<\n".

In case of strings, which contain the command's payload, like
"-----BEGIN CERTIFICATE-----\r\n", '\r\n' should be preserved
as is.

This patch fixes the GitHub issue #2818.

This patch should be backported in v3.1 and in v3.0.

(cherry picked from commit d60c893991699af1103bcdcd33c915885c056559)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

9 months agoBUG/MINOR: quic: too permissive exit condition for high loss detection in Startup...
Frederic Lecaille [Fri, 13 Dec 2024 10:56:49 +0000 (11:56 +0100)]
BUG/MINOR: quic: too permissive exit condition for high loss detection in Startup (BBR)

This bug fixes the 3rd condition used by bbr_check_startup_high_loss() to decide
it has detected some high loss as mentioned by the BBR v3 RFC draft:

   4.3.1.3. Exiting Startup Based on Packet Loss
   ...
   There are at least BBRStartupFullLossCnt=6 discontiguous sequence ranges lost in that round trip.

where a <= operator was used in place of <.

Must be backported to 3.1.

(cherry picked from commit 178109f608ae1e446d9ea898bf5c83e4e8313bb5)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

9 months agoBUG/MINOR: quic: fix the wrong tracked recovery start time value
Frederic Lecaille [Fri, 13 Dec 2024 07:28:56 +0000 (08:28 +0100)]
BUG/MINOR: quic: fix the wrong tracked recovery start time value

bbr_congestion_event() role is to track the start time of recovery periods.
This was done using <ts> passed as parameter. But this parameter is the
time the newest lost packet has been sent.
The timestamp value to store in ->recovery_start_ts is <now_ms>.

Must be backported to 3.1.

(cherry picked from commit e61b4189078f97d036952371c7fc9e5ceefdfff0)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

9 months agoCLEANUP: quic: remove a wrong comment about ->app_limited (drs)
Frederic Lecaille [Thu, 12 Dec 2024 15:28:37 +0000 (16:28 +0100)]
CLEANUP: quic: remove a wrong comment about ->app_limited (drs)

->app_limited quic_drs struct member is not a boolean. This is
the index of the last transmitted packet marked as application-limited, or 0 if
the connection is not currently application-limited (see C.app_limited
definition in BBR v3 draft).

(cherry picked from commit e1d25cdbddfa344ceee7dcecd6a84eff4a209122)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

9 months agoMINOR: quic: reduce the private data size of QUIC cc algos
Frederic Lecaille [Thu, 12 Dec 2024 11:30:18 +0000 (12:30 +0100)]
MINOR: quic: reduce the private data size of QUIC cc algos

After these commits:

    BUG/MINOR: quic: remove max_bw filter from delivery rate sampling
    BUG/MINOR: quic: fix BBB max bandwidth oscillation issue

where some members were removed from bbr struct, the private data
size of QUIC cc algorithms may be reduced from 160 to 144 uint32_t.

Should be easily backported to 3.1 alonside the commits mentioned above.

(cherry picked from commit eeaeb412dc978bdc9663061d9d2542dc1f7310cb)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

9 months agoBUG/MINOR: quic: reduce packet losses at least during ProbeBW_CRUISE (BBR)
Frederic Lecaille [Thu, 12 Dec 2024 11:15:31 +0000 (12:15 +0100)]
BUG/MINOR: quic: reduce packet losses at least during ProbeBW_CRUISE (BBR)

Upon congestion events (for a instance packet loss),
bbr_adapt_lower_bounds_from_congestion() role is to adapt some BBR internal
variables in relation with the estimated bandwidth (BBR.bw).

According to the BBR v3 draft, this function should do nothing
if BBRIsProbingBW() pseudo-code returns true. That said, this function
is not defined by the BBR v3 draft. But according to this part mentioned before
defining the pseudo-code for BBRAdaptLowerBoundsFromCongestion():

4.5.10.3. When not Probing for Bandwidth
When not explicitly accelerating to probe for bandwidth (Drain, ProbeRTT,
ProbeBW_DOWN, ProbeBW_CRUISE), BBR responds to loss by slowing down to some extent.
This is because loss suggests that the available bandwidth and safe volume of
in-flight data may have decreased recently, and the flow needs to adapt, slowing
down toward the latest delivery process. BBR flows implement this response by
reducing the short-term model parameters, BBR.bw_lo and BBR.inflight_lo.

BBRIsProbingBW() should concern the accelerating probe for bandwidth states
which are BBR_ST_PROBE_BW_REFILL and BBR_ST_PROBE_BW_UP.

Adapt the code to match this latter assumption. At least this reduce
drastically the packet loss volumes at least during ProbeBW_CRUISE.

As an example, on a 100MBits/s internet link with ~94ms as RTT, before
this patch, 4329640 sent packets were needed with 1617119 lost packets (!!!) to
download a 3GB object. After this patch, 2843952 sent packets vs 144134 lost packets
are needed. There may be some packet loss issue. I suspect the maximum bandwidth
which may be overestimated. More this is the case, more the packet loss is big.
That said, at this time, it remains below 5% depending on the size of the objects,
5% being for more than 2GB objects.

Must be backported to 3.1.

(cherry picked from commit 9813de0537882355514794eed7b738bed1806954)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

9 months agoBUG/MINOR: quic: underflow issue for bbr_inflight_hi_from_lost_packet()
Frederic Lecaille [Thu, 12 Dec 2024 11:04:14 +0000 (12:04 +0100)]
BUG/MINOR: quic: underflow issue for bbr_inflight_hi_from_lost_packet()

Add a test to ensure that values of a local variable used by
bbr_inflight_hi_from_lost_packet() is not be impacted by underflow issues
when subtracting too big numbers and make this function return a correct value.

Must be backported to 3.1.

(cherry picked from commit ebfc301d5d8dbae0c07a9d937bef18d5c3f841c6)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

9 months agoBUG/MINOR: quic: remove max_bw filter from delivery rate sampling
Frederic Lecaille [Thu, 12 Dec 2024 10:50:26 +0000 (11:50 +0100)]
BUG/MINOR: quic: remove max_bw filter from delivery rate sampling

This filter is no more needed after this commit:

 BUG/MINOR: quic: fix BBB max bandwidth oscillation issue.

Indeed, one added this filter at delivery rate sampling level to filter
the BBR max bandwidth estimations and was inspired from ngtcp2 code source when
trying to fix the oscillation issue. But this BBR max bandwidth oscillation issue
was fixed by the aforementioned commit.

Furthermore this code tends to always increment the BBR max bandwidth. From my point
of view, this is not a good idea at all.

Must be backported to 3.1.

(cherry picked from commit 22ab45a3a84d0a9a61cc90c6efedd893dd1cbe4a)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

9 months agoBUG/MINOR: quic: wrong bbr_target_inflight() implementation
Frederic Lecaille [Thu, 12 Dec 2024 10:44:10 +0000 (11:44 +0100)]
BUG/MINOR: quic: wrong bbr_target_inflight() implementation

This bug arrived with this commit:

  6404b7a18a BUG/MINOR: quic: fix bbr_inflight() calls with wrong gain value

This patch partially reverts after having checked the BBR v3 draft.
This bug was invisible when testing long BBR flows.

Must be backported to 3.1.

(cherry picked from commit 2bcd5b4cba4e9f123860560de0807312b909b306)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

9 months agoBUG/MINOR: quic: fix BBB max bandwidth oscillation issue.
Frederic Lecaille [Thu, 12 Dec 2024 10:08:26 +0000 (11:08 +0100)]
BUG/MINOR: quic: fix BBB max bandwidth oscillation issue.

Remove the code in relation with BBR.ack_phase as per this commit:
https://github.com/ietf-wg-ccwg/draft-ietf-ccwg-bbr/pull/5/commits/ee98c12ad6f0e93153656218a7df1b1ef92618d7

I do now kwow at this time why such a request was pushed on GH for the BBR v3 draft
pseudo-code. That said, the use of such an ack phase seemed confusing, adding much
more information about a BBR flow state than needed. Indeed, the ack phase
state is modified several times in the BBR draft pseudo-code but only used to
decide if the max bandwidth filter virtual clock had to be incremented by
BBRAdvanceMaxBwFilter().

In addition to this, when discussing about haproxy BBR implementation with
Neal Cardwell on the BBR development google group about an oscillation issue
of the max bandwidth (BBR.max_bw), I concluded that this was due to the fact
that its filter virutal clock was too often update, due to the ack phase wich
was stalled in BBR_ACK_PHASE_ACKS_PROBE_STOPPING state for too long. This is
where Neal asked me to test the aforementioned commit. This definitively
makes the max bandwidth (BBR.max_bw) oscillation issue disappear.

Another solution would have been to add a new ack phase enum afer
BBR_ACK_PHASE_ACKS_PROBE_STOPPING. BBR_ACK_PHASE_ACKS_PROBE_STOPPED
would have been a good candidate.

Remove the code in relation with BBR.ack_phase.

Must be backported to 3.1.

(cherry picked from commit b47e1e65df1b468bcd84f02fbc1eb4e1fd7e2bc9)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

9 months agoBUG/MINOR: quic: wrong logical statement in in_recovery_period() (BBR)
Frederic Lecaille [Thu, 12 Dec 2024 09:45:26 +0000 (10:45 +0100)]
BUG/MINOR: quic: wrong logical statement in in_recovery_period() (BBR)

A && logical operator was badly replaced by a || in this function which decides
if BBR is in a recovery period.

Must be backported to 3.1.

(cherry picked from commit 1dbf6b8bedadd5d287ab44e1a387a89e2fc6c70b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

9 months agoMINOR: window_filter: rely on the time to update the filter samples (QUIC/BBR)
Frederic Lecaille [Thu, 12 Dec 2024 08:59:35 +0000 (09:59 +0100)]
MINOR: window_filter: rely on the time to update the filter samples (QUIC/BBR)

The windowed filters are used only the BBR implementation for QUIC to filter
the maximum bandwidth samples for its estimation over a virtual time interval
tracked by counting the cyclical progression through ProbeBW cycles. ngtcp2
and quiche use such windowed filters in their BBR implementation. But in a
slightly different way. When updating the 2nd or 3rd filter samples, this
is done based on their values in place of the time they have been sampled.
It seems more logical to rely on the sample timestamps even if this has no
implication because when a sample is updated using another sample because it
has the same value, they have both the same timestamps!

This patch modifies two statements which compare two consecutive filter samples
based on their values (smp[]->v) by statements which compare them based on the
virtual time they have been sampled (smp[]->t). This fully complies which the
code used by the Linux kernel in lib/win_minmax.c.

Alo take the opportunity of this patch to shorten some statements using <smp>
local variable value to update smp[2] sample in place of initializing its two
members with the <smp> member values.

This patch SHOULD be easily backported to 3.1 where BBR was first implemented.

(cherry picked from commit a9a2f98f86a93dbefca636959234df161b572c21)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

10 months ago[RELEASE] Released version 3.1.1 v3.1.1
Willy Tarreau [Wed, 11 Dec 2024 14:03:59 +0000 (15:03 +0100)]
[RELEASE] Released version 3.1.1

Released version 3.1.1 with the following main changes :
    - BUG/MEDIUM: sock: Remove FD_POLL_HUP during connect() if FD_POLL_ERR is not set
    - MINOR: proxy: Add support of 421-Misdirected-Request in retry-on status
    - BUG/MINOR: log: fix lf_text() behavior with empty string
    - BUG/MINOR: improve BBR throughput on very fast links
    - BUG/MEDIUM: event_hdl: fix uninitialized value in async mode when no data is provided
    - BUG/MEDIUM: quic: prevent stream freeze on pacing
    - BUG/MEDIUM: http-ana: Reset request flag about data sent to perform a L7 retry
    - BUG/MINOR: h1-htx: Use default reason if not set when formatting the response
    - BUILD: quic: fix a build error about an non initialized timestamp
    - BUG/MINOR: signal: register default handler for SIGINT in signal_init()
    - BUG/MINOR: startup: close pidfd and free global.pidfile in handle_pidfile()
    - BUG/MINOR: startup: fix pidfile creation
    - BUG/MINOR: quic: fix bbr_inflight() calls with wrong gain value
    - BUG/MEDIUM: init: make sure only daemonized processes change their session
    - BUG/MINOR: init: do not call fork_poller() for non-forked processes
    - BUG/MEDIUM: mux-quic: remove pacing status when everything is sent
    - BUG/MINOR: quic: remove startup alert if conn socket-owner unsupported
    - BUG/MINOR: quic: remove startup alert if GSO unsupported
    - BUG/MEDIUM: mux-h2: make sure not to touch dummy streams when sending WU
    - BUG/MINOR: config: Fix parsing of accept-invalid-http-{request,response}
    - DOC: config: fix confusing init-state examples
    - BUG/MINOR: debug: COUNT_IF() should return true/false
    - BUILD: debug: fix build issues in COUNT_IF() with -Wunused-value
    - MINOR: mux-h2/traces: add a missing trace on negative initial window size
    - CLEANUP: mux-h2/traces: reword certain ambiguous traces
    - MINOR: mux-h2/glitches: add a description to the H2 glitches
    - BUG/MINOR: mux-h2: fix expression when detecting excess of CONTINUATION frames
    - BUG/MINOR: mworker: don't save program PIDs in oldpids
    - BUG/MINOR: mworker: fix -D -W -sf/-st modes
    - BUG/MINOR: startup: fix error path for master, if can't open pidfile
    - BUG/MEDIUM: startup: don't daemonize if started with -c
    - BUG/MEDIUM: startup: report status if daemonized process fails
    - BUG/MEDIUM: mworker: report status, if daemonized master fails
    - BUG/MINOR: namespace: handle a possible strdup() failure
    - BUG/MINOR: ssl_crtlist: handle a possible strdup() failure
    - BUG/MINOR: resolvers: handle a possible strdup() failure
    - BUG/MINOR: stats: decrement srv refcount on stats-file release
    - MINOR: list: define a watcher type
    - BUG/MEDIUM: stats/server: use watcher to track server during stats dump
    - BUG/MINOR: http-fetch: Ignore empty argument string for query()
    - BUG/MINOR: server-state: Fix expiration date of srvrq_check tasks
    - BUG/MINOR: hlua_fcn: restore server pairs iterator pointer consistency

10 months agoBUG/MINOR: hlua_fcn: restore server pairs iterator pointer consistency
Aurelien DARRAGON [Wed, 11 Dec 2024 09:42:11 +0000 (10:42 +0100)]
BUG/MINOR: hlua_fcn: restore server pairs iterator pointer consistency

Since 9c91b30 ("MINOR: server: remove prev_deleted server list"), hlua
server pair iterator may use and return invalid (stale) server pointer
if multiple servers were deleted between two iterations.

Indeed, the server refcount mechanism (using srv_take()) is no longer
sufficient as the prev_deleted mitigation was removed.

To ensure server pointer consistency between two yields, the new watcher
mechanism must be used (as it already the case for stats dumping).

Thus in this patch we slightly change the server iteration logic:
hlua_server_list_iterator_context struct now stores the next valid server
pointer, and a watcher is added to ensure this pointer is never stale.

Then in hlua_listable_servers_pairs_iterator(), this next pointer is used
to create the Lua server object, and the next valid pointer is obtained by
leveraging watcher_next().

No backport needed unless 9c91b30 ("MINOR: server: remove prev_deleted
server list") is. Please note that dynamic servers were not supported in
Lua prior to 2.8, so it doesn't make sense to backport this patch further
than 2.8.

(cherry picked from commit 358166ae6a3e98d36b378c7eeab1673cc8b4a4dd)
Signed-off-by: Willy Tarreau <w@1wt.eu>

10 months agoBUG/MINOR: server-state: Fix expiration date of srvrq_check tasks
Christopher Faulet [Wed, 11 Dec 2024 08:23:28 +0000 (09:23 +0100)]
BUG/MINOR: server-state: Fix expiration date of srvrq_check tasks

"hold.timeout" was used as expiration date for srvrq_check tasks. But it is
not accurrate. The expiration date must be based on the resolution timeouts
instead (resolve and retry).

The purpose of srvrq_check task is to clean up the server resolution status
when outdated info are inherited from the state file. Using "hold.timeout"
is not accurrate here because hold timeouts concern the resolution response
items not the resolution status of servers. It may be set to a huge value or
0. The expiration date of these tasks must be based on the resolution
timeouts instead.

So now the ("timeout resolve" + resolve_retries * "timeout retry") value is
used.

This patch should fix the issue #2816. It must be backported to all stable
versions.

(cherry picked from commit 647a2906626e9c3d9c3349d338a35798325496f2)
Signed-off-by: Willy Tarreau <w@1wt.eu>

10 months agoBUG/MINOR: http-fetch: Ignore empty argument string for query()
Christopher Faulet [Wed, 11 Dec 2024 08:09:26 +0000 (09:09 +0100)]
BUG/MINOR: http-fetch: Ignore empty argument string for query()

query() sample fetch function takes an optional argument string. During
configuration parsing, empty string must be ignored. It is especially
important when the sample is used with empty parenthesis. The argument is
optional and it is a list of options to configure the behavior of the sample
fetch. So it is logical to ignore empty strings.

This patch should fix the issue #2815. It must be backported to 3.1.

(cherry picked from commit e1525e7b8fa15c59c1c6fc4063b5218d134b7afb)
Signed-off-by: Willy Tarreau <w@1wt.eu>

10 months agoBUG/MEDIUM: stats/server: use watcher to track server during stats dump
Amaury Denoyelle [Wed, 23 Oct 2024 09:33:34 +0000 (11:33 +0200)]
BUG/MEDIUM: stats/server: use watcher to track server during stats dump

If a server A is deleted while a stats dump is currently on it, deletion
is delayed thanks to reference counting. Server A is nonetheless removed
from the proxy list. However, this list is a single linked list. If the
next server B is deleted and freed immediately, server A would still
point to it. This problem has been solved by the prev_deleted list in
servers.

This model seems correct, but it is difficult to ensure completely its
validity. In particular, it implies when stats dump is resumed, server A
elements will be accessed despite the server being in a half-deleted
state.

Thus, it has been decided to completely ditch the refcount mechanism for
stats dump. Instead, use the watcher element to register every stats
dump currently tracking a server instance. Each time a server is deleted
on the CLI, each stats dump element which may points to it are updated
to access the next server instance, or NULL if this is the last server.
This ensures that a server which was deleted via CLI but not completely
freed is never accessed on stats dump resumption.

Currently, no race condition related to dynamic servers and stats dump
is known. However, as described above, the previous model is deemed too
fragile, as such this patch is labelled as bug-fix. It should be
backported up to 2.6, after a reasonable period of observation. It
relies on the following patch :
  MINOR: list: define a watcher type

(cherry picked from commit 071ae8ce3d1a318d2227fad2ebf63e78a05815f0)
Signed-off-by: Willy Tarreau <w@1wt.eu>

10 months agoMINOR: list: define a watcher type
Amaury Denoyelle [Wed, 23 Oct 2024 09:31:44 +0000 (11:31 +0200)]
MINOR: list: define a watcher type

Define a new watcher type into list module. This type is similar to bref
and can be used to register an element which is currently tracking a
dynamic target. Contrary to bref, if the target is freed, every watcher
element are updated to point to a next valid entry or NULL.

This type will simplify handling of dynamic servers deletion, in
particular while stats dump are performed.

This patch is not a bug-fix. However, it is mandatory to fix a race
condition in dynamic servers. Thus, it should be backported along the
next commit up to 2.6.

(cherry picked from commit eafa8a32bb19ee567cb0a60a4042a8ef436e1e94)
Signed-off-by: Willy Tarreau <w@1wt.eu>

10 months agoBUG/MINOR: stats: decrement srv refcount on stats-file release
Amaury Denoyelle [Tue, 10 Dec 2024 14:57:17 +0000 (15:57 +0100)]
BUG/MINOR: stats: decrement srv refcount on stats-file release

Servers instance may be removed at runtime. This can occurs during a
stat dump which currently references this server instance. This case is
protected by server refcount to prevent the server immediate release.

CLI output may be interrupted prior to stats dump completion, for
example if client CLI has been disconnected before the full response
transfer. As such, srv_drop() must be called in every stats dump release
callback.

srv_drop() was missing for stats-file dump release callback. This could
cause a race condition which would prevent a server instance to be fully
removed. Fix this by adding srv_drop() invokation into
cli_io_handler_release_dump_stat_file().

This should be backported up to 3.0.

(cherry picked from commit 2199179461957c86f57e39c4d97c37f3e4d10935)
Signed-off-by: Willy Tarreau <w@1wt.eu>

10 months agoBUG/MINOR: resolvers: handle a possible strdup() failure
Ilia Shipitsin [Tue, 3 Dec 2024 18:54:46 +0000 (19:54 +0100)]
BUG/MINOR: resolvers: handle a possible strdup() failure

This defect was found by the coccinelle script "unchecked-strdup.cocci".
It can be backported to all supported branches.

(cherry picked from commit 193c94a53988f77b827e5f8bb5121d3dc3b2b5ed)
Signed-off-by: Willy Tarreau <w@1wt.eu>

10 months agoBUG/MINOR: ssl_crtlist: handle a possible strdup() failure
Ilia Shipitsin [Tue, 3 Dec 2024 16:13:05 +0000 (17:13 +0100)]
BUG/MINOR: ssl_crtlist: handle a possible strdup() failure

This defect was found by the coccinelle script "unchecked-strdup.cocci".
It can be backported to all supported branches.

(cherry picked from commit ce30bc17305e4ac2dec7d641eedcb301a237d863)
Signed-off-by: Willy Tarreau <w@1wt.eu>

10 months agoBUG/MINOR: namespace: handle a possible strdup() failure
Ilia Shipitsin [Tue, 3 Dec 2024 16:10:21 +0000 (17:10 +0100)]
BUG/MINOR: namespace: handle a possible strdup() failure

This defect was found by the coccinelle script "unchecked-strdup.cocci".
It can be backported to all supported branches.

(cherry picked from commit abee5468509a77537015a7e5dc37268d4d564e03)
Signed-off-by: Willy Tarreau <w@1wt.eu>

10 months agoBUG/MEDIUM: mworker: report status, if daemonized master fails
Valentine Krasnobaeva [Mon, 9 Dec 2024 17:56:01 +0000 (18:56 +0100)]
BUG/MEDIUM: mworker: report status, if daemonized master fails

As daemonization fork happens now very early and before the master-worker
fork, if master or worker processes fail during the initialization, some
critical errors can't be reported to stdout. The launching (parent) process in
such cases exits with 0. This makes an impression, that master and his worker
have successfully started at background, which really complicates the
operations.

In the previous commit a pipe was added to make daemonized child communicate
with his parent. Let's add the same logic to master-worker mode. Up to
receiving the READY message from the worker, master will "forward" it via the
pipe to the launching process. Launching process can obtain master's exit
status, if the master fails to start and nothing has been written in the pipe.

This fix should be backported only in 3.1.

(cherry picked from commit 97aaf7671654ceba16727f8375104064538c40e6)
Signed-off-by: Willy Tarreau <w@1wt.eu>

10 months agoBUG/MEDIUM: startup: report status if daemonized process fails
Valentine Krasnobaeva [Mon, 9 Dec 2024 17:51:34 +0000 (18:51 +0100)]
BUG/MEDIUM: startup: report status if daemonized process fails

Due to master-worker rework, daemonization fork happens now before parsing
and applying the configuration. This makes impossible to report correctly all
warnings and alerts to shell's stdout. Daemonzied process fails, while being
already in background, exit code reported by shell via '$?' equals to 0, as
it's the exit code of his parent.

To fix this, let's create a pipe between parent and daemonized child. The
child will send into this pipe a "READY" message, when it finishes his
initialization. The parent will wait on the "read" end of the pipe until
receiving something. If read() fails, parent obtains the status of the
exited child with waitpid(). So, the parent can correctly report the error to
the stdout and he can exit with child's exitcode.

This fix should be backported only in 3.1.

(cherry picked from commit 663d75e7a00a9c50185cca322cbc64de934b6631)
Signed-off-by: Willy Tarreau <w@1wt.eu>

10 months agoBUG/MEDIUM: startup: don't daemonize if started with -c
Valentine Krasnobaeva [Mon, 9 Dec 2024 11:00:35 +0000 (12:00 +0100)]
BUG/MEDIUM: startup: don't daemonize if started with -c

Due to master-worker refactoring, daemonization fork happens now very early,
before parsing and verifying the configuration. For the moment there is no
any specific syntax, which needs for the daemon mode to be really applied in
order to perform the tests.

So, it's better not to do the daemonization fork, if 'daemon' keyword is
presented in the config (or -D option), when we started with -c (MODE_CHECK).
Like this, during the config verification, the process will always stay in
foreground and all warning or errors will be delivered to the stdout.

This fix should be backported only in 3.1.

(cherry picked from commit 5f94e98d8958ff0003bfbb736427070d16270d6e)
Signed-off-by: Willy Tarreau <w@1wt.eu>

10 months agoBUG/MINOR: startup: fix error path for master, if can't open pidfile
Valentine Krasnobaeva [Tue, 3 Dec 2024 20:33:55 +0000 (21:33 +0100)]
BUG/MINOR: startup: fix error path for master, if can't open pidfile

If master process can't open a pidfile, there is no sense to send SIGTTIN to
oldpids, as it will exit. So, old workers will terminate as well. It's better
to send the last alert to the log about unrecoverable error, because master is
already in its polling loop.

For the standalone mode we should keep the previous logic in this case: send
SIGTTIN to old process and unbind listeners for the new one. So, it's better
to put this error path in main(), as it's done when other configuration settings
can't be applied.

This patch should be backported only in 3.1.

(cherry picked from commit cd0b58e23e12f12369bc1a6e65c6a97409ee8cd6)
Signed-off-by: Willy Tarreau <w@1wt.eu>

10 months agoBUG/MINOR: mworker: fix -D -W -sf/-st modes
Valentine Krasnobaeva [Tue, 3 Dec 2024 20:13:47 +0000 (21:13 +0100)]
BUG/MINOR: mworker: fix -D -W -sf/-st modes

When a new master process is launched like below:

./haproxy -W -D -p ha.pid -sf $(cat ha.pid)...

The old master process and its workers do not stop. Since the master-worker
refactoring, the code, which sends USR1/TERM to old pids from -sf, is called
only for the standalone mode. In master-worker mode we should receive the READY
message from the newly forked worker at first, in order to be able to terminate
the previous master.

So, to fix this, let's terminate the previous master in _send_status(), where
we parse the READY message from the newly forked worker. And let's continue to
use oldpids array, as it was in 3.0, in order to stop the workers, launched
before the reload.

This patch should be backported only in 3.1.

(cherry picked from commit ee111d2004ca653387515f340c504d18b71191c3)
Signed-off-by: Willy Tarreau <w@1wt.eu>

10 months agoBUG/MINOR: mworker: don't save program PIDs in oldpids
Valentine Krasnobaeva [Thu, 5 Dec 2024 15:00:12 +0000 (16:00 +0100)]
BUG/MINOR: mworker: don't save program PIDs in oldpids

After reload, previously launched programs are stopped explicitly in
mworker_ext_launch_all(). So, there is no longer need to save their PIDs in
oldpids array before the master reexec().

This also prepares the fix of "-D -W -sf/-st" modes, as we will need to
loop over this array in the master process context, in order to stop the
previous master, when the new one is ready.

This patch should be backported only in 3.1.

(cherry picked from commit 1fead6c0ca8f8ad2f354fb4dfab241ec5281737c)
Signed-off-by: Willy Tarreau <w@1wt.eu>

10 months agoBUG/MINOR: mux-h2: fix expression when detecting excess of CONTINUATION frames
Willy Tarreau [Fri, 6 Dec 2024 17:53:19 +0000 (18:53 +0100)]
BUG/MINOR: mux-h2: fix expression when detecting excess of CONTINUATION frames

Latest commit f0eca8fe7 ("MINOR: mux-h2/glitches: add a description to
the H2 glitches") misplaced the optional glitch description field, with
it appearing at the end of the if () condition and always reporting
an excess of CONTINUATION frames from the first exceeding one.

This needs to be backported along with that commit once it gets backported.

(cherry picked from commit cb21db04c748a77122c118a3d526fdc058ca564f)
Signed-off-by: Willy Tarreau <w@1wt.eu>

10 months agoMINOR: mux-h2/glitches: add a description to the H2 glitches
Willy Tarreau [Fri, 6 Dec 2024 16:55:05 +0000 (17:55 +0100)]
MINOR: mux-h2/glitches: add a description to the H2 glitches

Since we can now list them using "debug counters" and now support a
description, better add the description to all glitches. This patch may
be backported to 3.1, but before this the following patches must also
be picked:

    86823c828 MINOR: mux-h2/traces: add a missing trace on negative initial window size
    7c8e9420a CLEANUP: mux-h2/traces: reword certain ambiguous traces

(cherry picked from commit f0eca8fe73b5ddfbb642d39bdb8c4d67c200575b)
Signed-off-by: Willy Tarreau <w@1wt.eu>

10 months agoCLEANUP: mux-h2/traces: reword certain ambiguous traces
Willy Tarreau [Fri, 6 Dec 2024 17:24:38 +0000 (18:24 +0100)]
CLEANUP: mux-h2/traces: reword certain ambiguous traces

Some h2 traces were not very clear, let's reword them a bit.

(cherry picked from commit 7c8e9420a23584c7f366aacbfeb308d949f5c7b3)
Signed-off-by: Willy Tarreau <w@1wt.eu>

10 months agoMINOR: mux-h2/traces: add a missing trace on negative initial window size
Willy Tarreau [Fri, 6 Dec 2024 16:30:05 +0000 (17:30 +0100)]
MINOR: mux-h2/traces: add a missing trace on negative initial window size

When a negative initial windows size is reported, we're going to close
the connection, so it's important to report a trace to explain why!
This should be backported at least to 3.1 and possibly 3.0 (adapting the
context since there's no glitches there).

(cherry picked from commit 86823c828f983bd986b150542c7a6482d60b291d)
Signed-off-by: Willy Tarreau <w@1wt.eu>

10 months agoBUILD: debug: fix build issues in COUNT_IF() with -Wunused-value
Willy Tarreau [Mon, 9 Dec 2024 16:49:08 +0000 (17:49 +0100)]
BUILD: debug: fix build issues in COUNT_IF() with -Wunused-value

Commit 7f64bb79fd ("BUG/MINOR: debug: COUNT_IF() should return true/false")
allowed the COUNT_IF() macro to return the evaluated value. This is handy
to place it in "if ()" conditions and count them at the same time. When
glitches are disabled, the condition is just returned as-is, but most call
places do not use the result, making some compilers complain. In addition,
while reviewing this, it was noticed that when DEBUG_STRICT=0, the macro
would still be replaced by a "do { } while (0)" statement, which not only
does not evaluate the expression, but also cannot return anything. Ditto
for COUNT_IF_HOT().

Let's make sure both are always properly evaluated now.

(cherry picked from commit d6dc8120c0b13f896bf2628e611517701aff7a38)
Signed-off-by: Willy Tarreau <w@1wt.eu>

10 months agoBUG/MINOR: debug: COUNT_IF() should return true/false
Willy Tarreau [Wed, 27 Nov 2024 13:24:16 +0000 (14:24 +0100)]
BUG/MINOR: debug: COUNT_IF() should return true/false

The COUNT_IF() macro was initially meant to return true/false to be used
in if() conditions but had an extra do { } while(0) that prevents it from
doing so. Let's get rid of the do { } while(0) before the code generalizes
to too many places. There's no impact on existing code, but may have to be
backported if future fixes rely on it.

(cherry picked from commit 7f64bb79fd35f22537ce5d05f59838e49db5cb10)
Signed-off-by: Willy Tarreau <w@1wt.eu>

10 months agoDOC: config: fix confusing init-state examples
Aurelien DARRAGON [Fri, 6 Dec 2024 11:04:11 +0000 (12:04 +0100)]
DOC: config: fix confusing init-state examples

in 50322dff ("MEDIUM: server: add init-state"), some examples on how to
use init-state server keyword were added alongside with the keyword
documentation.

However, as reported by Nick Ramirez, there was an error because the
example that stated that haproxy will pass the traffic to the server after
3 successful health checks used the "init-state down" instead of the
"init-state fully-down". Thus the behavior wouldn't match what the
comment said (only 1 successful health check was required).

Here we fix the example in itself to match with the comment. Also the
following example ("# or") was also affected, but it is kind of
redundant as the main purpose of the examples are to illustrate the
feature in itself and not how to use server-template directive, so we
remove it.

This should be backported in 3.1 with 50322dff

(cherry picked from commit 7934eef25d1207392a1b4a915d21889babfb1fb0)
Signed-off-by: Willy Tarreau <w@1wt.eu>

10 months agoBUG/MINOR: config: Fix parsing of accept-invalid-http-{request,response}
Christopher Faulet [Wed, 4 Dec 2024 09:47:35 +0000 (10:47 +0100)]
BUG/MINOR: config: Fix parsing of accept-invalid-http-{request,response}

These options are now deprectated, but the proxy capabilities are not
properly checked during the configuration parsing leading to always ignore
these options. This is now fixed by checking the frontend capability for
"accept-invalid-http-request" option and the backend capability for
"accept-invalid-http-response" option.

In addition, the messages about the deprecation of these options are now
emitted with ha_warning() instead of ha_alert() because they are only
warnings and not errors.

This patch should fix the issue #2806. It must be backported to 3.1.

(cherry picked from commit bc453c51066beb8ed05baa6636fcb471b9d103ca)
Signed-off-by: Willy Tarreau <w@1wt.eu>

10 months agoBUG/MEDIUM: mux-h2: make sure not to touch dummy streams when sending WU
Willy Tarreau [Thu, 5 Dec 2024 14:18:38 +0000 (15:18 +0100)]
BUG/MEDIUM: mux-h2: make sure not to touch dummy streams when sending WU

Since commit 1cc851d9f2 ("MEDIUM: mux-h2: start to update stream when
sending WU") we started storing stream offsets in the h2s struct. These
offsets are updated at a few points, where it's safe to write to the
stream, and in h2c_send_strm_wu(), where the h2s->h2c was not performed.

Due to this, nothing protects the h2s from being updated when sending a
WU for a closed stream, which might only happen when acknowledging a
frame after resetting that stream, which is quite unlikely. In any case
if this happens, it will crash as in issue #2793 since the closed streams
are purposely read-only to catch such bugs.

The fix is trivial, just check h2s->h2c before deciding to update the
stream.

Thanks to @Wahnes for reporting this, and Christopher for spotting the
cause. This needs to be backported to 3.1 only.

(cherry picked from commit d649278fce9729415b31e873a3b91d6d0b259d16)
Signed-off-by: Willy Tarreau <w@1wt.eu>

10 months agoBUG/MINOR: quic: remove startup alert if GSO unsupported
Amaury Denoyelle [Wed, 4 Dec 2024 15:25:53 +0000 (16:25 +0100)]
BUG/MINOR: quic: remove startup alert if GSO unsupported

This patch is similar to the previous one, but for GSO support. Remove
alert level message to a diag report only visible with argument -dD.

This must be backported up to 3.1.

(cherry picked from commit 3c239b2f80d80a744711e18640bbcc90bae4eea5)
Signed-off-by: Willy Tarreau <w@1wt.eu>

10 months agoBUG/MINOR: quic: remove startup alert if conn socket-owner unsupported
Amaury Denoyelle [Wed, 4 Dec 2024 15:25:03 +0000 (16:25 +0100)]
BUG/MINOR: quic: remove startup alert if conn socket-owner unsupported

QUIC relies on several advanced network API features from the kernel to
perform optimally. Checks are performed during startup to ensure that
these features are supported. A fallback is automatically performed for
every incompatible feature.

Besides the automatic fallback mechanism, a message is also reported to
the user at the same time. Previously, alert level was used, but it is
incorrect as it is reserved for unrecoverable errors which should
prevent haproxy to start. Warning level could be used, but this can
annoy users running with zero-warning mode.

This patch removes the alert message when 'socket-owner connection' mode
cannot be activated. Convert the message to a diag level. This allows
users to start without forcing configuration modification to hide a
warning. Besides, several feature fallback such as the polling mechanism
does not emit any warning either, so it's better to adopt a similar
behavior for QUIC features.

This must be backported up to 2.8.

(cherry picked from commit 6fed219fd786f3fdca155f686cf2fa2f9e572697)
Signed-off-by: Willy Tarreau <w@1wt.eu>

10 months agoBUG/MEDIUM: mux-quic: remove pacing status when everything is sent
Amaury Denoyelle [Thu, 28 Nov 2024 10:27:15 +0000 (11:27 +0100)]
BUG/MEDIUM: mux-quic: remove pacing status when everything is sent

TASK_F_USR1 is used by MUX tasklet when emission has been interrupted
due to pacing. When the tasklet runs again, only qcc_purge_sending()
will be called as an optimization.

Pacing status is only removed via qcc_wakeup(). Until then, TASK_F_USR1
is not cleared. This causes an issue after emission with pacing
completion if the MUX tasklet is woken up for a recv subscribe, as
qcc_wakeup() is not used by quic-conn layer. The tasklet will
incorrectly run only for pacing emission, without handling reception
process. Worst, a crash will occur if QCC tx frames list is empty, due
to a BUG_ON() in qcc_purge_sending().

Recv subscribe is only used for 0-RTT, when QUIC MUX is instantiated
before quic-conn handshake completion. Thus, this bug can only be
reproduced with 0-rtt. Furthermore, MUX must already have emitted at
least a few response bytes with pacing, before QUIC handshake
completion. It cannot easily be reproduced, at least with CLI clients
where the handshake is always already completed before MUX exchanges.

To fix this, remove TASK_F_USR1 when pacing emission has been completed.
At least, this prevents BUG_ON() on qcc_purge_sending() as it won't be
called with an empty QCC Tx frame list anymore. However, this bug has
revealed that MUX tasklet architecture is not suitable when both
handling reception and emission part. This will be improved in a future
serie of patches.

This should fix github issue #2796.

This must be backported up to 3.1.

(cherry picked from commit 08f557f0c4e37ad69b6d55a247cbc9f89ddee995)
Signed-off-by: Willy Tarreau <w@1wt.eu>

10 months agoBUG/MINOR: init: do not call fork_poller() for non-forked processes
Willy Tarreau [Wed, 4 Dec 2024 14:58:49 +0000 (15:58 +0100)]
BUG/MINOR: init: do not call fork_poller() for non-forked processes

In 3.1-dev10, commit 8dd4efe42f ("MAJOR: mworker: move master-worker
fork in init()") made the fork_poller() code unconditional, while it
is only desirable for processes that have been forked from a parent
(standalone daemon mode) or from a master (master-worker mode). The
call can be expensive in some cases as it will create a new poller,
scan and try to migrate to it all existing FDs till the highest known
one. With very high numbers of FDs, this can take several seconds to
start.

This should be backported to 3.1.

(cherry picked from commit 8b16b725411d72d3bab184bcd0c0253dddeedeb1)
Signed-off-by: Willy Tarreau <w@1wt.eu>

10 months agoBUG/MEDIUM: init: make sure only daemonized processes change their session
Willy Tarreau [Wed, 4 Dec 2024 07:26:24 +0000 (08:26 +0100)]
BUG/MEDIUM: init: make sure only daemonized processes change their session

Commit 8dd4efe42f ("MAJOR: mworker: move master-worker fork in init()")
introduced some sensitive changes to the startup code (which was
expected), and one sensitive change is that the second call to setsid()
was accidentally made unconditional. As such it even applies to foreground
processes, resulting in foreground processes being detached from the
terminal and no longer responding to Ctrl-C nor Ctrl-Z. An example of
this simply consists in start haproxy -db under sudo. Then a new shell
is required to stop it.

This patch removes this second setsid(), as it is already done in
apply_daemon_mode().

This must be backported to 3.1.

(cherry picked from commit 70e4938aec671c5bd1d69fee4ca1c8eea9f72bd3)
Signed-off-by: Willy Tarreau <w@1wt.eu>

10 months agoBUG/MINOR: quic: fix bbr_inflight() calls with wrong gain value
Frederic Lecaille [Wed, 4 Dec 2024 17:47:15 +0000 (18:47 +0100)]
BUG/MINOR: quic: fix bbr_inflight() calls with wrong gain value

This patch fixes two wrong calls to bbr_inflight().

bbr_target_inflight() aim is to compute the number of bytes BBR has to put on
the network as bytes in flight (sent but not acked bytes). It must call
bbr_inflight() with the current window gain value (in place of a wrong fixed 100
gain value here, in percents).

bbr_is_time_to_cruise() also called bbr_inflight() with a wrong gain value
as parameter due to a confusion between the value mentioned by the RFC (1
meaning 100% of the current window) and our implementation which needs value in
percents (so 100 in place of 1 here). Note that bbr_is_time_to_cruise() aim is to
make BBR the decision to leave the probing_bw down state. The bug had as side
effect to make BBR stay in this state during too long periods of time during
which the bottleneck bandwidth is decreasing, leading to big oscillations
between the mininum and maximum bottleneck bandwidth estimations.

This patch must be backported to 3.1 where BBR was first implemented.

(cherry picked from commit 6404b7a18a518e04bf934c1635e8a0b34628149a)
Signed-off-by: Willy Tarreau <w@1wt.eu>

10 months agoBUG/MINOR: startup: fix pidfile creation
Valentine Krasnobaeva [Mon, 2 Dec 2024 15:05:16 +0000 (16:05 +0100)]
BUG/MINOR: startup: fix pidfile creation

Pidfile should be created at the latest initialization stage, when we are
sure, that process is able to start successfully, otherwise PID value, written
in this file is no longer valid.

So, for the standalone mode, let's move the block, which opens the pidfile and
let's put it just before applying "chroot". In master-worker mode, master
doesn't perform chroot. So it creates the pidfile, only when the "READY"
message from the newly forked worker is received.

This should be backported only in 3.1

(cherry picked from commit 295071007b1bf83eb0d66ad88b05d6aef05c69eb)
Signed-off-by: Willy Tarreau <w@1wt.eu>

10 months agoBUG/MINOR: startup: close pidfd and free global.pidfile in handle_pidfile()
Valentine Krasnobaeva [Mon, 2 Dec 2024 15:04:56 +0000 (16:04 +0100)]
BUG/MINOR: startup: close pidfd and free global.pidfile in handle_pidfile()

After master-worker mode refactoring, global.pidfile is only used in
handle_pidfile(), which opens the provided file and writes the PID into it. So,
it's more appropriate to perform the close(pidfd) and ha_free(&global.pidfile)
also in this function.

This commit prepares the fix of the pidfile creation, as it's created now very
early, when we are not sure, that process has successfully started. In
master-worker mode handle_pidfile() can be called in the master process context.
So, let's make it accessible from other compilation units via global.h.

This should be backported only in 3.1.

(cherry picked from commit a33977da48978299d2d71403593846b1b21ec768)
Signed-off-by: Willy Tarreau <w@1wt.eu>

10 months agoBUG/MINOR: signal: register default handler for SIGINT in signal_init()
Valentine Krasnobaeva [Mon, 2 Dec 2024 13:47:17 +0000 (14:47 +0100)]
BUG/MINOR: signal: register default handler for SIGINT in signal_init()

When haproxy is launched in a background and in a subshell (see example below),
according to POSIX standard (2.11. Signals and Error Handling), it inherits
from the subshell SIG_IGN signal handler for SIGINT and SIGQUIT.

$ (./haproxy -f env4.cfg &)

So, when haproxy is lanched like this, it doesn't stop upon receiving
the SIGINT. This can be a root cause of some unexpected timeouts, when haproxy
is started under VTest, as VTest sends to the process SIGINT in order to
terminate it. To fix this, let's explicitly register the default signal
handler for the SIGINT in signal_init() initcall.

This should be backported in all stable versions.

(cherry picked from commit d3c20b02469dea6f46369bb91965d8b4924bb2b7)
Signed-off-by: Willy Tarreau <w@1wt.eu>

10 months agoBUILD: quic: fix a build error about an non initialized timestamp
Frederic Lecaille [Fri, 29 Nov 2024 13:39:48 +0000 (14:39 +0100)]
BUILD: quic: fix a build error about an non initialized timestamp

This is to please a non identified compilers which complains about an hypothetic
<time_ns> variable which would be not initialized even if this is the case only
when it is not used.

This build issue arrived with this commit:
BUG/MINOR: improve BBR throughput on very fast links

Should be backported to 3.1 with this previous commit.

(cherry picked from commit 7868dc9c45aeb29774baac98711f563755b610d6)
Signed-off-by: Willy Tarreau <w@1wt.eu>

10 months agoBUG/MINOR: h1-htx: Use default reason if not set when formatting the response
Christopher Faulet [Fri, 29 Nov 2024 13:31:21 +0000 (14:31 +0100)]
BUG/MINOR: h1-htx: Use default reason if not set when formatting the response

When the response status line is formatted before sending it to the client,
if there is no reason set, HAProxy should add one that matches the status
code, as stated in the configuration manual. However it is not performed.

It is possible to hit this bug when the response comes from a H2 server,
because there is no reason field in HTTP/2 and above.

This patch should fix the issue #2798. It should be backported to all stable
versions.

(cherry picked from commit 37487ada739fc86e3acb46c9949196f4f15cc9b1)
Signed-off-by: Willy Tarreau <w@1wt.eu>

10 months agoBUG/MEDIUM: http-ana: Reset request flag about data sent to perform a L7 retry
Christopher Faulet [Thu, 28 Nov 2024 09:01:41 +0000 (10:01 +0100)]
BUG/MEDIUM: http-ana: Reset request flag about data sent to perform a L7 retry

It is possible to loose the request after several L7 retries, leading to
crashes, because the request channel flag stating some data were sent is not
properly reset.

When a L7 retry is performed, some flags on different entities must be reset
to be sure a new connection will be properly retried, just like it was the
first one, mainly because there was no connection establishment failure. One
of them, on the request channel, is not reset. The flag stating some data
were already sent. It is annoying because this flag is used during the
connection establishment to know if an error is triggered at the connection
level or at the data level. In the last case, the error must be handled by
the HTTP response analyzer, to eventually perform another L7 retry.

Because CF_WROTE_DATA flag is not removed when a L7 retry is performed, a
subsequent connection establishment error may be handled as a L7 error while
in fact the request was never sent. It also means the request was never
saved in the buffer used to performed L7 retries. Thus, on the next L7
retires, the request is just lost. This forecefully leads to a bunch of
undefined behavior. One of them is a crash, when the request is used to
perform the load-balancing.

This patch should fix issue #2793. It must be backported to all stable
versions.

(cherry picked from commit 62f37801c881f68060cedb7a74b5b8cb5fcfec81)
Signed-off-by: Willy Tarreau <w@1wt.eu>

10 months agoBUG/MEDIUM: quic: prevent stream freeze on pacing
Amaury Denoyelle [Fri, 29 Nov 2024 13:28:09 +0000 (14:28 +0100)]
BUG/MEDIUM: quic: prevent stream freeze on pacing

On snd_buf completion, QUIC MUX tasklet is scheduled if newly data has
been transferred from the stream layer. Thanks to qcc_wakeup(), pacing
status is removed from tasklet, which ensure next emission will reset Tx
frames and use the new data.

Tasklet is not scheduled if MUX is already subscribed on send due to a
previous blocking condition. This is an optimization to prevent an
unneeded IO handler execution. However, this causes a bug if an emission
is currently delayed due to pacing. As pacing status is not removed on
snd_buf, next emission process will continue emission with older data
without refreshing the newly transferred one.

This causes a transfer freeze. Unless there is some activity on the
connection, the transfer will be eventually aborted due to idle timeout.

To fix this, remove TASK_F_USR1 if tasklet wakeup is not called due to
send subscription. Note that this code is also duplicated in done_ff for
zero-copy transfer.

This must be backported up to 3.1.

(cherry picked from commit 9d4c26ebaa5c54714eae27a39422e0d47970c628)
Signed-off-by: Willy Tarreau <w@1wt.eu>

10 months agoBUG/MEDIUM: event_hdl: fix uninitialized value in async mode when no data is provided
Aurelien DARRAGON [Fri, 29 Nov 2024 07:42:01 +0000 (08:42 +0100)]
BUG/MEDIUM: event_hdl: fix uninitialized value in async mode when no data is provided

In _event_hdl_publish(), when we prepare the asynchronous event and no
<data> was provided (set to NULL), we forgot to initialize the _data
event_hdl_async_event struct member to NULL, which leads to uninitialized
reads in event_hdl_async_free_event() when the event is freed:

==1002331== Conditional jump or move depends on uninitialised value(s)
==1002331==    at 0x35D9D1: event_hdl_async_free_event (event_hdl.c:224)
==1002331==    by 0x1CC8EC: hlua_event_runner (hlua.c:9917)
==1002331==    by 0x39AD3F: run_tasks_from_lists (task.c:641)
==1002331==    by 0x39B7B4: process_runnable_tasks (task.c:883)
==1002331==    by 0x314B48: run_poll_loop (haproxy.c:2976)
==1002331==    by 0x315218: run_thread_poll_loop (haproxy.c:3190)
==1002331==    by 0x18061D: main (haproxy.c:3747)

The bug severity was set to MEDIUM because of its nature, and it's best
if this patch can be backported up to 2.8. But in practise it can only be
triggered with events that don't provide optional data: since PAT_REF
events are the first native events making use of this feature, this bug
shouldn't be an issue before f72a66e ("MINOR: pattern: publish event_hdl
events on pat_ref updates")

(cherry picked from commit dd56616067d19060425940f6906cefe6efcd1955)
Signed-off-by: Willy Tarreau <w@1wt.eu>

10 months agoBUG/MINOR: improve BBR throughput on very fast links
Frederic Lecaille [Wed, 27 Nov 2024 18:39:34 +0000 (19:39 +0100)]
BUG/MINOR: improve BBR throughput on very fast links

This patch fixes the loss of information when computing the delivery rate
(quic_cc_drs.c) on links with very low latency due to usage of 32bits
variables with the millisecond as precision.

Initialize the quic_conn task with TASK_F_WANTS_TIME flag ask it to ask
the scheduler to update the call date of this task. This allows this task to get
a nanosecond resolution on the call date calling task_mono_time(). This is enabled
only for congestion control algorithms with delivery rate estimation support
(BBR only at this time).

Store the send date with nanosecond precision of each TX packet into
->time_sent_ns new quic_tx_packet struct member to store the date a packet was
sent in nanoseconds thanks to task_mono_time().

Make use of this new timestamp by the delivery rate estimation algorithm (quic_cc_drs.c).

Rename current ->time_sent member from quic_tx_packet struct to ->time_sent_ms to
distinguish the unit used by this variable (millisecond) and update the code which
uses this variable. The logic found in quic_loss.c is not modified at all.

Must be backported to 3.1.

(cherry picked from commit f8b697c19b3d8dfe9e12e0c03b0069357c013f20)
Signed-off-by: Willy Tarreau <w@1wt.eu>

10 months agoBUG/MINOR: log: fix lf_text() behavior with empty string
Aurelien DARRAGON [Thu, 28 Nov 2024 11:03:17 +0000 (12:03 +0100)]
BUG/MINOR: log: fix lf_text() behavior with empty string

As reported by Baptiste in GH #2797, if a logformat alias leveraging
lf_text() ends up printing nothing (empty string), the whole logformat
evaluation stops, leading garbage log message.

This bug was introduced during 3.0 cycle in fcb7e4b ("MINOR: log: add
lf_rawtext{_len}() functions"). At that time I genuinely thought that
if strlcpy2() returned 0, it was due to a lack of space, actually
forgetting that the function may simply be called with an empty string.

Because of that, lf_text() would return NULL if called with an empty
string, and since all lf_*() helpers are expected to return NULL on
error, this explains why the logformat evaluation immediately stops in
this case.

To fix the issue, let's simply consider that strlcpy2() returning 0 is
not an error, like it was already the case before.

It should be backported in 3.1 and 3.0 with fcb7e4b.

(cherry picked from commit 3e470471b7e0ec113807f6981699fda9538e7ffc)
Signed-off-by: Willy Tarreau <w@1wt.eu>

10 months agoMINOR: proxy: Add support of 421-Misdirected-Request in retry-on status
Christopher Faulet [Thu, 28 Nov 2024 10:45:51 +0000 (11:45 +0100)]
MINOR: proxy: Add support of 421-Misdirected-Request in retry-on status

The "421" status can now be specified on retry-on directives. PR_RE_* flags
were updated to remains sorted.

This patch should fix the issue #2794. It is quite simple so it may safely
be backported to 3.1 if necessary.

(cherry picked from commit bc66d3198597c5731c25cab303d306850e79a346)
Signed-off-by: Willy Tarreau <w@1wt.eu>

10 months agoBUG/MEDIUM: sock: Remove FD_POLL_HUP during connect() if FD_POLL_ERR is not set
Christopher Faulet [Wed, 27 Nov 2024 09:04:45 +0000 (10:04 +0100)]
BUG/MEDIUM: sock: Remove FD_POLL_HUP during connect() if FD_POLL_ERR is not set

epoll_wait() may return EPOLLUP and/or EPOLLRDHUP after an asynchronous
connect(), to indicate that the peer accepted the connection then
immediately closed before epoll_wait() returned. When this happens,
sock_conn_check() is called to check whether or not the connection correctly
established, and after that the receive channel of the socket is assumed to
already be closed. This lets haproxy send the request at best (if RDHUP and
not HUP) then immediately close.

Over the last two years, there were a few reports about this spuriously
happening on connections where network captures proved that the server had
not closed at all (and sometimes even received the request and responded to
it after haproxy had closed). The logs show that a successful connection is
immediately reported on error after the request was sent. After
investigations, it appeared that a EPOLLUP, or eventually a EPOLLRDHUP, can
be reported by epool_wait() during the connect() but in sock_conn_check(),
the connect() reports a success. So the connection is validated but the HUP
is handled on the first receive and an error is reported.

The same behavior could be observed on health-checks, leading HAProxy to
consider the server as DOWN while it is not.

The only explanation at this point is that it is a kernel bug, notably
because it does not even match the documentation for connect() nor epoll. In
addition for now it was only observed with Ubuntu kernels 5.4 and 5.15 and
was never reproduced on any other one.

We have no reproducer but here is the typical strace observed:

socket(AF_INET, SOCK_STREAM, IPPROTO_IP) = 114
fcntl(114, F_SETFL, O_RDONLY|O_NONBLOCK) = 0
setsockopt(114, SOL_TCP, TCP_NODELAY, [1], 4) = 0
connect(114, {sa_family=AF_INET, sin_port=htons(11000), sin_addr=inet_addr("A.B.C.D")}, 16) = -1 EINPROGRESS (Operation now in progress)
epoll_ctl(19, EPOLL_CTL_ADD, 114, {events=EPOLLIN|EPOLLOUT|EPOLLRDHUP, data={u32=114, u64=114}}) = 0
epoll_wait(19, [{events=EPOLLIN, data={u32=15, u64=15}}, {events=EPOLLIN, data={u32=151, u64=151}}, {events=EPOLLIN, data={u32=59, u64=59}}, {events=EPOLLIN|EPOLLRDHUP, data={u32=114, u64=114}}], 200, 0) = 4
epoll_ctl(19, EPOLL_CTL_MOD, 114, {events=EPOLLOUT, data={u32=114, u64=114}}) = 0
epoll_wait(19, [{events=EPOLLOUT, data={u32=114, u64=114}}, {events=EPOLLIN, data={u32=15, u64=15}}, {events=EPOLLIN, data={u32=10, u64=10}}, {events=EPOLLIN, data={u32=165, u64=165}}], 200, 0) = 4
connect(114, {sa_family=AF_INET, sin_port=htons(11000), sin_addr=inet_addr("A.B.C.D")}, 16) = 0
sendto(114, "POST "..., 1009, MSG_DONTWAIT|MSG_NOSIGNAL, NULL, 0) = 1009
close(114)                              = 0

Some ressources about this issue:
  - https://www.spinics.net/lists/netdev/msg876470.html
  - https://github.com/haproxy/haproxy/issues/1863
  - https://github.com/haproxy/haproxy/issues/2368

So, to workaround the issue, we have decided to remove FD_POLL_HUP flag on
the FD during the connection establishement if FD_POLL_ERR is not reported
too in sock_conn_check(). This way, the call to connect() is able to
validate or reject the connection. At the end, if the HUP or RDHUP flags
were valid, either connect() would report the error itself, or the next
recv() would return 0 confirming the closure that the poller tried to
report. EPOLL_RDHUP is only an optimization to save a syscall anyway, and
this pattern is so rare that nobody will ever notice the extra call to
recv().

Please note that at least one reporter confirmed that using poll() instead
of epoll() also addressed the problem, so that can also be a temporary
workaround for those discovering the problem without the ability to
immediately upgrade.

The event is accounted via a COUNT_IF(), to be able to spot it in future
issue. Just in case.

This patch should fix the issue #1863 and #2368. It may be related
to #2751. It should be backported as far as 2.4. In 3.0 and below, the
COUNT_IF() must be removed.

(cherry picked from commit 7262433183f590377ace31ff96b1fafa4525b7c2)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

10 months ago[RELEASE] Released version 3.1.0 v3.1.0
Willy Tarreau [Tue, 26 Nov 2024 14:24:10 +0000 (15:24 +0100)]
[RELEASE] Released version 3.1.0

Released version 3.1.0 with the following main changes :
    - BUG/MAJOR: mux-h1: Properly handle wrapping on obuf when dumping the first-line
    - BUILD: activity/memprofile: fix a build warning in the posix_memalign handler
    - BUG/MINOR: quic: Avoid BUG_ON() on ->on_pkt_lost() BBR callback call
    - CI: update to the latest AWS-LC version
    - CI: update to the latest WolfSSL version
    - DOC: ot: mention planned deprecation of the OT filter
    - Revert "CI: update to the latest WolfSSL version"
    - CI: github: add a WolfSSL job which tries the latest version
    - BUILD: systemd: fix usage of reserved name "sun" in the address field
    - BUILD: init: use the more portable FD_CLOEXEC for /dev/null
    - CI: github: improve the Wolfssl job
    - CI: github: improve the AWS-LC job
    - BUG/MINOR: mux-quic: fix show quic report of QCS prepared bytes
    - BUG/MEDIUM: quic: fix sending performance due to qc_prep_pkts() return
    - MINOR: mux-quic: use sched call time for pacing
    - CI: github: allow to run the Illumos job manually
    - BUILD: tcp_sample: var_fc_counter defined but not used
    - CI: github: add 'workflow_dispatch' on remaining build jobs
    - DOC: config: refine a little bit the text on QUIC pacing
    - MINOR: proto_sockpair: send_fd_uxst: init iobuf, cmsghdr, cmsgbuf to zeros
    - MINOR: startup: rename on_new_child_failure to mworker_on_new_child_failure
    - REORG: startup: move on_new_child_failure in mworker.c
    - MINOR: startup: prefix prepare_master and run_master with mworker_*
    - REORG: startup: move mworker_prepare_master in mworker.c
    - MINOR: startup: keep updating verbosity modes only in haproxy.c
    - REORG: startup: move mworker_run_master and mworker_loop in mworker.c
    - REORG: startup: move mworker_reexec and mworker_reload in mworker.c
    - MINOR: startup: prefix apply_master_worker_mode with mworker_*
    - REORG: startup: move mworker_apply_master_worker_mode in mworker.c
    - MINOR: cfgparse-quic: strengthen quic-cc-algo parsing
    - BUG/MAJOR: quic: fix wrong packet building due to already acked frames
    - DEV: lags/show-sess-to-flags: Properly handle fd state on server side
    - BUG/MEDIUM: http-ana: Don't release too early the L7 buffer
    - MINOR: quic: make bbr consider the max window size setting
    - DOC: quic: Amend the pacing information about BBR.
    - BUG/MEDIUM: quic: prevent EMSGSIZE with GSO for larger bufsize
    - MINOR: cli: Add a "help" keyword to show sess
    - MINOR: cli/quic: Add a "help" keyword to show quic
    - DOC: management: mention "show sess help" and "show quic help"
    - DOC: install: update the list of supported versions
    - MINOR: version: mention that 3.1 is stable now

10 months agoMINOR: version: mention that 3.1 is stable now
Christopher Faulet [Tue, 26 Nov 2024 14:11:36 +0000 (15:11 +0100)]
MINOR: version: mention that 3.1 is stable now

This version will be maintained up to around Q1 2026. The INSTALL file
also mentions it.

10 months agoDOC: install: update the list of supported versions
Willy Tarreau [Tue, 26 Nov 2024 14:18:48 +0000 (15:18 +0100)]
DOC: install: update the list of supported versions

OpenSSL up to 3.4 was tested, and gcc up to 14 was tested, so let's
reflect this in the install doc.

10 months agoDOC: management: mention "show sess help" and "show quic help"
Willy Tarreau [Tue, 26 Nov 2024 14:00:51 +0000 (15:00 +0100)]
DOC: management: mention "show sess help" and "show quic help"

These ones were recently added but we forgot to update the doc.

10 months agoMINOR: cli/quic: Add a "help" keyword to show quic
Olivier Houchard [Mon, 25 Nov 2024 17:35:51 +0000 (18:35 +0100)]
MINOR: cli/quic: Add a "help" keyword to show quic

Add a help keyword to show quic, that will provide a longer explanation
of all the available options than what is provided by the command "help".

10 months agoMINOR: cli: Add a "help" keyword to show sess
Olivier Houchard [Mon, 25 Nov 2024 17:34:01 +0000 (18:34 +0100)]
MINOR: cli: Add a "help" keyword to show sess

Add a help keyword to show sess, that will provide a longer explanation of
all the available options than what is provided by the command "help".

10 months agoBUG/MEDIUM: quic: prevent EMSGSIZE with GSO for larger bufsize
Amaury Denoyelle [Tue, 26 Nov 2024 10:03:30 +0000 (11:03 +0100)]
BUG/MEDIUM: quic: prevent EMSGSIZE with GSO for larger bufsize

A UDP datagram cannot be greater than 65535 bytes, as UDP length header
field is encoded on 2 bytes. As such, sendmsg() will reject a bigger
input with error EMSGSIZE. By default, this does not cause any issue as
QUIC datagrams are limited to 1.252 bytes and sent individually.

However, with GSO support, value bigger than 1.252 bytes are specified
on sendmsg(). If using a bufsize equal to or greater than 65535, syscall
could reject the input buffer with EMSGSIZE. As this value is not
expected, the connection is immediately closed by haproxy and the
transfer is interrupted.

This bug can easily reproduced by requesting a large object on loopback
interface and using a bufsize of 65535 bytes. In fact, the limit is
slightly less than 65535, as extra room is also needed for IP + UDP
headers.

Fix this by reducing the count of datagrams encoded in a single GSO
invokation via qc_prep_pkts(). Previously, it was set to 64 as specified
by man 7 udp. However, with 1252 datagrams, this is still too many.
Reduce it to a value of 52. Input to sendmsg will thus be restricted to
at most 65.104 bytes if last datagram is full.

If there is still data available for encoding in qc_prep_pkts(), they
will be written in a separate batch of datagrams. qc_send_ppkts() will
then loop over the whole QUIC Tx buffer and call sendmsg() for each
series of at most 52 datagrams.

This does not need to be backported.

10 months agoDOC: quic: Amend the pacing information about BBR.
Frederic Lecaille [Tue, 26 Nov 2024 06:46:17 +0000 (07:46 +0100)]
DOC: quic: Amend the pacing information about BBR.

BBR handles itself its own burst size (mentioned as send_quantum in BBR RFC).

10 months agoMINOR: quic: make bbr consider the max window size setting
Frederic Lecaille [Tue, 26 Nov 2024 06:37:58 +0000 (07:37 +0100)]
MINOR: quic: make bbr consider the max window size setting

Limit the BBR congestion control window size as this is done for all the others
congestion control algorithms with tune.quic.frontend.default-max-window-size
or as first argument passed to "bbr" option for "quic-cc-algo".

10 months agoBUG/MEDIUM: http-ana: Don't release too early the L7 buffer
Christopher Faulet [Mon, 25 Nov 2024 21:05:27 +0000 (22:05 +0100)]
BUG/MEDIUM: http-ana: Don't release too early the L7 buffer

In some cases, the buffer used to store the request to be able to perform a
L7 retry is released released too early, leading to a crash because a retry
is performed with an empty request.

First, there is a test on invalid 101 responses that may be caught by the
"junk-response" retry policy. Then, it is possible to get an error
(empty-response, bad status code...) after an interim response. In both
cases, the L7 buffer is already released while it should not.

To fix the issue, the L7 buffer is now released at the end of the
AN_RES_WAIT_HTTP analyser, but only when a response was successfully
received and processed. In all error cases, the stream is quickly released,
with the L7 buffer. So there is no leak and it is safer this way.

This patch may fix the issue #2793. It must be as far as 2.4.

10 months agoDEV: lags/show-sess-to-flags: Properly handle fd state on server side
Christopher Faulet [Mon, 25 Nov 2024 20:57:27 +0000 (21:57 +0100)]
DEV: lags/show-sess-to-flags: Properly handle fd state on server side

It must be handled as an hexadecimal value.

10 months agoBUG/MAJOR: quic: fix wrong packet building due to already acked frames
Frederic Lecaille [Mon, 25 Nov 2024 10:14:20 +0000 (11:14 +0100)]
BUG/MAJOR: quic: fix wrong packet building due to already acked frames

If a packet build was asked to probe the peer with frames which have just
been acked, the frames build run by qc_build_frms() could be cancelled  by
qc_stream_frm_is_acked() whose aim is to check that current frames to
be built have not been already acknowledged. In this case the packet build run
by qc_do_build_pkt() is not interrupted, leading to the build of an empty packet
which should be ack-eliciting.

This is a bug detected by the BUG_ON() statement in qc_do_build_pk():

    BUG_ON(qel->pktns->tx.pto_probe &&
           !(pkt->flags & QUIC_FL_TX_PACKET_ACK_ELICITING));

Thank you to @Tristan971 for having reported this issue in GH #2709

This is an old bug which must be backported as far as 2.6.

10 months agoMINOR: cfgparse-quic: strengthen quic-cc-algo parsing
Amaury Denoyelle [Mon, 25 Nov 2024 14:37:46 +0000 (15:37 +0100)]
MINOR: cfgparse-quic: strengthen quic-cc-algo parsing

quic-cc-algo is a bind keyword which is used to specify the congestion
control algorithm. It is parsed via function bind_parse_quic_cc_algo().

The parsing function was too laxed as it used strncmp for algo token
matching. This could cause surprise if specifying an invalid algorithm
but starting identically to another entry. Especially if extra
parameters are specified in parenthesis, as in this case parameters
value will be completely ignored and default value used instead.

To fix this, convert algo argument to ist. Then, use istsplit() to
extract algo token from the optional extra arguments and compare the
whole value with isteq().

10 months agoREORG: startup: move mworker_apply_master_worker_mode in mworker.c
Valentine Krasnobaeva [Fri, 22 Nov 2024 22:42:17 +0000 (23:42 +0100)]
REORG: startup: move mworker_apply_master_worker_mode in mworker.c

mworker_apply_master_worker_mode() is called only in master-worker mode, so
let's move it mworker.c

10 months agoMINOR: startup: prefix apply_master_worker_mode with mworker_*
Valentine Krasnobaeva [Fri, 22 Nov 2024 22:33:31 +0000 (23:33 +0100)]
MINOR: startup: prefix apply_master_worker_mode with mworker_*

This patch prepares the move of apply_master_worker_mode in mworker.c. So,
let's at first rename it to mworker_apply_master_worker_mode.