]> git.ipfire.org Git - thirdparty/haproxy.git/log
thirdparty/haproxy.git
6 years agoMEDIUM: chunks: make the chunk struct's fields match the buffer struct
Willy Tarreau [Fri, 13 Jul 2018 08:54:26 +0000 (10:54 +0200)] 
MEDIUM: chunks: make the chunk struct's fields match the buffer struct

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

It's not needed anymore.

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

This slightly simplifies the code.

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

These are the last ones which need to be adapted.

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

Almost nothing required to be touched.

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

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

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

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

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

We don't need this function anymore.

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

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

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

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

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

Also, ci_swpbuf() was removed (unused).

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

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

6 years agoMINOR: channel: remove almost all references to buf->i and buf->o
Willy Tarreau [Fri, 15 Jun 2018 14:42:02 +0000 (16:42 +0200)] 
MINOR: channel: remove almost all references to buf->i and buf->o

We use ci_data() and co_data() instead now everywhere we read these
values.

6 years agoMINOR: buffer: Use b_add()/bo_add() instead of accessing b->i/b->o.
Olivier Houchard [Thu, 28 Jun 2018 16:17:23 +0000 (18:17 +0200)] 
MINOR: buffer: Use b_add()/bo_add() instead of accessing b->i/b->o.

Use the newly available functions instead of using the buffer fields directly.

6 years agoMINOR: buffer: use b_orig() to replace most references to b->data
Willy Tarreau [Fri, 15 Jun 2018 15:21:00 +0000 (17:21 +0200)] 
MINOR: buffer: use b_orig() to replace most references to b->data

This patch updates most users of b->data to use b_orig().

6 years agoMINOR: buffer: use c_head() instead of buffer_wrap_sub(c->buf, p-o)
Willy Tarreau [Fri, 15 Jun 2018 13:18:17 +0000 (15:18 +0200)] 
MINOR: buffer: use c_head() instead of buffer_wrap_sub(c->buf, p-o)

This way we don't need o anymore.

6 years agoMINOR: buffer: replace buffer_flush() with c_adv(chn, ci_data(chn))
Willy Tarreau [Fri, 15 Jun 2018 13:16:51 +0000 (15:16 +0200)] 
MINOR: buffer: replace buffer_flush() with c_adv(chn, ci_data(chn))

It used to forward some input into output.

6 years agoMINOR: buffer: replace buffer_pending() with ci_data()
Willy Tarreau [Fri, 15 Jun 2018 13:14:53 +0000 (15:14 +0200)] 
MINOR: buffer: replace buffer_pending() with ci_data()

It used to return b->i for channels, which is what ci_data() does.

6 years agoMINOR: buffer: replace bi_space_for_replace() with ci_space_for_replace()
Willy Tarreau [Fri, 15 Jun 2018 13:06:42 +0000 (15:06 +0200)] 
MINOR: buffer: replace bi_space_for_replace() with ci_space_for_replace()

This one computes the size that can be overwritten over the input part
of the buffer, so it's channel-specific.

6 years agoMINOR: buffer: replace buffer_full() with channel_full()
Willy Tarreau [Fri, 15 Jun 2018 12:54:53 +0000 (14:54 +0200)] 
MINOR: buffer: replace buffer_full() with channel_full()

It's only used by channels since we need to know the amount of output
data.

6 years agoMINOR: buffer: make bo_putchar() use b_tail()
Willy Tarreau [Mon, 9 Jul 2018 08:31:30 +0000 (10:31 +0200)] 
MINOR: buffer: make bo_putchar() use b_tail()

It's possible because we can't call bo_putchar() with i != 0.

6 years agoMINOR: buffer: replace buffer_empty() with b_empty() or c_empty()
Willy Tarreau [Tue, 10 Jul 2018 07:53:31 +0000 (09:53 +0200)] 
MINOR: buffer: replace buffer_empty() with b_empty() or c_empty()

For the same consistency reasons, let's use b_empty() at the few places
where an empty buffer is expected, or c_empty() if it's done on a channel.
Some of these places were there to realign the buffer so
{b,c}_realign_if_empty() was used instead.

