]> git.ipfire.org Git - thirdparty/haproxy.git/log
thirdparty/haproxy.git
6 years agoMEDIUM: snapshots: dynamically allocate the snapshots
Willy Tarreau [Fri, 7 Sep 2018 17:02:32 +0000 (19:02 +0200)] 
MEDIUM: snapshots: dynamically allocate the snapshots

Now upon error we dynamically allocate the snapshot instead of overwriting
it. This way there is no more memory wasted in the proxy to hold the two
error snapshot descriptors. Also an appreciable side effect of this is that
the proxy's lock is only taken during the pointer swap, no more while copying
the buffer's contents. This saves 480 bytes of memory per proxy.

6 years agoBUG/MEDIUM: snapshot: take the proxy's lock while dumping errors
Willy Tarreau [Fri, 7 Sep 2018 17:55:44 +0000 (19:55 +0200)] 
BUG/MEDIUM: snapshot: take the proxy's lock while dumping errors

The proxy's lock it held while filling the error but not while dumping
it, so it's possible to dereference pointers being replaced, typically
server pointers. The risk is very low and unlikely but not inexistent.

Since "show errors" is rarely used in parallel, let's simply grab the
proxy's lock while dumping. Ideally we should use an R/W lock here but
it will not make any difference.

This patch must be backported to 1.8, but the code is in proto_http.c
there, though mostly similar.

6 years agoREORG: cli: move the "show errors" handler from http to proxy
Willy Tarreau [Fri, 7 Sep 2018 16:34:24 +0000 (18:34 +0200)] 
REORG: cli: move the "show errors" handler from http to proxy

There's nothing HTTP-specific there anymore at all, let's move this
to the proxy where it belongs.

6 years agoMINOR: http: remove the pointer to the error snapshot in http_capture_bad_message()
Willy Tarreau [Fri, 7 Sep 2018 16:01:03 +0000 (18:01 +0200)] 
MINOR: http: remove the pointer to the error snapshot in http_capture_bad_message()

It's not needed anymore as we know the side thanks to the channel. This
will allow the proxy generic code to better manage the error snapshots.

6 years agoMINOR: http: make the HTTP error capture rely on the generic proxy code
Willy Tarreau [Fri, 7 Sep 2018 13:47:35 +0000 (15:47 +0200)] 
MINOR: http: make the HTTP error capture rely on the generic proxy code

Now that we have a generic error capture function, let's simplify
http_capture_bad_message() to make use of it. At this point the API
is not changed at all, but it could be further simplified.

6 years agoMINOR: proxy: add a new generic proxy_capture_error()
Willy Tarreau [Fri, 7 Sep 2018 15:43:26 +0000 (17:43 +0200)] 
MINOR: proxy: add a new generic proxy_capture_error()

This function now captures an error regardless of its side and protocol.
The caller must pass a number of elements and may pass a protocol-specific
structure and a callback to display it. Later this function may deal with
more advanced allocation techniques to avoid allocating as many buffers
as proxies.

6 years agoMEDIUM: snapshot: implement a show() callback and use it for HTTP
Willy Tarreau [Fri, 7 Sep 2018 12:01:39 +0000 (14:01 +0200)] 
MEDIUM: snapshot: implement a show() callback and use it for HTTP

The HTTP dumps are now configurable in the code : "show errors" now
calls a protocol-specific function to emit the decoded output. For
now only HTTP is implemented.

6 years agoMEDIUM: snapshot: start to reorder the HTTP snapshot output a little bit
Willy Tarreau [Fri, 7 Sep 2018 11:49:44 +0000 (13:49 +0200)] 
MEDIUM: snapshot: start to reorder the HTTP snapshot output a little bit

The output of "show errors" was slightly reordered to split the HTTP part
in a single chunk_appendf() call. The useless buffer total input was
replaced to report the buffer's start offset, which is the offset in the
stream of the first input byte (thus not counting output). Also it was
the opportunity to stop calling the stream "session".

6 years agoMINOR: snapshot: split the error snapshots into common and proto-specific parts
Willy Tarreau [Thu, 6 Sep 2018 17:41:22 +0000 (19:41 +0200)] 
MINOR: snapshot: split the error snapshots into common and proto-specific parts

The idea will be to make the error snapshot feature accessible to other
protocols than just HTTP. This patch only introduces an "http_snapshot"
structure and renames a few fields to make things more explicit. The
HTTP part was installed inside a union so that we can easily add more
protocols in the future.

6 years agoMINOR: snapshot: restart on the event ID and not the stream ID
Willy Tarreau [Fri, 7 Sep 2018 09:51:51 +0000 (11:51 +0200)] 
MINOR: snapshot: restart on the event ID and not the stream ID

The snapshots have the ability to restart a partial dump and they use
the stream ID as the restart point. Since it's purely HTTP, let's use
the event ID instead.

6 years agoBUG/MINOR: http/threads: atomically increment the error snapshot ID
Willy Tarreau [Fri, 7 Sep 2018 09:29:59 +0000 (11:29 +0200)] 
BUG/MINOR: http/threads: atomically increment the error snapshot ID

Let's use an atomic increment for the error snapshot, as we'd rather
not assign the same ID to two errors happening in parallel. It's very
unlikely that it will ever happen though.

This patch must be backported to 1.8 with the other one it relies on
("MINOR: thread: implement HA_ATOMIC_XADD()").

6 years agoBUG/MINOR: dns: check and link servers' resolvers right after config parsing
Baptiste Assmann [Fri, 10 Aug 2018 08:56:38 +0000 (10:56 +0200)] 
BUG/MINOR: dns: check and link servers' resolvers right after config parsing

On the Mailing list, Marcos Moreno reported that haproxy configuration
validation (through "haproxy -c cfgfile") does not detect when a
resolvers section does not exist for a server.
That said, this checking is done after HAProxy has started up.

The problem is that this can create production issue, since init
script can't detect the problem before starting / reloading HAProxy.

To fix this issue, this patch registers the function which validates DNS
configuration validity and run it right after configuration parsing is
finished (through cfg_register_postparser()).
Thanks to it, now "haproxy -c cfgfile" will fail when a server
points to a non-existing resolvers section (or any other validation made
by the function above).

Backport status: 1.8

6 years agoMINOR: log: One const should be enough.
Olivier Houchard [Thu, 6 Sep 2018 16:14:09 +0000 (18:14 +0200)] 
MINOR: log: One const should be enough.

