From: Amaury Denoyelle Date: Mon, 30 Sep 2024 12:39:15 +0000 (+0200) Subject: MEDIUM: quic: strengthen MUX send notification X-Git-Tag: v3.1-dev9~16 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=d7f4e5abf0b7129329d0ea716c104474fd934bc6;p=thirdparty%2Fhaproxy.git MEDIUM: quic: strengthen MUX send notification Previous commit implement a refactor of MUX send notification from quic_conn layer. With this new architecture, a proper callback is defined for each qc_stream_desc instance. This architecture change allows to simplify notification from quic_conn layer. First, ensure the MUX callback to properly ignore retransmission of an already emitted frame. Luckily, this can be handled easily by comparing offsets and FIN status. Also, each QCS instance can now be unregistered from send notification just prior qc_stream_desc releasing. This ensures a QCS is never manipulated from quic_conn after its emission ending. Both these changes render the send notification more robust. As a nice effect, flag QUIC_FL_CONN_TX_MUX_CONTEXT can be removed as it is now unneeded. --- diff --git a/include/haproxy/quic_conn-t.h b/include/haproxy/quic_conn-t.h index a85606ffce..6b5c6361b0 100644 --- a/include/haproxy/quic_conn-t.h +++ b/include/haproxy/quic_conn-t.h @@ -432,7 +432,7 @@ struct quic_conn_closed { #define QUIC_FL_CONN_NEED_POST_HANDSHAKE_FRMS (1U << 2) /* HANDSHAKE_DONE must be sent */ #define QUIC_FL_CONN_LISTENER (1U << 3) #define QUIC_FL_CONN_ACCEPT_REGISTERED (1U << 4) -#define QUIC_FL_CONN_TX_MUX_CONTEXT (1U << 5) /* sending in progress from the MUX layer */ +/* gap here */ #define QUIC_FL_CONN_IDLE_TIMER_RESTARTED_AFTER_READ (1U << 6) #define QUIC_FL_CONN_RETRANS_NEEDED (1U << 7) #define QUIC_FL_CONN_RETRANS_OLD_DATA (1U << 8) /* retransmission in progress for probing with already sent data */ @@ -472,7 +472,6 @@ static forceinline char *qc_show_flags(char *buf, size_t len, const char *delim, _(QUIC_FL_CONN_NEED_POST_HANDSHAKE_FRMS, _(QUIC_FL_CONN_LISTENER, _(QUIC_FL_CONN_ACCEPT_REGISTERED, - _(QUIC_FL_CONN_TX_MUX_CONTEXT, _(QUIC_FL_CONN_IDLE_TIMER_RESTARTED_AFTER_READ, _(QUIC_FL_CONN_RETRANS_NEEDED, _(QUIC_FL_CONN_RETRANS_OLD_DATA, @@ -491,7 +490,7 @@ static forceinline char *qc_show_flags(char *buf, size_t len, const char *delim, _(QUIC_FL_CONN_EXP_TIMER, _(QUIC_FL_CONN_CLOSING, _(QUIC_FL_CONN_DRAINING, - _(QUIC_FL_CONN_IMMEDIATE_CLOSE))))))))))))))))))))))))); + _(QUIC_FL_CONN_IMMEDIATE_CLOSE)))))))))))))))))))))))); /* epilogue */ _(~0U); return buf; diff --git a/src/mux_quic.c b/src/mux_quic.c index dc80c88fd6..a7299fbc73 100644 --- a/src/mux_quic.c +++ b/src/mux_quic.c @@ -80,7 +80,10 @@ static void qcs_free(struct qcs *qcs) qcc->app_ops->detach(qcs); /* Release qc_stream_desc buffer from quic-conn layer. */ - qc_stream_desc_release(qcs->stream, qcs->tx.fc.off_real); + if (qcs->stream) { + qc_stream_desc_sub_send(qcs->stream, NULL); + qc_stream_desc_release(qcs->stream, qcs->tx.fc.off_real); + } /* Free Rx buffer. */ qcs_free_ncbuf(qcs, &qcs->rx.ncbuf); @@ -561,7 +564,8 @@ static void qmux_ctrl_send(struct qc_stream_desc *stream, uint64_t data, uint64_ /* check if the STREAM frame has already been notified. It can happen * for retransmission. */ - if (offset + data < qcs->tx.fc.off_real) { + if (offset + data < qcs->tx.fc.off_real || + (!data && (!(qcs->flags & QC_SF_FIN_STREAM) || qc_stream_buf_get(qcs->stream) || qcs_prep_bytes(qcs)))) { TRACE_DEVEL("offset already notified", QMUX_EV_QCS_SEND, qcc->conn, qcs); goto out; } diff --git a/src/quic_stream.c b/src/quic_stream.c index f6adc2e439..80d56ba383 100644 --- a/src/quic_stream.c +++ b/src/quic_stream.c @@ -97,8 +97,7 @@ struct qc_stream_desc *qc_stream_desc_new(uint64_t id, enum qcs_type type, void } /* Mark the stream descriptor as released. It will be freed as soon as - * all its buffered data are acknowledged. Does nothing if is already - * NULL. + * all its buffered data are acknowledged. * * corresponds to the last offset sent for this stream. If there * is unsent data present, they will be remove first to guarantee that buffer @@ -107,9 +106,6 @@ struct qc_stream_desc *qc_stream_desc_new(uint64_t id, enum qcs_type type, void void qc_stream_desc_release(struct qc_stream_desc *stream, uint64_t final_size) { - if (!stream) - return; - /* A stream can be released only one time. */ BUG_ON(stream->flags & QC_SD_FL_RELEASE); diff --git a/src/quic_tx.c b/src/quic_tx.c index 1420e103b0..e5af014ed1 100644 --- a/src/quic_tx.c +++ b/src/quic_tx.c @@ -491,10 +491,8 @@ int qc_send_mux(struct quic_conn *qc, struct list *frms) } TRACE_STATE("preparing data (from MUX)", QUIC_EV_CONN_TXPKT, qc); - qc->flags |= QUIC_FL_CONN_TX_MUX_CONTEXT; qel_register_send(&send_list, qc->ael, frms); ret = qc_send(qc, 0, &send_list); - qc->flags &= ~QUIC_FL_CONN_TX_MUX_CONTEXT; TRACE_LEAVE(QUIC_EV_CONN_TXPKT, qc); return ret; @@ -1663,12 +1661,9 @@ static int qc_build_frms(struct list *outlist, struct list *inlist, LIST_DEL_INIT(&cf->list); LIST_APPEND(outlist, &cf->list); - /* Do not notify MUX on retransmission. */ - if (qc->flags & QUIC_FL_CONN_TX_MUX_CONTEXT) { - qc_stream_desc_send(cf->stream.stream, - cf->stream.offset.key, - cf->stream.len); - } + qc_stream_desc_send(cf->stream.stream, + cf->stream.offset.key, + cf->stream.len); } else { struct quic_frame *new_cf; @@ -1710,12 +1705,9 @@ static int qc_build_frms(struct list *outlist, struct list *inlist, cf->stream.offset.key += dlen; cf->stream.data = (unsigned char *)b_peek(&cf_buf, dlen); - /* Do not notify MUX on retransmission. */ - if (qc->flags & QUIC_FL_CONN_TX_MUX_CONTEXT) { - qc_stream_desc_send(new_cf->stream.stream, - new_cf->stream.offset.key, - new_cf->stream.len); - } + qc_stream_desc_send(new_cf->stream.stream, + new_cf->stream.offset.key, + new_cf->stream.len); } /* TODO the MUX is notified about the frame sending via