From: Amaury Denoyelle <adenoyelle@haproxy.com>
Date: Thu, 12 Dec 2024 11:03:37 +0000 (+0100)
Subject: MEDIUM/OPTIM: mux-quic: do not rebuild frms list on every send
X-Git-Tag: v3.2-dev2~40
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=14710b5e6bf76834343d58db22e00b72590b16fe;p=thirdparty%2Fhaproxy.git

MEDIUM/OPTIM: mux-quic: do not rebuild frms list on every send

A newly introduced frames list member has been defined into QCC instance
with pacing implementation. This allowed to preserve STREAM frames built
between different emission scheduled by pacing, without having to
regenerate it if no new QCS data is available.

Generalize this principle outside of pacing scheduling. Now, the frames
list will be reused accross several qcc_io_send() usage. Frames list is
only cleared when necessary. This will force its refreshing in the next
qcc_io_send() via qcc_build_frms_list().

Frames list refreshing is performed in the following cases :
* on successful transfer from stream snd_buf / done_ff / shut
* on stream reset or read abort
* on max_data/max_stream_data reception with window increase

Note that the two first cases are in fact covered directly due to
qcc_send_stream() usage when QCS is (re)inserted into the send_list.

The main objective of this patch will be to remove QUIC MUX pacing
specific code path. It could also provide better performance as emission
of large frames may often be rescheduled due to transport layer, either
on congestion or full socket buffer. When QUIC MUX is rescheduled, no
new data is available and frames list can be reuse as-is, avoiding an
unecessary loop over send_list.

This should be backported up to 3.1.
---

diff --git a/src/mux_quic.c b/src/mux_quic.c
index 26e21349a8..56674a78a8 100644
--- a/src/mux_quic.c
+++ b/src/mux_quic.c
@@ -1351,6 +1351,7 @@ static void qcc_notify_fctl(struct qcc *qcc)
 /* Free <qcc> STREAM frames in Tx list. */
 static void qcc_clear_frms(struct qcc *qcc)
 {
+	TRACE_STATE("resetting STREAM frames list", QMUX_EV_QCC_SEND, qcc->conn);
 	while (!LIST_ISEMPTY(&qcc->tx.frms)) {
 		struct quic_frame *frm = LIST_ELEM(qcc->tx.frms.n, struct quic_frame *, list);
 		qc_frm_free(qcc->conn->handle.qc, &frm);
@@ -1413,6 +1414,8 @@ void qcc_send_stream(struct qcs *qcs, int urg, int count)
 	/* Cannot send if already closed. */
 	BUG_ON(qcs_is_close_local(qcs));
 
+	qcc_clear_frms(qcc);
+
 	if (urg) {
 		/* qcc_emit_rs_ss() relies on resetted/aborted streams in send_list front. */
 		BUG_ON(!(qcs->flags & (QC_SF_TO_RESET|QC_SF_TO_STOP_SENDING|QC_SF_TXBUB_OOB)));
@@ -1664,6 +1667,9 @@ int qcc_recv_max_data(struct qcc *qcc, uint64_t max)
 
 		if (unblock_soft)
 			qcc_notify_fctl(qcc);
+
+		if (unblock_soft || unblock_real)
+			qcc_clear_frms(qcc);
 	}
 
 	TRACE_LEAVE(QMUX_EV_QCC_RECV, qcc->conn);
@@ -1713,6 +1719,9 @@ int qcc_recv_max_stream_data(struct qcc *qcc, uint64_t id, uint64_t max)
 				tot_time_stop(&qcs->timer.fctl);
 				qcs_notify_send(qcs);
 			}
+
+			if (unblock_soft || unblock_real)
+				qcc_clear_frms(qcc);
 		}
 	}
 
@@ -2468,8 +2477,6 @@ static int qcc_io_send(struct qcc *qcc)
 	 * apply for STREAM frames.
 	 */
 
-	qcc_clear_frms(qcc);
-
 	/* Check for transport error. */
 	if (qcc->flags & QC_CF_ERR_CONN || qcc->conn->flags & CO_FL_ERROR) {
 		TRACE_DEVEL("connection on error", QMUX_EV_QCC_SEND, qcc->conn);
@@ -2505,10 +2512,14 @@ static int qcc_io_send(struct qcc *qcc)
 		goto out;
 	}
 
-	/* Encode STREAM frames for registered streams. */
-	total = qcc_build_frms(qcc, &qcs_failed);
-	if (!total)
-		goto sent_done;
+	/* Encode new STREAM frames if list has been previously cleared. */
+	if (LIST_ISEMPTY(frms) && !LIST_ISEMPTY(&qcc->send_list)) {
+		total = qcc_build_frms(qcc, &qcs_failed);
+		if (!total) {
+			BUG_ON(!LIST_ISEMPTY(frms));
+			goto out;
+		}
+	}
 
 	if (qcc_is_pacing_active(qcc->conn)) {
 		if (!LIST_ISEMPTY(frms) && !quic_pacing_expired(&qcc->tx.pacer)) {
@@ -2550,22 +2561,18 @@ static int qcc_io_send(struct qcc *qcc)
 		}
 	}
 
- sent_done:
 	if (ret == 1) {
 		/* qcc_send_frames cannot return 1 if pacing not used. */
 		BUG_ON(!qcc_is_pacing_active(qcc->conn));
 		qcc_wakeup_pacing(qcc);
 		++qcc->tx.paced_sent_ctr;
 	}
-	else if (!LIST_ISEMPTY(&qcc->tx.frms)) {
-		/* Deallocate frames that the transport layer has rejected. */
-		qcc_clear_frms(qcc);
-	}
-	else {
+	else if (LIST_ISEMPTY(frms)) {
 		/* Everything sent */
 		HA_ATOMIC_AND(&qcc->wait_event.tasklet->state, ~TASK_F_USR1);
 	}
 
+ out:
 	/* Re-insert on-error QCS at the end of the send-list. */
 	if (!LIST_ISEMPTY(&qcs_failed)) {
 		list_for_each_entry_safe(qcs, qcs_tmp, &qcs_failed, el_send) {
@@ -2577,7 +2584,6 @@ static int qcc_io_send(struct qcc *qcc)
 			qcc_wakeup(qcc);
 	}
 
- out:
 	if (qcc->conn->flags & CO_FL_ERROR && !(qcc->flags & QC_CF_ERR_CONN)) {
 		TRACE_ERROR("error reported by transport layer",
 		            QMUX_EV_QCC_SEND, qcc->conn);