6 years agoMINOR: buffer: replace buffer_not_empty() with b_data() or c_data()
Willy Tarreau [Tue, 10 Jul 2018 07:50:25 +0000 (09:50 +0200)] 
MINOR: buffer: replace buffer_not_empty() with b_data() or c_data()

It's mostly for consistency as many places already use one of these instead.

6 years agoMINOR: buffer: use b_room() to determine available space in a buffer
Willy Tarreau [Fri, 15 Jun 2018 11:59:36 +0000 (13:59 +0200)] 
MINOR: buffer: use b_room() to determine available space in a buffer

We used to have variations around buffer_total_space() and
size-buffer_len() or size-b_data(). Let's simplify all this. buffer_len()
was also removed as not used anymore.

6 years agoMINOR: buffer: get rid of b_ptr() and convert its last users
Willy Tarreau [Fri, 15 Jun 2018 11:45:17 +0000 (13:45 +0200)] 
MINOR: buffer: get rid of b_ptr() and convert its last users

Now the new API functions are being used everywhere, we can get rid
of b_ptr(). A few last users like bi_istput() and bo_istput() appear
to only differ by what part of the buffer they're increasing, but
that should quickly be merged.

6 years agoMINOR: connection: add a new receive flag : CO_RFL_BUF_WET
Willy Tarreau [Tue, 19 Jun 2018 04:23:38 +0000 (06:23 +0200)] 
MINOR: connection: add a new receive flag : CO_RFL_BUF_WET

With this flag we introduce the notion of "dry" vs "wet" buffers : some
demultiplexers like the H2 mux require as much room as possible for some
operations that are not retryable like decoding a headers frame. For this
they need to know if the buffer is congested with data scheduled for
leaving soon or not. Since the new API will not provide this information
in the buffer itself, the caller must indicate it. We never need to know
the amount of such data, just the fact that the buffer is not in its
optimal condition to be used for receipt. This "CO_RFL_BUF_WET" flag is
used to mention that such outgoing data are still pending in the buffer
and that a sensitive receiver should better let it "dry" before using it.

6 years agoMINOR: connection: add a flags argument to rcv_buf()
Willy Tarreau [Tue, 19 Jun 2018 04:15:17 +0000 (06:15 +0200)] 
MINOR: connection: add a flags argument to rcv_buf()

The mux and transport rcv_buf() now takes a "flags" argument, just like
the snd_buf() one or like the equivalent syscall lower part. The upper
layers will use this to pass some information such as indicating whether
the buffer is free from outgoing data or if the lower layer may allocate
the buffer itself.

6 years agoMEDIUM: mux: make mux->rcv_buf() take a size_t for the count
Willy Tarreau [Wed, 18 Jul 2018 09:29:06 +0000 (11:29 +0200)] 
MEDIUM: mux: make mux->rcv_buf() take a size_t for the count

It also returns a size_t. This is in order to clean the API. Note
that the H2 mux still uses some ints in the functions called from
h2_rcv_buf(), though it's not really a problem given that H2 frames
are smaller. It may deserve a general cleanup later though.

6 years agoMEDIUM: connection: make xprt->rcv_buf() use size_t for the count
Willy Tarreau [Wed, 18 Jul 2018 09:22:03 +0000 (11:22 +0200)] 
MEDIUM: connection: make xprt->rcv_buf() use size_t for the count

Just like we have a size_t for xprt->snd_buf(), we adjust to use size_t
for rcv_buf()'s count argument and return value. It also removes the
ambiguity related to the possibility to see a negative value there.

6 years agoMEDIUM: mux: make mux->snd_buf() take the byte count in argument
Willy Tarreau [Thu, 14 Jun 2018 16:38:55 +0000 (18:38 +0200)] 
MEDIUM: mux: make mux->snd_buf() take the byte count in argument

This way the mux doesn't need to modify the buffer's metadata anymore
nor to know the output's size. The mux->snd_buf() function now takes a
const buffer and it's up to the caller to update the buffer's state.

The return type was updated to return a size_t to comply with the count
argument.

6 years agoMEDIUM: connection: make xprt->snd_buf() take the byte count in argument
Willy Tarreau [Thu, 14 Jun 2018 16:31:46 +0000 (18:31 +0200)] 
MEDIUM: connection: make xprt->snd_buf() take the byte count in argument

