Commit Graph

21569 Commits

Author SHA1 Message Date
Christopher Faulet
9704797fa2 BUG/MEDIUM: http-ana: Properly switch the request in tunnel mode on upgrade
Since the commit f2b02cfd9 ("MAJOR: http-ana: Review error handling during
HTTP payload forwarding"), during the payload forwarding, we are analyzing a
side, we stop to test the opposite side. It means when the HTTP request
forwarding analyzer is called, we no longer check the response side and vice
versa.

Unfortunately, since then, the HTTP tunneling is broken after a protocol
upgrade. On the response is switch in TUNNEL mode. The request remains in
DONE state. As a consequence, data received from the server are forwarded to
the client but not data received from the client.

To fix the bug, when both sides are in DONE state, both are switched in same
time in TUNNEL mode if it was requested. It is performed in the same way in
http_end_request() and http_end_response().

This patch should fix the issue #2125. It is 2.8-specific. No backport
needed.
2023-04-17 16:17:35 +02:00
William Lallemand
a21ca74e83 MINOR: ssl: remove OpenSSL 1.0.2 mention into certificate loading error
Remove the mention to OpenSSL 1.0.2 in the certificate chain loading
error, which is not relevant.

Could be backported in 2.7.
2023-04-17 14:45:40 +02:00
Ilya Shipitsin
2ca01589a0 CLEANUP: use "offsetof" where appropriate
let's use the C library macro "offsetof"
2023-04-16 09:58:49 +02:00
Frédéric Lécaille
b5efe7901d BUG/MINOR: quic: Do not use ack delay during the handshakes
As revealed by GH #2120 opened by @Tristan971, there are cases where ACKs
have to be sent without packet to acknowledge because the ACK timer has
been triggered and the connection needs to probe the peer at the same time.
Indeed

Thank you to @Tristan971 for having reported this issue.

Must be backported to 2.6 and 2.7.
2023-04-14 21:09:13 +02:00
Christopher Faulet
75b954fea4 BUG/MINOR: stconn: Don't set SE_FL_ERROR at the end of sc_conn_send()
When I reworked my series, this code was first removed and reinserted by
error. So let's remove it again.
2023-04-14 17:32:44 +02:00
Christopher Faulet
25d9fe50f5 MEDIUM: stconn: Rely on SC flags to handle errors instead of SE flags
It is the last commit on this subject. we stop to use SE_FL_ERROR flag from
the SC, except at the I/O level. Otherwise, we rely on SC_FL_ERROR
flag. Now, there should be a real separation between SE flags and SC flags.
2023-04-14 17:05:54 +02:00
Christopher Faulet
e182a8e651 MEDIUM: stream: Stop to use SE flags to detect endpoint errors
Here again, we stop to use SE_FL_ERROR flag from process_stream() and
sub-functions and we fully rely on SC_FL_ERROR to do so.
2023-04-14 17:05:54 +02:00
Christopher Faulet
d7bac88427 MEDIUM: stream: Stop to use SE flags to detect read errors from analyzers
In the same way the previous commit, we stop to use SE_FL_ERROR flag from
analyzers and their sub-functions. We now fully rely on SC_FL_ERROR to do so.
2023-04-14 17:05:54 +02:00
Christopher Faulet
725170eee6 MEDIUM: backend: Stop to use SE flags to detect connection errors
SE_FL_ERROR flag is no longer set when an error is detected durign the
connection establishment. SC_FL_ERROR flag is set instead. So it is safe to
remove test on SE_FL_ERROR to detect connection establishment error.
2023-04-14 17:05:54 +02:00
Christopher Faulet
88d05a0f3b MEDIUM: tree-wide: Stop to set SE_FL_ERROR from upper layer
We can now fully rely on SC_FL_ERROR flag from the stream. The first step is
to stop to set the SE_FL_ERROR flag. Only endpoints are responsible to set
this flag. It was a design limitation. It is now fixed.
2023-04-14 17:05:54 +02:00
Christopher Faulet
ad46e52814 MINOR: tree-wide: Test SC_FL_ERROR with SE_FL_ERROR from upper layer
From the stream, when SE_FL_ERROR flag is tested, we now also test the
SC_FL_ERROR flag. Idea is to stop to rely on the SE descriptor to detect
errors.
2023-04-14 17:05:54 +02:00
Christopher Faulet
340021b89f MINOR: stream: Set SC_FL_ERROR on channels' buffer allocation error
Set SC_FL_ERROR flag when we fail to allocate a buffer for a stream.
2023-04-14 17:05:54 +02:00
Christopher Faulet
38656f406c MINOR: backend: Set SC_FL_ERROR on connection error
During connection establishement, if an error occurred, the SC_FL_ERROR flag
is now set. Concretely, it is set when SE_FL_ERROR is also set.
2023-04-14 17:05:53 +02:00
Christopher Faulet
a1d14a7c7f MINOR: stconn: Add a flag to ack endpoint errors at SC level
The flag SC_FL_ERROR is added to ack errors on the endpoint. When
SE_FL_ERROR flag is detected on the SE descriptor, the corresponding is set
on the SC. Idea is to avoid, as far as possible, to manipulated the SE
descriptor in upper layers and know when an error in the endpoint is handled
by the SC.

For now, this flag is only set and cleared but never tested.
2023-04-14 17:05:53 +02:00
Christopher Faulet
638fe6ab0f MINOR: stconn: Don't clear SE_FL_ERROR when endpoint is reset
There is no reason to remove this flag. When the SC endpoint is reset, it is
replaced by a new one. The old one is released. It was useful when the new
endpoint inherited some flags from the old one.  But it is no longer
performed. Thus there is no reason still unset this flag.
2023-04-14 17:05:53 +02:00
Christopher Faulet
e8bcef5f22 MEDIUM: stconn: Forbid applets with more to deliver if EOI was reached
When an applet is woken up, before calling its io_handler, we pretend it has
no more data to deliver. So, after the io_handler execution, it is a bug if
an applet states it has more data to deliver while the end of input is
reached.

So a BUG_ON() is added to be sure it never happens.
2023-04-14 17:05:53 +02:00
Christopher Faulet
56a2b608b0 MINOR: stconn: Stop to set SE_FL_ERROR on sending path
It is not the SC responsibility to report errors on the SE descriptor. It is
the endpoint responsibility. It must switch SE_FL_ERR_PENDING into
SE_FL_ERROR if the end of stream was detected. It can even be considered as
a bug if it is not done by he endpoint.

So now, on sending path, a BUG_ON() is added to abort if SE_FL_EOS and
SE_FL_ERR_PENDING flags are set but not SE_FL_ERROR. It is trully important
to handle this case in the endpoint to be able to properly shut the endpoint
down.
2023-04-14 17:05:53 +02:00
Christopher Faulet
d3bc340e7e BUG/MINOR: cli: Don't close when SE_FL_ERR_PENDING is set in cli analyzer
SE_FL_ERR_PENDING is used to report an error on the write side. But it is
not a terminal error. Some incoming data may still be available. In the cli
analyzers, it is important to not close the stream when this flag is
set. Otherwise the response to a command can be truncated. It is probably
hard to observe. But it remains a bug.

While this patch could be backported to 2.7, there is no real reason to do
so, except if someone reports a bug about truncated responses.
2023-04-14 16:49:04 +02:00
Christopher Faulet
214f1b5c16 MINOR: tree-wide: Replace several chn_prod() by the corresponding SC
At many places, call to chn_prod() can be easily replaced by the
corresponding SC. It is a bit easier to understand which side is
manipulated.
2023-04-14 15:06:04 +02:00
Christopher Faulet
64350bbf05 MINOR: tree-wide: Replace several chn_cons() by the corresponding SC
At many places, call to chn_cons() can be easily replaced by the
corresponding SC. It is a bit easier to understand which side is
manipulated.
2023-04-14 15:04:03 +02:00
Christopher Faulet
b2b1c3a6ea MINOR: channel/stconn: Replace sc_shutw() by sc_shutdown()
All reference to a shutw is replaced by an abort. So sc_shutw() is renamed
sc_shutdown(). SC app ops functions are renamed accordingly.
2023-04-14 15:02:57 +02:00
Christopher Faulet
208c712b40 MINOR: stconn: Rename SC_FL_SHUTW in SC_FL_SHUT_DONE
Here again, it is just a flag renaming. In SC flags, there is no longer
shutdown for writes but shutdowns.
2023-04-14 15:01:21 +02:00
Christopher Faulet
cfc11c0eae MINOR: channel/stconn: Replace sc_shutr() by sc_abort()
All reference to a shutr is replaced by an abort. So sc_shutr() is renamed
sc_abort(). SC app ops functions are renamed accordingly.
2023-04-14 14:54:35 +02:00
Christopher Faulet
0c370eee6d MINOR: stconn: Rename SC_FL_SHUTR in SC_FL_ABRT_DONE
Here again, it is just a flag renaming. In SC flags, there is no longer
shutdown for reads but aborts. For now this flag is set when a read0 is
detected. It is of couse not accurate. This will be changed later.
2023-04-14 14:51:22 +02:00
Christopher Faulet
df7cd710a8 MINOR: channel/stconn: Replace channel_shutw_now() by sc_schedule_shutdown()
After the flag renaming, it is now the turn for the channel function to be
renamed and moved in the SC scope. channel_shutw_now() is replaced by
sc_schedule_shutdown(). The request channel is replaced by the front SC and
the response is replace by the back SC.
2023-04-14 14:49:45 +02:00
Christopher Faulet
e38534cbd0 MINOR: stconn: Rename SC_FL_SHUTW_NOW in SC_FL_SHUT_WANTED
Because shutowns for reads are now considered as aborts, the shudowns for
writes can now be considered as shutdowns. Here it is just a flag
renaming. SC_FL_SHUTW_NOW is renamed SC_FL_SHUT_WANTED.
2023-04-14 14:46:07 +02:00
Christopher Faulet
12762f09a5 MINOR: channel/stconn: Replace channel_shutr_now() by sc_schedule_abort()
After the flag renaming, it is now the turn for the channel function to be
renamed and moved in the SC scope. channel_shutr_now() is replaced by
sc_schedule_abort(). The request channel is replaced by the front SC and the
response is replace by the back SC.
2023-04-14 14:08:49 +02:00
Christopher Faulet
573ead1e68 MINOR: stconn: Rename SC_FL_SHUTR_NOW in SC_FL_ABRT_WANTED
It is the first step to transform shutdown for reads for the upper layer
into aborts. This patch is quite simple, it is just a flag renaming.
2023-04-14 14:06:01 +02:00
Christopher Faulet
7eb837df4a MINOR: stream: Introduce stream_abort() to abort on both sides in same time
The function stream_abort() should now be called when an abort is performed
on the both channels in same time.
2023-04-14 14:04:59 +02:00
Christopher Faulet
3db538ac2f MINOR: channel: Forwad close to other side on abort
Most of calls to channel_abort() are associated to a call to
channel_auto_close(). Others are in areas where the auto close is the
default. So, it is now systematically enabled when an abort is performed on
a channel, as part of channel_abort() function.
2023-04-14 13:56:28 +02:00
Christopher Faulet
c394e21933 REGTESTS: fix the race conditions in log_uri.vtc
A "Connection: close" header is added to responses to avoid any connection
reuse. This should avoid any "HTTP header incomplete" errors.
2023-04-14 12:13:09 +02:00
Christopher Faulet
0adffb62c1 MINOR: filters: Review and simplify errors handling
First, it is useless to abort the both channel explicitly. For HTTP streams,
http_reply_and_close() is called. This function already take care to abort
processing. For TCP streams, we can rely on stream_retnclose().

To set termination flags, we can also rely on http_set_term_flags() for HTTP
streams and sess_set_term_flags() for TCP streams. Thus no reason to handle
them by hand.

At the end, the error handling after filters evaluation is now quite simple.
2023-04-14 12:13:09 +02:00
Christopher Faulet
dbad8ec787 MINOR: stream: Uninline and export sess_set_term_flags() function
This function will be used to set termination flags on TCP streams from
outside of process_stream(). Thus, it must be uninlined and exported.
2023-04-14 12:13:09 +02:00
Christopher Faulet
95125886ee BUG/MEDIUM: stconn: Do nothing in sc_conn_recv() when the SC needs more room
We erroneously though that an attempt to receive data was not possible if the SC
was waiting for more room in the channel buffer. A BUG_ON() was added to detect
bugs. And in fact, it is possible.

The regression was added in commit 341a5783b ("BUG/MEDIUM: stconn: stop to
enable/disable reads from streams via si_update_rx").

This patch should fix the issue #2115. It must be backported if the commit
above is backported.
2023-04-14 12:13:09 +02:00
Christopher Faulet
915ba08b57 BUG/MEDIUM: stream: Report write timeouts before testing the flags
A regression was introduced when stream's timeouts were refactored. Write
timeouts are not testing is the right order. When timeous of the front SC
are handled, we must then test the read timeout on the request channel and
the write timeout on the response channel. But write timeout is tested on
the request channel instead. On the back SC, the same mix-up is performed.

We must be careful to handle timeouts before checking channel flags. To
avoid any confusions, all timeuts are handled first, on front and back SCs.
Then flags of the both channels are tested.

It is a 2.8-specific issue. No backport needed.
2023-04-14 12:13:09 +02:00
Christopher Faulet
925279ccf2 BUG/MINOR: stream: Fix test on SE_FL_ERROR on the wrong entity
There is a bug at begining of process_stream(). The SE_FL_ERROR flag is
tested against backend stream-connector's flags instead of its SE
descriptor's flags. It is an old typo, introduced when the stream-interfaces
were replaced by the conn-streams.

This patch must be backported as far as 2.6.
2023-04-14 12:13:09 +02:00
Ilya Shipitsin
ea5a5e6feb CI: enable monthly test on Fedora Rawhide
Fedora Rawhide is shipped with the most recent compilers, not yet released with
more conservative distro. It is good to catch compile errors on those compilers.
2023-04-14 10:05:32 +02:00
Ilya Shipitsin
86a40f5de9 CI: bump "actions/checkout" to v3 for cross zoo matrix
actions/checkout@v2 is deprecated, accidently it was not updated in our
build definition
2023-04-14 10:05:09 +02:00
Frédéric Lécaille
895700bd32 BUG/MINOR: quic: Wrong Application encryption level selection when probing
This bug arrived with this commit:

    MEDIUM: quic: Ack delay implementation

After having probed the Handshake packet number space, one must not select the
Application encryption level to continue trying building packets as this is done
when the connection is not probing. Indeed, if the ACK timer has been triggered
in the meantime, the packet builder will try to build a packet at the Application
encryption level to acknowledge the received packet. But there is very often
no 01RTT packet to acknowledge when the connection is probing before the
handshake is completed. This triggers a BUG_ON() in qc_do_build_pkt() which
checks that the tree of ACK ranges to be used is not empty.

Thank you to @Tristan971 for having reported this issue in GH #2109.

Must be backported to 2.6 and 2.7.
2023-04-13 19:20:09 +02:00
Frédéric Lécaille
a576c1b0c6 MINOR: quic: Remove a useless test about probing in qc_prep_pkts()
qel->pktns->tx.pto_probe is set to 0 after having prepared a probing
datagram. There is no reason to check this parameter. Furthermore
it is always 0 when the connection does not probe the peer.

Must be backported to 2.6 and 2.7.
2023-04-13 19:20:09 +02:00
Frédéric Lécaille
91369cfcd0 MINOR: quic: Display the packet number space flags in traces
Display this information when the encryption level is also displayed.

Must be backported to 2.6 and 2.7.
2023-04-13 19:20:08 +02:00
Frédéric Lécaille
595251f22e BUG/MINOR: quic: SIGFPE in quic_cubic_update()
As reported by @Tristan971 in GH #2116, the congestion control window could be zero
due to an inversion in the code about the reduction factor to be applied.
On a new loss event, it must be applied to the slow start threshold and the window
should never be below ->min_cwnd (2*max_udp_payload_sz).

Same issue in both newReno and cubic algorithm. Furthermore in newReno, only the
threshold was decremented.

Must be backported to 2.6 and 2.7.
2023-04-13 19:20:08 +02:00
Frédéric Lécaille
9d68c6aaf6 BUG/MINOR: quic: Possible wrapped values used as ACK tree purging limit.
Add two missing checks not to substract too big values from another too little
one. In this case the resulted wrapped huge values could be passed to the function
which has to remove the last range of a tree of ACK ranges as encoded limit size
not to go below, cancelling the ACK ranges deletion. The consequence could be that
no ACK were sent.

Must be backported to 2.6 and 2.7.
2023-04-13 19:20:08 +02:00
Frédéric Lécaille
45bf1a82f1 BUG/MEDIUM: quic: Code sanitization about acknowledgements requirements
qc_may_build_pkt() has been modified several times regardless of the conditions
the functions it is supposed to allow to send packets (qc_build_pkt()/qc_do_build_pkt())
really use to finally send packets just after having received others, leading
to contraditions and possible very long loops sending empty packets (PADDING only packets)
because qc_may_build_pkt() could allow qc_build_pkt()/qc_do_build_pkt to build packet,
and the latter did nothing except sending PADDING frames, because from its point
of view they had nothing to send.

For now on, this is the job of qc_may_build_pkt() to decide to if there is
packets to send just after having received others AND to provide this information
to the qc_build_pkt()/qc_do_build_pkt()

Note that the unique case where the acknowledgements are completely ignored is
when the endpoint must probe. But at least this is when sending at most two datagrams!

This commit also fixes the issue reported by Willy about a very low throughput
performance when the client serialized its requests.

Must be backported to 2.7 and 2.6.
2023-04-13 19:20:08 +02:00
Frédéric Lécaille
eb3e5171ed MINOR: quic: Add connection flags to traces
This should help in diagnosing issues.

Some adjustments have to be done to avoid deferencing a quic_conn objects from
TRACE_*() calls.

Must be backported to 2.7 and 2.6.
2023-04-13 19:20:08 +02:00
Frédéric Lécaille
809bd9fed1 BUG/MINOR: quic: Ignored less than 1ms RTTs
Do not ignore very short RTTs (less than 1ms) before computing the smoothed
RTT initializing it to an "infinite" value (UINT_MAX).

Must be backported to 2.7 and 2.6.
2023-04-13 19:20:08 +02:00
Frédéric Lécaille
fad0e6cf73 MINOR: quic: Add packet loss and maximum cc window to "show quic"
Add the number of packet losts and the maximum congestion control window computed
by the algorithms to "show quic".
Same thing for the traces of existent congestion control algorithms.

Must be backported to 2.7 and 2.6.
2023-04-13 19:20:08 +02:00
Olivier Houchard
f98a8c317e BUG/MEDIUM: fd: don't wait for tmask to stabilize if we're not in it.
In fd_update_events(), we loop until there's no bit in the running_mask
that is not in the thread_mask. Problem is, the thread sets its
running_mask bit before that loop, and so if 2 threads do the same, and
a 3rd one just closes the FD and sets the thread_mask to 0, then
running_mask will always be non-zero, and we will loop forever. This is
trivial to reproduce when using a DNS resolver that will just answer
"port unreachable", but could theoretically happen with other types of
file descriptors too.

To fix that, just don't bother looping if we're no longer in the
thread_mask, if that happens we know we won't have to take care of the
FD, anyway.

This should be backported to 2.7, 2.6 and 2.5.
2023-04-13 18:04:46 +02:00
Willy Tarreau
a07635ead5 MINOR: bind-conf: support a new shards value: "by-group"
Setting "shards by-group" will create one shard per thread group. This
can often be a reasonable tradeoff between a single one that can be
suboptimal on CPUs with many cores, and too many that will eat a lot
of file descriptors. It was shown to provide good results on a 224
thread machine, with a distribution that was even smoother than the
system's since here it can take into account the number of connections
per thread in the group. Depending on how popular it becomes, it could
even become the default setting in a future version.
2023-04-13 17:38:31 +02:00
Willy Tarreau
d30e82b9f0 MINOR: receiver: reserve special values for "shards"
Instead of artificially setting the shards count to MAX_THREAD when
"by-thread" is used, let's reserve special values for symbolic names
so that we can add more in the future. For now we use value -1 for
"by-thread", which requires to turn the type to signed int but it was
already used as such everywhere anyway.
2023-04-13 17:12:50 +02:00