]> git.ipfire.org Git - thirdparty/haproxy.git/log
thirdparty/haproxy.git
6 years agoMAJOR: connections: Detach connections from streams.
Olivier Houchard [Tue, 13 Nov 2018 15:48:36 +0000 (16:48 +0100)] 
MAJOR: connections: Detach connections from streams.

Do not destroy the connection when we're about to destroy a stream. This
prevents us from doing keepalive on server connections when the client is
using HTTP/2, as a new stream is created for each request.
Instead, the session is now responsible for destroying connections.
When reusing connections, the attach() mux method is now used to create a new
conn_stream.

6 years agoMINOR: sessions: Start to store the outgoing connection in sessions.
Olivier Houchard [Tue, 13 Nov 2018 15:44:31 +0000 (16:44 +0100)] 
MINOR: sessions: Start to store the outgoing connection in sessions.

Introduce a new field in session, "srv_conn", and a linked list of sessions
in the connection. It will be used later when we'll switch connections
from being managed by the stream, to being managed by the session.

6 years agoMINOR: mux: Add a destroy() method.
Olivier Houchard [Tue, 6 Nov 2018 15:32:42 +0000 (16:32 +0100)] 
MINOR: mux: Add a destroy() method.

Add a new method to muxes, destroy(), that is responsible for destroying
the mux and the associated connection, to be used for server connections.

6 years agoMINOR: mux: Add a new "avail_streams" method.
Olivier Houchard [Mon, 5 Nov 2018 17:37:53 +0000 (18:37 +0100)] 
MINOR: mux: Add a new "avail_streams" method.

Add a new method for mux, avail_streams, that returns the number of streams
still available for a mux.
For the mux_pt, it'll return 1 if the connection is in idle, or 0. For
the H2 mux, it'll return the max number of streams allowed, minus the number
of streams currently in use.

6 years agoMEDIUM: mux: Teach the mux_pt how to deal with idle connections.
Olivier Houchard [Mon, 5 Nov 2018 17:28:43 +0000 (18:28 +0100)] 
MEDIUM: mux: Teach the mux_pt how to deal with idle connections.

In order to make the mux_pt able to handle idle connections, give it its
own context, where it'll stores the connection, the current conn_stream if
any, and a wait_event, so that it can subscribe to I/O events.
Add a new parameter to the detach() method, that gives the mux a hint
if it should destroy the connection or not when detaching a conn_stream.
If 1, then the mux_pt immediately destroys the connecion, if 0, then it
just subscribes to any read event. If a read happens, it will call
conn_sock_drain(), and if there's a connection error, it'll free the
connection, after removing it from the idle list.

6 years agoMEDIUM: connections: Wait until the connection is established to try to recv.
Olivier Houchard [Wed, 7 Nov 2018 16:55:19 +0000 (17:55 +0100)] 
MEDIUM: connections: Wait until the connection is established to try to recv.

Instead of trying to receive as soon as the connection is created, and to
eventually have to transfer subscription if we move connections, wait
until the connection is established before attempting to recv.

6 years agoMINOR: stream-int: replace si_cant_put() with si_rx_room_{blk,rdy}()
Willy Tarreau [Thu, 15 Nov 2018 10:08:52 +0000 (11:08 +0100)] 
MINOR: stream-int: replace si_cant_put() with si_rx_room_{blk,rdy}()

Remaining calls to si_cant_put() were all for lack of room and were
turned to si_rx_room_blk(). A few places where SI_FL_RXBLK_ROOM was
cleared by hand were converted to si_rx_room_rdy().

The now unused si_cant_put() function was removed.

6 years agoMEDIUM: stream-int: make use of si_rx_chan_{rdy,blk} to control the stream-int from...
Willy Tarreau [Wed, 14 Nov 2018 16:10:36 +0000 (17:10 +0100)] 
MEDIUM: stream-int: make use of si_rx_chan_{rdy,blk} to control the stream-int from the channel

The channel can disable reading from the stream-interface using various
methods, such as :
  - CF_DONT_READ
  - !channel_may_recv()
  - and possibly others

Till now this was done by mangling SI_FL_RX_WAIT_EP which is not
appropriate at all since it's not the stream interface which decides
whether it wants to deliver data or not. Some places were also wrongly
relying on SI_FL_RXBLK_ROOM since it was the only other alternative,
but it's not suitable for CF_DONT_READ.

Let's use the SI_FL_RXBLK_CHAN flag for this instead. It will properly
prevent the stream interface from being woken up and reads from
subscribing to more receipt without being accidently removed. It is
automatically reset if CF_DONT_READ is not set in stream_int_notify().

The code is not trivial because it splits the logic between everything
related to buffer contents (channel_is_empty(), CF_WRITE_PARTIAL, etc)
and buffer policy (CF_DONT_READ). Also it now needs to decide timeouts
based on any blocking flag and not just SI_FL_RXBLK_ROOM anymore.

It looks like this patch has caused a minor performance degradation on
connection rate, which possibly deserves being investigated deeper as
the test conditions are uncertain (e.g. slightly more subscribe calls?).

6 years agoMEDIUM: stream-int: unconditionally call si_chk_rcv() in update and notify
Willy Tarreau [Thu, 15 Nov 2018 06:46:57 +0000 (07:46 +0100)] 
MEDIUM: stream-int: unconditionally call si_chk_rcv() in update and notify

For a long time, stream_int_update() and stream_int_notify() used to only
conditionally call si_chk_rcv() based on state change detection. This
detection is not reliable and quite complex. With the new blocked flags
that si_chk_rcv() checks, it's much more reliable to always call the
function to take into account recent changes,and let it decide if it needs
to wake something up or not.

This also removes the calls to si_chk_rcv() that were performed in
si_update_both() since these ones are systematically performed in
stream_int_update() after updating the Rx flags.

6 years agoMEDIUM: stream-int: use si_rx_shut_blk() to indicate the SI is closed
Willy Tarreau [Wed, 14 Nov 2018 15:58:52 +0000 (16:58 +0100)] 
MEDIUM: stream-int: use si_rx_shut_blk() to indicate the SI is closed

Till now we were using si_done_put() upon shutr, but these flags could
be reset upon next activity. Now let's switch to SI_FL_RXBLK_SHUT which
doesn't go away. It's also set in stream_int_update() in case a shutr
condition is detected.

The now unused si_done_put() was removed.

6 years agoMEDIUM: stream-int: fix the si_cant_put() calls used for buffer readiness
Willy Tarreau [Thu, 15 Nov 2018 10:03:21 +0000 (11:03 +0100)] 
MEDIUM: stream-int: fix the si_cant_put() calls used for buffer readiness

A number of calls to si_cant_put() were used in fact to request being
called back once a buffer is available. These ones are not needed anymore
since si_alloc_ibuf() already sets the SI_FL_RXBLK_BUFF flag when called
in appctx context. Those called with a foreign stream-int are simply turned
to si_rx_buff_blk().

6 years agoMEDIUM: stream-int: fix the si_cant_put() calls used for end point readiness
Willy Tarreau [Thu, 15 Nov 2018 09:57:41 +0000 (10:57 +0100)] 
MEDIUM: stream-int: fix the si_cant_put() calls used for end point readiness

A number of si_cant_put() calls were still present to in fact indicate
that the end point is ready (thus should be turned to si_rx_endp_more()).

One other call in the Lua handler indicates that the endpoint wanted to
be blocked until some room is made in the Rx buffer in order to detect
that the connection happened, which is in fact an indication that it
wants to be called once the endpoint is ready, this is the default case
for an applet so this call was removed.

A useless call to si_cant_put() before appctx_wakeup() in the Lua
applet wakeup call was removed as well since the first thing that will
be done there will be to set end ENDP blocking flag.

6 years agoMINOR: stream-int: automatically mark applets as ready if they block on the channel
Willy Tarreau [Fri, 16 Nov 2018 15:18:34 +0000 (16:18 +0100)] 
MINOR: stream-int: automatically mark applets as ready if they block on the channel

