509 Commits

Author SHA1 Message Date
Amaury Denoyelle
bbb1c68508 BUG/MINOR: quic: fix subscribe operation
Subscribing was not properly designed between quic-conn and quic MUX
layers. Align this as with in other haproxy components : <subs> field is
moved from the MUX to the quic-conn structure. All mention of qcc MUX is
cleaned up in quic_conn_subscribe()/quic_conn_unsubscribe().

Thanks to this change, ACK reception notification has been simplified.
It's now unnecessary to check for the MUX existence before waking it.
Instead, if <subs> quic-conn field is set, just wake-up the upper layer
tasklet without mentionning MUX. This should probably be extended to
other part in quic-conn code.

This should be backported up to 2.6.
2022-10-26 18:18:26 +02:00
Amaury Denoyelle
0aba11e9e7 MINOR: quic: remove unnecessary quic_session_accept()
A specialized listener accept was previously used for QUIC. This is now
unneeded and we can revert to the default one session_accept_fd().

One change of importance is that the call order between
conn_xprt_start() and conn_complete_session() is now reverted to the
default one. This means that MUX instance is now NULL during
qc_xprt_start() and its app-ops layer cannot be set here. This operation
has been delayed to qc_init() to prevent a segfault.

This should be backported up to 2.6.
2022-10-26 18:16:20 +02:00
Amaury Denoyelle
176174f7e4 BUG/MINOR: mux-quic: complete flow-control for uni streams
Max stream data was not enforced and respect for local/remote uni
streams. Previously, qcs instances incorrectly reused the limit defined
from bidirectional ones.

This is now fixed. Two fields are added in qcc structure connection :
* value for local flow control to enforce on remote uni streams
* value for remote flow control to respect on local uni streams

These two values can be reused to properly initialized msd field of a
qcs instance in qcs_new(). The rest of the code is similar.

This must be backported up to 2.6.
2022-10-21 17:31:18 +02:00
Amaury Denoyelle
036cc5d880 MINOR: mux-quic: check quic-conn return code on Tx
Inspect return code of qc_send_mux(). If quic-conn layer reports an
error, this will interrupt the current emission process.

This should be backported up to 2.6.
2022-10-05 11:08:32 +02:00
Amaury Denoyelle
d7755375a5 BUG/MINOR: mux-quic: ignore STOP_SENDING for locally closed stream
It is possible to receive a STOP_SENDING frame for a locally closed
stream. This was not properly managed as this would result in a BUG_ON()
crash from qcs_idle_open() call under qcc_recv_stop_sending().

Now, STOP_SENDING frames are ignored when received on streams already
locally closed. This has two consequences depending on the reason of
closure :

* if a RESET_STREAM was already emitted and closed the stream, this
  patch prevents to emit a new RESET_STREAM. This behavior is thus
  better.

* if stream was closed due to all data transmitted, no RESET_STREAM will
  be built. This is contrary to the RFC 9000 which advice to transmit
  it, even on "Data Sent" state. However, this is not mandatory so the
  new behavior is acceptable, even if it could be improved.

This crash has been detected on haproxy.org. This can be artifically
reproduced by adding the following snippet at the end of qc_send_mux()
when doing a request with a small payload response :
  qcc_recv_stop_sending(qc->qcc, 0, 0);

This must be backported up to 2.6.
2022-10-03 17:20:31 +02:00
Amaury Denoyelle
92fa63f735 CLEANUP: quic: create a dedicated quic_conn module
xprt_quic module was too large and did not reflect the true architecture
by contrast to the other protocols in haproxy.

Extract code related to XPRT layer and keep it under xprt_quic module.
This code should only contains a simple API to communicate between QUIC
lower layer and connection/MUX.

The vast majority of the code has been moved into a new module named
quic_conn. This module is responsible to the implementation of QUIC
lower layer. Conceptually, it overlaps with TCP kernel implementation
when comparing QUIC and HTTP1/2 stacks of haproxy.

This should be backported up to 2.6.
2022-10-03 16:25:17 +02:00
Amaury Denoyelle
0ed617ac2f BUG/MEDIUM: mux-quic: properly trim HTX buffer on snd_buf reset
MUX QUIC snd_buf operation whill return early if a qcs instance is
resetted. In this case, HTX is left untouched and the callback returns
the whole bufer size. This lead to an undefined behavior as the stream
layer is notified about a transfer but does not see its HTX buffer
emptied. In the end, the transfer may stall which will lead to a leak on
session.

