Commit Graph

135 Commits

Author SHA1 Message Date
Willy Tarreau
d14844a734 MINOR: stream-int: replace si_update() with si_update_both()
The function used to be called in turn for each side of the stream, but
since it's called exclusively from process_stream(), it prevents us from
making use of the knowledge we have of the operations in progress for
each side, resulting in having to go all the way through functions like
stream_int_notify() which are not appropriate there.

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

The next step will consist in trying to reinsert the rest of the socket
layer-specific update code to ultimately update the flags correctly at
the end of the operation.
2018-11-11 10:18:37 +01:00
Willy Tarreau
8fe516f08a MEDIUM: stream-int: make si_chk_rcv() check that SI_FL_WAIT_ROOM is cleared
After careful inspection, it now seems OK to call si_chk_rcv() only when
SI_FL_WAIT_ROOM is cleared and SI_FL_WANT_PUT is set, since all identified
call places have already taken care of this.
2018-11-11 10:18:37 +01:00
Willy Tarreau
abf531caa0 MEDIUM: stream-int: always call si_chk_rcv() when we make room in the buffer
Instead of clearing the SI_FL_WAIT_ROOM flag and losing the information
about the need from the producer to be woken up, we now call si_chk_rcv()
immediately. This is cheap to do and it could possibly be further improved
by only doing it when SI_FL_WAIT_ROOM was still set, though this will
require some extra auditing of the code paths.

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

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

Now we don't have any situation where we randomly clear SI_FL_WAIT_ROOM
without trying to wake the other side up, nor where we call si_chk_rcv()
with the flag set, so this flag should accurately represent a failed
attempt at putting data into the buffer.
2018-11-11 10:18:37 +01:00
Willy Tarreau
1f9de21c38 MEDIUM: stream-int: make SI_FL_WANT_PUT reflect CF_DONT_READ
When CF_DONT_READ is set, till now we used to set SI_FL_WAIT_ROOM, which
is not appropriate since it would lose the subscribe status. Instead let's
clear SI_FL_WANT_PUT (just like applets do), and set the flag only when
CF_DONT_READ is cleared.

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

Last, when si_chk_rcv() is called, it immediately clears the flag before
calling ->chk_rcv() so that we are not tempted to uselessly loop on the
same call until the receive function is called. This is the same principle
as what is done with the applet scheduler.
2018-11-11 10:18:37 +01:00
Willy Tarreau
1bdb598a55 MINOR: stream-int: factor the SI_ST_EST state test into si_chk_rcv()
This test is made in each implementation of the function, better to
merge it.
2018-11-11 10:18:37 +01:00
Willy Tarreau
96aadd5c55 MEDIUM: stream-int: temporarily make si_chk_rcv() take care of SI_FL_WAIT_ROOM
This flag should already be cleared before calling the *chk_rcv() functions.
Before adapting all call places, let's first make sure si_chk_rcv() clears
it before calling them so that these functions do not have to check it again
and so that they do not adjust it. This function will only call the lower
layers if the SI_FL_WANT_PUT flag is present so that the endpoint can decide
not to be called (as done with applets).
2018-11-11 10:18:37 +01:00
Willy Tarreau
57f08bb63b MINOR: stream-int: make it clear that si_ops cannot be null
There was an ambiguity in which functions of the si_ops struct could be
null or not. only ->update doesn't exist in one of the si_ops (the
embedded one), all others are always defined. ->shutr and ->shutw were
never tested. However ->chk_rcv() and ->chk_snd() were tested, causing
confusion about the proper way to wake the other side up if undefined
(which never happens).

