From: Amaury Denoyelle <adenoyelle@haproxy.com>
Date: Mon, 4 Nov 2024 16:27:39 +0000 (+0100)
Subject: MINOR: quic: extend return value of CRYPTO parsing
X-Git-Tag: v3.1-dev12~23
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=d65e782c8cd2f8554404dd1424e2d64f3786edb1;p=thirdparty%2Fhaproxy.git

MINOR: quic: extend return value of CRYPTO parsing

qc_handle_crypto_frm() is the function used to handled a newly received
CRYPTO frame. Change its API to use a newly dedicated return type. This
allows to report if the frame was properly handled, ignored if already
parsed previously or rejected after a fatal error.

This commit does not have any functional changes. However, it allows to
simplify qc_handle_crypto_frm() API by removing <fast_retrans> as output
parameter. Also, this patch will be necessary to support multiple
iteration of packet parsing for CRYPTO frames.
---

diff --git a/include/haproxy/quic_rx-t.h b/include/haproxy/quic_rx-t.h
index 6b5a0c4c9f..6639364dfa 100644
--- a/include/haproxy/quic_rx-t.h
+++ b/include/haproxy/quic_rx-t.h
@@ -58,4 +58,10 @@ struct quic_rx_packet {
 	unsigned int time_received;
 };
 
+enum quic_rx_ret_frm {
+	QUIC_RX_RET_FRM_DONE = 0, /* frame handled correctly */
+	QUIC_RX_RET_FRM_DUP,      /* frame ignored as already handled previously */
+	QUIC_RX_RET_FRM_FATAL,    /* error during frame handling, packet must not be acknowledged */
+};
+
 #endif /* _HAPROXY_RX_T_H */
diff --git a/src/quic_rx.c b/src/quic_rx.c
index fde8cf3a9c..826b032df9 100644
--- a/src/quic_rx.c
+++ b/src/quic_rx.c
@@ -616,17 +616,22 @@ static int qc_handle_strm_frm(struct quic_rx_packet *pkt,
 	return !ret;
 }
 
-/* Parse <frm> CRYPTO frame coming with <pkt> packet at <qel> <qc> connectionn.
- * Returns 1 if succeeded, 0 if not. Also set <*fast_retrans> to 1 if the
- * speed up handshake completion may be run after having received duplicated
- * CRYPTO data.
+/* Parse <frm> CRYPTO frame coming with <pkt> packet at <qel> <qc> connection.
+ *
+ * Returns 0 on success or a negative error code. A positive value is used to
+ * indicate that the current frame cannot be handled immediately, but it could
+ * be solved by running a new packet parsing iteration.
+ *
+ * Also set <*fast_retrans> as output parameter to 1 if the speed up handshake
+ * completion may be run after having received duplicated CRYPTO data.
  */
-static int qc_handle_crypto_frm(struct quic_conn *qc,
-                                struct qf_crypto *crypto_frm, struct quic_rx_packet *pkt,
-                                struct quic_enc_level *qel, int *fast_retrans)
+static enum quic_rx_ret_frm qc_handle_crypto_frm(struct quic_conn *qc,
+                                                 struct qf_crypto *crypto_frm,
+                                                 struct quic_rx_packet *pkt,
+                                                 struct quic_enc_level *qel)
 {
-	int ret = 0;
 	enum ncb_ret ncb_ret;
+	enum quic_rx_ret_frm ret = QUIC_RX_RET_FRM_DONE;
 	/* XXX TO DO: <cfdebug> is used only for the traces. */
 	struct quic_rx_crypto_frm cfdebug = {
 		.offset_node.key = crypto_frm->offset,
@@ -643,10 +648,8 @@ static int qc_handle_crypto_frm(struct quic_conn *qc,
 		if (crypto_frm->offset + crypto_frm->len <= cstream->rx.offset) {
 			/* Nothing to do */
 			TRACE_PROTO("Already received CRYPTO data",
-						QUIC_EV_CONN_RXPKT, qc, pkt, &cfdebug);
-			if (qc_is_listener(qc) && qel == qc->iel &&
-				!(qc->flags & QUIC_FL_CONN_HANDSHAKE_SPEED_UP))
-				*fast_retrans = 1;
+			            QUIC_EV_CONN_RXPKT, qc, pkt, &cfdebug);
+			ret = QUIC_RX_RET_FRM_DUP;
 			goto done;
 		}
 
@@ -661,7 +664,7 @@ static int qc_handle_crypto_frm(struct quic_conn *qc,
 
 	if (!quic_get_ncbuf(ncbuf) || ncb_is_null(ncbuf)) {
 		TRACE_ERROR("CRYPTO ncbuf allocation failed", QUIC_EV_CONN_PRSHPKT, qc);
-		goto leave;
+		goto err;
 	}
 
 	/* crypto_frm->offset > cstream-trx.offset */
@@ -677,7 +680,7 @@ static int qc_handle_crypto_frm(struct quic_conn *qc,
 			TRACE_ERROR("cannot bufferize frame due to gap size limit",
 			            QUIC_EV_CONN_PRSHPKT, qc);
 		}
-		goto leave;
+		goto err;
 	}
 
 	/* Reschedule with TASK_HEAVY if CRYPTO data ready for decoding. */
@@ -687,10 +690,12 @@ static int qc_handle_crypto_frm(struct quic_conn *qc,
 	}
 
  done:
-	ret = 1;
- leave:
 	TRACE_LEAVE(QUIC_EV_CONN_PRSHPKT, qc);
 	return ret;
+
+ err:
+	TRACE_DEVEL("leaving on error", QUIC_EV_CONN_PRSHPKT, qc);
+	return QUIC_RX_RET_FRM_FATAL;
 }
 
 /* Handle RETIRE_CONNECTION_ID frame from <frm> frame.
@@ -770,6 +775,7 @@ static int qc_parse_pkt_frms(struct quic_conn *qc, struct quic_rx_packet *pkt,
 {
 	struct quic_frame *frm = NULL;
 	const unsigned char *pos, *end;
+	enum quic_rx_ret_frm ret;
 	int fast_retrans = 0;
 
 	TRACE_ENTER(QUIC_EV_CONN_PRSHPKT, qc);
@@ -848,8 +854,20 @@ static int qc_parse_pkt_frms(struct quic_conn *qc, struct quic_rx_packet *pkt,
 			break;
 		}
 		case QUIC_FT_CRYPTO:
-			if (!qc_handle_crypto_frm(qc, &frm->crypto, pkt, qel, &fast_retrans))
+			ret = qc_handle_crypto_frm(qc, &frm->crypto, pkt, qel);
+			switch (ret) {
+			case QUIC_RX_RET_FRM_FATAL:
 				goto err;
+			case QUIC_RX_RET_FRM_DUP:
+				if (qc_is_listener(qc) && qel == qc->iel &&
+				    !(qc->flags & QUIC_FL_CONN_HANDSHAKE_SPEED_UP)) {
+					fast_retrans = 1;
+				}
+				break;
+			case QUIC_RX_RET_FRM_DONE:
+				/* nothing to do here */
+				break;
+			}
 			break;
 		case QUIC_FT_NEW_TOKEN:
 			/* TODO */