This way the senders don't need to modify the buffer's metadata anymore
nor to know about the output's split point. This way the functions can
take a const buffer and it's clearer who's in charge of updating the
buffer after a send. That's why the buffer realignment is now performed
by the caller of the transport's snd_buf() functions.

The return type was updated to return a size_t to comply with the count
argument.

6 years agoMINOR: buffer: make b_getblk_nc() take size_t for the block sizes
Willy Tarreau [Wed, 18 Jul 2018 09:49:27 +0000 (11:49 +0200)] 
MINOR: buffer: make b_getblk_nc() take size_t for the block sizes

Till now we used to reimplement it using ints to limit external changes
but we must adjust it and the various users to switch to size_t.

6 years agoMINOR: buffer: make b_getblk_nc() take const pointers
Willy Tarreau [Thu, 14 Jun 2018 13:27:31 +0000 (15:27 +0200)] 
MINOR: buffer: make b_getblk_nc() take const pointers

Now that there are no more users requiring to modify the buffer anymore,
switch these ones to const char and const buffer. This will make it more
obvious next time send functions are tempted to modify the buffer's output
count. Minor adaptations were necessary at a few call places which were
using char due to the function's previous prototype.

6 years agoMINOR: buffer: get rid of b_end() and b_to_end()
Willy Tarreau [Fri, 15 Jun 2018 11:47:29 +0000 (13:47 +0200)] 
MINOR: buffer: get rid of b_end() and b_to_end()

These ones are not used anymore.

6 years agoMEDIUM: h2: don't use b_ptr() nor b_end() anymore
Willy Tarreau [Fri, 15 Jun 2018 09:51:32 +0000 (11:51 +0200)] 
MEDIUM: h2: don't use b_ptr() nor b_end() anymore

The few places where they were still used were replaced with b_peek() and
b_wrap() respectively. The parts making use of ->i and ->o should now be
convertible to the new API.

6 years agoMEDIUM: h2: do not use buf->o anymore inside h2_snd_buf's loop
Willy Tarreau [Thu, 14 Jun 2018 14:54:01 +0000 (16:54 +0200)] 
MEDIUM: h2: do not use buf->o anymore inside h2_snd_buf's loop

buf->o is only retrieved at the loop entry and modified using b_del()
on exit. We're close to being able to change the API to take a count
argument.

6 years agoMINOR: h1: make h1_measure_trailers() use an offset and a count
Willy Tarreau [Thu, 14 Jun 2018 14:52:02 +0000 (16:52 +0200)] 
MINOR: h1: make h1_measure_trailers() use an offset and a count

This will be needed by the H2 encoder to restart after wrapping.

6 years agoMINOR: h1: make h1_parse_chunk_size() not depend on b_ptr() anymore
Willy Tarreau [Thu, 14 Jun 2018 13:59:05 +0000 (15:59 +0200)] 
MINOR: h1: make h1_parse_chunk_size() not depend on b_ptr() anymore

It's similar to the previous commit so that the function doesn't rely
on buf->p anymore.

6 years agoMINOR: h1: make h1_skip_chunk_crlf() not depend on b_ptr() anymore
Willy Tarreau [Thu, 14 Jun 2018 13:53:21 +0000 (15:53 +0200)] 
MINOR: h1: make h1_skip_chunk_crlf() not depend on b_ptr() anymore

It now takes offsets relative to the buffer's head. It's up to the
callers to add this offset which corresponds to the buffer's output
size.

6 years agoMEDIUM: h2: prevent the various mux encoders from modifying the buffer
Willy Tarreau [Thu, 14 Jun 2018 11:33:30 +0000 (13:33 +0200)] 
MEDIUM: h2: prevent the various mux encoders from modifying the buffer

Functions h2s_frt_make_resp_headers() and h2s_frt_make_resp_data() used
to modify the buffer's output data count. This is problematic for the
buffer's rework as we don't want to rely on this anymore. This commit
modifies these functions to take an offset (relative to the buffer's
head) and a maximum byte count. Thus h2_snd_buf() now calls them with
buf->o and takes care of removing deleted data itself. The send functions
now almost support being passed const buffers (except for the data part
which is still embedded).