To fix this, HTX buffer is now resetted when snd_buf is short-circuited.
This should fix the issue as now the stream layer can continue the
transfer until its completion.

This patch has already been tested by Tristan and is reported to solve
the github issue #1801.

This should be backported up to 2.6.
2022-09-20 15:35:33 +02:00
Amaury Denoyelle
9534e59bb9 MINOR: mux-quic: refactor snd_buf
Factorize common code between h3 and hq-interop snd_buf operation. This
is inserted in MUX QUIC snd_buf own callback.

The h3/hq-interop API has been adjusted to directly receive a HTX
message instead of a plain buf. This led to extracting part of MUX QUIC
snd_buf in qmux_http module.

This should be backported up to 2.6.
2022-09-20 15:35:29 +02:00
Amaury Denoyelle
d80fbcaca2 REORG: mux-quic: export HTTP related function in a dedicated file
Extract function dealing with HTX outside of MUX QUIC. For the moment,
only rcv_buf stream operation is concerned.

The main objective is to be able to support both TCP and HTTP proxy mode
with a common base and add specialized modules on top of it.

This should be backported up to 2.6.
2022-09-20 15:35:23 +02:00
Amaury Denoyelle
36d50bff22 REORG: mux-quic: extract traces in a dedicated source file
QUIC MUX implements several APIs to interface with stream, quic-conn and
app-ops layers. It is planified to better separate this roles, possibly
by using several files.

The first step is to extract QUIC MUX traces in a dedicated source
files. This will allow to reuse traces in multiple files.

The main objective is to be
able to support both TCP and HTTP proxy mode with a common base and add
specialized modules on top of it.

This should be backported up to 2.6.
2022-09-20 15:35:09 +02:00
Amaury Denoyelle
3dc4e5a5b9 BUG/MINOR: mux-quic: do not keep detached qcs with empty Tx buffers
A qcs instance free may be postponed in stream detach operation if the
stream is not locally closed. This condition is there to achieve
transfering data still present in Tx buffer. Once all data have been
emitted to quic-conn layer, qcs instance can be released.

However, the stream is only closed locally if HTX EOM has been seen or
it has been resetted. In case the transfer finished without EOM, a
detached qcs won't be freed even if there is no more activity on it.

This bug was not reproduced but was found on code analysis. Its precise
impact is unknown but it should not cause any leak as all qcs instances
are freed with its parent qcc connection : this should eventually happen
on MUX timeout or QUIC idle timeout.

To adjust this, condition to mark a stream as locally closed has been
extended. On qcc_streams_sent_done() notification, if its Tx buffer has
been fully transmitted, it will be closed if either FIN STREAM was set
or the stream is detached.

This must be backported up to 2.6.
2022-09-20 10:46:59 +02:00
Amaury Denoyelle
afb7b9d8e5 BUG/MEDIUM: mux-quic: fix nb_hreq decrement
nb_hreq is a counter on qcc for active HTTP requests. It is incremented
for each qcs where a full HTTP request was received. It is decremented
when the stream is closed locally :
- on HTTP response fully transmitted
- on stream reset

A bug will occur if a stream is resetted without having processed a full
HTTP request. nb_hreq will be decremented whereas it was not
incremented. This will lead to a crash when building with
DEBUG_STRICT=2. If BUG_ON_HOT are not active, nb_hreq counter will wrap
which may break the timeout logic for the connection.

This bug was triggered on haproxy.org. It can be reproduced by
simulating the reception of a STOP_SENDING frame instead of a STREAM one
by patching qc_handle_strm_frm() :

+       if (quic_stream_is_bidi(strm_frm->id))
+               qcc_recv_stop_sending(qc->qcc, strm_frm->id, 0);
+       //ret = qcc_recv(qc->qcc, strm_frm->id, strm_frm->len,
+       //               strm_frm->offset.key, strm_frm->fin,
+       //               (char *)strm_frm->data);

To fix this bug, a qcs is now flagged with a new QC_SF_HREQ_RECV. This
is set when the full HTTP request is received. When the stream is closed
locally, nb_hreq will be decremented only if this flag was set.