Let's update the comments to state these functions are mandatory and
remove the offending checks.
2018-11-11 10:18:37 +01:00
Willy Tarreau
af4f6f6d2f MINOR: stream-int: use si_cant_put() instead of setting SI_FL_WAIT_ROOM
We now do this on the si_cs_recv() path so that we always have
SI_FL_WANT_PUT properly set when there's a need to receive and
SI_FL_WAIT_ROOM upon failure.
2018-11-11 10:18:37 +01:00
Willy Tarreau
394970c297 MINOR: stream-int: add si_done_{get,put} to indicate that we won't do it anymore
This is useful on close or stream aborts as it saves us from having
to manipulate the (sometimes confusing) flags.
2018-11-11 10:18:37 +01:00
Willy Tarreau
0cd3bd628a MINOR: stream-int: rename si_applet_{want|stop|cant}_{get|put}
It doesn't make sense to limit this code to applets, as any stream
interface can use it. Let's rename it by simply dropping the "applet_"
part of the name. No other change was made except updating the comments.
2018-11-11 10:18:37 +01:00
Willy Tarreau
85f890174a MEDIUM: stream-int: make si_update() synchronize flag changes before the I/O
With the new synchronous si_cs_send() at the end of process_stream(),
we're seeing re-appear the I/O layer specific part of the stream interface
which is supposed to deal with I/O event subscription. The only difference
is that now we subscribe to I/Os only after having attempted (and failed)
them.

This patch brings a cleanup in this by reintroducing stream_int_update_conn()
with the send code from process_stream(). However this alone would not be
enough because the flags which are cleared afterwards would result in the
loss of the possible events (write events only at the moment). So the flags
clearing and stream-int state updates are also performed inside si_update()
between the generic code and the I/O specific code. This definitely makes
sense as after this call we can simply check again for channel and SI flag
changes and decide to loop once again or not.
2018-10-28 13:47:00 +01:00
Willy Tarreau
0979916d3b MINOR: stream-int: add si_alloc_ibuf() to ease input buffer allocation
This will supersed channel_alloc_buffer() while relying on it. It will
automatically adjust SI_FL_WAIT_ROOM on the stream-int depending on
success or failure to allocate this buffer.

It's worth noting that it could make sense to also set SI_FL_WANT_PUT
each time we do this to further simplify the code at user places such
as applets, but it would possibly not be easy to clean this flag
everywhere an rx operation stops.
2018-10-28 13:47:00 +01:00
Olivier Houchard
53216e7db9 MEDIUM: connections: Don't directly mess with the polling from the upper layers.
Avoid using conn_xprt_want_send/recv, and totally nuke cs_want_send/recv,
from the upper layers. The polling is now directly handled by the connection
layer, it is activated on subscribe(), and unactivated once we got the event
and we woke the related task.
2018-10-21 05:58:40 +02:00
Olivier Houchard
1fddc9b7bb BUG/MEDIUM: connections: Remove subscription if going in idle mode.
Make sure we don't have any subscription when the connection is going in
idle mode, otherwise there's a race condition when the connection is
reused, if there are still old subscriptions, new ones won't be done.

No backport is needed.
2018-10-21 05:55:20 +02:00
Olivier Houchard
4fdec7aafa BUG/MEDIUM: stream: Make sure to unsubscribe before si_release_endpoint.
Make sure we unsubscribe from events before si_release_endpoint destroys
the conn_stream, or it will be never called. To do so, move the call to
unsubscribe to si_release_endpoint() directly.

