From: Amaury Denoyelle <adenoyelle@haproxy.com>
Date: Fri, 12 May 2023 14:19:32 +0000 (+0200)
Subject: BUG/MINOR: mux-quic: differentiate failure on qc_stream_desc alloc
X-Git-Tag: v2.8-dev12~38
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=6c501ed23bea953518059117e7dd19e8d6cb6bd8;p=thirdparty%2Fhaproxy.git

BUG/MINOR: mux-quic: differentiate failure on qc_stream_desc alloc

qc_stream_buf_alloc() can fail for two reasons :
* limit of Tx buffer per connection reached
* allocation failure

The first case is properly treated. A flag QC_CF_CONN_FULL is set on the
connection to interrupt emission. It is cleared when a buffer became
available after in order ACK reception and the MUX tasklet is woken up.

The allocation failure was handled with the same mechanism which in this
case is not appropriate and could lead to a connection transfer freeze.
Instead, prefer to close the connection with a QUIC internal error code.

To differentiate the two causes, qc_stream_buf_alloc() API was changed
to return the number of available buffers to the caller.

This must be backported up to 2.6.
---

diff --git a/include/haproxy/quic_stream.h b/include/haproxy/quic_stream.h
index cb30b1ae4a..57f8c455f0 100644
--- a/include/haproxy/quic_stream.h
+++ b/include/haproxy/quic_stream.h
@@ -16,7 +16,7 @@ void qc_stream_desc_free(struct qc_stream_desc *stream, int closing);
 
 struct buffer *qc_stream_buf_get(struct qc_stream_desc *stream);
 struct buffer *qc_stream_buf_alloc(struct qc_stream_desc *stream,
-                                   uint64_t offset);
+                                   uint64_t offset, int *avail);
 void qc_stream_buf_release(struct qc_stream_desc *stream);
 
 #endif /* USE_QUIC */
diff --git a/src/mux_quic.c b/src/mux_quic.c
index e74bae0097..15fd5702bf 100644
--- a/src/mux_quic.c
+++ b/src/mux_quic.c
@@ -1827,7 +1827,7 @@ static int _qc_send_qcs(struct qcs *qcs, struct list *frms)
 	struct qcc *qcc = qcs->qcc;
 	struct buffer *buf = &qcs->tx.buf;
 	struct buffer *out = qc_stream_buf_get(qcs->stream);
-	int xfer = 0;
+	int xfer = 0, buf_avail;
 	char fin = 0;
 
 	TRACE_ENTER(QMUX_EV_QCS_SEND, qcc->conn, qcs);
@@ -1841,8 +1841,14 @@ static int _qc_send_qcs(struct qcs *qcs, struct list *frms)
 			if (qcc->flags & QC_CF_CONN_FULL)
 				goto out;
 
-			out = qc_stream_buf_alloc(qcs->stream, qcs->tx.offset);
+			out = qc_stream_buf_alloc(qcs->stream, qcs->tx.offset,
+			                          &buf_avail);
 			if (!out) {
+				if (!buf_avail) {
+					TRACE_ERROR("stream desc alloc failure", QMUX_EV_QCS_SEND, qcc->conn, qcs);
+					goto err;
+				}
+
 				TRACE_STATE("cannot allocate stream desc buffer", QMUX_EV_QCS_SEND, qcc->conn, qcs);
 				qcc->flags |= QC_CF_CONN_FULL;
 				goto out;
diff --git a/src/quic_stream.c b/src/quic_stream.c
index ef9ebcd68a..da86e04ff9 100644
--- a/src/quic_stream.c
+++ b/src/quic_stream.c
@@ -215,12 +215,11 @@ struct buffer *qc_stream_buf_get(struct qc_stream_desc *stream)
 	return &stream->buf->buf;
 }
 
-/* Check if a new stream buffer can be allocated for the connection <qc>.
- * Returns a boolean.
- */
+/* Returns the count of available buffer left for <qc>. */
 static int qc_stream_buf_avail(struct quic_conn *qc)
 {
-	return qc->stream_buf_count < global.tune.quic_streams_buf;
+	BUG_ON(qc->stream_buf_count > global.tune.quic_streams_buf);
+	return global.tune.quic_streams_buf - qc->stream_buf_count;
 }
 
 /* Allocate a new current buffer for <stream>. The buffer limit count for the
@@ -228,17 +227,19 @@ static int qc_stream_buf_avail(struct quic_conn *qc)
  * is not NULL prior to this call. The new buffer represents stream payload at
  * offset <offset>.
  *
- * Returns the buffer or NULL.
+ * Returns the buffer or NULL on error. Caller may check <avail> to ensure if
+ * the connection buffer limit was reached or a fatal error was encountered.
  */
 struct buffer *qc_stream_buf_alloc(struct qc_stream_desc *stream,
-                                   uint64_t offset)
+                                   uint64_t offset, int *avail)
 {
 	struct quic_conn *qc = stream->qc;
 
 	/* current buffer must be released first before allocate a new one. */
 	BUG_ON(stream->buf);
 
-	if (!qc_stream_buf_avail(qc))
+	*avail = qc_stream_buf_avail(qc);
+	if (!*avail)
 		return NULL;
 
 	stream->buf_offset = offset;