If an applet reports being blocked due to any of the channel-side flags,
it's reportedly ready to deliver incoming data. It's better to do this
after the return from the applet handler so that applet developers don't
have to worry about details related to flags ordering.

6 years agoMINOR: stream-int: make si_sync_recv() simply check ENDP before si_cs_recv()
Willy Tarreau [Fri, 16 Nov 2018 13:26:11 +0000 (14:26 +0100)] 
MINOR: stream-int: make si_sync_recv() simply check ENDP before si_cs_recv()

Instead of checking complex conditions to call si_cs_recv() upon first
call, let's simply use si_rx_endp_ready() now that si_cs_recv() reports
it accurately, and add si_rx_blocked() to cover any blocking situation.

6 years agoMEDIUM: stream-int: update the endp polling status only at the end of si_cs_recv()
Willy Tarreau [Thu, 15 Nov 2018 15:55:14 +0000 (16:55 +0100)] 
MEDIUM: stream-int: update the endp polling status only at the end of si_cs_recv()

Instead of first indicating that there's more data to read from the
conn_stream then re-adjusting this info along the function, we now
instead set the status according to the subscription status at the
end. It's easier, more accurate, and less sensitive to intermediary
changes.

This will soon allow to remove all the si_cant_put() calls that were
placed in the middle to force a subsequent callback and prevent the
function from subscribing to the mux layer.

6 years agoMINOR: stream-int: replace si_{want,stop}_put() with si_rx_endp_{more,done}()
Willy Tarreau [Wed, 14 Nov 2018 16:54:13 +0000 (17:54 +0100)] 
MINOR: stream-int: replace si_{want,stop}_put() with si_rx_endp_{more,done}()

Here it's only a 1-to-1 replacement.

6 years agoMEDIUM: stream-int: use si_rx_buff_{rdy,blk} to report buffer readiness
Willy Tarreau [Wed, 14 Nov 2018 14:12:08 +0000 (15:12 +0100)] 
MEDIUM: stream-int: use si_rx_buff_{rdy,blk} to report buffer readiness

The stream interface used to conflate a missing buffer and lack of
buffer space into SI_FL_WAIT_ROOM but this causes difficulties as
these cannot be checked at the same moment and are not resolved at
the same moment either. Now we instead mark the buffer as presumably
available using si_rx_buff_rdy() and mark it as unavailable+requested
using si_rx_buff_blk().

The call to si_alloc_buf() was moved after si_stop_put(). This makes
sure that the SI_FL_RX_WAIT_EP flag is cleared on allocation failure so
that the function is called again if the callee fails to do its work.

6 years agoMINOR: stream-int: use si_rx_blocked()/si_tx_blocked() to check readiness
Willy Tarreau [Wed, 14 Nov 2018 13:07:59 +0000 (14:07 +0100)] 
MINOR: stream-int: use si_rx_blocked()/si_tx_blocked() to check readiness

This way we don't limit ourselves to random flags only and the code
is more readable and safer for the long term.

6 years agoMINOR: stream-int: replace SI_FL_WANT_PUT with !SI_FL_RX_WAIT_EP
Willy Tarreau [Wed, 14 Nov 2018 12:43:35 +0000 (13:43 +0100)] 
MINOR: stream-int: replace SI_FL_WANT_PUT with !SI_FL_RX_WAIT_EP

The SI_FL_WANT_PUT flag is used in an awkward way, sometimes it's
set by the stream-interface to mean "I have something to deliver",
sometimes it's cleared by the channel to say "I don't want you to
send what you have", and it has to be set back once CF_DONT_READ
is cleared. This will have to be split between SI_FL_RX_WAIT_EP
and SI_FL_RXBLK_CHAN. This patch only replaces all uses of the
flag with its natural (but negated) replacement SI_FL_RX_WAIT_EP.
The code is expected to be strictly equivalent. The now unused flag
was completely removed.

6 years agoMINOR: stream-int: add new functions si_{rx,tx}_{blocked,endp_ready}()
Willy Tarreau [Wed, 14 Nov 2018 13:01:40 +0000 (14:01 +0100)] 
MINOR: stream-int: add new functions si_{rx,tx}_{blocked,endp_ready}()

The first ones are used to figure if a direction is blocked on the
stream interface for anything but the end point. The second ones are
used to detect if the end point is ready to receive/transmit. They
should be used instead of directly fiddling with the existing bits.

6 years agoMINOR: stream-int: introduce new SI_FL_RXBLK flags
Willy Tarreau [Wed, 14 Nov 2018 10:24:08 +0000 (11:24 +0100)] 
MINOR: stream-int: introduce new SI_FL_RXBLK flags

The plan is to have the following flags to describe why a stream interface
doesn't produce data :

    - SI_FL_RXBLK_CHAN : the channel doesn't want it to receive
    - SI_FL_RXBLK_BUFF : waiting for a buffer allocation to complete
    - SI_FL_RXBLK_ROOM : more room is required in the channel to receive
    - SI_FL_RXBLK_SHUT : input now closed, nothing new will come
    - SI_FL_RX_WAIT_EP : waiting for the endpoint to produce more data

Applets like the CLI which consume complete commands at once and produce
large chunks of responses will for example be able to stop being woken up
by clearing SI_FL_WANT_GET and setting SI_FL_RXBLK_ROOM when the rx buffer
is full. Once called they will unblock WANT_GET. The flags were moved
together in readable form with the Rx bits using 2 hex digits and still
have some room to do a similar operation on the Tx path later, with the
WAIT_EP flag being represented alone on a digit.

6 years agoMINOR: stream-int: rename SI_FL_WAIT_ROOM to SI_FL_RXBLK_ROOM
Willy Tarreau [Wed, 14 Nov 2018 10:10:26 +0000 (11:10 +0100)] 
MINOR: stream-int: rename SI_FL_WAIT_ROOM to SI_FL_RXBLK_ROOM

This flag is not enough to describe all blocking situations, as can be
seen in each case we remove it. The muxes has taught us that using multiple
blocking flags in parallel will be much easier, so let's start to do this
now. This patch only renames this flags in order to make next changes more
readable.

6 years agoMINOR: stream-int: expand the flags to 32-bit
Willy Tarreau [Wed, 14 Nov 2018 09:53:42 +0000 (10:53 +0100)] 
MINOR: stream-int: expand the flags to 32-bit

We used to have enough of 16 bits, with 3 still available but it's
not possible to add the rx/tx blocking bits there. Let's extend the
format to 32 bits and slightly reorder the fields to maintain the
struct size to 64 bytes. Nothing else was changed.

6 years agoMINOR: stream-int: relax the forwarding rules in stream_int_notify()
Willy Tarreau [Sun, 18 Nov 2018 14:46:10 +0000 (15:46 +0100)] 
MINOR: stream-int: relax the forwarding rules in stream_int_notify()

There currently is an optimization in stream_int_notify() consisting
in not trying to forward small bits of data if extra data remain to be
processed. The purpose is to avoid forwarding one chunk at a time if
multiple chunks are available to be parsed at once. It consists in
avoiding sending pending output data if there are still data to be
parsed in the channel's buffer, since process_stream() will have the
opportunity to deal with them all at once.

Not only this optimization is less useful with the new way the connections
work, but it even causes problems like lost events since WAIT_ROOM will
not be removed. And with HTX, it will never be able to update the input
buffer after the first read.

Let's relax the rules now, by always sending if we don't have the
CF_EXPECT_MORE flag (used to group writes), or if the buffer is
already full.

6 years agoMINOR: stream-int: make conn_si_send_proxy() use cs_get_first()
Willy Tarreau [Sun, 18 Nov 2018 20:38:19 +0000 (21:38 +0100)] 
MINOR: stream-int: make conn_si_send_proxy() use cs_get_first()

The function used to abuse the internals of mux_pt to retrieve a
conn_stream, which will not work anymore after the idle connection
changes. Let's make it rely on the more reliable cs_get_first()
instead.

