tcp: Add TCP ROCCET congestion control module.

net/ipv4/tcp_roccet.c line 134

[Severity: Low]
Will this bitfield layout cause a compile-time failure?

Because the preceding field is u16 (which ends at offset 102), the compiler inserts 2 bytes of padding to align the enum allocation unit, which needs 4-byte alignment, at offset 104. This expands the total struct size to 108 bytes.

net/ipv4/tcp_roccet.c line 1377

Since ICSK_CA_PRIV_SIZE is statically defined as 104 bytes in include/net/inet_connection_sock.h, it seems this check will unconditionally fail and break the module build.

[ ... ]

net/ipv4/tcp_roccet.c line 965

[Severity: High]
Does this comparison trigger false congestion events when ACKs outpace outgoing data?

If received exceeds sent, sent - received wraps to a very large u32. When cast to s32, it correctly represents a negative difference.

However, because div_u64() returns a u64, standard C arithmetic conversions cause the negative s32 to be sign-extended and promoted to u64.

This results in a huge positive number, making the condition incorrectly evaluate to true, which freezes congestion window growth.

[ ... ]

net/ipv4/tcp_roccet.c line 1008

[Severity: High]
Can returning early here during TCP_CA_Recovery stall Fast Retransmit?

When a socket enters loss recovery, the TCP core delegates all congestion control processing to the cong_control callback, bypassing the standard tcp_cwnd_reduction() PRR routine.

If roccet_orbiter_update() always returns early without inflating snd_cwnd for incoming dupACKs:

[ ... ]

net/ipv4/tcp_roccet.c line 1193

The snd_cwnd is statically reduced upon entering recovery. Because it is never inflated by dupACKs, tcp_packets_in_flight() will remain above the reduced snd_cwnd.

When tcp_xmit_retransmit_queue() attempts to fast retransmit, it calculates available segments as tcp_snd_cwnd(tp) - tcp_packets_in_flight(tp) which evaluates to <= 0, completely preventing retransmissions until an RTO fires.

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

net/ipv4/tcp_roccet.c line 176

[Severity: Low]
This isn't a bug, but both of these parameter descriptions disagree with how the parameters are used.

beta is only used as the multiplicative decrease factor
(cwnd * beta / BICTCP_BETA_SCALE) in roccet_congestion_event(), roccet_recalc_ssthresh() and roccet_handle_recovery(). Should the text say "multiplicative decrease"?

