With this patch we modify the stickiness server targets lookup behavior.
First we look for this server targets by their names before looking for them by their
IDs if not found. We also insert a dictionary entry for the name of the server targets
and store the address of this entry in the underlying stick-table.
MINOR: cfgparse: Space allocation for "server_name" stick-table data type.
When parsing sticking rules, with this patch we reserve some room for the new
"server_name" stick-table data type, as this is already done for "server_id",
setting the offset and used space (in bytes) in the stick-table entry thanks
to stkable_alloc_data_type().
MINOR: stick-table: Add "server_name" new data type.
This simple patch only adds definitions to create a new stick-table
data type ID and a new standard type to store information in relation
wich dictionary entries (STD_T_DICT).
MINOR: peers: Add a LRU cache implementation for dictionaries.
We want to send some stick-table data fields stored as strings in dictionaries
without consuming too much memory and CPU. To do so we implement with this patch
a cache for send/received dictionaries entries. These dictionary of strings entries are
stored in others real dictionary entries with an identifier as key (unsigned int)
and a pointer to the dictionary of strings entries as values.
This patch adds minimalistic definitions to implement dictionary new data structure
which is an ebtree of ebpt_node structs with strings as keys. Note that this has nothing
to see with real dictionary data structure (maps of keys in association with values).
When creating this patch "CLEANUP: peers: Replace hard-coded values by macros",
we realized there was a remaining place in peer_prepare_updatemsg() where the maximum
of an encoded length harcoded value could be replaced by PEER_MSG_ENCODED_LENGTH_MAXLEN
macro. But in this case, the 1 harcoded value for the header length is wrong. Should
be 2 or PEER_MSG_HEADER_LEN. So, there is a missing byte to encode the length of
remaining data after the header.
Note that the bug was never encountered because even with a missing byte, we could
encode a maximum length which would be (1<<25) (32MB) according to the following
extract of the peers protocol documentation which were from far a never reached limit
I guess:
CLEANUP: peers: Replace hard-coded values by macros.
All the peer stick-table messages are made of a 2-byte header (PEER_MSG_HEADER_LEN)
followed by the encoded length of the remaining data wich is harcoded as 5 (in bytes)
for the maximum (PEER_MSG_ENCODED_LENGTH_MAXLEN). With such a length we can encode
a maximum length which equals to (1 << 32) - 1, which is from far enough.
This patches replaces both these values by macros where applicable.
Willy Tarreau [Tue, 4 Jun 2019 15:16:29 +0000 (17:16 +0200)]
BUILD: task: fix a build warning when threads are disabled
The __decl_hathreads() macro will leave a lone semi-colon making the end
of variables declarations, resulting in a warning if threads are disabled.
Let's simply swap it with the last variable. Thanks to Ilya Shipitsin for
reporting this issue.
Willy Tarreau [Tue, 4 Jun 2019 14:43:29 +0000 (16:43 +0200)]
BUG/MEDIUM: vars: make the tcp/http unset-var() action support conditions
Patrick Hemmer reported that http-request unset-var(foo) if ... fails to
parse. The reason is that it reuses the same parser as "set-var(foo)" which
makes a special case of the arguments, supposed to be a sample expression
for set-var, but which must not exist for unset-var. Unfortunately the
parser finds "if" or "unless" and believes it's an expression. Let's simply
drop the test so that the outer rule parser deals with potential extraneous
keywords.
This should be backported to all versions supporting unset-var().
The tests on the variables scopes is not strict enough, it needs to always
verify if the stream is valid when accessing a req/res/txn variable. This
patch does this by adding a new get_vars() function which does the job
instead of open-coding all the lookups everywhere.
It must be backported to all versions supporting set-var and
"tcp-request session" so at least 1.9 and 1.8.
Willy Tarreau [Tue, 4 Jun 2019 14:02:26 +0000 (16:02 +0200)]
BUILD: tools: do not use the weak attribute for trace() on obsolete linkers
The default dummy trace() function is marked weak in order to be easily
replaced at link time. Some linkers are having issues with the weak
attribute, so let's not mark it on these linkers. They will simply not
be able to build with TRACE=1, which is no big deal since it's only used
by developers.
Willy Tarreau [Tue, 4 Jun 2019 12:06:31 +0000 (14:06 +0200)]
MINOR: server: increase the default pool-purge-delay to 5 seconds
The default used to be a very aggressive delay of 1 second before starting
to purge idle connections, but tests show that with bursty traffic it's a
bit short. Let's increase this to 5 seconds.
Willy Tarreau [Mon, 3 Jun 2019 15:27:52 +0000 (17:27 +0200)]
MEDIUM: stream: make a full process_stream() loop when completing I/O on exit
During 1.9 development cycle a shortcut was made in process_stream() to
update the analysers immediately after an I/O even detected on the send()
path while leaving the function. In order to prevent this from being abused
by a single stream stealing all the CPU, the loop didn't cover the initial
recv() call, so that events ultimately converge.
This has caused a number of issues over time because the conditions to
decide to loop are a bit tricky. For example the CF_READ_PARTIAL flag is
not immediately removed from rqf_last and may appear for a long time at
this point, sometimes causing some loops to last long.
Another unexpected side effect is that all analysers are called again with
no data to process, just because CF_WRITE_PARTIAL is present. We cannot get
rid of this event even if of very rare use, because some analysers might
wait for some data to leave a buffer before proceeding. With a full loop,
this event would have been merged with a subsequent recv() allowing analysers
to do something more useful than just ack an event they don't care about.
While during early 1.9-dev it was very important to be kind with the
scheduler, nowadays it's lock-free for local tasks so this optimization
is much less interesting to use it for I/Os, especially if we factor in
the trouble it causes.
This patch thus removes the use of the loop for regular I/Os and instead
performs a task_wakeup() with an I/O event so that the task will be
scheduled after all other ones and will have a chance to perform another
recv() and possibly to gather more I/O events to be processed at once.
Synchronous errors and transitions to SI_ST_DIS however are still handled
by the loop.
Doing so significantly reduces the average number of calls to analysers
(those are typically halved when compression is enabled in legacy mode),
and as a side benefit, has increased the H1 performance by about 1%.
Willy Tarreau [Mon, 3 Jun 2019 12:18:22 +0000 (14:18 +0200)]
MEDIUM: mux-h1: don't use CS_FL_REOS anymore
This flag was already removed from other muxes and from the upper layers,
because it was misused. It indicates to the mux that the end of a stream
was already seen and is pending after existing data, but this should not
be on the conn_stream but internal to the mux.
This patch creates a new H1S flag H1S_F_REOS to replace it and uses it to
replace the last uses of CS_FL_REOS.
Willy Tarreau [Mon, 3 Jun 2019 11:42:54 +0000 (13:42 +0200)]
BUG/MEDIUM: mux-h1: only check input data for the current stream, not next one
The mux-h1 doesn't properly propagate end of streams to the application
layer when requests are pipelined. This is visible by launching h2load
in h1 mode with -m greater than 1 : issuing Ctrl-C has no effect until
the client timeout expires.
The reason is that among the checks conditionning the reporting of the
end of stream status and waking up the streams, is a test on the presence
of remaining input data in the demux. But with pipelining, these data may
be present for another stream and should not prevent the end of stream
condition from being reported.
This patch addresses this issue by introducing a new function
"h1s_data_pending" which returns a boolean indicating if there are in
the demux buffer any data for the current stream. That is, if the stream
is in H1_MSG_DONE state, there are never any data for it. And if it's in
a different state, then the demux buffer is checked. This replaces the
tests on b_data(&h1c->ibuf) and correctly allows end of streams to be
reported at the end of requests.
It's worth noting that 1.9 doesn't suffer from this issue but it possibly
isn't completely immune either given that the same tests are present.
Willy Tarreau [Mon, 3 Jun 2019 08:12:22 +0000 (10:12 +0200)]
MINOR: mux-h1: don't try to recv() before the connection is ready
Just as we already do in h1_send(), if the connection is not yet ready,
do not proceed and instead subscribe. This avoids a needless recvfrom()
and subscription to polling for a case which will never work since the
request was not even sent.
Willy Tarreau [Mon, 3 Jun 2019 08:14:18 +0000 (10:14 +0200)]
MINOR: connection: also stop receiving after a SOCKS4 response
Just as is done in previous patch for all handshake handlers,
also stop receiving after a SOCKS4 response was received. This
one escaped the previous cleanup but must be done to keep the
code safe.
Connection handshakes were rarely stacked on top of each other, but the
recent experiments consisting in sending PROXY over SOCKS4 revealed a
number of issues in these lower layers. First, each handler waiting for
data MUST subscribe to recv events with __conn_sock_want_recv() and MUST
unsubscribe from send events using __conn_sock_stop_send() to avoid any
wake-up loop in case a previous sender has set this. Second, each handler
waiting for sending MUST subscribe to send events with __conn_sock_want_send()
and MUST unsubscribe from recv events using __conn_sock_stop_recv() to
avoid any wake-up loop in case some data are available on the connection.
Till now this was done at various random places, and in particular the
cases where the FD was not ready for recv forgot to re-enable reading.
Second, while senders can happily use conn_sock_send() which automatically
handles EINTR, loops, and marks the FD as not ready with fd_cant_send(),
there is no equivalent for recv so receivers facing EAGAIN MUST call
fd_cant_send() to enable polling. It could be argued that implementing
an equivalent conn_sock_recv() function could be useful and more
long-term proof than the current situation.
Third, both types of handlers MUST unsubscribe from their respective
events once they managed to do their job, and none may even play with
__conn_xprt_*(). Here again this was lacking, and one surprizing call
to __conn_xprt_stop_recv() was present in the proxy protocol parser
for TCP6 messages!
Thanks to Alexander Liu for his help on this issue.
This patch must be backported to 1.9 and possibly some older versions,
though the SOCKS parts should be dropped.
Willy Tarreau [Sun, 2 Jun 2019 10:06:08 +0000 (12:06 +0200)]
[RELEASE] Released version 2.0-dev5
Released version 2.0-dev5 with the following main changes :
- BUILD: watchdog: use si_value.sival_int, not si_int for the timer's value
- BUILD: signals: FreeBSD has SI_LWP instead of SI_TKILL
- BUILD: watchdog: condition it to USE_RT
- MINOR: raw_sock: report global traffic statistics
- MINOR: stats: report the global output bit rate in human readable form
- BUG/MINOR: proto-htx: Try to keep connections alive on redirect
- BUG/MEDIUM: spoe: Don't use the SPOE applet after releasing it
- BUG/MINOR: lua: Set right direction and flags on new HTTP objects
- BUG/MINOR: mux-h2: Count EOM in bytes sent when a HEADERS frame is formatted
- BUG/MINOR: mux-h1: Report EOI instead EOS on parsing error or H2 upgrade
- BUG/MEDIUM: proto-htx: Not forward too much data when 1xx reponses are handled
- BUG/MINOR: htx: Remove a forgotten while loop in htx_defrag()
- DOC: fix typos
- BUG/MINOR: ssl_sock: Fix memory leak when disabling compression
- OPTIM: freq-ctr: don't take the date lock for most updates
- MEDIUM: mux-h2: avoid doing expensive buffer realigns when not absolutely needed
- CLEANUP: debug: remove the TRACE() macro
- MINOR: buffer: introduce b_make() to make a buffer from its parameters
- MINOR: buffer: add a new buffer ring API to manipulate rings of buffers
- MEDIUM: mux-h2: replace all occurrences of mbuf with a buffer ring
- MEDIUM: mux-h2: make the conditions to send based on mbuf, not just its tail
- MINOR: mux-h2: introduce h2_release_mbuf() to release all buffers in the mbuf ring
- MEDIUM: mux-h2: make the send() function iterate over all mux buffers
- CLEANUP: mux-h2: consistently use a local variable for the mbuf
- MINOR: mux-h2: report the mbuf's head and tail in "show fd"
- MAJOR: mux-h2: switch to next mux buffer on buffer full condition.
- BUILD: connections: shut up gcc about impossible out-of-bounds warning
- BUILD: ssl: fix latest LibreSSL reg-test error
- MINOR: cli/activity: remove "fd_del" and "fd_skip" from show activity
- MINOR: cli/activity: add 3 general purpose counters in development mode
- BUG/MAJOR: lb/threads: make sure the avoided server is not full on second pass
- BUG/MEDIUM: queue: fix the tree walk in pendconn_redistribute.
- BUG/MEDIUM: threads: fix double-word CAS on non-optimized 32-bit platforms
- MEDIUM: config: now alert when two servers have the same name
- MINOR: htx: Remove the macro IS_HTX_SMP() and always use IS_HTX_STRM() instead
- MINOR: htx: Move the macro IS_HTX_STRM() in proto/stream.h
- MINOR: htx: Store the head position instead of the wrap one
- MINOR: htx: Store start-line block's position instead of address of its payload
- MINOR: htx: Add functions to get the first block of an HTX message
- MINOR: mux-h2/htx: Get the start-line from the head when HEADERS frame is built
- MINOR: htx: Replace the function http_find_stline() by http_get_stline()
- CLEANUP: htx: Remove unused function htx_get_stline()
- MINOR: http/htx: Use sl_pos directly to replace the start-line
- MEDIUM: http/htx: Perform analysis relatively to the first block
- MINOR: channel/htx: Call channel_htx_recv_max() from channel_recv_max()
- MINOR: htx: Add function htx_get_max_blksz()
- BUG/MINOR: htx: Change htx_xfer_blk() to also count metadata
- MEDIUM: mux-h1: Use the count value received from the SI in h1_rcv_buf()
- MINOR: mux-h2: Use the count value received from the SI in h2_rcv_buf()
- MINOR: stream-int: Don't use the flag CO_RFL_KEEP_RSV anymore in si_cs_recv()
- MINOR: connection: Remove the unused flag CO_RFL_KEEP_RSV
- MINOR: mux-h2/htx: Support zero-copy when possible in h2_rcv_buf()
- MINOR: htx: Add a field to set the memory used by headers in the HTX start-line
- MINOR: h2/htx: Set hdrs_bytes on the SL when an HTX message is produced
- MINOR: mux-h1: Set hdrs_bytes on the SL when an HTX message is produced
- MINOR: htx: Be sure to xfer all headers in one time in htx_xfer_blks()
- MEDIUM: htx: 1xx messages are now part of the final reponses
- MINOR: channel/htx: Add function to forward headers of an HTX message
- MINOR: filters/htx: Use channel_htx_fwd_headers() after headers filtering
- MINOR: proto-htx: Use channel_htx_fwd_headers() to forward 1xx responses
- MEDIUM: htx: Store the first block position instead of the start-line one
- MINOR: stats/htx: don't use the first block position but the head one
- MINOR: channel/htx: Add functions to forward a part or all HTX payload
- MINOR: proto-htx: Use channel_htx_fwd_all() when unfiltered body are forwarded
- MEDIUM: filters/htx: Filter body relatively to the first block
- MINOR: htx: Optimize htx_drain() when all data are drained
- MINOR: htx: don't rely on htx_find_blk() anymore in the function htx_truncate()
- MINOR: htx: remove the unused function htx_find_blk()
- MINOR: htx: Remove support of pseudo headers because it is unused
- BUG/MEDIUM: http: fix "http-request reject" when not final
- MINOR: ssl: Make sure the underlying xprt's init method doesn't fail.
- MINOR: ssl: Don't forget to call the close method of the underlying xprt.
- MINOR: htx: rename htx_append_blk_value() to htx_add_data_atonce()
- MINOR: htx: make htx_add_data() return the transmitted byte count
- MEDIUM: htx: make htx_add_data() never defragment the buffer
- MINOR: activity: write totals on the "show activity" output
- MINOR: activity: report totals and average separately
- MEDIUM: poller: separate the wait time from the wake events
- MINOR: activity: report the number of failed pool/buffer allocations
- MEDIUM: buffers: relax the buffer lock a little bit
- MINOR: task: turn the WQ lock to an RW_LOCK
- MEDIUM: task: don't grab the WR lock just to check the WQ
- BUG/MEDIUM: mux-h1: Don't skip the TCP splicing when there is no more data to read
- MEDIUM: sessions: Introduce session flags.
- BUG/MEDIUM: h2: Don't forget to set h2s->cs to NULL after having free'd cs.
- BUG/MEDIUM: mux-h2: fix the conditions to end the h2_send() loop
- BUG/MEDIUM: mux-h2: don't refrain from offering oneself a used buffer
- BUG/MEDIUM: connection: Use the session to get the origin address if needed.
- MEDIUM: tasks: Get rid of active_tasks_mask.
- MEDIUM: connection: Upstream SOCKS4 proxy support
- BUILD: contrib/prometheus: fix build breakage caused by move of idle_pct
- BUG/MINOR: deinit/threads: make hard-stop-after perform a clean exit
Willy Tarreau [Sun, 2 Jun 2019 09:11:29 +0000 (11:11 +0200)]
BUG/MINOR: deinit/threads: make hard-stop-after perform a clean exit
As reported in GH issue #99, when hard-stop-after triggers and threads
are in use, the chance that any thread releases the resources in use by
the other ones is non-null. Thus no thread should be allowed to deinit()
nor exit by itself.
Here we take a different approach. We simply use a 3rd possible value
for the "killed" variable so that all threads know they must break out
of the run-poll-loop and immediately stop.
This patch was tested by commenting the stream_shutdown() calls in
hard_stop() to increase the chances to see a stream use released
resources. With this fix applied, it never crashes anymore.
Willy Tarreau [Sun, 2 Jun 2019 08:38:48 +0000 (10:38 +0200)]
BUILD: contrib/prometheus: fix build breakage caused by move of idle_pct
The idle_pct thread-local variable was moved to struct thread_info by
commit 81036f2 ("MINOR: time: move the cpu, mono, and idle time to
thread_info") but not updated in service-prometheus.c, thus breaking
it.
Alexander Liu [Wed, 22 May 2019 11:44:48 +0000 (19:44 +0800)]
MEDIUM: connection: Upstream SOCKS4 proxy support
Have "socks4" and "check-via-socks4" server keyword added.
Implement handshake with SOCKS4 proxy server for tcp stream connection.
See issue #82.
I have the "SOCKS: A protocol for TCP proxy across firewalls" doc found
at "https://www.openssh.com/txt/socks4.protocol". Please reference to it.
[wt: for now connecting to the SOCKS4 proxy over unix sockets is not
supported, and mixing IPv4/IPv6 is discouraged; indeed, the control
layer is unique for a connection and will be used both for connecting
and for target address manipulation. As such it may for example report
incorrect destination addresses in logs if the proxy is reached over
IPv6]
Olivier Houchard [Wed, 29 May 2019 17:22:43 +0000 (19:22 +0200)]
MEDIUM: tasks: Get rid of active_tasks_mask.
Remove the active_tasks_mask variable, we can deduce if we've work to do
by other means, and it is costly to maintain. Instead, introduce a new
function, thread_has_tasks(), that returns non-zero if there's tasks
scheduled for the thread, zero otherwise.
Olivier Houchard [Wed, 29 May 2019 15:08:03 +0000 (17:08 +0200)]
BUG/MEDIUM: connection: Use the session to get the origin address if needed.
In conn_si_send_proxy(), if we don't have a conn_stream yet, because the mux
won't be created until the SSL handshake is done, retrieve the opposite's
connection from the session. At this point, we know the session associated
with the connection is the one that initiated it, and we can thus just use
the session's origin.
Willy Tarreau [Wed, 29 May 2019 15:50:48 +0000 (17:50 +0200)]
BUG/MEDIUM: mux-h2: don't refrain from offering oneself a used buffer
Usually when calling offer_buffer(), we don't expect to offer it to
ourselves. But with h2 we have the same buffer_wait for the two directions
so we can unblock the recv path when completing a send(), or we can unblock
part of the mux buffer after sending the first few buffers that we managed
to collect. Thus it is important to always accept to wake up any requester.
A few parts of this patch could possibly be backported but earlier versions
already have other issues related to low-buffer condition so it's not sure
it's worth taking the risk to make things worse.
Willy Tarreau [Wed, 29 May 2019 15:36:37 +0000 (17:36 +0200)]
BUG/MEDIUM: mux-h2: fix the conditions to end the h2_send() loop
The test for the mux alloc failure in h2_send() right after an attempt
at h2_process_mux() used to make sense as it tried to detect that this
latter failed to produce data. But now that we have a list of buffers,
it is a perfectly valid situation where there can still be data in the
buffer(s).
So now when we see this flag we only declare it's the last run on the
loop. In addition we need to make sure we break out of the loop on
snd_buf failure, or we'll loop indefinitely, for example when the buf
is full and we can't send.
Olivier Houchard [Wed, 29 May 2019 14:44:17 +0000 (16:44 +0200)]
BUG/MEDIUM: h2: Don't forget to set h2s->cs to NULL after having free'd cs.
In h2c_frt_stream_new, if we failed to create the stream for some reason,
don't forget to set h2s->cs to NULL before calling h2s_destroy(), otherwise
h2s_destroy() will call h2s_close(), which will attempt to access
h2s->cs->flags if it's non-NULL.
Olivier Houchard [Wed, 29 May 2019 13:01:50 +0000 (15:01 +0200)]
MEDIUM: sessions: Introduce session flags.
Add session flags, and add a new flag, SESS_FL_PREFER_LAST, to be set when
we use NTLM authentication, and we should reuse the last connection. This
should fix using NTLM with HTX. This totally replaces TX_PREFER_LAST.
BUG/MEDIUM: mux-h1: Don't skip the TCP splicing when there is no more data to read
When there is no more data to read (h1m->curr_len == 0 in the state
H1_MSG_DATA), we still call xprt->rcv_pipe() callback. It is important to update
connection's flags. Especially to remove the flag CO_FL_WAIT_ROOM. Otherwise,
the pipe remains marked as full, preventing the stream-interface to fallback on
rcv_buf(). So the connection may be freezed because no more data is received and
the mux H1 remains blocked in the state H1_MSG_DATA.
Willy Tarreau [Tue, 28 May 2019 16:57:25 +0000 (18:57 +0200)]
MEDIUM: task: don't grab the WR lock just to check the WQ
When profiling locks, it appears that the WQ's lock has become the most
contended one, despite the WQ being split by thread. The reason is that
each thread takes the WQ lock before checking if it it does have something
to do. In practice the WQ almost only contains health checks and rare tasks
that can be scheduled anywhere, so this is a real waste of resources.
This patch proceeds differently. Now that the WQ's lock was turned to RW
lock, we proceed in 3 phases :
1) locklessly check for the queue's emptiness
2) take an R lock to retrieve the first element and check if it is
expired. This way most visits are performed with an R lock to find
and return the next expiration date.
3) if one expiration is found, we perform the WR-locked lookup as
usual.
As a result, on a one-minute test involving 8 threads and 64 streams at
1.3 million ctxsw/s, before this patch the lock profiler reported this :
Stats about Lock TASK_WQ:
# write lock : 1125496
# write unlock: 1125496 (0)
# wait time for write : 263.143 msec
# wait time for write/lock: 233.802 nsec
# read lock : 0
# read unlock : 0 (0)
# wait time for read : 0.000 msec
# wait time for read/lock : 0.000 nsec
And after :
Stats about Lock TASK_WQ:
# write lock : 173
# write unlock: 173 (0)
# wait time for write : 0.018 msec
# wait time for write/lock: 103.988 nsec
# read lock : 1072706
# read unlock : 1072706 (0)
# wait time for read : 60.702 msec
# wait time for read/lock : 56.588 nsec
Willy Tarreau [Tue, 28 May 2019 15:21:18 +0000 (17:21 +0200)]
MEDIUM: buffers: relax the buffer lock a little bit
In lock profiles it's visible that there is a huge contention on the
buffer lock. The reason is that when offer_buffers() is called, it
systematically takes the lock before verifying if there is any
waiter. However doing so doesn't protect against races since a
waiter can happen just after we release the lock as well. Similarly
in h2 we take the lock every time an h2c is going to be released,
even without checking that the h2c belongs to a wait list. These
two have now been addressed by verifying non-emptiness of the list
prior to taking the lock.
Willy Tarreau [Tue, 28 May 2019 15:04:16 +0000 (17:04 +0200)]
MINOR: activity: report the number of failed pool/buffer allocations
Haproxy is designed to be able to continue to run even under very low
memory conditions. However this can sometimes have a serious impact on
performance that it hard to diagnose. Let's report counters of failed
pool and buffer allocations per thread in show activity.
Willy Tarreau [Tue, 28 May 2019 14:44:05 +0000 (16:44 +0200)]
MEDIUM: poller: separate the wait time from the wake events
We have been abusing the do_poll()'s timeout for a while, making it zero
whenever there is some known activity. The problem this poses is that it
complicates activity diagnostic by incrementing the poll_exp field for
each known activity. It also requires extra computations that could be
avoided.
This change passes a "wake" argument to say that the poller must not
sleep. This simplifies the operations and allows one to differenciate
expirations from activity.
Willy Tarreau [Tue, 28 May 2019 13:09:08 +0000 (15:09 +0200)]
MINOR: activity: write totals on the "show activity" output
Most of the time we find ourselves adding per-thread fields to observe
activity, so let's compute these on the fly and display them. Now the
output shows "field: total [ thr0 thr1 ... thrn ]".
Willy Tarreau [Tue, 28 May 2019 08:58:50 +0000 (10:58 +0200)]
MEDIUM: htx: make htx_add_data() never defragment the buffer
Now instead of trying to fit 100% of the input data into the output
buffer at the risk of defragmenting it, we put what fits into it only
and return the amount of bytes transferred. In a test, compared to the
previous commit, it increases the cached data rate from 44 Gbps to
55 Gbps and saves a lot in case of large buffers : with a 1 MB buffer,
uncached transfers jumped from 700 Mbps to 30 Gbps.
Willy Tarreau [Tue, 28 May 2019 08:30:11 +0000 (10:30 +0200)]
MINOR: htx: make htx_add_data() return the transmitted byte count
In order to later allow htx_add_data() to transmit partial blocks and
avoid defragmenting the buffer, we'll need to return the number of bytes
consumed. This first modification makes the function do this and its
callers take this into account. At the moment the function still works
atomically so it returns either the block size or zero. However all
call places have been adapted to consider any value between zero and
the block size.
Olivier Houchard [Thu, 23 May 2019 16:41:47 +0000 (18:41 +0200)]
MINOR: ssl: Don't forget to call the close method of the underlying xprt.
In ssl_sock_close(), don't forget to call the underlying xprt's close method
if it exists. For now it's harmless not to do so, because the only available
layer is the raw socket, which doesn't have a close method, but that will
change when we implement QUIC.
Willy Tarreau [Tue, 28 May 2019 06:26:17 +0000 (08:26 +0200)]
BUG/MEDIUM: http: fix "http-request reject" when not final
When "http-request reject" was introduced in 1.8 with commit 53275e8b0
("MINOR: http: implement the "http-request reject" rule"), it was already
broken. The code mentions "it always returns ACT_RET_STOP" and obviously
a gross copy-paste made it ACT_RET_CONT. If the rule is the last one it
properly blocks, but if not the last one it gets ignored, as can be seen
with this simple configuration :
MINOR: htx: don't rely on htx_find_blk() anymore in the function htx_truncate()
the function htx_find_blk() is used by only one function, htx_truncate(). So
because this function does nothing very smart, we don't use it anymore. It will
be removed by another commit.
MEDIUM: filters/htx: Filter body relatively to the first block
The filters filtering HTX body, in the callback http_payload, must now loop on
an HTX message starting from the first block position. The offset passed as
parameter is relative to this position and not the head one. It is mandatory
because once filtered, data are now forwarded using the function
channel_htx_fwd_payload(). So the first block position is always updated.
MINOR: channel/htx: Add functions to forward a part or all HTX payload
The functions channel_htx_fwd_payload() and channel_htx_fwd_all() should now be
used to forward, respectively, a part of the HTX payload or all of it. These
functions forward data and update the first block position.
MINOR: stats/htx: don't use the first block position but the head one
Applets must never rely on the first block position to consume an HTX
message. The head position must be used instead. For the request it is always
the start-line. At this stage, it is not a bug, because the first position of
the request is never changed by HTX analysers.
MEDIUM: htx: Store the first block position instead of the start-line one
We don't store the start-line position anymore in the HTX message. Instead we
store the first block position to analyze. For now, it is almost the same. But
once all changes will be made on this part, this position will have to be used
by HTX analyzers, and only in the analysis context, to know where the analyse
should start.
When new blocks are added in an HTX message, if the first block position is not
defined, it is set. When the block pointed by it is removed, it is set to the
block following it. -1 remains the value to unset the position. the first block
position is unset when the HTX message is empty. It may also be unset on a
non-empty message, meaning every blocks were already analyzed.
From HTX analyzers point of view, this position is always set during headers
analysis. When they are waiting for a request or a response, if it is unset, it
means the analysis should wait. But once the analysis is started, and as long as
headers are not forwarded, it points to the message start-line.
As mentionned, outside the HTX analysis, no code must rely on the first block
position. So multiplexers and applets must always use the head position to start
a loop on an HTX message.
MINOR: channel/htx: Add function to forward headers of an HTX message
The function channel_htx_fwd_headers() should now be used by HTX analyzers to
forward all headers of an HTX message, from the start-line to the corresponding
EOH. It takes care to update the star-line position.
MEDIUM: htx: 1xx messages are now part of the final reponses
1xx informational messages (all except 101) are now part of the HTTP reponse,
semantically speaking. These messages are not followed by an EOM anymore,
because a final reponse is always expected. All these parts can also be
transferred to the channel in same time, if possible. The HTX response analyzer
has been update to forward them in loop, as the legacy one.
MINOR: htx: Be sure to xfer all headers in one time in htx_xfer_blks()
In the function htx_xfer_blks(), we take care to transfer all headers in one
time. When the current block is a start-line, we check if there is enough space
to transfer all headers too. If not, and if the destination is empty, a parsing
error is reported on the source.
The H2 multiplexer is the only one to use this function. When a parsing error is
reported during the transfer, the flag CS_FL_EOI is also set on the conn_stream.
MINOR: htx: Add a field to set the memory used by headers in the HTX start-line
The field hdrs_bytes has been added in the structure htx_sl. It should be used
to set how many bytes are help by all headers, from the start-line to the
corresponding EOH block. it must be set to -1 if it is unknown.
MINOR: stream-int: Don't use the flag CO_RFL_KEEP_RSV anymore in si_cs_recv()
Because the channel_recv_max() always return the right value, for HTX and legacy
streams, we don't need to set this flag. The multiplexer don't use it anymore.
MINOR: mux-h2: Use the count value received from the SI in h2_rcv_buf()
Now, the SI calls h2_rcv_buf() with the right count value. So we can rely on
it. Unlike the H1 multiplexer, it is fairly easier for the H2 multiplexer
because the HTX message already exists, we only transfer blocks from the H2S to
the channel. And this part is handled by htx_xfer_blks().
MEDIUM: mux-h1: Use the count value received from the SI in h1_rcv_buf()
Now, the SI calls h1_rcv_buf() with the right count value. So we can rely on
it. During the parsing, we now really respect this value to be sure to never
exceed it. To do so, once headers are parsed, we should estimate the size of the
HTX message before copying data.
BUG/MINOR: htx: Change htx_xfer_blk() to also count metadata
This patch makes the function more accurate. Thanks to the function
htx_get_max_blksz(), the transfer of data has been simplified. Note that now the
total number of bytes copied (metadata + payload) is returned. This slighly
change how the function is used in the H2 multiplexer.
This functions should be used to get the maximum size for a block, not exceeding
the max amount of bytes passed in argument. Thus max may be set to -1 to have no
limit.
MINOR: channel/htx: Call channel_htx_recv_max() from channel_recv_max()
When channel_recv_max() is called for an HTX stream, we fall back on the HTX
version. This function is called from si_cs_recv(). This will let us pass the
max amount of bytes to read to HTX multiplexers.
MEDIUM: http/htx: Perform analysis relatively to the first block
The first block is the start-line, if defined. Otherwise it the head of the HTX
message. So now, during HTTP analysis, lookup are all done using the first block
instead of the head. Concretely, for now, it is the same because only one HTTP
message is stored at a time in an HTX message. 1xx informational messages are
handled separatly from the final reponse and from each other. But it will make
sense when the 1xx informational messages and the associated final reponse will
be stored in the same HTX message.
MINOR: http/htx: Use sl_pos directly to replace the start-line
Since the HTX start-line is now referenced by position instead of by its payload
address, it is fairly easier to replace it. No need to search the rigth block to
find the start-line comparing the payloads address. It just enough to get the
block at the position sl_pos.
MINOR: htx: Replace the function http_find_stline() by http_get_stline()
Now, we only return the start-line. If not found, NULL is returned. No lookup is
performed and the HTX message is no more updated. It is now the caller
responsibility to update the position of the start-line to the right value. So
when it is not found, i.e sl_pos is set to -1, it means the last start-line has
been already processed and the next one has not been inserted yet.
It is mandatory to rely on this kind of warranty to store 1xx informational
responses and final reponse in the same HTX message.
MINOR: mux-h2/htx: Get the start-line from the head when HEADERS frame is built
in the H2 multiplexer, when a HEADERS frame is built before sending it, we have
the warranty the start-line is the head of the HTX message. It is safer to rely
on this fact than on the sl_pos value. For now, it's safe to use sl_pos in muxes
because HTTP 1xx messages are considered as full messages in HTX and only one
HTTP message can be stored at a time in HTX. But we are trying to handle 1xx
messages as a part of the reponse message. In this way, an HTTP reponse will be
the sum of all 1xx informational messages followed by the final response. So it
will be possible to have several start-line in the same HTX message. And the
sl_pos will point to the first unprocessed start-line from the analyzers point
of view.
MINOR: htx: Add functions to get the first block of an HTX message
It is the first block relatively to the start-line. So it is the start-line if
its position is set (sl_pos != -1), otherwise it is the head. The functions
htx_get_first() and htx_get_first_blk() can be used to get it. This change is
mandatory to consider 1xx informational messages as part of a response.
MINOR: htx: Store the head position instead of the wrap one
The head of an HTX message is heavily used whereas the wrap position is only
used when a block is added or removed. So it is more logical to store the head
position in the HTX message instead of the wrap one. The wrap position can be
easily deduced. To get it, the new function htx_get_wrap() may be used.
Willy Tarreau [Mon, 27 May 2019 17:31:06 +0000 (19:31 +0200)]
MEDIUM: config: now alert when two servers have the same name
We've been emitting warnings for over 5 years (since 1.5-dev22) about
configs accidently carrying multiple servers with the same name in the
same backend, and this starts to cause some real trouble in dynamic
environments since it's still very difficult to accurately process
a state-file and we still can't transport a server's name over the
peers protocol because of this.
It's about time to force users to fix their configs if they still
hadn't given that there is zero technical justification for doing this,
beyond the "yyp" (or copy-paste accident) when editing the config.
The message remains as clear as before, indicating the file and lines
of the conflict so that the user can easily fix it.
Willy Tarreau [Mon, 27 May 2019 15:37:20 +0000 (17:37 +0200)]
BUG/MEDIUM: threads: fix double-word CAS on non-optimized 32-bit platforms
On armv7 haproxy doesn't work because of the fixes on the double-word
CAS. There are two issues. The first one is that the last argument in
case of dwcas is a pointer to the set of value and not a value ; the
second is that it's not enough to cast the data as (void*) since it will
be a single word. Let's fix this by using the pointers as an array of
long. This was tested on i386, armv7, x86_64 and aarch64 and it is now
fine. An alternate approach using a struct was attempted as well but it
used to produce less optimal code.
This fix must be backported to 1.9. This fixes github issue #105.
Willy Tarreau [Mon, 27 May 2019 06:10:11 +0000 (08:10 +0200)]
BUG/MEDIUM: queue: fix the tree walk in pendconn_redistribute.
In pendconn_redistribute() we scan the queue using eb32_next() on the
node we've just deleted, which is wrong since the node is not in the
tree anymore, and it could dereference one node that has already been
released by another thread. Note that we cannot use eb32_first() in the
loop here instead because we need to skip pendconns having SF_FORCE_PRST.
Instead, let's keep a copy of the next node before deleting it.
In addition, the pendconn retrieved there is wrong, it uses &node as
the pointer instead of node, resulting in very quick crashes when the
server list is scanned.
Fortunately this only happens when "option redispatch" is used in
conjunction with "maxconn" on server lines, "cookie" for the stickiness,
and when a server goes down with entries in its queue.
This bug was introduced by commit 0355dabd7 ("MINOR: queue: replace
the linked list with a tree") so the fix must be backported to 1.9.
Willy Tarreau [Mon, 27 May 2019 08:17:05 +0000 (10:17 +0200)]
BUG/MAJOR: lb/threads: make sure the avoided server is not full on second pass
In fwrr_get_next_server(), we optionally pass a server to avoid. It
usually points to the current server during a redispatch operation. If
this server is usable, an "avoided" pointer is set and we continue to
look for another server. If in the end no other server is found, then
we fall back to this avoided one, which is still better than nothing.
The problem that may arise with threads is that in the mean time, this
avoided server might have received extra connections and might not be
usable anymore. This causes it to be queued a second time in the "full"
list and the loop to search for a server again, ending up on this one
again and so on.
This patch makes sure that we break out of the loop when we have to
pick the avoided server. It's probably what the code intended to do
as the current break statement causes fwrr_update_position() and
fwrr_dequeue_srv() to be called again on the avoided server.
It must be backported to 1.9 and 1.8, and seems appropriate for older
versions though it's unclear what the impact of this bug might be
there since the race doesn't exist and we're left with the double
update of the server's position.
Willy Tarreau [Mon, 27 May 2019 05:03:38 +0000 (07:03 +0200)]
MINOR: cli/activity: add 3 general purpose counters in development mode
The unused fd_del and fd_skip were being abused during debugging sessions
as general purpose event counters. With their removal, let's officially
have dedicated counters for such use cases. These counters are called
"ctr0".."ctr2" and are listed at the end when DEBUG_DEV is set.
Willy Tarreau [Sun, 26 May 2019 09:50:08 +0000 (11:50 +0200)]
BUILD: connections: shut up gcc about impossible out-of-bounds warning
Since commit 88698d9 ("MEDIUM: connections: Add a way to control the
number of idling connections.") when building without threads, gcc
complains that the operations made on the idle_orphan_conns[] list is
out of bounds, which is always false since 1) <i> can only equal zero,
and 2) given it's equal to <tid> we never even enter the loop. But as
usual it thinks it knows better, so let's mask the origin of this <i>
value to shut it up. Another solution consists in making <i> unsigned
and adding an explicit range check.
Willy Tarreau [Sun, 26 May 2019 08:08:28 +0000 (10:08 +0200)]
MAJOR: mux-h2: switch to next mux buffer on buffer full condition.
Now when we fail to send because the mux buffer is full, before giving
up and marking MFULL, we try to allocate another buffer in the mux's
ring to try again. Thanks to this (and provided there are enough buffers
allocated to the mux's ring), a single stream picked in the send_list
cannot steal all the mux's room at once. For this, we expand the ring
size to 31 buffers as it seems to be optimal on benchmarks since it
divides the number of context switches by 3. It will inflate each H2
conn's memory by 1 kB.
The bandwidth is now much more stable. Prior to this, it a test on
h2->h1 with very large objects (1 GB), a few tens of connections and
a few tens of streams per connection would show a varying performance
between 34 and 95 Gbps on 2 cores/4 threads, with h2_snd_buf() stopped
on a buffer full condition between 300000 and 600000 times per second.
Now the performance is constantly between 88 and 96 Gbps. Measures show
that buffer full conditions are met around only 159 times per second
in this case, or rougly 2000 to 4000 times less often.
Willy Tarreau [Sun, 26 May 2019 08:05:50 +0000 (10:05 +0200)]
CLEANUP: mux-h2: consistently use a local variable for the mbuf
This makes the code more readable and reduces the calls to br_tail().
In addition, all calls to h2_get_buf() are now made via this local
variable, which should significantly help for retries.
Willy Tarreau [Sun, 26 May 2019 07:49:17 +0000 (09:49 +0200)]
MEDIUM: mux-h2: make the send() function iterate over all mux buffers
Now send() uses a loop to iterate over all buffers to be sent. These
buffers are released and deleted from the vector once completely sent.
If any buffer gets released, offer_buffers() is called to wake up some
waiters.
Willy Tarreau [Sun, 26 May 2019 07:38:07 +0000 (09:38 +0200)]
MEDIUM: mux-h2: replace all occurrences of mbuf with a buffer ring
For now it's only one buffer long so the head and tails are always the
same, thus it doesn't change what used to work. In short, br_tail(h2c->mbuf)
was inserted everywhere we used to have h2c->mbuf.
Willy Tarreau [Fri, 24 May 2019 12:55:06 +0000 (14:55 +0200)]
MINOR: buffer: add a new buffer ring API to manipulate rings of buffers
The purpose is to manipulate rings made of series of buffers so that
it is possible to continue to work on a next buffer once one is full.
This will be used by muxes to deal with contention between multiple
streams and a single output buffer. No data is expected to span over
multiple buffers, all of them will be used like a regular buffer. This
will significantly limit the amount of changes and the code complexity
while still supporting larger output buffering.
The ring is made of a head and a tail indexes both of which point to a
buffer descriptor. At least one descriptor is always valid, so it could
be seen as a form of pagination always presenting one buffer. The root
of the ring is itself stored into a buffer descriptor so that the user
only has to declare a buffer array and to call br_init() on it in order
to use it.
Willy Tarreau [Sun, 26 May 2019 07:25:59 +0000 (09:25 +0200)]
CLEANUP: debug: remove the TRACE() macro
It has not been used for many years, is unlikely to be reused and
conflicts with the similarly named macro in flt_trace, causing warnings
at build time when including debug.h in low-level files. Let's simply
remove it.