6 years agoMINOR: h2: clarify the fact that the send functions are unsigned
Willy Tarreau [Thu, 14 Jun 2018 11:21:28 +0000 (13:21 +0200)] 
MINOR: h2: clarify the fact that the send functions are unsigned

There's no more error return combined with the send output, though
the comments were misleading. Let's fix this as well as the functions'
prototypes. h2_snd_buf()'s return value wasn't changed yet since it
has to match the ->snd_buf prototype.

6 years agoMINOR: h1: make h1_measure_trailers() take the byte count in argument
Willy Tarreau [Thu, 14 Jun 2018 11:32:50 +0000 (13:32 +0200)] 
MINOR: h1: make h1_measure_trailers() take the byte count in argument

The principle is that it should not have to take this value from the
buffer itself anymore.

6 years agoMINOR: buffer: convert most b_ptr() calls to c_ptr()
Willy Tarreau [Fri, 15 Jun 2018 09:11:53 +0000 (11:11 +0200)] 
MINOR: buffer: convert most b_ptr() calls to c_ptr()

The latter uses the channel wherever a channel is known.

6 years agoMINOR: buffer: replace bi_del() and bo_del() with b_del()
Willy Tarreau [Fri, 15 Jun 2018 08:28:05 +0000 (10:28 +0200)] 
MINOR: buffer: replace bi_del() and bo_del() with b_del()

Till now the callers had to know which one to call for specific use cases.
Let's fuse them now since a single one will remain after the API migration.
Given that bi_del() may only be used where o==0, just combine the two tests
by first removing output data then only input.

6 years agoMINOR: buffer: replace bo_getblk_nc() with b_getblk_nc() which takes an offset
Willy Tarreau [Thu, 14 Jun 2018 12:38:11 +0000 (14:38 +0200)] 
MINOR: buffer: replace bo_getblk_nc() with b_getblk_nc() which takes an offset

This will be important so that we can parse a buffer without touching it.
Now we indicate where from the buffer's head we plan to start to copy, and
for how many bytes. This will be used by send functions to loop at the end
of the buffer without having to update the buffer's output byte count.

6 years agoMINOR: buffer: replace bo_getblk() with direction agnostic b_getblk()
Willy Tarreau [Fri, 15 Jun 2018 12:20:26 +0000 (14:20 +0200)] 
MINOR: buffer: replace bo_getblk() with direction agnostic b_getblk()

This new functoin limits itself to the amount of data available in the
buffer and doesn't care about the direction anymore. It's only called
from co_getblk() which already checks that no more than the available
output bytes is requested.

6 years agoMINOR: buffer: merge b{i,o}_contig_space()
Willy Tarreau [Thu, 7 Jun 2018 16:58:07 +0000 (18:58 +0200)] 
MINOR: buffer: merge b{i,o}_contig_space()

These ones were merged into a single b_contig_space() that covers both
(the bo_ case was a simplified version of the other one). The function
doesn't use ->i nor ->o anymore.

6 years agoMINOR: buffer: remove bo_contig_data()
Willy Tarreau [Thu, 7 Jun 2018 17:03:40 +0000 (19:03 +0200)] 
MINOR: buffer: remove bo_contig_data()

The two call places now make use of b_contig_data(0) and check by
themselves that the returned size is no larger than the scheduled
output data.

6 years agoMINOR: buffer: remove bi_end()
Willy Tarreau [Thu, 7 Jun 2018 16:46:28 +0000 (18:46 +0200)] 
MINOR: buffer: remove bi_end()

It was replaced by ci_tail() when the channel is known, or b_tail() in
other cases.

6 years agoMINOR: buffer: remove bo_end()
Willy Tarreau [Thu, 7 Jun 2018 16:22:26 +0000 (18:22 +0200)] 
MINOR: buffer: remove bo_end()

It was replaced by either b_tail() when the buffer has no input data, or
b_peek(b, b->o).

6 years agoMINOR: buffer: remove bo_ptr()
Willy Tarreau [Thu, 7 Jun 2018 16:16:48 +0000 (18:16 +0200)] 
MINOR: buffer: remove bo_ptr()

It was replaced by co_head() when a channel was known, otherwise b_head().