This is 1.9-specific and shouldn't be backported.
2018-10-11 17:16:43 +02:00
Olivier Houchard
fa8aa867b9 MEDIUM: connections: Change struct wait_list to wait_event.
When subscribing, we don't need to provide a list element, only the h2 mux
needs it. So instead, Add a list element to struct h2s, and use it when a
list is needed.
This forces us to use the unsubscribe method, since we can't just unsubscribe
by using LIST_DEL anymore.
This patch is larger than it should be because it includes some renaming.
2018-10-11 15:34:39 +02:00
Olivier Houchard
0e367bbb01 BUG/MEDIUM: process_stream: Don't use si_cs_io_cb() in process_stream().
Instead of using si_cs_io_cb() in process_stream()  use si_cs_send/si_cs_recv
instead, as si_cs_io_cb() may lead to process_stream being woken up when it
shouldn't be, and thus timeout would never get triggered.
2018-09-26 14:21:54 +02:00
Willy Tarreau
4ae4923c3e MINOR: stream-int: make si_appctx() never fail
Callers of si_appctx() always use the result without checking it because
they know by construction that it's valid. This results in unchecked null
pointer warnings at -Wextra, so let's remove this test and make it clear
that it's up to the caller to check validity first.
2018-09-20 11:42:15 +02:00
Olivier Houchard
8f0b4c66f5 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.
2018-08-16 17:29:54 +02:00
Olivier Houchard
91894cbf4c 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.
2018-08-16 17:29:54 +02:00
Willy Tarreau
83061a820e MAJOR: chunks: replace struct chunk with struct buffer
Now all the code used to manipulate chunks uses a struct buffer instead.
The functions are still called "chunk*", and some of them will progressively
move to the generic buffer handling code as they are cleaned up.
2018-07-19 16:23:43 +02:00
Willy Tarreau
c9fa0480af 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.
2018-07-19 16:23:43 +02:00
Willy Tarreau
10d81b8757 MINOR: applet: assign the same nice value to a new appctx as its owner task
When an applet is created, let's assign it the same nice value as the task
of the stream which owns it. It ensures that fairness is properly propagated
to applets, and that the CLI can regain a low latency behaviour again. Huge
differences have been seen under extreme loads, with the CLI being called
every 200 microseconds instead of 11 milliseconds.
2018-06-05 11:18:21 +02:00
Olivier Houchard
9aaf778129 MAJOR: connection : Split struct connection into struct connection and struct conn_stream.
All the references to connections in the data path from streams and
stream_interfaces were changed to use conn_streams. Most functions named
"something_conn" were renamed to "something_cs" for this. Sometimes the
connection still is what matters (eg during a connection establishment)
and were not always renamed. The change is significant and minimal at the
same time, and was quite thoroughly tested now. As of this patch, all
accesses to the connection from upper layers go through the pass-through
mux.
2017-10-31 18:03:23 +01:00
Emeric Brun
1138fd0c57 MAJOR: threads/applet: Handle multithreading for applets
A global lock has been added to protect accesses to the list of active
applets. A process mask has also been added on each applet. Like for FDs and
tasks, it is used to know which threads are allowed to process an
applet. Because applets are, most of time, linked to a session, it should be
sticky on the same thread. But in all cases, it is the responsibility of the
applet handler to lock what have to be protected in the applet context.
2017-10-31 13:58:31 +01:00
Willy Tarreau
3b737c9894 MINOR: stream-int: use conn_full_close() instead of conn_force_close()
We simply disable tracking before calling it.
2017-10-22 09:54:18 +02:00
Olivier Houchard
1a0545f3d7 REORG: connection: rename CO_FL_DATA_* -> CO_FL_XPRT_*
These flags are not exactly for the data layer, they instead indicate
what is expected from the transport layer. Since we're going to split
the connection between the transport and the data layers to insert a
mux layer, it's important to have a clear idea of what each layer does.

All function conn_data_* used to manipulate these flags were renamed to
conn_xprt_*.
2017-10-22 09:54:15 +02:00
Willy Tarreau
de40d798de CLEANUP: connection: completely remove CO_FL_WAKE_DATA
Since it's only set and never tested anymore, let's remove it.
2017-03-19 12:18:27 +01:00
Christopher Faulet
c34d19fc3c BUG: stream_interface: Reuse connection even if the output channel is empty
in function 'si_connect', an existing connection is reused (and considered as
established) only when there are some pending data in the output channel.

This can be problem when filters are used, because a filter can choose to not
forward data immediatly. So when we try to initiate a connection to a server,
the output channel can be empty. In this situation, if the connection already
exists, it is not considered as established and nothing happens. If the stream
interface is in the state SI_ST_ASS, this leads to an infinite loop in
process_stream because it remains in this state.

This patch fixes this problem. Now, in 'si_connect', we always reuse an existing
connection, whether or not there are pending data in the output channel.
2016-02-03 14:22:55 +01:00
Willy Tarreau
958f0742a2 BUG/MEDIUM: stream-int: avoid double-call to applet->release
While the SI_ST_DIS state is set *after* doing the close on a connection,
it was set *before* calling release on an applet. Applets have no internal
flags contrary to connections, so they have no way to detect they were
already released. Because of this it happened that applets were closed
twice, once via si_applet_release() and once via si_release_endpoint() at
the end of a transaction. The CLI applet could perform a double free in
this case, though the situation to cause it is quite hard because it
requires that the applet is stuck on output in states that produce very
few data.