"const const" doesn't bring much more constness, so only use one.

6 years agoMINOR: connection: add new function conn_is_back()
Willy Tarreau [Thu, 6 Sep 2018 12:52:21 +0000 (14:52 +0200)] 
MINOR: connection: add new function conn_is_back()

This function returns true if the connection is a backend connection
and false if it's a frontend connection.

6 years agoMINOR: connection: add new function conn_get_proxy()
Willy Tarreau [Thu, 6 Sep 2018 09:48:44 +0000 (11:48 +0200)] 
MINOR: connection: add new function conn_get_proxy()

This function returns the proxy associated to a connection. For front
connections it returns the frontend, and for back connections it
returns the backend. This will be used to retrieve some configuration
parameters from within a mux.

6 years agoMINOR: connection: make the initialization more consistent
Willy Tarreau [Thu, 6 Sep 2018 09:45:30 +0000 (11:45 +0200)] 
MINOR: connection: make the initialization more consistent

Sometimes a connection is prepared before the target is set, sometimes
after. There's no real rule since the few functions involved operate on
different and independent fields. Soon we'll benefit from knowing the
target at the connection layer, in order to figure the associated proxy
and retrieve the various parameters (timeouts etc). This patch slightly
reorders a few calls to conn_prepare() so that we can make sure that the
target is always known to the mux.

6 years agoBUG/MINOR: h1: fix buffer shift after realignment
Willy Tarreau [Thu, 6 Sep 2018 08:48:15 +0000 (10:48 +0200)] 
BUG/MINOR: h1: fix buffer shift after realignment

Commit 5e74b0b ("MEDIUM: h1: port to new buffer API.") introduced a
minor bug by which a buffer's head could stay shifted by the amount
of removed CRLF if it started with empty lines. This would cause the
second request (or response) not to work until it would receive a few
extra characters. This most only impacts requests sent by hand though.

This is purely 1.9, no backport is needed.

6 years agoMEDIUM: h2: produce some logs on early errors that prevent streams from being created
Willy Tarreau [Wed, 5 Sep 2018 17:55:58 +0000 (19:55 +0200)] 
MEDIUM: h2: produce some logs on early errors that prevent streams from being created

The h2 mux currently lacks some basic transparency. Some errors cause the
connection to be aborted but they couldn't be reported. With this patch,
almost all situations where an error will cause a stream or connection to
be aborted without the ability for an existing stream to report it will be
reported in the logs. This at least provides a solution to monitor the
activity and abnormal traffic.

6 years agoMINOR: log: provide a function to emit a log for a session
Willy Tarreau [Wed, 5 Sep 2018 17:51:10 +0000 (19:51 +0200)] 
MINOR: log: provide a function to emit a log for a session

The new function sess_log() only needs a session to emit a log. It will
ignore the parts that depend on the stream. It is usable to emit a log
to report early errors in muxes. These ones will typically mention
"<BADREQ>" for the request and 0 for the HTTP status code.

6 years agoMEDIUM: log: make sess_build_logline() support being called with no stream
Willy Tarreau [Wed, 5 Sep 2018 14:55:15 +0000 (16:55 +0200)] 
MEDIUM: log: make sess_build_logline() support being called with no stream

Till now it was impossible to emit logs from the lower layers only because
a stream was mandatory. From now on it will at least be possible to emit a
log to report a bad request or some timings for example. When the stream
is null, sess_build_logline() will use default values and will extract the
timing information from the session just like stream_new() does, so the
resulting log line is perfectly valid.

The termination state will indicate a proxy error during the request phase
since it is the only realistic use for such a call with no stream.

6 years agoMINOR: log: use zero as the request counter if there is no stream
Willy Tarreau [Wed, 5 Sep 2018 13:52:59 +0000 (15:52 +0200)] 
MINOR: log: use zero as the request counter if there is no stream

When s==NULL we don't have any assigned request counter. Ideally we
should proceed exactly like when a stream is initialized and assign
a unique value. For now we only place it into a local variable.

6 years agoMINOR: log: keep a copy of s->flags early to avoid a dereference
Willy Tarreau [Wed, 5 Sep 2018 13:51:28 +0000 (15:51 +0200)] 
MINOR: log: keep a copy of s->flags early to avoid a dereference

By placing s->flags into a local variable we'll be able to force it new
values when s is NULL.

6 years agoMINOR: log: use NULL for the unique_id if there is no stream
Willy Tarreau [Wed, 5 Sep 2018 13:49:01 +0000 (15:49 +0200)] 
MINOR: log: use NULL for the unique_id if there is no stream

Now s->unique_id is used as NULL (not set) if s==NULL.

6 years agoMINOR: log: don't check the stream-int's conn_retries if the stream is NULL
Willy Tarreau [Tue, 4 Sep 2018 17:21:44 +0000 (19:21 +0200)] 
MINOR: log: don't check the stream-int's conn_retries if the stream is NULL

Let's simply forget the conn_retries when there is no stream since we
haven't tried to connect yet.

6 years agoMINOR: log: be sure not to dereference a null stream for a target
Willy Tarreau [Wed, 5 Sep 2018 13:30:16 +0000 (15:30 +0200)] 
MINOR: log: be sure not to dereference a null stream for a target

The supported targets are either a server or an applet, so both are
NULL if the stream is NULL.

6 years agoMINOR: log: do not dereference a null stream to access captures
Willy Tarreau [Wed, 5 Sep 2018 13:28:07 +0000 (15:28 +0200)] 
MINOR: log: do not dereference a null stream to access captures

If the stream is null, let's simply not check captures. That's already
done if there is no capture.

6 years agoMINOR: log: keep a copy of the backend connection early in sess_build_logline()
Willy Tarreau [Wed, 5 Sep 2018 13:24:56 +0000 (15:24 +0200)] 
MINOR: log: keep a copy of the backend connection early in sess_build_logline()

This way we can avoid dereferencing a possibly inexisting stream.

6 years agoCLEANUP: log: make the low_level lf_{ip,port,text,text_len} functions take consts
Willy Tarreau [Wed, 5 Sep 2018 13:23:10 +0000 (15:23 +0200)] 
CLEANUP: log: make the low_level lf_{ip,port,text,text_len} functions take consts

These ones were abusively relying on variables making it hard to integrate
with const arguments.

