]> git.ipfire.org Git - thirdparty/haproxy.git/log
thirdparty/haproxy.git
8 months agoMEDIUM: mworker: depreciate the 'program' section
William Lallemand [Fri, 8 Nov 2024 16:06:58 +0000 (17:06 +0100)] 
MEDIUM: mworker: depreciate the 'program' section

The program section is unreliable and should not be used, more reliable
alternatives exist outside HAProxy. Let's depreciate the section so we
could remove it completely in 3.3.

8 months ago[RELEASE] Released version 3.1-dev12 v3.1-dev12
Willy Tarreau [Fri, 8 Nov 2024 14:46:54 +0000 (15:46 +0100)] 
[RELEASE] Released version 3.1-dev12

Released version 3.1-dev12 with the following main changes :
    - MINOR: startup: tune.renice.{startup,runtime} allow to change priorities
    - BUG/MEDIUM: promex: Fix dump of extra counters
    - BUILD: import/mt_list: support building with TCC
    - BUILD: compiler: define __builtin_prefetch() for tcc
    - CLEANUP: quic: Remove the useless directive "tune.quic.backend.max-idle-timeou"
    - DOC: config: document connection error 44 (reverse connect failure)
    - CLEANUP: connection: properly name the CO_ER_SSL_FATAL enum entry
    - DEBUG: cli: support closing "hard" using close() in addition to fd_delete()
    - MINOR: connection: add more connection error codes to cover common errno
    - MINOR: rawsock: set connection error codes when returning from recv/send/splice
    - MINOR: connection: add new sample fetch functions fc_err_name and bc_err_name
    - MINOR: quic: Help diagnosing malformed probing packets
    - BUG/MINOR: quic: fix malformed probing packet building
    - MINOR: listener: Remove useless checks on the receiver protocol existence
    - MINOR: http-conv: Remove unreachable goto statement in sample_conv_q_preferred
    - MINOR: http: don't %-encode the payload when not relevant
    - MINOR: quic: simplify qc_parse_pkt_frms() return path
    - MINOR: quic: use dynamically allocated frame on parsing
    - MINOR: quic: extend return value of CRYPTO parsing
    - BUG/MINOR: quic: repeat packet parsing to deal with fragmented CRYPTO
    - BUG/MINOR: mworker: do 'program' postparser checks in read_cfg_in_discovery_mode
    - EXAMPLES: add "traces.cfg" with traces examples
    - BUG/MEDIUM: quic: do not consider ACK on released stream as error
    - CLEANUP: stats: fix misleading comment on top of stat_idx_info
    - MINOR: wdt: move the local timers to a struct
    - MINOR: debug: add a function to dump a stuck thread
    - DEBUG: wdt: better detect apparently locked up threads and warn about them
    - DEBUG: cli: make it possible for "debug dev loop" to trigger warnings
    - DEBUG: wdt: make the blocked traffic warning delay configurable
    - DEBUG: wdt: add a stats counter "BlockedTrafficWarnings" in show info
    - DEBUG: wdt: set the default blocked task delay to 100 ms
    - MINOR: debug: move the "recover now" warn message after the optional notes
    - MINOR: event_hdl: add event_hdl_sub_list_empty() helper func
    - MINOR: pattern: add _pat_ref_new() helper func
    - OPTIM: pattern: use malloc() to initialize new pat_ref struct
    - MINOR: pattern: add pat_ref_free() helper func
    - CLEANUP: guid: remove global tree export
    - BUG/MINOR: guid/server: ensure thread-safety on GUID insert/delete
    - DOC: management: explain the change of behavior of the program section
    - BUG/MEDIUM: mux-h2: try to wait for the peer to read the GOAWAY
    - BUG/MEDIUM: quic: prevent crash due to CRYPTO parsing error

8 months agoBUG/MEDIUM: quic: prevent crash due to CRYPTO parsing error
Amaury Denoyelle [Fri, 8 Nov 2024 11:40:29 +0000 (12:40 +0100)] 
BUG/MEDIUM: quic: prevent crash due to CRYPTO parsing error

A packet which contains several splitted and out of order CRYPTO frames
may be parsed multiple times to ensure it can be handled via ncbuf. Only
3 iterations can be performed to prevent excessive CPU usage.

There is a risk of crash if packet parsing is interrupted after maximum
iterations is reached, or no progress can be made on the ncbuf. This is
because <frm> may be dangling after list_for_each_entry_safe()

The crash occurs on qc_frm_free() invokation, on error path of
qc_parse_pkt_frms(). To fix it, always reset frm to NULL after
list_for_each_entry_safe() to ensure it is not dangling.

This should fix new report on github isue #2776. This regression has
been triggered by the following patch :
  1767196d5b2d8d1e557f7b3911a940000166ecda
  BUG/MINOR: quic: repeat packet parsing to deal with fragmented CRYPTO

As such, it must be backported up to 2.6, after the above patch.

8 months agoBUG/MEDIUM: mux-h2: try to wait for the peer to read the GOAWAY
Willy Tarreau [Fri, 8 Nov 2024 09:02:47 +0000 (10:02 +0100)] 
BUG/MEDIUM: mux-h2: try to wait for the peer to read the GOAWAY

When timeout http-keep-alive is very short (e.g. 10ms), it's possible
sometimes for a client to face truncated responses due to an early
close that happens while the system is still pushing the last data,
colliding with the client's WINDOW_UPDATEs that trigger RSTs.

Here we're trying to do better: first we send a GOAWAY on timeout, then
we wait up to clientfin/client timeout for the peer to react so that we
don't immediately close. This is sufficient to avoid truncation as soon
as the timeout is more than a few hundred ms.

It's not certain it should be backported, because it's a bit sensistive
and might possibly fall into certain edge cases.

8 months agoDOC: management: explain the change of behavior of the program section
William Lallemand [Fri, 8 Nov 2024 10:57:11 +0000 (11:57 +0100)] 
DOC: management: explain the change of behavior of the program section

The program section does not work exactly the same way with the
master-worker rework of HAProxy 3.1. Let's explain it in the program
documentation.

8 months agoBUG/MINOR: guid/server: ensure thread-safety on GUID insert/delete
Amaury Denoyelle [Thu, 7 Nov 2024 10:08:40 +0000 (11:08 +0100)] 
BUG/MINOR: guid/server: ensure thread-safety on GUID insert/delete

Since 3.0, it is possible to assign a GUID to proxies, listeners and
servers. These objects are stored in a global tree guid_tree.

Proxies and listeners are static. However, servers may be added or
deleted at runtime, which imply that guid_tree must be protected. Fix
this by declaring a read-write lock to protect tree access.

For now, only guid_insert() and guid_remove() are protected using a
write lock. Outside of these, GUID tree is not accessed at runtime. If
server CLI commands are extended to support GUID as server identifier,
lookup operation should be extended with a read lock protection.

Note that during stat-file preloading, GUID tree is accessed for lookup.
However, as it is performed on startup which is single threaded, there
is no need for lock here. A BUG_ON() has been added to ensure this
precondition remains true.

This bug could caused a segfault when using dynamic servers with GUID.
However, it was never reproduced for now.

This must be backported up to 3.0. To avoid a conflict issue, the
previous cleanup patch can be merged before it.

8 months agoCLEANUP: guid: remove global tree export
Amaury Denoyelle [Thu, 7 Nov 2024 10:08:05 +0000 (11:08 +0100)] 
CLEANUP: guid: remove global tree export

guid_tree is not directly used outside of functions provided by the guid
module. Remove its export from the include file.

8 months agoMINOR: pattern: add pat_ref_free() helper func
Aurelien DARRAGON [Thu, 7 Nov 2024 09:11:17 +0000 (10:11 +0100)] 
MINOR: pattern: add pat_ref_free() helper func

For now, pat_ref struct are never freed, except during init in case of
error. The freeing is done directly in the init functions because we
don't have an helper for that.

No having an helper func to properly free pat_ref struct doesn't encourage
us to free unused pat_ref structs, plus it is error-prone if new dynamic
members are added to pat_ref struct in the future.

To fix that, let's add a pat_ref_free() helper func and use it where
relevant (which means only under pat_ref init function for now..)

8 months agoOPTIM: pattern: use malloc() to initialize new pat_ref struct
Aurelien DARRAGON [Thu, 7 Nov 2024 09:05:30 +0000 (10:05 +0100)] 
OPTIM: pattern: use malloc() to initialize new pat_ref struct

As mentioned in the previous commit, in _pat_ref_new(), it was not
strictly needed to explicitly assign all struct members to 0 since
the struct was allocated with calloc() which does the zeroing for us.

However, it was verified that we already initialize all fields explictly,
thus there is no reason to keep using calloc() instead of malloc(). In
fact using malloc() is less expensive, so let's use that instead now.

8 months agoMINOR: pattern: add _pat_ref_new() helper func
Aurelien DARRAGON [Thu, 7 Nov 2024 09:01:40 +0000 (10:01 +0100)] 
MINOR: pattern: add _pat_ref_new() helper func

pat_ref_newid() and pat_ref_new() are two functions to create and
initialize a pat_ref struct based on input parameters.

Both function perform the same generic allocation and initialization
for pat_ref struct, thus there is quite a lot of code redundancy.

This is error-prone if the pat_ref init sequence has to be updated at
some point.

To reduce maintenance costs, let's add a _pat_ref_new() helper func that
takes care of the generic allocation and base initialization for pat_ref
struct.

8 months agoMINOR: event_hdl: add event_hdl_sub_list_empty() helper func
Aurelien DARRAGON [Fri, 18 Oct 2024 16:45:45 +0000 (18:45 +0200)] 
MINOR: event_hdl: add event_hdl_sub_list_empty() helper func

