]> git.ipfire.org Git - thirdparty/haproxy.git/log
thirdparty/haproxy.git
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'.

8 months agoMINOR: mworker/cli: remove comment line for program when useless
William Lallemand [Thu, 24 Oct 2024 12:39:41 +0000 (14:39 +0200)] 
MINOR: mworker/cli: remove comment line for program when useless

Remove the '# programs' line on 'show proc' output when there are no
program.

8 months agoMINOR: mworker/cli: add 'debug' to 'show proc'
William Lallemand [Thu, 24 Oct 2024 12:18:30 +0000 (14:18 +0200)] 
MINOR: mworker/cli: add 'debug' to 'show proc'

This patch adds a 'debug' parameter to the 'show proc' command of the
master CLI. It allows to show debug details about the processes.

Example:

echo 'show proc debug' | socat /tmp/master.sock -
\#<PID>          <type>          <reloads>       <uptime>        <version>       <ipc_fd[0]>     <ipc_fd[1]>
391999          master          0 [failed: 0]   0d00h00m02s     3.1-dev10-b9095a-63 5               6
\# workers
392001          worker          0               0d00h00m02s     3.1-dev10-b9095a-63 3               -1
\# programs

8 months agoBUG/MINOR: stconn: Don't disable 0-copy FF if EOS was reported on consumer side
Christopher Faulet [Thu, 24 Oct 2024 09:53:10 +0000 (11:53 +0200)] 
BUG/MINOR: stconn: Don't disable 0-copy FF if EOS was reported on consumer side

There is no reason to disable the 0-copy data forwarding if an end-of-stream
was reported on the consumer side. Indeed, the consumer will send data in
this case. So there is no reason to check the read side here.

This patch may be backported as far as 2.9.

8 months agoBUG/MINOR: http-ana: Fix wrong client abort reports during responses forwarding
Christopher Faulet [Thu, 24 Oct 2024 09:58:46 +0000 (11:58 +0200)] 
BUG/MINOR: http-ana: Fix wrong client abort reports during responses forwarding

When the response forwarding is aborted, we must not report a client abort
if a EOS was seen on client side. On abort performed by the stream must be
considered.

This bug was introduced when the SHUTR was splitted in 2 flags.

This patch must be backported as far as 2.8.

8 months agoBUG/MEDIUM: stconn: Report blocked send if sends are blocked by an error
Christopher Faulet [Thu, 24 Oct 2024 09:35:21 +0000 (11:35 +0200)] 
BUG/MEDIUM: stconn: Report blocked send if sends are blocked by an error

When some data must be sent to the endpoint but an error was previously
reported, nothing is performed and we leave. But, in this case, the SC is not
notified the sends are blocked.

It is indeed an issue if the endpoint reports an error after consuming all
data from the SC. In the endpoint the outgoing data are trashed because of
the error, but on the SC, everything was sent, even if an error was also
reported.

Because of this bug, it is possible to have outgoing data blocked at the SC
level but without any write timeout armed. In some cases, this may lead to
blocking conditions where the stream is never closed.

So now, when outgoing data cannot be sent because an previous error was
triggered, a blocked send is reported. This way, it is possible to report a
write timeout.

This patch should fix the issue #2754. It must be backported as far as 2.8.

8 months agoBUG/MEDIUM: server: fix race on servers_list during server deletion
Amaury Denoyelle [Wed, 23 Oct 2024 16:18:48 +0000 (18:18 +0200)] 
BUG/MEDIUM: server: fix race on servers_list during server deletion

Each server is inserted in a global list named servers_list on
new_server(). This list is then only used to finalize servers
initialization after parsing.

On dynamic server creation, there is no issue as new_server() is under
thread isolation. However, when a server is deleted after its refcount
reached zero, srv_drop() removes it from servers_list without lock
protection. In the longterm, this can cause list corruption and crashes,
especially if multiple adjacent servers are removed in parallel.

To fix this, convert servers_list to a mt_list. This should not impact
performance as servers_list is not used during runtime outside of server
creation/deletion.

