From a7645d7cd5e99e09e0626c5e5bbe8ad455454419 Mon Sep 17 00:00:00 2001 From: Amaury Denoyelle Date: Tue, 18 Feb 2025 16:14:19 +0100 Subject: [PATCH] MINOR: mux-quic/h3: support temporary blocking on control stream sending When HTTP/3 layer is initialized via QUIC MUX, it first emits a SETTINGS frame on an unidirectional control stream. However, this could be prevented if client did not provide initial flow control. Previously, QUIC MUX was unable to deal with such situation. Thus, the connection was closed immediately and no transfer could occur. Improve this by extending QUIC MUX application layer API : initialization may now return a transient error. This allows MUX to continue to use the connection normally. Initialization will be retried periodically alter until it can succeed. This new API allows to deal with the flow control issue described above. Note that this patch is not considered as a bug fix. Indeed, clients are strongly advised to provide enough flow control for a SETTINGS frame exchange. --- src/h3.c | 38 +++++++++++++++++++++++--------------- src/mux_quic.c | 21 +++++++++++++++++---- 2 files changed, 40 insertions(+), 19 deletions(-) diff --git a/src/h3.c b/src/h3.c index 04aeada53..cb60e632e 100644 --- a/src/h3.c +++ b/src/h3.c @@ -1531,11 +1531,6 @@ static int h3_control_send(struct qcs *qcs, void *ctx) b_quic_enc_int(&pos, h3_settings_max_field_section_size, 0); } - if (qfctl_sblocked(&qcs->tx.fc) || qfctl_sblocked(&qcs->qcc->tx.fc)) { - TRACE_ERROR("not enough initial credit for control stream", H3_EV_TX_FRAME|H3_EV_TX_SETTINGS, qcs->qcc->conn, qcs); - goto err; - } - if (!(res = qcc_get_stream_txbuf(qcs, &err, 0))) { /* Only memory failure can cause buf alloc error for control stream due to qcs_send_metadata() usage. */ TRACE_ERROR("cannot allocate Tx buffer", H3_EV_TX_FRAME|H3_EV_TX_SETTINGS, qcs->qcc->conn, qcs); @@ -2409,26 +2404,35 @@ static int h3_init(struct qcc *qcc) /* Open control stream for HTTP/3 connection and schedule a SETTINGS * frame emission on it. * - * Returns 0 on success else non-zero. + * Returns 0 on success. If a transient error was encountered, a positive value + * is returned, finalize operation should be recalled later. A negative value is + * used for a fatal error, in this case the connection should be closed. */ static int h3_finalize(void *ctx) { struct h3c *h3c = ctx; struct qcc *qcc = h3c->qcc; - struct qcs *qcs; + struct qcs *qcs = h3c->ctrl_strm; TRACE_ENTER(H3_EV_H3C_NEW, qcc->conn); - qcs = qcc_init_stream_local(qcc, 0); if (!qcs) { - /* Error must be set by qcc_init_stream_local(). */ - BUG_ON(!(qcc->flags & QC_CF_ERRL)); - TRACE_ERROR("cannot init control stream", H3_EV_H3C_NEW, qcc->conn); - goto err; + qcs = qcc_init_stream_local(qcc, 0); + if (!qcs) { + /* Error must be set by qcc_init_stream_local(). */ + BUG_ON(!(qcc->flags & QC_CF_ERRL)); + TRACE_ERROR("cannot init control stream", H3_EV_H3C_NEW, qcc->conn); + goto err; + } + + qcs_send_metadata(qcs); + h3c->ctrl_strm = qcs; } - qcs_send_metadata(qcs); - h3c->ctrl_strm = qcs; + if (qfctl_sblocked(&qcs->tx.fc) || qfctl_sblocked(&qcs->qcc->tx.fc)) { + TRACE_STATE("not enough initial credit for control stream", H3_EV_TX_FRAME|H3_EV_TX_SETTINGS, qcs->qcc->conn, qcs); + goto again; + } /* RFC 9114 7.2.4.2. Initialization * @@ -2445,9 +2449,13 @@ static int h3_finalize(void *ctx) TRACE_LEAVE(H3_EV_H3C_NEW, qcc->conn); return 0; + again: + TRACE_DEVEL("leaving on transient state", H3_EV_H3C_NEW, qcc->conn); + return 1; + err: TRACE_DEVEL("leaving on error", H3_EV_H3C_NEW, qcc->conn); - return 1; + return -1; } /* Send a HTTP/3 GOAWAY followed by a CONNECTION_CLOSE_APP. */ diff --git a/src/mux_quic.c b/src/mux_quic.c index e6938bdff..1d1764e41 100644 --- a/src/mux_quic.c +++ b/src/mux_quic.c @@ -2465,21 +2465,34 @@ static void qcc_wakeup_pacing(struct qcc *qcc) ++qcc->tx.paced_sent_ctr; } -/* Finalize app layer initialization with I/O operations. +/* Conduct I/O operations to finalize app layer initialization. Note that + * app state may remain NULL even on success, if only a transient + * blocking was encountered. Finalize operation can be retry later. * * Returns 0 on success else non-zero. */ static int qcc_app_init(struct qcc *qcc) { + int ret; + TRACE_ENTER(QMUX_EV_QCC_SEND, qcc->conn); - if (qcc->app_ops->finalize && qcc->app_ops->finalize(qcc->ctx)) { - TRACE_ERROR("app ops finalize error", QMUX_EV_QCC_NEW, qcc->conn); - goto err; + if (qcc->app_ops->finalize) { + ret = qcc->app_ops->finalize(qcc->ctx); + if (ret < 0) { + TRACE_ERROR("app ops finalize error", QMUX_EV_QCC_NEW, qcc->conn); + goto err; + } + + if (ret) { + TRACE_STATE("cannot finalize app ops yet", QMUX_EV_QCC_NEW, qcc->conn); + goto again; + } } qcc->app_st = QCC_APP_ST_INIT; + again: TRACE_LEAVE(QMUX_EV_QCC_SEND, qcc->conn); return 0;