6 years agoMINOR: log: don't unconditionally pick log info from s->logs
Willy Tarreau [Wed, 5 Sep 2018 13:16:23 +0000 (15:16 +0200)] 
MINOR: log: don't unconditionally pick log info from s->logs

We'll soon support s==NULL so let's use an intermediary variable for the
logs structure. For now it only points to s->logs but will support a local
variable as an alternative later.

6 years agoMINOR: log: make sess_build_logline() not dereference a NULL stream for txn
Willy Tarreau [Wed, 5 Sep 2018 13:10:35 +0000 (15:10 +0200)] 
MINOR: log: make sess_build_logline() not dereference a NULL stream for txn

If the stream is NULL, the txn is NULL as well. This condition is already
handled everywhere else.

6 years agoMINOR: log: make the backend fall back to the frontend when there's no stream
Willy Tarreau [Wed, 5 Sep 2018 13:07:15 +0000 (15:07 +0200)] 
MINOR: log: make the backend fall back to the frontend when there's no stream

This is already what happens before the backend is assigned, except that
now we don't need to dereference a NULL stream to figure this.

6 years agoMINOR: log: move the log code to sess_build_logline() to add extra arguments
Willy Tarreau [Wed, 5 Sep 2018 12:58:15 +0000 (14:58 +0200)] 
MINOR: log: move the log code to sess_build_logline() to add extra arguments

The current build_logline() can only be used with valid streams, which
means it is not suitable for use from muxes. We start by moving it into
another more generic function which takes the session as an argument,
to avoid complexifying all the internal API for jsut a few use cases.
This new function is not supposed to be called directly from outside so
we'll be able to instrument it to support several calling conventions.

For now the behaviour and conditions remain unchanged.

6 years agoBUG/MAJOR: buffer: fix incorrect check in __b_putblk()
Willy Tarreau [Wed, 5 Sep 2018 17:00:20 +0000 (19:00 +0200)] 
BUG/MAJOR: buffer: fix incorrect check in __b_putblk()

This function was split in two at commit f7d0447 ("MINOR: buffers:
split b_putblk() into __b_putblk()") but it's wrong, the first half's
length is not adjusted to the requested size so it copies more than
desired.

This is purely 1.9-specific, no backport is needed.

6 years agoBUG/MEDIUM: h2: fix risk of memory leak on malformated wrapped frames
Willy Tarreau [Wed, 5 Sep 2018 16:30:05 +0000 (18:30 +0200)] 
BUG/MEDIUM: h2: fix risk of memory leak on malformated wrapped frames

While parsing a headers frame, if the frame is wrapped in the buffer
and needs to be unwrapped, it will be duplicated before being processed.
But if it contains certain combinations of invalid flags, the parser
returns without releasing the temporary buffer leading to a memory
leak.

This fix needs to be backported to 1.8.

6 years agoBUG/MEDIUM: session: fix reporting of handshake processing time in the logs
Willy Tarreau [Wed, 5 Sep 2018 09:56:48 +0000 (11:56 +0200)] 
BUG/MEDIUM: session: fix reporting of handshake processing time in the logs

The handshake processing time used to be stored per stream, which was
valid when there was exactly one stream per session. With H2 and
multiplexing it's not the case anymore and the reported handshake times
are wrong in the logs as it's computed between the TCP accept() and the
stream creation. Let's first move the handshake where it belongs, which
is the session.

However, this is not enough because we don't want to report an excessive
idle time either for H2 (since many requests use the connection).

So the solution used here is to have the stream retrieve sess->tv_accept
and the handshake duration when the stream is created, and let the mux
immediately reset them. This way, the handshake time becomes zero for the
second and subsequent requests in H2 (which was already the case in H1),
and the idle time exactly counts how long the connection remained unused
while it could be used, so in H1 it runs from the end of the previous
response and in H2 it runs from the end of the previous request since the
channel is already available.

This patch will need to be backported to 1.8.

6 years agoBUG/MINOR: stream: use atomic increments for the request counter
Willy Tarreau [Wed, 5 Sep 2018 14:21:29 +0000 (16:21 +0200)] 
BUG/MINOR: stream: use atomic increments for the request counter

The request counter is incremented when creating a new stream and when
resetting a stream, preparing for a new request. Unfortunately during
the thread migration this was missed, leading to non-atomic increments
in case threads are in use. The most visible side effect is that two
requests may have the same ID from time to time in the logs. However
the SPOE also uses this ID to route responses back to the stream so it
may also lead to occasional spurious SPOE timeouts.

Note that it still doesn't guarantee temporal unicity in the stream
identifiers since a long and a short connection could technically use
the same ID. The likeliness that this happens at the same time is almost
null (roughly threads*runqueue_depth/2^32 that it happens in the same
poll loop), but it will have to be addressed later anyway.

This patch must be backported to 1.8 with the other one it relies on
("MINOR: thread: implement HA_ATOMIC_XADD()").

6 years agoMINOR: thread: implement HA_ATOMIC_XADD()
Willy Tarreau [Wed, 5 Sep 2018 14:11:03 +0000 (16:11 +0200)] 
MINOR: thread: implement HA_ATOMIC_XADD()

We've been missing it several times and now we'll need it to increment
a request counter. Let's do it once for all.

This patch will need to be backported to 1.8 with the associated fix.

6 years agoMINOR: tools: make date2str_log() take some consts
Willy Tarreau [Tue, 4 Sep 2018 17:08:48 +0000 (19:08 +0200)] 
MINOR: tools: make date2str_log() take some consts

The "tm" and "date" field are not modified, they can be const instead
of forcing their callers to use vars.

6 years agoBUG/MEDIUM: ECC cert should work with TLS < v1.2 and openssl >= 1.1.1
Emmanuel Hocdet [Mon, 3 Sep 2018 14:29:16 +0000 (16:29 +0200)] 
BUG/MEDIUM: ECC cert should work with TLS < v1.2 and openssl >= 1.1.1

With openssl >= 1.1.1 and boringssl multi-cert is natively supported.
ECDSA/RSA selection is done and work correctly with TLS >= v1.2.
TLS < v1.2 have no TLSEXT_TYPE_signature_algorithms extension: ECC
certificate can't be selected, and handshake fail if no RSA cert
is present. Safe ECC certificate selection without client announcement
can be very tricky (browser compatibilty). The safer approach is to
select ECDSA certificate if no other certificate matches, like it is
with openssl < 1.1.1: certificate selection is only done via the SNI.

Thanks to Lukas Tribus for reporting this and analysing the problem.

This patch should be backported to 1.8

6 years agoBUG/MEDIUM: dns/server: fix incomatibility between SRV resolution and server state...
Baptiste Assmann [Mon, 2 Jul 2018 15:00:54 +0000 (17:00 +0200)] 
BUG/MEDIUM: dns/server: fix incomatibility between SRV resolution and server state file

Server state file has no indication that a server is currently managed
by a DNS SRV resolution.
And thus, both feature (DNS SRV resolution and server state), when used
together, does not provide the expected behavior: a smooth experience...

This patch introduce the "SRV record name" in the server state file and
loads and applies it if found and wherever required.

This patch applies to haproxy-dev branch only. For backport, a specific patch
is provided for 1.8.

6 years agoREGTEST/MINOR: lua: Add reg testing files for 70d318c.
Frédéric Lécaille [Tue, 4 Sep 2018 13:58:14 +0000 (15:58 +0200)] 
REGTEST/MINOR: lua: Add reg testing files for 70d318c.

6 years agoBUG/MEDIUM: hlua: Don't call RESET_SAFE_LJMP if SET_SAFE_LJMP returns 0.
Olivier Houchard [Mon, 27 Aug 2018 10:59:14 +0000 (12:59 +0200)] 
BUG/MEDIUM: hlua: Don't call RESET_SAFE_LJMP if SET_SAFE_LJMP returns 0.

If SET_SAFE_LJMP returns 0, the spinlock is already unlocked, and lua_atpanic
is already set back to hlua_panic_safe, so there's no need to call
RESET_SAFE_LJMP.

This should be MFC'd into 1.8.

6 years agoREGTEST/MINOR: Add a reg testing file for 3e60b11.
Frédéric Lécaille [Fri, 31 Aug 2018 09:05:39 +0000 (11:05 +0200)] 
REGTEST/MINOR: Add a reg testing file for 3e60b11.

Thank you to Daniel for having worked on this one.

6 years agoBUG/MAJOR: thread: lua: Wrong SSL context initialization.
Frédéric Lécaille [Wed, 29 Aug 2018 11:46:24 +0000 (13:46 +0200)] 
BUG/MAJOR: thread: lua: Wrong SSL context initialization.

When calling ->prepare_srv() callback for SSL server which
depends on global "nbthread" value, this latter was not already parsed,
so equal to 1 default value. This lead to bad memory accesses.

Thank you to Pieter (PiBa-NL) for having reported this issue and
for having provided a very helpful reg testing file to reproduce
this issue (reg-test/lua/b00002.*).

Must be backported to 1.8.

6 years agoBUG/MEDIUM: stream_interface: try to call si_cs_send() earlier.
Olivier Houchard [Tue, 28 Aug 2018 17:37:41 +0000 (19:37 +0200)] 
BUG/MEDIUM: stream_interface: try to call si_cs_send() earlier.

Call si_cs_send() at the beginning of si_cs_wake_cb(), instead of from
stream_int_notify-), so that if we get a connection error while trying to
send, the stream_interface will get SI_FL_ERR, the associated task will
be woken up, and the connection will be properly destroyed.