This should fix github issue #2733. Thanks to Chris Staite who first
found the issue here.

This must be backported up to 2.6.

8 months agoBUG/MINOR: server: fix dynamic server leak with check on failed init
Amaury Denoyelle [Tue, 22 Oct 2024 09:02:15 +0000 (11:02 +0200)] 
BUG/MINOR: server: fix dynamic server leak with check on failed init

If a dynamic server is added with check or agent-check, its refcount is
incremented after server keyword parsing. However, if add server fails
at a later stage, refcount is only decremented once, which prevented the
server to be fully released.

This causes a leak with a server which is detached from most of the
lists but still exits in the system.

This bug is considered minor as only a few conditions may cause a
failure in add server after check/agent-check initialization. This is
the case if there is a naming collision or the dynamic ID cannot be
generated.

To fix this, simply decrement server refcount on add server error path
if either check and/or agent-check are flagged as activated.

This bug is related to github issue #2733. Thanks to Chris Staite who
first found the leak.

This must be backported up to 2.6.

8 months agoMINOR: mworker/cli: split mworker_cli_proxy_create
Valentine Krasnobaeva [Wed, 23 Oct 2024 15:29:10 +0000 (17:29 +0200)] 
MINOR: mworker/cli: split mworker_cli_proxy_create

There are two parts in mworker_cli_proxy_create(): allocating and setting up
MASTER proxy and allocating and setting up servers on ipc_fd[0] of the
sockpairs shared with workers.

So, let's split mworker_cli_proxy_create() into two functions respectively.
Each of them takes **errmsg as an argument to write an error message, which may
be triggered by some subcalls. The content of this errmsg will allow to extend
the final alert message shown to user, if these new functions will fail.

The main goals of this split is to allow to move these two parts independantly
in future and makes the code of haproxy initialization in haproxy.c more
transparent.

8 months agoCLEANUP: mworker: clean mworker_reexec
Valentine Krasnobaeva [Wed, 23 Oct 2024 13:46:54 +0000 (15:46 +0200)] 
CLEANUP: mworker: clean mworker_reexec

Before refactoring master-worker architecture, resources to setup master CLI
for the new worker process (shared sockpair, entry in proc_list) were created
in init() before parsing the configuration and binding listening sockets. So,
master during its re-exec has had to cleanup the new worker's ressources in
a case, when it fails at some initialization step before the fork.

Now fork happens very early and worker parses its configuration by itself. If
it fails during the initialization stage, all clean ups (deleting the fds of
the shared sockpair, proc_list cleanup) are performed in SIGCHLD handler up to
catching the SIGCHLD corresponded to this new worker. So, there is no longer
need to call mworker_cleanup_proc() in mworker_reexec().

As for mworker_cleanlisteners(), there is no longer need to call this function.
Master parses now only "global" and "program" sections, so it allocates only
MASTER proxy, which is stopped in mworker_reexec() by mworker_cli_proxy_stop().

Let's keep the definitions of mworker_cleanlisteners() and
mworker_cleanup_proc() in mworker.c for the moment. We may reuse parts of its
code later.

8 months agoBUG/MINOR: mworker: show worker warnings in startup logs
Valentine Krasnobaeva [Mon, 21 Oct 2024 14:27:07 +0000 (16:27 +0200)] 
BUG/MINOR: mworker: show worker warnings in startup logs

As master-worker fork happens now at early init stage and worker then parses
its configuration and performs all initialization steps, let's duplicate
startup logs ring for it, just before the moment when it enters in its pollong
loop. Startup logs ring content is shown as an output of the "reload" master
CLI command and we should be able to dump here worker initialization logs.

Log messages are written in startup logs ring only, when mode MODE_STARTING is
set (see print_message()). So, to be able to keep in startup logs the last
worker alerts, let's withdraw MODE_STARTING and let's reset user messages
context respectively just before entering in polling loop.

This fix does not need to be backported as it is a part of previous patches
from this version, which refactor master-worker architecture.

8 months agoMINOR: error: simplify startup_logs_init_shm
Valentine Krasnobaeva [Mon, 21 Oct 2024 15:10:18 +0000 (17:10 +0200)] 
MINOR: error: simplify startup_logs_init_shm

