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

On Mon, Aug 31, 2026 at 7:04 PM Tim Fuechsel [off-list ref] wrote:

TCP ROCCET is an new congestion control algorithm based on TCP CUBIC
that improves its overall performance in cellular networks. By its mode
of function, CUBIC causes bufferbloat while it tries to detect the
available throughput of a network path. This is particularly a problem
with large buffers in mobile networks. A more detailed description and
analysis of this problem caused by TCP CUBIC can be found in [1].
TCP ROCCET addresses the bufferbloat problem by adding two additional
metrics to detect bufferbloat. The first metric is the relative increase
in RTT from its minimum, the srRTT. Here, bufferbloat can be detected
when RTTs increase due to buffer filling. The second metric is the
acknowledgment arrival rate sampled over 100ms intervals. If CUBIC
increases the send rate or congestion window, and the acknowledgment
arrival rate stays on the same level, the connection is limited by the
bottleneck link's capacity. In such cases, ROCCET reduces the send rate
to prevent bufferbloat. ROCCET uses a modified version of slow start
rather than HyStart because HyStart is known to enter the congestion
avoidance phase too early when used in cellular networks [2]. Therefore,
ROCCET uses a combination of srRTT and the acknowledgment arrival rate to
determine when to exit slow start. For the congestion avoidance phase,
ROCCET relies on the srRTT and monitoring the send and received Bytes
to detect the filling of the bottleneck buffer.

In real-world mobile 5G NR measurements, TCP ROCCET achieves better
performance than CUBIC and BBRv3, by maintaining similar throughput
while reducing the latency. In stationary 5G NR scenarios, the performance
is similar to that of BBRv3. More information about TCP ROCCET and
measurement evaluations can be found here [3].
For the version (2026-07-21) we provide additional performance
evaluation regarding throughput, latency and bandwidth share [4].

[1] https://doi.org/10.1109/VTC2023-Fall60731.2023.10333357
[2] https://doi.org/10.1109/WMNC.2016.7543932
[3] https://doi.org/10.23919/WONS68803.2026.11501781
[4] http://go.lu-h.de/roccet-2026-07-21

Signed-off-by: Lukas Prause <redacted>
Signed-off-by: Tim Fuechsel <redacted>

Hi Lukas, Tim,

Here is a AI review of your patch. There are several architectural issues,
race conditions, and logic bugs that need to be addressed.

1. Concurrency data race on module parameters in socket init
------------------------------------------------------------
In roccettcp_init():
    param_check(true);
    param_precompute();

Both functions mutate global static variables (beta, bic_scale, beta_scale,
cube_rtt_scale, cube_factor) that are marked __read_mostly.
Calling this in roccettcp_init() on every socket creation causes unsynchronized
concurrent writes across CPUs. Furthermore, param_check() calls pr_err() /
pr_info(), which will spam the kernel log on every connection creation if an
invalid parameter was configured.

Global scale factors should only be precomputed at module init time
(roccettcp_register), or validated using kernel_param_ops.


2. Bypassing PRR and broken loss recovery via .cong_control
----------------------------------------------------------
By defining .cong_control = roccet_control, TCP core completely delegates
congestion control and skips PRR (tcp_cwnd_reduction()).

However, in roccet_control():
    if (!ca->is_in_recovery)
        roccettcp_cong_avoid(sk, ack, rs->acked_sacked);

During recovery (TCP_CA_Recovery), ROCCET does nothing to regulate cwnd or
clock out retransmissions according to RFC 6937. Unless you have a full
custom recovery engine (like BBR), you should stick to .cong_avoid so the
stack handles PRR and loss recovery properly.


3. Broken .ssthresh callback and ignored ECN (TCP_CA_CWR)
---------------------------------------------------------
roccettcp_ssthresh() simply returns tp->snd_ssthresh without recalculation:
    static u32 roccettcp_ssthresh(struct sock *sk)
    {
        return tcp_sk(sk)->snd_ssthresh;
    }

The recalculation was moved to roccettcp_state(..., TCP_CA_Recovery).
This breaks the TCP stack contract:
a) tcp_init_cwnd_reduction() initializes PRR state using the return value of
   icsk_ca_ops->ssthresh(sk). Returning the unreduced ssthresh corrupts PRR.
