Re: [PATCHv5 net-next] tcp: Add TCP ROCCET congestion control module.

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
- [Critical] Concurrent socket initialization mutates global configuration parameters without synchronization, leading to severe data races and corruption of `cube_factor`.
- [Critical] The `curr_rtt` field is implicitly initialized to 0, which permanently corrupts the `curr_min_rtt` calculation if evaluated before a valid RTT sample is received.
- [High] The `.ssthresh` callback erroneously returns the unmodified threshold, bypassing TCP's standard Retransmission Timeout (RTO) backoff and causing congestion floods.
- [High] ROCCET completely fails to reduce its congestion window upon receiving Explicit Congestion Notification (ECN) CE marks, violating RFC 3168.
- [High] ROCCET bypasses and breaks TCP's Proportional Rate Reduction (PRR) fast recovery by blindly overriding `cwnd` assignments during `TCP_CA_Recovery`.
- [High] Fast convergence logic incorrectly calculates `last_max_cwnd` using the artificially slashed probe window instead of the true network capacity.
- [Medium] Integer truncation of the acknowledged packet count allows attackers to spoof a low ACK rate and bypass congestion control.
--

commit 8c2a501e71c2cbc59259d9d2e53dda94d869cfdb
Author: Tim Fuechsel <t.fuechsel@gmx.de>
Subject: tcp: Add TCP ROCCET congestion control module.

This commit adds TCP ROCCET, a new congestion control algorithm based
on TCP CUBIC. It aims to improve overall performance and reduce bufferbloat
in cellular networks by utilizing srRTT and acknowledgment arrival rates.

Link: https://doi.org/10.1109/VTC2023-Fall60731.2023.10333357
Link: https://doi.org/10.1109/WMNC.2016.7543932
Link: https://doi.org/10.23919/WONS68803.2026.11501781
Link: http://go.lu-h.de/roccet-2026-07-21

