mirrors/haproxy
1
0
Fork 0
mirror of https://git.haproxy.org/git/haproxy.git/ synced 2026-05-06 21:46:10 +02:00
Code Issues Projects Releases Wiki Activity

BUG/MEDIUM: mux_quic: adjust qcc_is_dead() to account detached streams

Browse Source 
Muxes are responsible to release connections once they are inactive and
won't be reusable. In QUIC mux, such connections are detected via
qcc_is_dead(). The first precondition is that there is no more upper
streams attached. This was accounted via QCC <nb_sc> counter.

A special characteristic of QCS instances is that they can be in
detached state : upper stream has been removed but there is still data
to emit. Such QCS were not taken into account in qcc_is_dead(), so a
connection could be freed with some remaining data not yet emitted.

It is also not possible for QUIC MUX to simply look at the QCS tree to
determine if the connection is inactive. Indeed, some streams are opened
for protocol internal usage. This is the case for example with HTTP/3
unidirectional control stream or QPACK encoder/decoder streams. These
streams are never closed. In the end, only requests streams should be
taken into account for the connection activity.

This patch improves the situation by reworking <nb_hreq> QCC counter.
Previously, it served for http-request timeout implementation. However,
this timeout only relies on <opening_list> now. Thus, <nb_hreq> scope is
changed : it is now incremented via qcs_wait_http_req(), used by app
protocol layer once a request stream is identified. Decrement is
performed on qcs_free(), so this guarantees that a connection cannot be
freed anymore if request streams still exists, unless if inactivity
timeout fires. As such, <nb_hreq> now supersedes <nb_sc> entirely, so
the qcc_is_dead() can now relies on the former.

Along with this change, qcc_timeout_task() must be updated. Call to
qcc_is_dead() was unnecessary prior to this patch as timeout handling
was only active when no upper streams were attached. When tested, both
<nb_sc> and QCC <task> were already null, so a connection was always
released on timeout, as expected. With qcc_is_dead() now checking
<nb_hreq> instead, this is not always the case anymore. In fact, this
check is unnecessary as inactivity timeout serves precisely to free a
stucked connection with remaining data to emit.

This patch also has some impact on http-keep-alive timeout. Previously,
this timeout could be armed if only detached streams remained. Now, it
is only applicable if all QCS request instances are closed and freed.
Thus, qcc_reset_idle_start() is now closed directly on qcs_free().

Ideally this should be backported up to 2.6, or at least 2.8 as QUIC
experimental status was removed there.
This commit is contained in:
Amaury Denoyelle 2026-04-22 10:06:31 +02:00
parent 81eda41d5c
commit 3cfb08c07b
2 changed files with 33 additions and 41 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
include/haproxy/mux_quic.h
View File

@ -98,9 +98,10 @@ static inline char *qcs_st_to_str(enum qcs_state st)
int qcc_install_app_ops(struct qcc *qcc);
/* Register <qcs> stream for http-request timeout. If the stream is not yet
* attached in the configured delay, qcc timeout task will be triggered. This
* means the full header section was not received in time.
/* Flags <qcs> as a request stream. The connection will be considered as active
* until all request streams are closed or on inactivity timeout. On the
* frontend side, http-request timeout will be applied on the stream to ensure
* headers are received in time.
*
* This function should be called by the application protocol layer on request
* streams initialization.
@ -109,6 +110,9 @@ static inline void qcs_wait_http_req(struct qcs *qcs)
{
struct qcc *qcc = qcs->qcc;
/* For frontend connections, register the stream in QCC opening_list.
* This is necessary for http-request timeout.
*/
if (!conn_is_back(qcc->conn)) {
/* A stream cannot be registered several times. */
BUG_ON_HOT(tick_isset(qcs->start));
@ -119,6 +123,11 @@ static inline void qcs_wait_http_req(struct qcs *qcs)
*/
LIST_APPEND(&qcc->opening_list, &qcs->el_opening);
}
/* Ensure flag is only set once per stream to avoid nb_hreq counter wrapping. */
BUG_ON_HOT(qcs->flags & QC_SF_HREQ_RECV);
qcs->flags |= QC_SF_HREQ_RECV;
++qcc->nb_hreq;
}
void qcc_show_quic(struct qcc *qcc);

59
src/mux_quic.c
View File

