]> git.ipfire.org Git - thirdparty/haproxy.git/log
thirdparty/haproxy.git
6 years agoBUG/MINOR: threads: Handle nbthread == MAX_THREADS.
Olivier Houchard [Fri, 27 Jul 2018 15:06:59 +0000 (17:06 +0200)] 
BUG/MINOR: threads: Handle nbthread == MAX_THREADS.

If nbthread is MAX_THREADS, the shift operation needed to compute
all_threads_mask fails in thread_sync_init(). Instead pass a number
of threads to this function and let it compute the mask without
overflowing.

This should be backported to 1.8.

6 years agoBUILD/MINOR: threads: unbreak build with threads disabled
Willy Tarreau [Fri, 27 Jul 2018 15:14:41 +0000 (17:14 +0200)] 
BUILD/MINOR: threads: unbreak build with threads disabled

Depending on the optimization level, gcc may complain that wake_thread()
uses an invalid array index for poller_wr_pipe[] when called from
__task_wakeup(). Normally the condition to get there never happens,
but it's simpler to ifdef out this part of the code which is only
used to wake other threads up. No backport is needed, this was brought
by the recent introduction of the ability to wake a sleeping thread.

6 years agoBUG/MINOR: config: stick-table is not supported in defaults section
Willy Tarreau [Fri, 27 Jul 2018 08:26:22 +0000 (10:26 +0200)] 
BUG/MINOR: config: stick-table is not supported in defaults section

Thierry discovered that the following config crashes haproxy while
parsing the config (it's probably the smallest crasher) :

   defaults
       stick-table type ip size 1M

And indeed it does because it looks for the current proxy's name which it
does not have as it's the default one. This affects all versions since 1.6.

This fix must be backported to all versions back to 1.6.

6 years agoBUG/MEDIUM: h2: prevent orphaned streams from blocking a connection forever
Willy Tarreau [Fri, 27 Jul 2018 07:55:14 +0000 (09:55 +0200)] 
BUG/MEDIUM: h2: prevent orphaned streams from blocking a connection forever

Some h2 connections remaining in CLOSE_WAIT state forever have been
reported for a while. Thanks to detailed captures provided by Milan
Petruzelka, the sequence where this happens became clearer :

  1) multiple streams compete for the mux and are queued in the send_list

  2) at this point the mux has to emit a GOAWAY for any reason (for
     example because it received a bad message)

  3) the streams are woken up, notified about the error

  4) h2_detach() is called for each of them

  5) the CS they are detached from the H2S

  6) since the streams are marked as blocked for some room, they are
     orphaned and nothing more is done on them.

  7) at this point, any activity on the connection goes through h2_wake()
     which sees the conneciton in ERROR2 state, tries again to release
     the streams, cannot, and stops polling (thus even connection errors
     cannot be detected anymore).

=> from this point, no more events can be received on the connection, and
   the streams remain orphaned forever.

This patch makes sure that we never return without doing anything once
an error was met. It has to act both on the h2_detach() side (for h2
streams being detached after the error was emitted) and on the h2_wake()
side (for errors reported after h2s have already been orphaned).

Many thanks to Milan Petruzelka and Janusz Dziemidowicz for their
awesome work on this issue, collecting traces and testing patches,
and to Olivier Doucet for extra testing and confirming the fix.

This fix must be backported to 1.8.

6 years agoMINOR: ssl: BoringSSL matches OpenSSL 1.1.0
Emmanuel Hocdet [Mon, 18 Jun 2018 09:04:19 +0000 (11:04 +0200)] 
MINOR: ssl: BoringSSL matches OpenSSL 1.1.0

Since BoringSSL 3b2ff028, API now correctly match OpenSSL 1.1.0.
The patch revert part of haproxy 019f9b10: "Fix BoringSSL call and
openssl-compat.h/#define occordingly.".
This will not break openssl/libressl compat.

6 years agoBUG/MEDIUM: threads/sync: use sched_yield when available
Willy Tarreau [Fri, 27 Jul 2018 05:47:24 +0000 (07:47 +0200)] 
BUG/MEDIUM: threads/sync: use sched_yield when available

There is a corner case with the sync point which can significantly
degrade performance. The reason is that it forces all threads to busy
spin there, and that if there are less CPUs available than threads,
this busy activity from some threads will force others to wait longer
in epoll() or to simply be scheduled out while doing something else,
and will increase the time needed to reach the sync point.

Given that the sync point is not expected to be stressed *that* much,
better call sched_yield() while waiting there to release the CPU and
offer it to waiting threads.

On a simple test with 4 threads bound to two cores using "maxconn 1"
on the server line, the performance was erratic before the recent
scheduler changes (between 40 and 200 conn/s with hundreds of ms
response time), and it jumped to 7200 with 12ms response time with
this fix applied.

It should be backported to 1.8 since 1.8 is affected as well.

6 years agoMINOR: threads/queue: Get rid of THREAD_WANT_SYNC in the queue code.
Olivier Houchard [Thu, 26 Jul 2018 16:47:27 +0000 (18:47 +0200)] 
MINOR: threads/queue: Get rid of THREAD_WANT_SYNC in the queue code.

Now that we can wake one thread sleeping in the poller, we don't have to
use THREAD_WANT_SYNC any more.

This gives a significant performance boost on highly contended accesses
(servers with maxconn 1), showing a jump from 21k to 31k conn/s on a
test involving 8 threads.

6 years agoMINOR: pollers: Add a way to wake a thread sleeping in the poller.
Olivier Houchard [Thu, 26 Jul 2018 15:55:11 +0000 (17:55 +0200)] 
MINOR: pollers: Add a way to wake a thread sleeping in the poller.

Add a new pipe, one per thread, so that we can write on it to wake a thread
sleeping in a poller, and use it to wake threads supposed to take care of a
task, if they are all sleeping.

6 years agoMINOR: tasks: Make global_tasks_mask volatile.
Olivier Houchard [Thu, 26 Jul 2018 16:53:28 +0000 (18:53 +0200)] 
MINOR: tasks: Make global_tasks_mask volatile.

In order to make sure modifications are noticed by other threads when needed,
make global_tasks_mask volatile.

6 years agoMINOR: tasks: Make active_tasks_mask volatile.
Olivier Houchard [Thu, 26 Jul 2018 16:45:22 +0000 (18:45 +0200)] 
MINOR: tasks: Make active_tasks_mask volatile.

