Re: [PATCH net-next v6 06/16] quic: add stream management
From: Paolo Abeni <pabeni@redhat.com>
Date: 2026-01-08 15:36:02
Also in:
linux-cifs
On 1/5/26 3:04 PM, Xin Long wrote:
+/* Create and register new streams for sending or receiving. */
+static struct quic_stream *quic_stream_create(struct quic_stream_table *streams,
+ s64 max_stream_id, bool send, bool is_serv)
+{
+ struct quic_stream_limits *limits = &streams->send;
+ struct quic_stream *stream = NULL;
+ gfp_t gfp = GFP_KERNEL_ACCOUNT;
+ s64 stream_id;
+
+ if (!send) {
+ limits = &streams->recv;
+ gfp = GFP_ATOMIC | __GFP_ACCOUNT;
+ }
+ stream_id = limits->next_bidi_stream_id;
+ if (quic_stream_id_uni(max_stream_id))
+ stream_id = limits->next_uni_stream_id;
+
+ /* rfc9000#section-2.1: A stream ID that is used out of order results in all streams
+ * of that type with lower-numbered stream IDs also being opened.
+ */
+ while (stream_id <= max_stream_id) {
+ stream = kzalloc(sizeof(*stream), gfp);
+ if (!stream)
+ return NULL;Do you need to release the allocated ids in case of failure? It would be sourprising to find some ids allocated when this call fails/returns NULL.
+
+ stream->id = stream_id;
+ if (quic_stream_id_uni(stream_id)) {
+ if (send) {
+ stream->send.max_bytes = limits->max_stream_data_uni;
+ } else {
+ stream->recv.max_bytes = limits->max_stream_data_uni;
+ stream->recv.window = stream->recv.max_bytes;
+ }
+ /* Streams must be opened sequentially. Update the next stream ID so the
+ * correct starting point is known if an out-of-order open is requested.
+ */
+ limits->next_uni_stream_id = stream_id + QUIC_STREAM_ID_STEP;
+ limits->streams_uni++;
+
+ quic_stream_add(streams, stream);
+ stream_id += QUIC_STREAM_ID_STEP;
+ continue;
+ }
+
+ if (quic_stream_id_local(stream_id, is_serv)) {
+ stream->send.max_bytes = streams->send.max_stream_data_bidi_remote;
+ stream->recv.max_bytes = streams->recv.max_stream_data_bidi_local;
+ } else {
+ stream->send.max_bytes = streams->send.max_stream_data_bidi_local;
+ stream->recv.max_bytes = streams->recv.max_stream_data_bidi_remote;
+ }
+ stream->recv.window = stream->recv.max_bytes;
+
+ limits->next_bidi_stream_id = stream_id + QUIC_STREAM_ID_STEP;
+ limits->streams_bidi++;
+
+ quic_stream_add(streams, stream);
+ stream_id += QUIC_STREAM_ID_STEP;
+ }
+ return stream;
+}
+
+/* Check if a send or receive stream ID is already closed. */
+static bool quic_stream_id_closed(struct quic_stream_table *streams, s64 stream_id, bool send)
+{
+ struct quic_stream_limits *limits = send ? &streams->send : &streams->recv;
+
+ if (quic_stream_id_uni(stream_id))
+ return stream_id < limits->next_uni_stream_id;
+ return stream_id < limits->next_bidi_stream_id;I can't recall if I mentioned the following in a past review... it looks like the above assumes wrap around are not possible, which is realistic given the u64 counters - it would require > 100y on a server allocating 4G ids per second. But it would be nice to explcitly document such assumption somewhere.
+}
+
+/* Check if a stream ID would exceed local (recv) or peer (send) limits. */
+bool quic_stream_id_exceeds(struct quic_stream_table *streams, s64 stream_id, bool send)
+{
+ u64 nstreams;
+
+ if (!send) {
+ if (quic_stream_id_uni(stream_id))
+ return stream_id > streams->recv.max_uni_stream_id;
+ return stream_id > streams->recv.max_bidi_stream_id;
+ }
+
+ if (quic_stream_id_uni(stream_id)) {
+ if (stream_id > streams->send.max_uni_stream_id)
+ return true;
+ stream_id -= streams->send.next_uni_stream_id;
+ nstreams = quic_stream_id_to_streams(stream_id);It's not clear to me why send streams only have this additional check.
+ return nstreams + streams->send.streams_uni > streams->send.max_streams_uni;
Possibly it would be more consistent max_uni_stream_id -> max_stream_ids_uni (no strong preferences)
+ } + + if (stream_id > streams->send.max_bidi_stream_id) + return true; + stream_id -= streams->send.next_bidi_stream_id; + nstreams = quic_stream_id_to_streams(stream_id); + return nstreams + streams->send.streams_bidi > streams->send.max_streams_bidi; +}
[...]
+/* Get or create a receive stream by ID. Requires sock lock held. */
+struct quic_stream *quic_stream_recv_get(struct quic_stream_table *streams, s64 stream_id,
+ bool is_serv)
+{
+ struct quic_stream *stream;
+
+ if (!quic_stream_id_valid(stream_id, is_serv, false))
+ return ERR_PTR(-EINVAL);
+
+ stream = quic_stream_find(streams, stream_id);
+ if (stream)
+ return stream;
+
+ if (quic_stream_id_local(stream_id, is_serv)) {
+ if (quic_stream_id_closed(streams, stream_id, true))
+ return ERR_PTR(-ENOSTR);
+ return ERR_PTR(-EINVAL);
+ }
+
+ if (quic_stream_id_closed(streams, stream_id, false))
+ return ERR_PTR(-ENOSTR);
+
+ if (quic_stream_id_exceeds(streams, stream_id, false))
+ return ERR_PTR(-EAGAIN);
+
+ stream = quic_stream_create(streams, stream_id, false, is_serv);
+ if (!stream)
+ return ERR_PTR(-ENOSTR);
+ if (quic_stream_id_valid(stream_id, is_serv, true))
+ streams->send.active_stream_id = stream_id;This function is really similar to quic_stream_send_get(), I think it should be easy factor out a common helper (and possibly use directly such helper with no send/recv wrapper). /P