For ack_rate_diff_ss, roccet_launch_update() exits LAUNCH when the ACK rate increase is at most this value, and only if srRTT is also above sr_rtt_upper_bound:

	if ((ca->curr_srrtt > sr_rtt_upper_bound &&
	     get_ack_rate_diff(ca) <= ack_rate_diff_ss) ||

get_ack_rate_diff() clamps decreases to 0. A negative value written to this 0644 int therefore permanently disables the srRTT based LAUNCH exit. Should it be bounds checked the same way beta is?

[ ... ]

net/ipv4/tcp_roccet.c line 457

[Severity: Low]
Will this link on 32-bit architectures? This divides a u64 by 100 with a plain '/'. The compiler turns that into a libgcc call (__udivdi3 on i386, __aeabi_uldivmod on arm), and the kernel doesn't provide those. The module link would then fail with an undefined reference.

TCP_CONG_ROCCET has no architecture dependency, so 32-bit allmodconfig builds will enable it. The rest of the file already uses div_u64() and do_div(). Would div_u64(..., 100) work here?

[ ... ]

net/ipv4/tcp_roccet.c line 586

[Severity: Medium]
Can this raise cwnd instead of lowering it?

roccet_congestion_event() and roccet_handle_recovery() let cwnd drop as low as 2, so a cwnd between 2 and 9 is reachable in ORBITER. With cwnd 4, probe_cwnd becomes TCP_INIT_CWND (10). That adds queueing right after curr_min_rtt has been reset to U32_MAX, while the new minimum is being measured.

The same TCP_INIT_CWND floor is used for the LAUNCH exit in roccet_launch_update():

	tcp_snd_cwnd_set(tp, max(tcp_snd_cwnd(tp) / 2,
				 TCP_INIT_CWND));

and in roccet_state() for TCP_CA_Loss:

	tcp_snd_cwnd_set(tp, TCP_INIT_CWND);

None of these writes applies tp->snd_cwnd_clamp, unlike roccet_congestion_event() and tcp_cong_avoid_ai(). A locked route cwnd metric can set the clamp below 10 in tcp_init_metrics():

	if (tcp_metric_locked(tm, TCP_METRIC_CWND))
		tp->snd_cwnd_clamp = tcp_metric_get(tm, TCP_METRIC_CWND);

BPF can do the same. Is that clamp then exceeded on probe entry, LAUNCH exit and RTO?

[ ... ]

net/ipv4/tcp_roccet.c line 682

[Severity: Medium]
Is this enough idle handling for a .cong_control module? CUBIC can get away with only shifting epoch_start, because the core restarts cwnd after idle and applies RFC 7661 validation for it. Both of those core paths are skipped when cong_control is set:

include/net/tcp.h:tcp_slow_start_after_idle_check() {
	if (!READ_ONCE(sock_net(sk)->ipv4.sysctl_tcp_slow_start_after_idle) ||
	    tp->packets_out || ca_ops->cong_control)
		return;
	...
}

net/ipv4/tcp_output.c:tcp_cwnd_validate() {
	...
		if (READ_ONCE(sock_net(sk)->ipv4.sysctl_tcp_slow_start_after_idle) &&
		    (s32)(tcp_jiffies32 - tp->snd_cwnd_stamp) >= inet_csk(sk)->icsk_rto &&
		    !ca_ops->cong_control)
			tcp_cwnd_application_limited(sk);
	...
}

As a result, net.ipv4.tcp_slow_start_after_idle has no effect for ROCCET. After an idle period longer than RTO, the connection resumes with its full stale cwnd, paced only over the old srtt.

No other ROCCET path lowers cwnd based on idle time. The idle reset in update_ack_rate() only clears counters.

[ ... ]

net/ipv4/tcp_roccet.c line 965

[Severity: Medium]
Does this comparison do what's intended? (s32)(sent - received) is compared with the u64 that div_u64() returns. The usual arithmetic conversions turn the s32 into a u64, so any negative difference becomes a value near 2^64 and sent_more_than_acked comes out true.

sent - received is the in-flight bytes now minus the in-flight bytes at the start of the interval. It goes negative whenever the flight shrinks:

- after a cwnd reduction (DRAIN lasts only 100ms, but the flight keeps shrinking for about an RTT)
- during Recovery, when snd_nxt stalls
- when the sender is application limited

In those cases, can the srRTT branch below fire another roccet_congestion_event() while the queue is actually draining?

The growth check has the same input:

	if (!tcp_is_cwnd_limited(sk) || sent_more_than_acked ||
	    tcp_in_cwnd_reduction(sk))
		return;

Does it then block CUBIC growth right when growth should resume?

[ ... ]

net/ipv4/tcp_roccet.c line 994

[Severity: Medium]
Can a single ACK reduce cwnd twice when Recovery starts? tcp_ack() runs tcp_fastretrans_alert() before tcp_cong_control(). For a flow in ORBITER:

tcp_fastretrans_alert()
tcp_enter_recovery()
tcp_set_ca_state(sk, TCP_CA_Recovery)
roccet_state()
roccet_handle_recovery() /* cwnd = beta * cwnd */
tcp_cong_control()
roccet_control()
roccet_orbiter_update()
roccet_congestion_event() /* cwnd = beta * cwnd again */

roccet_state() makes the first cut. tcp_set_ca_state() calls set_state before it updates icsk_ca_state, so tcp_in_cwnd_reduction() is still false there. The Recovery path doesn't change the ROCCET state, set roccet_last_event_time_us, or push out next_srrtt_check_ts.

Suppose the srRTT deadline has passed, curr_srrtt is above roccet_xj, and sent_more_than_acked is true. It is often true in Recovery because of the signed/unsigned comparison above. Then roccet_congestion_event() runs here. The tcp_in_cwnd_reduction() check in roccet_orbiter_update() comes after this branch and only guards growth. Later ACKs during Recovery can hit the same branch.

With the default beta, cwnd 100 would go to 70 and then to 49. The v9 changelog item "Avoid double-cwnd-reduction via tcp_in_cwnd_reduction check" covers the roccet_state() path, but not this one.

[ ... ]

net/ipv4/tcp_roccet.c line 1080

[Severity: Medium]
On loss, the commit message only says that ROCCET "reacts to packet loss by adapting CUBIC's behavior". Is it intended that a loss in LAUNCH never reduces cwnd?

When Recovery starts in LAUNCH, roccet_recalc_ssthresh() returns the unreduced cwnd. roccet_handle_recovery() then switches to ORBITER and keeps the same cwnd. roccet_last_event_time_us isn't set, so no DRAIN follows. roccet_orbiter_update() only blocks growth during Recovery, so the slow start overshoot window survives the first recovery episode. Since .cong_control is set, tcp_cong_control() returns before the core reduction runs, so nothing else lowers it either.

On RTO, tcp_enter_loss() sets cwnd to packets_in_flight + 1, normally 1, which is the RFC 5681 loss window. roccet_state(TCP_CA_Loss) then overwrites it with TCP_INIT_CWND. If the RTO happens in LAUNCH, ssthresh also stays at the full overshoot cwnd. roccet_recalc_ssthresh() returned cwnd, and roccet_state() only applies max(ssthresh, TCP_INIT_CWND). Does the flow then slow start straight back to the window that caused the timeout?

These deviations are only described in two places. One is the file header ("LAUNCH, where loss is not considered as a congestion event"). The other is the changelog below the --- line ("Increase cwnd back to TCP_CWND_INIT on RTO"), which is dropped when the patch is applied. These are departures from RFC 5681 and RFC 9438. Could the commit message describe them?

[ ... ]

net/ipv4/tcp_roccet.c line 1176

[Severity: Medium]
What happens here if the RTO turns out to be spurious? roccet_reset() puts ROCCET into LAUNCH and clears last_max_cwnd and delay_min. Suppose F-RTO later undoes the loss:

tcp_process_loss()
tcp_try_undo_loss()
tcp_undo_cwnd_reduction() /* cwnd and ssthresh restored */
tcp_set_ca_state(sk, TCP_CA_Open) /* ignored by roccet_state() */

tcp_reno_undo_cwnd() restores cwnd to max(cwnd, prior_cwnd), and ssthresh goes back to prior_ssthresh. In congestion avoidance that leaves cwnd >= ssthresh while ROCCET is still in LAUNCH.

On the same ACK, roccet_control()->roccet_launch_update() sees !tcp_in_slow_start(tp) and takes the exit branch:

	tcp_snd_cwnd_set(tp, max(tcp_snd_cwnd(tp) -
				 (tcp_snd_cwnd(tp) / 3),
				 TCP_INIT_CWND));
	WRITE_ONCE(tp->snd_ssthresh, tcp_snd_cwnd(tp));

Does this cut the restored window by about a third, and lose the CUBIC Wmax and delay history, even though the RTO was spurious?

net/ipv4/tcp_roccet.c line 1193

[Severity: Medium]
Because .cong_control is set, the core skips tcp_cwnd_reduction() (PRR). That includes the fast retransmit PRR forces when recovery starts. tcp_rack_reo_timeout() also leaves this to the module:

	if (!inet_csk(sk)->icsk_ca_ops->cong_control)
		tcp_cwnd_reduction(sk, 1, tp->lost - lost, 0);

Here cwnd is set straight to about 0.7 * cwnd, with no packet conservation step (BBR uses in_flight + acked). roccet_orbiter_update() then blocks growth while tcp_in_cwnd_reduction() is true. Can this delay, or stall, the first retransmission?

tcp_xmit_retransmit_queue() stops while in_flight >= cwnd:

	segs = tcp_snd_cwnd(tp) - tcp_packets_in_flight(tp);
	if (segs <= 0)
		break;

tcp_schedule_loss_probe() only arms TLP in Open or CWR.

With a single loss, the first retransmit waits until SACKs bring in_flight below 0.7 * cwnd, roughly 30% of an RTT.

With heavy tail loss that isn't marked lost yet, it stalls completely. For example: cwnd 100, packet 10 lost, 11..30 SACKed, 31..100 lost, so in_flight == 70 == cwnd. Then no retransmission or probe goes out until the RTO, which also resets all ROCCET state. PRR would have sent one forced retransmit in this case.

net/ipv4/tcp_roccet.c line 1227

[Severity: Medium]
This early return comes from CUBIC, but here it also skips the curr_rtt and last_rtt updates below. Is that intended?

bictcp_update() starts a new epoch on the first ORBITER growth after any
reduction: DRAIN to ORBITER, Recovery, CWR, Loss, or LAUNCH exit. roccet_cwnd_event_tx_start() can also clamp epoch_start to now after idle. For one second after that, curr_rtt and last_rtt stay frozen.

Meanwhile, roccet_control() keeps calling update_min_rtt() and update_srrtt() on every ACK with acked_sacked > 0. The srRTT EWMA and the jitter term then converge on RTT data up to a second old. That could trigger or suppress srRTT congestion events.

The min RTT probe looks affected too. roccet_enter_min_rtt_probe() resets curr_min_rtt to U32_MAX but leaves epoch_start alone. If the probe starts less than a second after an epoch start, every probe sample is discarded. update_min_rtt() then copies the stale, pre-probe curr_rtt into curr_min_rtt. Would the probe record an inflated minimum RTT in that case?

[ ... ]

net/ipv4/tcp_roccet.c line 1303

[Severity: Low]
Is jiffies_to_usecs(tcp_jiffies32) safe as a wrapping timestamp for time_between32() and time_after32()? It is only linear mod 2^32 when HZ divides USEC_PER_SEC. For HZ values such as 300, it uses the out-of-line version in kernel/time/time.c:

#if BITS_PER_LONG == 32
	return (HZ_TO_USEC_MUL32 * j) >> HZ_TO_USEC_SHR32;
#else
	return (j * HZ_TO_USEC_NUM) / HZ_TO_USEC_DEN;

#endif

That result is not congruent mod 2^32. tcp_jiffies32 wraps 5 minutes after boot (because of INITIAL_JIFFIES) and then every 2^32 jiffies. At each wrap, now jumps by about 1.43e9 us.

Wouldn't all of ROCCET's time windows then misfire at once, for every socket? That covers the DRAIN exit, the ack rate idle reset, the next_min_rtt_probe expiry (a spurious probe that halves cwnd), the next_srrtt_check_ts evaluation, the probe and refill timers, and the 500ms initial limit check.

BBR avoids this by using tp->tcp_mstamp.

[ ... ]

Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/arqDuiD4Q4GWOFBc%40volt-roccet-vm