Thread (34 messages) 34 messages, 2 authors, 2026-03-05

Re: [net-next,v10,09/15] quic: add congestion control

From: Paolo Abeni <pabeni@redhat.com>
Date: 2026-03-03 08:32:41
Also in: linux-cifs

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
quic: add congestion control

This patch introduces quic_cong for RTT measurement and congestion
control, implementing a state machine with slow start, congestion
avoidance, and recovery phases using the New Reno algorithm.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
quoted hunk ↗ jump to hunk
diff --git a/net/quic/Makefile b/net/quic/Makefile
index 1565fb5cef9d..4d4a42c6d565 100644
--- a/net/quic/Makefile
+++ b/net/quic/Makefile
@@ -5,4 +5,5 @@

 obj-$(CONFIG_IP_QUIC) += quic.o

-quic-y := common.o family.o protocol.o socket.o stream.o connid.o path.o
+quic-y := common.o family.o protocol.o socket.o stream.o connid.o path.o \
+	  cong.o
diff --git a/net/quic/cong.c b/net/quic/cong.c
new file mode 100644
index 000000000000..e0f78ecb00e4
--- /dev/null
+++ b/net/quic/cong.c
[ ... ]
+static void quic_reno_on_packet_lost(struct quic_cong *cong, u64 time, u32 bytes, s64 number)
+{
+	if (quic_cong_check_persistent_congestion(cong, time))
+		return;
+
+	switch (cong->state) {
+	case QUIC_CONG_SLOW_START:
+		pr_debug("%s: slow_start -> recovery, cwnd: %u, ssthresh: %u\n",
+			 __func__, cong->window, cong->ssthresh);
+		break;
+	case QUIC_CONG_RECOVERY_PERIOD:
+		return;
+	case QUIC_CONG_CONGESTION_AVOIDANCE:
+		pr_debug("%s: cong_avoid -> recovery, cwnd: %u, ssthresh: %u\n",
+			 __func__, cong->window, cong->ssthresh);
+		break;
+	default:
+		pr_debug("%s: wrong congestion state: %d\n", __func__, cong->state);
+		return;
+	}
+
+	cong->recovery_time = cong->time;
+	cong->state = QUIC_CONG_RECOVERY_PERIOD;
+	cong->ssthresh = max(cong->window >> 1U, cong->min_window);
+	cong->window = cong->ssthresh;
+}
[ ... ]
+static void quic_reno_on_process_ecn(struct quic_cong *cong)
+{
+	switch (cong->state) {
+	case QUIC_CONG_SLOW_START:
+		pr_debug("%s: slow_start -> recovery, cwnd: %u, ssthresh: %u\n",
+			 __func__, cong->window, cong->ssthresh);
+		break;
+	case QUIC_CONG_RECOVERY_PERIOD:
+		return;
+	case QUIC_CONG_CONGESTION_AVOIDANCE:
+		pr_debug("%s: cong_avoid -> recovery, cwnd: %u, ssthresh: %u\n",
+			 __func__, cong->window, cong->ssthresh);
+		break;
+	default:
+		pr_debug("%s: wrong congestion state: %d\n", __func__, cong->state);
+		return;
+	}
+
+	cong->recovery_time = cong->time;
+	cong->state = QUIC_CONG_RECOVERY_PERIOD;
+	cong->ssthresh = max(cong->window >> 1U, cong->min_window);
+	cong->window = cong->ssthresh;
+}
This isn't a bug, but these two functions share almost identical logic
for entering the recovery state. The switch statement has the same three
cases with identical handling in both functions, and the four-line
recovery-entry block at the end is exactly the same:

    cong->recovery_time = cong->time;
    cong->state = QUIC_CONG_RECOVERY_PERIOD;
    cong->ssthresh = max(cong->window >> 1U, cong->min_window);
    cong->window = cong->ssthresh;

The only difference is quic_reno_on_packet_lost() checks for persistent
congestion first. Would factoring this shared logic into a helper
function make future maintenance easier and ensure the recovery path
stays consistent across both ECN and loss events?

[ ... ]
+void quic_cong_set_algo(struct quic_cong *cong, u8 algo)
+{
+	/* The caller must ensure algo < QUIC_CONG_ALG_MAX. */
+	cong->algo = algo;
+	cong->state = QUIC_CONG_SLOW_START;
+	cong->ssthresh = U32_MAX;
+	cong->ops = &quic_congs[algo];
+	cong->ops->on_init(cong);
+}
Should this function validate the algo parameter? The comment says the
caller must ensure algo is less than QUIC_CONG_ALG_MAX, but there's no
runtime check. Since QUIC_CONG_ALG_MAX is defined in the UAPI and this
function is exported in cong.h, could a future caller (for example, a
setsockopt path accepting user-supplied algorithm choices) pass an
out-of-range value and cause an out-of-bounds array access into
quic_congs[]?

[ ... ]
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help