6 years agoMINOR: buffer: remove bi_ptr()
Willy Tarreau [Thu, 7 Jun 2018 16:08:04 +0000 (18:08 +0200)] 
MINOR: buffer: remove bi_ptr()

It's now been replaced by b_head() when b->o is null, ci_head() when
the channel is known, or b_peek(b, b->o) in other situations.

6 years agoMINOR: buffer: split bi_contig_data() into ci_contig_data and b_config_data()
Willy Tarreau [Wed, 6 Jun 2018 14:55:45 +0000 (16:55 +0200)] 
MINOR: buffer: split bi_contig_data() into ci_contig_data and b_config_data()

This function was sometimes used from a channel and sometimes from a buffer.
In both cases it requires knowledge of the size of the output data (to skip
them). Here the split ensures the channel can deal with this point, and that
other places not having output data can continue to work.

6 years agoMINOR: buffer: remove bi_getblk() and bi_getblk_nc()
Willy Tarreau [Wed, 6 Jun 2018 14:02:40 +0000 (16:02 +0200)] 
MINOR: buffer: remove bi_getblk() and bi_getblk_nc()

These ones were relying on bi_ptr() and are not used. They may be
reimplemented later in the channel if needed.

6 years agoMINOR: buffer: replace calls to buffer_space_wraps() with b_space_wraps()
Willy Tarreau [Thu, 12 Jul 2018 08:33:12 +0000 (10:33 +0200)] 
MINOR: buffer: replace calls to buffer_space_wraps() with b_space_wraps()

And remove the unused function.

6 years agoMINOR: channel/buffer: replace b_{adv,rew} with c_{adv,rew}
Willy Tarreau [Wed, 6 Jun 2018 05:13:22 +0000 (07:13 +0200)] 
MINOR: channel/buffer: replace b_{adv,rew} with c_{adv,rew}

These ones manipulate the output data count which will be specific to
the channel soon, so prepare the call points to use the channel only.
The b_* functions are now unused and were removed.

6 years agoMINOR: buffer: remove buffer_slow_realign() and the swap_buffer allocation code
Willy Tarreau [Thu, 12 Jul 2018 09:02:51 +0000 (11:02 +0200)] 
MINOR: buffer: remove buffer_slow_realign() and the swap_buffer allocation code

Since all call places can use the trash now, this is not needed anymore.

6 years agoMINOR: h2: use b_slow_realign() with the trash as a swap buffer
Willy Tarreau [Thu, 12 Jul 2018 09:00:01 +0000 (11:00 +0200)] 
MINOR: h2: use b_slow_realign() with the trash as a swap buffer

H2 doesn't use the trash so it can make use of it as a swap area when
calling b_slow_realign(). This way we don't need buffer_slow_realign()
anymore.

6 years agoMEDIUM: channel: make channel_slow_realign() take a swap buffer
Willy Tarreau [Thu, 12 Jul 2018 08:57:15 +0000 (10:57 +0200)] 
MEDIUM: channel: make channel_slow_realign() take a swap buffer

The few call places where it's used can use the trash as a swap buffer,
which is made for this exact purpose. This way we can rely on the
generic b_slow_realign() call.

6 years agoMINOR: channel/buffer: replace buffer_slow_realign() with channel_slow_realign()...
Willy Tarreau [Wed, 6 Jun 2018 04:53:15 +0000 (06:53 +0200)] 
MINOR: channel/buffer: replace buffer_slow_realign() with channel_slow_realign() and b_slow_realign()

Where relevant, the channel version is used instead. The buffer version
was ported to be more generic and now takes a swap buffer and the output
byte count to know where to set the alignment point. The H2 mux still
uses buffer_slow_realign() with buf->o but it will change later.

6 years agoMINOR: channel/buffer: use c_realign_if_empty() instead of buffer_realign()
Willy Tarreau [Wed, 6 Jun 2018 04:42:46 +0000 (06:42 +0200)] 
MINOR: channel/buffer: use c_realign_if_empty() instead of buffer_realign()

This patch removes buffer_realign() and replaces it with c_realign_if_empty()
instead.

6 years agoMINOR: channel: add a few basic functions for the new buffer API
Willy Tarreau [Wed, 6 Jun 2018 13:09:28 +0000 (15:09 +0200)] 
MINOR: channel: add a few basic functions for the new buffer API

