BUG/MAJOR: Revert "MEDIUM: mux-quic: add BUG_ON if sending on locally closed QCS"

This reverts commit 235e8f1afd7e9753a26051b30c47ecc398ccfd12.

Prior to the above commit, snd_buf callback for QUIC MUX was able to
deal with data even after stream closure. The excess was simply
discarded, as no STREAM frame can be emitted after FIN/RESET_STREAM.
This code was later removed and replaced by a BUG_ON() to ensure snd_buf
is never called after stream closure.

However, this approach is too strict. Indeed, there is nothing in the
haproxy stream architecture which forbids this scheduling, in part
because QUIC MUX is the sole responsible of the stream closure. As such,
it is preferable to revert to the old code to prevent any triggering of
a BUG_ON() failure.

Note that nego_ff does not implement data draining if called after
stream closure. This will be done in a future patch.

Thanks to Mike Walker for his investigation on the subject.

This must be backported up to 2.8.

diff --git a/include/haproxy/qmux_http.h b/include/haproxy/qmux_http.h
index 218bd012cf31cccac6d512f34acade403eb09412..e016be354d21efdd2cb3c822a1596b68d2c73641 100644 (file)
--- a/include/haproxy/qmux_http.h
+++ b/include/haproxy/qmux_http.h
@@ -13,6 +13,7 @@ int qcs_http_handle_standalone_fin(struct qcs *qcs);
 
 size_t qcs_http_snd_buf(struct qcs *qcs, struct buffer *buf, size_t count,
                         char *fin);
+size_t qcs_http_reset_buf(struct qcs *qcs, struct buffer *buf, size_t count);
 
 #endif /* USE_QUIC */
 

diff --git a/src/mux_quic.c b/src/mux_quic.c
index 2db2e3320eaf4149f74886adc2c3c5ed40230ff7..7816b63ab406ea13361bb81faf8cd45359ffed3d 100644 (file)
--- a/src/mux_quic.c
+++ b/src/mux_quic.c
@@ -4077,9 +4077,6 @@ static size_t qmux_strm_snd_buf(struct stconn *sc, struct buffer *buf,
 
        TRACE_ENTER(QMUX_EV_STRM_SEND, qcs->qcc->conn, qcs);
 
-       /* Sending forbidden if QCS is locally closed (FIN or RESET_STREAM sent). */
-       BUG_ON(qcs_is_close_local(qcs) || (qcs->flags & QC_SF_TO_RESET));
-
        /* stream layer has been detached so no transfer must occur after. */
        BUG_ON_HOT(qcs->flags & QC_SF_DETACH);
 
@@ -4090,6 +4087,12 @@ static size_t qmux_strm_snd_buf(struct stconn *sc, struct buffer *buf,
                goto end;
        }
 
+       /* Cannot emit data after FIN/RESET_STREAM, drain extra payload. */
+       if (qcs_is_close_local(qcs) || (qcs->flags & QC_SF_TO_RESET)) {
+               ret = qcs_http_reset_buf(qcs, buf, count);
+               goto end;
+       }
+
        if (LIST_INLIST(&qcs->el_buf)) {
                TRACE_DEVEL("leaving on no buf avail", QMUX_EV_STRM_SEND, qcs->qcc->conn, qcs);
                goto end;
@@ -4145,9 +4148,6 @@ static size_t qmux_strm_nego_ff(struct stconn *sc, struct buffer *input,
 
        TRACE_ENTER(QMUX_EV_STRM_SEND, qcs->qcc->conn, qcs);
 
-       /* Sending forbidden if QCS is locally closed (FIN or RESET_STREAM sent). */
-       BUG_ON(qcs_is_close_local(qcs) || (qcs->flags & QC_SF_TO_RESET));
-
        /* stream layer has been detached so no transfer must occur after. */
        BUG_ON_HOT(qcs->flags & QC_SF_DETACH);
 

diff --git a/src/qmux_http.c b/src/qmux_http.c
index b5494ad2970dfd2a553a48e05cd2acc24eeba55a..d67ed4b9337cb26dfd871bb2c15cbc8c10485cbc 100644 (file)
--- a/src/qmux_http.c
+++ b/src/qmux_http.c
@@ -102,3 +102,23 @@ size_t qcs_http_snd_buf(struct qcs *qcs, struct buffer *buf, size_t count,
 
        return ret;
 }
+
+/* QUIC MUX snd_buf reset. HTX data stored in <buf> of length <count> will be
+ * cleared. This can be used when data should not be transmitted any longer.
+ *
+ * Return the size in bytes of cleared data.
+ */
+size_t qcs_http_reset_buf(struct qcs *qcs, struct buffer *buf, size_t count)
+{
+       struct htx *htx;
+
+       TRACE_ENTER(QMUX_EV_STRM_SEND, qcs->qcc->conn, qcs);
+
+       htx = htx_from_buf(buf);
+       htx_reset(htx);
+       htx_to_buf(htx, buf);
+
+       TRACE_LEAVE(QMUX_EV_STRM_SEND, qcs->qcc->conn, qcs);
+
+       return count;
+}