MEDIUM: quic: streamline error notification
When an error is detected at quic-conn layer, the upper MUX must be
notified. Previously, this was done relying on quic_conn flag
QUIC_FL_CONN_NOTIFY_CLOSE set and the MUX wake callback called on
connection closure.
Adjust this mechanism to use an approach more similar to other transport
layers in haproxy. On error, connection flags are updated with
CO_FL_ERROR, CO_FL_SOCK_RD_SH and CO_FL_SOCK_WR_SH. The MUX is then
notified when the error happened instead of just before the closing. To
reflect this change, qc_notify_close() has been renamed qc_notify_err().
This function must now be explicitely called every time a new error
condition arises on the quic_conn layer.
To ensure MUX send is disabled on error, qc_send_mux() now checks
CO_FL_SOCK_WR_SH. If set, the function returns an error. This should
prevent the MUX from sending data on closing or draining state.
To complete this patch, MUX layer must now check for CO_FL_ERROR
explicitely. This will be the subject of the following commit.
This should be backported up to 2.7.
diff --git a/include/haproxy/quic_conn-t.h b/include/haproxy/quic_conn-t.h
index 366ff34..2d45924 100644
--- a/include/haproxy/quic_conn-t.h
+++ b/include/haproxy/quic_conn-t.h
@@ -623,7 +623,7 @@
#define QUIC_FL_CONN_TO_KILL (1U << 24) /* Unusable connection, to be killed */
#define QUIC_FL_CONN_TX_TP_RECEIVED (1U << 25) /* Peer transport parameters have been received (used for the transmitting part) */
#define QUIC_FL_CONN_FINALIZED (1U << 26) /* QUIC connection finalized (functional, ready to send/receive) */
-#define QUIC_FL_CONN_NOTIFY_CLOSE (1U << 27) /* MUX notified about quic-conn imminent closure (idle-timeout or CONNECTION_CLOSE emission/reception) */
+/* gap here */
#define QUIC_FL_CONN_EXP_TIMER (1U << 28) /* timer has expired, quic-conn can be freed */
#define QUIC_FL_CONN_CLOSING (1U << 29) /* closing state, entered on CONNECTION_CLOSE emission */
#define QUIC_FL_CONN_DRAINING (1U << 30) /* draining state, entered on CONNECTION_CLOSE reception */
diff --git a/include/haproxy/quic_conn.h b/include/haproxy/quic_conn.h
index 4640e08..ba2df64 100644
--- a/include/haproxy/quic_conn.h
+++ b/include/haproxy/quic_conn.h
@@ -670,7 +670,7 @@
unsigned char *buf, size_t buf_len);
int qc_send_mux(struct quic_conn *qc, struct list *frms);
-void qc_notify_close(struct quic_conn *qc);
+void qc_notify_err(struct quic_conn *qc);
int qc_notify_send(struct quic_conn *qc);
void qc_release_frm(struct quic_conn *qc, struct quic_frame *frm);
diff --git a/src/mux_quic.c b/src/mux_quic.c
index 2906c7f..45efb79 100644
--- a/src/mux_quic.c
+++ b/src/mux_quic.c
@@ -1659,6 +1659,11 @@
static int qcc_subscribe_send(struct qcc *qcc)
{
struct connection *conn = qcc->conn;
+
+ /* Do not subscribe if lower layer in error. */
+ if (conn->flags & CO_FL_ERROR)
+ return 0;
+
if (qcc->wait_event.events & SUB_RETRY_SEND)
return 1;
@@ -1683,7 +1688,6 @@
if (!qc_send_mux(qcc->conn->handle.qc, frms)) {
TRACE_DEVEL("error on sending", QMUX_EV_QCC_SEND, qcc->conn);
- /* TODO should subscribe only for a transient send error */
qcc_subscribe_send(qcc);
goto err;
}
@@ -2731,11 +2735,6 @@
TRACE_ENTER(QMUX_EV_QCC_WAKE, conn);
- if (conn->handle.qc->flags & QUIC_FL_CONN_NOTIFY_CLOSE)
- qcc->conn->flags |= (CO_FL_SOCK_RD_SH|CO_FL_SOCK_WR_SH);
-
- qc_send(qcc);
-
if (qc_process(qcc)) {
TRACE_STATE("releasing dead connection", QMUX_EV_QCC_WAKE, qcc->conn);
goto release;
diff --git a/src/quic_conn.c b/src/quic_conn.c
index 5c56484..b607362 100644
--- a/src/quic_conn.c
+++ b/src/quic_conn.c
@@ -814,6 +814,9 @@
TRACE_PROTO("killing the connection", QUIC_EV_CONN_KILL, qc);
qc->flags |= QUIC_FL_CONN_TO_KILL;
task_wakeup(qc->idle_timer_task, TASK_WOKEN_OTHER);
+
+ qc_notify_err(qc);
+
TRACE_LEAVE(QUIC_EV_CONN_KILL, qc);
}
@@ -1286,6 +1289,7 @@
qc->flags |= QUIC_FL_CONN_IMMEDIATE_CLOSE;
qc->err.code = err.code;
qc->err.app = err.app;
+
leave:
TRACE_LEAVE(QUIC_EV_CONN_CLOSE, qc);
}
@@ -1972,6 +1976,7 @@
if (++frm->loss_count >= global.tune.quic_max_frame_loss) {
TRACE_ERROR("retransmission limit reached, closing the connection", QUIC_EV_CONN_PRSAFRM, qc);
quic_set_connection_close(qc, quic_err_transport(QC_ERR_INTERNAL_ERROR));
+ qc_notify_err(qc);
close = 1;
}
@@ -2976,6 +2981,7 @@
if (ncb_ret == NCB_RET_DATA_REJ) {
TRACE_ERROR("overlapping data rejected", QUIC_EV_CONN_PRSHPKT, qc);
quic_set_connection_close(qc, quic_err_transport(QC_ERR_PROTOCOL_VIOLATION));
+ qc_notify_err(qc);
}
else if (ncb_ret == NCB_RET_GAP_SIZE) {
TRACE_ERROR("cannot bufferize frame due to gap size limit",
@@ -3074,6 +3080,7 @@
return ret;
protocol_violation:
quic_set_connection_close(qc, quic_err_transport(QC_ERR_PROTOCOL_VIOLATION));
+ qc_notify_err(qc);
goto leave;
}
@@ -3297,7 +3304,7 @@
qc->flags |= QUIC_FL_CONN_DRAINING|QUIC_FL_CONN_IMMEDIATE_CLOSE;
qc_detach_th_ctx_list(qc, 1);
qc_idle_timer_do_rearm(qc, 0);
- qc_notify_close(qc);
+ qc_notify_err(qc);
}
break;
case QUIC_FT_HANDSHAKE_DONE:
@@ -3885,7 +3892,6 @@
(pkt->flags & QUIC_FL_TX_PACKET_CC)) {
qc->flags |= QUIC_FL_CONN_CLOSING;
qc_detach_th_ctx_list(qc, 1);
- qc_notify_close(qc);
/* RFC 9000 10.2. Immediate Close:
* The closing and draining connection states exist to ensure
@@ -4811,6 +4817,12 @@
TRACE_ENTER(QUIC_EV_CONN_TXPKT, qc);
BUG_ON(qc->mux_state != QC_MUX_READY); /* Only MUX can uses this function so it must be ready. */
+ if (qc->conn->flags & CO_FL_SOCK_WR_SH) {
+ qc->conn->flags |= CO_FL_ERROR | CO_FL_SOCK_RD_SH;
+ TRACE_DEVEL("connection on error", QUIC_EV_CONN_TXPKT, qc);
+ return 0;
+ }
+
/* Try to send post handshake frames first unless on 0-RTT. */
if ((qc->flags & QUIC_FL_CONN_NEED_POST_HANDSHAKE_FRMS) &&
qc->state >= QUIC_HS_ST_COMPLETE) {
@@ -5903,7 +5915,7 @@
/* Notify the MUX before settings QUIC_FL_CONN_EXP_TIMER or the MUX
* might free the quic-conn too early via quic_close().
*/
- qc_notify_close(qc);
+ qc_notify_err(qc);
/* If the MUX is still alive, keep the quic-conn. The MUX is
* responsible to call quic_close to release it.
@@ -8420,25 +8432,26 @@
goto leave;
}
-/* Notify the MUX layer if alive about an imminent close of <qc>. */
-void qc_notify_close(struct quic_conn *qc)
+/* Notify upper layer of a fatal error which forces to close the connection. */
+void qc_notify_err(struct quic_conn *qc)
{
TRACE_ENTER(QUIC_EV_CONN_CLOSE, qc);
- if (qc->flags & QUIC_FL_CONN_NOTIFY_CLOSE)
- goto leave;
+ if (qc->mux_state == QC_MUX_READY) {
+ TRACE_STATE("error notified to mux", QUIC_EV_CONN_CLOSE, qc);
- qc->flags |= QUIC_FL_CONN_NOTIFY_CLOSE;
- /* wake up the MUX */
- if (qc->mux_state == QC_MUX_READY && qc->conn->mux->wake) {
- TRACE_STATE("connection closure notidfied to mux",
- QUIC_FL_CONN_NOTIFY_CLOSE, qc);
- qc->conn->mux->wake(qc->conn);
+ /* Mark socket as closed. */
+ qc->conn->flags |= CO_FL_ERROR | CO_FL_SOCK_RD_SH | CO_FL_SOCK_WR_SH;
+
+ /* TODO quic-conn layer must stay active until MUX is released.
+ * Thus, we have to wake up directly to ensure upper stream
+ * layer will be notified of the error. If a proper separation
+ * is made between MUX and quic-conn layer, wake up could be
+ * conducted only with qc.subs.
+ */
+ tasklet_wakeup(qc->qcc->wait_event.tasklet);
}
- else
- TRACE_STATE("connection closure not notidfied to mux",
- QUIC_FL_CONN_NOTIFY_CLOSE, qc);
- leave:
+
TRACE_LEAVE(QUIC_EV_CONN_CLOSE, qc);
}