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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] tcp_roccet: struct roccettcp exceeds ICSK_CA_PRIV_SIZE causing BUILD_BUG_ON
- [Medium] tcp_roccet: ca->next_min_rtt_probe type confusion (jiffies vs usecs) on LAUNCH transition
--

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

TCP ROCCET is a new congestion control algorithm based on TCP CUBIC that
improves its overall performance in cellular networks. It relies on two
additional metrics (relative increase in RTT from its minimum and
acknowledgment arrival rate) to determine the send rate and avoid bufferbloat.
It features a modified slow start and specific minimum RTT probing logic.

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
> index 0000000000000..04b9e52466126
> --- /dev/null
> +++ b/net/ipv4/tcp_roccet.c
> @@ -0,0 +1,1408 @@
[ ... ]
> +struct roccettcp {
> +	u32 cnt;	/* increase cwnd by 1 after ACKs */
> +	u32 last_max_cwnd;	/* last maximum snd_cwnd */
> +	u32 last_cwnd;	/* the last snd_cwnd */
> +	u32 last_time;	/* time when updated last_cwnd */
> +	u32 bic_origin_point;	/* origin point of bic function */
> +	u32 bic_K;	/* time to origin point from the
> +			 * beginning of the current epoch
> +			 */
> +	u32 delay_min;	/* min delay (usec) */
> +	u32 epoch_start;	/* beginning of an epoch */
> +	u32 ack_cnt;	/* number of acks */
> +	u32 tcp_cwnd;	/* estimated tcp cwnd */
> +	u32 curr_rtt;	/* last sample rtt of current round */
> +
> +	u32 roccet_last_event_time_us;	/* The last time ROCCET was triggered */
> +	u32 curr_min_rtt;	/* The current observed minRTT */
> +	u32 next_min_rtt_probe;	/* Next time to probe the minRTT */
> +	u32 probe_min_rtt_until;	/* End of minRTT probing period.
> +					 * Set while in RTT_PROBE states
> +					 */
> +	u32 refill_until;	/* End of pipe refill after minRTT probe.
> +				 * Set while in RTT_PROBE states
> +				 */
> +	u32 cwnd_before_min_rtt_probe;	/* cwnd before min RTT probing. */
> +	u32 curr_srrtt;	/* srRTT calculated based on the latest ACK */
> +	u32 next_srrtt_check_ts;	/* Next check for srRTT */
> +	u32 last_rtt;	/* sample rtt of previous round.
> +			 * Used for jitter calculation
> +			 */
> +
> +	u32 interval_snd_seq_start;
> +	u32 interval_una_seq_start;
> +
> +	u32 ack_carry_over;	/* Used to carry over leftover acks from
> +				 * LAUNCH to ORBITER
> +				 */
> +
> +	u32 last_ack_rate_time;	/* Timestamp of the last ACK-rate */
> +	u16 ack_rate_last_rate;	/* Last ACK-rate */
> +	u16 curr_ack_rate;	/* Current ACK-rate */
> +	u16 ack_rate_cnt;	/* Used for counting acks */
> +
> +	enum roccet_state state : 3; /* Current operating state of roccet */
> +	bool initial_limit_reached: 1;	/* Set to true after the connection
> +					 * initially gets cwnd-limited
> +					 */
> +	bool is_in_initial_launch: 1;	/* true if the connection is in
> +					 * the initial launch phase.
> +					 */
> +	bool last_event_time_set: 1;	/* true if the last event time
> +					 * has been set/initialized.
> +					 */
> +	bool rtt_probe_timers_set: 1;	/* true if refill_until and
> +					 * probe_min_rtt_until are currently
> +					 * set.
> +					 */
> +};
[ ... ]
> +static void roccet_orbiter_update(struct sock *sk, u32 acked)
> +{
[ ... ]
> +	/* Enter RTT_PROBE when the "timer" has expired.
> +	 *
> +	 * Since we are in ORBITER, we should have already received at least
> +	 * one RTT sample. However safeguard against it if not.
> +	 *
> +	 * Use double the probing period as a guard space to avoid
> +	 * idle-time locking us out of probing.
> +	 */
> +	if (!time_between32(now, ca->next_min_rtt_probe
> +				 - 2 * ROCCET_NEXT_MIN_RTT_PROBE_MS
> +				 * USEC_PER_MSEC, ca->next_min_rtt_probe) &&
> +	    ca->curr_rtt != U32_MAX) {
> +		ca->state = RTT_PROBE_ENTER;
> +		return;
> +	}

[Severity: Medium]
Could this condition falsely trigger a premature RTT probe if the connection
was previously idle?

In roccet_enter_min_rtt_probe(), ca->next_min_rtt_probe is assigned a raw
jiffies timestamp to track the probe epoch. Normal state transitions out of
probing explicitly reinitialize next_min_rtt_probe to a usecs timestamp.

If the connection then enters ORBITER from LAUNCH, this block compares a usecs
timestamp (now) against the residual jiffies timestamp. Since usecs are vastly
larger, won't this check instantly fail and incorrectly re-enter probing?

[ ... ]
> +static void roccet_handle_state_transitions(struct sock *sk,
> +					    enum roccet_state prev_state,
> +					    u32 now)
> +{
[ ... ]
> +	/* Whenever we leave the min RTT probing states (and not just
> +	 * transition between them), we want to reset the probing timers.
> +	 */
> +	if ((prev_state == RTT_PROBE_ENTER || prev_state == RTT_PROBE ||
> +	     prev_state == RTT_PROBE_REFILL) &&
> +	    (ca->state != RTT_PROBE && ca->state != RTT_PROBE_REFILL)) {
> +		ca->probe_min_rtt_until = 0;
> +		ca->refill_until = 0;
> +		ca->rtt_probe_timers_set = false;
> +	}
> +}

[Severity: Medium]
When the connection goes idle during probing, roccet_cwnd_event_tx_start()
forces an immediate transition to LAUNCH. This cleanup logic handles clearing
the probe timers, but does it miss reinitializing next_min_rtt_probe to usecs
for that transition?

[ ... ]
> +static int __init roccet_register(void)
> +{
> +	int param_err;
> +
> +	BUILD_BUG_ON(sizeof(struct roccettcp) > ICSK_CA_PRIV_SIZE);

[Severity: High]
Does this assertion fail unconditionally because the struct exceeds 104 bytes?

struct roccettcp contains 24 u32 fields (96 bytes) and 3 u16 fields (6 bytes),
consuming 102 bytes. The next field is the enum roccet_state bitfield, which
forces 2 bytes of padding after the u16 fields, placing the bitfield block at
offset 104, pushing the total struct size to 108 bytes.

Since ICSK_CA_PRIV_SIZE is exactly 104 bytes, won't this break compilation on
standard architectures?

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