No backport needed.

6 years agoMINOR: checks: Call wake_srv_chk() when we can finally send data.
Olivier Houchard [Tue, 28 Aug 2018 17:36:18 +0000 (19:36 +0200)] 
MINOR: checks: Call wake_srv_chk() when we can finally send data.

Instead of calling __event_srv_chk_w, call wake_srv_chk(), which will then
either call tcpcheck_main() or __event_srv_chk_w().
Also make tcpcheck_main() subscribe if it can't send.

6 years agoBUG/MEDIUM: hlua: Make sure we drain the output buffer when done.
Olivier Houchard [Tue, 28 Aug 2018 12:41:31 +0000 (14:41 +0200)] 
BUG/MEDIUM: hlua: Make sure we drain the output buffer when done.

In hlua_applet_tcp_fct(), drain the output buffer when the applet is done
running, every time we're called.
Overwise, there's a race condition, and the output buffer could be filled
after the applet ran, and as it is never cleared, the stream interface
will never be destroyed.

This should be backported to 1.8 and 1.7.

6 years agoMINOR: Add srv_conn_free sample fetch
Patrick Hemmer [Thu, 14 Jun 2018 22:01:35 +0000 (18:01 -0400)] 
MINOR: Add srv_conn_free sample fetch

This adds the 'srv_conn_free([<backend>/]<server>)' sample fetch. This fetch
provides the number of available connections on the designated server.

6 years agoMINOR: add be_conn_free sample fetch
Patrick Hemmer [Thu, 14 Jun 2018 21:10:27 +0000 (17:10 -0400)] 
MINOR: add be_conn_free sample fetch

This adds the sample fetch 'be_conn_free([<backend>])'. This sample fetch
provides the total number of unused connections across available servers in the
specified backend.

6 years agoBUG/MEDIUM: lua: reset lua transaction between http requests
Patrick Hemmer [Wed, 22 Aug 2018 14:02:00 +0000 (10:02 -0400)] 
BUG/MEDIUM: lua: reset lua transaction between http requests

Previously LUA code would maintain the transaction state between http
requests, resulting in things like txn:get_priv() retrieving data from
a previous request. This addresses the issue by ensuring the LUA state
is reset between requests.

Co-authored-by: Tim Düsterhus <tim@bastelstu.be>
6 years agoREGTEST/MINOR: Add a reg testing file for b406b87 commit.
Frédéric Lécaille [Fri, 24 Aug 2018 14:14:28 +0000 (16:14 +0200)] 
REGTEST/MINOR: Add a reg testing file for b406b87 commit.

6 years agoBUG/MEDIUM: mux_pt: dereference the connection with care in mux_pt_wake()
Willy Tarreau [Fri, 24 Aug 2018 13:48:59 +0000 (15:48 +0200)] 
BUG/MEDIUM: mux_pt: dereference the connection with care in mux_pt_wake()

mux_pt_wake() calls data->wake() which can return -1 indicating that the
connection was just destroyed. We need to check for this condition and
immediately exit in this case otherwise we dereference a just freed
connection. Note that this mainly happens on idle connections between
two HTTP requests. It can have random implications between requests as
it may lead a wrong connection's polling to be re-enabled or disabled
for example, especially with threads.