This adds :
  - c_orig()  : channel buffer's origin
  - c_size()  : channel buffer's size
  - c_wrap()  : channel buffer's wrapping location
  - c_data()  : channel buffer's total data count
  - c_room()  : room left in channel buffer's
  - c_empty() : true if channel buffer is empty
  - c_full()  : true if channel buffer is full

  - c_ptr()   : pointer to an offset relative to input data in the buffer
  - c_adv()   : advances the channel's buffer (bytes become part of output)
  - c_rew()   : rewinds the channel's buffer (output bytes not output anymore)
  - c_realign_if_empty() : realigns the buffer if it's empty

  - co_data() : # of output data
  - co_head() : beginning of output data
  - co_tail() : end of output data
  - ci_data() : # of input data
  - ci_head() : beginning of input data
  - ci_tail() : end of input data
  - ci_stop() : location after ci_tail()
  - ci_next() : pointer to next input byte

And for the ci_* / co_* functions above, the "__*" variants which disable
wrapping checks, and the "_ofs" variants which return an offset relative to
the buffer's origin instead.

6 years agoMINOR: compression: pass the channel to http_compression_buffer_end()
Willy Tarreau [Wed, 6 Jun 2018 05:15:47 +0000 (07:15 +0200)] 
MINOR: compression: pass the channel to http_compression_buffer_end()

This will be needed to access the output data count from the channel
after the buffer/channel changes.

6 years agoMINOR: buffer: introduce b_realign_if_empty()
Willy Tarreau [Fri, 15 Jun 2018 15:50:15 +0000 (17:50 +0200)] 
MINOR: buffer: introduce b_realign_if_empty()

Many places deal with buffer realignment after data removal. The method
is always the same : if the buffer is empty, set its pointer to the origin.
Let's have a function for this so that we have less code to change with the
new API.

6 years agoMINOR: buffer: Add b_set_data().
Olivier Houchard [Thu, 28 Jun 2018 17:10:25 +0000 (19:10 +0200)] 
MINOR: buffer: Add b_set_data().

Add a new function that lets you set the amount of input in a buffer.
For now it extends/truncates b->i except if the total length is
below b->o in which case it clears i and adjusts o.

6 years agoMINOR: buffer: Introduce b_sub(), b_add(), and bo_add()
Olivier Houchard [Thu, 28 Jun 2018 17:17:38 +0000 (19:17 +0200)] 
MINOR: buffer: Introduce b_sub(), b_add(), and bo_add()

Instead of doing b->i -= directly, introduce b_sub(), that does the job, to
make it easier to switch to the future API.

Also add b_add(), that increases b->i, instead of using it directly, and
bo_add(), that does increase b->o.

6 years agoMINOR: buffer: add a few basic functions for the new API
Willy Tarreau [Wed, 6 Jun 2018 12:30:50 +0000 (14:30 +0200)] 
MINOR: buffer: add a few basic functions for the new API

Here's the list of newly introduced functions :

- b_data(), returning the total amount of data in the buffer (currently i+o)

- b_orig(), returning the origin of the storage area, that is, the place of
  position 0.

- b_wrap(), pointer to wrapping point (currently data+size)

- b_size(), returning the size of the buffer

- b_room(), returning the amount of bytes left available

- b_full(), returning true if the buffer is full, otherwise false

- b_stop(), pointer to end of data mark (currently p+i), used to compute
  distances or a stop pointer for a loop.

- b_peek(), this one will help make the transition to the new buffer model.
  It returns a pointer to a position in the buffer known from an offest
  relative to the beginning of the data in the buffer. Thus, we can replace
  the following occurrences :

     bo_ptr(b)     => b_peek(b, 0);
     bo_end(b)     => b_peek(b, b->o);
     bi_ptr(b)     => b_peek(b, b->o);
     bi_end(b)     => b_peek(b, b->i + b->o);
     b_ptr(b, ofs) => b_peek(b, b->o + ofs);

- b_head(), pointer to the beginning of data (currently bo_ptr())

- b_tail(), pointer to first free place (currently bi_ptr())

- b_next() / b_next_ofs(), pointer to the next byte, taking wrapping
  into account.

- b_dist(), returning the distance between two pointers belonging to a buffer

