The conn_retries counter was set to the max value and decremented at each
connection retry. Thus the counter reflected the number of retries left and
not the real number of retries. All calculations of redispatch or reporting
of number of retries experienced were made using subtracts from the
configured retries, which was complicated and didn't bring any benefit.
Now, this counter is set to 0 and incremented at each retry. We know we've
reached the maximum allowed connection retries by comparing it to the
configured value. In all other cases, we directly use the counter.
This patch should address the feature request #1608.
MINOR: stream-int/stream: Move conn_retries counter in the stream
The conn_retries counter may be moved into the stream structure. It only
concerns the connection establishment. The frontend stream-interface does not
use it. So it is a logical change.
MINOR: stream-int/txn: Move buffer for L7 retries in the HTTP transaction
The L7 retries only concerns the stream when a server connection is
established. Thus instead of storing the L7 buffer into the
stream-interface, it may be moved to the stream. And because it is only
available for HTTP streams, it may be moved in the HTTP transaction.
Associated flags are also moved into the HTTP transaction.
MEDIUM: tree-wide: Use CS util functions instead of SI ones
At many places, we now use the new CS functions to get a stream or a channel
from a conn-stream instead of using the stream-interface API. It is the
first step to reduce the scope of the stream-interfaces. The main change
here is about the applet I/O callback functions. Before the refactoring, the
stream-interface was the appctx owner. Thus, it was heavily used. Now, as
far as possible,the conn-stream is used. Of course, it remains many calls to
the stream-interface API.
MINOR: conn-stream: Add header file with util functions related to conn-streams
cs_utils.h header file will contain all util functions related to the
conn_streams. For now, few functions were added, all are equivalent to SI
functions. Idea is to progressively replace SI functions by CS ones.
CS_FL_ISBACK is a new flag, set on backend conn-streams. We must just be
careful to preserve this flag when the endpoint is detached from the
conn-stream.
MINOR: mux-pt: Rely on the endpoint instead of the conn-stream when possible
Instead of testing if a conn-stream exists or not, we rely on CS_EP_ORPHAN
endpoint flag. In addition, if possible, we access the endpoint from the
mux_pt context. Finally, the endpoint flags are now reported in trace
messages.
MEDIUM: conn-stream: Move remaning flags from CS to endpoint
All old flags CS_FL_* are now moved in the endpoint scope and renamed
CS_EP_* accordingly. It is a systematic replacement. There is no true change
except for the health-check and the endpoint reset. Here it is a bit special
because the same conn-stream is reused. Thus, we must handle endpoint
allocation errors. To do so, cs_reset_endp() has been adapted.
Thanks to this last change, it will now be possible to simplify the
multiplexer and probably the applets too. A review must also be performed to
remove some flags in the channel or the stream-interface. The HTX will
probably be simplified too. Finally, there is now some place in the
conn-stream to move info from the stream-interface.
MAJOR: conn-stream: Share endpoint struct between the CS and the mux/applet
The conn-stream endpoint is now shared between the conn-stream and the
applet or the multiplexer. If the mux or the applet is created first, it is
responsible to also create the endpoint and share it with the conn-stream.
If the conn-stream is created first, it is the opposite.
When the endpoint is only owned by an applet or a mux, it is called an
orphan endpoint (there is no conn-stream). When it is only owned by a
conn-stream, it is called a detached endpoint (there is no mux/applet).
The last entity that owns an endpoint is responsible to release it. When a
mux or an applet is detached from a conn-stream, the conn-stream
relinquishes the endpoint to recreate a new one. This way, the endpoint
state is never lost for the mux or the applet.
MEDIUM: conn-stream: Pre-allocate endpoint to create CS from muxes and applets
It is a transient commit to prepare next changes. Now, when a conn-stream is
created from an applet or a multiplexer, an endpoint is always provided. In
addition, the API to create a conn-stream was specialized to have one
function per type.
The next step will be to share the endpoint structure.
MEDIUM: conn-stream: Be able to pass endpoint to create a conn-stream
It is a transient commit to prepare next changes. It is possible to pass a
pre-allocated endpoint to create a new conn-stream. If it is NULL, a new
endpoint is created, otherwise the existing one is used. There no more
change at the conn-stream level.
In the applets, all conn-stream are created with no pre-allocated
endpoint. But for multiplexers, an endpoint is systematically created before
creating the conn-stream.
MINOR: conn-stream: Move some CS flags to the endpoint
Some CS flags, only related to the endpoint, are moved into the endpoint
struct. More will probably moved later. Those ones are not critical. So it
is pretty safe to move them now and this will ease next changes.
MEDIUM: conn-stream: Add an endpoint structure in the conn-stream
Group the endpoint target of a conn-stream, its context and the associated
flags in a dedicated structure in the conn-stream. It is not inlined in the
conn-stream structure. There is a dedicated pool.
For now, there is no complexity. It is just an indirection to get the
endpoint or its context. But the purpose of this structure is to be able to
share a refcounted context between the mux and the conn-stream. This way, it
will be possible to preserve it when the mux is detached from the
conn-stream.
REORG: Initialize the conn-stream by hand in cs_init()
The function cs_init() is only called by cs_new(). The conn-stream
initialization will be reviewed. It is easier to do it in cs_new() instead
of using a dedicated function. cs_new() is pretty simple, there is no reason
to split the code in this case.
MAJOR: conn-stream: Invert conn-stream endpoint and its context
This change is only significant for the multiplexer part. For the applets,
the context and the endpoint are the same. Thus, there is no much change. For
the multiplexer part, the connection was used to set the conn-stream
endpoint and the mux's stream was the context. But it is a bit strange
because once a mux is installed, it takes over the connection. In a
wonderful world, the connection should be totally hidden behind the mux. The
stream-interface and, in a lesser extent, the stream, still access the
connection because that was inherited from the pre-multiplexer era.
Now, the conn-stream endpoint is the mux's stream (an opaque entity for the
conn-stream) and the connection is the context. Dedicated functions have
been added to attached an applet or a mux to a conn-stream.
MEDIUM: applet: Set the appctx owner during allocation
The appctx owner is now always a conn-stream. Thus, it can be set during the
appctx allocation. But, to do so, the conn-stream must be created first. It
is not a problem on the server side because the conn-stream is created with
the stream. On the client side, we must take care to create the conn-stream
first.
This change should ease other changes about the applets bootstrapping.
MINOR: conn-stream: Add flags to set the type of the endpoint
This patch is mandatory to invert the endpoint and the context in the
conn-stream. There is no common type (at least for now) for the entity
representing a mux (h1s, h2s...), thus we must set its type when the
endpoint is attached to a conn-stream. There is 2 types for the conn-stream
endpoints: the mux (CS_FL_ENDP_MUX) and the applet (CS_FL_ENDP_APP).
For now there is no much change. Only the appctx is passed as argument when
the .init callback function is called. And it is not possible to yield at
this stage. It is not a problem because the feature is not used. Only the
lua defines this callback function for the lua TCP/HTTP services. The idea
is to be able to use it for all applets to initialize the appctx context.
BUG/MINOR: mux-h1: Don't release unallocated CS on error path
cs_free() must not be called when we fail to allocate the conn-stream in
h1s_new_cs() function. This bug was introduced by the commit cda94accb
("MAJOR: stream/conn_stream: Move the stream-interface into the
conn-stream").
BUG/MINOR: cache: do not display expired entries in "show cache"
It was mentioned in issue #12 that expired entries would appear with a
negative expire delay in "show cache". Instead of listing them, let's
just evict them.
This could be backported to all versions since this was reported on
1.8 already.
BUG/MINOR: mux-h2: do not send GOAWAY if SETTINGS were not sent
It was reported in issue #13 that a GOAWAY frame was sent on timeout even
if no SETTINGS frame was sent. The approach imagined by then was to track
the fact that a SETTINGS frame was already sent to avoid this, but that's
already what is done through the state, though it doesn't stand due to the
fact that we switch the frame to the error state. Thus instead what we're
doing here is to instead set the GOAWAY_FAILED flag in h2c_error() before
switching to the ERROR state when the state indicates we've not yet sent
settings, and refrain from sending anything from the h2c_send_goaway_error()
function for such states.
This could be backported to all versions where it applies well.
qcs by_id field has been replaced by a new field named "id". Adjust the
h3_debug_printf traces. This is the case since the introduction of the
qc_stream_desc type.
BUILD: halog: fix some incorrect signs in printf formats for integers
In issue #1184, cppcheck found several issues in the printf formats
used to display integers, some of which are unsigned but which used to
still rely on "%d".
BUILD/DEBUG: hpack-tbl: fix format string in standalone debug code
In issue #1184, cppcheck reports that an incorrect format "%d" was
used to print an unsigned in the debug code, though values are always
very small and this will never be an issue.
BUILD: peers: adjust some printf format to silence cppcheck
In issue #1184, cppcheck complains about some inconsistent printf
formats. At least the one in peer_prepare_hellomsg() that uses "%u"
for the int "min_ver" is wrong. Let's force other types to make it
happy, though constants cannot cause trouble.
cppcheck reports in issue #1184 a type mismatch between "%d" and the
unsigned int "misses" in the standalone debug code of lru.c. Let's
switch to "%u".
MINOR: log: add '~' to frontend when the transport layer provides SSL
We used to check if the transport layer was ssl_sock to decide to log
"~" after a frontend's name. Now that QUIC is present, this doesn't work
anymore. Better rely on the transport layer's get_ssl_sock_ctx() method.
BUG/MINOR: sock: do not double-close the accepted socket on the error path
Coverity found in issue #1646 that I added a double-close bug in last
commit e4d09cedb ("MINOR: sock: check configured limits at the sock layer,
not the listener's") because the error path already closes the FD. No
backport needed.
MINOR: ssl: refine the error testing for fc_err and fc_err_str
In issue #1645, coverity suspects some dead code due to a pair of
remaining tests on "if (!ctx)". While all other functions test the
context earlier, these ones used to only test the connection and the
transport. It's still not very clear to me if there are certain error
cases that can lead to no SSL being initially set while the rest is
ready, and the SSL arriving later, but better preserve this original
construct by testing first the connection and only later the context.
BUILD: ssl: add an unchecked version of __conn_get_ssl_sock_ctx()
First gcc, then now coverity report possible null derefs in situations
where we know these cannot happen since we call the functions in
contexts that guarantee the existence of the connection and the method
used. Let's introduce an unchecked version of the function for such
cases, just like we had to do with objt_*. This allows us to remove the
ALREADY_CHECKED() statements (which coverity doesn't see), and addresses
github issues #1643, #1644, #1647.
BUILD: ssl: fix build warning with previous changes to ssl_sock_ctx
Some compilers see a possible null deref after conn_get_ssl_sock_ctx()
in ssl_sock_parse_heartbeat, which cannot happen there, so let's mark
it as safe. No backport needed.
It was supposed to be there, and probably was not placed there due to
historic limitations in listener_accept(), but now there does not seem
to be a remaining valid reason for keeping the quic_conn out of the
handle. In addition in new_quic_cli_conn() the handle->fd was incorrectly
set to the listener's FD.
By being able to return the ssl_sock_ctx, we're now enabling the whole
set of SSL sample fetch methods to work on the current SSL context of
the QUIC connection, as seen in the following test showing a request
forwarded to an HTTP/1 server with plenty of SSL headers filled:
The code is trivial, but this is marked as medium as there's always
the risk that some of the callable functions do not like being called
on such SSL contexts.
MEDIUM: ssl: stop using conn->xprt_ctx to access the ssl_sock_ctx
The SSL functions must not use conn->xprt_ctx anymore but find the context
by calling conn_get_ssl_sock_ctx(), which will properly pass through the
transport layers to retrieve the desired information. Otherwise when the
functions are called on a QUIC connection, they refuse to work for not
being called on the proper transport.
MEDIUM: ssl: improve retrieval of ssl_sock_ctx and SSL detection
Historically there was a single way to have an SSL transport on a
connection, so detecting if the transport layer was SSL and a context
was present was sufficient to detect SSL. With QUIC, things have changed
because QUIC also relies on SSL, but the context is embedded inside the
quic_conn and the transport layer doesn't match expectations outside,
making it difficult to detect that SSL is in use over the connection.
The approach taken here to improve this consists in adding a new method
at the transport layer, get_ssl_sock_ctx(), to retrieve this often needed
ssl_sock_ctx, and to use this to detect the presence of SSL. This will
even allow some simplifications and cleanups to be made in the SSL code
itself, and QUIC will be able to provide one to export its ssl_sock_ctx.
MINOR: quic-sock: provide a pair of get_src/get_dst functions
These functions will allow the connection layer to retrieve a quic_conn's
source or destination when possible. The quic_conn holds the peer's address
but not the local one, and the sockets API doesn't always makes that easy
for datagrams. Thus for frontend connection what we're doing here is to
retrieve the listener's address when the destination address is desired.
Now it finally becomes possible to fetch the source and destination using
"src" and "dst", and to pass an incoming connection's endpoints via the
proxy protocol.
MINOR: protocol: add get_src() and get_dst() at the protocol level
Right now the proto_fam descriptor provides a family-specific
get_src() and get_dst() pair of calls to retrieve a socket's source
or destination address. However this only works for connected mode
sockets. QUIC provides its own stream protocol, which relies on a
datagram protocol underneath, so the get_src()/get_dst() at that
protocol's family will not work, and QUIC would need to provide its
own.
This patch implements get_src() and get_dst() at the protocol level
from a connection, and makes sure that conn_get_src()/conn_get_dst()
will automatically use them if defined before falling back to the
family's pair of functions.
MINOR: connection: rearrange conn_get_src/dst to be a bit more extensible
We'll want conn_get_src/dst to support other means of retrieving these
respective IP addresses, but the functions as they're designed are a bit
too restrictive for now.
This patch arranges them to have a default error fallback allowing to
test different mechanisms. In addition we now make sure the underlying
protocol is of type stream before calling the family's get_src/dst as
it makes no sense to do that on dgram sockets for example.
MINOR: mux-quic: properly set the flags and name fields
The mux didn't have its flags nor name set, as seen in this output of
"haproxy -vv":
Available multiplexer protocols :
(protocols marked as <default> cannot be specified using 'proto' keyword)
quic : mode=HTTP side=FE mux= flags=
h2 : mode=HTTP side=FE|BE mux=H2 flags=HTX|CLEAN_ABRT|HOL_RISK|NO_UPG
This might have random impacts at certain points like forcing some
connections to close instead of aborting a stream, or not always
handling certain streams as fully HTX-compliant.
MEDIUM: connection: panic when calling FD-specific functions on FD-less conns
Certain functions cannot be called on an FD-less conn because they are
normally called as part of the protocol-specific setup/teardown sequence.
Better place a few BUG_ON() to make sure none of them is called in other
situations. If any of them would trigger in ambiguous conditions, it would
always be possible to replace it with an error.
MINOR: connection: skip FD-based syscalls for FD-less connections
Some syscalls at the TCP level act directly on the FD. Some of them
are used by TCP actions like set-tos, set-mark, silent-drop, others
try to retrieve TCP info, get the source or destination address. These
ones must not be called with an invalid FD coming from an FD-less
connection, so let's add the relevant tests for this. It's worth
noting that all these ones already have fall back plans (do nothing,
error, or switch to alternate implementation).
MINOR: connection: add conn_fd() to retrieve the FD only when it exists
There are plenty of places (particularly in debug code) where we try to
dump the connection's FD only when the connection is defined. That's
already a pain but now it gets one step further with QUIC because we do
*not* want to dump this FD in this case.
conn_fd() checks if the connection exists, is ready and is not fd-less,
and returns the FD only in this case, otherwise returns -1. This aims at
simplifying most of these conditions.
MINOR: connection: add a new flag CO_FL_FDLESS on fd-less connections
QUIC connections do not use a file descriptor, instead they use the
quic equivalent which is the quic_conn. A number of our historical
functions at the connection level continue to unconditionally touch
the file descriptor and this may have consequences once QUIC starts
to be used.
This patch adds a new flag on QUIC connections, CO_FL_FDLESS, to
mention that the connection doesn't have a file descriptor, hence the
FD-based API must never be used on them.
From now on it will be possible to intrument existing functions to
panic when this flag is present.
MINOR: sock: check configured limits at the sock layer, not the listener's
listener_accept() used to continue to enforce the FD limits relative to
global.maxsock by itself while it's the last FD-specific test in the
whole file. This test has nothing to do there, it ought to be placed in
sock_accept_conn() which is the one in charge of FD allocation and tests.
Similar tests are already located there by the way. The only tiny
difference is that listener_accept() used to pause for one second when
this limit was reached, while other similar conditions were pausing only
100ms, so now the same 100ms will apply. But that's not important and
could even be considered as an improvement.
OpenSSL 3.0 emits tons of deprecation warnings for the engine API, and
it becomes a real problem because these hide other real warnings and
will prevent distros from building with -Werror. Fortunately there's a
macro to shut this one, OPENSSL_SUPPRESS_DEPRECATED, that is sufficient
to get things back to normal, so let's define it when USE_ENGINE is set.
This way we still get a chance to see other deprecation warnings when
engines are not used.
DOC: install: document the fact that SSL engines are not enabled by default
SSL engines used to be built by default for a long time but they're now
disabled consecutive to the API change that makes OpenSSL 3.0 spew plenty
of warnings. Support may still be enabled by passing USE_ENGINE=1.
BUILD: xprt-quic: replace ERR_func_error_string() with ERR_peek_error_func()
OpenSSL 3.0 warns that ERR_func_error_string() is deprecated. Using
ERR_peek_error_func() solves it instead, and this function was added to
the compat layer by commit 1effd9aa0 ("MINOR: ssl: Remove call to
ERR_func_error_string with OpenSSLv3").
Previous patch forgot to add USE_ENGINE to the list of options to be
transferred to CFLAGS, so USE_ENGINE had no effect and engines would
remain disabled.
BUG/MINOR: stats: define the description' background color in dark color scheme
Shawn Heisey reported that the proxy's description was unreadable in dark
color scheme. This is because the text color is changed in the table but
not the cell's background.
CLEANUP: connection: reduce the with of the mux dump output
In "haproxy -vv" we produce a list of available muxes with their
capabilities, but that list is often quite large for terminals due
to excess of spaces, so let's reduce them a bit to make the output
more readable.
Released version 2.6-dev5 with the following main changes :
- DOC: reflect H2 timeout changes
- BUG/MEDIUM: mux-fcgi: Properly handle return value of headers/trailers parsing
- BUG/MEDIUM: mux-h1: Properly detect full buffer cases during message parsing
- BUG/MINOR: log: Initialize the list element when allocating a new log server
- BUG/MINOR: samples: add missing context names for sample fetch functions
- MINOR: management: add some basic keyword dump infrastructure
- MINOR: config: add a function to dump all known config keywords
- MINOR: filters: extend flt_dump_kws() to dump to stdout
- MINOR: services: extend list_services() to dump to stdout
- MINOR: cli: add a new keyword dump function
- MINOR: acl: add a function to dump the list of known ACL keywords
- MINOR: samples: add a function to list register sample fetch keywords
- MINOR: sample: list registered sample converter functions
- MINOR: tools: add strordered() to check whether strings are ordered
- MINOR: action: add a function to dump the list of actions for a ruleset
- MINOR: config: alphanumerically sort config keywords output
- MINOR: sample: alphanumerically sort sample & conv keyword dumps
- MINOR: acl: alphanumerically sort the ACL dump
- MINOR: cli: alphanumerically sort the dump of supported commands
- MINOR: filters: alphabetically sort the list of filter names
- MINOR: services: alphabetically sort service names
- MEDIUM: httpclient/lua: be stricter with httpclient parameters
- MINOR: ssl: split the cert commit io handler
- MINOR: ssl: move the cert_exts and the CERT_TYPE enum
- MINOR: ssl: simplify the certificate extensions array
- MINOR: ssl: export ckch_inst_rebuild()
- MINOR: ssl: add "crt" in the cert_exts array
- MINOR: ssl/lua: CertCache.set() allows to update an SSL certificate file
- BUILD: ssl/lua: CacheCert needs OpenSSL
- DOC: lua: CertCache class documentation
- BUG/MEDIUM: quic: do not use qcs from quic_stream on ACK parsing
- MINOR: mux-quic: return qcs instance from qcc_get_qcs
- MINOR: mux-quic: reorganize qcs free
- MINOR: mux-quic: define release app-ops
- BUG/MINOR: h3: release resources on close
- BUG/MINOR: mux-quic: ensure to free all qcs on MUX release
- CLEANUP: quic: complete comment on qcs_try_to_consume
- MINOR: quic: implement stream descriptor for transport layer
- MEDIUM: quic: move transport fields from qcs to qc_conn_stream
- MEDIUM: mux-quic: remove qcs tree node
- BUG/MINOR: cli/stream: fix "shutdown session" to iterate over all threads
- DOC: management: add missing dot in 9.4.1
- BUG/MAJOR: mux_pt: always report the connection error to the conn_stream
- DOC: remove double blanks in configuration.txt
- CI: github actions: update OpenSSL to 3.0.2
- BUG/MEDIUM: quic: Possible crash in ha_quic_set_encryption_secrets()
- CLEANUP: quic: Remove all atomic operations on quic_conn struct
- CLEANUP: quic: Remove all atomic operations on packet number spaces
- MEDIUM: quic: Send ACK frames asap
- BUG/MINOR: quic: Missing probing packets when coalescing
- BUG/MINOR: quic: Discard Initial packet number space only one time
- MINOR: quic: Do not display any timer value from process_timer()
- BUG/MINOR: quic: Do not probe from an already probing packet number space
- BUG/MINOR: quic: Non duplicated frames upon fast retransmission
- BUG/MINOR: quic: Too much prepared retransmissions due to anti-amplification
- MINOR: quic: Useless call to SSL_CTX_set_default_verify_paths()
- MINOR: quic: Add traces about list of frames
- BUG/MINOR: h3: Missing wait event struct field initialization
- BUG/MINOR: quic: QUIC TLS secrets memory leak
- BUG/MINOR: quic: Missing ACK range deallocations
- BUG/MINOR: quic: Missing TX packet deallocations
- CLEANUP: hpack: be careful about integer promotion from uint8_t
- OPTIM: hpack: read 32 bits at once when possible.
- MEDIUM: ssl: allow loading of a directory with the ca-file directive
- BUG/MINOR: ssl: continue upon error when opening a directory w/ ca-file
- MINOR: ssl: ca-file @system-ca loads the system trusted CA
- DOC: configuration: add the ca-file changes
- MINOR: sample: converter: Add add_item convertor
- BUG/MINOR: ssl: handle X509_get_default_cert_dir() returning NULL
- BUG/MINOR: ssl/cli: Remove empty lines from CLI output
- MINOR: httpclient: enable request buffering
- MEDIUM: httpclient: enable l7-retry
- BUG/MINOR: httpclient: end callback in applet release
- MINOR: quic: Add draining connection state.
- MINOR: quic: Add closing connection state
- BUG/MEDIUM: quic: ensure quic-conn survives to the MUX
- CLEANUP: quic: use static qualifer on quic_close
- CLEANUP: mux-quic: remove unused QC_CF_CC_RECV
- BUG/MINOR: fix memleak on quic-conn streams cleaning
- MINOR: mux-quic: factorize conn-stream attach
- MINOR: mux-quic: adjust timeout to accelerate closing
- MINOR: mux-quic: define is_active app-ops
- MINOR: mux-quic: centralize send operations in qc_send
- MEDIUM: mux-quic: report CO_FL_ERROR on send
- MEDIUM: mux-quic: report errors on conn-streams
- MEDIUM: quic: report closing state for the MUX
- BUG/MINOR: fcgi-app: Don't add C-L header on response to HEAD requests
- BUG/MEDIUM: stats: Be sure to never set EOM flag on an empty HTX message
- BUG/MEDIUM: hlua: Don't set EOM flag on an empty HTX message in HTTP applet
- BUG/MEDIUM: promex: Be sure to never set EOM flag on an empty HTX message
- BUG/MEDIUM: mux-h1: Set outgoing message to DONE when payload length is reached
- BUG/MINOR: http_client: Don't add input data on an empty request buffer
- BUG/MEDIUM: http-conv: Fix url_enc() to not crush const samples
- BUG/MEDIUM: http-act: Don't replace URI if path is not found or invalid
- CLEANUP: mux-quic: remove uneeded TODO in qc_detach
- BUG/MEDIUM: mux-quic: properly release conn-stream on detach
- BUG/MINOR: quic: set the source not the destination address on accept()
- BUG/MEDIUM: quic: Possible crash from quic_free_arngs()
- MINOR: quic_tls: Add reusable cipher contexts to QUIC TLS contexts
- MINOR: quic_tls: Stop hardcoding cipher IV lengths
- CLEANUP: quic: Do not set any cipher/group from ssl_quic_initial_ctx()
- MINOR: quic: Add short packet key phase bit values to traces
- MINOR: quic_tls: Make key update use of reusable cipher contexts
- BUG/MINOR: opentracing: setting the return value in function flt_ot_var_set()
- BUG/BUILD: opentracing: fixed OT_DEFINE variable setting
- EXAMPLES: opentracing: refined shell scripts for testing filter performance
- DOC: opentracing: corrected comments in function descriptions
- CLEANUP: opentracing: removed unused function flt_ot_var_unset()
- CLEANUP: opentracing: removed unused function flt_ot_var_get()
- Revert "MINOR: opentracing: change the scope of the variable 'ot.uuid' from 'sess' to 'txn'"
- MINOR: opentracing: only takes the variables lock on shared entries
- CLEANUP: opentracing: added flt_ot_smp_init() function
- CLEANUP: opentracing: added variable to store variable length
- MINOR: opentracing: improved normalization of context variable names
- DEBUG: opentracing: show return values of all functions in the debug output
- CLEANUP: opentracing: added FLT_OT_PARSE_INVALID_enum enum
- DEBUG: opentracing: display the contents of the err variable after setting
- MAJOR: opentracing: reenable usage of vars to transmit opentracing context
- Revert "BUILD: opentracing: display warning in case of using OT_USE_VARS at compile time"
- MEDIUM: global: Add a "close-spread-time" option to spread soft-stop on time window
MEDIUM: global: Add a "close-spread-time" option to spread soft-stop on time window
The new 'close-spread-time' global option can be used to spread idle and
active HTTP connction closing after a SIGUSR1 signal is received. This
allows to limit bursts of reconnections when too many idle connections
are closed at once. Indeed, without this new mechanism, in case of
soft-stop, all the idle connections would be closed at once (after the
grace period is over), and all active HTTP connections would be closed
by appending a "Connection: close" header to the next response that goes
over it (or via a GOAWAY frame in case of HTTP2).
This patch adds the support of this new option for HTTP as well as HTTP2
connections. It works differently on active and idle connections.
On active connections, instead of sending systematically the GOAWAY
frame or adding the 'Connection: close' header like before once the
soft-stop has started, a random based on the remainder of the close
window is calculated, and depending on its result we could decide to
keep the connection alive. The random will be recalculated for any
subsequent request/response on this connection so the GOAWAY will still
end up being sent, but we might wait a few more round trips. This will
ensure that goaways are distributed along a longer time window than
before.
On idle connections, a random factor is used when determining the expire
field of the connection's task, which should naturally spread connection
closings on the time window (see h2c_update_timeout).
This feature request was described in GitHub issue #1614.
This patch should be backported to 2.5. It depends on "BUG/MEDIUM:
mux-h2: make use of http-request and keep-alive timeouts" which
refactorized the timeout management of HTTP2 connections.
MAJOR: opentracing: reenable usage of vars to transmit opentracing context
Since commit 3a4bedccc ("MEDIUM: vars: replace the global name index with
a hash") the names of HAProxy variables are no longer saved, ie their
64-bit hashes are saved instead.
This is very convenient for the HAProxy itself, but for the OpenTracing
module it is a problem because the names of the variables are important
when transferring the OpenTracing context. Namely, this context consists
of an unknown amount of data stored in a key-value format. The number
of these data (and the name of the variable used for this purpose) is
determined with the configuration of the OpenTracing filter, as well as
with the tracer used. The two previous sentences seem to be in conflict,
but that is only so at first glance. The function in the OpenTracing
filter used to read the context does not really know this, neither their
number nor its name. The only thing that function actually knows is the
prefix of the variable names used for context transfer, and by that it
could find all the necessary data. Of course, until the application of
the above-mentioned commit.
The problem is solved in a very simple way: in a common variable that
the filter always knows its name, the names of all variables that are the
product of the OpenTracing context are saved. The names of these context
variables can only be added to that common variable. When that variable
is no longer needed (when we no longer need context), it is deleted.
The format for saving data to this common variable is as follows:
+-----+---------------+-- .. --+-----+---------------+
| len | variable name | | len | variable name |
+-----+---------------+-- .. --+-----+---------------+
The amount of memory space used to store the length of the name is 1 byte,
with a sign (the minus sign is provided for inactive records, but this is
not currently used). This means that the maximum length of the variable
name to be saved is 127 characters, which is quite enough for use in the
filter. The buffer size for such data storage is global.tune.bufsize.
It's always good to replace "hard-coded" values with something that looks
less "hard-coded" (even though that doesn't change anything in the code).
This is done here using FLT_OT_PARSE_INVALID_enum enum.
DEBUG: opentracing: show return values of all functions in the debug output
If the OpenTracing filter is compiled using the 'OT_DEBUG=1' option, then
log messages are printed to stderr when the filter is running. In the log
one can then find (among other things) the order in which the function is
called and the value that the function returns (if it is not a void type).
Prior to applying this patch, no value returned by a function was logged.
MINOR: opentracing: improved normalization of context variable names
The flag_cpy parameter has been added to the flt_ot_normalize_name()
function, through which we can determine whether the function converts
special characters (which are part of that name) when copying a variable
name.
CLEANUP: opentracing: added variable to store variable length
The same variable should not be used to store multiple different results,
because it can be confusing. Therefore, the var_name_len variable has
been added in several functions, in order to avoid the use of the retval
variable for several different purposes.
CLEANUP: opentracing: added flt_ot_smp_init() function
The flt_ot_smp_init() function has been added to make initializing the
sample structure easier. The contents of the structure in question are
set in several places in the source of the OpenTracing filter, so it is
better to do this in one place.
Miroslav Zagorac [Wed, 23 Feb 2022 17:15:56 +0000 (18:15 +0100)]
MINOR: opentracing: only takes the variables lock on shared entries
Regarding commit #61ecf2838:
There's no point taking the variables locks for sess/txn/req/res
contexts since these ones always run inside the same thread anyway.
The ot.uuid variable should have the 'sess' scope because it is created
when an OpenTracing filter is attached to a stream. After that, the
stream processing is started and on that occasion the contexts for the
variables that have the range 'txn' and 'req' are initialized. This
means that we cannot use variables with the specified scopes before that
point.
EXAMPLES: opentracing: refined shell scripts for testing filter performance
When calling the 'basename' command, the argument ${0} is enclosed in
quotation marks. This is necessary if the path of the executable script
(contained in that argument) has "non-standard" elements, such as space
and the like.
Also, in the script 'test-speed.sh' the function sh_exit() has been added
for easier printing of messages at the end of execution.
In case of using parameter 'OT_USE_VARS=1', the value of the OT_DEFINE
variable is set incorrectly (i.e. the previous value was deleted and a
new one set instead of adding new content).
MINOR: quic_tls: Make key update use of reusable cipher contexts
We modify the key update feature implementation to support reusable cipher contexts
as this is done for the other cipher contexts for packet decryption and encryption.
To do so we attach a context to the quic_tls_kp struct and initialize it each time
the underlying secret key is updated. Same thing when we rotate the secrets keys,
we rotate the contexts as the same time.
CLEANUP: quic: Do not set any cipher/group from ssl_quic_initial_ctx()
These settings are potentially cancelled by others setting initialization shared
with SSL sock bindings. This will have to be clarified when we will adapt the
QUIC bindings configuration.
MINOR: quic_tls: Add reusable cipher contexts to QUIC TLS contexts
Add ->ctx new member field to quic_tls_secrets struct to store the cipher context
for each QUIC TLS context TX/RX parts.
Add quic_tls_rx_ctx_init() and quic_tls_tx_ctx_init() functions to initialize
these cipher context for RX and TX parts respectively.
Make qc_new_isecs() call these two functions to initialize the cipher contexts
of the Initial secrets. Same thing for ha_quic_set_encryption_secrets() to
initialize the cipher contexts of the subsequent derived secrets (ORTT, Handshake,
1RTT).
Modify quic_tls_decrypt() and quic_tls_encrypt() to always use the same cipher
context without allocating it each time they are called.
BUG/MINOR: quic: set the source not the destination address on accept()
When a QUIC connection is accepted, the wrong field is set from the
client's source address, it's the destination instead of the source.
No backport needed.
BUG/MEDIUM: mux-quic: properly release conn-stream on detach
On qc_detach(), the qcs must cleared the conn-stream context and set its
cs pointer to NULL. This prevents the qcs to point to a dangling
reference.
Without this, a SEGFAULT may occurs in qc_wake_some_streams() when
accessing an already detached conn-stream instance through a qcs.
Here is the SEGFAULT observed on haproxy.org.
Program terminated with signal 11, Segmentation fault.
1234 else if (qcs->cs->data_cb->wake) {
(gdb) p qcs.cs.data_cb
$1 = (const struct data_cb *) 0x0
CLEANUP: mux-quic: remove uneeded TODO in qc_detach
The stream mux buffering has been reworked since the introduction of the
struct qc_stream_desc. A qcs is now able to quickly release its buffer
to the quic-conn.
BUG/MEDIUM: http-act: Don't replace URI if path is not found or invalid
For replace-path, replace-pathq and replace-uri actions, we must take care
to not match on the selected element if it is not defined.
regex_exec_match2() function expects to be called with a defined
subject. However, if the request path is invalid or not found, the function
is called with a NULL subject, leading to a crash when compiled without the
PRCE/PCRE2 support.
For instance the following rules crashes HAProxy on a CONNECT request:
BUG/MEDIUM: http-conv: Fix url_enc() to not crush const samples
url_enc() encodes an input string by calling encode_string(). To do so, it
adds a trailing '\0' to the sample string. However it never restores the
sample string at the end. It is a problem for const samples. The sample
string may be in the middle of a buffer. For instance, the HTTP headers
values are concerned.
However, instead of modifying the sample string, it is easier to rely on
encode_chunk() function. It does the same but on a buffer.
BUG/MINOR: http_client: Don't add input data on an empty request buffer
When compiled in debug mode, a BUG_ON triggers an error when the payload is
fully transfered from the http-client buffer to the request channel
buffer. In fact, when channel_add_input() is called, the request buffer is
empty. So an error is reported when those data are directly forwarded,
because we try to add some output data on a buffer with no data.
To fix the bug, we must be sure to call channel_add_input() after the data
transfer.
The bug was introduced by the commit ccc7ee45f ("MINOR: httpclient: enable
request buffering"). So, this patch must be backported if the above commit
is backported.
BUG/MEDIUM: mux-h1: Set outgoing message to DONE when payload length is reached
When a message is sent, we can switch it state to MSG_DONE if all the
announced payload was processed. This way, if the EOM flag is not set on the
message when the last expected data block is processed, the message can
still be set to MSG_DONE state.
This bug is related to the previous ones. There is a design issue with the
HTX since the 2.4. When the EOM HTX block was replaced by a flag, I tried
hard to be sure the flag is always set with the last HTX block on a
message. It works pretty well for all messages received from a client or a
server. But for internal messages, it is not so simple. Because applets
cannot always properly handle the end of messages. So, there are some cases
where the EOM flag is set on an empty message.
As a workaround, for chunked messages, we can add an EOT HTX block. It does
the trick. But for messages with a content-length, there is no empty DATA
block. Thus, the only way to be sure the end of the message was reached in
this case is to detect it in the H1 multiplexr.
We already count the amount of data processed when the payload length is
announced. Thus, we must only switch the message in DONE state when last
bytes of the payload are received. Or when the EOM flag is received of
course.
BUG/MEDIUM: promex: Be sure to never set EOM flag on an empty HTX message
It is the same bug than "BUG/MEDIUM: stats: Be sure to never set EOM flag on
an empty HTX message". We must be sure to set the EOM flag on a non empyt
message to be sure the mux on the client will properly handle
shutdowns. Otherwise, it may miss the end of the message and considers any
shutdown as an abort.
This patch must be backported as far as 2.4. On previous version there is
still the EOM HTX block.
BUG/MEDIUM: hlua: Don't set EOM flag on an empty HTX message in HTTP applet
In a lua HTTP applet, when the script is finished, we must be sure to not
set the EOM on an empty message. Otherwise, because there is no data to
send, the mux on the client side may miss the end of the message and
consider any shutdown as an abort.
See "UG/MEDIUM: stats: Be sure to never set EOM flag on an empty HTX
message" for details.
This patch must be backported as far as 2.4. On previous version there is
still the EOM HTX block.
BUG/MEDIUM: stats: Be sure to never set EOM flag on an empty HTX message
During the last call to the stats I/O handle, it is possible to have nothing
to dump. It may happen for many reasons. For instance, all remaining proxies
are disabled or they don't match the specified scope. In HTML or in JSON, it
is not really an issue because there is a footer. So there are still some
data to push in the response channel buffer. In CSV, it is a problem because
there is no footer. It means it is possible to finish the response with no
payload at all in the HTX message. Thus, the EOM flag may be added on an
empty message. When this happens, a shutdown is performed on an empty HTX
message. Because there is nothing to send, the mux on the client side is not
notified that the message was properly finished and interprets the shutdown
as an abort.
The response is chunked. So an abort at this stage means the last CRLF is
never sent to the client. All data were sent but the message is invalid
because the response chunking is not finished. If the reponse is compressed,
because of a similar bug in the comppression filter, the compression is also
aborted and the content is truncated because some data a lost in the
compression filter.
It is design issue with the HTX. It must be addressed. And there is an
opportunity to do so with the recent conn-stream refactoring. It must be
carefully evaluated first. But it is possible. In the means time and to also
fix stable versions, to workaround the bug, a end-of-trailer HTX block is
systematically added at the end of the message when the EOM flag is set if
the HTX message is empty. This way, there are always some data to send when
the EOM flag is set.
Note that with a H2 client, it is only a problem when the response is
compressed.
This patch should fix the issue #1478. It must be backported as far as
2.4. On previous versions there is still the EOM block.
BUG/MINOR: fcgi-app: Don't add C-L header on response to HEAD requests
In the FCGI app, when a full response is received, if there is no
content-length and transfer-encoding headers, a content-length header is
automatically added. This avoid, as far as possible to chunk the
response. This trick was added because, most of time, scripts don"t add
those headers.
But this should not be performed for response to HEAD requests. Indeed, in
this case, there is no payload. If the payload size is not specified, we
must not added it by hand. Otherwise, a "content-length: 0" will always be
added while it is not the real payload size (unknown at this stage).
This patch should solve issue #1639. It must be backported as far as 2.2.
Define a new API to notify the MUX from the quic-conn when the
connection is about to be closed. This happens in the following cases :
- on idle timeout
- on CONNECTION_CLOSE emission or reception
The MUX wake callback is called on these conditions. The quic-conn
QUIC_FL_NOTIFY_CLOSE is set to only report once. On the MUX side,
connection flags CO_FL_SOCK_RD_SH|CO_FL_SOCK_WR_SH are set to interrupt
future emission/reception.
This patch is the counterpart to
"MEDIUM: mux-quic: report CO_FL_ERROR on send".
Now the quic-conn is able to report its closing, which may be translated
by the MUX into a CO_FL_ERROR on the connection for the upper layer.
This allows the MUX to properly react to the QUIC closing mechanism for
both idle-timeout and closing/draining states.
Complete the error reporting. For each attached streams, if CO_FL_ERROR
is set, mark them with CS_FL_ERR_PENDING|CS_FL_ERROR. This will notify
the upper layer to trigger streams detach and release of the MUX.
This reporting is implemented in a new function qc_wake_some_streams(),
called by qc_wake(). This ensures that a lower-layer error is quickly
reported to the individual streams.
Mark the connection with CO_FL_ERROR on qc_send() if the socket Tx is
closed. This flag is used by the upper layer to order a close on the
MUX. This requires to check CO_FL_ERROR in qcc_is_dead() to process to
immediate MUX free when set.
The qc_wake() callback has been completed. Most notably, it now calls
qc_send() to report a possible CO_FL_ERROR. This is useful because
qc_wake() is called by the quic-conn on imminent closing.
Note that for the moment the error flag can never be set because the
quic-conn does not report when the Tx socket is closed. This will be
implemented in a following patch.
projects / thirdparty / haproxy.git / log