This patch must be backported to 1.8.

6 years agoREGEST/MINOR: Add reg testing files.
Frédéric Lécaille [Thu, 23 Aug 2018 16:06:35 +0000 (18:06 +0200)] 
REGEST/MINOR: Add reg testing files.

Reg testing files for a LUA bug fixed by commit 83ed5d5 ("BUG/MINOR:
lua: Bad HTTP client request duration.")

6 years agoBUG/MINOR: lua: Bad HTTP client request duration.
Frédéric Lécaille [Wed, 18 Jul 2018 12:25:26 +0000 (14:25 +0200)] 
BUG/MINOR: lua: Bad HTTP client request duration.

HTTP LUA applet callback should not update the date on which the HTTP client requests
arrive. This was done just after the LUA applet has completed its job.

This patch simply removes the affected statement. The same fixe has been applied
to TCP LUA applet callback.

To reproduce this issue, as reported by Patrick Hemmer, implement an HTTP LUA applet
which sleeps a bit before replying:

  core.register_service("foo", "http", function(applet)
      core.msleep(100)
      applet:set_status(200)
      applet:start_response()
  end)

This had as a consequence to log %TR field with approximatively the same value as
the LUA sleep time.

Thank you to Patrick Hemmer for having reported this issue.

Must be backported to 1.8, 1.7 and 1.6.

6 years agoMINOR: connection: make conn_sock_drain() work for all socket families
Willy Tarreau [Fri, 24 Aug 2018 12:31:53 +0000 (14:31 +0200)] 
MINOR: connection: make conn_sock_drain() work for all socket families

This patch improves the previous fix by implementing the socket draining
code directly in conn_sock_drain() so that it always applies regardless
of the protocol's family. Thus it gets rid of tcp_drain().

6 years agoBUG/MEDIUM: unix: provide a ->drain() function
Willy Tarreau [Fri, 24 Aug 2018 12:31:53 +0000 (14:31 +0200)] 
BUG/MEDIUM: unix: provide a ->drain() function

Right now conn_sock_drain() calls the protocol's ->drain() function if
it exists, otherwise it simply tries to disable polling for receiving
on the connection. This doesn't work well anymore since we've implemented
the muxes in 1.8, and it has a side effect with keep-alive backend
connections established over unix sockets. What happens is that if
during the idle time after a request, a connection reports some data,
si_idle_conn_null_cb() is called, which will call conn_sock_drain().
This one sees there's no drain() on unix sockets and will simply disable
polling for data on the connection. But it doesn't do anything on the
conn_stream. Thus while leaving the conn_fd_handler, the mux's polling
is updated and recomputed based on the conn_stream's polling state,
which is still enabled, and nothing changes, so we see the process
use 100% CPU in this case because the FD remains active in the cache.

There are several issues that need to be addressed here. The first and
most important is that we cannot expect some protocols to simply stop
reading data when asked to drain pending data. So this patch make the
unix sockets rely on tcp_drain() since the functions are the same. This
solution is appropriate for backporting, but a better one is desired for
the long term. The second issue is that si_idle_conn_null_cb() shouldn't
drain the connection but the conn_stream.

At the moment we don't have any way to drain a conn_stream, though a flag
on rcv_buf() will do it well. Until we support muxes on the server side
it is not a problem so this part can be addressed later.

This fix must be backported to 1.8.

6 years agoREGTEST/MINOR: Add a new class of regression testing files.
Frédéric Lécaille [Wed, 22 Aug 2018 08:41:33 +0000 (10:41 +0200)] 
REGTEST/MINOR: Add a new class of regression testing files.

Add LEVEL #4 regression testing files which is dedicated to
VTC files in relation with bugs they help to reproduce.
At the date of this commit, all VTC files are LEVEL 4 VTC files.

6 years agoREGTEST/MINOR: Missing mandatory "ignore_unknown_macro".
Frédéric Lécaille [Tue, 21 Aug 2018 13:29:24 +0000 (15:29 +0200)] 
REGTEST/MINOR: Missing mandatory "ignore_unknown_macro".

Since bbc34e2 varnish commit (for varnishtest), a new "cli"
macro is automatically created for each VTC script to dialog with
the CLI. Consequently, as this macro is unknown from higher level
code for varnishtest, it makes the scripts fail if we
we do not ask varnishtest to disregard the unknown macros.
To prevent this, from now on, for each VTC file for haproxy we MUST add
"feature ignore_unknown_macro" line to do so. This is mandatory

6 years agoDOC: Fix spelling error in configuration doc
Jens Bissinger [Thu, 23 Aug 2018 12:11:27 +0000 (14:11 +0200)] 
DOC: Fix spelling error in configuration doc

Fix spelling error in logging section of configuration doc.

6 years agoMINOR: sample: remove impossible tests on negative smp->data.u.str.data
Willy Tarreau [Wed, 22 Aug 2018 03:07:14 +0000 (05:07 +0200)] 
MINOR: sample: remove impossible tests on negative smp->data.u.str.data

Since commit 843b7cb ("MEDIUM: chunks: make the chunk struct's fields
match the buffer struct") a chunk length is unsigned so we can remove
negative size checks.

6 years agoMINOR: chunk: remove impossible tests on negative chunk->data
Willy Tarreau [Wed, 22 Aug 2018 02:59:48 +0000 (04:59 +0200)] 
MINOR: chunk: remove impossible tests on negative chunk->data

Since commit 843b7cb ("MEDIUM: chunks: make the chunk struct's fields
match the buffer struct") a chunk length is unsigned so we can remove
negative size checks.

6 years agoBUG/MEDIUM: cli/ssl: don't store base64dec() result in the trash's length
Willy Tarreau [Wed, 22 Aug 2018 03:26:57 +0000 (05:26 +0200)] 
BUG/MEDIUM: cli/ssl: don't store base64dec() result in the trash's length

By convenience or laziness we used to store base64dec()'s return code
into trash.data and to compare it against 0 to check for conversion
failure, but it's now unsigned since commit 843b7cb ("MEDIUM: chunks:
make the chunk struct's fields match the buffer struct"). Let's clean
this up and test the result itself without storing it first.

No backport is needed.

6 years agoBUG/MEDIUM: connection: don't store recv() result into trash.data
Willy Tarreau [Wed, 22 Aug 2018 03:20:32 +0000 (05:20 +0200)] 
BUG/MEDIUM: connection: don't store recv() result into trash.data

Cyril Bonté discovered that the proxy protocol randomly fails since
commit 843b7cb ("MEDIUM: chunks: make the chunk struct's fields match
the buffer struct"). This is because we used to store recv()'s return
code into trash.data which is now unsigned, so it never compares as
negative against 0. Let's clean this up and test the result itself
without storing it first.

No backport is needed.

6 years agoBUG/MEDIUM: map: don't store exp_replace() result in the trash's length
Willy Tarreau [Wed, 22 Aug 2018 02:55:43 +0000 (04:55 +0200)] 
BUG/MEDIUM: map: don't store exp_replace() result in the trash's length

By convenience or laziness we used to store exp_replace()'s return code
into str->data. The result checks applied there compare str->data to -1
while it's now unsigned since commit 843b7cb ("MEDIUM: chunks: make the
chunk struct's fields match the buffer struct"). Let's clean this up
and test the result itself without storing it first.

No backport is needed.

6 years agoBUG/MEDIUM: dns: don't store dns_build_query() result in the trash's length
Willy Tarreau [Wed, 22 Aug 2018 02:52:02 +0000 (04:52 +0200)] 
BUG/MEDIUM: dns: don't store dns_build_query() result in the trash's length

By convenience or laziness we used to store dns_build_query()'s return code
into trash.data. The result checks applied there compare trash.data to -1
while it's now unsigned since commit 843b7cb ("MEDIUM: chunks: make the
chunk struct's fields match the buffer struct"). Let's clean this up
and test the result itself without storing it first.

No backport is needed.

6 years agoBUG/MEDIUM: http: don't store url_decode() result in the samples's length
Willy Tarreau [Wed, 22 Aug 2018 03:08:57 +0000 (05:08 +0200)] 
BUG/MEDIUM: http: don't store url_decode() result in the samples's length

By convenience or laziness we used to store url_decode()'s return code
into smp->data.u.str.data. The result checks applied there compare it
to 0 while it's now unsigned since commit 843b7cb ("MEDIUM: chunks: make
the chunk struct's fields match the buffer struct "). Let's clean this up
and test the result itself without storing it first.

No backport is needed.

6 years agoBUG/MEDIUM: http: don't store exp_replace() result in the trash's length
Willy Tarreau [Wed, 22 Aug 2018 02:46:47 +0000 (04:46 +0200)] 
BUG/MEDIUM: http: don't store exp_replace() result in the trash's length

By convenience or laziness we used to store exp_replace()'s return code
into trash.data. The result checks applied there compare trash.data to -1
while it's now unsigned since commit 843b7cb ("MEDIUM: chunks: make the
chunk struct's fields match the buffer struct "). Let's clean this up
and test the result itself without storing it first.

No backport is needed.

6 years agoBUG/MINOR: chunks: do not store -1 into chunk_printf() in case of error
Willy Tarreau [Wed, 22 Aug 2018 03:14:37 +0000 (05:14 +0200)] 
BUG/MINOR: chunks: do not store -1 into chunk_printf() in case of error

Since commit 843b7cb ("MEDIUM: chunks: make the chunk struct's fields
match the buffer struct") a chunk length is unsigned so we can't reliably
store -1 and check for negative values in the caller. Only one such
location was found in proto_http's http-request auth rules (which cannot
realistically fail).

No backport is needed.

6 years agoBUG/MEDIUM: check/threads: do not involve the rendez-vous point for status updates
Willy Tarreau [Tue, 21 Aug 2018 17:54:09 +0000 (19:54 +0200)] 
BUG/MEDIUM: check/threads: do not involve the rendez-vous point for status updates

thread_isolate() is currently being called with the server lock held.
This is not acceptable because it prevents other threads from reaching
the rendez-vous point. Now that the LB algos are thread-safe, let's get
rid of this call.

No backport is nedeed.

6 years agoBUG/MEDIUM: lb/threads: always properly lock LB algorithms on maintenance operations
Willy Tarreau [Tue, 21 Aug 2018 17:44:53 +0000 (19:44 +0200)] 
BUG/MEDIUM: lb/threads: always properly lock LB algorithms on maintenance operations

Since commit 3ff577e ("MAJOR: server: make server state changes
synchronous again"), srv_update_status() calls the various maintenance
operations of the LB algorithms (->set_server_up, ->set_server_down,
->update_server_weight()). These ones are called with a single thread
guaranteed by the rendez-vous point, so the fact that they're lacking
some locks has no effect. However we'll need to remove the rendez-vous
point so we have to take care of properly locking all the LB algos.

The comments have been properly updated on the various functions to
mention their locking expectations. All these functions are called
with the server lock held, and all of them now support concurrent
calls by using the lbprm's lock.

This fix doesn't need to be backported at the moment, though if any
check-specific issue surfaced in 1.8, it could make sense to reuse it.

6 years agoBUG/MEDIUM: connection: don't forget to always delete the list's head
Willy Tarreau [Tue, 21 Aug 2018 16:33:20 +0000 (18:33 +0200)] 
BUG/MEDIUM: connection: don't forget to always delete the list's head

During a test it happened that a connection was deleted before the
stream it's attached to, resulting in a crash related to the fix
18a85fe ("BUG/MEDIUM: streams: Don't forget to remove the si from
the wait list.") during the LIST_DEL(). Make sure to always delete
the list's head in this case so that other elements can safely
detach later.

This is purely 1.9, no backport is needed.

6 years agoBUG/MAJOR: queue/threads: make pendconn_redistribute not lock the server
Willy Tarreau [Tue, 21 Aug 2018 16:11:03 +0000 (18:11 +0200)] 
BUG/MAJOR: queue/threads: make pendconn_redistribute not lock the server

Since commit 3ff577e ("MAJOR: server: make server state changes
synchronous again"), srv_update_status() is called with the server
lock held. It calls (among others) pendconn_redistribute() which used
to take this lock, causing CPU loops by default, or crashes if build
with -DDEBUG_THREAD. Since this function is not called from any other
place anymore, it doesn't require the lock on its own so let's simply
drop it from there.

No backport is needed, this is 1.9-specific.

6 years agoBUG/MEDIUM: stream_interface: Call the wake callback after sending.
Olivier Houchard [Tue, 21 Aug 2018 14:37:06 +0000 (16:37 +0200)] 
BUG/MEDIUM: stream_interface: Call the wake callback after sending.

If we subscribed to send, and the callback is called, call the wake callback
after, so that process_stream() may be woken up if needed.

This is 1.9-specific, no backport is needed.

6 years agoBUG/MEDIUM: H2: Activate polling after successful h2_snd_buf().
Olivier Houchard [Tue, 21 Aug 2018 14:36:10 +0000 (16:36 +0200)] 
BUG/MEDIUM: H2: Activate polling after successful h2_snd_buf().

Make sure h2_send() is called after h2_snd_buf() by activating polling.

This is 1.9-specific, no backport is needed.

6 years agoBUG/MEDIUM: stream-int: Check if the conn_stream exist in si_cs_io_cb.
Olivier Houchard [Tue, 21 Aug 2018 13:59:43 +0000 (15:59 +0200)] 
BUG/MEDIUM: stream-int: Check if the conn_stream exist in si_cs_io_cb.

It is possible that the conn_stream gets detached from the stream_interface,
and as it subscribed to the wait list, si_cs_io_cb() gets called anyway,
so make sure we have a conn_stream before attempting to send more data.

This is 1.9-specific, no backport is needed.

6 years agoBUG/MEDIUM: tasklets: Add the thread as active when waking a tasklet.
Olivier Houchard [Fri, 17 Aug 2018 16:57:51 +0000 (18:57 +0200)] 
BUG/MEDIUM: tasklets: Add the thread as active when waking a tasklet.

Set the flag for the current thread in active_threads_mask when waking a
tasklet, or we will never run it if no tasks are available.

This is 1.9-specific, no backport is needed.

6 years agoBUG/MEDIUM: streams: Don't forget to remove the si from the wait list.
Olivier Houchard [Tue, 21 Aug 2018 12:25:52 +0000 (14:25 +0200)] 
BUG/MEDIUM: streams: Don't forget to remove the si from the wait list.

When freeing the stream, make sure we remove the stream interfaces from the
wait lists, in case it was in there.

This is 1.9-specific, no backport is needed.

6 years agoBUG/MEDIUM: cli/threads: protect some server commands against concurrent operations
Willy Tarreau [Tue, 21 Aug 2018 13:35:31 +0000 (15:35 +0200)] 
BUG/MEDIUM: cli/threads: protect some server commands against concurrent operations

The server-specific CLI commands "set weight", "set maxconn",
"disable agent", "enable agent", "disable health", "enable health",
"disable server" and "enable server" were not protected against
concurrent accesses. Now they take the server lock around the
sensitive part.

This patch must be backported to 1.8.

6 years agoDOC: server/threads: document which functions need to be called with/without locks
Willy Tarreau [Tue, 21 Aug 2018 09:54:26 +0000 (11:54 +0200)] 
DOC: server/threads: document which functions need to be called with/without locks

At the moment it's totally unclear while reading the server's code which
functions require to be called with the server lock held and which ones
grab it and cannot be called this way. This commit simply inventories
all of them to indicate what is detected depending on how these functions
use the struct server. Only functions used at runtime were checked, those
dedicated to config parsing were skipped. Doing so already has uncovered
a few bugs on some CLI actions.

6 years agoBUG/MEDIUM: cli/threads: protect all "proxy" commands against concurrent updates
Willy Tarreau [Tue, 21 Aug 2018 12:50:44 +0000 (14:50 +0200)] 
BUG/MEDIUM: cli/threads: protect all "proxy" commands against concurrent updates

The proxy-related commands like "{enable|disable|shutdown} frontend",
"{enable|disable} dynamic-cookie", "set dynamic-cookie-key" were not
protected against concurrent accesses making their use dangerous with
threads.

This patch must be backported to 1.8.

6 years agoBUG/MEDIUM: server: update our local state before propagating changes
Willy Tarreau [Tue, 21 Aug 2018 06:22:26 +0000 (08:22 +0200)] 
BUG/MEDIUM: server: update our local state before propagating changes

Commit 3ff577e ("MAJOR: server: make server state changes synchronous again")
reintroduced synchronous server state changes. However, during the previous
change from synchronous to asynchronous, the server state propagation was
placed at the end of the function to ease the code changes, and the commit
above didn't put it back at its place. This has resulted in propagated
states to be incomplete. For example, making a server leave maintenance
would make it up but would leave its tracking servers down because they
see their tracked server is still down.

Let's just move the status update right to its place. It also adds the
benefit of reporting state changes in the order they appear and not in
reverse.

No backport is needed.

6 years agoBUG/MINOR: lua: fix extra 500ms added to socket timeouts
Cyril Bonté [Sun, 19 Aug 2018 20:08:50 +0000 (22:08 +0200)] 
BUG/MINOR: lua: fix extra 500ms added to socket timeouts

Since commit #56cc12509, haproxy accepts double values for timeouts. The
value is then converted to  milliseconds before being rounded up and cast
to int. The issue is that to round up the value, a constant value of 0.5
is added to it, but too early in the conversion, resulting in an
additional 500ms to the value. We are talking about a precision of 1ms,
so we can safely get rid of this rounding trick and adjust resulting
timeouts equal to 0 to a minimum of 1ms.

This patch is specific to the 1.9 branch and doesn't require to be
backported.

6 years agoBUG/MEDIUM: lua: socket timeouts are not applied
Cyril Bonté [Fri, 17 Aug 2018 21:51:02 +0000 (23:51 +0200)] 
BUG/MEDIUM: lua: socket timeouts are not applied

Sachin Shetty reported that socket timeouts set in LUA code have no effect.
Indeed, connect timeout is never modified and is always set to its default,
set to 5 seconds. Currently, this patch will apply the specified timeout
value to the connect timeout.
For the read and write timeouts, the issue is that the timeout is updated but
the expiration dates were not updated.

This patch should be backported up to the 1.6 branch.

6 years agoMINOR: fd cache: And the thread_mask with all_threads_mask.
Olivier Houchard [Fri, 17 Aug 2018 11:37:59 +0000 (13:37 +0200)] 
MINOR: fd cache: And the thread_mask with all_threads_mask.

When we choose to insert a fd in either the global or the local fd update list,
and the thread_mask against all_threads_mask before checking if it's tid_bit,
that way, if we run with nbthreads==1, we will always use the local list,
which is cheaper than the global one.

6 years agoMINOR: tasks: Don't special-case when nbthreads == 1
Olivier Houchard [Fri, 17 Aug 2018 11:36:08 +0000 (13:36 +0200)] 
MINOR: tasks: Don't special-case when nbthreads == 1

Instead of checking if nbthreads == 1, just and thread_mask with
all_threads_mask to know if we're supposed to add the task to the local or
the global runqueue.

6 years agoDOC: update the layering design notes
Willy Tarreau [Fri, 17 Aug 2018 07:58:29 +0000 (09:58 +0200)] 
DOC: update the layering design notes

Explain the change around cs_recv()/cs_send() and the move of the CS'
rxbuf and txbuf to the mux.

6 years agoDOC: ssl: Use consistent naming for TLS protocols
Bertrand Jacquin [Mon, 13 Aug 2018 23:56:13 +0000 (00:56 +0100)] 
DOC: ssl: Use consistent naming for TLS protocols

In most cases, "TLSv1.x" naming is used across and documentation, lazy
people tend to grep too much and may not find what they are looking for.

Fixing people is hard.

6 years agoDOC: add documentation for prio_class and prio_offset sample fetches.
Patrick Hemmer [Mon, 13 Aug 2018 18:07:57 +0000 (14:07 -0400)] 
DOC: add documentation for prio_class and prio_offset sample fetches.

This adds documentation that was missed as part of 268a707.

6 years agoDOC: dns: explain set server ... fqdn requires resolver
Lukas Tribus [Tue, 14 Aug 2018 09:39:35 +0000 (11:39 +0200)] 
DOC: dns: explain set server ... fqdn requires resolver

Abhishek Gupta reported on discourse that set server [...] fqdn always
fails. Further investigation showed that this requires the internal
DNS resolver to be configured. Add this requirement to the docs.

Must be backported to 1.8.

6 years agoBUG/MINOR: map: fix map_regm with backref
Emeric Brun [Tue, 17 Jul 2018 13:47:07 +0000 (09:47 -0400)] 
BUG/MINOR: map: fix map_regm with backref

Due to a cascade of get_trash_chunk calls the sample is
corrupted when we want to read it.

The fix consist to use a temporary chunk to copy the sample
value and use it.

[wt: for 1.8 and older, a backport was successfully tested here :
 https://www.mail-archive.com/haproxy@formilux.org/msg30694.html]

6 years agoBUG/MEDIUM: ssl: loading dh param from certifile causes unpredictable error.
Emeric Brun [Thu, 16 Aug 2018 13:14:12 +0000 (15:14 +0200)] 
BUG/MEDIUM: ssl: loading dh param from certifile causes unpredictable error.

If the dh parameter is not found, the openssl's error global
stack was not correctly cleared causing unpredictable error
during the following parsing (chain cert parsing for instance).

This patch should be backported in 1.8 (and perhaps 1.7)

6 years agoBUG/MEDIUM: ssl: fix missing error loading a keytype cert from a bundle.
Emeric Brun [Thu, 16 Aug 2018 13:11:12 +0000 (15:11 +0200)] 
BUG/MEDIUM: ssl: fix missing error loading a keytype cert from a bundle.

If there was an issue loading a keytype's part of a bundle, the bundle
was implicitly ignored without errors.

This patch should be backported in 1.8 (and perhaps 1.7)

6 years agoBUG/MEDIUM: sessions: Don't use t->state.
Olivier Houchard [Thu, 16 Aug 2018 17:03:50 +0000 (19:03 +0200)] 
BUG/MEDIUM: sessions: Don't use t->state.

In session_expire_embryonic(), don't use t->state, use the "state" argument
instead, as t->state has been cleaned before we're being called.

6 years agoBUG/MEDIUM: tasks: Don't insert in the global rqueue if nbthread == 1
Olivier Houchard [Thu, 16 Aug 2018 17:03:02 +0000 (19:03 +0200)] 
BUG/MEDIUM: tasks: Don't insert in the global rqueue if nbthread == 1

Make sure we don't insert a task in the global run queue if nbthread == 1,
as, as an optimisation, we avoid reading from it if nbthread == 1.

6 years agoMINOR: checks: Add event_srv_chk_io().
Olivier Houchard [Tue, 14 Aug 2018 15:04:58 +0000 (17:04 +0200)] 
MINOR: checks: Add event_srv_chk_io().

In checks, introduce event_srv_chk_io() as a callback to be called if data
can be sent again, instead of abusing event_srv_chk_w.

6 years agoMINOR: mux_h2: Don't use h2_send() as a callback.
Olivier Houchard [Thu, 2 Aug 2018 16:56:36 +0000 (18:56 +0200)] 
MINOR: mux_h2: Don't use h2_send() as a callback.

Instead of using h2_send() directly as a callback, introcude h2_io_cb(), that
will call h2_send() if it is possible to send data.

6 years agoMINOR: stream_interface: Give stream_interface its own wait_list.
Olivier Houchard [Thu, 2 Aug 2018 16:21:38 +0000 (18:21 +0200)] 
MINOR: stream_interface: Give stream_interface its own wait_list.

Instead of just using the conn_stream wait_list, give the stream_interface
its own. When the conn_stream will have its own buffers, the stream_interface
may have to wait on it.

6 years agoMINOR: stream_interface: Don't use si_cs_send() as a task handler.
Olivier Houchard [Thu, 2 Aug 2018 16:06:28 +0000 (18:06 +0200)] 
MINOR: stream_interface: Don't use si_cs_send() as a task handler.

Instead of using si_cs_send() as a task handler, define a new function,
si_cs_io_cb(), and give si_cs_send() its original prototype. Right now
si_cs_io_cb() just handles send, but later it'll handle recv() too.

6 years agoMINOR: connections/mux: Add the wait reason(s) to wait_list.
Olivier Houchard [Wed, 1 Aug 2018 15:06:43 +0000 (17:06 +0200)] 
MINOR: connections/mux: Add the wait reason(s) to wait_list.

Add a new element to the wait_list, that let us know which event(s) we are
waiting on.

6 years agoMINOR: tasks: Allow tasklet_wakeup() to wakeup a task.
Olivier Houchard [Wed, 1 Aug 2018 13:58:44 +0000 (15:58 +0200)] 
MINOR: tasks: Allow tasklet_wakeup() to wakeup a task.

Modify tasklet_wakeup() so that it handles a task as well, and inserts it
directly into the tasklet list, making it effectively a tasklet.
This should make future developments easier.