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.
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.
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.
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.
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.
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():
Willy Tarreau [Wed, 23 Oct 2024 13:10:45 +0000 (15:10 +0200)]
BUILD: spoe: fix build warning on older gcc around sub-struct initialization
gcc-4.8 is unhappy with the cfg_file initialization:
src/flt_spoe.c: In function 'parse_spoe_flt':
src/flt_spoe.c:2202:9: warning: missing braces around initializer [-Wmissing-braces]
struct cfgfile cfg_file = {0};
^
src/flt_spoe.c:2202:9: warning: (near initialization for 'cfg_file.list') [-Wmissing-braces]
This is due to the embedded list member. Initializing it to empty like
we do almost everywhere else makes it happy. No backport is needed as
this was changed in 3.1-dev5 only.
BUG/MEDIUM: connection/http-reuse: fix address collision on unhandled address families
As described in GH #2765, there were situations where http connections
would be re-used for requests to different endpoints, which is obviously
unexpected. In GH #2765, this occured with httpclient and UNIX socket
combination, but later code analysis revealed that while disabling http
reuse on httpclient proxy helped, it didn't fix the underlying issue since
it was found that conn_calculate_hash_sockaddr() didn't take into account
families such as AF_UNIX or AF_CUST_SOCKPAIR, and because of that the
sock_addr part of the connection wasn't hashed.
To properly fix the issue, let's explicly handle UNIX (both regular and
ABNS) and AF_CUST_SOCKPAIR families, so that the destination address is
properly hashed. To prevent this bug from re-appearing: when the family
isn't known, instead of doing nothing like before, let's fall back to a
generic (unoptimal) hashing which hashes the whole sockaddr_storage struct
As a workaround, http-reuse may be disabled on impacted proxies.
(unfortunately this doesn't help for httpclient since reuse policy
defaults to safe and cannot be modified from the config)
It should be backported to all stable versions.
Shout out to @christopherhibbert for having reported the issue and
provided a trivial reproducer.
[ada: prior to 3.0, ctx adjt is required because conn_hash_update()'s
prototype is slightly different]
Willy Tarreau [Tue, 22 Oct 2024 17:43:18 +0000 (19:43 +0200)]
MINOR: sample: add the "when" converter to condition some expressions
Sometimes it would be desirable to include some debugging output only
under certain conditions, but the end of the transfer is too late to
apply some rules.
Here we take the approach of making a converter ("when") that takes a
condition among an arbitrary list, and decides whether or not to let
the input sample pass through or not based on the condition. This
allows for example to log debugging information only when an error
was encountered during the processing (sort of an extension of
dontlog-normal). The conditions are quite limited (stopping, error,
normal, toapplet, forwarded, processed) and can be negated. The
converter can also be chained to use more complex conditions.
A suggested example will be:
# log "dbg={-}" when fine, or "dbg={... debug info ...}" on error:
log-format "$HAPROXY_HTTP_LOG_FMT dbg={%[bs.debug_str,when(!normal)]}"
Willy Tarreau [Tue, 22 Oct 2024 14:57:03 +0000 (16:57 +0200)]
MINOR: filters: add per-filter call counters
The idea here is to record how many times a filter is being called on a
stream. We're incrementing the same counter all along, regardless of the
type of event, since the purpose is essentially to detect one that might
be misbehaving. The number of calls is reported in "show sess all" next
to the filter name. It may also help detect suboptimal processing. For
example compressing 1GB shows 138k calls to the compression filter, which
is roughly two calls per buffer. Maybe we wake up with incomplete buffers
and compress less. That's left for a future analysis.
Willy Tarreau [Tue, 22 Oct 2024 14:35:04 +0000 (16:35 +0200)]
MINOR: stream: maintain per-stream counters of the number of passes on code
Process_stream() is a complex function and a few times some lopos were
either witnessed or suspected. Each time this happens it's extremely
difficult to figure why because it involves combinations of analysers,
filters, errors etc.
Let's at least maintain a set of 4 counters per stream that report the
number of times we've been through each of the 4 most important blocks
(stconn changes, request analysers, response analysers, and propagation
of changes down). These ones are stored in the stream and reported in
"show sess all", just like they will be reported in panic dumps.
DEBUG: mux-h1: Add debug counters to track some errors
Debug counters are added to track errors about wrong the payload length
during the message formatting (on the sending path). Aborts are also
concerned. connection shutdowns and errors while the end of the message was
not reached are now tracked. On the sending path, shutdown performed while
all the message was not forwarded are tracked too.
DEBUG: stream: Add debug counters to track some client/server aborts
Not all aborts are tracked for now but only those a bit ambiguous. Mainly,
aborts during the data forwarding are concerned. Those triggered during the
request or the response analysis are easier to analyze with the stream
termination state.
BUG/MINOR: stconn: Pretend the SE have more data to deliver on abortonclose
When abortonclose option is enabled on the backend, at the SC level, we must
still pretend the SE have more data to deliver to be able to receive the
EOS. It must be performed at 2 places:
* When the backend is set and the connection is requested. It is when the
option is seen for the first time.
* After a receive attempt, if the EOI flag is set on the sedesc.
Otherwise, when an abort is detected by the mux, the SC is not
notified.
This patch should fix the issue #2764.
This bug probably exists in all stable version but is only visible since bca5e1423 ("OPTIM: stconn: Don't pretend mux have more data to deliver on
EOI/EOS/ERROR"). So I suggest to not backport it for now, except if the commit
above is backported.
BUG/MEDIUM: mux-h2: Remove H2S from send list if data are sent via 0-copy FF
When data are sent via the zero-copy data forwarding, in h2_done_ff, we must
be sure to remove the H2 stream from the send list if something is send. It
was only performed if no blocking condition was encountered. But we must
also do it if something is sent. Otherwise the transfer may be blocked till
timeout.
BUG/MEDIUM: stats-html: Never dump more data than expected during 0-copy FF
During the zero-copy data forwarding, the caller specify the maximum amount
of data the producer may push. However, the HTML stats applet does not use
it and can fill all the free space in the buffer. It is especially an issue
when the consumer is limited by a flow control, like the H2. Because we may
emit too large DATA frame in this case. It is especially visible with big
buffer (for instance 32k).
In the early age or zero-copy data forwarding, the caller was responsible to
pass a properly resized buffer. And during the different refactoring steps,
this has changed but the HTML stats applet was not updated accordingly.
To fix the bug, the buffer used to dump the HTML page is resized to be sure
not too much data are dumped.
This patch should solve the issue #2757. It must be backported to 3.0.
Willy Tarreau [Mon, 21 Oct 2024 16:51:55 +0000 (18:51 +0200)]
MINOR: debug: add "debug dev counters" to list code counters
Issuing "debug dev counters" on the CLI will now scan all existing
counters, and report their count, type, location, function name, the
condition and an optional comment passed to the macro.
The command takes a number of arguments:
- "show": this is the default, it will just list the counters
- "reset": will reset the matching counters instead of listing them
- "all": by default, only non-zero counters are listed. With "all",
they are all listed
- "bug": restrict the reset or dump to counters of type "BUG" (BUG_ON usually)
- "chk": restrict the reset or dump to counters of type "CHK" (CHECK_IF)
- "cnt": restrict the reset or dump to counters of type "CNT" (COUNT_IF)
The types may be cumulated, and the option entered in any order. Here's
an example of the output of "debug dev counters show all bug":
Willy Tarreau [Mon, 21 Oct 2024 16:34:21 +0000 (18:34 +0200)]
MINOR: debug: add a new debug macro COUNT_IF()
This macro works exactly like BUG_ON() except that it never logs anything
nor crashes, it only implements an atomic counter that is incremented on
every call. This can be used to count a number of unlikely events that are
worth checking at run time on setups showing unusual and unreproducible
behaviors.
Willy Tarreau [Mon, 21 Oct 2024 16:29:00 +0000 (18:29 +0200)]
MEDIUM: debug: add match counters for BUG_ON/WARN_ON/CHECK_IF
These macros do not always kill the process, and sometimes it would be
nice to know if some match or not, and how many times (especially for the
CHECK_IF one).
This commit adds a new section "dbg_cnt" made of structs that contain
function name, file name, line number, check type, condition and match
count. A newe macro __DBG_COUNT() adds one to the counter, and is placed
inside _BUG_ON() and _BUG_ON_ONCE(). It's worth noting that the exact
type of the check is not very precise but in practice we don't care,
as most checks will cause the process to die anyway unless they're of
type _BUG_ON_ONCE() (used by CHECK_IF by default).
All of this is limited to !defined(USE_OBSOLETE_LINKER) because we're
creating a section, thus we need a modern linker to be able to scan
this section later. Doing so adds ~50kB to the executable due to the
~1266 BUG_ON() and others placed there. That's not huge in comparison
to the visibility it can provide.
Willy Tarreau [Mon, 21 Oct 2024 16:17:25 +0000 (18:17 +0200)]
CLEANUP: debug: make the BUG_ON() macros check the condition in the outer one
The BUG_ON() macros are made of two levels so as to resolve the condition
to a string. However this doesn't offer much flexibility for performing
other operations when the condition is validated, so let's adjust them so
that the condition is checked in the outer macro and the operations are
performed in the inner one.
Amaury Denoyelle [Fri, 18 Oct 2024 15:46:06 +0000 (17:46 +0200)]
BUG/MINOR: mux-quic: do not close STREAM with empty FIN if no data sent
A stream may be shut without any HTX EOM reported to report a proper
closure. This is the case for QCS instances flagged with
QC_SF_UNKNOWN_PL_LENGTH. Shut is performed with an empty FIN emission
instead of a RESET_STREAM. This has been implemented since the following
patch :
However, in case of HTTP/3, an empty FIN should only be done after a
full message is emitted, which requires at least a HEADERS frame. If an
empty FIN is emitted without it, client may interpret this as invalid
and close the connection. To prevent this, fallback to a RESET_STREAM
emission if no data were emitted on the stream.
This was reproduced using ngtcp2-client with 10% loss (-r 0.1) on a
remote host, with httpterm request "/?s=100k&C=1&b=0&P=400". An error
ERR_H3_FRAME_UNEXPECTED is returned by ngtcp2-client when the bug
occurs.
Note that this change is incomplete. The message validity depends solely
on the application protocol in use. As such, a new app_ops callback
should be implemented to ensure the stream is closed accordingly.
However, this first patch ensures that at least HTTP/3 case is valid
while keeping a minimal backport process.
Amaury Denoyelle [Mon, 21 Oct 2024 08:28:18 +0000 (10:28 +0200)]
MINOR: mux-quic: simplify sending of empty STREAM FIN
An empty STREAM frame can be emitted by QUIC MUX to notify about a
delayed FIN when there is no data left to transmit. This requires a
tedious comparison on stream offset in qmux_ctrl_send() to ensure an
empty stream frame is not always considered as retransmitted, which is
necessary to locally close the QCS instance.
Simplify this by unsubscribe from streamdesc layer when the QCS is
locally closed on FIN transmission notification. This prevents all
future retransmitted frames to be reported to the QCS instance,
especially any potentially retransmitted empty FIN.
Before this patch, when wrong argument was provided in the configuration for
mworker-max-reloads keyword, parser shows these errors below on the stderr:
So, as 'mworker-max-reloads' is parsed in discovery mode by master process
let's align now its parser with all others, which could be called for this
mode. Like this in cases, when there are too many args or argument isn't a
valid integer we return proper error codes to global section parser and
messages are formated properly.
This fix should be backported in all stable versions.
Ilya Shipitsin [Thu, 17 Oct 2024 21:02:40 +0000 (23:02 +0200)]
CI: bump development builds explicitely to Ubuntu 24.04
Initially we agreed to split builds into "latest" for development branch
and fixed 22.04 for stable branches. It got broken when "latest" label migrated
from ubuntu-22 to ubuntu-24 ... because of build cache. Cache key is built using
runner label, it was not prepared to use the same "latest" cache from ubuntu 22
on ubuntu 24. To make things clear, let's stick explicitely to ubuntu 24.
Ilya Shipitsin [Thu, 17 Oct 2024 21:02:39 +0000 (23:02 +0200)]
CI: prepare Coverity build for Ubuntu 24
PCRE2 is recommended, PCRE was chosen for no reason. GHA Ubuntu 22 images include both libs,
but recent Ubuntu 24 does not. Let us prepare for Ubuntu 24
Willy Tarreau [Mon, 21 Oct 2024 02:17:59 +0000 (04:17 +0200)]
BUILD: mux-h2/traces: fix build on 32-bit due to size of the DATA frame
Commit cf3fe1eed ("MINOR: mux-h2/traces: print the size of the DATA
frames") added the size of the DATA frame to the traces. Unfortunately
it uses ullong instead of ulong to cast a pointer, which breaks the
build on 32-bit platforms. Let's just switch it to ulong which works
on both.
Willy Tarreau [Sat, 19 Oct 2024 12:53:11 +0000 (14:53 +0200)]
MEDIUM: debug: on panic, make the target thread automatically allocate its buf
One main problem with panic dumps is that they're filling the dumping
thread's trash, and that the global thread_dump_buffer is too small to
catch enough of them.
Here we're proceeding differently. When dumping threads for a panic, we're
passing the magic value 0x2 as the buffer, and it will instruct the target
thread to allocate its own buffer using get_trash_chunk() (which is signal
safe), so that each thread dumps into its own buffer. Then the thread will
wait for the buffer to be consumed, and will assign its own thread_dump_buffer
to it. This way we can simply dump all threads' buffers from gdb like this:
(gdb) set $t=0
while ($t < global.nbthread)
printf "%s\n", ha_thread_ctx[$t].thread_dump_buffer.area
set $t=$t+1
end
For now we make it wait forever since it's only called on panic and we
want to make sure the thread doesn't leave and continues to use that trash
buffer or do other nasty stuff. That way the dumping thread will make all
of them die.
This would be useful to backport to the most recent branches to help
troubleshooting. It backports well to 2.9, except for some trivial
context in tinfo-t.h for an updated comment. 2.8 and older would also
require TAINTED_PANIC. The following previous patches are required:
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
Willy Tarreau [Sat, 19 Oct 2024 12:15:20 +0000 (14:15 +0200)]
MINOR: debug: replace ha_thread_dump() with its two components
At the few places we were calling ha_thread_dump(), now we're
calling separately ha_thread_dump_fill() and ha_thread_dump_done()
once the data are consumed.
Willy Tarreau [Sat, 19 Oct 2024 12:52:35 +0000 (14:52 +0200)]
MINOR: debug: make ha_thread_dump_done() take the pointer to be used
This will allow the caller to decide whether to definitely clear the
pointer and release the thread, or to leave it unlocked so that it's
easy to analyse from the struct (the goal will be to use that in panic()
so that cores are easy to analyse).
Willy Tarreau [Sat, 19 Oct 2024 11:53:39 +0000 (13:53 +0200)]
MINOR: debug: slightly change the thread_dump_pointer signification
Now the thread_dump_pointer is returned ORed with 1 once done, or NULL
when cancelled (for now noone cancels). The goal will be to permit
the callee to provide its own pointer.
The ha_thread_dump_fill() function now returns the buffer pointer that
was used (without OR 1) or NULL, for ease of use from the caller.
Willy Tarreau [Sat, 19 Oct 2024 11:45:57 +0000 (13:45 +0200)]
MINOR: debug: split ha_thread_dump() in two parts
We want to have a function to trigger the dump and another one to wait
for it to be completed. This will be important to permit panic dumps to
be done on local threads. For now this does not change anything, as the
function still calls the two new functions one after the other.
Willy Tarreau [Sat, 19 Oct 2024 13:18:44 +0000 (15:18 +0200)]
MINOR: chunk: drop the global thread_dump_buffer
This variable is not very useful and is confusing anyway. It was mostly
used to detect that a panic dump was still in progress, but we can now
check mark_tainted() for this. The pointer was set to one of the dumping
thread's trash chunks. Let's temporarily continue to copy the dumps to
that trash, we'll remove it later.
Willy Tarreau [Sat, 19 Oct 2024 13:12:47 +0000 (15:12 +0200)]
MINOR: debug: make mark_tainted() return the previous value
Since mark_tainted() uses atomic ops to update the tainted status, let's
make it return the prior value, which will allow the caller to detect
if it's the first one to set it or not.
Willy Tarreau [Fri, 18 Oct 2024 16:42:47 +0000 (18:42 +0200)]
OPTIM: buffers: avoid a useless wrapping check for ofs == 0
As mentioned in previous commit, b_peek_ofs() performs a wrapping check
but is often called with ofs == 0 as a constant. We can detect this case
with __builtin_const_p() so it makes sense to use it. A test shows a size
reduction of about 320 bytes, which is not much, but it happens in hot code
paths, and each 16 bytes reduction indicates an eliminated conditional
branch.
Some clear winners are ci_getblk_nc() (-48 bytes), h2c_dec_hdrs (-141B),
h1_copy_msg_data (-124B), tcpcheck_spop_expect_hello (-80B),
h1_parse_msg_data (-44B). These ones will definitely benefit from doing
less conditional jumps.
Willy Tarreau [Fri, 18 Oct 2024 16:28:39 +0000 (18:28 +0200)]
CLEANUP: buffers: simplify b_get_varint()
The function is an exact copy of b_peek_varint() with ofs==0 and doing a
b_del() at the end. We can simply call that other one and delete the
contents. It turns out that the code is bigger with this change because
b_peek_varint() passes its offset to b_peek() which performs a wrapping
check. When ofs==0 the wrapping cannot happen, but there's no real way
to tell that to the compiler. Instead conditioning the if() in b_peek()
with (!__builtin_constant_p(ofs) || ofs) does the job, but it's not worth
it at the moment since we have no users of b_get_varint() for now. Let's
just stick to the simple normal code.
Willy Tarreau [Fri, 18 Oct 2024 15:53:25 +0000 (17:53 +0200)]
BUILD: buffers: keep b_getblk_nc() and b_peek_varint() in buf.h
Some large functions were moved to buf.c by commit ac66df4e2 ("REORG:
buffers: move some of the heavy functions from buf.h to buf.c"). However,
as found by Amaury, haring doesn't build anymore. Upon close inspection,
b_getblk_nc() isn't that big since it's very much inlinable, and a part
of its apparently large size comes from the BUG_ON_HOT() that were
implemented. Regarding b_peek_varint(), it doesn't have any dependency
and is used only at 4 places in the DNS code, so its loop will not have
big impacts, and the rest around can be optimised away by the compiler
so it remains relevant to keep it inlined. Also it can serve as a base
to deduplicate the code in b_get_varint().
Dragan Dosen [Thu, 17 Oct 2024 08:59:01 +0000 (10:59 +0200)]
MINOR: arg: add an argument type for identifier
The ARGT_ID argument type may now be used to set a custom resolve
function in order to help resolve the argument string value. If the
custom resolve function is not set, the behavior is the same as of
type ARGT_STR.
CLEANUP: http_ext: remove useless BUG_ON() in http_handle_xot_header()
A useless BUG_ON() statement was let in a conditional block that already
checks that the condition cannot be met within the block. Remove the
useless BUG_ON()
"option forwarded" provides a convenient way to automatically insert
rfc7239 forwarded header to requests sent to servers.
On the other hand, manually crafting the header is quite complicated due
to specific formatting rules that must be followed as per rfc7239.
However, sometimes it may be necessary to craft the header manually, for
instance if it has to be conditional or based on parameters that "option
forwarded" doesn't provide. To ease this task, in this patch we implement
rfc7239_nn and rfc7239_np which are respectively meant to craft nodename:
nodeport values, specifically intended to manually build rfc7239 'for'
and 'by' header fields while ensuring rfc7239 compliancy.
f627b92 BUG/MEDIUM: quic: always validate sender address on 0-RTT
and could be easily reproduced with picoquic QUIC client with -Q option
which splits a big ClientHello TLS message into two Initial datagrams.
A second condition must be fulfilled to reprodue this issue: picoquic
must not send the token provided by haproxy (NEW_TOKEN). To do that,
haproxy must be patched to prevent it to send such tokens.
Under these conditions, if haproxy has enough time to reply to the first Initial
datagrams, when it receives the second Initial datagram it sends a Retry paquet.
Then the client ignores the Retry paquet as mentionned by RFC 9000:
17.2.5.2. Handling a Retry Packet
A client MUST accept and process at most one Retry packet for each connection
attempt. After the client has received and processed an Initial or Retry packet
from the server, it MUST discard any subsequent Retry packets that it receives.
On its side, haproxy has closed the connection. When it receives the second
Initial datagram, it open a new connection but with Initial packets it
cannot decrypt (wrong ODCID) leaving the client without response.
To fix this, as the aim of the token (NEW_TOKEN) sent by haproxy is to validate
the peer address, in place of closing the connection when no token was received
for a 0RTT connection, one leaves this validation to the handshake process.
Indeed, the peer adress is validated during the handshake when a valid handshake
packet is received by the listener. But as one does not want haproxy to process
0RTT data when no token was received, one does not accept the connection before
the successful handshake completion. In addition to this, the 0RTT packets
are not released after successful handshake completion when no token was received
to leave a chance to haproxy to process these 0RTT data in such case (see
quic_conn_io_cb()).
MINOR: quic: send new tokens (NEW_TOKEN) even for 1RTT sessions
Tokens are sent when opening a connection, just after the handshake, to
be possibly reused by the peer for the next connection. They are used
to validate the peer address during the 0RTT connection openings.
But there is no reason to reserve this feature to 0RTT connections.
This patch modifies quic_build_post_handshake_frames() to do so.
BUG/MINOR: quic: avoid leaking post handshake frames
This bug came with this commit: f627b92 BUG/MEDIUM: quic: always validate sender address on 0-RTT
If an error happens in quic_build_post_handshake_frames() during the
code exexuted for th NEW_TOKEN frame allocation, some could leak because
of the wrong label used to interrupt this function asap.
Replace the "goto leave" by "goto err" to deallocated such frames to fix
this issue.
BUG/MAJOR: filters/htx: Add a flag to state the payload is altered by a filter
When a filter is registered on the data, it means it may change the payload
length by rewritting data. It means consumers of the message cannot trust the
expected length of payload as announced by the producer. The commit 8bd835b2d2
("MEDIUM: filters/htx: Don't rely on HTX extra field if payload is filtered")
was pushed to solve this issue. When the HTTP payload of a message is filtered,
the extra field is set to 0 to be sure it will never be used by error by any
consumer. However, it is not enough.
Indeed, the filters must be called before fowarding some data. They cannot be
by-passed. But if a consumer is unable to flush the HTX message, some outgoing
data can remain blocked in the channel's buffer. If some new data are then
pushed because there is some room in the channel's buffe, the producer will set
the HTX extra field. At this stage, if the consumer is unblocked and can send
again data, it is possible to call it to forward outgoing data blocked in the
channel's buffer before waking the stream up to filter new input data. It is the
purpose of the data fast-forwarding. In this case, the HTX extra field will be
seen by the consumer. It is unexpected and leads to undefined behavior.
One consequence of this bug is to perform a wrong chunking on compressed
messages, leading to processing errors at the end of the message, reported as
"ID--" in logs.
To fix the bug, a HTX flag is added to state the payload of the current HTX
message is altered. When this flag is set (HTX_FL_ALTERED_PAYLOAD), the HTX
extra field must not be trusted. And to keep things simple, when this flag is
set, the HTX extra field is automatically set to 0 when the HTX message is
loaded, in htxbuf() function.
It is probably the less intrusive way to fix the bug for now. But this part must
be reviewed to save meta-info of the HTX message outside of the message itself.
This commit should solve the issue #2741. It must be backported as far as 2.9.
BUG/MEDIUM: stconn: Check FF data of SC to perform a shutdown in sc_notify()
In sc_notify() function, the consumer side of the SC is tested to verify if
we must perform a shutdown on the endpoint. To do so, no output data must be
present in the buffer and in the iobuf. However, there is a bug here, the
iobuf of the opposite SC is tested instead of the one of the current SC. So
a shutdown can be performed on the endpoint while there are still output
data in the iobuf that must be sent. Concretely, it can only be data blocked
in a pipe.
Because of this bug, data blocked in the pipe will be never sent. I've not
tested but I guess this may block the stream during the client or server
timeout.
BUG/MINOR: http-ana: Don't report a server abort if response payload is invalid
If a parsing error is reported by the mux on the response payload, a proxy
error (PRXCOND) must be reported instead of a server abort (SRVCL). Because
of this bug, inavlid response may are reported as "SD--" or "SL--" in logs
instead of "PD--" or "PL--".
This patch must be backported to all stable versions.
MINOR: mux-h1: Add a trace on shutdown when keep-alive is not possible
When the stream is shut down, some tests are performed to know if the
connection must also be closed or not. There are trace messages for all
cases, except for the default one: Abort or close-mode. Thanks to this
patch, there is now a message too in this case.
MINOR: mux-h1: Show the SD iobuf in trace messages on stream send events
Info about the SD iobuf are now dumped in trace messages when a stream send
event is processed. It is a useful information to debug zero-copy forwarding
issues.
BUG/MEDIUM: stconn: Wait iobuf is empty to shut SE down during a check send
When a send attempt is performed on the opposite side from sc_notify() and
all outgoing data are sent while a shut was scheduled, the SE is shut down
because we consider all data were sent and no more are expected. However,
here we must also be carefull to have sent all pending data in the
iobuf. Indeed, some spliced data may be blocked. In this case, if the SE is
shut down, these data may be lost.
This patch should fix the original bug reported in #2749. It must be
backported as far as 2.9.
BUG/MINOR: resolvers/mworker: missing default resolvers in mworker mode
Since commit fe75c1e12da061 ("MEDIUM: startup: remove
MODE_MWORKER_WAIT") the MODE_MWORKER_WAIT constant disappeared. The
initialization of the default resolvers section was conditionned by this
constant.
The section must be created in mworker mode, but only in the worker not in
the master. It was currently completely disabled in both the master and
the worker which could break configuration using it, as well as the
httpclient.
BUG/MEDIUM: mworker/httpclient: initialization skipped by accident in mworker mode
Since commit fe75c1e12da061 ("MEDIUM: startup: remove
MODE_MWORKER_WAIT") the MODE_MWORKER_WAIT constant disappearded. The
initialization of the httpclient proxy was conditionned by this
constant.
The proxy must be created in mworker mode, but only in the worker not in
the master. It was currently completely disabled in both the master and
the worker provoking a NULL dereference upon httpclient usage.
BUG/MINOR: httpclient: return NULL when no proxy available during httpclient_new()
Latest patches on the mworker rework skipped the httpclient_proxy
creation by accident. This is not supposed to happen because haproxy is
supposed to stop when the proxy creation failed, but it shows a flaw in
the API.
When the httpclient_proxy or the proxy used in parameter of
httpclient_new_from_proxy() is NULL, it will be dereferenced and cause a
crash.
The patch only returns a NULL when doing an httpclient_new() if the
proxy is not available.
Willy Tarreau [Wed, 16 Oct 2024 20:57:52 +0000 (22:57 +0200)]
[RELEASE] Released version 3.1-dev10
Released version 3.1-dev10 with the following main changes :
- BUG/MAJOR: mux-quic: do not crash on empty STREAM frame emission
- BUG/MINOR: stats: Fix the name for the total number of streams created
- MINOR: quic: strengthen qc_release_frm()
- MEDIUM: quic: decount acknowledged data for MUX txbuf window
- MINOR: quic: implement dedicated type for out-of-order stream ACK
- MEDIUM: quic: merge contiguous/overlapping buffered ack stream range
- MEDIUM: quic: decount out-of-order ACK data range for MUX txbuf window
- MINOR: log: add do_log() logging helper
- MINOR: log: add do_log_parse_act() helper func
- MINOR: action: add do-log action
- REGTESTS: add some tests for 'do-log' action
- BUG/MEDIUM: hlua: make hlua_ctx_renew() safe
- BUG/MEDIUM: hlua: properly handle sample func errors in hlua_run_sample_{fetch,conv}()
- BUG/MINOR: quic: fix discarding of already stored out-of-order ACK
- BUG/MEDIUM: quic: properly decount out-of-order ACK on stream release
- MINOR: ssl: disable server side default CRL check with WolfSSL
- MEDIUM: sink: implement sink_find_early()
- MINOR: trace: postresolve sink names
- MINOR: sample: postresolve sink names in debug() converter
- BUG/MEDIUM: mux-quic: ensure timeout server is active for short requests
- MINOR: cfgparse: simulate long configuration parsing with force-cfg-parser-pause
- BUILD: cache: silence an uninitialized warning at -Og with gcc-12.2
- BUG/MINOR: mux-h2/traces: present the correct buffer for trailers errors traces
- MINOR: mux-h2/traces: print the size of the DATA frames
- CLEANUP: muxes: remove useless inclusion of ebmbtree.h
- REORG: buffers: move some of the heavy functions from buf.h to buf.c
- MINOR: buffer: add a buffer list type with functions
- MINOR: mux-h2: split the amount of rx data from the amount to ack
- MINOR: mux-h2: create and initialize an rx offset per stream
- MEDIUM: mux-h2: start to update stream when sending WU
- MEDIUM: mux-h2: start to introduce the window size in the offset calculation
- MINOR: mux-h2: count within a connection, how many streams are receiving data
- MINOR: mux-h2: allocate the array of shared rx bufs in the h2c
- MINOR: mux-h2: add rxbuf head/tail/count management for h2s
- MINOR: mux-h2: move H2_CF_WAIT_IN_LIST flag away from the demux flags
- MINOR: mux-h2: simplify the exit code in h2_rcv_buf()
- MINOR: mux-h2: simplify the wake up code in h2_rcv_buf()
- MINOR: mux-h2: clear up H2_CF_DEM_DFULL and H2_CF_DEM_SHORT_READ ambiguity
- MAJOR: mux-h2: make streams use the connection's buffers
- MAJOR: mux-h2: permit a stream to allocate as many buffers as desired
- MAJOR: mux-h2: make the rxbuf allocation algorithm a bit smarter
- MINOR: mux-h2: add tune.h2.be.rxbuf and tune.h2.fe.rxbuf global settings
- MEDIUM: mux-h2: change the default initial window to 16kB
- DOC: design-thoughts: add diagrams illustrating an rx win groth
- MEDIUM: mux-h2: rework h2_restart_reading() to differentiate recv and demux
- OPTIM: mux-h2: make h2_send() report more accurate wake up conditions
- OPTIM: mux-h2: try to continue reading after demuxing when useful
- OPTIM: mux-h2: use tasklet_wakeup_after() in h2s_notify_recv()
- MINOR: mux-h2/traces: add missing flags and proxy ID in traces
- MINOR: mux-h2/traces: add buffer-related info to h2s and h2c
- CI: cirrus-ci: bump FreeBSD image to 14-1
- REGTESTS: fix a reload race in abns_socket.vtc
- MINOR: activity/memprofile: always return "other" bin on NULL return address
- MINOR: quic: notify connection layer on handshake completion
- BUG/MINOR: stream: unblock stream on wait-for-handshake completion
- BUG/MEDIUM: quic: support wait-for-handshake
- BUG/MEDIUM: server: server stuck in maintenance after FQDN change
- BUG/MEDIUM: queue: make sure never to queue when there's no more served conns
- DEBUG: mux-h2/flags: add H2_CF_DEM_RXBUF & H2_SF_EXPECT_RXDATA for the decoder
- REGTESTS: cli: add delay 0.1 before connect to cli
- MINOR: startup: add O_CLOEXEC flag to open /dev/null
- MEDIUM: startup: move daemonization fork in init
- MINOR: startup: refactor "daemonization" fork
- MEDIUM: startup: move PID handling in init()
- MAJOR: mworker: move master-worker fork in init()
- BUG/MINOR: mworker: fix memory leak due to master-worker fork
- REORG: mworker: set nbthread=1 for master after fork
- MINOR: init: check MODE_MWORKER before creating master CLI
- REORG: mworker: move mworker_create_master_cli in master 'case'
- MEDIUM: startup: call chroot() if needed in one place
- MEDIUM: startup: do set_identity() if needed in one place
- MINOR: startup: only worker gets capabilities from bin
- CLEANUP: haproxy: rm no longer used mworker_reexec_waitmode
- MINOR: startup: rename exit_on_waitmode_failure to exit_on_failure
- MINOR: defaults: update MASTER_MAXCONN description
- MEDIUM: startup: remove MODE_MWORKER_WAIT
- MINOR: global: add MODE_DISCOVERY flag
- MEDIUM: cfgparse: add KWF_DISCOVERY keyword flag
- MEDIUM: cfgparse: call some parsers only in MODE_DISCOVERY
- MEDIUM: cfgparse-global: parse only KWF_DISCOVERY keywords in MODE_DISCOVERY
- MEDIUM: cfgparse: parse only "global" section in MODE_DISCOVERY
- MEDIUM: startup: introduce load_cfg and read_cfg
- MINOR: cfgparse: fix *thread keywords sensitive to global section position
- MINOR: mworker/cli: rename mworker_cli_proxy_new_listener
- MINOR: mworker/cli: rename and clean mworker_cli_sockpair_new
- MINOR: mworker/cli: create master CLI sockpair before fork
- MINOR: mworker/cli: create MASTER proxy before mcli listeners
- MINOR: mworker: add and set state PROC_O_INIT for new worker
- MEDIUM: mworker/cli: close child and parent fds, setup listeners
- MINOR: mworker: mworker_catch_sigchld: use fd_delete instead of close
- MINOR: startup: rename and adapt reexec_on_failure
- MINOR: mworker: add support for case when new worker dies
- MINOR: mworker: simplify the code that sets PROC_O_LEAVING
- MINOR: mworker/cli: add _send_status to support state transition
- MEDIUM: startup: split sending oldpids_sig logic for standalone and mworker modes
- MINOR: startup: split init() into separate initialization routines
- MINOR: startup: split main: add step_init_3
- MINOR: startup: simplify check for calling sock_get_old_sockets
- MINOR: startup: encapsulate sock_get_old_sockets in a function
- MINOR: startup: add bind_listeners
- MINOR: startup: split main: add step_init_4
- MINOR: startup: encapsulate master's code in run_master
- MINOR: startup: add read_cfg_in_discovery_mode
- MINOR: mworker: adapt exit_on_failure for master recovery mode
- MEDIUM: mworker: add support of master recovery mode
- MINOR: startup: add set_verbosity
- MEDIUM: mworker: block reloads
- MINOR: mworker: slow load status delivery if worker is starting
- MINOR: mworker: readapt program support in mworker_catch_sigchld
- MINOR: mworker: deserialize process list before read_cfg_in_discovery_mode
- MINOR: mworker: parse program only in MODE_DISCOVERY
- MINOR: cfgparse: add support for program section
- MINOR: startup: reintroduce program support
- MINOR: mworker-prog: stop old programs in mworker_ext_launch_all
- MINOR: mworker: reintroduce systemd support
- MINOR: mworker: report explicitly when worker exits due to max reloads
- MINOR: cfgparse-global: parse *env keywords in MODE_DISCOVERY
- MINOR: startup: reintroduce *env keywords support
- MINOR: startup: close devnullfd, when daemon mode is applied
MINOR: startup: close devnullfd, when daemon mode is applied
In case of daemon mode now daemonization fork happens in the early init stage
before parsing and applying the configuration, so we can't close
stdio/stderr/stdout immediately after forking. We keep it open until the most
of configuration, including chroot are applied in order to show alerts, if
there are some problems. To achieve this /dev/null is opened just before calling
chroot(), and after the chroot block it's used to close all standard outputs
and stdin. At this point we no longer need the fd of /dev/null, so we can close
it as well.
setenv/resetenv/presetenv/unsetenv keywords in the configuration modify the
process environment. In case of master-worker and programs we need to restore
the initial process environment before reload, as the configuration could
change in between and newly forked workers and programs should be launched
in the environment corresponded to this new configuration.
To achieve this we backup the initial process environment before the first
configuration read, when 'global' and 'program' sections are read. And then we
clean up master process environment and restore the initial one from the backup
in mworker_reexec().
MINOR: cfgparse-global: parse *env keywords in MODE_DISCOVERY
setenv/resetenv/presetenv/unsetenv keywords should be parsed by master
process and by worker. As some other master parameters could be enabled in
conditional blocks (.if...endif). To achieve this let's tag '*env' keywords
with KWF_DISCOVERY flag.
MINOR: mworker: report explicitly when worker exits due to max reloads
It's convienient for testing and for usage to produce different warning
messages, when the former worker exits due to max reloads exceeded, and when it
was terminated by the master.
Let's reintroduce systemd support in the refactored master-worker mode.
As for now, the master-worker fork happens during early initialization steps and
then the master process receieves the "READY" status message from the newly
forked worker, that has successfully loaded. Let's propagate this "READY" status
message at this moment to the systemd from the master process context
(_send_status()). We use the master process to send messages to systemd,
because it is only the process, monitored by systemd.
In master recovery mode, we also need to send to the systemd the "READY"
message, but with the status "Reload failed". "READY" will signal to systemd,
that master process is still alive, because it doesn't exit in recovery mode
and it keeps the existed worker. Status "Reload failed" will signal to user,
that something wrong has happened with the configuration. Same message logic
was originally preserved for the case, when the worker fails to read its
configuration, see on_new_child_failure() for more details.
MINOR: mworker-prog: stop old programs in mworker_ext_launch_all
This patch is a part of series to reintroduce the program support in the new
master-worker architecture.
Now, after refactoring in master-worker mode it's the master process, who
stops workers forked before the reload. Current worker no longer sends USR1 or
TERM signals to the previous one after ports binding. This behaviour is kept
only for the standalone mode.
So, in case of programs, it's up to master process as well to stop programs,
which were launched before reload. Let's do this in mworker_ext_launch_all(),
just before starting the new programs.
This patch is a part of series to reintroduce the program support in the new
master-worker architecture.
Let's add here mworker_ext_launch_all() call before master-worker fork to
start external programs. We keep the order and the place of these two forks
(program and master-worker) the same as before the refactoring, in order to
avoid regressions.
This patch is a part of series to reintroduce the program support in the new
master-worker architecture.
Programs are launched by master, thus only the master process needs its
configuration. Therefore, program section parser should be called only in
discovery mode, when master parses its configuration.
Program section has a post section parser. It should be called only in
discovery mode as well.
MINOR: mworker: parse program only in MODE_DISCOVERY
This patch is a part of series to reintroduce the program support in the new
master-worker architecture.
Master process launches external programs, so it needs to read program
section. Thus, it should be parsed in MODE_DISCOVERY. Worker does not need
program settings, so let's check the runtime mode in cfg_parse_program. Worker
should always skip this section.
MINOR: mworker: deserialize process list before read_cfg_in_discovery_mode
This patch is a part of series to reintroduce the program support in the new
master-worker architecture.
For the moment we keep the order of program and worker forks the same as before
the refactoring, as we need to be sure that this won't introduce regressions.
So, programs are forked before the new worker process.
Before the program's fork we already need deserialized processes list to find
the programs launched before reload and to stop them. Processes list saved
before the reload in HAPROXY_PROCESSES variable. It should be deserialized
before the first configuration read in discovery mode, because resetenv keyword
could be presented in the global section.
So, let's move mworker_env_to_proc_list() from mworker_create_master_cli() to
main(). We need to call it only after reload in master-worker mode, thus
HAPROXY_MWORKER_REEXEC and HAPROXY_PROCESSES should be still presented in the
re-executing process environment before the first configuration read.
MINOR: mworker: readapt program support in mworker_catch_sigchld
This patch is a part of series to reintroduce the program support in the new
master-worker architecture.
We just only launch and stop external programs and there is no any
communication between the master process and the started program binary. So,
ipc_fd[0] and ipc_fd[1] are not used and kept as -1 for programs processes. Due
to this, no need for the exiting program process to call fd_delete on this
fds. Otherwise, this will trigger a BUG_ON.
MINOR: mworker: slow load status delivery if worker is starting
With refactored master-worker architecture master and worker processes parse
its parts of the configuration. Worker could have a huge configuration, so it
will take some time to load. As now HAPROXY_LOAD_SUCCESS is set to 1 only
after receiving the status READY from the new worker
cli_io_handler_show_loadstatus() may exit very fast by showing load status 0,
and in such case and mcli socket will be closed.
This already breaks some regression tests and can confuse some APIs. So, let's
slow down the load status delivery. If in the process list there is still some
process, which is loading (PROC_O_INIT). appctx task will sleep in this case for
50ms and then return 0. cli_io_handler_show_loadstatus() is called in loop, so
with such pacing, there is a high chance that the next time, when we enter in
its scope all processes will have the state READY. Like this master CLI
connection socket won't be closed until the loading of the new worker is really
finished, thus the reload status and logs (Success=1/0) will be shown in
synchronious way.
When reloads arrive very often (sent by some APIs), newly forked workers
almost don't have a time to load completely and to send its READY status to
master, which allows then to stop the previous worker (launched before reload).
As a result, the number of workers increases very quickly, previous workers are
still alive and the memory consumption is very high.
To avoid such situations let's return in cli_parse_reload() reload status 0
with the text ""Another reload is still in progress", if there is still a
process with PROC_O_INIT flag in the processes list.
Let's encapsulate the logic to set verbosity modes (MODE_DEBUG and MODE_VERBOSE)
in a separate function set_verbosity(). This makes the code of main() more
readable and this allows to call set_verbosity() for master process in recovery
mode. So, in this mode, verbosity settings before the master re-execution will
be re-applied to master. set_verbosity() will be extended in future commits to
reduce the verbosiness of master in order not to dump pollers list and filters,
if it was started with -V or -d.
MEDIUM: mworker: add support of master recovery mode
In this commit we add run_master_in_recovery_mode(), which groups all necessary
initialization steps, which master should perform to be able to enter in its
polling loop (run_master()), when it fails while parsing its new config.
As exit_on_failure() is now adapted for master recovery mode. Let's register
it as atexit handler, when master enters in this mode. And let's remove
atexit_flag variable for master, because we no longer use it.
We also slightly refactor here read_cfg_in_discovery_mode() in order to call
run_master_in_recovery_mode() for the case, described above. Warning messages
are mandatory before calling the run_master_in_recovery_mode() as this allows
to stop haproxy with error, if it was launched in zero-warning mode.
So, in recovery mode master does not launch any worker. It just performs its
necessary initialization routines and enters in its polling loop to continue
to monitor the existed worker process.
MINOR: mworker: adapt exit_on_failure for master recovery mode
Master recovery mode replaces the former wait-mode with a difference, that
master in this case doesn't try to fork the new worker process. But it still
needs to enter to its polling loop in order to monitor the previous worker.
Master performs some initialization steps for this and it recreates its master
CLI. During its initialization steps, master could potentially fail again.
As we use for the moment for master init steps some common routines
(step_init_2() and step_init_3()), there is no way there to signal to user that
failure has happened for the master and in addition, in its recovery mode. So,
in such case exit_on_failure() can be still useful in order to print an
appropriate alert, as we can register this function as atexit handler for the
master.
Let's encapsulate here the code to load and to read the configuration at the
first time in MODE_DISCOVERY. This makes the code of main() more readable and
this adds the structure for adding necessary master initializations routines
to support master recovery mode.
MINOR: startup: encapsulate master's code in run_master
Let's encapsulate master's code (steps which it does before entering in its
polling loop and deinitialization routines after) in a separate run_master()
function. This makes the code of main() more readable. In future we plan to put
in run_master() more master process related code, in order to clean completely
init_step_2(), init_step_3() and init_step_4().
Let's encapsulate here another part of main, after binding listeners sockets
and before calling the master's code in master-worker mode. This block
contains the code, which applies verbosity settings, checks limits and updates
the ready date. It will take some time to figure out, which of these parts are
really needed for the master, or which ones it could skip. So let's put all
these for the moment in step_init_4() and let's call it for all modes.
Let's encapsulate here the code, which tries to bind listeners for the new
process in a separate function. This will make the main() code more readable.
Master process, even if it has failed while reading its new configuration, has
to bind its master CLI sockets. So like this we will can call this function in
the master recovery mode.
Master CLI socket address and port for external connections (user, monitoring
tools) are provided for now only via the command line. So, master, even after
this failure can and must reestablish master CLI connections again.
MINOR: startup: encapsulate sock_get_old_sockets in a function
Let's encapsulate here the code, that calls sock_get_old_sockets() to obtain
listeners sockets from the previous process into a separate function. This
will make the code of main() more readable and we can move this new function
(if we might need so) in future.
MINOR: startup: simplify check for calling sock_get_old_sockets
MODE_CHECK and MODE_CHECK_CONDITION are applied now very early in
step_init_1() and step_init_2() in order to check the configuration or to check
some condition provided via the command line. When these checks have
terminated, the main process exits. So, no longer need to verify these modes at
the moment, when the current process have already done its basic initialization
routines and is asking for listeners sockets from the previously started one.
The first part of main(), just after calling the former init() and before
trying to bind listeners, need to be also encapsulated into a separate
step_init_3() as it is. It contains important blocks to register signals, to
apply memory and nofile limits, etc. The order of these blocks should be also
preserved (especially the signals part).
For the moment step_init_3() must be also executed for all runtime modes.
MINOR: startup: split init() into separate initialization routines
This is the first commit in a series to add a support of the 5-th reload
use case, when the master process fails to read its new configuration. In this
case it just need to perform its initialization steps and keep the existed
worker.
To add the support for this last use case we need to split init() and main()
in a shorter steps in order to encapsulate necessary initialization routines
into separate functions.
Let's at first, make here progname as a global variable for haproxy.c, as it
will be used in error messages in the initialization functions. Then let's
split the init() into separate routines, which set and apply modes, write
process PID in a pidfile, etc.
The big part of the former init(), which called functions to allocate pools,
to initialize proxies, to calculate maxconn and to perform some post checks was
just encasulated as is, into step_init_2(). It will take some time to figure
out exactly which parts of this initialization block are really necessary for
the master process and which ones it could skip. So, for the moment
step_init_2() is called for all runtime modes.
MEDIUM: startup: split sending oldpids_sig logic for standalone and mworker modes
Before refactoring the master-worker mode, in all runtime modes, when the new
process successfully parsed its configuration and bound to sockets, it sent
either SIGUSR1 or SIGTERM to the previous one in order to terminate it.
Let's keep this logic as is for the standalone mode. In addition, in standalone
mode we need to send the signal to old process before calling set_identity(),
because in set_identity() effective user or group may change. So, the order is
important here.
In case of master-worker mode after refactoring, master terminates the previous
worker by itself up to receiving "READY" status from the new one in
_send_status(). Master also sets at this moment HAPROXY_LOAD_SUCCESS env
variable and checks, if there are some other workers to terminate with
max_reloads exceeded.
So, now in master-worker mode we terminate old workers only, when the new one
has successfully done all initialization steps and has sent "READY" status to
master.
MINOR: mworker/cli: add _send_status to support state transition
In the new master-worker architecture, when a worker process is forked and
successfully initialized it needs somehow to communicate its "READY" state to
the master, in order to terminate the previous worker and workers, that might
exceeded max_reloads counter.
So, let's implement for this a new master CLI _send_status command. A new
worker can send its status string "READY" to the master, when it's about
entering to the run poll loop, thus it can start to receive data.
In _send_status() in the master context we update the status of the new worker:
PROC_O_INIT flag is withdrawn.
When TERM signal is sent to a worker, worker terminates and this triggers the
mworker_catch_sigchld() handler in master. This handler deletes the exiting
process entry from the processes list.
In _send_status() we loop over the processes list twice. At the first time, in
order to stop workers that exceeded the max_reloads counter. At the second time,
in order to stop the worker forked before the last reload. In the corner case,
when max_reloads=1, we avoid to send SIGTERM twice to the same worker by
setting sigterm_sent flag during the first loop.
MINOR: mworker: simplify the code that sets PROC_O_LEAVING
When master performs a reexec it should set for an already existed worker the
flag PROC_O_LEAVING. It means that existed worked is marked as the previous one
and will be terminated after the reload.
In the previous implementation master process was need to do the reexec
twice (the first time for parsing its configuration and the second time to free
unused ressources). So the logic of setting PROC_O_LEAVING was based on
comparing the number of reloads, performed by each process from the processes
list, except the master.
Now, as being mentioned before, reexec is performed only once. So, in this case
we need to set PROC_O_LEAVING flag, when we deserialize the list. It is done for
all processes, which have the number of reloads stricly positive.
MINOR: mworker: add support for case when new worker dies
The case, when the new worker fails while it parses its configuration or while
it tries to apply it, could be considered as the new one, because the master
process is no longer need to reexec again. The master simply keeps the previous
worker (forked before the reload) and it let the new one to exit with failure.
When the new worker exits, in the master process context (mworker_catch_sigchld)
we need to stop a MASTER proxy listener and we need to drop the server,
attached to new worker's CLI sockpair (it's inherited in master). Then we
explicitly delete master's end of this sockpair (child->ipc_fd[0]) from the
fdtab and we free the memory allocated for the worker process.
on_new_child_failure() is called before the clean up to signal systemd that
reload/load was failed.
If the new worker fails during the first start, so there is no any previous
worker, master process should exit immediately in order to keep the same
behaviour, as it was before this architecture change.
MINOR: startup: rename and adapt reexec_on_failure
Previously reexec_on_failure() was called in cases when the process has failed
after reload, while it was parsing its configuration or it was trying to apply
it. reexec_on_failure() has called mworker_reexec() and the master process has
been reexecuted.
With the new architecture in such cases there is no longer need to reexecute
the master process after its reload again. It simply keeps the previous worker,
forked before the reload, and it lets the new one to exit with an error. But we
still need the code, which increments the number of failed reloads and which
notifies systemd with new "Reload failed!" status. So, let's reuse and adapt
for this reexec_on_failure() and let's rename it to on_new_child_failure().
MINOR: mworker: mworker_catch_sigchld: use fd_delete instead of close
If the worker exits due to failure or due to receiving TERM signal, in the
master context, we can't now simply close the master's fd (ipc_fd[0]) of
the inherited master CLI sockpair.
When the worker is created, in the master process context MASTER proxy listener
is bound to ipc_fd[0]. When this worker fails or exits, master process is
always in its polling loop. So, closing some fd in its context immediately
triggers the BUG_ON(fd->owner), as the poller try to reinsert the "freed" fd
into fdtab and try to reuse it. We must call fd_delete in this case. This will
deinitializes fd auxilary data and closes its properly.
MEDIUM: mworker/cli: close child and parent fds, setup listeners
Basically, this is the continuation of the previous commits. So, here after the
fork, worker process closes the "master" end of the copied CLI sockpair and
binds its end, ipc_fd[1], to the GLOBAL proxy listener.
mworker_cli_global_proxy_new_listener() guarantees that GLOBAL proxy will be
created, if it wasn't the case before.
Master process, at first, allocates the MASTER proxy, creates master CLI listener
(-S command line option) and reload sockpair and then closes the "worker" end of
the copied CLI sockpair and binds its end, ipc_fd[0], to the created MASTER proxy.
Usage of the new PROC_O_INIT state helps to reduce test conditions to find the
newly forked worker.
MINOR: mworker: add and set state PROC_O_INIT for new worker
Here, to distinguish between the new worker and the previous one let's add a
new process state PROC_O_INIT and let's set it, when the memory is allocated
for the new worker in the processes list.
MINOR: mworker/cli: create MASTER proxy before mcli listeners
For the master process we always need to create a MASTER proxy, even if
master cli settings were not provided via command line, because now we bind a
listener in the master process context at ipc_fd[0]. So, MASTER proxy should be
already allocated at this moment.
MINOR: mworker/cli: create master CLI sockpair before fork
The main idea here is to create a master CLI inherited sockpair just before the
master-worker fork. And only then after the fork let each process to bind a
needed listener to the its end of this sockpair.
Like this master and worker processes can close unused "ends" of its sockpair
copy (ipc_fd[0] for worker and and ipc_fd[1] for master).
When this sockpair creation happens inside the
mworker_cli_global_proxy_new_listener() is not possible for the master to
close ipc_fd[1] bound to the GLOBAL proxy listener, as this triggers a
BUG_ON(fd->owner) in fd_insert() in master context, because master process
has alredy entered in its polling loop and poller in its turn tries to reused
closed fd.
MINOR: mworker/cli: rename and clean mworker_cli_sockpair_new
Let's rename mworker_cli_sockpair_new() to
mworker_cli_global_proxy_new_listener() to outline that this function creates
the GLOBAL proxy, allocates the listener with "master-socket" bind conf and
attaches this listener to this GLOBAL proxy. Listener is bound to ipc_fd[1] of
the sockpair inherited in master and in worker (master CLI sockpair).
This is the first commit in a series to add the support of 4 primary reload
use-cases for the new master-worker architecture:
1. Newly forked worker process dies before any reload, due to some errors in
the configuration. Newly forked worker process crashes before any reload
after sending its "READY" state to master.
2. Newly forked worker process dies due to some errors in the new
configuration. This happens after reload, when this new configuration was
supplied, so the previous worker process is still here.
3. Newly forked worker process crashes after sending its "READY" state to
master due to some bugs. This happens after reload, so the previous worker
process is still here.
4. Newly forked worker process has sent its "READY" state to master and starts
to receive traffic. This happens after reload, the old worker hasn't
terminated yet, as it is waiting on some idle connection and it crashes.
Let's rename in this commit mworker_cli_proxy_new_listener() to
mworker_cli_master_proxy_new_listener() to outline, that this function creates
"master-socket" bind conf and allocates a listener. This listener is attached
to the MASTER proxy and it's bound to the ipc_fd[0] of the sockpair,
inherited in master and in worker processes (master CLI sockpair).
MINOR: cfgparse: fix *thread keywords sensitive to global section position
*thread keywords parsers are sensitive to global section position. If they are
present there, the global section must be the first section in the
configuration. *thread parsers logic is based on non_global_section_parsed
counter. So, we need to reset it explicitly before the second configuration
read done by worker or in a standalone mode.
This commit is a part of the series to add a support of discovery mode in the
configuration parser and in initialization sequence.
In order to support discovery mode, we need to read the configuration twice.
So, we need to split the stage, when we load all configuration files, from
the stage when we parse it. To do this, let's encapsulate in read_cfg() the
part, where we load the configuration files in a separate function, load_cfg().
Like this we can call only the parsing part as many times as we need.
Before reading configuration at the first time we set MODE_DISCOVERY. After
the reading this mode is immediately unset, as the real runtime mode has been
already set by discovery keywords parsers.
Second read is performed when all primary runtime modes (daemon, master-worker)
are applied, because we should not read the configuration twice in the master
process.
MEDIUM: cfgparse: parse only "global" section in MODE_DISCOVERY
This commit is a part of the series to add a support of discovery mode in the
configuration parser and in initialization sequence.
So, in discovery mode, when we read the configuration the first time, we
parse for the moment only the "global" section. Unknown section names will be
ignored.
MEDIUM: cfgparse-global: parse only KWF_DISCOVERY keywords in MODE_DISCOVERY
This commit is a part of the series to add a support of discovery mode in the
configuration parser and in initialization sequence.
Global section parser parses the majority of keywords in its function, so
those keywords don't have any dedicated parsers yet. Only after this parsing
block cfg_parse_global() starts to call dedicated parsers for any other
discovered keywords, which were not found in the block.
As all keywords, which should be parsed in MODE_DISCOVERY have its own parser
funtions, we can skip this block with goto discovery_kw and start directly from
the part, where we call parsers from the keywords list. KWF_DISCOVERY flag helps
to call in MODE_DISCOVERY only the parsers, which we are needed at this mode.
All unknown keywords and garbage will be ignored at this stage.
projects / thirdparty / haproxy.git / log