- b_reset(), which resets the buffer

- b_space_wraps(), indicating if the free space wraps around the buffer

- b_almost_full(), indicating if 3/4 or more of the buffer are used

Some of these are provided with the unchecked variants using the "__"
prefix, or with the "_ofs" suffix indicating they return a relative
position to the buffer's origin instead of a pointer.

Cc: Olivier Houchard <ohouchard@haproxy.com>
6 years agoMINOR: buffer: switch buffer sizes and offsets to size_t
Willy Tarreau [Wed, 18 Jul 2018 08:07:58 +0000 (10:07 +0200)] 
MINOR: buffer: switch buffer sizes and offsets to size_t

Passing unsigned ints everywhere is painful, and will cause some headache
later when we'll want to integrate better with struct ist which already
uses size_t. Let's switch buffers to use size_t instead.

6 years agoMINOR: buffer: implement a new file for low-level buffer manipulation functions
Willy Tarreau [Wed, 11 Jul 2018 07:39:05 +0000 (09:39 +0200)] 
MINOR: buffer: implement a new file for low-level buffer manipulation functions

The buffer code currently depends on pools and other stuff and is not
really autonomous anymore. The rewrite of the new API is an opportunity
to clean this up. This patch creates a new file (buf.h) which does not
depend on other elements and which will only contain what is needed to
perform the most basic buffer operations. The new API will be introduced
in this file and the conversion will be finished once buffer.h is empty.

The definition of struct buffer was moved to this new file, using more
explicity stdint types for the sizes and offsets.

Most new functions will be implemented in two variants :

  __b_something() : unchecked variant, no wrapping is expected
  b_something() : wrapping-checked variant

This way callers will be able to select which one to use depending on
the use cases.

6 years agoMINOR: tasklet: Set process to NULL.
Olivier Houchard [Thu, 19 Jul 2018 14:02:16 +0000 (16:02 +0200)] 
MINOR: tasklet: Set process to NULL.

Some consumers expect the process to be NULL when a tasklet it created, so
do so.

6 years agoBUG/MEDIUM: h2: make sure the last stream closes the connection after a timeout
Willy Tarreau [Wed, 13 Jun 2018 12:24:56 +0000 (14:24 +0200)] 
BUG/MEDIUM: h2: make sure the last stream closes the connection after a timeout

If a timeout strikes on the connection side with some active streams,
there is a corner case which can sometimes cause the following sequence
to happen :

  - There are active streams but there are data in the mux buffer
    (eg: a client suddenly disconnected during a download with pending
    requests). The timeout is active.

  - The timeout strikes, h2_timeout_task() is called, kills the task and
    doesn't close the connection since there are streams left ; The
    connection is marked in H2_CS_ERROR ;

  - the streams are woken up and closed ;

  - when the last stream closes, calling h2_detach(), it sees the
    tree list is empty, but there is no condition allowing the
    connection to be closed (mbuf->o > 0), thus it does nothing ;

  - since the task is dead, there's no more hope to clear this
    situation later

For now we can take care of this by adding a test for the presence of
H2_CS_ERROR and !task, implying the timeout task triggered already
and will not be able to handle this again.

Over the long term it seems like a more reliable test on should be
made, so that it is possible to know whether or not someone is still
able to close this connection.

A big thanks to Janusz Dziemidowicz and Milan Petruzelka for providing
many details helping in figuring this bug.

6 years agoBUG/MEDIUM: h2: never leave pending data in the output buffer on close
Willy Tarreau [Thu, 19 Jul 2018 08:58:28 +0000 (10:58 +0200)] 
BUG/MEDIUM: h2: never leave pending data in the output buffer on close

We currently don't process trailers on H2, but this has an impact : on
chunked HTTP/1 responses, we decide to emit the ES bit once we see the
0CRLF. From this point the stream switches to the CLOSED state, which
aborts processing of the remaining bytes. Thus the extra CRLF which ends
trailers is not processed and remains in the buffer. This prevents the
stream from being notified about end of transmission, which in turn keeps
the mux busy and prevents the connection from quitting.

The case of the trailers is not the root cause of this issue, though it
is what triggers it. The root cause is that upon error and/or close, once
we know we're not going to process any more data, we must absolutely flush
any remaining bytes from the output buffer, otherwise there is no way the
stream can quit. This is what this patch does.