event_hdl_sub_list_empty() may be used to know if the subscription list
passed as argument is empty or not (ie: if there currently are any
subcribers or not). It can be useful to know if the subscription is empty
is order to avoid unecessary preparation work and skip event publishing to
save CPU time if we already know that no one is interested in tracking the
changes for a given subscription list.

8 months agoMINOR: debug: move the "recover now" warn message after the optional notes
Willy Tarreau [Thu, 7 Nov 2024 06:56:13 +0000 (07:56 +0100)] 
MINOR: debug: move the "recover now" warn message after the optional notes

At the end of the too long processing warning added by commit 0950778b3a
("MINOR: debug: add a function to dump a stuck thread"), there can be some
optional notes about lua and memory trimming. However it's a bit awkward
that they appear after the "trying to recover now" message. Let's just move
that message after the notes.

8 months agoDEBUG: wdt: set the default blocked task delay to 100 ms
Willy Tarreau [Wed, 6 Nov 2024 16:51:57 +0000 (17:51 +0100)] 
DEBUG: wdt: set the default blocked task delay to 100 ms

The warn-blocked-traffic-after can be significantly lowered. In any
case, in order to be usable it must be well below the limit to have a
chance to emit exploitable traces before the watchdog finally fires.
Even configured at 1ms it looks very difficult to trigger it on a
laptop doing SSL and compression, so applying a 100-fold factor to
cover for large configs and small machines sounds sane for 3.1. In any
case, even at 100ms, the service degradation becomes quite visible.

8 months agoDEBUG: wdt: add a stats counter "BlockedTrafficWarnings" in show info
Willy Tarreau [Wed, 6 Nov 2024 17:10:01 +0000 (18:10 +0100)] 
DEBUG: wdt: add a stats counter "BlockedTrafficWarnings" in show info

Every time a warning is issued about traffic being blocked, let's
increment a global counter so that we can check for this situation
in "show info".

8 months agoDEBUG: wdt: make the blocked traffic warning delay configurable
Willy Tarreau [Wed, 6 Nov 2024 16:48:41 +0000 (17:48 +0100)] 
DEBUG: wdt: make the blocked traffic warning delay configurable

The new global "warn-blocked-traffic-after" allows one to configure
after how much time a warning should be emitted when traffic is blocked.

8 months agoDEBUG: cli: make it possible for "debug dev loop" to trigger warnings
Willy Tarreau [Wed, 6 Nov 2024 10:47:55 +0000 (11:47 +0100)] 
DEBUG: cli: make it possible for "debug dev loop" to trigger warnings

A new argument "warn" allows to force the emission of a warning while
stuck in the loop by making the internal state inconsistent.

8 months agoDEBUG: wdt: better detect apparently locked up threads and warn about them
Willy Tarreau [Wed, 6 Nov 2024 10:21:45 +0000 (11:21 +0100)] 
DEBUG: wdt: better detect apparently locked up threads and warn about them

In order to help users detect when threads are behaving abnormally, let's
try to emit a warning when one is no longer making any progress. This will
allow to catch faulty situations more accurately, instead of occasionally
triggering just after the long task. It will also let users know that there
is something wrong with their configuration, and inspect the call trace to
figure whether they're using excessively long rules or Lua for example (the
usual warnings about lua-load vs lua-load-per-thread are still reported).

The warning will only be emitted for threads not yet marked as stuck so
as not to interfere with panic dumps and avoid sending a warning just
before a panic. A tainted flag is set when this happens however (0x2000).

8 months agoMINOR: debug: add a function to dump a stuck thread
Willy Tarreau [Wed, 6 Nov 2024 10:20:45 +0000 (11:20 +0100)] 
MINOR: debug: add a function to dump a stuck thread

There's currently no way to just emit a warning informing that a thread
is stuck without crashing. This is a problem because sometimes users
would benefit from this info to clean up their configuration (e.g. abuse
of map_regm, lua-load etc).

This commit adds a new function ha_stuck_warning() that will emit a
warning indicating that the designated thread has been stuck for XX
milliseconds, with a number of streams blocked, and will make that
thread dump its own state. The warning will then be sent to stderr,
along with some reminders about the impacts of such situations to
encourage users to fix their configuration.

In order not to disrupt operations, a local 4kB buffer is allocated
in the stack. This should be quite sufficient.

For now the function is not used.

8 months agoMINOR: wdt: move the local timers to a struct
Willy Tarreau [Wed, 6 Nov 2024 09:54:05 +0000 (10:54 +0100)] 
MINOR: wdt: move the local timers to a struct

Better have a local struct for per-thread timers, as this will allow us
to store extra info that are useful to improve accurate reporting.

8 months agoCLEANUP: stats: fix misleading comment on top of stat_idx_info
Willy Tarreau [Wed, 6 Nov 2024 17:07:14 +0000 (18:07 +0100)] 
CLEANUP: stats: fix misleading comment on top of stat_idx_info

The comment asks to update the "metrics_info" array, which does not
exist, instead it's called stat_cols_info[] and is in stats.c. Let's
mention all that to save time searching for the needed info.

While no version seems to have ever known that "metrics_info", it's not
needed to backport this as it's only a comment.

8 months agoBUG/MEDIUM: quic: do not consider ACK on released stream as error
Amaury Denoyelle [Wed, 6 Nov 2024 16:18:45 +0000 (17:18 +0100)] 
BUG/MEDIUM: quic: do not consider ACK on released stream as error

When an ACK is received by haproxy, a lookup is performed to retrieve
the related emitted frames. For STREAM type frames, a lookup is
performed under quic_conn stream_desc tree. Indeed, the corresponding
stream instance could be already released if multiple ACK were received
refering to the same stream offset, which can happen notably if
retransmission occured.

qc_handle_newly_acked_frm() implements this logic. If the case with an
already released stream is encounted, an error is returned. In the end,
this error is propagated via qc_parse_pkt_frms() into
qc_treat_rx_pkts(), despite being in fact a perfectly valid case. Fix
this by adjusting ACK handling function to return a success value for
the particular case of released stream instead.

The impact of this bug is unknown, but it can have several consequences.
* if the packet with the ACK contains other frames after it, their
  content will be skipped
* the packet won't be acknowledged by haproxy, even if it contains other
  frames and is ack-eliciting. This may cause unneeded retransmission by
  the client.
* RTT sampling information related to this ACK is ignored by haproxy

Finally, it also caused the increment of the quic_conn counter
dropped_parsing (droppars in "show quic" output) which should be
reserved only for real error cases.

This regression is present since the following patch :
  e7578084b0536e3e5988be7f09091c85beb8fa9d
  MINOR: quic: implement dedicated type for out-of-order stream ACK

Before, qc_handle_newly_acked_frm() return type was always ignored. As
such, no backport is needed.

8 months agoEXAMPLES: add "traces.cfg" with traces examples
William Lallemand [Wed, 6 Nov 2024 16:19:57 +0000 (17:19 +0100)] 
EXAMPLES: add "traces.cfg" with traces examples

Add an example on how to use the traces section. The example use the
3.1-dev8 syntax and enables all traces on stderr.

8 months agoBUG/MINOR: mworker: do 'program' postparser checks in read_cfg_in_discovery_mode
Valentine Krasnobaeva [Thu, 31 Oct 2024 10:44:36 +0000 (11:44 +0100)] 
BUG/MINOR: mworker: do 'program' postparser checks in read_cfg_in_discovery_mode

cfg_program_postparser() contains 2 parts:

- check the combination of MODE_MWORKER and "program" section. if
"program" section was parsed, MODE_MWORKER is mandatory;

- check "command" keyword, which is mandatory for this section as
well.

This is more appropriate now, after the master-worker refactoring, do the
first part in read_cfg_in_discovery_mode, where we already check the
combination of MODE_MWORKER and -S option.

We need to do the second part just below, in read_cfg_in_discovery_mode() as
well, because it's only the master process, who parses now program section and
programs are forked before running postparser functions in step_init_2.
Otherwise, mworker_ext_launch_all() will emit a log message, that program is
started, but actually nothing has been launched, if 'command' keyword is
absent.

This not needs to be backported, as related to the master-worker refactoring.

8 months agoBUG/MINOR: quic: repeat packet parsing to deal with fragmented CRYPTO
Amaury Denoyelle [Mon, 4 Nov 2024 16:28:02 +0000 (17:28 +0100)] 
BUG/MINOR: quic: repeat packet parsing to deal with fragmented CRYPTO

A ClientHello may be splitted accross several different CRYPTO frames,
then mixed in a single QUIC packet. This is used notably by clients such
as chrome to render the first Initial packet opaque to middleboxes.

Each packet frame is handled sequentially. Out-of-order CRYPTO frames
are buffered in a ncbuf, until gaps are filled and data is transferred
to the SSL stack. If CRYPTO frames are heavily splitted with small
fragments, buffering may fail as ncbuf does not support small gaps. This
causes the whole packet to be rejected and unacknowledged. It could be
solved if the client reemits its ClientHello after remixing its CRYPTO
frames.

This patch is written to improve CRYPTO frame parsing. Each CRYPTO
frames which cannot be buffered due to ncbuf limitation are now stored
in a temporary list. Packet parsing is completed until all frames have
been handled. If temporary list is not empty, reparsing is done on the
stored frames. With the newly buffered CRYPTO frames, ncbuf insert
operation may this time succeeds if the frame now covers a whole gap.
Reparsing will loop until either no progress can be made or it has been
done at least 3 times, to prevent CPU utilization.

This patch should fix github issue #2776.

This should be backported up to 2.6, after a period of observation. Note
that it relies on the following refactor patches :
  MINOR: quic: extend return value of CRYPTO parsing
  MINOR: quic: use dynamically allocated frame on parsing
  MINOR: quic: simplify qc_parse_pkt_frms() return path

8 months agoMINOR: quic: extend return value of CRYPTO parsing
Amaury Denoyelle [Mon, 4 Nov 2024 16:27:39 +0000 (17:27 +0100)] 
MINOR: quic: extend return value of CRYPTO parsing