This must be backported up to 2.6.
2022-09-19 12:12:21 +02:00
cui fliter
a94bedc0de CLEANUP: quic,ssl: fix tiny typos in C comments
This fixes 4 tiny and harmless typos in mux_quic.c, quic_tls.c and
ssl_sock.c. Originally sent via GitHub PR #1843.

Signed-off-by: cui fliter <imcusg@gmail.com>
[Tim: Rephrased the commit message]
[wt: further complete the commit message]
2022-09-17 10:59:59 +02:00
Amaury Denoyelle
d1310f8d32 BUG/MINOR: mux-quic: do not remotely close stream too early
A stream is considered as remotely closed once we have received all the
data with the FIN bit set.

The condition to close the stream was wrong. In particular, if we
receive an empty STREAM frame with FIN bit set, this would have close
the stream even if we do not have yet received all the data. The
condition is now adjusted to ensure that Rx buffer contains all the data
up to the stream final size.

In most cases, this bug is harmless. However, if compiled with
DEBUG_STRICT=2, a BUG_ON_HOT crash would have been triggered if close is
done too early. This was most notably the case sometimes on interop test
suite with quinn or kwik clients. This can also be artificially
reproduced by simulating reception of an empty STREAM frame with FIN bit
set in qc_handle_strm_frm() :

+       if (strm_frm->fin) {
+               qcc_recv(qc->qcc, strm_frm->id, 0,
+                        strm_frm->len, strm_frm->fin,
+                        (char *)strm_frm->data);
+       }
        ret = qcc_recv(qc->qcc, strm_frm->id, strm_frm->len,
                       strm_frm->offset.key, strm_frm->fin,
                       (char *)strm_frm->data);

This must be backported up to 2.6.
2022-09-16 14:17:27 +02:00
Amaury Denoyelle
8d4ac48d3d CLEANUP: mux-quic: remove stconn usage in h3/hq
Small cleanup on snd_buf for application protocol layer.
* do not export h3_snd_buf
* replace stconn by a qcs argument. This is better as h3/hq-interop only
  uses the qcs instance.

This should be backported up to 2.6.
2022-09-16 13:53:30 +02:00
Amaury Denoyelle
f8aaf8bdfa BUG/MEDIUM: mux-quic: fix crash on early app-ops release
H3 SETTINGS emission has recently been delayed. The idea is to send it
with the first STREAM to reduce sendto syscall invocation. This was
implemented in the following patch :
  3dd79d378c86b3ebf60e029f518add5f1ed54815
  MINOR: h3: Send the h3 settings with others streams (requests)

This patch works fine under nominal conditions. However, it will cause a
crash if a HTTP/3 connection is released before having sent any data,
for example when receiving an invalid first request. In this case,
qc_release will first free qcc.app_ops HTTP/3 application protocol layer
via release callback. Then qc_send is called to emit any closing frames
built by app_ops release invocation. However, in qc_send, as no data has
been sent, it will try to complete application layer protocol
intialization, with a SETTINGS emission for HTTP/3. Thus, qcc.app_ops is
reused, which is invalid as it has been just freed. This will cause a
crash with h3_finalize in the call stack.

This bug can be reproduced artificially by generating incomplete HTTP/3
requests. This will in time trigger http-request timeout without any
data send. This is done by editing qc_handle_strm_frm function.