This patch simplifies the code of startup_logs_init_shm(). We no longer re-exec
master process twice after each reload to free its unused memory, which it had
to allocate, because it has parsed all configuration sections. So, there is no
longer need to keep SHM fd opened between the first and the next reloads. We
can completely remove HAPROXY_STARTUPLOGS_FD.

In step_init_1() we continue to call startup_logs_init_shm() to open SHM and to
allocate startup logs ring area within it. In master-worker mode, worker
duplicates initial startup logs ring after sending its READY state to master.
Sharing the same ring between two processes until the worker finishes its
initialization allows to show at master CLI output worker's startup logs.

During the next reload master process should free the memory allocated for the
ring structure. Then after the execvp() it will reopen and map SHM area again
and it will reallocate again the ring structure.

8 months agoMINOR: mworker: stop MASTER proxy listener on worker mcli sockpair
Valentine Krasnobaeva [Tue, 22 Oct 2024 13:09:29 +0000 (15:09 +0200)] 
MINOR: mworker: stop MASTER proxy listener on worker mcli sockpair

After sending its "READY" status worker should not keep the access
to MASTER proxy, thus, it shouldn't be able to send any other commands further
to master process.

To achieve this, let's stop in master context master CLI listener attached on
the sockpair shared with worker. We do this just after receiving the worker's
status message.

8 months agoBUG/MINOR: mworker/cli: show master startup logs in recovery mode
Valentine Krasnobaeva [Fri, 18 Oct 2024 18:25:34 +0000 (20:25 +0200)] 
BUG/MINOR: mworker/cli: show master startup logs in recovery mode

When master enters in recovery mode after unsuccessfull reload
HAPROXY_LOAD_SUCCESS should be set as 0. Like this
cli_io_handler_show_cli_sock() could dump in master CLI its warnings and alerts,
saved in startup logs ring.

No need to backport this fix, as this is related to the previous patches in
this version to refactor master-worker architecture.

8 months agoMINOR: activity/memprofile: show per-DSO stats
Willy Tarreau [Thu, 24 Oct 2024 08:46:06 +0000 (10:46 +0200)] 
MINOR: activity/memprofile: show per-DSO stats

On systems where many libs are loaded, it's hard to track suspected
leaks. Having a per-DSO summary makes it more convenient. That's what
we're doing here by summarizing all calls per DSO before showing the
total.

8 months agoBUG/MINOR: mux-h1: Fix conditions on pipe in some COUNT_IF()
Christopher Faulet [Thu, 24 Oct 2024 07:50:15 +0000 (09:50 +0200)] 
BUG/MINOR: mux-h1: Fix conditions on pipe in some COUNT_IF()

The previous commit contains a bug in some COUNT_IF() relying on the pipe
inside the IOBUF. We must take care to have a pipe before checking its size.

No backport needed.

8 months agoDEBUG: mux-h1: Add debug counters to track errors with in/out pending data
Christopher Faulet [Wed, 23 Oct 2024 20:51:18 +0000 (22:51 +0200)] 
DEBUG: mux-h1: Add debug counters to track errors with in/out pending data

Debug counters were added on all connection error when pending data remain
blocked in the input or ouput buffers. The same is performed when the H1C is
released, when the connection is closed and when a timeout is reached. Idea
is to be able to count all cases where data are lost, especially the
outgoing ones.

8 months agoRevert "OPTIM: mux-h2: make h2_send() report more accurate wake up conditions"
Willy Tarreau [Wed, 23 Oct 2024 17:08:55 +0000 (19:08 +0200)] 
Revert "OPTIM: mux-h2: make h2_send() report more accurate wake up conditions"

This reverts commit 9fbc01710a313968c90e72537a5906432f438062.