It looks very likely related to the issues reported and debugged by
Janusz Dziemidowicz and Milan Petruzelka.

One way to reproduce it is to chain two proxies with the last one emitting
chunked data (typically using the stats page) :

    global
        stats socket /tmp/sock1 mode 666 level admin
        stats timeout 1h
        tune.ssl.default-dh-param 1024
        tune.bufsize 16384

    defaults
        mode http
        timeout connect 4s
        timeout client 10s
        timeout server 20s

    listen px1
        bind :4443 ssl crt rsa+dh2048.pem npn h2 alpn h2
        server s1 127.0.0.1:4445

    listen px2
        bind :4444 ssl crt rsa+dh2048.pem npn h2 alpn h2
        bind :4445
        stats uri /

Then use curl to fetch the stats through px1 :

    curl --http2 -k "https://127.0.0.1:4443/"

When curl is sent to the first one, "show sess" issued to the CLI will
show a remaining session during the client timeout. When curl is aimed at
port 4444 (px2), there is no such remaining session.

This fix needs to be backported to 1.8.

6 years agoMINOR: h2: add the mux and demux buffer lengths on "show fd"
Willy Tarreau [Thu, 19 Jul 2018 08:54:43 +0000 (10:54 +0200)] 
MINOR: h2: add the mux and demux buffer lengths on "show fd"

It is convenient during debugging sessions to know if the mux and demux
buffers are empty/full/other. Let's report this on "show fd" output.

6 years agoBUG/MEDIUM: h2: don't accept new streams if conn_streams are still in excess
Willy Tarreau [Thu, 19 Jul 2018 08:11:38 +0000 (10:11 +0200)] 
BUG/MEDIUM: h2: don't accept new streams if conn_streams are still in excess

The streams bookkeeping made in H2 is used for protocol compliance only
but it doesn't consider the number of conn_streams still attached to the
mux. It causes an issue when http-request set-nice rules are applied on
H2 requests processed on a saturated machine. Indeed, in this case, the
requests are accepted and assigned a default nice value of zero. When
they are processed, their nice value changes to a higher one (say 1024).
The response is sent through the H2 mux, which detects the end of stream
and decrements the protocol-level stream count (h2c->nb_streams). The
client may then send a new request. But the conn_stream is still attached
and will require a new call to process_stream() to finish, which is made
through the scheduler. Given that the machine is saturated, it is assumed
that many tasks are present in the scheduler. Thus the closing tasks holding
a higher nice value will pass after the new stream creations. If the client
is fast enough with a low latency link, it may add a lot of new stream
creations before the stream terminations have a chance to disappear due
to their high nice value, resulting in a huge amount of memory being used.

The solution consists in letting a mux always monitor its conn_streams and
refrain from creating new ones when it is full. Here the H2 mux checks the
nb_cs counter and sets a new blocked flag (H2_CF_DEM_TOOMANY) if the limit
was reached, so that the frame parser requests a pause in the new stream
creation, leaving some time for the pending conn_streams to vanish.

Several experiments were made using varying thresholds to see if
overbooking would provide any benefit here but it turned out not to be
the case, so the conn_stream limit remains set to the exact streams
limit. Interestingly various performance measurements showed that the
code tends to be slightly faster now than without the limit, probably
due to the smoother memory usage.

This commit requires previous patch ("MINOR: h2: keep a count of the number
of conn_streams attached to the mux"). It needs to be backported to 1.8.

6 years agoMINOR: h2: keep a count of the number of conn_streams attached to the mux
Willy Tarreau [Thu, 19 Jul 2018 07:04:05 +0000 (09:04 +0200)] 
MINOR: h2: keep a count of the number of conn_streams attached to the mux

The h2 mux only knows about the number of H2 streams which are not in a
CLOSED state. This is used for protocol compliance. But it doesn't hold
the number of really attached streams. It is a problem because depending
on scheduling, it is possible that more streams are attached to the mux
than the ones seen at the protocol level, due to some streams taking some
time to be detached. Let's add this count based on the conn_streams.

Note: this patch is part of a series of fixes which will have to be
backported to 1.8.