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.
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.
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.
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.
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
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.
This fix should be backported to 1.9 and 1.8.
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.
No backport is needed. This fixes GH issue #110.
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]
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.
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.
This should be backported to 1.9.
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.
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.
No backport is needed.
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.
This should be backported to 1.9.
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.
This should be backported to 1.9.
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.
This patch must be backported to 1.9.
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
Thus the contention was divided by 4.3.
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.
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.
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.
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 ]".
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.
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.
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.
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 :
frontend f1
bind :8011
mode http
http-request reject
http-request redirect location /
This trivial fix must be backported to 1.9 and 1.8. It is tracked by
github issue #107.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