b) When ECN CE marks arrive, tcp_enter_cwr() transitions to TCP_CA_CWR and calls
   icsk_ca_ops->ssthresh(sk). Since roccettcp_state() only checks for Loss and
   Recovery, TCP_CA_CWR is completely ignored. Consequently, neither ssthresh
   nor cwnd is ever reduced on ECN.


4. Congestion avoidance starvation in ORBITER
---------------------------------------------
In roccettcp_cong_avoid():
    send = tp->snd_nxt - ca->interval_snd_seq_start;
    received = tp->snd_una - ca->interval_una_seq_start;
    send_more_than_acked =
        send > received + ((tcp_snd_cwnd(tp) * tp->mss_cache) / 100);
    ...
    if (!tcp_is_cwnd_limited(sk) || send_more_than_acked)
        return;

During steady-state transmission, the in-flight byte count (send - received)
is approximately cwnd * mss. This is ~100x larger than the 1% guard space
((cwnd * mss) / 100). Thus, send_more_than_acked evaluates to true on almost
every ACK, returning before bictcp_update() or tcp_cong_avoid_ai() can run.
Window growth in congestion avoidance is completely starved.


5. curr_min_rtt poisoning and premature slow-start exit
-------------------------------------------------------
In roccettcp_reset(), ca->curr_rtt is 0 and ca->curr_min_rtt is ~0U.
In update_min_rtt():
    if (ca->curr_rtt < ca->curr_min_rtt) {
        ca->curr_min_rtt = max(ca->curr_rtt, 1);
        ...

If an ACK arrives before an RTT sample is collected (e.g. sample->rtt_us <= 0),
ca->curr_rtt is 0. ca->curr_min_rtt is set to max(0, 1) = 1 us.
Because 1 us is smaller than any real path RTT, ca->curr_min_rtt remains stuck
at 1 us.
Then in update_srrtt():
    u32 rrtt = div_u64(100 * (u64)(ca->curr_rtt - ca->curr_min_rtt),
                       ca->curr_min_rtt);
For a 20 ms RTT (20000 us), rrtt evaluates to 1,999,900. curr_srrtt immediately
exceeds sr_rtt_upper_bound and terminates slow start on the very first sample.


6. Perpetual min-RTT probe loop
-------------------------------
In roccet_min_rtt_probe(), when the probe finishes:
    ca->probe_min_rtt_until = 0;
    ca->state = ORBITER;

ca->next_min_rtt_probe is not updated when exiting the probe. If no new lower
RTT was found during the probe, next_min_rtt_probe remains in the past.
On the next ACK, after(now, ca->next_min_rtt_probe) immediately evaluates to
true, trapping the flow in a continuous probe loop and repeatedly halving cwnd
down to TCP_INIT_CWND.


7. 71-minute timestamp wrap and permanent DRAIN state
-----------------------------------------------------
now is calculated via jiffies_to_usecs(tcp_jiffies32), which wraps every
~71.5 minutes (2^32 us).

In roccet_control():
    else if ((s32)now - ca->roccet_last_event_time_us <= 100 * USEC_PER_MSEC)
        ca->state = DRAIN;

ca->roccet_last_event_time_us is initialized to 0. When bit 31 of now is set
(for 35.7 minutes out of every 71.5 minutes), (s32)now is negative, so
(s32)now - 0 is negative and always <= 100000. Newly created connections during
that 35-minute window will immediately enter and remain permanently stuck in
DRAIN state.

Also, before() and after() from <net/tcp.h> are sequence number comparison
macros (32-bit TCP seqno), not time comparison macros.


8. Re-implementing tcp_update_pacing_rate()
-------------------------------------------
roccet_control() duplicates the pacing rate calculation from tcp_input.c and
hardcodes 100% pacing instead of honoring sysctl_tcp_pacing_ca_ratio. If you
switch to .cong_avoid, the core stack will manage pacing updates automatically.


9. Coding style & formatting
----------------------------
- Declarations after statements (ISO C90 violations) in roccettcp_init(),
  update_srrtt(), roccettcp_acked(), and roccettcp_register().
- C++ style comments (//) instead of /* ... */ in multiple places.
- Commented-out dead code: //WRITE_ONCE(sk->sk_pacing_rate, 0);
- Typos in comments ("sRrTT", "probeing", "has was idle", "defer", etc.)
  and references to non-existent struct fields ("curr_min_rtt_timed.rtt").

Thanks.