In 3.1-dev10, commit 9fbc01710a ("OPTIM: mux-h2: make h2_send() report
more accurate wake up conditions") leveraged the more accurate distinction
between demux and recv to decide when to wake the tasklet up after a send.
But other cases are needed. When we just need to wake the processing task
up so that it itself wakes up other streams, for example because these ones
are blocked. Indeed, a temporarily blocked stream may block other ones,
which will never be woken up if the demux has nothing to do.

In an ideal world we would check all cases where blocking flags were
dropped. However it looks like this case after a send is probably the
only one that deserves waking up the connection again. It's likely that
in practice the MUX_MFULL flag was dropped and that it was that one that
was blocking the send.

In addition, dealing with these cases was not sufficient, as one case was
encountered where dbuf was empty, subs=0, short_read still present while
in FRH state... and the timeouts were still there (easily found with
halog -tcn cD at a rate of 1-2 every 2 minutes roughly).

Interestingly, in a dump, some MBUF_HAS_DATA were seen on an empty mbuf,
so it means that certain conditions must be taken very carefully in the
wakeup conditions.

So overall this indicates that there remain subtle inconsistencies that
this optimization is sensitive to. It may have to be revisited later but
for now better revert it.

No backport is needed.

Annex:
  - first dump showing a dependency on WAIT_INLIST after h2_send():

    0x6dc2800: [23/Oct/2024:18:07:22.861247] id=1696 proto=tcpv4
      flags=0x100c4a, conn_retries=0, conn_exp=<NEVER> conn_et=0x000 srv_conn=0x597a900, pend_pos=(nil) waiting=0 epoch=0
      frontend=public (id=2 mode=http), listener=SSL (id=5)
      backend=gitweb-haproxy (id=6 mode=http)
      task=0x6e1d090 (state=0x00 nice=0 calls=23 rate=0 exp=2s tid=0(1/0) age=57s)
      txn=0x6e3f7c0 flags=0x43000 meth=1 status=200 req.st=MSG_DONE rsp.st=MSG_DATA req.f=0x4c rsp.f=0x2e
      scf=0x6dc33a0 flags=0x00002482 ioto=1m state=EST endp=CONN,0x6dc6c20,0x40405001 sub=3 rex=<NEVER> wex=3s rto=3s wto=3s
        iobuf.flags=0x00000000 .pipe=0 .buf=0@(nil)+0/0
          h2s=0x6dc6c20 h2s.id=59 .st=HCR .flg=0x7001 .rxwin=32712 .rxbuf.c=0 .t=0@(nil)+0/0 .h=0@(nil)+0/0
           .sc=0x6dc33a0(.flg=0x00002482 .app=0x6dc2800) .sd=0x6e83fd0(.flg=0x40405001)
           .subs=0x6dc33b8(ev=3 tl=0x6e22a20 tl.calls=10 tl.ctx=0x6dc33a0 tl.fct=sc_conn_io_cb)
           h2c=0x6e66570 h2c.st0=FRH .err=0 .maxid=77 .lastid=-1 .flg=0x2000e00 .nbst=2 .nbsc=2 .nbrcv=0 .glitches=0
           .fctl_cnt=0 .send_cnt=2 .tree_cnt=2 .orph_cnt=0 .sub=1 .dsi=77 .dbuf=0@(nil)+0/0
           .mbuf=[4..4|32],h=[0@(nil)+0/0],t=[0@(nil)+0/0] .task=0x6dbdc60 .exp=<NEVER>
          co0=0x7f84881614b0 ctrl=tcpv4 xprt=SSL mux=H2 data=STRM target=LISTENER:0x2acb7c0
          flags=0x80000300 fd=19 fd.state=121 updt=0 fd.tmask=0x1
      scb=0x2a8da90 flags=0x00001211 ioto=1m state=EST endp=CONN,0x6e5a530,0x106c0001 sub=0 rex=<NEVER> wex=<NEVER> rto=3s wto=<NEVER>
        iobuf.flags=0x00000000 .pipe=0 .buf=0@(nil)+0/0
          h1s=0x6e5a530 h1s.flg=0x14094 .sd.flg=0x106c0001 .req.state=MSG_DONE .res.state=MSG_DATA
           .meth=GET status=200 .sd.flg=0x106c0001 .sc.flg=0x00001211 .sc.app=0x6dc2800 .subs=(nil)
           h1c=0x7f84880f5f40 h1c.flg=0x80000020 .sub=0 .ibuf=32704@0x6ddef30+16262/32768 .obuf=0@(nil)+0/0 .task=0x6e131d0 .exp=<NEVER>
          co1=0x7f8488172b70 ctrl=tcpv4 xprt=RAW mux=H1 data=STRM target=SERVER:0x597a900
          flags=0x00000300 fd=31 fd.state=10122 updt=0 fd.tmask=0x1
      filters={0x6e49f30="cache store filter", 0x6e67ad0="compression filter"}
      req=0x6dc2828 (f=0x21840000 an=0x48000 tofwd=0 total=224)
          an_exp=<NEVER> buf=0x6dc2830 data=(nil) o=0 p=0 i=0 size=0
          htx=0x104d2c0 flags=0x0 size=0 data=0 used=0 wrap=NO extra=0
      res=0x6dc2870 (f=0xa0040000 an=0x24000000 tofwd=0 total=309982)
          an_exp=<NEVER> buf=0x6dc2878 data=0x6dceef0 o=16333 p=16333 i=16435 size=32768
          htx=0x6dceef0 flags=0x0 size=32720 data=16333 used=1 wrap=NO extra=0
      -----------------------------------
      strm.flg       0x100c4a  SF_SRV_REUSED SF_HTX SF_REDIRECTABLE SF_CURR_SESS SF_BE_ASSIGNED SF_ASSIGNED
      task.state            0  0
      txn.meth              1  GET
      txn.flg         0x43000  TX_NOT_FIRST TX_CACHE_COOK TX_CACHEABLE
      txn.req.flg        0x4c  HTTP_MSGF_BODYLESS HTTP_MSGF_VER_11 HTTP_MSGF_XFER_LEN
      txn.rsp.flg        0x2e  HTTP_MSGF_COMPRESSING HTTP_MSGF_VER_11 HTTP_MSGF_XFER_LEN HTTP_MSGF_TE_CHNK
      f.sc.flg         0x2482  SC_FL_SND_EXP_MORE SC_FL_RCV_ONCE SC_FL_WONT_READ SC_FL_EOI
      f.sc.sd.flg  0x40405001  SE_FL_HAVE_NO_DATA SE_FL_MAY_FASTFWD_CONS SE_FL_EOI SE_FL_NOT_FIRST SE_FL_T_MUX
      f.h2s.flg        0x7001  H2_SF_HEADERS_RCVD H2_SF_OUTGOING_DATA H2_SF_HEADERS_SENT H2_SF_ES_RCVD
      f.h2s.sd.flg 0x40405001  SE_FL_HAVE_NO_DATA SE_FL_MAY_FASTFWD_CONS SE_FL_EOI SE_FL_NOT_FIRST SE_FL_T_MUX
      f.h2c.flg     0x2000e00  H2_CF_MBUF_HAS_DATA H2_CF_DEM_IN_PROGRESS H2_CF_DEM_SHORT_READ H2_CF_WAIT_INLIST
      f.co.flg     0x80000300  CO_FL_XPRT_TRACKED CO_FL_XPRT_READY CO_FL_CTRL_READY
      f.co.fd.st        0x121  FD_POLL_IN FD_EV_READY_W FD_EV_ACTIVE_R
      b.sc.flg         0x1211  SC_FL_SND_NEVERWAIT SC_FL_NEED_ROOM SC_FL_NOHALF SC_FL_ISBACK
      b.sc.sd.flg  0x106c0001  SE_FL_WAIT_DATA SE_FL_MAY_FASTFWD_CONS SE_FL_MAY_FASTFWD_PROD SE_FL_WANT_ROOM SE_FL_RCV_MORE SE_FL_T_MUX
      b.h1s.sd.flg 0x106c0001  SE_FL_WAIT_DATA SE_FL_MAY_FASTFWD_CONS SE_FL_MAY_FASTFWD_PROD SE_FL_WANT_ROOM SE_FL_RCV_MORE SE_FL_T_MUX
      b.h1s.flg       0x14094  H1S_F_HAVE_CLEN H1S_F_HAVE_O_CONN H1S_F_NOT_FIRST H1S_F_WANT_KAL H1S_F_RX_CONGESTED
      b.h1c.flg    0x80000020  H1C_F_IS_BACK H1C_F_IN_FULL
      b.co.flg          0x300  CO_FL_XPRT_READY CO_FL_CTRL_READY
      b.co.fd.st       0x278a  FD_POLL_OUT FD_POLL_PRI FD_POLL_IN FD_EV_ERR_RW FD_EV_READY_R 0x2008
      req.flg      0x21840000  CF_FLT_ANALYZE CF_DONT_READ CF_AUTO_CONNECT CF_WROTE_DATA
      req.ana         0x48000  AN_REQ_FLT_END AN_REQ_HTTP_XFER_BODY
      req.htx.flg           0  0
      res.flg      0xa0040000  CF_ISRESP CF_FLT_ANALYZE CF_WROTE_DATA
      res.ana      0x24000000  AN_RES_FLT_END AN_RES_HTTP_XFER_BODY
      res.htx.flg           0  0
      -----------------------------------

  - second example of stuck connection after properly checking for WAIT_INLIST
    as well:

    0x73438d0: [23/Oct/2024:18:46:57.235709] id=3963 proto=tcpv4
      flags=0x100c4a, conn_retries=0, conn_exp=<NEVER> conn_et=0x000 srv_conn=0x5dd3f50, pend_pos=(nil) waiting=0 epoch=0x13
      p_stc=25 p_req=29 p_res=29 p_prp=29
      frontend=public (id=2 mode=http), listener=SSL (id=5)
      backend=gitweb-haproxy (id=6 mode=http)
      task=0x72a13e0 (state=0x00 nice=0 calls=24 rate=0 exp=7s tid=0(1/0) age=53s)
      txn=0x7287260 flags=0x43000 meth=1 status=200 req.st=MSG_DONE rsp.st=MSG_DATA req.f=0x4c rsp.f=0x2e
      scf=0x729e520 flags=0x00042082 ioto=1m state=EST endp=CONN,0x737ffd0,0x4040d001 sub=2 rex=<NEVER> wex=46s rto=46s wto=46s
        iobuf.flags=0x00000000 .pipe=0 .buf=0@(nil)+0/0
          h2s=0x737ffd0 h2s.id=57 .st=HCR .flg=0x7001 .rxwin=32712 .rxbuf.c=0 .t=0@(nil)+0/0 .h=0@(nil)+0/0
           .sc=0x729e520(.flg=0x00042082 .app=0x73438d0) .sd=0x72afd50(.flg=0x4040d001)
           .subs=0x729e538(ev=2 tl=0x72af760 tl.calls=10 tl.ctx=0x729e520 tl.fct=sc_conn_io_cb)
           h2c=0x72555a0 h2c.st0=FRH .err=0 .maxid=77 .lastid=-1 .flg=0x60e00 .nbst=1 .nbsc=1 .nbrcv=0 .glitches=0
           .fctl_cnt=0 .send_cnt=1 .tree_cnt=1 .orph_cnt=0 .sub=0 .dsi=77 .dbuf=0@(nil)+0/0
           .mbuf=[2..2|32],h=[0@(nil)+0/0],t=[0@(nil)+0/0] .task=0x725e660 .exp=<NEVER>
          co0=0x7378e00 ctrl=tcpv4 xprt=SSL mux=H2 data=STRM target=LISTENER:0x2f24800
          flags=0x80040300 fd=23 fd.state=1122 updt=0 fd.tmask=0x1
      scb=0x2ee74c0 flags=0x00001211 ioto=1m state=EST endp=CONN,0x7287190,0x106c0001 sub=0 rex=<NEVER> wex=<NEVER> rto=46s wto=<NEVER>
        iobuf.flags=0x00000000 .pipe=0 .buf=0@(nil)+0/0
          h1s=0x7287190 h1s.flg=0x14094 .sd.flg=0x106c0001 .req.state=MSG_DONE .res.state=MSG_DATA
           .meth=GET status=200 .sd.flg=0x106c0001 .sc.flg=0x00001211 .sc.app=0x73438d0 .subs=(nil)
           h1c=0x7373920 h1c.flg=0x80000020 .sub=0 .ibuf=32704@0x7272700+318/32768 .obuf=0@(nil)+0/0 .task=0x729e700 .exp=<NEVER>
          co1=0x72f5290 ctrl=tcpv4 xprt=RAW mux=H1 data=STRM target=SERVER:0x5dd3f50
          flags=0x00000300 fd=19 fd.state=10122 updt=0 fd.tmask=0x1
      filters={0x728f1f0="cache store filter" [3], 0x728fea0="compression filter" [28]}
      req=0x73438f8 (f=0x21840000 an=0x48000 tofwd=0 total=224)
          an_exp=<NEVER> buf=0x7343900 data=(nil) o=0 p=0 i=0 size=0
          htx=0x105f440 flags=0x0 size=0 data=0 used=0 wrap=NO extra=0
      res=0x7343940 (f=0xa0040000 an=0x24000000 tofwd=0 total=359574)
          an_exp=<NEVER> buf=0x7343948 data=0x72b1b30 o=16333 p=16333 i=16435 size=32768
          htx=0x72b1b30 flags=0x8 size=32720 data=16333 used=1 wrap=NO extra=0
      -----------------------------------
      strm.flg       0x100c4a  SF_SRV_REUSED SF_HTX SF_REDIRECTABLE SF_CURR_SESS SF_BE_ASSIGNED SF_ASSIGNED
      task.state            0  0
      txn.meth              1  GET
      txn.flg         0x43000  TX_NOT_FIRST TX_CACHE_COOK TX_CACHEABLE
      txn.req.flg        0x4c  HTTP_MSGF_BODYLESS HTTP_MSGF_VER_11 HTTP_MSGF_XFER_LEN
      txn.rsp.flg        0x2e  HTTP_MSGF_COMPRESSING HTTP_MSGF_VER_11 HTTP_MSGF_XFER_LEN HTTP_MSGF_TE_CHNK
      f.sc.flg        0x42082  SC_FL_EOS SC_FL_SND_EXP_MORE SC_FL_WONT_READ SC_FL_EOI
      f.sc.sd.flg  0x4040d001  SE_FL_HAVE_NO_DATA SE_FL_MAY_FASTFWD_CONS SE_FL_EOS SE_FL_EOI SE_FL_NOT_FIRST SE_FL_T_MUX
      f.h2s.flg        0x7001  H2_SF_HEADERS_RCVD H2_SF_OUTGOING_DATA H2_SF_HEADERS_SENT H2_SF_ES_RCVD
      f.h2s.sd.flg 0x4040d001  SE_FL_HAVE_NO_DATA SE_FL_MAY_FASTFWD_CONS SE_FL_EOS SE_FL_EOI SE_FL_NOT_FIRST SE_FL_T_MUX
      f.h2c.flg       0x60e00  H2_CF_END_REACHED H2_CF_RCVD_SHUT H2_CF_MBUF_HAS_DATA H2_CF_DEM_IN_PROGRESS H2_CF_DEM_SHORT_READ
      f.co.flg     0x80040300  CO_FL_XPRT_TRACKED CO_FL_SOCK_RD_SH CO_FL_XPRT_READY CO_FL_CTRL_READY
      f.co.fd.st       0x1122  FD_POLL_HUP FD_POLL_IN FD_EV_READY_W FD_EV_READY_R
      b.sc.flg         0x1211  SC_FL_SND_NEVERWAIT SC_FL_NEED_ROOM SC_FL_NOHALF SC_FL_ISBACK
      b.sc.sd.flg  0x106c0001  SE_FL_WAIT_DATA SE_FL_MAY_FASTFWD_CONS SE_FL_MAY_FASTFWD_PROD SE_FL_WANT_ROOM SE_FL_RCV_MORE SE_FL_T_MUX