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 366ff344b9354417226e153fc9f353187058c85b..2d459241c33e9bd3dcb9d2083d6d349413245d7c 100644 (file)
--- a/include/haproxy/quic_conn-t.h
+++ b/include/haproxy/quic_conn-t.h
@@ -623,7 +623,7 @@ enum qc_mux_state {
 #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 4640e08d05e328804bb6f9d925110abb1f17816f..ba2df6434bf2772228a1b3e0a8f2724d097ede83 100644 (file)
--- a/include/haproxy/quic_conn.h
+++ b/include/haproxy/quic_conn.h
@@ -670,7 +670,7 @@ int quic_get_cid_tid(const unsigned char *cid, size_t cid_len,
                      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 2906c7f42de8eb976a293653a1fba1787fda4fe5..45efb7953974122779b804dd4e70f1194b8e528b 100644 (file)
--- a/src/mux_quic.c
+++ b/src/mux_quic.c
@@ -1659,6 +1659,11 @@ void qcc_streams_sent_done(struct qcs *qcs, uint64_t data, uint64_t offset)
 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 @@ static int qc_send_frames(struct qcc *qcc, struct list *frms)
 
        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 @@ static int qc_wake(struct connection *conn)
 
        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 5c564843bfe0dc8641a780e7c73727866b4e8c2a..b60736210e8068b27e5e6a65046a8718febd6f15 100644 (file)
--- a/src/quic_conn.c
+++ b/src/quic_conn.c
@@ -814,6 +814,9 @@ void qc_kill_conn(struct quic_conn *qc)
        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 @@ void quic_set_connection_close(struct quic_conn *qc, const struct quic_err err)
        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 @@ static inline int qc_requeue_nacked_pkt_tx_frms(struct quic_conn *qc,
                        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 @@ static int qc_handle_crypto_frm(struct quic_conn *qc,
                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 @@ static int qc_handle_retire_connection_id_frm(struct quic_conn *qc,
        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 @@ static int qc_parse_pkt_frms(struct quic_conn *qc, struct quic_rx_packet *pkt,
                                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 @@ int qc_send_ppkts(struct buffer *buf, struct ssl_sock_ctx *ctx)
                            (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 @@ int qc_send_mux(struct quic_conn *qc, struct list *frms)
        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 @@ struct task *qc_idle_timer_task(struct task *t, void *ctx, unsigned int state)
        /* 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 @@ int quic_get_dgram_dcid(unsigned char *pos, const unsigned char *end,
        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);
+
+               /* Mark socket as closed. */
+               qc->conn->flags |= CO_FL_ERROR | CO_FL_SOCK_RD_SH | CO_FL_SOCK_WR_SH;
 
-       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);
+               /* 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);
 }