-       ret = qcc_recv(qc->qcc, strm_frm->id, strm_frm->len,
+       ret = qcc_recv(qc->qcc, strm_frm->id, strm_frm->len - 1,
                       strm_frm->offset.key, strm_frm->fin,
                       (char *)strm_frm->data);

To fix this, application layer closing API has been adjusted to be done
in two-steps. A new shutdown callback is implemented : it is used by the
HTTP/3 layer to generate GOAWAY frame in qc_release prologue.
Application layer context qcc.app_ops is then freed later in qc_release
via the release operation which is now only used to liberate app layer
ressources. This fixes the problem as the intermediary qc_send
invocation will be able to reuse app_ops before it is freed.

This patch fixes the crash, but it would be better to adjust H3 SETTINGS
emission in case of early connection closing : in this case, there is no
need to send it. This should be implemented in a future patch.

This should fix the crash recently experienced by Tristan in github
issue #1801.

This must be backported up to 2.6.
2022-09-15 10:41:44 +02:00
Frédéric Lécaille
3dd79d378c MINOR: h3: Send the h3 settings with others streams (requests)
This is the ->finalize application callback which prepares the unidirectional STREAM
frames for h3 settings and wakeup the mux I/O handler to send them. As haproxy is
at the same time always waiting for the client request, this makes haproxy
call sendto() to send only about 20 bytes of stream data. Furthermore in case
of heavy loss, this give less chances to short h3 requests to succeed.

Drawback: as at this time the mux sends its streams by their IDs ascending order
the stream 0 is always embedded before the unidirectional stream 3 for h3 settings.
Nevertheless, as these settings may be lost and received after other h3 request
streams, this is permitted by the RFC.

Perhaps there is a better way to do. This will have to be checked with Amaury.

Must be backported to 2.6.
2022-09-08 18:04:58 +02:00
Willy Tarreau
41e701e2c1 DEBUG: quic: export the few task handlers that often appear in task dumps
The following task/tasklet handlers often appear in "show profiling tasks"
but were not resolved since static:

 qc_io_cb, quic_conn_app_io_cb, process_timer,
 quic_accept_run, qc_idle_timer_task

This commit simply exports them so they can be resolved now. "process_timer"
which was a bit too generic and renamed to qc_process_timer.
2022-09-08 16:13:38 +02:00
Willy Tarreau
b4a4feee87 MINOR: mux-quic: provide a "show_sd" helper to output stream debugging info
It's very limited but at least provides the very basic info about QCS and
QCC when issuing "show sess all":

  scf=0x7fa9642394a0 flags=0x00000080 state=EST endp=CONN,0x7fa9642351f0,0x02001001 sub=3
>     qcs=0x7fa9642351f0 .flg=0x5 .id=396 .st=HCR .ctx=0x7fa9642353f0, .err=0
>     qcc=0x7fa96405ce20 .flg=0 .nbsc=100 .nbhreq=100, .task=0x7fa964054260
      co0=0x7fa96405cd50 ctrl=quic4 xprt=QUIC mux=QUIC data=STRM target=LISTENER:0x328c530
      flags=0x00200300 fd=-1 fd.state=00 updt=0 fd.tmask=0x0

It will need to be improved but it's better than nothing already. This
should be backported to 2.6 if the other dumps are backported.
2022-09-02 16:43:25 +02:00
Frédéric Lécaille
e4c3074c00 MINOR: quic: Add the QUIC connection to mux traces
This should help for debugging purpose.

Should be backported to 2.6
2022-08-19 12:02:29 +02:00
Amaury Denoyelle
704675656b BUG/MEDIUM: quic: fix crash on MUX send notification
MUX notification on TX has been edited recently : it will be notified
only when sending its own data, and not for example on retransmission by
the quic-conn layer. This is subject of the patch :
  b29a1dc2f4a334c1c7fea76c59abb4097422c05c
  BUG/MINOR: quic: do not notify MUX on frame retransmit

A new flag QUIC_FL_CONN_RETRANS_LOST_DATA has been introduced to
differentiate qc_send_app_pkts invocation by MUX and directly by the
quic-conn layer in quic_conn_app_io_cb(). However, this is a first
problem as internal quic-conn layer usage is not limited to
retransmission. For example for NEW_CONNECTION_ID emission.

Another problem much important is that send functions are also called
through quic_conn_io_cb() which has not been protected from MUX
notification. This could probably result in crash when trying to notify
the MUX.

To fix both problems, quic-conn flagging has been inverted : when used
by the MUX, quic-conn is flagged with QUIC_FL_CONN_TX_MUX_CONTEXT. To
improve the API, MUX must now used qc_send_mux which ensure the flag is
set. qc_send_app_pkts is now static and can only be used by the
quic-conn layer.

This must be backported wherever the previously mentionned patch is.
2022-08-18 11:33:22 +02:00
Amaury Denoyelle
cc13047364 MINOR: quic: refactor application send
Adjust qc_send_app_pkts function : remove <old_data> arg and provide a
new wrapper function qc_send_app_probing() which should be used instead
when probing with old data.

This simplifies the interface of the default function, most notably for
the MUX which does not interfer with retransmission.
QUIC_FL_CONN_RETRANS_OLD_DATA flag is set/unset directly in the wrapper
qc_send_app_probing().

At the same time, function documentation has been updated to clarified
arguments and return values.

This commit will be useful for the next patch to differentiate MUX and
retransmission send context. As a consequence, the current patch should
be backported wherever the next one will be.
2022-08-17 11:05:49 +02:00
Amaury Denoyelle
3baab744e7 MINOR: mux-quic: add missing args on some traces
Complete some MUX traces by adding qcc or qcs instance as arguments when
this is possible. This will be useful when several connections are
interleaved.
2022-08-17 11:05:47 +02:00
Amaury Denoyelle
fd79ddb2d6 MINOR: mux-quic: adjust traces on stream init
Adjust traces on qcc_init_stream_remote() : replace "opening" by
"initializing" to avoid confusion with traces dealing with OPEN stream
state.
2022-08-17 11:05:46 +02:00
Amaury Denoyelle
bf3c208760 BUG/MEDIUM: mux-quic: reject uni stream ID exceeding flow control
Emit STREAM_LIMIT_ERROR if a client tries to open an unidirectional
stream with an ID greater than the value specified by our flow-control
limit. The code is similar to the bidirectional stream opening.

MAX_STREAMS_UNI emission is not implement for the moment and is left as
a TODO. This should not be too urgent for the moment : in HTTP/3, a
client has only a limited use for unidirectional streams (H3 control
stream + 2 QPACK streams). This is covered by the value provided by
haproxy in transport parameters.

This patch has been tagged with BUG as it should have prevented last
crash reported on github issue #1808 when opening a new unidirectional
streams with an invalid ID. However, it is probably not the main cause
of the bug contrary to the patch
  commit 11a6f4007b908b49ecd3abd5cd10fba177f07c11
  BUG/MINOR: quic: Wrong status returned by qc_pkt_decrypt()

This must be backported up to 2.6.
2022-08-17 11:05:19 +02:00
Amaury Denoyelle
35a66c0a36 BUG/MINOR: mux-quic: fix crash with traces in qc_detach()
qc_detach() is used to free a qcs as notified by sedesc. If there is no
more stream active and the connection is considered as dead, it will
then be freed. This prevent to dereference qcc in TRACE macro. Else this
will cause a crash.

Use a different code-path on release for qc_detach() to fix this bug.

This will fix the last occurence of crash on github issue #1808.

This has been introduced by recent QUIC MUX traces rework. Thus, it does
not need to be backport.
2022-08-12 16:02:00 +02:00
Amaury Denoyelle
6bdf9367fb BUG/MEDIUM: mux-quic: fix crash due to invalid trace arg
Traces argument were incorrectly used in qcs_free(). A qcs was specified
as first arg instead of a connection. This will lead to a crash if
developer qmux traces are activated. This is now fixed.

This bug has been introduced with QUIC MUX traces rework. No need to
backport.
2022-08-11 18:24:53 +02:00
Amaury Denoyelle
4c9a1642c1 MINOR: mux-quic: define new traces
Add new traces to help debugging on QUIC MUX. Most notable, the
following functions are now traced :
* qcc_emit_cc
* qcs_free
* qcs_consume
* qcc_decode_qcs
* qcc_emit_cc_app
* qcc_install_app_ops
* qcc_release_remote_stream
* qcc_streams_sent_done
* qc_init
2022-08-11 15:20:44 +02:00
Amaury Denoyelle
047d86a34b CLEANUP: mux-quic: adjust traces level
Change default devel level for some traces in QUIC MUX:
* proto : used to notify about reception/emission of frames
* state : modification of internal state of connection or streams
* data : detailled information about transfer and flow-control
2022-08-11 15:20:44 +02:00
Amaury Denoyelle
c7fb0d2b7a MINOR: mux-quic: define protocol error traces
Replace devel traces with error level on all errors situation. Also a
new event QMUX_EV_PROTO_ERR is used. This should help to detect invalid
situations quickly.
2022-08-11 15:20:44 +02:00
Amaury Denoyelle
f0b67f995c MINOR: mux-quic: adjust enter/leave traces
Improve MUX traces by adding some missing enter/leave trace points. In
some places, early function returns have been replaced by a goto
statement.
2022-08-11 15:20:43 +02:00
Amaury Denoyelle
96ca1b7c39 BUG/MINOR: mux-quic: open stream on STOP_SENDING
Considered a stream as opened when receiving a STOP_SENDING frame as the
first frame on the stream.

This patch is tagged as BUG because a BUG_ON may occur if only a
STOP_SENDING frame has been received for a frame. This will reset the
stream in respect with RFC9000 but internally it is considered invalid
transition to reset an idle stream.

To fix this, simply use qcs_idle_open() on STOP_SENDING parsing
function. This will mark the stream as OPEN before resetting it.

This was detected on haproxy.org with the following backtrace :

FATAL: bug condition "qcs->st == QC_SS_IDLE" matched at
src/mux_quic.c:383
  call trace(12):
  |       0x490dd3 [b8 01 00 00 00 c6 00 00]: main-0x1d0633
  |       0x4975b8 [48 8b 85 58 ff ff ff 8b]: main-0x1c9e4e
  |       0x497df4 [48 8b 45 c8 48 89 c7 e8]: main-0x1c9612
  |       0x49934c [48 8b 45 c8 48 89 c7 e8]: main-0x1c80ba
  |       0x6b3475 [48 8b 05 54 1b 3a 00 64]: run_tasks_from_lists+0x45d/0x8b2
  |       0x6b4093 [29 c3 89 d8 89 45 d0 83]: process_runnable_tasks+0x7c9/0x824
  |       0x660bde [8b 05 fc b3 4f 00 83 f8]: run_poll_loop+0x74/0x430
  |       0x6611de [48 8b 05 7b a6 40 00 48]: main-0x228
  | 0x7f66e4fb2ea5 [64 48 89 04 25 30 06 00]: libpthread:+0x7ea5
  | 0x7f66e455ab0d [48 89 c7 e8 5b 72 fc ff]: libc:clone+0x6d/0x86

Stream states have been implemented in the current dev tree. Thus, this
patch does not need to be backported.
2022-08-09 17:58:02 +02:00
Amaury Denoyelle
56c6154dba CLEANUP: mux-quic: remove loop on sending frames
qc_send_app_pkts() has now a while loop implemented which allows to send
all possible frames even if the send buffer is full between packet
prepare and send. This is present since commit :
  dc07751ed7ebad10f49081d28a9a5ae785f53d76
  MINOR: quic: Send packets as much as possible from qc_send_app_pkts()

This means we can remove code from the MUX which implement this at the
upper layer. This is useful to simplify qc_send_frames() function.

As mentionned commit is subject to backport, this commit should be
backported as well to 2.6.
2022-08-09 15:41:07 +02:00
Ilya Shipitsin
3b64a28e15 CLEANUP: assorted typo fixes in the code and comments
This is 31st iteration of typo fixes
2022-08-06 17:12:51 +02:00
Amaury Denoyelle
30e260e2e6 MEDIUM: mux-quic: implement http-request timeout
Implement http-request timeout for QUIC MUX. It is used when the
connection is opened and is triggered if no HTTP request is received in
time. By HTTP request we mean at least a QUIC stream with a full header
section. Then qcs instance is attached to a sedesc and upper layer is
then responsible to wait for the rest of the request.

This timeout is also used when new QUIC streams are opened during the
connection lifetime to wait for full HTTP request on them. As it's
possible to demux multiple streams in parallel with QUIC, each waiting
stream is registered in a list <opening_list> stored in qcc with <start>
as timestamp in qcs for the stream opening. Once a qcs is attached to a
sedesc, it is removed from <opening_list>. When refreshing MUX timeout,
if <opening_list> is not empty, the first waiting stream is used to set
MUX timeout.

This is efficient as streams are stored in the list in their creation
order so CPU usage is minimal. Also, the size of the list is
automatically restricted by flow control limitation so it should not
grow too much.

Streams are insert in <opening_list> by application protocol layer. This
is because only application protocol can differentiate streams for HTTP
messaging from internal usage. A function qcs_wait_http_req() has been
added to register a request stream by app layer. QUIC MUX can then
remove it from the list in qc_attach_sc().

As a side-note, it was necessary to implement attach qcc_app_ops
callback on hq-interop module to be able to insert a stream in waiting
list. Without this, a BUG_ON statement would be triggered when trying to
remove the stream on sedesc attach. This is to ensure that every
requests streams are registered for http-request timeout.

MUX timeout is explicitely refreshed on MAX_STREAM_DATA and STOP_SENDING
frame parsing to schedule http-request timeout if a new stream has been
instantiated. It was already done on STREAM parsing due to a previous
patch.
2022-08-03 15:04:18 +02:00
Amaury Denoyelle
6ec9837fca MINOR: mux-quic: refactor refresh timeout function
Try to reorganize qcc_refresh_timeout() to improve its readability. The
main objective is to reduce the indentation level and if sequences by
using goto statement to the end of the function. Also, backend and
frontend code path should be more explicit with this new version.
2022-08-03 15:04:18 +02:00
Amaury Denoyelle
418ba21461 MINOR: mux-quic: refresh timeout on frame decoding
Refresh the MUX connection timeout in frame parsing functions. This is
necessary as these Rx operation are completed directly from the
quic-conn layer outside of MUX I/O callback. Thus, the timeout should be
refreshed on this occasion.

Note that however on STREAM parsing refresh is only conducted when
receiving the current consecutive data offset.

Timeouts related function have been moved up in the source file to be
able to use them in qcc_decode_qcs().

This commit will be useful for http-request timeout. Indeed, a new
stream may be opened during qcc_decode_qcs() which should trigger this
timeout until a full header section is received and qcs instance is
attached to sedesc.
2022-08-03 15:04:18 +02:00
Amaury Denoyelle
bd6ec1bf84 MEDIUM: mux-quic: implement http-keep-alive timeout
Complete QUIC MUX timeout refresh function by using http-keep-alive
timeout. It is used when the connection is idle after having handle at
least one request.

To implement this a new member <idle_start> has been defined in qcc
structure. This is used as timestamp for when the connection became idle
and is used as base time for http keep-alive timeout
2022-08-01 15:00:13 +02:00
Amaury Denoyelle
c603de4d84 MINOR: mux-quic: count in-progress requests
Add a new qcc member named <nb_hreq>. Its purpose is close to <nb_sc>
which represents the number of attached stream connectors. Both are
incremented inside qc_attach_sc().

The difference is on the decrement operation. While <nb_cs> is
decremented on sedesc detach callback, <nb_hreq> is decremented when the
qcs is locally closed.

In most cases, <nb_hreq> will be decremented before <nb_cs>. However, it
will be the reverse if a stream must be kept alive after detach callback.

The main purpose of this field is to implement http-keep-alive timeout.
Both <nb_sc> and <nb_hreq> must be null to activate the http-keep-alive
timeout.
2022-08-01 14:58:41 +02:00
Amaury Denoyelle
5fc05d17ad MEDIUM: mux-quic: adjust timeout refresh
Implement a new internal function qcc_refresh_timeout(). Its role will be
to reset QUIC MUX timeout depending if there is requests in progress or
not.

qcc_update_timeout() does not set a timeout if there is still attached
streams as in this case the upper layer is responsible to manage it.
Else it will activate the timeout depending on the connection current
status.

Timeout is refreshed on several locations : on stream detach and in I/O
handler and wake callback.

For the moment, only the default timeout is used (client or server). The
function may be expanded in the future to support more specific ones :
* http-keep-alive if connection is idle
* http-request when waiting for incomplete HTTP requests
* client/server-fin for graceful shutdown
2022-08-01 14:58:36 +02:00
Amaury Denoyelle
b6309456d0 MINOR: mux-quic: use timeout server for backend conns
Use timeout server in qcc_init() as default timeout for backend
connections. No impact for the moment as QUIC backend support is not
implemented.
2022-08-01 14:23:21 +02:00
Amaury Denoyelle
07bf8f4d86 MINOR: mux-quic: save proxy instance into qcc
Store a reference to proxy in the qcc structure. This will be useful to
access to proxy members outside of qcc_init().

Most notably, this change is required to implement timeout refreshing by
using the various timeouts configured at the proxy level.
2022-08-01 14:23:21 +02:00
Amaury Denoyelle
09ec3e09bd BUG/MINOR: mux-quic: do not free conn if attached streams
Ensure via qcc_is_dead() that a connection is not released instance
until all of qcs streams are detached by the upper layer, even if an
error has been reported or the timeout has fired.

On the other side, as qc_detach() always check the connection status,
this should ensure that we do not keep a connection if not necessary.

Without this patch, a qcc instance may be freed with some of its qcs
streams not detached. This is an incorrect behavior and will lead to a
BUG_ON fault. Note however that no occurence of this bug has been
produced currently. This patch is mainly a safety against future
occurences.

This should be backported up to 2.6.
2022-08-01 14:23:19 +02:00
Amaury Denoyelle
4ea5090f55 CLEANUP: mux-quic: remove useless app_ops is_active callback
Timeout in QUIC MUX has evolved from the simple first implementation. At
the beginning, a connection was considered dead unless bidirectional
streams were opened. This was abstracted through an app callback
is_active().

Now this paradigm has been reversed and a connection is considered alive
by default, unless an error has been reported or a timeout has already
been fired. The callback is_active() is thus not used anymore and can be
safely removed to simplify qcc_is_dead().

This commit should be backported to 2.6.
2022-08-01 14:13:51 +02:00
Amaury Denoyelle
d3973853c2 BUG/MINOR: mux-quic: prevent crash if conn released during IO callback
A qcc instance may be freed in the middle of qc_io_cb() if all streams
were purged. This will lead to a crash as qcc instance is reused after
this step. Jump directly to the end of the function to avoid this.

Note that this bug has not been triggered for the moment. This is a
safety fix to prevent it.

This must be backported up to 2.6.
2022-08-01 14:13:51 +02:00
Amaury Denoyelle
72a78e8290 BUG/MEDIUM: mux-quic: fix missing EOI flag to prevent streams leaks
On H3 DATA frame transfer from the client, some streams are not properly
closed by the upper layer, despite all transfer operation completed.
Data integrity is not impacted but this will prevent the stream timeout
to fire and thus keep the owner session opened.

In most cases, sessions are closed on QUIC idle timeout, but it may stay
forever if a client emits PING frames at a regular interval to maintain
it.

This bug is caused by a missing EOI stream desc flag on certain
condition in qc_rcv_buf(). To be triggered, we have to use the
optimization when conn-stream buffer is empty and can be swapped with
qcs buffer. The problem is that it will skip the function body for
default copy but also a condition to check if EOI must be set. Thus this
bug does not happens for every H3 post requets : it requires that
conn-stream buffer is empty on last qc_rcv_buf() invocation.

This was reproduced more frequently when using ngtcp2 client with one or
multiple streams :
$ ngtcp2-client -m POST -d ~/infra/html/10K 127.0.0.1 20443 \
  http://127.0.0.1:20443/post

This may fix at least partially github issue #1801.

This must be backported up to 2.6.
2022-07-29 16:01:21 +02:00
Frédéric Lécaille
a18c3339c8 BUG/MAJOR: mux_quic: fix invalid PROTOCOL_VIOLATION on POST data overlap
Stream data reception is incorrect when dealing with a partially new
offset with some data already consumed out of the RX buffer. In this
case, data length is adjusted but not the data buffer. In most cases,
ncb_add() operation will be rejected as already stored data does not
correspond with the new inserted offset. This will result in an invalid
CONNECTION_CLOSE with PROTOCOL_VIOLATION.

To fix this, buffer pointer is advanced while the length is reduced.

This can be reproduced with a POST request and patching haproxy to call
qcc_recv() multiple times by copying a quic_stream frame with different
offsets.

Must be backported to 2.6.
2022-07-20 15:34:58 +02:00
Amaury Denoyelle
a154dc0290 MINOR: mux-quic: send one last time before release
Call qc_send() on qc_release(). This is mostly useful for application
protocol with a connection closing procedure. Most notably, this will be
useful to implement HTTP/3 GOAWAY emission.
2022-07-15 15:56:13 +02:00
Amaury Denoyelle
c49d5d1a4b CLEANUP: mux-quic: move qc_release()
This change is purely cosmetic. qc_release() function is moved just
before qc_io_cb(). It's cleaner as it brings it closer where it is used.
More importantly, this will be required to be able to use it in
qc_send() function.
2022-07-15 15:56:13 +02:00
Amaury Denoyelle
069288b4c0 MINOR: mux-quic/h3: prepare CONNECTION_CLOSE on release
When MUX is released, a CONNECTION_CLOSE frame should be emitted. This
will ensure that the client does not use anymore a half-dead connection.

App protocol layer is responsible to provide the error code via release
callback. For HTTP/3 NO_ERROR is used as specified in RFC 9114. If no
release callback is provided, generic QUIC NO_ERROR code is used. Note
that a graceful shutdown is used : quic_conn must emit CONNECTION_CLOSE
frame when possible. This will be provided in another patch.

This change should limit the risk of browsers stuck on webpage loading
if MUX has been released. On CONNECTION_CLOSE reception, the client will
reopen a new QUIC connection.
2022-07-15 15:20:33 +02:00