qc_handle_crypto_frm() is the function used to handled a newly received
CRYPTO frame. Change its API to use a newly dedicated return type. This
allows to report if the frame was properly handled, ignored if already
parsed previously or rejected after a fatal error.

This commit does not have any functional changes. However, it allows to
simplify qc_handle_crypto_frm() API by removing <fast_retrans> as output
parameter. Also, this patch will be necessary to support multiple
iteration of packet parsing for CRYPTO frames.

8 months agoMINOR: quic: use dynamically allocated frame on parsing
Amaury Denoyelle [Tue, 5 Nov 2024 15:33:27 +0000 (16:33 +0100)] 
MINOR: quic: use dynamically allocated frame on parsing

qc_parse_pkt_frms() is the function responsible to parse a received QUIC
packet. Payload is decoded and splitted into individual frames which are
then handled individually. Previously, frame was used as locally stack
allocated. Change this to work on a dynamically allocated frame.

This commit does bring any functional changes. However, it will be
useful to extend packet parsing. In particular, it will be necessary to
save some frames during parsing to reparse them after the others.

8 months agoMINOR: quic: simplify qc_parse_pkt_frms() return path
Amaury Denoyelle [Mon, 4 Nov 2024 17:17:01 +0000 (18:17 +0100)] 
MINOR: quic: simplify qc_parse_pkt_frms() return path

Change qc_parse_pkt_frms() return path for normal and error cases. Most
notably, it allows to remove local variable ret as now return value is
hardcoded on normal and err label. This also allows to define a
different trace for error leaving code.

8 months agoMINOR: http: don't %-encode the payload when not relevant
Aurelien DARRAGON [Wed, 6 Nov 2024 08:48:32 +0000 (09:48 +0100)] 
MINOR: http: don't %-encode the payload when not relevant

As reported by Pierre Maoui in GH #2477, it's not possible to render
control chars from variables or expressions verbatim in the payload part
of http-return statements. That's a problem because this part should not
require to be encoded at all (we could even imagine building favicons on
the fly for example).

In fact it is the LOG_OPT_HTTP option when passed as default options on
parse_logformat_string() which tells the log encoder that the payload
should be http-encoded using lf_chunk() instead of being printed using the
per-type encoder.

This option was set when parsing logformat expressions for lf-string
expression under http-return statements, as well as logformat expressions
for set-map action. While it is true that those actions may only be
used under http context, the LOG_OPT_HTTP logformat option is not relevant
there, because the payload is expected to be used without being encoded.

So let's simply get rid of this option when parsing logformat expressions
for set-map action key/value and lf-string from http-request return
action, and add a note next to LOG_OPT_HTTP option to indicate that it is
used to tell the log encoder that the payload should be HTTP-encoded.

Thanks to Pierre for having reported the issue and Willy for the
analysis and patch proposal.

8 months agoMINOR: http-conv: Remove unreachable goto statement in sample_conv_q_preferred
Christopher Faulet [Wed, 6 Nov 2024 09:06:50 +0000 (10:06 +0100)] 
MINOR: http-conv: Remove unreachable goto statement in sample_conv_q_preferred

This was reported by Coverity. In sample_conv_q_preferred() function, a goto
statement after a "while(1)" loop is unreachable. Instead of just removing
it, the same goto statement in the loop is replaced by a break. It is safer
this way, in case the loop change in future.

This patch should fix the issue #2683.

8 months agoMINOR: listener: Remove useless checks on the receiver protocol existence
Christopher Faulet [Wed, 6 Nov 2024 08:35:00 +0000 (09:35 +0100)] 
MINOR: listener: Remove useless checks on the receiver protocol existence

The receiver protocol is always set when a listener is created or cloned. At
least for now. And there is no check on it at many places, except in
listener_accept() function. So, let's remove remaining useless checks. That
will avoid false Coverity reports in future.

This patch should fix the issue #2631.

8 months agoBUG/MINOR: quic: fix malformed probing packet building
Frederic Lecaille [Mon, 4 Nov 2024 17:50:10 +0000 (18:50 +0100)] 
BUG/MINOR: quic: fix malformed probing packet building

This bug arrived with this commit:

   cdfceb10a MINOR: quic: refactor qc_prep_pkts() loop

which prevents haproxy from sending PING only packets/datagrams (some
packets/datagrams with only PING frame as ack-eliciting frames inside).
Such packets/datagrams are useful in rare cases during retransmissions
when one wants to probe the peer without exceeding the anti-amplification
limit.

Modify the condition passed to qc_build_pkt() to add padding to the current
datagram. One does not want to do that when probing the peer without ack-eliciting
frames passed as <frms> parameter. Indeed qc_build_pkt() calls qc_do_build_pkt()
which supports this case: if <probe> is true (probing required), qc_do_build_pkt()
handles the case where some padding must be added to a PING only packet/datagram.
This is the case when probing with an empty <frms> frame list of ack-eliciting
frames without exceeding the anti-amplification limit from qc_dgrams_retransmit().

Add some comments to qc_build_pkt() and qc_do_build_pkt() to clarify this
as this code is easy to break!

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

Must be backported to 3.0.

8 months agoMINOR: quic: Help diagnosing malformed probing packets
Frederic Lecaille [Mon, 4 Nov 2024 16:55:38 +0000 (17:55 +0100)] 
MINOR: quic: Help diagnosing malformed probing packets

Add a BUG_ON() to detect some malformed packets which are supposed to probe the
peer without being ack-eliciting: the peer would not acknowledged such packets.

8 months agoMINOR: connection: add new sample fetch functions fc_err_name and bc_err_name
Willy Tarreau [Tue, 5 Nov 2024 17:04:21 +0000 (18:04 +0100)] 
MINOR: connection: add new sample fetch functions fc_err_name and bc_err_name

These functions return a symbolic error code such as ECONNRESET to keep
logs compact while making them human-readable. It's a good alternative
to the numeric code in that it's more expressive, and a good one to the
full message since it's shorter and more precise (some codes even match
errno names).

The doc was updated so that the symbolic names appear in the table. It
could be useful to backport this feature to help with troubleshooting
some issues, though backporting the doc might possibly be more annoying
in case users have local patches already, so maybe the table update does
not need to be backported in this case.

8 months agoMINOR: rawsock: set connection error codes when returning from recv/send/splice
Willy Tarreau [Tue, 5 Nov 2024 16:57:43 +0000 (17:57 +0100)] 
MINOR: rawsock: set connection error codes when returning from recv/send/splice

For a long time the errno values returned by recv/send/splice() were not
translated to connection error codes. There are not that many eligible
and having them would help a lot when debugging some complex issues where
logs disagree with network traces. Let's add them now.

8 months agoMINOR: connection: add more connection error codes to cover common errno
Willy Tarreau [Tue, 5 Nov 2024 16:49:15 +0000 (17:49 +0100)] 
MINOR: connection: add more connection error codes to cover common errno

While we get reports of connection setup errors in fc_err/bc_err, we
don't have the equivalent for the recv/send/splice syscalls. Let's
add provisions for new codes that cover the common errno values that
recv/send/splice can return, i.e. ECONNREFUSED, ENOMEM, EBADF, EFAULT,
EINVAL, ENOTCONN, ENOTSOCK, ENOBUFS, EPIPE. We also add a special case
for when the poller reported the error itself. It's worth noting that
EBADF/EFAULT/EINVAL will generally indicate serious bugs in the code
and should not be reported.

The only thing is that it's quite hard to forcefully (and reliably)
trigger these errors in automated tests as the timing is critical.
Using iptables to manually reset established connections in the
middle of large transfers at least permits to see some ECONNRESET
and/or EPIPE, but the other ones are harder to trigger.

8 months agoDEBUG: cli: support closing "hard" using close() in addition to fd_delete()
Willy Tarreau [Tue, 5 Nov 2024 16:43:35 +0000 (17:43 +0100)] 
DEBUG: cli: support closing "hard" using close() in addition to fd_delete()

"debug dev close <fd>" currently closes that FD using fd_delete() after
checking that it's known from the fdtab. Sometimes we also want to just
perform a pure close() of FDs not in the fdtab (pollers, etc) in order
to provoke certain error cases. The optional "hard" argument to the
command will make it use a plain close() instead of fd_delete() and skip
the fd owner check. The main visible effect when closing a traffic socket
with it is that instead of dying from a double fd_delete() by seeing that
fd.owner is already 0, it will die during the next fd_insert() seeing that
fd.owner was not 0.

8 months agoCLEANUP: connection: properly name the CO_ER_SSL_FATAL enum entry
Willy Tarreau [Tue, 5 Nov 2024 17:05:58 +0000 (18:05 +0100)] 
CLEANUP: connection: properly name the CO_ER_SSL_FATAL enum entry