To be sure we have the relevant informations, make active_tasks_mask volatile

6 years agoMEDIUM: queue: get rid of the pendconn lock
Willy Tarreau [Thu, 26 Jul 2018 06:23:24 +0000 (08:23 +0200)] 
MEDIUM: queue: get rid of the pendconn lock

This lock was necessary to manipulate the pendconn element between
concurrent places, but was causing great difficulties in the list walk
by having to iterate over multiple entries instead of being able to
safely pick the first one (in fact the first element was always the
right one but the locking model was hard to prove).

Here since we know we can always rely on the queue's locks, we take
the queue's lock every time we need to modify the element. In practice
it was already the case everywhere except in pendconn_dequeue() which
only works on an element that was already detached. This function had
to be protected against the risk of meeting an incompletely detached
element (which could be unlinked but not yet assigned). By taking the
queue lock around the LIST_ISEMPTY test, it's enough to ensure that a
concurrent thread either didn't begin or had completed the operation.

The true benefit really is in pendconn_process_next_strm() where we
can again safely work with the first element of each queue. This will
significantly simplify next updates to this code.

6 years agoMINOR: queue: implement pendconn queue locking functions
Willy Tarreau [Thu, 26 Jul 2018 06:03:14 +0000 (08:03 +0200)] 
MINOR: queue: implement pendconn queue locking functions

The new pendconn_queue_lock() and pendconn_queue_unlock() functions are
made to make it more convenient to lock or unlock the pendconn queue
either at the proxy or the server depending on pendconn->srv. This way
it is possible to remove the open-coding of these locks at various places.
These ones have been used in pendconn_unlink() and pendconn_add(), thus
significantly simplifying the logic there.

6 years agoMINOR: queue: use a distinct variable for the assigned server and the queue
Willy Tarreau [Thu, 26 Jul 2018 05:38:54 +0000 (07:38 +0200)] 
MINOR: queue: use a distinct variable for the assigned server and the queue

