From: Amaury Denoyelle Date: Wed, 21 May 2025 09:39:20 +0000 (+0200) Subject: MINOR: quic: refactor handling of streams after MUX release X-Git-Tag: v3.2-dev17~5 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=f286288471929c8d7e5e4a94fb0fac7115883c59;p=thirdparty%2Fhaproxy.git MINOR: quic: refactor handling of streams after MUX release quic-conn layer has to handle itself STREAM frames after MUX release. If the stream was already seen, it is probably only a retransmitted frame which can be safely ignored. For other streams, an active closure may be needed. Thus it's necessary that quic-conn layer knows the highest stream ID already handled by the MUX after its release. Previously, this was done via member array in quic-conn structure. Refactor this by replacing by two members called /. Indeed, it is unnecessary for quic-conn layer to monitor locally opened uni streams, as the peer cannot by definition emit a STREAM frame on it. Also, bidirectional streams are always opened by the remote side. Previously, were set by quic-stream layer. Now, / members are only set one time, just prior to QUIC MUX release. This is sufficient as quic-conn do not use them if the MUX is available. Note that previously, IDs were used relatively to their type, thus incremented by 1, after shifting the original value. For simplification, use the plain stream ID, which is incremented by 4. --- diff --git a/include/haproxy/mux_quic-t.h b/include/haproxy/mux_quic-t.h index 6ce9adfe2..fd03f861f 100644 --- a/include/haproxy/mux_quic-t.h +++ b/include/haproxy/mux_quic-t.h @@ -28,9 +28,6 @@ enum qcs_type { QCS_SRV_BIDI, QCS_CLT_UNI, QCS_SRV_UNI, - - /* Must be the last one */ - QCS_MAX_TYPES }; enum qcc_app_st { diff --git a/include/haproxy/mux_quic.h b/include/haproxy/mux_quic.h index eadce5286..ca71e940e 100644 --- a/include/haproxy/mux_quic.h +++ b/include/haproxy/mux_quic.h @@ -51,15 +51,7 @@ static inline int qmux_stream_rx_bufsz(void) return global.tune.bufsize - NCB_RESERVED_SZ; } -/* Bit shift to get the stream sub ID for internal use which is obtained - * shifting the stream IDs by this value, knowing that the - * QCS_ID_TYPE_SHIFT less significant bits identify the stream ID - * types (client initiated bidirectional, server initiated bidirectional, - * client initiated unidirectional, server initiated bidirectional). - * Note that there is no reference to such stream sub IDs in the RFC. - */ #define QCS_ID_TYPE_MASK 0x3 -#define QCS_ID_TYPE_SHIFT 2 /* The less significant bit of a stream ID is set for a server initiated stream */ #define QCS_ID_SRV_INTIATOR_BIT 0x1 /* This bit is set for unidirectional streams */ diff --git a/include/haproxy/quic_conn-t.h b/include/haproxy/quic_conn-t.h index a060eca60..f97ef4289 100644 --- a/include/haproxy/quic_conn-t.h +++ b/include/haproxy/quic_conn-t.h @@ -385,10 +385,10 @@ struct quic_conn { /* RX buffer */ struct buffer buf; struct list pkt_list; - struct { - /* Number of open or closed streams */ - uint64_t nb_streams; - } strms[QCS_MAX_TYPES]; + + /* first unhandled streams ID, set by MUX after release */ + uint64_t stream_max_uni; + uint64_t stream_max_bidi; } rx; struct { struct quic_tls_kp prv_rx; diff --git a/src/mux_quic.c b/src/mux_quic.c index 68377498a..00c044282 100644 --- a/src/mux_quic.c +++ b/src/mux_quic.c @@ -3225,15 +3225,20 @@ static void qcc_release(struct qcc *qcc) qcs_free(qcs); } - /* unsubscribe from all remaining qc_stream_desc */ if (conn) { qc = conn->handle.qc; + + /* unsubscribe from all remaining qc_stream_desc */ node = eb64_first(&qc->streams_by_id); while (node) { struct qc_stream_desc *stream = eb64_entry(node, struct qc_stream_desc, by_id); qc_stream_desc_sub_room(stream, NULL); node = eb64_next(node); } + + /* register streams IDs so that quic-conn layer can ignore already closed streams. */ + qc->rx.stream_max_uni = qcc->largest_uni_r; + qc->rx.stream_max_bidi = qcc->largest_bidi_r; } tasklet_free(qcc->wait_event.tasklet); diff --git a/src/quic_conn.c b/src/quic_conn.c index a9706b151..6f7be84d7 100644 --- a/src/quic_conn.c +++ b/src/quic_conn.c @@ -1030,7 +1030,6 @@ struct quic_conn *qc_new_conn(const struct quic_version *qv, int ipv4, struct sockaddr_storage *peer_addr, int server, int token, void *owner) { - int i; struct quic_conn *qc = NULL; struct listener *l = server ? owner : NULL; struct proxy *prx = l ? l->bind_conf->frontend : NULL; @@ -1216,8 +1215,7 @@ struct quic_conn *qc_new_conn(const struct quic_version *qv, int ipv4, qc->bytes.rx = 0; memset(&qc->rx.params, 0, sizeof(qc->rx.params)); qc->rx.buf = b_make(qc->rx.buf.area, QUIC_CONN_RX_BUFSZ, 0, 0); - for (i = 0; i < QCS_MAX_TYPES; i++) - qc->rx.strms[i].nb_streams = 0; + qc->rx.stream_max_uni = qc->rx.stream_max_bidi = 0; qc->nb_pkt_for_cc = 1; qc->nb_pkt_since_cc = 0; diff --git a/src/quic_rx.c b/src/quic_rx.c index 7d6fbd9ec..6f650c4c1 100644 --- a/src/quic_rx.c +++ b/src/quic_rx.c @@ -955,12 +955,13 @@ static int qc_parse_pkt_frms(struct quic_conn *qc, struct quic_rx_packet *pkt, case QUIC_FT_STREAM_8 ... QUIC_FT_STREAM_F: { struct qf_stream *strm_frm = &frm->stream; - unsigned nb_streams = qc->rx.strms[qcs_id_type(strm_frm->id)].nb_streams; const char fin = frm->type & QUIC_STREAM_FRAME_TYPE_FIN_BIT; + const uint64_t max = quic_stream_is_uni(strm_frm->id) ? + qc->rx.stream_max_uni : qc->rx.stream_max_bidi; /* The upper layer may not be allocated. */ if (qc->mux_state != QC_MUX_READY) { - if ((strm_frm->id >> QCS_ID_TYPE_SHIFT) < nb_streams) { + if (strm_frm->id < max) { TRACE_DATA("Already closed stream", QUIC_EV_CONN_PRSHPKT, qc); } else { diff --git a/src/quic_stream.c b/src/quic_stream.c index b1b2e7023..0d3a438ec 100644 --- a/src/quic_stream.c +++ b/src/quic_stream.c @@ -80,7 +80,6 @@ struct qc_stream_desc *qc_stream_desc_new(uint64_t id, enum qcs_type type, void else { stream->by_id.key = id; eb64_insert(&qc->streams_by_id, &stream->by_id); - qc->rx.strms[type].nb_streams++; } stream->qc = qc;