Willy Tarreau [Tue, 11 Dec 2018 18:50:43 +0000 (19:50 +0100)]
MEDIUM: mux-h1: implement true zero-copy of DATA blocks
When the mux's buffer is empty and the htx area contains exactly one
data block of the same size as the requested count, then it's possible
to simply swap the caller's buffer with the mux's output buffer and
adjust offsets and length to match the entire DATA HTX block in the
middle. In this case we perform a true zero-copy operation from
end-to-end. This is the situation that happens all the time with large
files. With this change, the HTX bit rate performance catches up again
with the legacy mode (measured at 97%).
These flags haven't been used for a while. SF_TUNNEL was reintroduced
by commit d62b98c6e ("MINOR: stream: don't set backend's nor response
analysers on SF_TUNNEL") to handle the two-level streams needed to
deal with the first model for H2, and was not removed after this model
was abandonned. SF_INITIALIZED was only set. SF_CONN_TAR was never
referenced at all.
Willy Tarreau [Tue, 11 Dec 2018 09:59:20 +0000 (10:59 +0100)]
REORG: h1: move legacy http functions to http_msg.c
Now that h1 and legacy HTTP are two distinct things, there's no need
to keep the legacy HTTP parsers in h1.c since they're only used by
the legacy code in proto_http.c, and h1.h doesn't need to include
hdr_idx anymore. This concerns the following functions :
Willy Tarreau [Tue, 11 Dec 2018 10:42:27 +0000 (11:42 +0100)]
REORG: http: create http_msg.c to place there some legacy HTTP parts
Lots of HTTP code still uses struct http_msg. Not only this code is
still huge, but it's part of the legacy interface. Let's move most
of these functions to a separate file http_msg.c to make it more
visible which file relies on what. It's mostly symmetrical with
what is present in http_htx.c.
The function http_transform_header_str() which used to rely on two
function pointers to look up a header was simplified to rely on
two variants http_legacy_replace_{,full_}header(), making both
sides of the function much simpler.
Willy Tarreau [Tue, 11 Dec 2018 09:22:41 +0000 (10:22 +0100)]
REORG: htx: merge types+proto into common/htx.h
All the HTX definition is self-contained and doesn't really depend on
anything external since it's a mostly protocol. In addition, some
external similar files (like h2) also placed in common used to rely
on it, making it a bit awkward.
This patch moves the two htx.h files into a single self-contained one.
The historical dependency on sample.h could be also removed since it
used to be there only for http_meth_t which is now in http.h.
MEDIUM: cache: Require an explicit filter declaration if other filters are used
As for the compression filter, the cache filter must be explicitly declared
(using the filter keyword) if other filters than cache are used. It is mandatory
to explicitly define the filters order.
MEDIUM: cache/compression: Add a way to safely combined compression and cache
This is only true for HTX proxies. On legacy HTTP proxy, if the compression and
the cache are both enabled, an error during HAProxy startup is triggered.
With the HTX, now you can use both in any order. If the compression is defined
before the cache, then the responses will be stored compressed. If the
compression is defined after the cache, then the responses will be stored
uncompressed. So in the last case, when a response is served from the cache, it
will compressed too like any response.
MINOR: cache: Improve and simplify the cache configuration check
To do so, a dedicated configuration has been added on cache filters. Before the
cache filter configuration pointed directly to the cache it used. Now, it is the
dedicated structure cache_flt_conf. Store and use rules also point to this
structure. It is linked to the cache the filter must used. It also contains a
flags field. This will allow us to define the behavior of a cache filter when a
response is stored in the cache or delivered from it.
And now, Store and use rules uses a common parsing function. So if it does not
already exists, a filter is always created for both kind of rules. The cache
filters configuration is checked using their check callback. In the postparser
function, we only check the caches configuration. This removes the loop on all
proxies in the postparser function.
MEDIUM: cache/htx: Add the HTX support into the cache
The cache is now able to store and resend HTX messages. When an HTX message is
stored in the cache, the headers are prefixed with their block's info (an
uint32_t), containing its type and its length. Data, on their side, are stored
without any prefix. Only the value is copied in the cache. 2 fields have been
added in the structure cache_entry, hdrs_len and data_len, to known the size, in
the cache, of the headers part and the data part. If the message is chunked, the
trailers are also copied, the same way as data. When the HTX message is
recreated in the cache applet, the trailers size is known removing the headers
length and the data lenght from the total object length.
MINOR: cache: Register the cache as a data filter only if response is cacheable
Instead of calling register_data_filter() when the stream analyze starts, we now
call it when we are sure the response is cacheable. It is done in the
http_headers callback, just before the body analyzis, and only if the headers
was already been cached. And during the body analyzis, if an error occurred or
if the response is too big, we unregistered the cache immediatly.
This patch may be backported in 1.8. It is not a bug but a significant
improvement.
MINOR: cache/htx: Don't use the same cache on HTX and legacy HTTP proxies
It is not possible to mix the format of messages stored in a cache. So we reject
the configurations with a cache used by an HTX proxy and a legacy HTTP proxy in
same time.
The CLI proxy was not handling payload. To do that, we needed to keep a
connection active on a server and to transfer each new line over that
connection until we receive a empty line.
The CLI proxy handles the payload in the same way that the CLI do it.
BUG/MINOR: cli: wait for payload data even without prompt
During a payload transfer, we need to wait for the data even when we are
not in interactive mode. Indeed, the data could be received line per
line progressively instead of in one recv.
Previously the CLI was doing a SHUTW just after the first line if it was
not in interactive mode. We now check if we are in payload mode to do
a SHUTW.
Willy Tarreau [Tue, 11 Dec 2018 15:37:42 +0000 (16:37 +0100)]
REORG: connection: centralize the conn_set_{tos,mark,quickack} functions
There were a number of ugly setsockopt() calls spread all over
proto_http.c, proto_htx.c and hlua.c just to manipulate the front
connection's TOS, mark or TCP quick-ack. These ones entirely relied
on the connection, its existence, its control layer's presence, and
its addresses. Worse, inet_set_tos() was placed in proto_http.c,
exported and used from the two other ones, surrounded in #ifdefs.
This patch moves this code to connection.h and makes the other ones
rely on it without ifdefs.
Willy Tarreau [Mon, 10 Dec 2018 18:26:51 +0000 (19:26 +0100)]
MINOR: hpack: provide a function to encode an HTTP path
The new function hpack_encode_path() supports encoding a path into
the ":path" header. It knows about "/" and "/index.html" which use
a single byte, and falls back to literal encoding for other ones,
with a fast path for short paths < 127 bytes.
Willy Tarreau [Mon, 10 Dec 2018 18:16:20 +0000 (19:16 +0100)]
MINOR: hpack: provide a function to encode an HTTP scheme
The new function hpack_encode_scheme() supports encoding a scheme
into the ":scheme" header. It knows about "https" and "http" which use
a single byte, and falls back to literal encoding for other ones.
Willy Tarreau [Mon, 10 Dec 2018 17:24:19 +0000 (18:24 +0100)]
MINOR: hpack: provide a function to encode an HTTP method
The new function hpack_encode_method() supports encoding a method.
It knows about GET and POST which use a single byte, and falls back
to literal encoding for other ones.
Willy Tarreau [Mon, 10 Dec 2018 17:06:40 +0000 (18:06 +0100)]
MEDIUM: mux-h2: make use of standard HPACK encoding functions for the status
This way we don't open-code the HPACK status codes anymore in the H2
code. Special care was taken not to cause any slowdown as this code is
very sensitive.
Willy Tarreau [Mon, 10 Dec 2018 17:04:42 +0000 (18:04 +0100)]
MINOR: hpack: provide new functions to encode the ":status" header
This header exists with 7 different values, it's worth taking them
into account for the encoding, hence these functions. One of them
makes use of an integer only and computes the 3 output bytes in case
of literal. The other one benefits from the knowledge of an existing
string, which for example exists in the case of H1 to H2 encoding.
Willy Tarreau [Tue, 11 Dec 2018 06:44:19 +0000 (07:44 +0100)]
MINOR: hpack: provide a function to encode a long indexed header
For long header values whose index is known, hpack_encodde_long_idx()
may now be used. This function emits the short index and follows with
the header's value.
Willy Tarreau [Tue, 11 Dec 2018 05:16:45 +0000 (06:16 +0100)]
MINOR: hpack: provide a function to encode a short indexed header
Most direct calls to HPACK functions are made to encode short header
fields like methods, schemes or statuses, whose lengths and indexes
are known. Let's have a small function to do this.
Willy Tarreau [Tue, 11 Dec 2018 06:44:00 +0000 (07:44 +0100)]
MINOR: hpack: move the length computation and encoding functions to .h
We'll need these functions from other inline functions, let's make them
accessible. len_to_bytes() was renamed to hpack_len_to_bytes() since it's
now exposed.
Willy Tarreau [Mon, 10 Dec 2018 14:19:14 +0000 (15:19 +0100)]
MEDIUM: hpack: make it possible to encode any static header name
We used to have a series of well-known header fields that were looked
up, but most of them were not. The current model couldn't scale with
the addition of the new headers or pseudo-headers required to process
requests, resulting in their encoding being hard-coded in the caller.
This patch implements a quick lookup which retrieves any header from
the static table. A binary stream is made of header names prefixed by
lengths and indexes. These header names are sorted by length, then by
frequency, then by direction (preference for response), then by name,
the the lowest index of each is stored only in case of multiple
entries. A parallel length index table provides the index of the first
header for a given string. This allows to focus on the first few values
matching the same length.
Everything was made to limit the cache footprint. Interestingly, the
lookup ends up being slightly faster than the previous one, while
covering the 54 distinct headers instead of only 10.
A test with a curl request and a basic response showed that the request
size has dropped from 85 to 56 bytes and that the response size has
dropped from 197 to 170 bytes, thus we can now shave roughly 25-30 bytes
per message.
Willy Tarreau [Mon, 10 Dec 2018 14:26:35 +0000 (15:26 +0100)]
CONTRIB: hpack: add a compressed stream generator for the encoder
This generates the tables and indexes which will be used by the HPACK
encoder. The headers are sorted by length, then by statistical frequency,
then by direction (preference for responses), then by name, then by index.
The purpose is to speed up their lookup.
Willy Tarreau [Tue, 11 Dec 2018 05:42:01 +0000 (06:42 +0100)]
MINOR: hpack: optimize header encoding for short names
For unknown fields, since we know that most of them are less than 127
characters, we don't need to go through the loop and can instead directly
emit the one-byte length encoding. This increases the request rate by
approximately 0.5%.
Willy Tarreau [Tue, 11 Dec 2018 05:27:06 +0000 (06:27 +0100)]
MINOR: hpack: use ist2bin() to copy header names in hpack_encode_header()
memcpy() tends to be overkill to copy short strings, better use ist's
naive functions for this. This shows a consistent 1.2% performance
gain with h2load.
Willy Tarreau [Mon, 10 Dec 2018 12:36:56 +0000 (13:36 +0100)]
MINOR: hpack: simplify the len to bytes conversion
The len-to-bytes conversion can be slightly simplified and optimized
by hardcoding a tree lookup. Just doing this increases by 1% the
request rate on H2. It could be made almost branch-free by using
fls() but it looks overkill for most situations since most headers
are very short.
Willy Tarreau [Tue, 11 Dec 2018 05:46:03 +0000 (06:46 +0100)]
BUG/MINOR: hpack: fix off-by-one in header name encoding length calculation
In hpack_encode_header() there is a length check to verify that a literal
header name fits in the buffer, but there it an off-by-one in this length
check, which forgets the byte required to mark the encoding type (literal
without indexing). It should be harmless though as it cannot be triggered
since response headers passing through haproxy are limited by the reserve,
which is not the case of the output buffer.
BUG/MEDIUM: htx: Always do a defrag if a block value is replace by a bigger one
Otherwise, after such replaces, the HTX message appears to wrap but the head
block address is not necessarily the first one. So adding new blocks will
override data of old ones.
BUG/MEDIUM: mux-h1: Don't loop on the headers parsing if the read0 was received
If a server sends part of headers and then close its connection, the mux H1
reamins blocked in an infinite loop trying to read more data to finish the
parsing of the message. The flag CS_FL_REOS is set on the conn_stream. But
because there are some data in the input buffer, CS_FL_EOS is never set.
To fix the bug, in h1_process_input, when CS_FL_REOS is set on the conn_stream,
we also set CS_FL_EOS if the input buffer is empty OR if the channel's buffer is
empty.
BUG/MEDIUM: mux-h1: Add a BUSY mode to not loop on pipelinned requests
When a request is fully processed, no more data are parsed until the response is
totally processed and a new transaction starts. But during this time, the mux is
trying to read more data and subscribes to read. If requests are pipelined, we
start to receive the next requests which will stay in the input buffer, leading
to a loop consuming all the CPU. This loop ends when the transaction ends. To
avoid this loop, the flag H1C_F_IN_BUSY has been added. It is set when the
request is fully parsed and unset when the transaction ends. Once set on H1C, it
blocks the reads. So the mux never tries to receive more data in this state.
BUG/MINOR: mux-h1: Fix conn_mode processing for headerless outgoing messages
Condition to process the connection mode on outgoing messages whithout
'Connection' header was wrong. It relied on the wrong H1M
state. H1_MSG_HDR_L2_LWS is only a possible state for messages with at least one
header. Now, to fix the bug, we just check the H1M state is not
H1_MSG_LAST_LF. So, we have the warranty the EOH was not processed yet.
Willy Tarreau [Mon, 10 Dec 2018 10:08:04 +0000 (11:08 +0100)]
BUG/MEDIUM: mux-h2: fix encoding of non-GET/POST methods
Jerome reported that outgoing H2 failed for methods different from GET
or POST. It turns out that the HPACK encoding is performed by hand in
the outgoing headers encoding function and that the data length was not
incremented to cover the literal method value, resulting in a corrupted
HEADERS frame.
Admittedly this code should move to the generic HPACK code.
PiBa-NL [Sat, 8 Dec 2018 18:48:37 +0000 (19:48 +0100)]
REGTEST/MINOR: remove double body specification for server txresp
fix http-rules/h00000.vtc / http-rules/h00000.vtc as both 'bodylen' and
'body' are specified, these settings conflict with each other as they
both generate/present the body to send.
BUG/MEDIUM: connection: Don't use the provided conn_stream if it was tried.
In connect_server(), don't attempt to reuse the conn_stream associated to
the stream_interface, if we already attempted a connection with it.
Using that conn_stream is only there for the cases where a connection and
a conn_stream was created ahead, mostly by http_proxy or by the LUA code.
If we already attempted to connect, that means we fail, and so we should
create a new connection.
Willy Tarreau [Sat, 8 Dec 2018 15:20:55 +0000 (16:20 +0100)]
[RELEASE] Released version 1.9-dev10
Released version 1.9-dev10 with the following main changes :
- MINOR: htx: Rename functions htx_*_to_str() to be H1 specific
- BUG/MINOR: htx: Force HTTP/1.1 on H1 formatting when version is 1.1 or above
- BUG/MINOR: fix ssl_fc_alpn and actually add ssl_bc_alpn
- BUG/MEDIUM: mworker: stop proxies which have no listener in the master
- BUG/MEDIUM: h1: Destroy a connection after detach if it has no owner.
- BUG/MEDIUM: h2: Don't forget to wake the tasklet after shutr/shutw.
- BUG/MINOR: flt_trace/compression: Use the right flag to add the HTX support
- BUG/MEDIUM: stream_interface: Make REALLY sure we read all the data.
- MEDIUM: mux-h1: Revamp the way subscriptions are handled.
- BUG/MEDIUM: mux-h1: Always set CS_FL_RCV_MORE when data are received in h1_recv()
- MINOR: mux-h1: Make sure to return 1 in h1_recv() when needed
- BUG/MEDIUM: mux-h1: Release the mux H1 in h1_process() if there is no h1s
- BUG/MINOR: proto_htx: Truncate the request when an error is detected
- BUG/MEDIUM: h2: When sending in HTX, make sure the caller knows we sent all.
- BUG/MEDIUM: mux-h2: properly update the window size in HTX mode
- BUG/MEDIUM: mux-h2: make sure to always report HTX EOM when consumed by headers
- BUG/MEDIUM: mux-h2: stop sending HTX once the mux is blocked
- BUG/MEDIUM: mux-h2: don't send more HTX data than requested
- MINOR: mux-h2: stop on non-DATA and non-EOM HTX blocks
- BUG/MEDIUM: h1: Correctly report used data with no len.
- MEDIUM: h1: Realign the ibuf before calling rcv_buf if needed.
- BUG/MEDIUM: mux_pt: Always set CS_FL_RCV_MORE.
- MINOR: htx: make htx_from_buf() adjust the size only on new buffers
- MINOR: htx: add buf_room_for_htx_data() to help optimize buffer transfers
- MEDIUM: mux-h1: make use of buf_room_for_htx_data() instead of b_room()
- MEDIUM: mux-h1: attempt to zero-copy Rx DATA transfers
- MEDIUM: mux-h1: avoid a double copy on the Tx path whenever possible
- BUG/MEDIUM: stream-int: don't mark as blocked an empty buffer on Rx
- BUG/MINOR: mux-h1: Check h1m flags to set the server conn_mode on request path
- MEDIUM: htx: Rework conversion from a buffer to an htx structure
- MEDIUM: channel/htx: Add functions for forward HTX data
- MINOR: mux-h1: Don't adjust anymore the amount of data sent in h1_snd_buf()
- CLEANUP: htx: Fix indentation here and there in HTX files
- MINOR: mux-h1: Allow partial data consumption during outgoing data processing
- BUG/MEDIUM: mux-h2: use the correct offset for the HTX start line
- BUG/MEDIUM: mux-h2: stop sending using HTX on errors
- MINOR: mux-h1: Drain obuf if the output is closed after sending data
- BUG/MEDIUM: mworker: stop every tasks in the master
- BUG/MEDIUM: htx: Set the right start-line offset after a defrag
- BUG/MEDIUM: stream: Don't dereference s->txn when it is not there yet.
- BUG/MEDIUM: connections: Reuse an already attached conn_stream.
- MINOR: stream-int: add a new blocking condition on the remote connection
- BUG/MEDIUM: stream-int: don't attempt to receive if the connection is not established
- BUG/MEDIUM: lua: block on remote connection establishment
- BUG/MEDIUM: mworker: fix several typos in mworker_cleantasks()
- SCRIPTS/REGTEST: merge grep+sed into sed in run-regtests
- BUG/MEDIUM: connections: Split CS_FL_RCV_MORE into 2 flags.
- BUG/MEDIUM: h1: Don't free the connection if it's an outgoing connection.
- BUG/MEDIUM: h1: Set CS_FL_REOS if we had a read0.
- BUG/MEDIUM: mux-h1: Be sure to have a conn_stream to set CS_FL_REOS in h1_recv
- REGTEST: Move LUA reg test 4 to level 1.
- MINOR: ist: add functions to copy/uppercase/lowercase into a buffer or string
- MEDIUM: ist: always turn header names to lower case
- MINOR: h2: don't turn HTX header names to lower case anymore
- MEDIUM: ist: use local conversion arrays to case conversion
- MINOR: htx: switch to case sensitive search of lower case header names
- MINOR: mux-h1: Set CS_FL_EOS when read0 is detected and no data are pending
- BUG/MINOR: stream-int: Process read0 even if no data was received in si_cs_recv
- REGTEST: fix the Lua test file name in test lua/h00002 :-)
- REGTEST: add a basic test for HTTP rules manipulating headers
- BUG/MEDIUM: sample: Don't treat SMP_T_METH as SMP_T_STR.
- MINOR: sample: add bc_http_major
- BUG/MEDIUM: htx: fix typo in htx_replace_stline() making it fail all the time
- REGTEST: make the HTTP rules test compatible with HTTP/2 as well
- BUG/MEDIUM: h2: Don't try to chunk data when using HTX.
- MINOR: compiler: add a new macro ALREADY_CHECKED()
- BUILD: h2: mark the start line already checked to avoid warnings
- BUG/MINOR: mux-h1: Remove the connection header when it is useless
BUG/MINOR: mux-h1: Remove the connection header when it is useless
When the connection mode can be deduced from the HTTP version, we remove the
redundant connection header. So "keep-alive" connection header is removed from
HTTP/1.1 messages and "close" connection header is remove from HTTP/1.0
messages.
Willy Tarreau [Sat, 8 Dec 2018 14:30:46 +0000 (15:30 +0100)]
BUILD: h2: mark the start line already checked to avoid warnings
Gcc 7 warns about a potential null pointer deref that cannot happen
since the start line block is guaranteed to be present in the functions
where it's dereferenced. Let's mark it as already checked.
Willy Tarreau [Sat, 8 Dec 2018 14:20:43 +0000 (15:20 +0100)]
MINOR: compiler: add a new macro ALREADY_CHECKED()
This macro may be used to block constant propagation that lets the compiler
detect a possible NULL dereference on a variable resulting from an explicit
assignment in an impossible check. Sometimes a function is called which does
safety checks and returns NULL if safe conditions are not met. The place
where it's called cannot hit this condition and dereferencing the pointer
without first checking it will make the compiler emit a warning about a
"potential null pointer dereference" which is hard to work around. This
macro "washes" the pointer and prevents the compiler from emitting tests
branching to undefined instructions. It may only be used when the developer
is absolutely certain that the conditions are guaranteed and that the
pointer passed in argument cannot be NULL by design.
A typical use case is a top-level function doing this :
Above parse_frame() is only called for frame->type == HEADERS so it will
never get a NULL in return from frame_hdr_start(). Thus it's always safe
to dereference *frame_hdr since the check was already performed above.
It's then safe to address it this way instead of inventing dummy error
code paths that may create real bugs :
BUG/MEDIUM: h2: Don't try to chunk data when using HTX.
When we're using HTX, we don't have to generate chunk header/trailers, and
that ultimately leads to a crash when we try to access a buffer that
contains just chunk trailers.
Willy Tarreau [Fri, 7 Dec 2018 17:21:21 +0000 (18:21 +0100)]
REGTEST: make the HTTP rules test compatible with HTTP/2 as well
The HTTP rules test now runs an H1 and an H2 client. Since the H2 one
requires the "proto" directive on the bind line, a new file has been
created requiring version 1.9 and the previous one was marked as usable
below 1.9 so that it's skipped by default but still usable when testing
backports.
Jérôme Magnin [Fri, 7 Dec 2018 08:03:11 +0000 (09:03 +0100)]
MINOR: sample: add bc_http_major
This adds the sample fetch bc_http_major. It returns the backend connection's HTTP
version encoding, which may be 1 for HTTP/0.9 to HTTP/1.1 or 2 for HTTP/2.0. It is
based on the on-wire encoding, and not the version present in the request header.
Willy Tarreau [Fri, 7 Dec 2018 14:18:54 +0000 (15:18 +0100)]
REGTEST: add a basic test for HTTP rules manipulating headers
There is always a risk of breaking HTTP processing when performing certain
code changes. This test modifies a request's start line, uses variables,
adds and modifies headers, interleaves them with the start-line changes,
and makes use of different header formats involving duplicated names,
duplicated values, empty fields and spaces around values. These operations
are performed both in the frontend and in the backend, for both the request
and the response. A CRC is computed on the concatenation of all the values,
and the concatenations are sent as individual header fields as well to help
debugging when the test fails.
The test reliably works since 1.6, implying that the HTTP processing did
not change. It currently fails on HTX.
BUG/MINOR: stream-int: Process read0 even if no data was received in si_cs_recv
The flag CS_FL_EOS can be set while no data was received. So the flas
CS_FL_RCV_MORE is not set. In this case, the read0 was never processed by the
stream interface. To be sure to process it, the test on CS_FL_RCV_MORE has been
moved after the one on CS_FL_EOS.
Willy Tarreau [Fri, 7 Dec 2018 08:40:01 +0000 (09:40 +0100)]
MEDIUM: ist: use local conversion arrays to case conversion
Calling tolower/toupper for each character is slow, a lookup into a
256-byte table is cheaper, especially for common characters used in
header field names which all fit into a cache line. Let's create these
two variables marked weak so that they're included only once.
Willy Tarreau [Fri, 7 Dec 2018 07:57:20 +0000 (08:57 +0100)]
MINOR: h2: don't turn HTX header names to lower case anymore
Since HTX stores header names in lower case already, we don't need to
do it again anymore. This increased H2 performance by 2.7% on quick
tests, now making H2 overr HTX about 5.5% faster than H2 over H1.
Willy Tarreau [Fri, 7 Dec 2018 07:47:45 +0000 (08:47 +0100)]
MEDIUM: ist: always turn header names to lower case
HTTP/2 and above require header names to be lower cased, while HTTP/1
doesn't care. By making lower case the standard way to store header
names in HTX, we can significantly simplify all operations applying to
header names retrieved from HTX (including, but not limited to, lookups
and lower case checks which are not needed anymore).
As a side effect of replacing memcpy() with ist2bin_lc(), a small increase
of the request rate performance of about 0.5-1% was noticed on keep-alive
traffic, very likely due to memcpy() being overkill for tiny strings.
This trivial patch was marked medium because it may have a visible end-user
impact (e.g. non-HTTP compliant agent, etc).
Willy Tarreau [Fri, 7 Dec 2018 07:35:07 +0000 (08:35 +0100)]
MINOR: ist: add functions to copy/uppercase/lowercase into a buffer or string
The ist functions were missing functions to copy an IST into a target
buffer, making some code have to resort to memcpy(), which tends to be
overkill for small strings, that the compiler cannot guess. In addition
sometimes there is a need to turn a string to lower or upper case so it
had to be overwritten after the operation.
This patch adds 6 functions to copy an ist to a buffer, as binary or as a
string (i.e. a zero is or is not appended), and optionally to apply a
lower case or upper case transformation on the fly.
A number of tests were performed to optimize the processing for small
strings. The loops are marked unlikely to dissuade the compilers from
over-optimizing them and switching to SIMD instructions. The lower case
or upper case transformations used to rely on external functions for
each character and to crappify the code due to clobbered registers,
which is not acceptable when we know that only a certain class of chars
has to be transformed, so the test was open-coded.
BUG/MEDIUM: mux-h1: Be sure to have a conn_stream to set CS_FL_REOS in h1_recv
In the commit 6a2d33481 ("BUG/MEDIUM: h1: Set CS_FL_REOS if we had a read0."),
We set the flag CS_FL_REOS on the conn_stream when a read0 is detected. But we
must be sure to have a conn_stream first.
BUG/MEDIUM: h1: Don't free the connection if it's an outgoing connection.
In h1_process(), don't release the connection if it is an outgoing connection
and we don't have an h1s associated, if it is so it is probably just in
a pool.
BUG/MEDIUM: connections: Split CS_FL_RCV_MORE into 2 flags.
CS_FL_RCV_MORE is used in two cases, to let the conn_stream
know there may be more data available, and to let it know that
it needs more room. We can't easily differentiate between the
two, and that may leads to hangs, so split it into two flags,
CS_FL_RCV_MORE, that means there may be more data, and
CS_FL_WANT_ROOM, that means we need more room.
Willy Tarreau [Thu, 6 Dec 2018 14:49:27 +0000 (15:49 +0100)]
SCRIPTS/REGTEST: merge grep+sed into sed in run-regtests
Some commands chain grep and sed while sed already does grep by
default, let's simply use sed. In addition to being more intuitive,
it saves up to 150ms per run on the 13 tests covered by level 4.
BUG/MEDIUM: mworker: fix several typos in mworker_cleantasks()
Commit 27f3fa5 ("BUG/MEDIUM: mworker: stop every tasks in the master")
used MAX_THREADS as a mask instead of MAX_THREADS_MASK to clean the
global run queue, and used rq_next (global variable) instead of next_rq.
Renamed next_rq as tmp_rq and next_wq as tmp_wq to avoid confusion.
Willy Tarreau [Thu, 6 Dec 2018 14:29:50 +0000 (15:29 +0100)]
BUG/MEDIUM: lua: block on remote connection establishment
We used to wait for the other side to be connected, but the blocking
flags were inaccurate. It used to work fine almost by accident before
the stream interface changes. Now we use the new RXBLK_CONN flag to
explicitly subscribe to this event.
Thanks to Adis for reporting the issue, PiBaNL for the test case,
and Olivier for the diagnostic.
Willy Tarreau [Thu, 6 Dec 2018 14:25:58 +0000 (15:25 +0100)]
BUG/MEDIUM: stream-int: don't attempt to receive if the connection is not established
If we try to receive before the connection is established, we lose the
send event and are not woken up anymore once the connection is established.
This was diagnosed by Olivier.
Willy Tarreau [Thu, 6 Dec 2018 14:24:01 +0000 (15:24 +0100)]
MINOR: stream-int: add a new blocking condition on the remote connection
There are some situations where we need to wait for the other side to
be connected. None of the current blocking flags support this. It used
to work more or less by accident using the old flags. Let's add a new
flag to mention we're blocking on this, it's removed by si_chk_rcv()
when a connection is established. It should be enough for now.
BUG/MEDIUM: connections: Reuse an already attached conn_stream.
In connect_server(), if we already have a conn_stream, reuse it
instead of trying to create a new one. http_proxy and LUA both
manually create a conn_stream and a connection, and we want
to use it.
BUG/MEDIUM: htx: Set the right start-line offset after a defrag
The offset was always wrong after an HTX defragmentation because the wrong
address was used and because the update could occcur several time on the same
defragmentation.
BUG/MEDIUM: mworker: stop every tasks in the master
The master is not supposed to run (at the moment) any task before the
polling loop, the created tasks should be run only in the workers but in
the master they should be disabled or removed.
MINOR: mux-h1: Drain obuf if the output is closed after sending data
It avoids to subscribe to send events because some may remain in the output
buffer. If the output is closed or if an error occurred, there is no way to send
these data anyway, so it is safe to drain them.
Willy Tarreau [Thu, 6 Dec 2018 13:07:27 +0000 (14:07 +0100)]
BUG/MEDIUM: mux-h2: use the correct offset for the HTX start line
Due to a thinko, I used sl_off as the start line index number but it's
not it, it's its offset. The first index is obtained using htx_get_head(),
and the start line is obtained using htx_get_sline(). This caused crashes
to happen when forwarding HTX traffic via the H2 mux once the HTX buffer
started to wrap.
MINOR: mux-h1: Allow partial data consumption during outgoing data processing
In h1_process_output(), instead of waiting to have enough data to send to
consume a full block of data, we are now able consume partially these blocks.
MEDIUM: channel/htx: Add functions for forward HTX data
To ease the fast forwarding and the infinte forwarding on HTX proxies, 2
functions have been added to let the channel be almost aware of the way data are
stored in its buffer. By calling these functions instead of legacy ones, we are
sure to forward the right amount of data.
MEDIUM: htx: Rework conversion from a buffer to an htx structure
Now, the function htx_from_buf() will set the buffer's length to its size
automatically. In return, the caller should call htx_to_buf() at the end to be
sure to leave the buffer hosting the HTX message in the right state. When the
caller can use the function htxbuf() to get the HTX message without any update
on the underlying buffer.
BUG/MINOR: mux-h1: Check h1m flags to set the server conn_mode on request path
On the server side, we must test the request headers to deduce if we able to do
keepalive or not. Otherwise, by default, the keepalive will be enabled on the
server's connection, whatever the client said.
Willy Tarreau [Wed, 5 Dec 2018 12:45:41 +0000 (13:45 +0100)]
BUG/MEDIUM: stream-int: don't mark as blocked an empty buffer on Rx
After 8706c8131 ("BUG/MEDIUM: mux_pt: Always set CS_FL_RCV_MORE."), a
side effect caused failed receives to mark the buffer as missing room,
a flag that no other place can remove since it's empty. Ideally we need
a separate flag to mean "failed to deliver data by lack of room", but
in the mean time at the very least we must not mark as blocked an
empty buffer.
Willy Tarreau [Wed, 5 Dec 2018 10:19:27 +0000 (11:19 +0100)]
MEDIUM: mux-h1: avoid a double copy on the Tx path whenever possible
In order to properly deal with unaligned contents, the output data are
currently copied into a temporary buffer, to be copied into the mux's
output buffer at the end. The new buffer API allows several buffers to
share the same data area, so we're using this here to make the temporary
buffer point to the same area as the output buffer when that one is
empty. This is enough to avoid the copy at the end, only pointers and
lengths have to be adjusted. In addition the output buffer's head is
advanced by the HTX header size so that the remaining copy is aligned.
By doing this we improve the large object performance by an extra 10%,
which is 64% above the 1.9-dev9 state. It's worth noting that there are
no more calls to __memcpy_sse2_unaligned() now.
Since this code deals with various block types, it appears difficult to
adjust it to be smart enough to even avoid the first copy. However a
distinct approach could consist in trying to detect a single blocked
HTX and jump to dedicated code in this case.
Willy Tarreau [Wed, 5 Dec 2018 09:02:39 +0000 (10:02 +0100)]
MEDIUM: mux-h1: attempt to zero-copy Rx DATA transfers
When transferring large objects, most calls are made between a full
buffer and an empty buffer. In this case there is a large opportunity
for performing zero-copy calls, with a few exceptions : the input data
must fit into the output buffer, and the data need to be properly
aligned and formated to let the HTX header fit before and the HTX
block(s) fit after.
This patch does two things :
1) it makes sure that we prepare an empty input buffer before an recv()
call so that it appears as holding an HTX block at the front, which is
removed afterwards. This way the data received using recv() are placed
exactly at the target position in the input buffer for a later cast to
HTX.
2) when receiving data in h1_process_data(), if it appears that the input
buffer can be cast to an HTX buffer and the target buffer is empty,
then the buffers are swapped, an HTX block is prepended in front of the
data area, and the HTX block is appended to reference this data block.
In practice, this ensures that in most cases when transferring large files,
calls to h1_rcv_buf() are made using zero copy and a little bit of buffer
preparation (~40 bytes to be written).
Doing this adds an extra 13% performance boost on top of previous patch,
resulting in a total of 50% speed up on large transfers.
Willy Tarreau [Wed, 5 Dec 2018 06:59:27 +0000 (07:59 +0100)]
MEDIUM: mux-h1: make use of buf_room_for_htx_data() instead of b_room()
Just by using this buffer room estimation for the demux buffer, the large
object performance has increased by up to 33%. This is mostly due to less
recv() calls and unaligned copies.