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 a85606ffce4255df00b8f7228f2f06fb80de8c4e..6b5c6361b07ca340bd03abd7c6312d46e21d9943 100644 (file)
--- 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 dc80c88fd6824805da9d7a36c1ffaea62c5e4098..a7299fbc733dc9d8311c13178c8d77624d70d0ce 100644 (file)
--- 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 f6adc2e439149a196a56a8a57f175b70df951c2c..80d56ba38328a7bfd91c7e2c60f83237aa0e6812 100644 (file)
--- 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 <stream> as released. It will be freed as soon as
- * all its buffered data are acknowledged. Does nothing if <stream> is already
- * NULL.
+ * all its buffered data are acknowledged.
  *
  * <final_size> 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 1420e103b039f2527401fa444bafa4d5fc26362b..e5af014ed16448d680ee1c35af357db8bd6aaeff 100644 (file)
--- 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