There seems to be a tricky case in the H2 mux related to stream flow
control versus buffer a full situation : is a large response cannot
be entirely sent to the client due to the stream window being too
small, the stream is paused with the SFCTL flag. Then the upper
layer stream might get bored and expire this stream. It will then
shut it down first. But the shutdown operation might fail if the
mux buffer is full, resulting in the h2s being subscribed to the
deferred_shut event with the stream *not* added to the send_list
since it's blocked in SFCTL. In the mean time the upper layer completely
closes, calling h2_detach(). There we have a send_wait (the pending
shutw), the stream is marked with SFCTL so we orphan it.
Then if the client finally reads all the data that were clogging the
buffer, the send_list is run again, but our stream is not there. From
this point, the connection's stream list is not empty, the mux buffer
is empty, so the connection's timeout is not set. If the client
disappears without updating the stream's window, nothing will expire
the connection.
This patch makes sure we always keep the connection timeout updated.
There might be finer solutions, such as checking that there are still
living streams in the connection (i.e. streams not blocked in SFCTL
state), though this is not necessarily trivial nor useful, since the
client timeout is the same for the upper level stream and the connection
anyway.
This patch needs to be backported to 1.9 and 1.8 after some observation.
Released version 2.0-dev6 with the following main changes :
- BUG/MEDIUM: connection: fix multiple handshake polling issues
- MINOR: connection: also stop receiving after a SOCKS4 response
- MINOR: mux-h1: don't try to recv() before the connection is ready
- BUG/MEDIUM: mux-h1: only check input data for the current stream, not next one
- MEDIUM: mux-h1: don't use CS_FL_REOS anymore
- CLEANUP: connection: remove the now unused CS_FL_REOS flag
- CONTRIB: debug: add 4 missing connection/conn_stream flags
- MEDIUM: stream: make a full process_stream() loop when completing I/O on exit
- MINOR: server: increase the default pool-purge-delay to 5 seconds
- BUILD: tools: do not use the weak attribute for trace() on obsolete linkers
- BUG/MEDIUM: vars: make sure the scope is always valid when accessing vars
- BUG/MEDIUM: vars: make the tcp/http unset-var() action support conditions
- BUILD: task: fix a build warning when threads are disabled
- CLEANUP: peers: Remove tabs characters.
- CLEANUP: peers: Replace hard-coded values by macros.
- BUG/MINOR: peers: Wrong stick-table update message building.
- MINOR: dict: Add dictionary new data structure.
- MINOR: peers: Add a LRU cache implementation for dictionaries.
- MINOR: stick-table: Add "server_name" new data type.
- MINOR: cfgparse: Space allocation for "server_name" stick-table data type.
- MINOR: proxy: Add a "server by name" tree to proxy.
- MINOR: server: Add a dictionary for server names.
- MINOR: stream: Stickiness server lookup by name.
- MINOR: peers: Make peers protocol support new "server_name" data type.
- MINOR: stick-table: Make the CLI stick-table handler support dictionary entry data type.
- REGTEST: Add a basic server by name stickiness reg test.
- MINOR: peers: Add dictionary cache information to "show peers" CLI command.
- MINOR: peers: Replace hard-coded for peer protocol 64-bits value encoding by macros.
- MINOR: peers: Replace hard-coded values for peer protocol messaging by macros.
- CLEANUP: ssl: remove unneeded defined(OPENSSL_IS_BORINGSSL)
- BUILD: travis-ci improvements
- MINOR: SSL: add client/server random sample fetches
- BUG/MINOR: channel/htx: Don't alter channel during forward for empty HTX message
- BUG/MINOR: contrib/prometheus-exporter: Add HTX data block in one time
- BUG/MINOR: mux-h1: errflag must be set on H1S and not H1M during output processing
- MEDIUM: mux-h1: refactor output processing
- MINOR: mux-h1: Add the flag HAVE_O_CONN on h1s
- MINOR: mux-h1: Add h1_eval_htx_hdrs_size() to estimate size of the HTX headers
- MINOR: mux-h1: Don't count the EOM in the estimated size of headers
- MEDIUM: cache/htx: Always store info about HTX blocks in the cache
- MEDIUM: htx: Add the parsing of trailers of chunked messages
- MINOR: htx: Don't use end-of-data blocks anymore
- BUG/MINOR: mux-h1: Don't send more data than expected
- BUG/MINOR: flt_trace/htx: Only apply the random forwarding on the message body.
- BUG/MINOR: peers: Wrong "server_name" decoding.
- BUG/MEDIUM: servers: Don't attempt to destroy idle connections if disabled.
- MEDIUM: checks: Make sure we unsubscribe before calling cs_destroy().
- MEDIUM: connections: Wake the upper layer even if sending/receiving is disabled.
- MEDIUM: ssl: Handle subscribe by itself.
- MINOR: ssl: Make ssl_sock_handshake() static.
- MINOR: connections: Add a new xprt method, remove_xprt.
- MINOR: connections: Add a new xprt method, add_xprt().
- MEDIUM: connections: Introduce a handshake pseudo-XPRT.
- MEDIUM: connections: Remove CONN_FL_SOCK*
- BUG/MEDIUM: ssl: Don't forget to initialize ctx->send_recv and ctx->recv_wait.
- BUG/MINOR: peers: Wrong server name parsing.
- MINOR: server: really increase the pool-purge-delay default to 5 seconds
- BUG/MINOR: stream: don't emit a send-name-header in conn error or disconnect states
- MINOR: stream-int: use bit fields to match multiple stream-int states at once
- MEDIUM: stream-int: remove dangerous interval checks for stream-int states
- MEDIUM: stream-int: introduce a new state SI_ST_RDY
- MAJOR: stream-int: switch from SI_ST_CON to SI_ST_RDY on I/O
- MEDIUM: stream-int: make idle-conns switch to ST_RDY
- MEDIUM: stream: re-arrange the connection setup status reporting
- MINOR: stream-int: split si_update() into si_update_rx() and si_update_tx()
- MINOR: stream-int: make si_sync_send() from the send code of si_update_both()
- MEDIUM: stream: rearrange the events to remove the loop
- MEDIUM: stream: only loop on flags relevant to the analysers
- MEDIUM: stream: don't abusively loop back on changes on CF_SHUT*_NOW
- BUILD: stream-int: avoid a build warning in dev mode in si_state_bit()
- BUILD: peers: fix a build warning about an incorrect intiialization
- BUG/MINOR: time: make sure only one thread sets global_now at boot
- BUG/MEDIUM: tcp: Make sure we keep the polling consistent in tcp_probe_connect.
In tcp_probe_connect(), if the connection is still pending, do not disable
want_recv, we don't have any business to do so, but explicitely use
__conn_xprt_want_send(), otherwise the next time we'll reach tcp_probe_connect,
fd_send_ready() would return 0 and we would never flag the connection as
CO_FL_CONNECTED, which can lead to various problems, such as check not
completing because they consider it is not connected yet.
All threads call tv_update_date(-1) at boot to set their own local time
offset. While doing so they also overwrite global_now, which is not that
much of a problem except that it's not done using an atomic write and
that it will be overwritten by every there in parallel. We only need the
first thread to set it anyway, so let's simply set it if not set and do
it using a CAS. This should fix GH issue #111.
This may be backported to 1.9.
Just got this one :
src/peers.c:528:13: warning: missing braces around initializer [-Wmissing-braces]
src/peers.c:528:13: warning: (near initialization for 'cde.key') [-Wmissing-braces]
Indeed, this struct contains two structs so scalar zero is not a valid
value for the first field. Let's just leave it as an empty struct since
it was the purpose.
These flags are not used by analysers, only by the shut* functions, and
they were covered by CF_MASK_STATIC only because in the past the shut
functions were in the middle of the analysers. But here they are causing
excess loop backs which provide no value and increase processing cost.
Ideally the CF_MASK_STATIC bitfield should be revisited, but doing this
alone is enough to reduce by 30% the number of calls to si_sync_send().
In process_stream() we detect a number of conditions to decide to loop
back to the analysers. Some of them are excessive in that they perform
a strict comparison instead of filtering on the flags relevant to the
analysers as is done at other places, resulting in excess wakeups. One
of the effect is that after a successful WRITE_PARTIAL, a second send is
not possible, resulting in the loss of WRITE_PARTIAL, causing another
wakeup! Let's apply the same mask and verify the flags correctly.
The "goto redo" at the end of process_stream() to make the states converge
is still a big source of problems and mostly stems from the very late call
to the send() functions, whose results need to be considered, while it's
being done in si_update_both() when leaving.
This patch extracts the si_sync_send() calls from si_update_both(), and
places them at the relevant places in process_stream(), which are just
after the amount of data to forward is updated and before the shutw()
calls (which were also moved). The stream-interface resynchronization
needs to go slightly upper to take into account the transition from CON
to RDY that will happen consecutive to some successful send(), and that's
all.
By doing so we can now get rid of this loop and have si_update_both()
called only to update the stream interface and channel when leaving the
function, as it was initially designed to work.
It is worth noting that a number of the remaining conditions to perform
a goto resync_XXX still seem suboptimal and would benefit from being
refined to perform les resynchronization. But what matters at this stage
is that the code remains valid and efficient.
Just like we have a synchronous recv() function for the stream interface,
let's have a synchronous send function that we'll be able to call from
different places. For now this only moves the code, nothing more.
We should not update the two directions at once, in fact we should update
the Rx path after recv() and the Tx path after send(). Let's start by
splitting the update function in two for this.
Till now when a wakeup happens after a connection is attempted, we go
through sess_update_st_con_tcp() to deal with the various possible events,
then to sess_update_st_cer() to deal with a possible error detected by the
former, or to sess_establish() to complete the connection validation. There
are multiple issues in the way this is handled, which have accumulated over
time. One of them is that any spurious wakeup during SI_ST_CON would validate
the READ_ATTACHED flag and wake the analysers up. Another one is that nobody
feels responsible for clearing SI_FL_EXP if it happened at the same time as
a success (and it is present in all reports of loops to date). And another
issue is that aborts cannot happen after a clean connection setup with no
data transfer (since CF_WRITE_NULL is part of CF_WRITE_ACTIVITY). Last, the
flags cleanup work was hackish, added here and there to please the next
function (typically what had to be donne in commit 7a3367cca to work around
the url_param+reuse issue by moving READ_ATTACHED to CON).
This patch performs a significant lift up of this setup code. First, it
makes sure that the state handlers are the ones responsible for the cleanup
of the stuff they rely on. Typically sess_sestablish() will clean up the
SI_FL_EXP flag because if we decided to validate the connection it means
that we want to ignore this late timeout. Second, it splits the CON and
RDY state handlers because the former only has to deal with failures,
timeouts and non-events, while the latter has to deal with partial or
total successes. Third, everything related to connection success was
moved to sess_establish() since it's the only safe place to do so, and
this function is also called at a few places to deal with synchronous
connections, which are not seen by intermediary state handlers.
The code was made a bit more robust, for example by making sure we
always set SI_FL_NOLINGER when aborting a connection so that we don't
have any risk to leave a connection in SHUTW state in case it was
validated late. The useless return codes of some of these functions
were dropped so that callers only rely on the stream-int's state now
(which was already partially the case anyway).
The code is now a bit cleaner, could be further improved (and functions
renamed) but given the sensitivity of this part, better limit changes to
strictly necessary. It passes all reg tests.
The purpose of making idle-conns switch to SI_ST_CON was to make the
transition detectable and the operation retryable in case of connection
error. Now we have the RDY state for this which is much more suitable
since it indicates a validated connection on which we didn't necessarily
send anything yet. This will still lead to a transition to EST while not
requiring unnatural write polling nor connect timeouts.
Now whenever an I/O event succeeds during a connection attempt, we
switch the stream-int's state to SI_ST_RDY. This allows si_update()
to update R/W timeouts on the channel and end points to start to
consume outgoing data and to subscribe to lower layers in case of
failure. It also allows chk_rcv() to be performed on the other side
to enable data forwarding and make sure we don't fall into a situation
where no more events happen and nothing moves anymore.
The main reason for all the trouble we're facing with stream interface
error or timeout reports during the connection phase is that we currently
can't make the difference between a connection attempt and a validated
connection attempt. It is problematic because we tend to switch early
to SI_ST_EST but can't always do what we want in this state since it's
supposed to be set when we don't need to visit sess_establish() again.
This patch introduces a new state betwen SI_ST_CON and SI_ST_EST, which
is SI_ST_RDY. It indicates that we've verified that the connection is
ready. It's a transient state, like SI_ST_DIS, that cannot persist when
leaving process_stream(). For now it is not set, only verified in various
tests where SI_ST_CON was used or SI_ST_EST depending on the cases.
The stream-int state diagram was minimally updated to reflect the new
state, though it is largely obsolete and would need to be seriously
updated.
The stream interface state checks involving ranges were replaced with
checks on a set of states, already revealing some issues. No issue was
fixed, all was replaced in a one-to-one mapping for easier control. Some
checks involving a strict difference were also replaced with fields to
be clearer. At this stage, the result must be strictly equivalent. A few
tests were also turned to their bit-field equivalent for better readability
or in preparation for upcoming changes.
The test performed in the SPOE filter was swapped so that the closed and
error states are evicted first and that the established vs conn state is
tested second.
At some places we do check for ranges of stream-int states but those
are confusing as states ordering is not well known (e.g. it's not obvious
that CER is between CON and EST). Let's create a bit field from states so
that we can match multiple states at once instead. The new enum si_state_bit
contains SI_SB_* which are state bits instead of state values. The function
si_state_in() indicates if the state in argument is one of those represented
by the bit mask in second argument.
The test for the send-name-header field used to cover all states between
SI_ST_CON and SI_ST_CLO, which include SI_ST_CER and SI_ST_DIS. Trying to
send a header in these states makes no sense at all, so let's fix this.
This should have no visible impact so no backport is needed.
Commit fb55365f9 ("MINOR: server: increase the default pool-purge-delay
to 5 seconds") did this but the setting placed in new_server() was
overwritten by srv_settings_cpy() from the default-server values preset
in init_default_instance(). Now let's put it at the right place.
This commit was not complete:
BUG/MINOR: peers: Wrong "server_name" decoding.
We forgot forgotten to move forward <msg_cur> pointer variable after
having parse the server name string.
Again this bug may happen only if we add stick-table new data type after
the server name which is the current last one. Furthermore this bug is
visible only the first time a peer sends a server name for a stick-table
entry.
Nothing to backport.
When creating a new ssl_sock_ctx, don't forget to initialize its send_recv
and recv_wait to NULL, or we may end up dereferencing random values, and
crash.
Now that the various handshakes come with their own XPRT, there's no
need for the CONN_FL_SOCK* flags, and the conn_sock_want|stop functions,
so garbage-collect them.
Add a new XPRT that is used when using non-SSL handshakes, such as proxy
protocol or Netscaler, instead of taking care of it in conn_fd_handler().
This XPRT is installed when any of those is used, and it removes itself once
the handshake is done.
This should allow us to remove the distinction between CO_FL_SOCK* and
CO_FL_XPRT*.
Add a new method to xprt_ops, remove_xprt. When called, if the provided
xprt_ctx is the same as the xprt's underlying xprt_ctx, it then uses the
new xprt provided, otherwise it calls the remove_xprt method of the next
xprt.
The goal is to be able to add a temporary xprt, that removes itself from
the chain when it did what it had to do. This will be used to implement
a pseudo-xprt for anything that just requires a handshake (such as the
proxy protocol).
As the SSL code may have different needs than the upper layer, ie it may want
to receive when the upper layer wants to right, instead of directly forwarding
the subscribe to the underlying xprt, handle it ourself. The SSL code will
know remember any subscribe call, and wake the tasklet when it is ready
for more I/O.
In conn_fd_handler(), if the fd is ready to send/recv, wake the upper layer
even if we have CO_FL_ERROR, or if CO_FL_XPRT_RD_ENA/CO_FL_XPRT_WR_ENA isn't
set. The only reason we should reach that point is if we had a shutw/shutr,
and the upper layer may want to know about it, and is supposed to handle it
anyway.
When we want to destroy the conn_stream for some reason, usually on error,
make sure we unsubscribed before doing so. If we subsscribed, the xprt may
ultimately wake our tasklet on close, aand the check tasklet doesn't expect
it ot happen when we have no longer any conn_stream.
In connect_server(), when deciding if we should attempt to remove idle
connections, because we have to many file descriptors opened, don't attempt
to do so if idle connection pool is disabled (with pool-max-conn 0), as
if it is, srv->idle_orphan_conns won't even be allocated, and trying to
dereference it will cause a crash.
This patch fixes a bug which does not occur at this time because the "server_name"
stick-table data type is the last one (see STKTABLE_DT_SERVER_NAME). It was introduced
by this commit: "MINOR: peers: Make peers protocol support new "server_name" data type".
Indeed when receiving STD_T_DICT stick-table data type we first decode the length
of these data, then we decode the ID of this dictionary entry. To know if there
is remaining data to parse, we check if we have reached the end of the current data,
relying on <msg_end> variable. But <msg_end> is at the end of the entire message!
So this patch computes the correct end of the current STD_T_DICT before doing
anything else with it.
Nothing to backport.
In the function trace_http_payload(), when the random forwarding is enabled,
only blocks of type HTX_BLK_DATA must be considered. Because other blocks must
be forwarding in one time.
This patch must be backported to 1.9. But it will have to be adapted. Because
several changes on the HTX in the 2.0 are missing in the 1.9.
In h1_snd_buf(), we try to consume as much data as possible in a loop. In this
loop, we first format the raw HTTP message from the HTX message, then we try to
send it. But we must be carefull to never send more data than specified by the
stream-interface.
This patch must be backported to 1.9.
This type of blocks is useless because transition between data and trailers is
obvious. And when there is no trailers, the end-of-message is still there to
know when data end for chunked messages.
HTTP trailers are now parsed in the same way headers are. It means trailers are
converted to K/V blocks followed by an end-of-trailer marker. For now, to make
things simple, the type for trailer blocks are not the same than for header
blocks. But the aim is to make no difference between headers and trailers by
using the same type. Probably for the end-of marker too.
It was only done for the headers (including the EOH marker). data were prefixed
by the info field of these blocks. The payload and the trailers of the messages
were stored in raw. The total size of headers and payload were kept in the
cached object state to help output formatting.
Now, info about each HTX block is store in the cache. Only data are allowed to
be splitted. Otherwise, all blocks of an HTX message are handled the same way,
both when storing a message in the cache and when delivering it from the
cache. This will help the cache implementation to be more robust to internal
changes in the HTX. Especially for the upcoming parsing of trailers. There is
also no more need to keep extra info in the cached object state.
If there is not enough space in the HTX message, the EOM can be delayed when a
bodyless message is added. So, don't count it in the estimated size of headers.
This flag is set on h1s when output messages are formatted to know the
connection mode was already processed. It replace the variable process_conn_mode
in the function h1_process_output().
When we format the H1 output, in the loop on the HTX message, instead of
switching on the block types, we now switch on the message state. It is almost
the same, but it will ease futur changes, on trailers and end-of markers.
This bug is in an unexpected clause of the switch..case, inside
h1_process_output(). The wrong structure is used to set the error flag.
This patch must be backported to 1.9.
Since recent changes on the way HTX data blocks are added in an HTX message, we
must now be sure the prometheus service add its own blocks in one time. Indeed,
the function htx_add_data() may now decide to only copy a part of data. So
instead, we must call htx_add_data_atonce() instead.
In channel_htx_forward() and channel_htx_forward_forever(), if the HTX message
is empty, the underlying buffer may be really empty too. And we have no warranty
the caller will call htx_to_buf() later. And in practice, it is almost never
done. So the channel's buffer must not be altered. Otherwise, the buffer may be
considered as full (data == size) for an empty HTX message and no outgoing data.
This patch must be backported to 1.9.
This adds 4 sample fetches:
- ssl_fc_client_random
- ssl_fc_server_random
- ssl_bc_client_random
- ssl_bc_server_random
These fetches retrieve the client or server random value sent during the
handshake.
Their use is to be able to decrypt traffic sent using ephemeral ciphers. Tools
like wireshark expect a TLS log file with lines in a few known formats
(https://code.wireshark.org/review/gitweb?p=wireshark.git;a=blob;f=epan/dissectors/packet-tls-utils.c;h=28a51fb1fb029eae5cea52d37ff5b67d9b11950f;hb=HEAD#l5209).
Previously the only format supported using data retrievable from HAProxy state
was the one utilizing the Session-ID. However an SSL/TLS session ID is
optional, and thus cannot be relied upon for this purpose.
This change introduces the ability to extract the client random instead which
can be used for one of the other formats. The change also adds the ability to
extract the server random, just in case it might have some other use, as the
code change to support this was trivial.
full list:
update LibreSSL to 2.9.2
speed up build by using "make -j3"
cache BoringSSL checkout
build prometeus exporter
add basic cygwin build
add USE_TFO=1, USE_SYSTEMD=1 to linux builds
With this patch we define macros for the minimum values which are
encoded for 2 up to 10 bytes. This latter is big enough to encode
UINT64_MAX. We replaced at several places 240 value by PEER_ENC_2BYTES_MIN
which is the minimum value which is encoded with 2 bytes. The peer protocol
encoding consisting in encoding with only one byte a value which is
less than PEER_ENC_2BYTES_MIN and with at least 2 bytes a 64-bits value greater
than PEER_ENC_2BYTES_MIN.
With this new reg test we ensure the server by names stickiness is functional
between servers organized differently (with identical names, but with different IDs)
among two haproxy processes backends.