6 years agoMINOR: mux: implement a get_first_cs() method
Willy Tarreau [Sun, 18 Nov 2018 20:29:20 +0000 (21:29 +0100)] 
MINOR: mux: implement a get_first_cs() method

This method is used to retrieve the first known good conn_stream from
the mux. It will be used to find the other end of a connection when
dealing with the proxy protocol for example.

6 years agoCLEANUP: h2: minimum documentation for recent API changes
Willy Tarreau [Sun, 18 Nov 2018 05:30:59 +0000 (06:30 +0100)] 
CLEANUP: h2: minimum documentation for recent API changes

Commit d4dd22d ("MINOR: h2: Let user of h2_recv() and h2_send() know xfer
has been done") changed the API without documenting the expected returned
values which appear to come out of nowhere in the code :-(  Please don't
do that anymore! The description was recovered from the commit message.

6 years agoBUG/MINOR: config: Copy default error messages when parsing of a backend starts
Christopher Faulet [Mon, 12 Nov 2018 10:57:31 +0000 (11:57 +0100)] 
BUG/MINOR: config: Copy default error messages when parsing of a backend starts

To be used, error messages declared in a default section must be copied when the
parsing of a proxy section starts. But this was only done for frontends.

This patch may be backported to older versions.

6 years agoMINOR: stream: move the conn_stream specific calls to the stream-int
Willy Tarreau [Sat, 17 Nov 2018 18:51:07 +0000 (19:51 +0100)] 
MINOR: stream: move the conn_stream specific calls to the stream-int

There are still some unwelcome synchronous calls to si_cs_recv() in
process_stream(). Let's have a new function si_sync_recv() to perform
a synchronous receive call on a stream interface regardless of the type
of its endpoint, and move these calls there. For now it only implements
conn_streams since it doesn't seem useful to support applets there. The
function implements an extra check for the stream interface to be in an
established state before attempting anything.

6 years agoBUG/MINOR: stream-int: set SI_FL_WANT_PUT in sess_establish()
Willy Tarreau [Sat, 17 Nov 2018 18:14:35 +0000 (19:14 +0100)] 
BUG/MINOR: stream-int: set SI_FL_WANT_PUT in sess_establish()

In commit f26c26c ("BUG/MEDIUM: stream-int: change the way buffer room
is requested by a stream-int") we used to call si_want_put() at the
end of sess_update_st_con_tcp(), when switching to SI_ST_EST state.
But this is incorrect as there are a few other situations where we
can switch to this state, such as in si_connect() where a connection
reuse is detected, or when directly calling an applet (in which case
that was already covered anyway). For now it doesn't have any side
effect but it could impact connection reuse after the stream-int
changes by stalling an immediately reused connection.

Let's move this flag change to sess_establish() instead, which is the
only place which is always called exactly once on connection setup.

No backport is needed, this is purely 1.9.

6 years agoMEDIUM: cli: worker socketpair is unstoppable
William Lallemand [Fri, 16 Nov 2018 15:57:22 +0000 (16:57 +0100)] 
MEDIUM: cli: worker socketpair is unstoppable

In master-worker mode, the socketpair CLI listener of the worker is now
marked unstoppable, which allows to connect to the CLI of an old process
which is in a leaving state, allowing to debug it.

6 years agoMEDIUM: listeners: support unstoppable listener
William Lallemand [Fri, 16 Nov 2018 15:57:21 +0000 (16:57 +0100)] 
MEDIUM: listeners: support unstoppable listener

An unstoppable listener is a listener which won't be stop during a soft
stop. The unstoppable_jobs variable is incremented and the listener
won't prevent the process to leave properly.

It is not a good idea to use this feature (the LI_O_NOSTOP flag) with a
listener that need to be bind again on another process during a soft
reload.

6 years agoMEDIUM: jobs: support unstoppable jobs for soft stop
William Lallemand [Fri, 16 Nov 2018 15:57:20 +0000 (16:57 +0100)] 
MEDIUM: jobs: support unstoppable jobs for soft stop

This patch allows a process to properly quit when some jobs are still
active, this feature is handled by the unstoppable_jobs variable, which
must be atomically incremented.

During each new iteration of run_poll_loop() the break condition of the
loop is now (jobs - unstoppable_jobs) == 0.

The unique usage of this at the moment is to handle the socketpair CLI
of a the worker during the stopping of the process.  During the soft
stop, we could mark the CLI listener as an unstoppable job and still
handle new connections till every other jobs are stopped.

6 years agoBUG/MINOR: http: Be sure to sent fully formed HTTP 103 responses
Christopher Faulet [Thu, 15 Nov 2018 14:41:55 +0000 (15:41 +0100)] 
BUG/MINOR: http: Be sure to sent fully formed HTTP 103 responses

The previous commit fedceaf33 ("MINOR: http: Regroup return statements of
http_req_get_intercept_rule at the end") partly fixes the problem. But not
entierly. Because HTTP 103 reponses were sent line by line it is possible to mix
them with others. For instance, an early-hint rule followed by a redirect rule
leaving the response buffer totally messed up. Furthermore, if we fail to add
the last CRLF to finish the HTTP 103 response because there is no more space in
the buffer, it leave the buffer with an unfinished and invalid message.

This patch fixes the bug by creating a fully formed HTTP 103 response before
trying to push it in the response buffer. If an error occurred during the copy
or if another response was already sent, the HTTP 103 response is
ignored. However, the last point should never happened because, for redirects
and authentication errors, we first try to copy any pending HTTP 103 response.

6 years agoMINOR: http: Regroup return statements of http_res_get_intercept_rule at the end
Christopher Faulet [Thu, 15 Nov 2018 14:40:29 +0000 (15:40 +0100)] 
MINOR: http: Regroup return statements of http_res_get_intercept_rule at the end

Instead of having multiple return statements spreaded here and there in middle
of the function, we just exit from the loop setting the right return code. It
let a chance to do some work before leaving the function. It is also less error
prone.

6 years agoMINOR: http: Regroup return statements of http_req_get_intercept_rule at the end
Christopher Faulet [Thu, 15 Nov 2018 14:34:11 +0000 (15:34 +0100)] 
MINOR: http: Regroup return statements of http_req_get_intercept_rule at the end

Instead of having multiple return statements spreaded here and there in middle
of the function, we just exit from the loop setting the right return code. It
let a chance to do some work before leaving the function. It is also less error
prone.

6 years agoBUG/MINOR: http_fetch: Remove the version part when capturing the request uri
Christopher Faulet [Thu, 15 Nov 2018 13:35:18 +0000 (14:35 +0100)] 
BUG/MINOR: http_fetch: Remove the version part when capturing the request uri

This patch fixes a bug introduced in the commit 6b952c810 ("REORG: http: move
http_get_path() to http.c"). In the reorg, the code responsible to skip the
version to only extract the path in the HTTP request was dropped.

No backport is needed, this only affects 1.9.

6 years agoREGTEST: fix scripts 1 and 3 to accept development version
Willy Tarreau [Fri, 16 Nov 2018 14:54:23 +0000 (15:54 +0100)] 
REGTEST: fix scripts 1 and 3 to accept development version

These scripts were checking that the program's name was exactly "haproxy"
which clearly is not workable during development.

6 years agoCONTRIB: debug: fix build related to conn_stream flags change
Willy Tarreau [Fri, 16 Nov 2018 09:37:20 +0000 (10:37 +0100)] 
CONTRIB: debug: fix build related to conn_stream flags change

Commit 53216e7db ("MEDIUM: connections: Don't directly mess with the
polling from the upper layers.") removed the CS_FL_DATA_RD_ENA and
CS_FL_DATA_WR_ENA flags without updating flags.c, thus breaking the
build. This patch also adds flag CL_FL_NOT_FIRST which was brought
by commit 08088e77c.

6 years agoBUG/MINOR: stream-int: make sure not to go through the rcv_buf path after splice()
Willy Tarreau [Thu, 15 Nov 2018 15:06:02 +0000 (16:06 +0100)] 
BUG/MINOR: stream-int: make sure not to go through the rcv_buf path after splice()

When splice() reports a pipe full condition, we go through the common
code used to release a possibly empty pipe (which we don't have) and which
immediately tries to allocate a buffer that will never be used. Further,
it may even subscribe to get this buffer if the resources are low. Let's
simply get out of this way if the pipe is full.

This fix could be backported to 1.8 though the code is a bit different
overthere.

6 years agoBUG/MEDIUM: stream-int: clear CO_FL_WAIT_ROOM after splicing data in
Willy Tarreau [Thu, 15 Nov 2018 14:52:53 +0000 (15:52 +0100)] 
BUG/MEDIUM: stream-int: clear CO_FL_WAIT_ROOM after splicing data in

Since we don't necessarily pass through conn_fd_handler() when reading,
conn_refresh_polling_flags() is not necessarily called when performing
a recv() operation, thus flags like CO_FL_WAIT_ROOM are not cleared.

It happens that si_cs_recv() checks CO_FL_WAIT_ROOM before deciding to
receive into a buffer, to see if the previous rcv_pipe() call failed by
lack of pipe room. The combined effect of these two statements is that
at the end of a file transmission, when there's too little data to
warrant the use of a pipe and the pipe is empty, we refrain from using
rcv_pipe() for the last few bytes, but since CO_FL_WAIT_ROOM is still
present, we don't use rcv_buf() either, and the connection remains
frozen in this state with si_cs_recv() called in loops.

In order to fix this we can simply manually clear CO_FL_WAIT_ROOM when
not using pipe so that the next check sees the result of the previous
operation and not an old one. We could equally call
cond_refresh_polling_flags() but that would be overkill and dangerous
given that it would manipulate the connection's flags under the mux.
By the way ideally the mux should report this flag into the connstream
for cleaner manipulation.

No backport is needed as this is only post 1.9-dev2.

6 years agoBUG/MEDIUM: stream-int: make failed splice_in always subscribe to recv
Willy Tarreau [Thu, 15 Nov 2018 13:33:05 +0000 (14:33 +0100)] 
BUG/MEDIUM: stream-int: make failed splice_in always subscribe to recv

As part of the changes that went into 1.9-dev2 regarding the polling
modifications, the changes consecutive to the removal of the wait_list
from the conn_streams (commit 71384551a) made si_cs_recv() occasionally
return without subscribing to receive events, causing spliced transfers
to randomly fail if the client was at least as fast as the server. This
may remain unnoticed on most deployments since servers are usually close
to haproxy with higher bandwidth than clients have, resulting in buffers
always being full.

In order to reproduce his effect, it is better to do it on the local
machine and to transfer very large objects (hundreds of gigs) over a
single connection, to see it suddenly stall after a few tens of gigs.
Now with this fix it's fine even after 3 TB over a single connection.

No backport is needed.

6 years agoBUG/MEDIUM: Make sure stksess is properly aligned.
Olivier Houchard [Wed, 14 Nov 2018 16:54:36 +0000 (17:54 +0100)] 
BUG/MEDIUM: Make sure stksess is properly aligned.

When we allocate struct stksess, we also allocate memory to store the
associated data before the struct itself.
As the data can be of different types, they can have different size. However,
we need the struct stksess to be properly aligned, as it can do 64bits
load/store (including atomic load/stores) on 64bits platforms, and some of
them doesn't support unaligned access.
So, when allocating the struct stksess, round the size up to the next
multiple of sizeof(void *), and make sure the struct stksess itself is
properly aligned.
Many thanks to Paul Martin for investigating and reporting that bug.

This should be backported to earlier releases.

6 years agoBUG/MEDIUM: log: don't CLOEXEC the inherited FDs
William Lallemand [Tue, 13 Nov 2018 17:30:12 +0000 (18:30 +0100)] 
BUG/MEDIUM: log: don't CLOEXEC the inherited FDs

When configuring the logs with a FD and using the master worker, the FD
was closed upon a reload because it was configured with CLOEXEC. It
leads to using the wrong FD for the logs and to close them. Which is
unfortunate since the master rely on the FD left opened during a reload.

The fix is to stop doing a CLOEXEC when the FD is inherited.
No backport needed.

6 years agoMINOR: mworker: only close std{in,out,err} in daemon mode
William Lallemand [Tue, 13 Nov 2018 15:18:23 +0000 (16:18 +0100)] 
MINOR: mworker: only close std{in,out,err} in daemon mode

This allows to output messages when we are not in daemon mode which is
useful to use log stdout in master worker mode.

6 years agoDOC: early-hints: fix truncated line.
Frédéric Lécaille [Tue, 13 Nov 2018 08:42:13 +0000 (09:42 +0100)] 
DOC: early-hints: fix truncated line.

6 years agoMINOR: doc: Add information about "early-hint" http-request action.
Frédéric Lécaille [Mon, 12 Nov 2018 10:01:10 +0000 (11:01 +0100)] 
MINOR: doc: Add information about "early-hint" http-request action.

6 years agoMINOR: http: Implement "early-hint" http request rules.
Frédéric Lécaille [Mon, 12 Nov 2018 09:06:54 +0000 (10:06 +0100)] 
MINOR: http: Implement "early-hint" http request rules.

This patch implements http_apply_early_hint_rule() function is responsible of
building HTTP 103 Early Hint responses each time a "early-hint" rule is matched.

6 years agoMINOR: http: Make new "early-hint" http-request action really be parsed.
Frédéric Lécaille [Tue, 6 Nov 2018 13:30:19 +0000 (14:30 +0100)] 
MINOR: http: Make new "early-hint" http-request action really be parsed.

This patch adds a "early_hint" struct to "arg" union of "act_rule" struct
and parse "early-hint" http-request keyword with it using the same
code as for "(add|set)-header" parser.

6 years agoMINOR: http: Add new "early-hint" http-request action.
Frédéric Lécaille [Tue, 6 Nov 2018 09:55:34 +0000 (10:55 +0100)] 
MINOR: http: Add new "early-hint" http-request action.

This patch adds the new "early-hint" action to "http-request" rules parser.
This action should be parsed the same way as "(add|set)-header" actions.

6 years agoBUILD/MEDIUM: threads/affinity: DragonFly build fix
David Carlier [Mon, 12 Nov 2018 16:22:19 +0000 (16:22 +0000)] 
BUILD/MEDIUM: threads/affinity: DragonFly build fix

DragonFlyBSD does not have a build on its own, it has always
used the FreeBSD's. To be able to support the cpu affinity,
it needs few more headers.

6 years agoMINOR: namespaces: don't build namespace.c if disabled
Willy Tarreau [Sun, 11 Nov 2018 13:38:09 +0000 (14:38 +0100)] 
MINOR: namespaces: don't build namespace.c if disabled

When namespaces are disabled, support is still reported because the file
is built with almost nothing in it but built anyway. Instead of extending
the scope of the numerous ifdefs in this file, better avoid building it
when namespaces are diabled. In this case we define my_socketat() as an
inline function mapping directly to socket(). The struct netns_entry
still needs to be defined because it's used by various other functions
in the code.

6 years agoBUG/MEDIUM: stream-int: convert some co_data() checks to channel_is_empty()
Willy Tarreau [Mon, 12 Nov 2018 17:48:52 +0000 (18:48 +0100)] 
BUG/MEDIUM: stream-int: convert some co_data() checks to channel_is_empty()

Splicing was in great part broken over the last few development version
due to the use of co_data() to detect if data are available in the channel.
But co_data() only looks at buffered data, not spliced data.

Channel_is_empty() takes care of both and should be used. With this,
splicing restarts to work but there are still a few cases where transfers
may stall.

No backport is needed.

6 years agoBUG/MEDIUM: stream-int: change the way buffer room is requested by a stream-int
Willy Tarreau [Mon, 12 Nov 2018 15:11:08 +0000 (16:11 +0100)] 
BUG/MEDIUM: stream-int: change the way buffer room is requested by a stream-int

Subsequent to the recent stream-int updates, we started to consider that
SI_FL_WANT_PUT needs to be set when receipt is enabled, but this is wrong
and results in 100% CPU when an HTTP client stays idle after a keep-alive
request because the stream-int has nothing to provide and nothing to send.

In fact just like for applets this flag should reflect the continuation
of an attempt. So it's si_cs_recv() which should set the flag, and clear
it if it has nothing more to provide. This function is called the first
time in process_stream()), and called again during transfers, so it will
always be up to date during stream_int_update() and stream_int_notify().

As a special case, it should also be set when a connection switches to
the established state. And we should absolutely refrain from calling
si_cs_recv() to re-enable reading, normally just setting this flag
(from within the stream-int's handler or prior to calling si_chk_rcv())
is expected to be OK.

A corner case remains where it was observed that in stream_int_notify() we
can sometimes be called with an empty output channel with SI_FL_WAIT_ROOM
and no CF_WRITE_PARTIAL, so there's no way to detect that we should
re-enable receiving. It's easy to also take care of this condition
there for the time it takes to figure if this situation is expected
or not.

Now it becomes more obvious that relying on a single flag to request
room (or on two flags to arbiter activity) is not workable given the
autonomy of both sides. The mux_h2 has taught us that blocking flags
are much more reliable, require much less condition and are much easier
to deal with. That's probably something to consider quickly in this
area.

No backport is needed.

6 years agoMEDIUM: log: add a new "raw" format
Willy Tarreau [Mon, 12 Nov 2018 10:57:56 +0000 (11:57 +0100)] 
MEDIUM: log: add a new "raw" format

This format is pretty similar to the previous "short" format except
that it also removes the severity level. Thus only the raw message is
sent. This is suitable for use in containers, where only the raw
information is expected and where the severity is supposed to come
from the file descriptor used.

6 years agoMEDIUM: log: support a new "short" format
Willy Tarreau [Mon, 12 Nov 2018 07:45:00 +0000 (08:45 +0100)] 
MEDIUM: log: support a new "short" format

This format is meant to be used with local file descriptors. It emits
messages only prefixed with a level, removing all the process name,
system name, date and so on. It is similar to the printk() format used
on Linux. It's suitable to be sent to a local logger compatible with
systemd's output format.

Note that the facility is still required but not used, hence it is
suggested to use "daemon" to remind that it's a local logger.
Example :

    log stdout format short daemon          # send everything to stdout
    log stderr format short daemon notice   # send important events to stderr

6 years agoMEDIUM: log: add support for logging to existing file descriptors
Willy Tarreau [Mon, 12 Nov 2018 06:34:59 +0000 (07:34 +0100)] 
MEDIUM: log: add support for logging to existing file descriptors

In certain situations it would be desirable to log to an existing file
descriptor, the most common case being a pipe between containers or
processes. The main issue with pipes is that using write() on them will
randomly truncate messages. But there is a trick. By using writev(), we
can atomically deliver or drop a message, which perfectly fits the
purpose. The only caveat is that large messages (4096 bytes on modern
operating systems) may be interleaved with messages from other processes
if using nbproc for example. In practice such messages are rare and most
of the time when users need such type of logging, the load is low enough
for a single process to be running so this is not really a problem.

This logging method thus uses unbuffered writev() calls and is uses more
CPU than if it used its own buffer with large writes at once, though this
is not a problem for moderate loads.

Logging to a file descriptor attached to a file also works with the side
effect that the process is significantly slowed down during disk accesses
and that it's not possible to rotate the file without restarting the
process. For this reason this option is not offered as a configuration
option, since it would confuse most users, but one could decide to
redirect haproxy's output to a file during debugging sessions. Two aliases
"stdout" and "stderr" are provided, but keep in mind that these are closed
by default in daemon mode.

When logging to a pipe or socket at a high enough rate, some logs will be
dropped and the number of dropped messages is reported in "show info".

6 years agoMINOR: log: report the number of dropped logs in the stats
Willy Tarreau [Mon, 12 Nov 2018 06:25:28 +0000 (07:25 +0100)] 
MINOR: log: report the number of dropped logs in the stats

It's easy to detect when logs on some paths are lost as sendmsg() will
return EAGAIN. This is particularly true when sending to /dev/log, which
often doesn't support a big logging capacity. Let's keep track of these
and report the total number of dropped messages in "show info".

6 years agoDOC: logs: the format directive was missing from the second log part
Willy Tarreau [Mon, 12 Nov 2018 06:56:13 +0000 (07:56 +0100)] 
DOC: logs: the format directive was missing from the second log part

The "log" statement appears both in the global section and in listeners.
The "format" directive was only documented in the first one. Maybe we
should think about moving this definition to the log section by now.

6 years agoMINOR: log: slightly improve error message syntax on log failure
Willy Tarreau [Mon, 12 Nov 2018 06:00:11 +0000 (07:00 +0100)] 
MINOR: log: slightly improve error message syntax on log failure

The error messages used to say something along "socket logger 2 failed"
or "sendmsg logger 2 failed" which are confusing. Let's rephrase this
"sendmsg() failed for logger 2".

6 years agoDOC: Fix typos in README and CONTRIBUTING
Joseph Herlant [Sat, 10 Nov 2018 01:44:10 +0000 (17:44 -0800)] 
DOC: Fix typos in README and CONTRIBUTING

Few typos detected by misspell in the README and CONTRIBUTING.
Even if one of them is on a listing of commits. I'm assuming that
if we want to enforce less typos in the commits, having one in the
contributing guide is not the best example.

6 years agoCLEANUP: fix typos in comments for contrib/wireshark-dissectors
Joseph Herlant [Sat, 10 Nov 2018 03:00:24 +0000 (19:00 -0800)] 
CLEANUP: fix typos in comments for contrib/wireshark-dissectors

This fixes a typo in the README of the peers section of this subsystem
and 2 typos in code comments. Groupped together as cleanup to avoid too
many 1 char patches.

6 years agoCLEANUP: fix typos in comments for contrib/spoa_example
Joseph Herlant [Sat, 10 Nov 2018 02:36:35 +0000 (18:36 -0800)] 
CLEANUP: fix typos in comments for contrib/spoa_example

Fixes 3 common typos in the comments of the contrib/spoa_example
subsystem.

6 years agoCLEANUP: fix typos in comments for the contrib/modsecurity subsystem
Joseph Herlant [Sat, 10 Nov 2018 02:25:59 +0000 (18:25 -0800)] 
CLEANUP: fix typos in comments for the contrib/modsecurity subsystem

3 typos detected in code comments in the contrib/modsecurity subsystem.

6 years agoCLEANUP: fix a typo in a comment for the contrib/halog subsystem
Joseph Herlant [Sat, 10 Nov 2018 02:02:35 +0000 (18:02 -0800)] 
CLEANUP: fix a typo in a comment for the contrib/halog subsystem

Typo in comment, not visible by end-users.

6 years agoCLEANUP: fix typos in the comments of the Makefile
Joseph Herlant [Sat, 10 Nov 2018 01:50:30 +0000 (17:50 -0800)] 
CLEANUP: fix typos in the comments of the Makefile

This is not user-visible issues, just a cleanup of comments.

6 years agoBUILD: cache: fix a build warning regarding too large an integer for the age
Willy Tarreau [Sun, 11 Nov 2018 13:00:28 +0000 (14:00 +0100)] 
BUILD: cache: fix a build warning regarding too large an integer for the age

Building on 32 bit gives this :

  src/cache.c: In function 'http_action_store_cache':
  src/cache.c:466:4: warning: this decimal constant is unsigned only in ISO C90 [enabled by default]
  src/cache.c:467:5: warning: this decimal constant is unsigned only in ISO C90 [enabled by default]
  src/cache.c: In function 'cache_channel_append_age_header':
  src/cache.c:578:2: warning: this decimal constant is unsigned only in ISO C90 [enabled by default]
  src/cache.c:579:3: warning: this decimal constant is unsigned only in ISO C90 [enabled by default]

It's because of the definition below added in commit e7a770c ("MINOR:
cache: Add "Age" header.") :

  #define CACHE_ENTRY_MAX_AGE 2147483648

Just appending "U" to mark it unsigned is enough to fix it. This only
affects 1.9, no backport is needed.

6 years ago[RELEASE] Released version 1.9-dev6 v1.9-dev6
Willy Tarreau [Sun, 11 Nov 2018 09:43:39 +0000 (10:43 +0100)] 
[RELEASE] Released version 1.9-dev6

Released version 1.9-dev6 with the following main changes :
    - BUG/MEDIUM: tools: fix direction of my_ffsl()
    - BUG/MINOR: cli: forward the whole command on master CLI
    - BUG/MEDIUM: auth/threads: use of crypt() is not thread-safe
    - MINOR: compat: automatically detect support for crypt_r()
    - MEDIUM: auth/threads: make use of crypt_r() on systems supporting it
    - DOC: split the http-request actions in their own section
    - DOC: split the http-response actions in their own section
    - BUG/MAJOR: stream-int: don't call si_cs_recv() in stream_int_chk_rcv_conn()
    - BUG/MINOR: tasks: make sure wakeup events are properly reported to subscribers
    - MINOR: stats: report the number of active jobs and listeners in "show info"
    - MINOR: stats: report the number of active peers in "show info"
    - MINOR: stats: report the number of currently connected peers
    - MINOR: cli: show the number of reload in 'show proc'
    - MINOR: cli: can't connect to the target CLI
    - MEDIUM: mworker: does not create the CLI proxy when no listener
    - MINOR: mworker: displays more information when leaving
    - MEDIUM: mworker: exit with the incriminated exit code
    - MINOR: mworker: displays a message when a worker is forked
    - MEDIUM: mworker: leave when the master die
    - CLEANUP: stream-int: retro-document si_cs_io_cb()
    - BUG/MEDIUM: mworker: does not abort() in mworker_pipe_register()
    - BUG/MEDIUM: stream-int: don't wake up for nothing during SI_ST_CON
    - BUG/MEDIUM: cli: crash when trying to access a worker
    - DOC: restore note about "independant" typo
    - MEDIUM: stream: implement stream_buf_available()
    - MEDIUM: appctx: check for allocation attempts in buffer allocation callbacks
    - MINOR: stream-int: rename si_applet_{want|stop|cant}_{get|put}
    - MINOR: stream-int: add si_done_{get,put} to indicate that we won't do it anymore
    - MINOR: stream-int: use si_cant_put() instead of setting SI_FL_WAIT_ROOM
    - MINOR: stream-int: make use of si_done_{get,put}() in shut{w,r}
    - MINOR: stream-int: make it clear that si_ops cannot be null
    - MEDIUM: stream-int: temporarily make si_chk_rcv() take care of SI_FL_WAIT_ROOM
    - MINOR: stream-int: factor the SI_ST_EST state test into si_chk_rcv()
    - MEDIUM: stream-int: make SI_FL_WANT_PUT reflect CF_DONT_READ
    - MEDIUM: stream-int: always call si_chk_rcv() when we make room in the buffer
    - MEDIUM: stream-int: make si_chk_rcv() check that SI_FL_WAIT_ROOM is cleared
    - MINOR: stream-int: replace si_update() with si_update_both()
    - MEDIUM: stream-int: make stream_int_update() aware of the lower layers
    - CLEANUP: stream-int: remove the now unused si->update() function
    - MEDIUM: stream-int: Rely only on SI_FL_WAIT_ROOM to stop data receipt
    - MEDIUM: stream-int: Try to read data even if channel's buffer seems to be full
    - BUG/MINOR: config: better detect the presence of the h2 pattern in npn/alpn

6 years agoBUG/MINOR: config: better detect the presence of the h2 pattern in npn/alpn
Willy Tarreau [Sun, 11 Nov 2018 09:36:25 +0000 (10:36 +0100)] 
BUG/MINOR: config: better detect the presence of the h2 pattern in npn/alpn

In 1.8, commit 45a66cc ("MEDIUM: config: ensure that tune.bufsize is at
least 16384 when using HTTP/2") tried to avoid an annoying issue making
H2 fail when haproxy is built with default buffer sizes smaller than 16kB,
which used to be the case for a very long time. Sadly, the test only sees
when NPN/ALPN exactly match "h2" and not when it's combined like
"h2,http/1.1" nor "http/1.1,h2". We can safely use strstr() there because
the string is prefixed by the token's length (0x02) which is unambiguous
as it cannot be part of any other token.

This fix should be backported to 1.8 as a safety guard against bad
configurations.

6 years agoMEDIUM: stream-int: Try to read data even if channel's buffer seems to be full
Christopher Faulet [Thu, 11 Oct 2018 13:29:21 +0000 (15:29 +0200)] 
MEDIUM: stream-int: Try to read data even if channel's buffer seems to be full

Before calling the mux to get incoming data, we get the amount of space
available at the input of the buffer. If there is no space, we don't try to read
more data. This is good enough when raw data are stored in the buffer. But this
info has no meaning when structured data are stored. Because with the HTTP
refactoring, such kind of data will be stored in buffers, it is a bit annoying.

So, to avoid any problems, we always call the mux. It is the mux's responsiblity
to notify the stream interface it needs more space to store more data. This must
be done by setting the flag CS_FL_RCV_MORE on the conn_stream.

This is exactly what we do in the pass-through mux when <count> is null.

6 years agoMEDIUM: stream-int: Rely only on SI_FL_WAIT_ROOM to stop data receipt
Christopher Faulet [Thu, 11 Oct 2018 11:54:13 +0000 (13:54 +0200)] 
MEDIUM: stream-int: Rely only on SI_FL_WAIT_ROOM to stop data receipt

This flag is set on the stream interface when we should wait for more space in
the channel's buffer to store more incoming data. This means we should wait some
outgoing data are sent before retrying to receive more data.

But in stream interface functions, at many places, instead of checking this
flag, we use the function channel_may_recv to know if we can (re)start
reading. This currently works but it is not really consistent. And, it works
because only raw data are stored in buffers. But it will be a problem when we
start to store structured data in buffers.

So to avoid any problems with futur implementations, we now rely only on
SI_FL_WAIT_ROOM. The function channel_may_recv can still be called, but only
when we are sure to handle raw data (for instance in functions ci_put*). To do
so, among other things, we must be sure to unset SI_FL_WAIT_ROOM and offer an
opportunity to call chk_rcv() on a stream interface when some data are sent
on the other end, which is now granted by the previous patch series.

6 years agoCLEANUP: stream-int: remove the now unused si->update() function
Willy Tarreau [Fri, 9 Nov 2018 13:56:01 +0000 (14:56 +0100)] 
CLEANUP: stream-int: remove the now unused si->update() function

We exclusively use stream_int_update() now, the lower layers are not
called anymore so let's remove them, as well as si_update() which used
to be their wrapper.

6 years agoMEDIUM: stream-int: make stream_int_update() aware of the lower layers
Willy Tarreau [Fri, 9 Nov 2018 13:59:25 +0000 (14:59 +0100)] 
MEDIUM: stream-int: make stream_int_update() aware of the lower layers

It's far from being clean, but at least it allows to resync both CS and
applets from the same place, taking into account the fact that CS are
processed synchronously for the send side while appletx are processed
outside of the process_stream() loop. The arrangement is optimised to
minimize the amount of iteration by handling send first, then updating
the SI_FL_WAIT_ROOM flags and only then dealing with si_chk_rcv() on
both sides. The SI_FL_WANT_PUT flag is set if needed before calling
si_chk_rcv() since this is done prior to calling stream_int_update().

Now there's no risk that stream_int_notify() is called anymore during
such operations, thus we cannot have any spurious wake-up anymore. The
case where a successful send() could complete a pending connect() is
handled by taking any stream-int state changes into account at the
call place, which is normal since process_stream() is designed to
iterate till stabilisation.

Doing this solves most of the remaining inconsistencies between CS and
applets.

6 years agoMINOR: stream-int: replace si_update() with si_update_both()
Willy Tarreau [Thu, 8 Nov 2018 17:15:29 +0000 (18:15 +0100)] 
MINOR: stream-int: replace si_update() with si_update_both()

The function used to be called in turn for each side of the stream, but
since it's called exclusively from process_stream(), it prevents us from
making use of the knowledge we have of the operations in progress for
each side, resulting in having to go all the way through functions like
stream_int_notify() which are not appropriate there.

That patch creates a new function, si_update_both() which takes two
stream interfaces expected to belong to the same stream, and processes
their flags in a more suitable order, but for now doesn't change the
logic at all.

The next step will consist in trying to reinsert the rest of the socket
layer-specific update code to ultimately update the flags correctly at
the end of the operation.

6 years agoMEDIUM: stream-int: make si_chk_rcv() check that SI_FL_WAIT_ROOM is cleared
Willy Tarreau [Fri, 9 Nov 2018 15:21:43 +0000 (16:21 +0100)] 
MEDIUM: stream-int: make si_chk_rcv() check that SI_FL_WAIT_ROOM is cleared

After careful inspection, it now seems OK to call si_chk_rcv() only when
SI_FL_WAIT_ROOM is cleared and SI_FL_WANT_PUT is set, since all identified
call places have already taken care of this.

6 years agoMEDIUM: stream-int: always call si_chk_rcv() when we make room in the buffer
Willy Tarreau [Wed, 7 Nov 2018 17:53:29 +0000 (18:53 +0100)] 
MEDIUM: stream-int: always call si_chk_rcv() when we make room in the buffer

Instead of clearing the SI_FL_WAIT_ROOM flag and losing the information
about the need from the producer to be woken up, we now call si_chk_rcv()
immediately. This is cheap to do and it could possibly be further improved
by only doing it when SI_FL_WAIT_ROOM was still set, though this will
require some extra auditing of the code paths.

The only remaining place where the flag was cleared without a call to
si_chk_rcv() is si_alloc_ibuf(), but since this one is called from a
receive path woken up from si_chk_rcv() or not having failed, the
clearing was not necessary anymore either.

And there was one place in stream_int_notify() where si_chk_rcv() was
called with SI_FL_WAIT_ROOM still explicitly set so this place was
adjusted in order to clear the flag prior to calling si_chk_rcv().

Now we don't have any situation where we randomly clear SI_FL_WAIT_ROOM
without trying to wake the other side up, nor where we call si_chk_rcv()
with the flag set, so this flag should accurately represent a failed
attempt at putting data into the buffer.

6 years agoMEDIUM: stream-int: make SI_FL_WANT_PUT reflect CF_DONT_READ
Willy Tarreau [Wed, 7 Nov 2018 14:07:35 +0000 (15:07 +0100)] 
MEDIUM: stream-int: make SI_FL_WANT_PUT reflect CF_DONT_READ

When CF_DONT_READ is set, till now we used to set SI_FL_WAIT_ROOM, which
is not appropriate since it would lose the subscribe status. Instead let's
clear SI_FL_WANT_PUT (just like applets do), and set the flag only when
CF_DONT_READ is cleared.

We have to do this in stream_int_update(), and in si_cs_io_cb() after
returning from si_cs_recv() since it would be a bit invasive to hack
this one for now. It must not be done in stream_int_notify() otherwise
it would re-enable blocked applets.

Last, when si_chk_rcv() is called, it immediately clears the flag before
calling ->chk_rcv() so that we are not tempted to uselessly loop on the
same call until the receive function is called. This is the same principle
as what is done with the applet scheduler.

6 years agoMINOR: stream-int: factor the SI_ST_EST state test into si_chk_rcv()
Willy Tarreau [Wed, 7 Nov 2018 13:59:45 +0000 (14:59 +0100)] 
MINOR: stream-int: factor the SI_ST_EST state test into si_chk_rcv()

This test is made in each implementation of the function, better to
merge it.

6 years agoMEDIUM: stream-int: temporarily make si_chk_rcv() take care of SI_FL_WAIT_ROOM
Willy Tarreau [Wed, 7 Nov 2018 10:55:54 +0000 (11:55 +0100)] 
MEDIUM: stream-int: temporarily make si_chk_rcv() take care of SI_FL_WAIT_ROOM

This flag should already be cleared before calling the *chk_rcv() functions.
Before adapting all call places, let's first make sure si_chk_rcv() clears
it before calling them so that these functions do not have to check it again
and so that they do not adjust it. This function will only call the lower
layers if the SI_FL_WANT_PUT flag is present so that the endpoint can decide
not to be called (as done with applets).

6 years agoMINOR: stream-int: make it clear that si_ops cannot be null
Willy Tarreau [Wed, 7 Nov 2018 10:28:12 +0000 (11:28 +0100)] 
MINOR: stream-int: make it clear that si_ops cannot be null

There was an ambiguity in which functions of the si_ops struct could be
null or not. only ->update doesn't exist in one of the si_ops (the
embedded one), all others are always defined. ->shutr and ->shutw were
never tested. However ->chk_rcv() and ->chk_snd() were tested, causing
confusion about the proper way to wake the other side up if undefined
(which never happens).

Let's update the comments to state these functions are mandatory and
remove the offending checks.

6 years agoMINOR: stream-int: make use of si_done_{get,put}() in shut{w,r}
Willy Tarreau [Tue, 6 Nov 2018 18:23:03 +0000 (19:23 +0100)] 
MINOR: stream-int: make use of si_done_{get,put}() in shut{w,r}

It's cleaner to use these functions there to properly clear the flags.

6 years agoMINOR: stream-int: use si_cant_put() instead of setting SI_FL_WAIT_ROOM
Willy Tarreau [Tue, 6 Nov 2018 18:10:53 +0000 (19:10 +0100)] 
MINOR: stream-int: use si_cant_put() instead of setting SI_FL_WAIT_ROOM

We now do this on the si_cs_recv() path so that we always have
SI_FL_WANT_PUT properly set when there's a need to receive and
SI_FL_WAIT_ROOM upon failure.

6 years agoMINOR: stream-int: add si_done_{get,put} to indicate that we won't do it anymore
Willy Tarreau [Tue, 6 Nov 2018 18:17:31 +0000 (19:17 +0100)] 
MINOR: stream-int: add si_done_{get,put} to indicate that we won't do it anymore

This is useful on close or stream aborts as it saves us from having
to manipulate the (sometimes confusing) flags.

6 years agoMINOR: stream-int: rename si_applet_{want|stop|cant}_{get|put}
Willy Tarreau [Tue, 6 Nov 2018 17:46:37 +0000 (18:46 +0100)] 
MINOR: stream-int: rename si_applet_{want|stop|cant}_{get|put}

It doesn't make sense to limit this code to applets, as any stream
interface can use it. Let's rename it by simply dropping the "applet_"
part of the name. No other change was made except updating the comments.

6 years agoMEDIUM: appctx: check for allocation attempts in buffer allocation callbacks
Willy Tarreau [Tue, 6 Nov 2018 16:32:37 +0000 (17:32 +0100)] 
MEDIUM: appctx: check for allocation attempts in buffer allocation callbacks

The buffer allocation callback appctx_res_wakeup() used to rely on old
tricks to detect if a buffer was already granted to an appctx, namely
by checking the task's state. Not only this test is not valid anymore,
but it's inaccurate.

Let's solely on SI_FL_WAIT_ROOM that is now set on allocation failure by
the functions trying to allocate a buffer. The buffer is now allocated on
the fly and the flag removed so that the consistency between the two
remains granted. The patch also fixes minor issues such as the function
being improperly declared inline(!) and the fact that using appctx_wakeup()
sets the wakeup reason to TASK_WOKEN_OTHER while we try to use TASK_WOKEN_RES
when waking up consecutive to a ressource allocation such as a buffer.

6 years agoMEDIUM: stream: implement stream_buf_available()
Willy Tarreau [Tue, 6 Nov 2018 14:50:21 +0000 (15:50 +0100)] 
MEDIUM: stream: implement stream_buf_available()

This function replaces stream_res_available(), which is used as a callback
for the buffer allocator. It now carefully checks which stream interface
was blocked on a buffer allocation, tries to allocate the input buffer to
this stream interface, and wakes the task up once such a buffer was found.
It will automatically remove the SI_FL_WAIT_ROOM flag upon success since
the info this flag indicates becomes wrong as soon as the buffer is
allocated.

The code is still far from being perfect because if a call to si_cs_recv()
fails to allocate a buffer, we'll still end up passing via process_stream()
again, but this could be improved in the future by using finer-grained
wake-up notifications.

6 years agoDOC: restore note about "independant" typo
Lukas Tribus [Thu, 8 Nov 2018 11:41:42 +0000 (12:41 +0100)] 
DOC: restore note about "independant" typo

The independant -> independent error was fixed in 801a0a35 ("DOC: fix
name for "option independant-streams"), but the note about the wrong
name was erroneously fixed in 0e82b92a ("DOC: fix a few config typos").

Restore the "wrong" name so that when reasearching this option people
can actually find it.

Could be backported to 1.8.

6 years agoBUG/MEDIUM: cli: crash when trying to access a worker
William Lallemand [Thu, 8 Nov 2018 11:00:14 +0000 (12:00 +0100)] 
BUG/MEDIUM: cli: crash when trying to access a worker

When using the CLI proxy of the master and trying to access a worker
with the @ prefix, the worker just crash.

The commit 7216032 ("MEDIUM: mworker: leave when the master die")
reintroduced the old code of the pipe, which was not trying to access
the pointers before. The owner of the FD was modified to a different
value, this is a problem since we call listener_accept() in most cases
now from the mworker_accept_wrapper() and it casts the owner variable to
get the listener.

This patch fix the issue by setting back the previous owner of the FD.

6 years agoBUG/MEDIUM: stream-int: don't wake up for nothing during SI_ST_CON
Willy Tarreau [Thu, 8 Nov 2018 13:32:16 +0000 (14:32 +0100)] 
BUG/MEDIUM: stream-int: don't wake up for nothing during SI_ST_CON

Commit eafd8ebcf ("MEDIUM: stream-int: call si_cs_process() in
stream_int_update_conn") uncovered a sleeping bug. By calling
si_cs_process() within si_update(), we end up calling stream_int_notify().
We rely on it to update the stream-int before quitting as a hack, but
it happens to immediately wake the task up while the stream int's
state is still SI_ST_CON (during the connection establishment). The
observable effect is that an unreachable server causes haproxy to
use 100% CPU until the connection timeout strikes.

This patch fixes this by not causing the wake up for the SI_ST_CON
state. It would equally be possible to check for states higher than
SI_ST_EST as is done in other places, but for now better stay on the
safe side by covering the only issue that can be triggered. It's
suspected that this issue slightly affects older versions by causing
one extra call to process_stream() during the connection setup for
each activity change on the other side, but this should not have
any observable effect.

No backport is needed.

6 years agoBUG/MEDIUM: mworker: does not abort() in mworker_pipe_register()
William Lallemand [Wed, 7 Nov 2018 07:38:32 +0000 (08:38 +0100)] 
BUG/MEDIUM: mworker: does not abort() in mworker_pipe_register()

The process was aborting with nbthread > 1.

The mworker_pipe_register() could be called several time in multithread
mode, we don't want to abort() there.

6 years agoCLEANUP: stream-int: retro-document si_cs_io_cb()
Willy Tarreau [Wed, 7 Nov 2018 06:47:52 +0000 (07:47 +0100)] 
CLEANUP: stream-int: retro-document si_cs_io_cb()

It took me 17 minutes this morning to figure where si->wait_event was
set (it's in si_reset() which should now probably be renamed since it
doesn't just perform a reset anymore but also an allocation) and what
its task was assigned to (si_cs_io_cb() even for applets and empty SI).

This is too confusing and not intuitive enough, let's at least add a
few comments for now to help figure how this stuff works next time.

6 years agoMEDIUM: mworker: leave when the master die
William Lallemand [Tue, 6 Nov 2018 16:37:16 +0000 (17:37 +0100)] 
MEDIUM: mworker: leave when the master die

When the master die, the worker should exit too, this is achieved by
checking if the FD of the socketpair/pipe was closed between the master
and the worker.

In the former architecture of the master-worker, there was only a pipe
between the master and the workers, and it was easy to check an EOF on
the pipe FD to exit() the worker.

With the new architecture, we use a socketpair by process, and this
socketpair is also used to accept new connections with the
listener_accept() callback.

This accept callback can't handle the EOF and the exit of the process,
because it's very specific to the master worker. This is why we
transformed the mworker_pipe_handler() function in a wrapper which check
if there is an EOF and exit the process, and if not call
listener_accept() to achieve the accept.

6 years agoMINOR: mworker: displays a message when a worker is forked
William Lallemand [Tue, 6 Nov 2018 16:37:15 +0000 (17:37 +0100)] 
MINOR: mworker: displays a message when a worker is forked

Displays the PID and the relative PID when we fork a new worker.

6 years agoMEDIUM: mworker: exit with the incriminated exit code
William Lallemand [Tue, 6 Nov 2018 16:37:14 +0000 (17:37 +0100)] 
MEDIUM: mworker: exit with the incriminated exit code

The former behavior was to exit() the master process with the latest
status code known, which was the one of the last process to exit.

The problem is that the master process was not exiting with the status
code which provoked the exit-on-failure.

6 years agoMINOR: mworker: displays more information when leaving
William Lallemand [Tue, 6 Nov 2018 16:37:13 +0000 (17:37 +0100)] 
MINOR: mworker: displays more information when leaving

When a worker is leaving, we display the relative PID and the result of
the strsignal() function if it was killed by a signal.

6 years agoMEDIUM: mworker: does not create the CLI proxy when no listener
William Lallemand [Tue, 6 Nov 2018 16:37:12 +0000 (17:37 +0100)] 
MEDIUM: mworker: does not create the CLI proxy when no listener

Does not create the CLI proxy if no -S argument was specified. It
prevents a warning that says that the MASTER proxy does not have any
bind option.

6 years agoMINOR: cli: can't connect to the target CLI
William Lallemand [Tue, 6 Nov 2018 16:37:11 +0000 (17:37 +0100)] 
MINOR: cli: can't connect to the target CLI

Return an error and quit if the CLI proxy is not able to connect to a
target.

6 years agoMINOR: cli: show the number of reload in 'show proc'
William Lallemand [Tue, 6 Nov 2018 16:37:10 +0000 (17:37 +0100)] 
MINOR: cli: show the number of reload in 'show proc'

Displays the number of reload in the life of each worker.

6 years agoMINOR: stats: report the number of currently connected peers
Willy Tarreau [Mon, 5 Nov 2018 16:12:27 +0000 (17:12 +0100)] 
MINOR: stats: report the number of currently connected peers

The active peers output indicates both the number of established peers
connections and the number of peers connection attempts. The new counter
"ConnectedPeers" also indicates the number of currently connected peers.
This helps detect that some peers cannot be reached for example. It's
worth mentioning that this value changes over time because unused peers
are often disconnected and reconnected. Most of the time it should be
equal to ActivePeers.

6 years agoMINOR: stats: report the number of active peers in "show info"
Willy Tarreau [Mon, 5 Nov 2018 15:31:22 +0000 (16:31 +0100)] 
MINOR: stats: report the number of active peers in "show info"

Peers are the last type of activity which can maintain a job present, so
it's important to report that such an entity is still active to explain
why the job count may be higher than zero. Here by "ActivePeers" we report
peers sessions, which include both established connections and outgoing
connection attempts.