> diff --git a/net/ipv4/tcp_roccet.c b/net/ipv4/tcp_roccet.c
> new file mode 100644
> --- /dev/null
> +++ b/net/ipv4/tcp_roccet.c
[ ... ]
> +static void roccettcp_reset(struct roccettcp *ca)
> +{
> +	memset(ca, 0, sizeof(struct roccettcp));
> +	ca->next_srrtt_check = 0;
> +	ca->curr_min_rtt = ~0U;
[ ... ]
> +	ca->initial_round_completed = false;
> +}
> +
> +static void update_min_rtt(struct sock *sk)
> +{
> +	struct roccettcp *ca = inet_csk_ca(sk);
> +
> +	/* Check if new lower min RTT was found. If so, set it directly */
> +	if (ca->curr_rtt < ca->curr_min_rtt) {
> +		ca->curr_min_rtt = max(ca->curr_rtt, 1);

[Severity: Critical]
Does this initialization logic safely handle early ACKs without valid RTT
samples?

roccettcp_reset() initializes the struct with zeroes, making curr_rtt = 0
and curr_min_rtt = ~0U. If an early ACK lacks a valid RTT sample (for
example, a pure duplicate ACK where sample->rtt_us < 0), curr_rtt remains
0. The comparison 0 < ~0U will succeed in update_min_rtt(), locking
curr_min_rtt to 1. Future valid RTTs will never be smaller than 1.

[ ... ]
> +	} else {
> +		ca->ack_rate_last_rate_time = now;
> +		ca->ack_rate_last_rate = ca->ack_rate_curr_rate;
> +		ca->ack_rate_curr_rate = ca->ack_rate_cnt;
> +		ca->ack_rate_cnt =
> +			acked; // start counting for the new interval
> +	}

[Severity: Medium]
Could this direct assignment truncate the 32-bit acked parameter?

The struct definition restricts ack_rate_cnt to only 16 bits. Without a
bounding check like min_t(u32, acked, U16_MAX) here, a malicious receiver
utilizing large windows or LRO/GRO aggregation could acknowledge more than
65535 packets at once. This would cause the counter to silently wrap
around and spoof a low ACK rate.

[ ... ]
> +static void roccet_min_rtt_probe(struct sock *sk, u32 now)
> +{
[ ... ]
> +	} else if (before(now, ca->refill_until)) {
> +		/* Reset cwnd and refill the pipe. */
> +		if (ca->state != RTT_PROBE_REFILL) {
> +			tcp_snd_cwnd_set(tp, ca->cwnd_before_min_rtt_probe);
> +			tcp_sk(sk)->snd_ssthresh = tcp_snd_cwnd(tp);
> +			ca->state = RTT_PROBE_REFILL;
> +		}

[Severity: High]
What happens if this min_rtt probe timer expires while the connection is
mid-recovery?

Abruptly inflating the cwnd back to the pre-probe value during fast
recovery would bypass the PRR pacing mechanism and inflate the window.

[ ... ]
> +static void param_precompute(void)
> +{
[ ... ]
> +	/* divide by bic_scale and by constant Srtt (100ms) */
> +	do_div(cube_factor, bic_scale * 10);
> +}
> +
> +static void roccettcp_init(struct sock *sk)
> +{
> +	/* Check & precompute on `init` in order to use the newest
> +	 * available params.
> +	 */
> +	param_check(true);
> +	param_precompute();

[Severity: Critical]
Does calling param_precompute() per socket initialization create a data
race on the global cube_factor variable?

Since roccettcp_init() is invoked dynamically for every newly created
socket, the unprotected in-place division do_div(cube_factor, ...) mutates
shared global state. If concurrent sockets are created, threads will
subject the variable to multiple divisions, resulting in a miscalculation
of the cwnd target in bictcp_update().

[ ... ]
> +static u32 roccettcp_ssthresh(struct sock *sk)
> +{
> +	return tcp_sk(sk)->snd_ssthresh;
> +}

[Severity: High]
Does returning the unmodified snd_ssthresh bypass TCP's required threshold
reductions?

When a severe congestion event like an RTO timeout occurs, or during an
ECN CE-mark response in TCP_CA_CWR, the core stack queries the .ssthresh
callback to shrink the slow-start threshold. Returning the current
threshold unmodified prevents PRR from pacing properly and allows the
window to exponentially grow back to massive sizes.

[ ... ]
> +static u32 roccettcp_recalc_ssthresh(struct sock *sk)
> +{
[ ... ]
> +	if (ca->state == RTT_PROBE) {
> +		/* Handle ECN as cubic congestion event in min
> +		 * RTT probe.
> +		 */
> +		ca->ece_received = false;
> +
> +		ca->epoch_start = 0; /* end of epoch */
> +
> +		/* Wmax and fast convergence */
> +		if (cwnd < ca->last_max_cwnd && fast_convergence)
> +			ca->last_max_cwnd =
> +				(cwnd * (BICTCP_BETA_SCALE + beta)) /
> +				(2 * BICTCP_BETA_SCALE);
> +		else
> +			ca->last_max_cwnd = cwnd;
> +
> +		cwnd = ca->cwnd_before_min_rtt_probe;

[Severity: High]
Is this fast convergence calculation using the correct window size?

The active cwnd is artificially halved during the min_rtt probe. By
calculating the new last_max_cwnd using this slashed probe window before
restoring the true cwnd, the historical maximum window (which anchors the
CUBIC curve origin) would become permanently understated.

[ ... ]
> +	/* Handle ECN as ROCCET congestion event. */
> +	if (ca->ece_received) {
> +		ca->ece_received = false;
> +		roccet_congestion_event(sk, jiffies_to_usecs(tcp_jiffies32));
> +		return tcp_snd_cwnd(tp);
> +	}

[Severity: High]
Is this ECN handling code unreachable?

This intended ECN handling code sits inside roccettcp_recalc_ssthresh(),
which appears to only be invoked by roccettcp_state() during
TCP_CA_Recovery. Since ECN marks normally trigger TCP_CA_CWR (which
roccettcp_state() ignores), this logic will never execute during actual
ECN events, violating protocol requirements.

[ ... ]
> +static void roccettcp_state(struct sock *sk, u8 new_state)
> +{
> +	struct roccettcp *ca = inet_csk_ca(sk);
> +	struct tcp_sock *tp = tcp_sk(sk);
> +
> +	ca->is_in_recovery = false;
> +
> +	if (new_state == TCP_CA_Loss) {
> +		roccettcp_reset(ca);
> +	} else if (new_state == TCP_CA_Recovery) {
> +		ca->is_in_recovery = true;
> +
> +		/* Here we set the cwnd and ssthresh to the same value so
> +		 * the TCP state machine knows we are in cong. avoid and
> +		 * not in slow start.
> +		 */
> +		tcp_sk(sk)->snd_ssthresh = roccettcp_recalc_ssthresh(sk);
> +		tcp_snd_cwnd_set(tp, tcp_sk(sk)->snd_ssthresh);
> +	}
> +}

[Severity: High]
Does instantly setting the cwnd here bypass TCP's Proportional Rate
Reduction (PRR) fast recovery?

Blindly overriding the cwnd assignment upon entering TCP_CA_Recovery
instantly shrinks the window and destroys the core stack's PRR pacing
mechanism, which leads to retransmission bursts.

Additionally, the TCP_CA_Loss branch misses the manual ssthresh reduction
applied to Recovery, cementing the RTO backoff bypass.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/apWz9rPcGMgYCKOR@volt-roccet-vm?part=1