MINOR: quic: refactor acknowledgment for emitted STREAM

Author: a-denoyelle

include/haproxy/quic_stream.h

@@ -7,12 +7,13 @@
 #include <haproxy/quic_stream-t.h>
 
 struct quic_conn;
+struct quic_frame;
 
 struct qc_stream_desc *qc_stream_desc_new(uint64_t id, enum qcs_type, void *ctx,
                                           struct quic_conn *qc);
 void qc_stream_desc_release(struct qc_stream_desc *stream, uint64_t final_size,
                             void *new_ctx);
-int qc_stream_desc_ack(struct qc_stream_desc *stream, size_t offset, size_t len, int fin);
+int qc_stream_desc_ack(struct qc_stream_desc *stream, struct quic_frame *frm);
 void qc_stream_desc_free(struct qc_stream_desc *stream, int closing);
 
 struct buffer *qc_stream_buf_get(struct qc_stream_desc *stream);

src/quic_rx.c

@@ -225,14 +225,11 @@ static int quic_stream_try_to_consume(struct quic_conn *qc,
 		while (frm_node) {
 			struct qf_stream *strm_frm;
 			struct quic_frame *frm;
-			size_t offset, len;
-			int fin;
+			size_t offset;
 
 			strm_frm = eb64_entry(frm_node, struct qf_stream, offset);
 			frm = container_of(strm_frm, struct quic_frame, stream);
 			offset = strm_frm->offset.key;
-			len = strm_frm->len;
-			fin = frm->type & QUIC_STREAM_FRAME_TYPE_FIN_BIT;
 
 			if (offset > stream->ack_offset)
 				break;
@@ -241,13 +238,13 @@ static int quic_stream_try_to_consume(struct quic_conn *qc,
 			 * stream_buf instance is removed via qc_stream_desc_ack().
 			 */
 			eb64_delete(frm_node);
-			qc_release_frm(qc, frm);
 
-			if (qc_stream_desc_ack(stream, offset, len, fin)) {
+			if (qc_stream_desc_ack(stream, frm)) {
 				TRACE_DEVEL("stream consumed", QUIC_EV_CONN_ACKSTRM,
 				            qc, strm_frm, stream);
 				ret = 1;
 			}
+			qc_release_frm(qc, frm);
 
 			/* Always retrieve first buffer as the previously used
 			 * instance could have been removed during qc_stream_desc_ack().
@@ -284,9 +281,6 @@ static void qc_handle_newly_acked_frm(struct quic_conn *qc, struct quic_frame *f
 		struct qf_stream *strm_frm = &frm->stream;
 		struct eb64_node *node = NULL;
 		struct qc_stream_desc *stream = NULL;
-		const size_t offset = strm_frm->offset.key;
-		const size_t len = strm_frm->len;
-		const int fin = frm->type & QUIC_STREAM_FRAME_TYPE_FIN_BIT;
 		int ack;
 
 		/* do not use strm_frm->stream as the qc_stream_desc instance
@@ -306,31 +300,24 @@ static void qc_handle_newly_acked_frm(struct quic_conn *qc, struct quic_frame *f
 		stream = eb64_entry(node, struct qc_stream_desc, by_id);
 
 		TRACE_DEVEL("acked stream", QUIC_EV_CONN_ACKSTRM, qc, strm_frm, stream);
-		if (offset <= stream->ack_offset) {
-			ack = qc_stream_desc_ack(stream, offset, len, fin);
-			if (ack || fin) {
-				TRACE_DEVEL("stream consumed", QUIC_EV_CONN_ACKSTRM,
-				            qc, strm_frm, stream);
+		ack = qc_stream_desc_ack(stream, frm);
+		if (!ack) {
+			TRACE_DEVEL("stream consumed", QUIC_EV_CONN_ACKSTRM,
+			            qc, strm_frm, stream);
 
-				quic_stream_try_to_consume(qc, stream);
+			quic_stream_try_to_consume(qc, stream);
 
-				if (qc_stream_desc_done(stream)) {
-					/* no need to continue if stream freed. */
-					TRACE_DEVEL("stream released and freed", QUIC_EV_CONN_ACKSTRM, qc);
-					qc_check_close_on_released_mux(qc);
-				}
+			if (qc_stream_desc_done(stream)) {
+				/* no need to continue if stream freed. */
+				TRACE_DEVEL("stream released and freed", QUIC_EV_CONN_ACKSTRM, qc);
+				qc_check_close_on_released_mux(qc);
 			}
 
 			qc_release_frm(qc, frm);
 		}
 		else {
-			struct qc_stream_buf *stream_buf;
-			struct eb64_node *buf_node;
-
-			buf_node = eb64_lookup_le(&stream->buf_tree, offset);
-			BUG_ON(!buf_node);
-			stream_buf = eb64_entry(buf_node, struct qc_stream_buf, offset_node);
-			eb64_insert(&stream_buf->acked_frms, &strm_frm->offset);
+			TRACE_DEVEL("handled out-of-order stream ACK", QUIC_EV_CONN_ACKSTRM,
+			            qc, strm_frm, stream);
 		}
 	}
 	break;

src/quic_stream.c

@@ -9,7 +9,7 @@
 #include <haproxy/mux_quic.h>
 #include <haproxy/pool.h>
 #include <haproxy/quic_conn.h>
-#include <haproxy/quic_frame.h>
+#include <haproxy/quic_frame-t.h>
 #include <haproxy/task.h>
 
 DECLARE_STATIC_POOL(pool_head_quic_stream_desc, "qc_stream_desc",
@@ -134,26 +134,43 @@ void qc_stream_desc_release(struct qc_stream_desc *stream,
 	}
 }
 
-/* Acknowledge data at <offset> of length <len> for <stream> with <fin> set for
- * the final data.
+/* Acknowledge <frm> STREAM frame whose content is managed by <stream>
+ * descriptor.
  *
- * Returns the count of byte removed from stream.
+ * Returns 0 if the frame has been handled and can be removed.
+ * Returns a positive value if the frame cannot be acknowledged and has been
+ * buffered.
  */
-int qc_stream_desc_ack(struct qc_stream_desc *stream, size_t offset, size_t len,
-                       int fin)
+int qc_stream_desc_ack(struct qc_stream_desc *stream, struct quic_frame *frm)
 {
+	struct qf_stream *strm_frm = &frm->stream;
+	uint64_t offset = strm_frm->offset.key;
+	uint64_t len = strm_frm->len;
+	int fin = frm->type & QUIC_STREAM_FRAME_TYPE_FIN_BIT;
+
 	struct qc_stream_buf *stream_buf = NULL;
+	struct eb64_node *buf_node;
 	struct buffer *buf = NULL;
 	size_t diff;
 
 	/* Cannot advertise FIN for an inferior data range. */
 	BUG_ON(fin && offset + len < stream->ack_offset);
 
-	/* No support now for out-of-order ACK reporting. */
-	BUG_ON(offset > stream->ack_offset);
-
-	if (offset + len < stream->ack_offset)
+	if (offset + len < stream->ack_offset) {
 		return 0;
+	}
+	else if (offset > stream->ack_offset) {
+		buf_node = eb64_lookup_le(&stream->buf_tree, offset);
+		if (buf_node) {
+			stream_buf = eb64_entry(buf_node, struct qc_stream_buf, offset_node);
+			eb64_insert(&stream_buf->acked_frms, &strm_frm->offset);
+			return 1;
+		}
+		else {
+			ABORT_NOW();
+			return 0;
+		}
+	}
 
 	diff = offset + len - stream->ack_offset;
 	if (diff) {
@@ -193,7 +210,7 @@ int qc_stream_desc_ack(struct qc_stream_desc *stream, size_t offset, size_t len,
 		stream->flags &= ~QC_SD_FL_WAIT_FOR_FIN;
 	}
 
-	return diff;
+	return 0;
 }
 
 /* Free the stream descriptor <stream> content. This function should be used