The pendconn struct uses ->px and ->srv to designate where the element is
queued. There is something confusing regarding threads though, because we
have to lock the appropriate queue before inserting/removing elements, and
this queue may only be determined by looking at ->srv (if it's not NULL
it's the server, otherwise use the proxy). But pendconn_grab_from_px() and
pendconn_process_next_strm() both assign this ->srv field, making it
complicated to know what queue to lock before manipulating the element,
which is exactly why we have the pendconn_lock in the first place.

This commit introduces pendconn->target which is the target server that
the two aforementioned functions will set when assigning the server.
Thanks to this, the server pointer may always be relied on to determine
what queue to use.

6 years agoMINOR: queue: make sure pendconn->strm->pend_pos is always valid
Willy Tarreau [Thu, 26 Jul 2018 05:33:44 +0000 (07:33 +0200)] 
MINOR: queue: make sure pendconn->strm->pend_pos is always valid

pendconn_add() used to assign strm->pend_pos very late, after unlocking
the queue, so that a watching thread could see a random value in
pendconn->strm->pend_pos even while holding the lock on the element and
the queue itself. While there's currently nothing wrong with this, it
costs nothing to arrange it and will simplify code analysis later.

6 years agoDOC: queue: document the expected locking model for the server's queue
Willy Tarreau [Wed, 25 Jul 2018 13:21:00 +0000 (15:21 +0200)] 
DOC: queue: document the expected locking model for the server's queue

The locking model is not trivial and is worth documenting to avoid seeing
apparent bugs everywhere while they are not.

6 years agoMEDIUM: queue: make pendconn_free() work on the stream instead
Willy Tarreau [Wed, 25 Jul 2018 09:13:53 +0000 (11:13 +0200)] 
MEDIUM: queue: make pendconn_free() work on the stream instead

Now pendconn_free() takes a stream, checks that pend_pos is set, clears
it, and uses pendconn_unlink() to complete the job. It's cleaner and
centralizes all the bookkeeping work in pendconn_unlink() only and
ensures that there's a single place where the stream's position in the
queue is manipulated.

6 years agoMINOR: queue: centralize dequeuing code a bit better
Willy Tarreau [Wed, 25 Jul 2018 06:04:20 +0000 (08:04 +0200)] 
MINOR: queue: centralize dequeuing code a bit better

For now the pendconns may be dequeued at two places :
  - pendconn_unlink(), which operates on a locked queue
  - pendconn_free(), which operates on an unlocked queue and frees
    everything.

Some changes are coming to the queue and we'll need to be able to be a
bit stricter regarding the places where we dequeue to keep the accounting
accurate. This first step renames the locked function __pendconn_unlink()
as it's for use by those aware of it, and introduces a new general purpose
pendconn_unlink() function which automatically grabs the necessary locks
before calling the former, and pendconn_cond_unlink() which additionally
checks the pointer and the presence in the queue.

6 years agoBUG/MEDIUM: tasks: make __task_unlink_rq responsible for the rqueue size.
Olivier Houchard [Thu, 26 Jul 2018 13:59:38 +0000 (15:59 +0200)] 
BUG/MEDIUM: tasks: make __task_unlink_rq responsible for the rqueue size.

As __task_wakeup() is responsible for increasing
rqueue_local[tid]/global_rqueue_size, make __task_unlink_rq responsible for
decreasing it, as process_runnable_tasks() isn't the only one that removes
tasks from runqueues.

6 years agoMINOR: tasks: Add a flag that tells if we're in the global runqueue.
Olivier Houchard [Thu, 26 Jul 2018 14:19:58 +0000 (16:19 +0200)] 
MINOR: tasks: Add a flag that tells if we're in the global runqueue.

How that we have bits available in task->state, add a flag that tells if we're
in the global runqueue or not.

6 years agoMINOR: tasks: extend the state bits from 8 to 16 and remove the reason
Willy Tarreau [Thu, 26 Jul 2018 14:13:00 +0000 (16:13 +0200)] 
MINOR: tasks: extend the state bits from 8 to 16 and remove the reason

By removing the reason code for the wakeup we can gain 8 extra bits to
encode the task's state. The reason code was never used at all and is
wrong by design since subsequent calls will OR this value anyway. Let's
say it goodbye and leave the room for more precious bits. The woken bits
were moved to the higher byte so that the most important bits can stay
grouped together.

6 years agoMINOR: signal: don't pass the signal number anymore as the wakeup reason
Willy Tarreau [Thu, 26 Jul 2018 14:11:33 +0000 (16:11 +0200)] 
MINOR: signal: don't pass the signal number anymore as the wakeup reason

This is never used and would even be wrong since the reasons are ORed
so two signals would be turned into a third value, just like if any
other reason was used at the same time.

6 years agoBUG/MEDIUM: tasks: Make sure there's no task left before considering inactive.
Olivier Houchard [Thu, 26 Jul 2018 13:25:49 +0000 (15:25 +0200)] 
BUG/MEDIUM: tasks: Make sure there's no task left before considering inactive.

We may remove the thread's bit in active_tasks_mask despite tasks for that
thread still being present in the global runqueue. To fix that, introduce
global_tasks_mask, and set the correspnding bits when we add a task to the
runqueue.

6 years agoBUG/MEDIUM: tasks: use atomic ops for active_tasks_mask
Willy Tarreau [Thu, 26 Jul 2018 13:16:43 +0000 (15:16 +0200)] 
BUG/MEDIUM: tasks: use atomic ops for active_tasks_mask

We don't have the lock anymore so we need to protect it.

6 years agoBUG/MEDIUM: tasks: Decrement rqueue_size at the right time.
Olivier Houchard [Thu, 26 Jul 2018 12:57:49 +0000 (14:57 +0200)] 
BUG/MEDIUM: tasks: Decrement rqueue_size at the right time.

We need to decrement requeue_size when we remove a task form rqueue_local,
not when we remove if from the task list, or we'd also decrement it for any
tasklet, that was never in the rqueue in the first place.

6 years agoBUG/MEDIUM: tasks: make sure we pick all tasks in the run queue
Willy Tarreau [Thu, 26 Jul 2018 11:36:01 +0000 (13:36 +0200)] 
BUG/MEDIUM: tasks: make sure we pick all tasks in the run queue

Commit 09eeb76 ("BUG/MEDIUM: tasks: Don't forget to increase/decrease
tasks_run_queue.") addressed a count issue in the run queue and uncovered
another issue with the way the tasks are dequeued from the global run
queue. The number of tasks to pick is computed using an integral divide,
which results in up to nbthread-1 tasks never being run. The fix simply
consists in getting rid of the divide and checking the task count in the
loop.

No backport is needed, this is 1.9-specific.

6 years agoBUG/MINOR: servers: Don't make "server" in a frontend fatal.
Olivier Houchard [Tue, 24 Jul 2018 14:48:59 +0000 (16:48 +0200)] 
BUG/MINOR: servers: Don't make "server" in a frontend fatal.

When parsing the configuration, if "server", "default-server" or
"server-template" are found in a frontend, we first warn that it will be
ignored, only to be considered a fatal error later. Be true to our word, and
just ignore it.

This should be backported to 1.8 and 1.7.

6 years agoBUG/MEDIUM: stats: don't ask for more data as long as we're responding
Willy Tarreau [Tue, 24 Jul 2018 15:05:54 +0000 (17:05 +0200)] 
BUG/MEDIUM: stats: don't ask for more data as long as we're responding

The stats applet is still a bit hackish. It uses the HTTP txn to parse
the POST contents. Due to this it pretends not having parsed the request
from the buffer so that the HTTP parser continues to work fine on these
data. This comes with a side effect : the request lies pending in the
channel's buffer, and because of this, stream_int_update_applet() always
wakes the applet up. It's very visible when retrieving a large stats page
over a slow link as haproxy eats 100% of the CPU waiting for the data to
leave.

While the proper long term solution definitely is to consume these data
and parse the body from the applet, changing this is not suitable for a
fix.

What this patch does instead is to disable request polling as long as there
are pending data in the response buffer. Given that for almost all cases,
the applet remains busy sending data, this is at least enough to ensure
that we don't wake up for the pending request data while we're waiting for
the client to receive these data. Now a 5k backend stats page is dumped at
1% CPU over a 10 Mbps link instead of 100%, using 1500 epoll_wait() calls
instead of 80000.

Note that the previous fix (BUG/MEDIUM: stream-int: don't immediately
enable reading when the buffer was reportedly full) is necessary for the
effects of the fix to be noticed since both bugs have the exact same
effect.

This fix must be backported at least as far as 1.5.

6 years agoBUG/MEDIUM: stream-int: don't immediately enable reading when the buffer was reported...
Willy Tarreau [Tue, 24 Jul 2018 14:56:34 +0000 (16:56 +0200)] 
BUG/MEDIUM: stream-int: don't immediately enable reading when the buffer was reportedly full

There is a long-time issue which affects some applets, at least the stats
applet. If a large stats page is read over a slow link, regularly the
channel's buffer contains too many response data to allow another round
of ci_putblk() to copy a new message. In this case the applet calls
si_applet_cant_put() to mention that it failed to emit data into the
channel's buffer, and wants to be called only once some room is made.

The problem is that stream_int_update(), which is called from
process_stream(), will clear this flag whenever it sees there's some
spare room in the channel's buffer. It causes the applet to be woken
again immediately. This is very visible when reading a large stats
page over a slow link, because in this case haproxy will run at 100%
CPU and strace shows mostly epoll_wait(0). It is very likely that some
other applets like CLI, Lua, peers or SPOE have also been affected but
that the effect were less noticeable because it was mixed with traffic.

Ideally stream_int_update() should not touch these flags, but changing
this would require a very careful auditing of all users. Instead here
what we do is that we respect the flag if the channel still has output
data. This way the flag will automatically disappear once the buffer is
empty, and the applet function will be called only when input data
remains, if at all.

This patch alone is not enough to observe the behaviour change on the
stats page because another bug takes over, addressed by next patch
(BUG/MEDIUM: stats: don't ask for more data as long as we're responding).
When both are applied, dumping stats for 5k backends over a 10 Mbps link
take 1% CPU instead of 100%, with 1.5k epoll_wait() calls instead of 80k.

This fix should be backported at least as far as 1.5.

6 years agoMINOR: h2: add the error code and the max/last stream IDs to "show fd"
Willy Tarreau [Tue, 24 Jul 2018 12:12:42 +0000 (14:12 +0200)] 
MINOR: h2: add the error code and the max/last stream IDs to "show fd"

This is intented to help debugging H2 in field.

6 years agoDOC: add more design feedback on the new layering model
Willy Tarreau [Mon, 23 Jul 2018 15:29:37 +0000 (17:29 +0200)] 
DOC: add more design feedback on the new layering model

Introduce the distinction between structured messages and raw data,
and how to make them coexist in a buffer. This is still a design draft.

6 years agoMEDIUM: h2: use the default conn_stream's receive function
Willy Tarreau [Fri, 2 Mar 2018 12:56:14 +0000 (13:56 +0100)] 
MEDIUM: h2: use the default conn_stream's receive function

This removes h2_rcv_buf() now that the generic code can handle it fine.

6 years agoMINOR: h2: make use of CS_FL_REOS to indicate that end of stream was seen
Willy Tarreau [Fri, 2 Mar 2018 11:26:37 +0000 (12:26 +0100)] 
MINOR: h2: make use of CS_FL_REOS to indicate that end of stream was seen

This allows h2_rcv_buf() not to depend anymore on h2s at all and to become
generic.

6 years agoMEDIUM: h2: don't call data_cb->recv() anymore
Willy Tarreau [Mon, 26 Feb 2018 19:11:24 +0000 (20:11 +0100)] 
MEDIUM: h2: don't call data_cb->recv() anymore

Now we simply call data_cb->wake() which will automatically perform the
recv() call if required.

6 years agoMEDIUM: h2: perform a single call to the data layer in demux()
Willy Tarreau [Mon, 26 Feb 2018 17:50:57 +0000 (18:50 +0100)] 
MEDIUM: h2: perform a single call to the data layer in demux()

Instead of calling the data layer from each individual frame processing
function, we now call it from demux. This requires to know the h2s that
was created inside h2c_frt_handle_headers(), which is why the pointer is
now returned. This results in a small performance boost from 58k to 60k
POST requests/s compared to -master, thanks to half the number of
si_cs_recv_cb() calls and 66% calls to si_cs_wake_cb().

It's interesting to note that all calls to data_cb->recv() are now always
immediately followed by a call to data_cb->wake(). The next step should
be to let the ->wake handler perform the recv() call itself. For this it
will be useful to have some info on the CS to indicate whether or not it
is ready to be read (ie: contains a non-empty input buffer).

6 years agoMEDIUM: buffers: make b_xfer() automatically swap buffers when possible
Willy Tarreau [Fri, 20 Jul 2018 16:58:51 +0000 (18:58 +0200)] 
MEDIUM: buffers: make b_xfer() automatically swap buffers when possible

Whenever it's possible to avoid a copy, b_xfer() will simply swap the
buffer's heads without touching the data. This has brought the performance
back from 140 kH/s to 202 kH/s on the test case.

6 years agoMEDIUM: h2: move headers and data frame decoding to their respective parsers
Willy Tarreau [Mon, 26 Feb 2018 14:59:07 +0000 (15:59 +0100)] 
MEDIUM: h2: move headers and data frame decoding to their respective parsers

Now we entirely process the input frame before transfering it above, so
that h2_rcv_buf() doesn't have to "speak" h2 anymore.

6 years agoMEDIUM: h2: centralize transfer of decoded frames in h2_rcv_buf()
Willy Tarreau [Mon, 26 Feb 2018 14:50:05 +0000 (15:50 +0100)] 
MEDIUM: h2: centralize transfer of decoded frames in h2_rcv_buf()

We still call the parser but it should soon not be needed anymore. The
decode functions don't need the buffer nor the max size anymore. They
must also not touch the CS_FL_EOS or CS_FL_RCV_MORE flags either, so
this is done within h2_rcv_buf() after transmission.

The "flags" argument to h2_frt_decode_headers() and h2_frt_transfer_data()
has been removed since it's not used anymore.

6 years agoMEDIUM: h2: make h2_frt_transfer_data() copy via an intermediary buffer
Willy Tarreau [Mon, 26 Feb 2018 14:44:54 +0000 (15:44 +0100)] 
MEDIUM: h2: make h2_frt_transfer_data() copy via an intermediary buffer

The purpose here is also to ensure we can split the lower from the top
layers. The way the CS_FL_MSG_MORE flag is set was updated so that it's
set or cleared upon exit depending on the buffer's remaining contents.

6 years agoMEDIUM: h2: make h2_frt_decode_headers() use an intermediary buffer
Willy Tarreau [Mon, 26 Feb 2018 14:22:17 +0000 (15:22 +0100)] 
MEDIUM: h2: make h2_frt_decode_headers() use an intermediary buffer

The purpose is to decode to a temporary buffer and then to copy this buffer
to the caller. This double-copy definitely has an impact on performance, the
test code goes down from 220k to 140k req/s, but this memcpy() will disappear
soon.

The test on CO_RFL_BUF_WET has become irrelevant now since we only use
the cs' rxbuf, so we cannot be blocked by "output" data that has to be
forwarded first. Thus instead we don't start until the rxbuf is empty
(it will be drained from any input data once the stream processes it).

6 years agoMINOR: h2: make each H2 stream support an intermediary input buffer
Willy Tarreau [Mon, 26 Feb 2018 14:22:17 +0000 (15:22 +0100)] 
MINOR: h2: make each H2 stream support an intermediary input buffer

The purpose is to decode to a temporary buffer and then to copy this buffer
to the caller upon request to avoid having to process frames on the fly
when called from the higher level. For now the buffer is only initialized
on stream creation via cs_new() and allocated if the buffer_wait's callback
is called.

6 years agoMEDIUM: stream-int: automatically call si_cs_recv_cb() if the cs has data on wake()
Willy Tarreau [Mon, 26 Feb 2018 19:08:13 +0000 (20:08 +0100)] 
MEDIUM: stream-int: automatically call si_cs_recv_cb() if the cs has data on wake()

If the cs has data pending or shutdown and the input channel is still
waiting for reads, let's simply call the recv() function from the wake()
callback. This will allow the lower layers to simply wake the upper one
up without having to consider the recv() nor anything else.

6 years agoMEDIUM: conn_stream: add cs_recv() as a default rcv_buf() function
Willy Tarreau [Fri, 2 Mar 2018 12:55:01 +0000 (13:55 +0100)] 
MEDIUM: conn_stream: add cs_recv() as a default rcv_buf() function

This function is generic and is able to automatically transfer data
from a conn_stream's rx buffer to the destination buffer. It does this
automatically if the mux doesn't define another rcv_buf() function.

6 years agoMINOR: conn_stream: add an rx buffer to the conn_stream
Willy Tarreau [Fri, 2 Mar 2018 09:43:58 +0000 (10:43 +0100)] 
MINOR: conn_stream: add an rx buffer to the conn_stream

In order to reorganize the connection layers, recv() operations will
need to be retryable and to support partial transfers. This requires
an intermediary buffer to hold the data coming from the mux. After a
few attempts, it turns out that this buffer is best placed inside the
conn_stream itself. For now it's only set to buf_empty and it will be
up to the caller to allocate it if required.

6 years agoMINOR: conn_stream: add a new CS_FL_REOS flag
Willy Tarreau [Fri, 2 Mar 2018 11:25:45 +0000 (12:25 +0100)] 
MINOR: conn_stream: add a new CS_FL_REOS flag

This flag indicates that the mux layer has already detected an end of
stream which will become CS_FL_EOS during a recv() once the rx buffer
is empty.

6 years agoDOC: add some design notes about the new layering model
Willy Tarreau [Wed, 21 Feb 2018 17:07:26 +0000 (18:07 +0100)] 
DOC: add some design notes about the new layering model

This explains how streams and connection should interact.

6 years agoMINOR: buffers: add b_xfer() to transfer data between buffers
Willy Tarreau [Fri, 20 Jul 2018 14:24:39 +0000 (16:24 +0200)] 
MINOR: buffers: add b_xfer() to transfer data between buffers

Instead of open-coding buffer-to-buffer transfers using blocks, let's
have a dedicated function for this. It also adjusts the buffer counts.

6 years agoMINOR: buffers: split b_putblk() into __b_putblk()
Willy Tarreau [Fri, 20 Jul 2018 14:20:34 +0000 (16:20 +0200)] 
MINOR: buffers: split b_putblk() into __b_putblk()

The latter function is more suited to operations that don't require any
check because the check has already been performed. It will be used by
other b_* functions.

6 years agoMINOR: buffers: simplify b_contig_space()
Willy Tarreau [Fri, 20 Jul 2018 14:07:42 +0000 (16:07 +0200)] 
MINOR: buffers: simplify b_contig_space()

This function is used a lot in block copies and is needlessly
complicated since it still uses pointer arithmetic. Let's fall
back to regular offsets and simplify it. This removed around
23 bytes from b_putblk() and it removed any conditional jump.

6 years agoBUG/MEDIUM: mux_h2: Call h2_send() before updating polling.
Olivier Houchard [Fri, 20 Jul 2018 16:15:23 +0000 (18:15 +0200)] 
BUG/MEDIUM: mux_h2: Call h2_send() before updating polling.

In h2_wake(), make sure we call h2_send() before we try to update the
polling flags, and detect connection errors, or errors will never be
detected.

6 years agoBUG/MEDIUM: threads: Fix the exit condition of the thread barrier
Christopher Faulet [Fri, 20 Jul 2018 07:31:53 +0000 (09:31 +0200)] 
BUG/MEDIUM: threads: Fix the exit condition of the thread barrier

In thread_sync_barrier, we exit when all threads have set their own bit in the
barrier mask. It is done by comparing it to all_threads_mask. But we must not
use a simple equality to do so, becaue all_threads_mask may change. Since commit
ba86c6c25 ("MINOR: threads: Be sure to remove threads from all_threads_mask on
exit"), when a thread exit, its bit is removed from all_threads_mask. Instead,
we must use a bitwise AND to test is all bits of all_threads_mask are set.

This also requires that all_threads_mask is set to volatile if we want to
catch changes.

This patch must be backported in 1.8.

6 years agoMINOR: ist: Add the function isteqi
Christopher Faulet [Wed, 6 Jun 2018 14:33:53 +0000 (16:33 +0200)] 
MINOR: ist: Add the function isteqi

This new function does the same as isteq, but ignoring the case.

6 years agoMINOR: debug: Add checks for conn_stream flags
Christopher Faulet [Thu, 1 Mar 2018 07:55:21 +0000 (08:55 +0100)] 
MINOR: debug: Add checks for conn_stream flags

This may be carefully backported to 1.8 (a few flags don't exist there).

6 years agoMINOR: debug: Add check for CO_FL_WILL_UPDATE
Christopher Faulet [Tue, 27 Feb 2018 14:41:10 +0000 (15:41 +0100)] 
MINOR: debug: Add check for CO_FL_WILL_UPDATE

This could be backported to 1.8.

6 years agoBUILD: Generate sha256 checksums in publish-release
Tim Duesterhus [Thu, 19 Jul 2018 21:57:56 +0000 (23:57 +0200)] 
BUILD: Generate sha256 checksums in publish-release

Currently only md5 signatures are generated. While md5
still is not broken with regard to preimage attacks, sha256
clearly is the current secure solution.

This patch should be backported to all supported branches.

6 years agoBUG/MINOR: build: Fix compilation with debug mode enabled
Christopher Faulet [Fri, 20 Jul 2018 08:16:29 +0000 (10:16 +0200)] 
BUG/MINOR: build: Fix compilation with debug mode enabled

It remained some fragments of the old buffers API in debug messages, here and
there.

This was caused by the recent buffer API changes, no backport is needed.

6 years agoBUG/MINOR: http: Set brackets for the unlikely macro at the right place
Christopher Faulet [Fri, 20 Jul 2018 07:54:26 +0000 (09:54 +0200)] 
BUG/MINOR: http: Set brackets for the unlikely macro at the right place

When test on the header "Early-Data" is made, the unlikely macro must encompass
the condition.

This patch must be backported in 1.8.

6 years agoMINOR: connection: simplify subscription by adding a registration function
Willy Tarreau [Wed, 18 Jul 2018 06:18:20 +0000 (08:18 +0200)] 
MINOR: connection: simplify subscription by adding a registration function

This new function wl_set_waitcb() prepopulates a wait_list with a tasklet
and a context and returns it so that it can be passed to ->subscribe() to
be added to a connection or conn_stream's wait_list. The caller doesn't
need to know all the insiders details anymore this way.

6 years agoMEDIUM: connections/mux: Revamp the send direction.
Olivier Houchard [Tue, 17 Jul 2018 16:49:38 +0000 (18:49 +0200)] 
MEDIUM: connections/mux: Revamp the send direction.

Totally nuke the "send" method, instead, the upper layer decides when it's
time to send data, and if it's not possible, uses the new subscribe() method
to be called when it can send data again.

6 years agoMINOR: connections/mux: Add a new "subscribe" method.
Olivier Houchard [Tue, 17 Jul 2018 16:46:31 +0000 (18:46 +0200)] 
MINOR: connections/mux: Add a new "subscribe" method.

Add a new "subscribe" method for connection, conn_stream and mux, so that
upper layer can subscribe to them, to be called when the event happens.
Right now, the only event implemented is "SUB_CAN_SEND", where the upper
layer can register to be called back when it is possible to send data.

The connection and conn_stream got a new "send_wait_list" entry, which
required to move a few struct members around to maintain an efficient
cache alignment (and actually this slightly improved performance).

6 years agoMINOR: tasklets: Don't attempt to add a tasklet in the list twice.
Olivier Houchard [Tue, 17 Jul 2018 16:29:22 +0000 (18:29 +0200)] 
MINOR: tasklets: Don't attempt to add a tasklet in the list twice.

Don't try to add a tasklet to the run queue if it's already in there, or we
might get an infinite loop.

6 years agoDOC: buffers: remove obsolete docs about buffers
Willy Tarreau [Wed, 18 Jul 2018 13:53:53 +0000 (15:53 +0200)] 
DOC: buffers: remove obsolete docs about buffers

A number of outdated docs dating 2012 about buffers implementation
and management were totally irrelevant to the current code (and even
to most 1.8 code as well). These docs have all been removed so that
only the up to date documentation remains.

6 years agoDOC: buffers: document the new buffers API
Willy Tarreau [Fri, 13 Jul 2018 16:49:41 +0000 (18:49 +0200)] 
DOC: buffers: document the new buffers API

Most of the new functions have been documented in buffer-api.txt, with
a diagram of what a buffer looks like and some hints to convert older
code.

6 years agoMAJOR: chunks: replace struct chunk with struct buffer
Willy Tarreau [Fri, 13 Jul 2018 09:56:34 +0000 (11:56 +0200)] 
MAJOR: chunks: replace struct chunk with struct buffer

Now all the code used to manipulate chunks uses a struct buffer instead.
The functions are still called "chunk*", and some of them will progressively
move to the generic buffer handling code as they are cleaned up.

6 years agoMEDIUM: chunks: make the chunk struct's fields match the buffer struct
Willy Tarreau [Fri, 13 Jul 2018 08:54:26 +0000 (10:54 +0200)] 
MEDIUM: chunks: make the chunk struct's fields match the buffer struct

Chunks are only a subset of a buffer (a non-wrapping version with no head
offset). Despite this we still carry a lot of duplicated code between
buffers and chunks. Replacing chunks with buffers would significantly
reduce the maintenance efforts. This first patch renames the chunk's
fields to match the name and types used by struct buffers, with the goal
of isolating the code changes from the declaration changes.

Most of the changes were made with spatch using this coccinelle script :

  @rule_d1@
  typedef chunk;
  struct chunk chunk;
  @@
  - chunk.str
  + chunk.area

  @rule_d2@
  typedef chunk;
  struct chunk chunk;
  @@
  - chunk.len
  + chunk.data

  @rule_i1@
  typedef chunk;
  struct chunk *chunk;
  @@
  - chunk->str
  + chunk->area

  @rule_i2@
  typedef chunk;
  struct chunk *chunk;
  @@
  - chunk->len
  + chunk->data

Some minor updates to 3 http functions had to be performed to take size_t
ints instead of ints in order to match the unsigned length here.

6 years agoMAJOR: buffer: finalize buffer detachment
Willy Tarreau [Tue, 10 Jul 2018 15:43:27 +0000 (17:43 +0200)] 
MAJOR: buffer: finalize buffer detachment

Now the buffers only contain the header and a pointer to the storage
area which can be anywhere. This will significantly simplify buffer
swapping and will make it possible to map chunks on buffers as well.

The buf_empty variable was removed, as now it's enough to have size==0
and area==NULL to designate the empty buffer (thus a non-allocated head
is the empty buffer by default). buf_wanted for now is indicated by
size==0 and area==(void *)1.

The channels and the checks now embed the buffer's head, and the only
pointer is to the storage area. This slightly increases the unallocated
buffer size (3 extra ints for the empty buffer) but considerably
simplifies dynamic buffer management. It will also later permit to
detach unused checks.

The way the struct buffer is arranged has proven quite efficient on a
number of tests, which makes sense given that size is always accessed
and often first, followed by the othe ones.

6 years agoMINOR: buffer: rename the data length member to '->data'
Willy Tarreau [Tue, 10 Jul 2018 08:43:27 +0000 (10:43 +0200)] 
MINOR: buffer: rename the data length member to '->data'

It used to be called 'len' during the reorganisation but strictly speaking
it's not a length since it wraps. Also we already use '_data' as the suffix
to count available data, and data is also what we use to indicate the amount
of data in a pipe so let's improve consistency here. It was important to do
this in two operations because data used to be the name of the pointer to
the storage area.

6 years agoMINOR: buffer: replace buffer_replace2() with b_rep_blk()
Willy Tarreau [Thu, 12 Jul 2018 13:55:34 +0000 (15:55 +0200)] 
MINOR: buffer: replace buffer_replace2() with b_rep_blk()

This one is more generic and designed to work on a random block. It
may later get a b_rep_ist() variant since many strings are already
available as (ptr,len).

6 years agoMINOR: buffers/channel: replace buffer_insert_line2() with ci_insert_line2()
Willy Tarreau [Thu, 12 Jul 2018 13:43:32 +0000 (15:43 +0200)] 
MINOR: buffers/channel: replace buffer_insert_line2() with ci_insert_line2()

There was no point keeping that function in the buffer part since it's
exclusively used by HTTP at the channel level, since it also automatically
appends the CRLF. This further cleans up the buffer code.

6 years agoCLEANUP: buffer: minor cleanups to buffer.h
Willy Tarreau [Tue, 10 Jul 2018 08:35:02 +0000 (10:35 +0200)] 
CLEANUP: buffer: minor cleanups to buffer.h

Remove a few unused functions and add some comments to split the file
parts in sections.

6 years agoMINOR: buffers: remove b_putstr()
Willy Tarreau [Thu, 12 Jul 2018 13:03:59 +0000 (15:03 +0200)] 
MINOR: buffers: remove b_putstr()

It's not needed anymore.

6 years agoMINOR: checks: use b_putist() instead of b_putstr()
Willy Tarreau [Thu, 12 Jul 2018 07:53:57 +0000 (09:53 +0200)] 
MINOR: checks: use b_putist() instead of b_putstr()

This slightly simplifies the code.

6 years agoMINOR: buffer: add a new file for ist + buffer manipulation functions
Willy Tarreau [Thu, 12 Jul 2018 07:02:47 +0000 (09:02 +0200)] 
MINOR: buffer: add a new file for ist + buffer manipulation functions

The new file istbuf.h links the indirect strings (ist) with the buffers.
The purpose is to encourage addition of more standard buffer manipulation
functions that rely on this in order to improve the overall ease of use
along all the code. Just like ist.h and buf.h, this new file is not
expected to depend on anything beyond these two files.

A few functions were added and/or converted from buffer.h :
  - b_isteq()  : indicates if a buffer and a string match
  - b_isteat() : consumes a string from the buffer if it matches
  - b_istput() : appends a small string to a buffer (all or none)
  - b_putist() : appends part of a large string to a buffer

The equivalent functions were removed from buffer.h and changed at the
various call places.

6 years agoMINOR: buffer: replace b{i,o}_put* with b_put*
Willy Tarreau [Tue, 10 Jul 2018 08:04:02 +0000 (10:04 +0200)] 
MINOR: buffer: replace b{i,o}_put* with b_put*

The two variants now do exactly the same (appending at the tail of the
buffer) so let's not keep the distinction between these classes of
functions and have generic ones for this. It's also worth noting that
b{i,o}_putchk() wasn't used at all and was removed.

6 years agoMINOR: buffer: replace bi_fast_delete() with b_del()
Willy Tarreau [Tue, 10 Jul 2018 07:59:31 +0000 (09:59 +0200)] 
MINOR: buffer: replace bi_fast_delete() with b_del()

There's no distinction between in and out data now. The latter covers
the needs of the former and supports wrapping. The extra cost is
negligible given the locations where it's used.

6 years agoMEDIUM: buffers: move "output" from struct buffer to struct channel
Olivier Houchard [Fri, 22 Jun 2018 17:26:39 +0000 (19:26 +0200)] 
MEDIUM: buffers: move "output" from struct buffer to struct channel

Since we never access this field directly anymore, but only through the
channel's wrappers, it can now move to the channel. The buffers are now
completely free from the distinction between input and output data.

6 years agoMINOR: buffer: rename the "data" field to "area"
Willy Tarreau [Mon, 9 Jul 2018 08:55:37 +0000 (10:55 +0200)] 
MINOR: buffer: rename the "data" field to "area"

Since we use "_data" for the amount of data at many places, as opposed to
"_space" for the amount of space, let's rename the "data" field to "area"
so that we can reuse "data" later for the amount of data in the buffer
(currently called "len" despite not being contigous).

6 years agoMINOR: buffer: b_set_data() doesn't truncate output data anymore
Willy Tarreau [Mon, 9 Jul 2018 09:43:36 +0000 (11:43 +0200)] 
MINOR: buffer: b_set_data() doesn't truncate output data anymore

b_set_data() is used :
  - in proto_http and hlua to trim input data (b_set_data(co_data()))
  - in SPOE to append data to a buffer while building a message

In no case will this truncate a buffer so we can safely remove the
test for len < b->output.

6 years agoMINOR: buffer: remove the check for output on b_del()
Willy Tarreau [Mon, 9 Jul 2018 09:39:49 +0000 (11:39 +0200)] 
MINOR: buffer: remove the check for output on b_del()

b_del() is used in :
  - mux_h2 with the demux buffer : always processes input data
  - checks with output data though output is not considered at all there
  - b_eat() which is not used anywhere
  - co_skip() where the len is always <= output

Thus the distinction for output data is not needed anymore and the
decrement can be made inconditionally in co_skip().

6 years agoMAJOR: start to change buffer API
Willy Tarreau [Fri, 29 Jun 2018 16:42:02 +0000 (18:42 +0200)] 
MAJOR: start to change buffer API

This is intentionally the minimal and safest set of changes, some cleanups
area still required. These changes are quite tricky and cannot be
independantly tested, so it's important to keep this patch as bisectable
as possible.

buf_empty and buf_wanted were changed and are now exactly similar since
there's no <p> member in the structure anymore. Given that no test is
ever made in the code to check that buf == &buf_wanted, it may be possible
that we don't need to have two anymore, unless some buf_empty tests have
precedence. This will have to be investigated.

A significant part of this commit affects the HTTP compression code,
which used to deeply manipulate the input and output buffers without
any reasonable solution for a better abstraction. For this reason, if
any regression is met and designates this patch as the culprit, it is
important to run tests which specifically involve compression or which
definitely don't use it in order to spot the issue.

Cc: Olivier Houchard <ohouchard@haproxy.com>
6 years agoMINOR: buffer: adapt buffer_slow_realign() and buffer_dump() to the new API
Willy Tarreau [Tue, 19 Jun 2018 05:48:13 +0000 (07:48 +0200)] 
MINOR: buffer: adapt buffer_slow_realign() and buffer_dump() to the new API

These are the last ones which need to be adapted.

6 years agoMINOR: buffer: convert part bo_putblk() and bi_putblk() to the new API
Willy Tarreau [Mon, 9 Jul 2018 08:32:29 +0000 (10:32 +0200)] 
MINOR: buffer: convert part bo_putblk() and bi_putblk() to the new API

These functions are pretty similar and will be merged at the end of the
migration. For now they still need to remain distinct.

6 years agoMINOR: lua: use the wrappers instead of directly manipulating buffer states
Willy Tarreau [Fri, 15 Jun 2018 16:07:57 +0000 (18:07 +0200)] 
MINOR: lua: use the wrappers instead of directly manipulating buffer states

This replaces chn->buf->p with ci_head(chn), chn->buf->o with co_data(chn)
and chn->buf->i with ci_data(chn). This is in order to help porting to the
new buffer API.

6 years agoMEDIUM: compression: start to move to the new buffer API
Olivier Houchard [Fri, 29 Jun 2018 16:16:31 +0000 (18:16 +0200)] 
MEDIUM: compression: start to move to the new buffer API

This part is tricky, it passes a channel where we used to have a buffer,
in order to reduce the API changes during the big switch. This way all
the channel's wrappers to distinguish between input and output are
available. It also makes sense given that the compression applies on
a channel since it's in the forwarding path.

6 years agoMINOR: flt_trace: adapt to the new buffer API
Willy Tarreau [Tue, 19 Jun 2018 05:22:43 +0000 (07:22 +0200)] 
MINOR: flt_trace: adapt to the new buffer API

The trace_hexdump() function now takes a count in argument to know where
to start dumping from.

6 years agoMEDIUM: h1: port to new buffer API.
Willy Tarreau [Tue, 19 Jun 2018 06:03:19 +0000 (08:03 +0200)] 
MEDIUM: h1: port to new buffer API.

The parser now uses the channel exclusively to access the data. In order
to avoid the cost of indirection, a local variable "input" was added to
the function that replaces buf->p. Given that this part is on the critical
path, it will have to be tested again for any visible performance loss.

6 years agoMINOR: payload: convert to the new buffer API
Willy Tarreau [Tue, 19 Jun 2018 05:19:56 +0000 (07:19 +0200)] 
MINOR: payload: convert to the new buffer API

Mostly mechanical changes. It seems that some of them could be further
factored out by adding a few more wrappers at the channel level.

6 years agoMINOR: filters: convert to the new buffer API
Willy Tarreau [Tue, 19 Jun 2018 05:16:31 +0000 (07:16 +0200)] 
MINOR: filters: convert to the new buffer API

Use b_set_data() to modify the buffer size, and use the usual wrappers.

6 years agoMEDIUM: http: use wrappers instead of directly manipulating buffers states
Willy Tarreau [Fri, 15 Jun 2018 16:31:02 +0000 (18:31 +0200)] 
MEDIUM: http: use wrappers instead of directly manipulating buffers states

This is aimed at easing the transition to the new API. There are a few places
which deserve some simplifications afterwards because ci_head() is called
often and may be placed into a local pointer.

6 years agoMINOR: backend: use new buffer API
Willy Tarreau [Tue, 19 Jun 2018 05:04:45 +0000 (07:04 +0200)] 
MINOR: backend: use new buffer API

The few locations dealing with the buffer rewind were updated not to
touch ->o nor ->p anymore and to use the channel's functions instead.

6 years agoMINOR: stream: use wrappers instead of directly manipulating buffers
Willy Tarreau [Fri, 15 Jun 2018 17:24:46 +0000 (19:24 +0200)] 
MINOR: stream: use wrappers instead of directly manipulating buffers

This will help transitioning to the new API. These changes are very
scarce limited.

6 years agoMINOR: stream-int: use the new buffer API
Willy Tarreau [Tue, 19 Jun 2018 05:03:14 +0000 (07:03 +0200)] 
MINOR: stream-int: use the new buffer API

A few locations still accessing ->i and ->o directly were changed to
use ci_data() and co_data() respectively. A call to b_del() was replaced
with co_set_data() in si_cs_send() so that ->o will is automatically be
decremented after the migration.

6 years agoMINOR: cache: use the new buffer API
Willy Tarreau [Tue, 19 Jun 2018 05:13:36 +0000 (07:13 +0200)] 
MINOR: cache: use the new buffer API

A few direct accesses to buf->p now use ci_head() instead.

6 years agoMINOR: cli: use the new buffer API
Willy Tarreau [Tue, 19 Jun 2018 05:01:36 +0000 (07:01 +0200)] 
MINOR: cli: use the new buffer API

Almost nothing required to be touched.

6 years agoMINOR: stats: adapt to the new buffers API
Willy Tarreau [Fri, 15 Jun 2018 17:41:31 +0000 (19:41 +0200)] 
MINOR: stats: adapt to the new buffers API

The changes are fairly straightforward. Some places require to trim
the length. Maybe we'd need a b_extend() or b_adjust() for this.

6 years agoMEDIUM: spoe: use the new buffer API for the SPOE buffer
Willy Tarreau [Fri, 15 Jun 2018 17:37:42 +0000 (19:37 +0200)] 
MEDIUM: spoe: use the new buffer API for the SPOE buffer

The buffer is not used as a forwarding buffer so we can simply map ->i
to ->len and ->p to b_head(). It *seems* that p is never modified, so
that we could even always use b_orig(). This needs to be rechecked.

6 years agoMINOR: buffer: remove unused bo_add()
Willy Tarreau [Mon, 9 Jul 2018 08:20:16 +0000 (10:20 +0200)] 
MINOR: buffer: remove unused bo_add()

We don't need this function anymore.

6 years agoMEDIUM: h2: update to the new buffer API
Willy Tarreau [Mon, 18 Jun 2018 11:33:09 +0000 (13:33 +0200)] 
MEDIUM: h2: update to the new buffer API

There is no more distinction between ->i and ->o for the mux's buffers,
we always use b_data() to know the buffer's length since only one side
is used for each direction.

6 years agoMINOR: checks: adapt to the new buffer API
Willy Tarreau [Mon, 18 Jun 2018 09:11:07 +0000 (11:11 +0200)] 
MINOR: checks: adapt to the new buffer API

The code exclusively used ->i for data received and ->o for data sent. Now
it always uses b_data(), b_head() and b_tail() so that there is no more
distinction between ->i and ->o.

6 years agoMEDIUM: channel: adapt to the new buffer API
Willy Tarreau [Tue, 19 Jun 2018 05:33:28 +0000 (07:33 +0200)] 
MEDIUM: channel: adapt to the new buffer API

Also, ci_swpbuf() was removed (unused).

6 years agoMINOR: channel: Add co_set_data().
Olivier Houchard [Fri, 29 Jun 2018 14:17:34 +0000 (16:17 +0200)] 
MINOR: channel: Add co_set_data().

Add a new function that lets one set the channel's output amount.