It was the only one prefixed with "CO_ERR_", making it harder to batch
process and to look up. It was added in 2.5 by commit 61944f7a73 ("MINOR:
ssl: Set connection error code in case of SSL read or write fatal failure")
so it can be backported as far as 2.6 if needed to help integrate other
patches.

8 months agoDOC: config: document connection error 44 (reverse connect failure)
Willy Tarreau [Tue, 5 Nov 2024 16:54:59 +0000 (17:54 +0100)] 
DOC: config: document connection error 44 (reverse connect failure)

It was missing from commit ac1164de7c ("MINOR: connection: define error
for reverse connect"), and can be backported to 3.0 and 2.9.

8 months agoCLEANUP: quic: Remove the useless directive "tune.quic.backend.max-idle-timeou"
Christopher Faulet [Tue, 5 Nov 2024 17:51:04 +0000 (18:51 +0100)] 
CLEANUP: quic: Remove the useless directive "tune.quic.backend.max-idle-timeou"

First there is a typo in the directive name, then it is not documented and
finally, it is not used at all. The directive is only removed from the
keyword list. Parsing function is not updated.

This patch should fix the issue #2601.

8 months agoBUILD: compiler: define __builtin_prefetch() for tcc
Willy Tarreau [Tue, 5 Nov 2024 14:41:57 +0000 (15:41 +0100)] 
BUILD: compiler: define __builtin_prefetch() for tcc

We're using a few occurrences of __builtin_prefetch() but tcc doesn't
know about it so let's give it a dummy definition. Now the code builds
and works again with tcc without thread support.

8 months agoBUILD: import/mt_list: support building with TCC
Willy Tarreau [Tue, 5 Nov 2024 14:25:31 +0000 (15:25 +0100)] 
BUILD: import/mt_list: support building with TCC

TCC is often convenient to quickly test builds, run CI tests etc. It has
limited thread support (e.g. no thread-local stuff) but that is often
sufficient for testing. TCC lacks __atomic_exchange_n() but has the
exactly equivalent __atomic_exchange(), and doesn't have any barrier.
For this reason we force the atomic_exchange to use the stricter SEQ_CST
mem ordering that allows to ignore the barrier.

[wt: that's upstream commit ca8b865 ("BUILD: support building with TCC")]

8 months agoBUG/MEDIUM: promex: Fix dump of extra counters
Christopher Faulet [Tue, 5 Nov 2024 14:30:56 +0000 (15:30 +0100)] 
BUG/MEDIUM: promex: Fix dump of extra counters

When extra counters are dumped for an entity (frontend, backend, server or
listener), there is a filter on capabilities. Some extra counters are not
available for all entities and must be ignored. However, when this was
performed, the field number, used as an index to dump the metric value, was
still incremented while it should not and leads to an overflow or a stats
mix-up.

This patch must be backported to 3.0.

8 months agoMINOR: startup: tune.renice.{startup,runtime} allow to change priorities
William Lallemand [Tue, 29 Mar 2022 05:53:31 +0000 (07:53 +0200)] 
MINOR: startup: tune.renice.{startup,runtime} allow to change priorities

This commit introduces the tune.renice.startup and tune.renice.runtime
global keywords that allows to change the priority with setpriority().

tune.renice.startup is parsed and applied in the worker or the standalone
process for configuration parsing. If this keyword is used alone, the
nice value is changed to the previous one after configuration parsing.

tune.renice.runtime is applied after configuration parsing, so in the
worker or a standalone process. Combined with tune.renice.startup it
allows to have a different nice value during configuration parsing and
during runtime.

The feature was discussed in github issue #1919.

Example:

   global
        tune.renice.startup 15
        tune.renice.runtime 0

8 months ago[RELEASE] Released version 3.1-dev11 v3.1-dev11
Willy Tarreau [Fri, 1 Nov 2024 09:17:02 +0000 (10:17 +0100)] 
[RELEASE] Released version 3.1-dev11

Released version 3.1-dev11 with the following main changes :
    - BUG/MINOR: httpclient: return NULL when no proxy available during httpclient_new()
    - BUG/MEDIUM: mworker/httpclient: initialization skipped by accident in mworker mode
    - BUG/MINOR: resolvers/mworker: missing default resolvers in mworker mode
    - MINOR: mworker/ocsp: skip ocsp-update proxy init in master
    - BUG/MEDIUM: stconn: Wait iobuf is empty to shut SE down during a check send
    - MINOR: mux-h1: Show the SD iobuf in trace messages on stream send events
    - MINOR: mux-h1: Add a trace on shutdown when keep-alive is not possible
    - BUG/MINOR: http-ana: Don't report a server abort if response payload is invalid
    - BUG/MEDIUM: stconn: Check FF data of SC to perform a shutdown in sc_notify()
    - BUG/MAJOR: filters/htx: Add a flag to state the payload is altered by a filter
    - REGTESTS: Never reuse server connection in http-messaging/truncated.vtc
    - BUG/MINOR: quic: avoid leaking post handshake frames
    - MINOR: quic: send new tokens (NEW_TOKEN) even for 1RTT sessions
    - BUG/MEDIUM: quic: avoid freezing 0RTT connections
    - DOC: config: fix rfc7239 forwarded typo in desc
    - MINOR: http_ext: implement rfc7239_{nn,np} converters
    - CLEANUP: http_ext: remove useless BUG_ON() in http_handle_xot_header()
    - BUG/MINOR: sample: free err2 in smp_resolve_args for type ARGT_REG
    - MINOR: arg: add an argument type for identifier
    - BUILD: buffers: keep b_getblk_nc() and b_peek_varint() in buf.h
    - CLEANUP: buffers: simplify b_get_varint()
    - OPTIM: buffers: avoid a useless wrapping check for ofs == 0
    - MINOR: debug: make mark_tainted() return the previous value
    - MINOR: chunk: drop the global thread_dump_buffer
    - MINOR: debug: split ha_thread_dump() in two parts
    - MINOR: debug: slightly change the thread_dump_pointer signification
    - MINOR: debug: make ha_thread_dump_done() take the pointer to be used
    - MINOR: debug: replace ha_thread_dump() with its two components
    - MEDIUM: debug: on panic, make the target thread automatically allocate its buf
    - BUILD: mux-h2/traces: fix build on 32-bit due to size of the DATA frame
    - CI: prepare Coverity build for Ubuntu 24
    - CI: bump development builds explicitely to Ubuntu 24.04
    - CI: modernize macos builds to macos-15
    - BUG/MINOR: mworker: fix mworker-max-reloads parser
    - MINOR: mux-quic: simplify sending of empty STREAM FIN
    - BUG/MINOR: mux-quic: do not close STREAM with empty FIN if no data sent
    - CLEANUP: debug: make the BUG_ON() macros check the condition in the outer one
    - MEDIUM: debug: add match counters for BUG_ON/WARN_ON/CHECK_IF
    - MINOR: debug: add a new debug macro COUNT_IF()
    - MINOR: debug: add "debug dev counters" to list code counters
    - BUG/MEDIUM: stats-html: Never dump more data than expected during 0-copy FF
    - BUG/MEDIUM: mux-h2: Remove H2S from send list if data are sent via 0-copy FF
    - BUG/MINOR: stconn: Pretend the SE have more data to deliver on abortonclose
    - CLEANUP: stream: remove outdated comments
    - DEBUG: stream: Add debug counters to track some client/server aborts
    - DEBUG: mux-h1: Add debug counters to track some errors
    - MINOR: mux-h1: Add support of the debug string for logs
    - MINOR: stream: maintain per-stream counters of the number of passes on code
    - MINOR: filters: add per-filter call counters
    - MINOR: sample: add the "when" converter to condition some expressions
    - BUG/MEDIUM: connection/http-reuse: fix address collision on unhandled address families
    - BUILD: spoe: fix build warning on older gcc around sub-struct initialization
    - Revert "OPTIM: mux-h2: make h2_send() report more accurate wake up conditions"
    - DEBUG: mux-h1: Add debug counters to track errors with in/out pending data
    - BUG/MINOR: mux-h1: Fix conditions on pipe in some COUNT_IF()
    - MINOR: activity/memprofile: show per-DSO stats
    - BUG/MINOR: mworker/cli: show master startup logs in recovery mode
    - MINOR: mworker: stop MASTER proxy listener on worker mcli sockpair
    - MINOR: error: simplify startup_logs_init_shm
    - BUG/MINOR: mworker: show worker warnings in startup logs
    - CLEANUP: mworker: clean mworker_reexec
    - MINOR: mworker/cli: split mworker_cli_proxy_create
    - BUG/MINOR: server: fix dynamic server leak with check on failed init
    - BUG/MEDIUM: server: fix race on servers_list during server deletion
    - BUG/MEDIUM: stconn: Report blocked send if sends are blocked by an error
    - BUG/MINOR: http-ana: Fix wrong client abort reports during responses forwarding
    - BUG/MINOR: stconn: Don't disable 0-copy FF if EOS was reported on consumer side
    - MINOR: mworker/cli: add 'debug' to 'show proc'
    - MINOR: mworker/cli: remove comment line for program when useless
    - MINOR: mworker/cli: 'show proc debug' for old workers
    - BUILD: debug: silence a build warning with threads disabled
    - CLEANUP: mux-h2: remove the unused "full" variable in h2_frt_transfer_data()
    - MINOR: pools: export the pools variable
    - MINOR: debug: place a magic pattern at the beginning of post_mortem
    - MINOR: debug: place the post_mortem struct in its own section.
    - MINOR: debug: store important pointers in post_mortem
    - MINOR: debug: do not limit backtraces to stuck threads
    - MINOR: cli: remove non-printable characters from 'debug dev fd'
    - MINOR: cli: add an 'echo' command
    - MINOR: debug: also add a pointer to struct global to post_mortem
    - CLEANUP: mworker: make mworker_create_master_cli more readable
    - BUG/MEIDUM: mworker: fix fd leak from master to worker
    - BUG/MINOR: mworker/cli: fix mworker_cli_global_proxy_new_listener
    - MINOR: tools: add strnlen2() helper
    - CLEANUP: log: use strnlen2() in _lf_text_len() to compute string length
    - DOC: design: add notes about more detailed error reporting for logs
    - MINOR: debug: also add fdtab and acitvity to struct post_mortem
    - MINOR: debug: remove the redundant process.thread_info array from post_mortem
    - DEV: gdb: add a number of gdb scripts to navigate in core dumps
    - BUG/MINOR: trace: stop rewriting argv with -dt
    - MEDIUM: protocol: make abns a custom unix socket address family
    - MEDIUM: protocol: rely on AF_CUST_ABNS family to recognize ABNS sockets
    - CLEANUP: tools: rely on address family to detect ABNS sockets
    - MINOR: protocol: create abnsz socket address family
    - MINOR: sock: restore effective UNIX family in sock_get_old_sockets()
    - MEDIUM: sock: also restore effective unix family in get_{src,dst}()
    - MEDIUM: sock_unix: use per-family addrcmp function
    - MEDIUM: socket: add zero-terminated ABNS alternative
    - BUG/MINOR: ssl/cli: 'set ssl cert' does not check the transaction name correctly
    - BUG/MINOR: mworker: mworker_reexec: unset MODE_STARTING before free startup logs ring
    - BUG/MINOR: errors: startup_logs_free: set global startup_logs ptr to NULL
    - BUG/MINOR: errors: print_message: don't allocate startup logs ring
    - BUG/MINOR: startup: don't fork worker if started with -c -W
    - BUG/MINOR: startup: dump libs only in worker if started with -W -dL
    - BUG/MINOR: startup: dump keywords only in worker if started with -W -dKAll
    - BUG/MINOR: startup: don't dump polling info for master in verbose mode
    - CI: switch QUIC Interop on AWS-LC to common docker image
    - CI: switch QUIC Interop on LibreSSL to common docker image
    - CI: enable chacha20 test on LibreSSL QUIC Interop
    - DOC: config: add missing glitch_{cnt,rate} data types
    - DOC: config: add missing glitch_{cnt,rate} sample definitions
    - CI: LibreSSL QUIC Interop: fix docker context
    - DEBUG: mux-h1: Add H1C expiration dates in trace messages
    - BUG/MEDIUM: mux-h1: Fix how timeouts are applied on H1 connections
    - BUG/MINOR: http-ana: Report internal error if an action yields on a final eval
    - MINOR: stream: Save last evaluated rule on invalid yield
    - MINOR: quic: complete trace in qc_may_build_pkt()
    - MINOR: quic: move qc_send_mux() prototype into quic_tx.h
    - MINOR: stream: Replace last_rule_file/line fields by a more generic field
    - MINOR: stream: Save the last filter evaluated interrupting the processing
    - MINOR: stream: Save the entity waiting to continue its processing
    - MINOR: stream: Use an enum to identify last and waiting entities for streams
    - MINOR: stream: Add http-buffer-request option in the waiting entities
    - DOC: config: Add documentation about last_entity sample fetch
    - DOC: config: Add documentation about waiting_entity sample fetch

8 months agoDOC: config: Add documentation about waiting_entity sample fetch
Christopher Faulet [Thu, 31 Oct 2024 15:33:19 +0000 (16:33 +0100)] 
DOC: config: Add documentation about waiting_entity sample fetch

The commit adds the documentation for the waiting_entity sample fetch.

8 months agoDOC: config: Add documentation about last_entity sample fetch
Christopher Faulet [Thu, 31 Oct 2024 15:33:12 +0000 (16:33 +0100)] 
DOC: config: Add documentation about last_entity sample fetch

The commit adds the documentation for the last_entity sample fetch.

8 months agoMINOR: stream: Add http-buffer-request option in the waiting entities
Christopher Faulet [Thu, 31 Oct 2024 13:16:01 +0000 (14:16 +0100)] 
MINOR: stream: Add http-buffer-request option in the waiting entities

When http-buffer-request option is set on a proxy, the processing will be
paused to wait the full request payload or a full buffer. So it is an entity
that block the processing, just like a rule or a filter that yields. So now,
it is reported as a waiting entity if an error or a timeout occurred.

To do so, an stream entity type is added for this option. There is no
pointer. And "waiting_entity" sample fetch returns the option name.

8 months agoMINOR: stream: Use an enum to identify last and waiting entities for streams
Christopher Faulet [Thu, 31 Oct 2024 12:53:56 +0000 (13:53 +0100)] 
MINOR: stream: Use an enum to identify last and waiting entities for streams

Instead of using 1 for last/waiting rule and 2 for last/waiting filter, an
enum is used. It is less ambiguous this way.

8 months agoMINOR: stream: Save the entity waiting to continue its processing
Christopher Faulet [Thu, 31 Oct 2024 10:34:54 +0000 (11:34 +0100)] 
MINOR: stream: Save the entity waiting to continue its processing

When a rule or a filter yields because it waits for something to be able to
continue its processing, this entity is saved in the stream. If an error or
a timeout occurred, info on this entity may be retrieved via the
"waiting_entity" sample fetch, for instance to dump it in the logs. This
info may be useful to found root cause of some bugs because it is a way to
know the processing was temporarily stopped. This may explain timeouts for
instance.

The sample fetch is not documented yet.

8 months agoMINOR: stream: Save the last filter evaluated interrupting the processing
Christopher Faulet [Thu, 31 Oct 2024 10:30:46 +0000 (11:30 +0100)] 
MINOR: stream: Save the last filter evaluated interrupting the processing

It is very similar to the last evaluated rule. When a filter returns an
error that interrupts the processing, it is saved in the stream, in the
last_entity field, with the type 2. The pointer on filter config is
saved. This pointer never changes during runtime and is part of the proxy's
structure. It is an element of the filter_configs list in the proxy
structure.

"last_entity" sample fetch was update accordingly. The filter identifier is
returned, if defined. Otherwise the save pointer.

8 months agoMINOR: stream: Replace last_rule_file/line fields by a more generic field
Christopher Faulet [Thu, 31 Oct 2024 10:23:12 +0000 (11:23 +0100)] 
MINOR: stream: Replace last_rule_file/line fields by a more generic field

The last evaluated rule is now saved in a generic structure, named
last_entity, with a type to identify it. The idea is to be able to store
other kind of entity that may interrupt a specific processing.

The type of the last evaluated rule is set to 1. It will be replace later by
an enum to be more explicit. In addition, the pointer to the rule itself is
saved instead of its location.

The sample fetch "last_entity" was added to retrieve the information about
it. In this case, it is the rule localtion, the config file containing the
rule followed by the line where the rule is defined, separated by a
colon. This sample fetch is not documented yet.

8 months agoMINOR: quic: move qc_send_mux() prototype into quic_tx.h
Amaury Denoyelle [Thu, 24 Oct 2024 14:32:29 +0000 (16:32 +0200)] 
MINOR: quic: move qc_send_mux() prototype into quic_tx.h

qc_send_mux() is defined in quic_tx.c. As such, its prototype is moved
from quic_conn.h to quic_tx.h.

8 months agoMINOR: quic: complete trace in qc_may_build_pkt()
Amaury Denoyelle [Fri, 25 Oct 2024 14:27:59 +0000 (16:27 +0200)] 
MINOR: quic: complete trace in qc_may_build_pkt()

Log the encryption level in qc_may_build_pkt(). This is necessary to
fully understand the sending conditions of the QUIC stack.

8 months agoMINOR: stream: Save last evaluated rule on invalid yield
Christopher Faulet [Tue, 29 Oct 2024 17:15:20 +0000 (18:15 +0100)] 
MINOR: stream: Save last evaluated rule on invalid yield

When an action yields while it is not allowed, an internal error is
reported. This interrupts the processing. So info about the last evaluated
rule must be filled.

This patch may be bakcported if needed. If so, the commit ("MINOR: stream:
Save last evaluated rule on invalid yield") must be backported first.

8 months agoBUG/MINOR: http-ana: Report internal error if an action yields on a final eval
Christopher Faulet [Tue, 29 Oct 2024 17:09:51 +0000 (18:09 +0100)] 
BUG/MINOR: http-ana: Report internal error if an action yields on a final eval

This was already performed for tcp actions at content level, but not for
HTTP actions. It is always a bug, so it must be reported accordingly.

This patch may be backported to all stable versions.

8 months agoBUG/MEDIUM: mux-h1: Fix how timeouts are applied on H1 connections
Christopher Faulet [Mon, 28 Oct 2024 07:18:32 +0000 (08:18 +0100)] 
BUG/MEDIUM: mux-h1: Fix how timeouts are applied on H1 connections

There were several flaws in the way the different timeouts were applied on
H1 connections. First, the H1C task handling timeouts was not created if no
client/server timeout was specified. But there are other timeouts to
consider. First, the client-fin/server-fin timeouts. But for frontend
connections, http-keey-alive and http-request timeouts may also be used. And
finally, on soft-stop, the close-spread-time value must be considered too.

So at the end, it is probably easier to always create a task to manage H1C
timeouts. Especially since the client/server timeouts are most often set.

Then, when the expiration date of the H1C's task must only be updated if the
considered timeout is set. So tick_add_ifset() must be used instead of
tick_add(). Otherwise, if a timeout is undefined, the taks may expire
immediately while it should in fact never expire.

Finally, the idle expiration date must only be considered for idle
connections.

This patch should be backported in all stable versions, at least as far as
2.6. On the 2.4, it will have to be slightly adapted for the idle_exp
part. On 2.2 and 2.0, the patch will have to be rewrite because
h1_refresh_timeout() is quite different.

8 months agoDEBUG: mux-h1: Add H1C expiration dates in trace messages
Christopher Faulet [Mon, 28 Oct 2024 06:43:05 +0000 (07:43 +0100)] 
DEBUG: mux-h1: Add H1C expiration dates in trace messages

The expiration date of the H1C task and the H1C idle expiration date are now
dumped in the trace messages.

8 months agoCI: LibreSSL QUIC Interop: fix docker context
Ilia Shipitsin [Wed, 30 Oct 2024 18:27:52 +0000 (19:27 +0100)] 
CI: LibreSSL QUIC Interop: fix docker context

in the commit https://github.com/haproxy/haproxy/commit/98099287ee37e697d1d2aaf01e19107bd064c3c6
building docker was switched to URL, but I forgotten to change context.

this is a followup fix.

8 months agoDOC: config: add missing glitch_{cnt,rate} sample definitions
Aurelien DARRAGON [Wed, 30 Oct 2024 16:37:39 +0000 (17:37 +0100)] 
DOC: config: add missing glitch_{cnt,rate} sample definitions

Following previous commit, when glitch_cnt and glitch_rate data types were
implemented in c9c6b683f ("MEDIUM: stick-tables: add a new stored type for
glitch_cnt and glitch_rate"), newly exposed samples such as
table_glitch_cnt(), table_glitch_rate, src_glitch_cnt() and
src_glitch_rate() were documented but their definitions was missing in
supported keywords list.

It should be backported in 3.0 with c9c6b683f

8 months agoDOC: config: add missing glitch_{cnt,rate} data types
Aurelien DARRAGON [Wed, 30 Oct 2024 16:22:33 +0000 (17:22 +0100)] 
DOC: config: add missing glitch_{cnt,rate} data types

When glitch_cnt and glitch_rate data types were implemented in
c9c6b683f ("MEDIUM: stick-tables: add a new stored type for glitch_cnt and
glitch_rate"), the data types list for "stick-table" keyword documentation
was overlooked.

This was reported by Nick Ramirez.

It should be backported in 3.0 with c9c6b683f.

8 months agoCI: enable chacha20 test on LibreSSL QUIC Interop
Ilia Shipitsin [Tue, 29 Oct 2024 20:49:03 +0000 (21:49 +0100)] 
CI: enable chacha20 test on LibreSSL QUIC Interop

it was commented on purpose "until LibreSSL-4.0 is released".
lets enable it

8 months agoCI: switch QUIC Interop on LibreSSL to common docker image
Ilia Shipitsin [Tue, 29 Oct 2024 20:49:02 +0000 (21:49 +0100)] 
CI: switch QUIC Interop on LibreSSL to common docker image

previously we used different docker images for different SSL libs,
now all of them are merged into one, lets switch to it

8 months agoCI: switch QUIC Interop on AWS-LC to common docker image
Ilia Shipitsin [Tue, 29 Oct 2024 20:49:01 +0000 (21:49 +0100)] 
CI: switch QUIC Interop on AWS-LC to common docker image

previously we used different docker images for different SSL libs,
now all of them are merged into one, lets switch to it

8 months agoBUG/MINOR: startup: don't dump polling info for master in verbose mode
Valentine Krasnobaeva [Wed, 30 Oct 2024 09:49:26 +0000 (10:49 +0100)] 
BUG/MINOR: startup: don't dump polling info for master in verbose mode

As master-worker fork happens now before step_init_2(), when pollers are
initialized and polling settings and dumped then in verbose and in debug modes
to stdout, it turns out that master and worker dump its same polling
settings separately. This creates long and messy output in these modes.

Polling settings are the same for master and for worker process for the moment.
Even if they would diverge in future we are interested here in worker's
settings. So, when started in the master-worker mode let's dump it only in the
worker context.

This doesn't need to be backported as related to the latest master-worker
refactoring.

8 months agoBUG/MINOR: startup: dump keywords only in worker if started with -W -dKAll
Valentine Krasnobaeva [Tue, 29 Oct 2024 18:25:07 +0000 (19:25 +0100)] 
BUG/MINOR: startup: dump keywords only in worker if started with -W -dKAll

If haproxy was started with -W -dK*, after master-worker refactoring, we dump
registered keywords to stdout twice in master and in worker processes. This
information is redundant and output has no longer the right format. So, as the
keyword registration happens very early before the fork, let's dump keywords
only in the worker context, if haproxy was launched with -W.

This does not need to be backported, as related to the latest master-worker
refactoring.

8 months agoBUG/MINOR: startup: dump libs only in worker if started with -W -dL
Valentine Krasnobaeva [Tue, 29 Oct 2024 18:22:40 +0000 (19:22 +0100)] 
BUG/MINOR: startup: dump libs only in worker if started with -W -dL

If haproxy was started with -W -dL, after master-worker refactoring we dump
libs to stdout twice in master and in worker processes. This is information is
redundant. So let's show linked libraries only in the worker context, if
haproxy was started also with -W.

This does not need to be backported, as related to the latest master-worker
rework.

8 months agoBUG/MINOR: startup: don't fork worker if started with -c -W
Valentine Krasnobaeva [Tue, 29 Oct 2024 18:14:11 +0000 (19:14 +0100)] 
BUG/MINOR: startup: don't fork worker if started with -c -W

Don't do master-worker fork if MODE_CHECK is detected from the command line along
with the master-worker mode. We should exit in MODE_CHECK, after the
configuration parsing and validation. So, with the new master-worker architecture
it's better to align this mode with the standalone.

This patch does not need to be backported, as related to the latest
master-worker rework.

8 months agoBUG/MINOR: errors: print_message: don't allocate startup logs ring
Valentine Krasnobaeva [Mon, 28 Oct 2024 14:17:11 +0000 (15:17 +0100)] 
BUG/MINOR: errors: print_message: don't allocate startup logs ring

Don't call startup_logs_init() in order to allocate the startup logs ring
again, if startup_logs pointer is NULL. Startup logs ring is allocated
explicitly in step_init_1 routine, when the process starts, and it's freed
explicitly for master process at the end of mworker_reexec scope. So, when
we no longer have this pointer, let's just save the log message in the
message buffer.

Otherwise, in case of master process, we will allocate the startup logs ring
again here and we will lost its address after execvp.

No need to backport this fix as it's related to the latest master-worker
refactoring.

8 months agoBUG/MINOR: errors: startup_logs_free: set global startup_logs ptr to NULL
Valentine Krasnobaeva [Mon, 28 Oct 2024 15:03:44 +0000 (16:03 +0100)] 
BUG/MINOR: errors: startup_logs_free: set global startup_logs ptr to NULL

ring_free() calls free() on the ring struct pointer, but startup_logs continues
to keep this address. So let's reset at the end startup_logs to NULL.
startup_logs is checked in print_message().

No need to backport this fix, as it's related to the latest master-worker
refactoring.

8 months agoBUG/MINOR: mworker: mworker_reexec: unset MODE_STARTING before free startup logs...
Valentine Krasnobaeva [Sat, 26 Oct 2024 21:02:38 +0000 (23:02 +0200)] 
BUG/MINOR: mworker: mworker_reexec: unset MODE_STARTING before free startup logs ring

Flag MODE_STARTING should be unset for master just before freeing the startup
logs ring, as it triggers the copy of process logs to this ring, see the code
of print_message().

Moreover with this flag set, if startup logs ring pointer is NULL, any
print_message() triggered just before the execvp in mworker_reexec() will call
startup_logs_init(). So ring will be allocated again "discretely" and after
execvp we will lost its address, as in step_init_1() we will call again
startup_logs_init().

No need to backport this fix as it's related to the latest master-worker
refactoring.

8 months agoBUG/MINOR: ssl/cli: 'set ssl cert' does not check the transaction name correctly
William Lallemand [Tue, 29 Oct 2024 14:31:00 +0000 (15:31 +0100)] 
BUG/MINOR: ssl/cli: 'set ssl cert' does not check the transaction name correctly

Since commit  089c13850f ("MEDIUM: ssl: ssl-load-extra-del-ext work
only with .crt"), the 'set ssl cert' CLI command does not check
correctly if the transaction you are trying to update is the right one.

The consequence is that you could commit accidentaly a transaction on
the wrong certificate.

The fix introduces the check again in case you are not using
ssl-load-extra-del-ext.

This must be backported in all stable versions.

8 months agoMEDIUM: socket: add zero-terminated ABNS alternative
Tristan [Mon, 28 Oct 2024 15:23:31 +0000 (16:23 +0100)] 
MEDIUM: socket: add zero-terminated ABNS alternative

When an abstract unix socket is bound by HAProxy (using "abns@" prefix),
NUL bytes are appended at the end of its path until sun_path is filled
(for a total of 108 characters).

Here we add an alternative to pass only the non-NUL length of that path
to connect/bind calls, such that the effective path of the socket's name
is as humanly written. This may be useful to interconnect with existing
softwares that implement abstract sockets with this logic instead of the
default haproxy one.

This is achieved by implementing the "abnsz" socket prefix (instead of
"abns"), which stands for "zero-terminated ABNS". "abnsz" prefix may be
used anywhere "abns" is. Internally, haproxy uses the custom socket
family (AF_CUST_ABNS vs AF_CUST_ABNSZ) to differentiate default abns
sockets from zero-terminated ones.

Documentation was updated and regtest was added.

Fixes GH issues #977 and #2479

Co-authored-by: Aurelien DARRAGON <adarragon@haproxy.com>
8 months agoMEDIUM: sock_unix: use per-family addrcmp function
Aurelien DARRAGON [Mon, 28 Oct 2024 14:02:19 +0000 (15:02 +0100)] 
MEDIUM: sock_unix: use per-family addrcmp function

Thanks to previous commit, we may now use dedicated addrcmp functions for
each UNIX address family. This allows to simplify sock_unix_addrcmp()
function and avoid useless checks in order to try to guess the socket
type.

In this patch we implement sock_abns_addrcmp() and sock_abnsz_addrcmp()
functions, which are respectively used for ABNS and ABNSZ custom families

sock_unix_addrcmp() now only holds regular UNIX socket comparing logic.

8 months agoMEDIUM: sock: also restore effective unix family in get_{src,dst}()
Aurelien DARRAGON [Tue, 29 Oct 2024 10:38:11 +0000 (11:38 +0100)] 
MEDIUM: sock: also restore effective unix family in get_{src,dst}()

As in previous commit, let's push the logic a bit further in order to
properly restore the effective UNIX socket type when leveraging
get_src() and get_dst() sock functions, since they rely on getpeername()
and getsockname() under the hood, both of which will actually loose the
effective family and return AF_UNIX for all our custom UNIX sockets.

To do this, add sock_restore_unix_family() helper function from the logic
implemented in the previous commit, and call this function from get_src()
and get_dst() in case of unix socket prior to returning.

8 months agoMINOR: sock: restore effective UNIX family in sock_get_old_sockets()
Aurelien DARRAGON [Mon, 28 Oct 2024 14:17:10 +0000 (15:17 +0100)] 
MINOR: sock: restore effective UNIX family in sock_get_old_sockets()

When getting sockets from older process in sock_get_old_sockets(), we
leverage getsockname() to fill sockaddr struct from known fd.

However, the kernel doesn't know about our custom UNIX families such
as CUST_ABNS and CUST_ABNSZ which are both based on AF_UNIX real family.

Since haproxy socket API relies on effective family (and not real family)
to recognize the socket type instead of having to guess it by analyzing
the path content, let's restore it right after getsockname() since we
have all the infos needed to deduce the right family.

If the path starts with a NULL byte, we know that it is an abstract sock.
Then we simply check <addrlen> value from getsockname() to know if the
addr makes uses of the whole path space (normal ABNS) or partial path
space (zero ABNS / aka ABNZ) terminated by 0.

8 months agoMINOR: protocol: create abnsz socket address family
Willy Tarreau [Fri, 9 Aug 2024 19:12:23 +0000 (21:12 +0200)] 
MINOR: protocol: create abnsz socket address family

For now it's the same as abns. We'll need to modify sock_unix_addrcmp(),
and a few other ones to support effective path length when dealing with
the \0. Let's check with Tristan's patch for this (upcoming patch).

Co-authored-by: Aurelien DARRAGON <adarragon@haproxy.com>
8 months agoCLEANUP: tools: rely on address family to detect ABNS sockets
Aurelien DARRAGON [Thu, 24 Oct 2024 14:29:14 +0000 (16:29 +0200)] 
CLEANUP: tools: rely on address family to detect ABNS sockets

Following previous commit, in str2sa_range(), make use of address' family
which was just set to check if the socket is ABNS or not instead of
relying on an extra boolean to save this info.

8 months agoMEDIUM: protocol: rely on AF_CUST_ABNS family to recognize ABNS sockets
Aurelien DARRAGON [Thu, 24 Oct 2024 12:20:01 +0000 (14:20 +0200)] 
MEDIUM: protocol: rely on AF_CUST_ABNS family to recognize ABNS sockets

Now that we can easily distinguish regular UNIX socket from ABNS sockets
by simply looking at the address family, stop looking at the first byte
from addr->sun_path to guess if the socket is an ABNS one or not. Looking
at the family is straightforward and will allow to differentiate between
upcoming ABNSZ and ABNS (where looking at the first byte from path won't
help anymore).

8 months agoMEDIUM: protocol: make abns a custom unix socket address family
Willy Tarreau [Fri, 9 Aug 2024 16:48:14 +0000 (18:48 +0200)] 
MEDIUM: protocol: make abns a custom unix socket address family

This is a pre-requisite to adding the abnsz socket address family:

in this patch we make use of protocol API rework started by 732913f
("MINOR: protocol: properly assign the sock_domain and sock_family") in
order to implement a dedicated address family for ABNS sockets (based on
UNIX parent family).

Thanks to this, it will become trivial to implement a new ABNSZ (for abns
zero) family which is essentially the same as ABNS but with a slight
difference when it comes to path handling (ABNS uses the whole sun_path
length, while ABNSZ's path is zero terminated and evaluation stops at 0)

It was verified that this patch doesn't break reg-tests and behaves
properly (tests performed on the CLI with show sess and show fd).

Anywhere relevant, AF_CUST_ABNS is handled alongside AF_UNIX. If no
distinction needs to be made, real_family() is used to fetch the proper
real family type to handle it properly.

Both stream and dgram were converted, so no functional change should be
expected for this "internal" rework, except that proto will be displayed
as "abns_{stream,dgram}" instead of "unix_{stream,dgram}".

Before ("show sess" output):
  0x64c35528aab0: proto=unix_stream src=unix:1 fe=GLOBAL be=<NONE> srv=<none> ts=00 epoch=0 age=0s calls=1 rate=0 cpu=0 lat=0 rq[f=848000h,i=0,an=00h,ax=] rp[f=80008000h,i=0,an=00h,ax=] scf=[8,0h,fd=21,rex=10s,wex=] scb=[8,1h,fd=-1,rex=,wex=] exp=10s rc=0 c_exp=

After:
  0x619da7ad74c0: proto=abns_stream src=unix:1 fe=GLOBAL be=<NONE> srv=<none> ts=00 epoch=0 age=0s calls=1 rate=0 cpu=0 lat=0 rq[f=848000h,i=0,an=00h,ax=] rp[f=80008000h,i=0,an=00h,ax=] scf=[8,0h,fd=22,rex=10s,wex=] scb=[8,1h,fd=-1,rex=,wex=] exp=10s rc=0 c_exp=

Co-authored-by: Aurelien DARRAGON <adarragon@haproxy.com>
8 months agoBUG/MINOR: trace: stop rewriting argv with -dt
William Lallemand [Tue, 29 Oct 2024 09:50:27 +0000 (10:50 +0100)] 
BUG/MINOR: trace: stop rewriting argv with -dt

When using trace with -dt, the trace_parse_cmd() function is doing a
strtok which write \0 into the argv string.

When using the mworker mode, and reloading, argv was modified and the
trace won't work anymore because the first : is replaced by a '\0'.

This patch fixes the issue by allocating a temporary string so we don't
modify the source string directly. It also replace strtok by its
reentrant version strtok_r.

Must be backported as far as 2.9.

8 months agoDEV: gdb: add a number of gdb scripts to navigate in core dumps
Willy Tarreau [Mon, 28 Oct 2024 16:51:29 +0000 (17:51 +0100)] 
DEV: gdb: add a number of gdb scripts to navigate in core dumps

These is a collection of functions I'm occasionally using to navigate
in core dumps. Only working ones were extracted.

Those requiring knowledge of global variables (e.g. pools, proxy list)
use the one extracted from the post_mortem struct. That one is defined
in post-mortem.gdb and needs to be initialized using "pm_init post_mortem"
or "pm_init <pointer>". From this point a number of global variables are
accessible even if symbols are missing; those ones are then used by other
functions to dump streams, threads, pools, proxies etc.

The files can be sourced or copy-pasted into a gdb session. It's worth
trying to keep them up-to-date, as the old ones used to navigate through
tasks are no longer usable due to massive changes.

8 months agoMINOR: debug: remove the redundant process.thread_info array from post_mortem
Willy Tarreau [Mon, 28 Oct 2024 06:47:23 +0000 (07:47 +0100)] 
MINOR: debug: remove the redundant process.thread_info array from post_mortem

That one is huge and unneeded since we now have the pointer to the
whole thread_info[] array, which does contain the freshest version
of these info and many more. Let's just get rid of it entirely.

8 months agoMINOR: debug: also add fdtab and acitvity to struct post_mortem
Willy Tarreau [Mon, 28 Oct 2024 06:44:14 +0000 (07:44 +0100)] 
MINOR: debug: also add fdtab and acitvity to struct post_mortem

These ones are often used as well when trying to analyse sequences of
events, let's add them.

8 months agoDOC: design: add notes about more detailed error reporting for logs
Willy Tarreau [Mon, 28 Oct 2024 16:10:19 +0000 (17:10 +0100)] 
DOC: design: add notes about more detailed error reporting for logs

These are the notes of a day long code analysis session (CFA+WTA)
aimed at figuring what's missing during most code troubleshooting
sessions.  The goal is to provide good indications about what rules/
filters were still active when the processing ended (timeout, error
etc), what subscribers are still active (indicating waiting for an
event), and what shut/abort events were met at the various levels
of each side's stack, in each direction.

8 months agoCLEANUP: log: use strnlen2() in _lf_text_len() to compute string length
Aurelien DARRAGON [Fri, 25 Oct 2024 15:06:30 +0000 (17:06 +0200)] 
CLEANUP: log: use strnlen2() in _lf_text_len() to compute string length

Thanks to previous commit, we can now use strnlen2() function to perform
strnlen() portable equivalent instead of re-implementing the logic under
_lf_text_len() function.

8 months agoMINOR: tools: add strnlen2() helper
Aurelien DARRAGON [Fri, 25 Oct 2024 15:04:37 +0000 (17:04 +0200)] 
MINOR: tools: add strnlen2() helper

strnlen2() is functionally equivalent to strnlen(). Goal is to provide
an alternative to strnlen() which is not portable since it requires
_POSIX_C_SOURCE >= 200809L

8 months agoBUG/MINOR: mworker/cli: fix mworker_cli_global_proxy_new_listener
Valentine Krasnobaeva [Thu, 24 Oct 2024 16:39:55 +0000 (18:39 +0200)] 
BUG/MINOR: mworker/cli: fix mworker_cli_global_proxy_new_listener

There is no need to close proc->ipc_fd[0] on the error path in
mworker_cli_global_proxy_new_listener(), as it's already closed before by the
caller.

8 months agoBUG/MEIDUM: mworker: fix fd leak from master to worker
Valentine Krasnobaeva [Sat, 26 Oct 2024 13:01:54 +0000 (15:01 +0200)] 
BUG/MEIDUM: mworker: fix fd leak from master to worker

During re-execution master keeps always opened "reload" sockpair FDs and
shared sockpair ipc_fd[0], the latter is using to transfert listeners sockets
from the previously forked worker to the new one. So, these master's FDs are
inherited in the newly forked worker and must be closed in its context.

"reload" sockpair inherited FDs and shared sockpair FD (ipc_fd[0]) are closed
separately, becase master doesn't recreate "reload" sockpair each time after
its re-exec. It always keeps the same FDs for this "reload" sockpair. So in
worker context it can be closed immediately after the fork.

At contrast, shared sockpair is created each time after reload, when the new
worker will be forked. So, if N previous workers are still exist at this moment,
the new worker will inherit N ipc_fd[0] from master. So, it's more save to
close all these FDs after get_listeners_fd() and bind_listeners() calls.
Otherwise, early closed FDs in the worker context will be immediately bound to
listeners and we could potentially have some bugs.

8 months agoCLEANUP: mworker: make mworker_create_master_cli more readable
Valentine Krasnobaeva [Sat, 26 Oct 2024 13:03:19 +0000 (15:03 +0200)] 
CLEANUP: mworker: make mworker_create_master_cli more readable

Using nested 'if' operator, while checking if we will need to allocate again the
"reload" sockpair, does not degrade performance, as mworker_create_master_cli is
a startup routine.

This nested 'if' (we check one condition in each operator) makes more visible the
fact, that the "reload" sockpair is allocated only once, when the master process
starts and it does not re-allocated again (hence, its FDs are not closed) during
reloads. This way of checking multiple conditions here makes more easy to spot
this fact, while analysing the code in order to investigate FD leaks between
master and worker.

8 months agoMINOR: debug: also add a pointer to struct global to post_mortem
Willy Tarreau [Sat, 26 Oct 2024 09:33:09 +0000 (11:33 +0200)] 
MINOR: debug: also add a pointer to struct global to post_mortem

The pointer to struct global is also an important element to have in
post_mortem given that it's used a lot to take decisions in the code.
Let's just add it. It's worth noting that we could get rid of argc/argv
at this point since they're also present in the global struct, but they
don't cost much there anyway.

8 months agoMINOR: cli: add an 'echo' command
William Lallemand [Thu, 24 Oct 2024 15:20:57 +0000 (17:20 +0200)] 
MINOR: cli: add an 'echo' command

Add an echo command to write text over the CLI output.

8 months agoMINOR: cli: remove non-printable characters from 'debug dev fd'
William Lallemand [Thu, 24 Oct 2024 14:31:56 +0000 (16:31 +0200)] 
MINOR: cli: remove non-printable characters from 'debug dev fd'

When using 'debug dev fd', the output of laddr and raddr can contain
some garbage.

This patch replaces any control or non-printable character by a '.'.

8 months agoMINOR: debug: do not limit backtraces to stuck threads
Willy Tarreau [Thu, 24 Oct 2024 13:14:55 +0000 (15:14 +0200)] 
MINOR: debug: do not limit backtraces to stuck threads

Historically for size limitation reasons, we would only dump the
backtrace of stuck threads. The problem is that when triggering
a panic or other reasons, we have no backtrace, which effectively
limits it to the watchdog timer. It's also visible in "show threads"
which used to report backtraces for all threads in 2.4 and displays
none nowadays, making its use much more limited.

A first approach could be to just dump the thread that triggers the
panic (in addition to stuck threads). But that remains quite limited
since "show threads" would still display nothing. This patch takes a
better approach consisting in dumping all non-idle threads. This way
the output is less polluted that with the older approach (no need to
dump all those waiting in the poller), and all active threads are
visible, in panics as well as in "show threads". As such, the CLI
command "debug dev panic" now dmups backtraces again. This is already
a benefit which will ease testing of various locations against the
ability to resolve useful symbols.

8 months agoMINOR: debug: store important pointers in post_mortem
Willy Tarreau [Thu, 24 Oct 2024 12:37:12 +0000 (14:37 +0200)] 
MINOR: debug: store important pointers in post_mortem

Dealing with a core and a stripped executable is a pain when it comes
to finding pools, proxies or thread contexts. Let's put a pointer to
these heads and arrays in the post_mortem struct for easier location.
Other critical lists like this could possibly benefit from being added
later.

Here we now have:
  - tgroup_info
  - thread_info
  - tgroup_ctx
  - thread_ctx
  - pools
  - proxies

Example:
  $ objdump -h haproxy|grep post
   34 _post_mortem  000014b0  0000000000cfd400  0000000000cfd400  008fc400  2**8

  (gdb) set $pm=(struct post_mortem*)0x0000000000cfd400

  (gdb) p $pm->tgroup_ctx[0]
  $8 = {
    threads_harmless = 254,
    threads_idle = 254,
    stopping_threads = 0,
    timers = {
      b = {0x0, 0x0}
    },
    niced_tasks = 0,
    __pad = 0xf5662c <ha_tgroup_ctx+44> "",
    __end = 0xf56640 <ha_tgroup_ctx+64> ""
  }

  (gdb) info thr
    Id   Target Id                         Frame
  * 1    Thread 0x7f9e7706a440 (LWP 21169) 0x00007f9e76a9c868 in raise () from /lib64/libc.so.6
    2    Thread 0x7f9e76a60640 (LWP 21175) 0x00007f9e76b343c7 in wait4 () from /lib64/libc.so.6
    3    Thread 0x7f9e7613d640 (LWP 21176) 0x00007f9e76b343c7 in wait4 () from /lib64/libc.so.6
    4    Thread 0x7f9e7493a640 (LWP 21179) 0x00007f9e76b343c7 in wait4 () from /lib64/libc.so.6
    5    Thread 0x7f9e7593c640 (LWP 21177) 0x00007f9e76b343c7 in wait4 () from /lib64/libc.so.6
    6    Thread 0x7f9e7513b640 (LWP 21178) 0x00007f9e76b343c7 in wait4 () from /lib64/libc.so.6
    7    Thread 0x7f9e6ffff640 (LWP 21180) 0x00007f9e76b343c7 in wait4 () from /lib64/libc.so.6
    8    Thread 0x7f9e6f7fe640 (LWP 21181) 0x00007f9e76b343c7 in wait4 () from /lib64/libc.so.6
  (gdb) p/x $pm->thread_info[0].pth_id
  $12 = 0x7f9e7706a440
  (gdb) p/x $pm->thread_info[1].pth_id
  $13 = 0x7f9e76a60640

  (gdb) set $px = *$pm->proxies
  while ($px != 0)
     printf "%#lx %s served=%u\n", $px, $px->id, $px->served
     set $px = ($px)->next
  end

  0x125eda0 GLOBAL served=0
  0x12645b0 stats served=0
  0x1266940 comp served=0
  0x1268e10 comp_bck served=0
  0x1260cf0 <OCSP-UPDATE> served=0
  0x12714c0 <HTTPCLIENT> served=0

8 months agoMINOR: debug: place the post_mortem struct in its own section.
Willy Tarreau [Thu, 24 Oct 2024 09:59:32 +0000 (11:59 +0200)] 
MINOR: debug: place the post_mortem struct in its own section.

Placing it in its own section will ease its finding, particularly in
gdb which is too dumb to find anything in memory. Now it will be
sufficient to issue this:

  $ gdb -ex "info files" -ex "quit" ./haproxy core 2>/dev/null |grep _post_mortem
  0x0000000000cfd300 - 0x0000000000cfe780 is _post_mortem

or this:

   $ objdump -h haproxy|grep post
    34 _post_mortem  00001480  0000000000cfd300  0000000000cfd300  008fc300  2**8

to spot the symbol's address. Then it can be read this way:

   (gdb) p *(struct post_mortem *)0x0000000000cfd300

8 months agoMINOR: debug: place a magic pattern at the beginning of post_mortem
Willy Tarreau [Thu, 24 Oct 2024 09:56:07 +0000 (11:56 +0200)] 
MINOR: debug: place a magic pattern at the beginning of post_mortem

In order to ease finding of the post_mortem struct in core dumps, let's
make it start with a recognizable pattern of exactly 32 chars (to
preserve alignment):

  "POST-MORTEM STARTS HERE+7654321\0"

It can then be found like this from gdb:

  (gdb) find 0x000000012345678, 0x0000000100000000, 'P','O','S','T','-','M','O','R','T','E','M'
  0xcfd300 <post_mortem>
  1 pattern found.

Or easier with any other more practical tool (who as ever used "find" in
gdb, given that it cannot iterate over maps and is 100% useless?).

8 months agoMINOR: pools: export the pools variable
Willy Tarreau [Thu, 24 Oct 2024 12:36:30 +0000 (14:36 +0200)] 
MINOR: pools: export the pools variable

We want it to be accessible from debuggers for inspection and it's
currently unavailable. Let's start by exporting it as a first step.

8 months agoCLEANUP: mux-h2: remove the unused "full" variable in h2_frt_transfer_data()
Willy Tarreau [Thu, 24 Oct 2024 12:11:06 +0000 (14:11 +0200)] 
CLEANUP: mux-h2: remove the unused "full" variable in h2_frt_transfer_data()

During 11th and 12th iteration of the development cycle for the H2 auto
rx window, several approaches were attempted to figure if another buffer
could be allocated or not. One of them consisted in looping back to the
beginning of the function requesting a new buffer slot and getting one
if the buffer was either apparently or confirmed full. The latest one
consisted in directly allocating the next buffer from the two places
where it's found to be proven full, instead of checking with the now
defunct h2s_may_get_rxbuf() if we were allowed to get once an loop.
That approach was retained. In this case the "full" variabled is no
longer needed, so let's get rid of it because the construct looks bogus
and confuses coverity (and possibly code readers as the intent is unclear
compared to the code).

8 months agoBUILD: debug: silence a build warning with threads disabled
Willy Tarreau [Thu, 24 Oct 2024 13:04:25 +0000 (15:04 +0200)] 
BUILD: debug: silence a build warning with threads disabled

Commit 091de0f9b2 ("MINOR: debug: slightly change the thread_dump_pointer
signification") caused the following warning to be emitted when threads
are disabled:

  src/debug.c: In function 'ha_thread_dump_one':
  src/debug.c:359:9: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]

Let's just disguise the pointer to silence it. It should be backported
where the patch above was backported, since it was part of a series aiming
at making thread dumps more exploitable from core dumps.

8 months agoMINOR: mworker/cli: 'show proc debug' for old workers
William Lallemand [Thu, 24 Oct 2024 12:47:28 +0000 (14:47 +0200)] 
MINOR: mworker/cli: 'show proc debug' for old workers

Add FD details for old workers in 'show proc debug'.