In order to solve this, we now assign the SI_ST_DIS state *after* calling
->release, and we refrain from doing so if the state is already assigned.
This makes applets work much more like connections and definitely avoids
this double release.

In the future it might be worth making applets have their own flags like
connections to carry their own state regardless of the stream interface's
state, especially when dealing with connection reuse.

No backport is needed since this issue was caused by the rearchitecture
in 1.6.
2015-09-25 21:16:03 +02:00
Willy Tarreau
aa977ba205 MINOR: stream-int: rename si_applet_done() to si_applet_wake_cb()
This function is a callback made only for calls from the applet handler.
Rename it to remove confusion. It's currently called from the Lua code
but that's not correct, we should call the notify and update functions
instead otherwise it will not enable the applet again.
2015-09-25 21:16:02 +02:00
Willy Tarreau
335520305c MEDIUM: stream-int: completely remove stream_int_update_embedded()
This one is not needed anymore as what it used to do is either
completely covered by the new stream_int_notify() function, or undesired
and inherited from the past as a side effect of introducing the
connections.

This update is theorically never called since it's assigned only when
nothing is connected to the stream interface. However a test has been
added to si_update() to stay safe if some foreign code decides to call
si_update() in unsafe situations.
2015-09-25 21:16:02 +02:00
Willy Tarreau
615f28bec1 MINOR: stream-int: implement the stream_int_notify() function
stream_int_notify() was taken from the common part between si_conn_wake_cb()
and si_applet_done(). It is designed to report activity to a stream from
outside its handler. It'll generally be used by lower layers to report I/O
completion but may also be used by remote streams if the buffer processing
is shared.
2015-09-25 21:16:02 +02:00
Willy Tarreau
cbc32601a6 MINOR: stream-int: export stream_int_update_*
Not only these functions were not static, but we'll also want to export
them.
2015-09-25 18:07:16 +02:00
Willy Tarreau
5d5b2fecac MEDIUM: stream-int: call stream_int_update() from si_update()
Now the call to stream_int_update() is moved to si_update(), which
is exclusively called from the stream, so that the socket layer may
be updated without updating the stream layer. This will later permit
to call it individually from other places (other tasks or applets for
example).
2015-09-25 18:07:16 +02:00
Willy Tarreau
25f1310f33 MINOR: stream-int: implement a new stream_int_update() function
This function is designed to be called from within the stream handler to
update the channels' expiration timers and the stream interface's flags
based on the channels' flags. It needs to be called only once after the
channels' flags have settled down, and before they are cleared, though it
doesn't harm to call it as often as desired (it just slightly hurts
performance). It must not be called from outside of the stream handler,
as what it does will be used to compute the stream task's expiration.

The code was taken directly from stream_int_update_applet() and
stream_int_update_conn() which had exactly the same one except for
applet-specific or connection-specific status update.
2015-09-25 18:07:16 +02:00
Willy Tarreau
0fd99094c3 CLEANUP: stream-int: remove obsolete function si_applet_call()
This one is not used anymore and is bogus due to the way applets
now work. Remove it instead of fixing it before someone finds it
fun to use it.
2015-09-23 13:42:08 +02:00
Willy Tarreau
4320eaac62 MINOR: stream-int: make si_idle_conn() only accept valid connections
This function is now dedicated to idle connections only, which means
that it must not be used without any endpoint nor anything not a
connection. The connection remains attached to the stream interface.
2015-08-06 11:11:10 +02:00
Willy Tarreau
323a2d925c MEDIUM: stream-int: queue idle connections at the server
Now we get a per-server list of all idle connections. That way we'll
be able to reclaim them upon shortage later.
2015-08-06 11:06:25 +02:00
Willy Tarreau
41b09f4bbc MINOR: stream-int: add new function si_detach_endpoint()
This function only detaches the endpoint from the stream-int and
optionally returns the original pointer. This will be needed to
steal idle connections from other connections.
2015-08-05 21:54:25 +02:00
Willy Tarreau
973a54235f MEDIUM: stream-int: simplify si_alloc_conn()
Since we now always call this function with the reuse parameter cleared,
let's simplify the function's logic as it cannot return the existing
connection anymore. The savings on this inline function are appreciable
(240 bytes) :