@ -102,6 +102,11 @@ static void qcs_free(struct qcs *qcs)
sedesc_free(qcs->sd);
qcs->sd = NULL;
if (qcs->flags & QC_SF_HREQ_RECV) {
BUG_ON(!qcc->nb_hreq);
--qcc->nb_hreq;
}
/* Release app-layer context. */
if (qcs->ctx && qcc->app_ops->detach)
qcc->app_ops->detach(qcs);
@ -270,44 +275,25 @@ static forceinline void qcc_rm_sc(struct qcc *qcc)
{
BUG_ON(!qcc->nb_sc); /* Ensure sc count is always valid (ie >=0). */
--qcc->nb_sc;
/* Reset qcc idle start for http-keep-alive timeout. Timeout will be
* refreshed after this on stream detach.
*/
if (!conn_is_back(qcc->conn) && qcc_may_expire(qcc) && !qcc->nb_hreq)
qcc_reset_idle_start(qcc);
}
/* Decrement <qcc> hreq. */
static forceinline void qcc_rm_hreq(struct qcc *qcc)
{
BUG_ON(!qcc->nb_hreq); /* Ensure http req count is always valid (ie >=0). */
--qcc->nb_hreq;
/* Reset qcc idle start for http-keep-alive timeout. Timeout will be
* refreshed after this on I/O handler.
*/
if (!conn_is_back(qcc->conn) && qcc_may_expire(qcc) && !qcc->nb_hreq)
qcc_reset_idle_start(qcc);
}
static inline int qcc_is_dead(const struct qcc *qcc)
{
/* Maintain connection if stream endpoints are still active. */
if (qcc->nb_sc)
/* Maintain connection if there is still request streams active. */
if (qcc->nb_hreq)
return 0;
/* Connection considered dead if either :
* - remote error detected at transport level
* - error detected locally
* - MUX timeout expired
* - app layer shut and all transfers done (FE side only - used for stream.max-total)
* - app layer shut (FE side only - used for stream.max-total)
* - new stream initiating definitely blocked (BE side only - used for H3 GOAWAY reception)
*/
if (qcc->flags & (QC_CF_ERR_CONN|QC_CF_ERRL_DONE) ||
!qcc->task ||
(!conn_is_back(qcc->conn) && !qcc->nb_hreq && qcc->app_st == QCC_APP_ST_SHUT) ||
(conn_is_back(qcc->conn) && !qcc->nb_hreq && (qcc->flags & QC_CF_CONN_SHUT))) {
(!conn_is_back(qcc->conn) && qcc->app_st == QCC_APP_ST_SHUT) ||
(conn_is_back(qcc->conn) && (qcc->flags & QC_CF_CONN_SHUT))) {
return 1;
}
@ -450,9 +436,6 @@ void qcs_close_local(struct qcs *qcs)
if (quic_stream_is_bidi(qcs->id)) {
qcs->st = (qcs->st == QC_SS_HREM) ? QC_SS_CLO : QC_SS_HLOC;
if (qcs->flags & QC_SF_HREQ_RECV)
qcc_rm_hreq(qcs->qcc);
}
else {
/* Only local uni streams are valid for this operation. */
@ -1044,13 +1027,9 @@ int qcs_attach_sc(struct qcs *qcs, struct buffer *buf, char fin)
return -1;
}
/* QC_SF_HREQ_RECV must be set once for a stream. Else, nb_hreq counter
* will be incorrect for the connection.
*/
BUG_ON_HOT(qcs->flags & QC_SF_HREQ_RECV);
qcs->flags |= QC_SF_HREQ_RECV;
/* QCS must be identified as request stream prior to stconn instantiation. */
BUG_ON(!(qcs->flags & QC_SF_HREQ_RECV));
++qcc->nb_sc;
++qcc->nb_hreq;
++qcc->tot_sc;
/* TODO duplicated from mux_h2 */
@ -2535,6 +2514,12 @@ static void qcs_destroy(struct qcs *qcs)
qcs_free(qcs);
/* Rearm http-keep-alive timeout when last request stream is freed. */
if (!conn_is_back(qcc->conn) && qcc_may_expire(qcc) && !qcc->nb_hreq) {
qcc_reset_idle_start(qcc);
qcc_refresh_timeout(qcc);
}
TRACE_LEAVE(QMUX_EV_QCS_END, conn);
}
@ -3784,11 +3769,9 @@ static struct task *qcc_timeout_task(struct task *t, void *ctx, unsigned int sta
* shutdown should occurs. For all other cases, an immediate close
* seems legitimate.
*/
if (qcc_is_dead(qcc)) {
TRACE_STATE("releasing dead connection", QMUX_EV_QCC_WAKE, qcc->conn);
qcc_app_shutdown(qcc);
qcc_release(qcc);
}
TRACE_STATE("releasing dead connection", QMUX_EV_QCC_WAKE, qcc->conn);
qcc_app_shutdown(qcc);
qcc_release(qcc);
out:
TRACE_LEAVE(QMUX_EV_QCC_WAKE);