$ size haproxy.old haproxy.new
   text    data     bss     dec     hex filename
1020383   40816   36928 1098127  10c18f haproxy.old
1020143   40816   36928 1097887  10c09f haproxy.new
2015-08-05 21:51:09 +02:00
Willy Tarreau
d7c7475ad6 MINOR: stream-int: only use si_release_endpoint() to release a connection
Change si_alloc_conn() to call si_release_endpoint() instead of
open-coding the connection releasing code when reuse is disabled.
This fuses the code with the one already dealing with applets, makes
it shorter and helps centralizing the connection freeing logic at a
single place.
2015-07-19 19:14:10 +02:00
Willy Tarreau
7a08d3b2d7 CLEANUP: stream-int: remove stream_int_unregister_handler() and si_detach()
The former was not used anymore and the latter was only used by the former.
They were only aliases to other existing functions anyway.
2015-07-19 18:48:20 +02:00
Willy Tarreau
eb406dc73c MINOR: stream-int: add two flags to indicate an applet's wishes regarding I/O
Currently we have a problem. There are some cases where a sleeping applet
is not woken up (eg: show sess during an injection). The reason is that
the applet is marked WAIT_DATA and is not woken up when WAIT_ROOM leaves,
because we wait for both flags to be cleared in order to call it.

And if we wait for either flag, then we have the opposite situation, which
is that we're not waiting for room in the output buffer so we're spinning
calling the applet to do nothing.

What is missing is an indication of what the applet needs. Since it only
manipulates the WAIT_ROOM/WAIT_DATA which are overwritten later, that cannot
work. In the case of connections, the problem doesn't happen because the
connection maintains these extra states. Ideally we'd need to have similar
states for each appctx and to store those information there. But it would
be overcomplicated given that an applet doesn't exist alone without a
stream-int, so we can safely put these information into the stream int and
make the code simpler.

With this patch we introduce two new flags in the stream interface :
  - SI_FL_WANT_PUT : the applet wants to put something into the buffer
  - SI_FL_WANT_GET : the applet wants to get something from the buffer

We also have the new functions si_applet_{stop|want|cant}_{get|put}
to make the code look similar to the connection code.

For now these flags are not used yet.
2015-04-23 17:56:17 +02:00
Willy Tarreau
e5f8649102 MEDIUM: stream-int: add a new function si_applet_done()
This is the equivalent of si_conn_wake() but for applets. It will be
called after changes to the stream interface are brought by the applet
I/O handler. Ultimately it will release buffers and may be even wake
the stream's task up if some important changes are detected.

It would be nice to be able to merge it with the connection's wake
function since it mostly manipulates the stream interface, but there
are minor differences (such as how to enable/disable polling on a fd
vs applet) and some specificities to applets (eg: don't wake the
applet up until the output is empty) which would require abstract
functions which would slow down everything.
2015-04-23 17:56:16 +02:00
Willy Tarreau
d45b9f8991 REORG: stream-int: create si_applet_ops dedicated to applets
These functions are dedicated to applets so that we don't use the default
ones anymore in this case.
2015-04-23 17:56:16 +02:00
Willy Tarreau
3057645b37 CLEANUP: applet: rename struct si_applet to applet
Since this one does not depend on stream_interface anymore, remove the
"si_" prefix.
2015-04-23 17:56:16 +02:00
Willy Tarreau
8a8d83b85c REORG: applet: move the applet definitions out of stream_interface
We're tidying the definitions so that appctx lives on its own. A new
set of applet.h files has been added for this purpose.
2015-04-23 17:56:16 +02:00
Willy Tarreau
00a37f0029 MEDIUM: applet: make the applet not depend on a stream interface anymore
Now that applet's functions only take an appctx in argument, not a
stream interface. This slightly simplifies the code and will be needed
to take the appctx out of the stream interface.
2015-04-23 17:56:16 +02:00
Willy Tarreau
19c8161b3d MINOR: applet: add a new "owner" pointer in the appctx
This pointer indicates what stream-interface the appctx belongs to, just
like we have for the connections.
2015-04-23 17:56:16 +02:00