tcp: Add TCP ROCCET congestion control module.
Subsystems (4)
PATCH v6 1/1 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. 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's main method of addressing the bufferbloat problem is by adding two additional metrics. 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 and the bytes sent and received over 5 RTTs. If CUBIC increases the send rate or congestion window, and the acknowledgment arrival rate stays at 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 sent and received bytes to detect the filling of the bottleneck buffer. To keep the minimum RTT updated, ROCCET performs minimum RTT probing if no new minimum RTT was found in the last 5 seconds. To this end ROCCET halves the cwnd and probes for 200ms but at least for one RTT, for a new minimum RTT. After probing, ROCCET refills the pipe carefully. In addition, ROCCET reacts to packet loss by adapting CUBIC's behavior. ECNs are handled as ROCCET congestion event, where the cwnd gets reduced and ROCCET enters a buffer drain state.
In real-world mobile 5G NR measurements, TCP ROCCET achieves better performance than CUBIC, by maintaining similar throughput while reducing the latency. 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
---
Changes since v1:
* Adjust some comments & Rework commit message
* Fix Kconfig format
* Add div-by-0 check variables
* Fix ack summation from +=1 to +=acked (counting now ACKs instead of ACK events)
* Fix wrapping in time comparisons
* Remove most module_params, including 'ignore_loss'
* Remove check that always evaluated to 'true' regarding 'bw_limit_detect'
Changes since v2:
* Improve initialization of roccettcp struct
* Always react to ECE bits and reset flag
* Detect plateaus in ACK-rate
* Fix potential overflow in ACK-rate tracking for high-speed connections
* Reset ACK-counter correctly on new interval
* Fix potential integer overflow in rRTT calculation
Changes since v3:
* Changes in comments & Kconfig help text
* Adjust commit message, adding reference to new performance evaluation
* Add minimum RTT probing
* Use cong_control callback instead of cong_avoid
* Move CA_EVENT_TX_START event check to dedicated callback (cwnd_event_tx_start)
* Add monitoring of send and receive rate as mentioned in the original paper
* Refactor of the code
* Remove kfunc exports
* Fix potential overflow in rRTT calculation by using 64-bit arithmetic
* Fix potential division-by-zero in srRTT calculation due to RTT value
* Initialize ack_rate timestamp to current time
* Fix potential double penalty when receiving ECN (ECE) mark
* Reset ack counting when connection goes idle
* Add module-parameter bounds check on register
Changes since v4:
* Remove __always_inline annotations & unused function
* Fix module parameter bounds check desync
* Add idle-check to fix connection freezing
* Exchange .ssthresh callback from recalc_ssthresh to a ssthresh-getter, fixing double penalties
* Remove unnecessary seq-no wrapping check & Remove 0 as 'uninitialized' seq-no value
* Wait for first control round to finish before starting to monitor potential slow-start exit conditions
Changes since v5:
* Complete refactor of the roccet-state-machine, improving maintainability and robustness
* Fix incorrect state transitions and state-exit conditions
* Reconsider & fix handling of RTO, CWR and Recovery states
* Fix module parameter usage & only re-calculate scaling factors on module-init
* Fix usage of .ssthresh callback and use ssthresh more consciously
* Fix incorrect coding-style
--- net/ipv4/Kconfig | 12 + net/ipv4/Makefile | 1 + net/ipv4/tcp_roccet.c | 1233 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 1246 insertions(+) create mode 100644 net/ipv4/tcp_roccet.c
net/ipv4/Kconfig +12 -0
@@ -663,6 +663,18 @@ config TCP_CONG_CDG delay gradients." In Networking 2011. Preprint: http://caia.swin.edu.au/cv/dahayes/content/networking2011-cdg-preprint.pdf +config TCP_CONG_ROCCET + tristate "ROCCET TCP" + default n + help + TCP ROCCET (RTT oriented CUBIC congestion control exTension) is a + sender-side-only congestion control algorithm based on the TCP CUBIC + protocol stack/TCP CUBIC congestion control algorithm that optimizes + its performance. Especially for networks with large buffers + (wireless, cellular networks), TCP ROCCET has improved performance by + maintaining a similar throughput as CUBIC while reducing latency. + For more information, see: https://arxiv.org/abs/2510.25281 + config TCP_CONG_BBR tristate "BBR TCP" default n
net/ipv4/Makefile +1 -0
@@ -45,6 +45,7 @@ obj-$(CONFIG_INET_TCP_DIAG) += tcp_diag.o obj-$(CONFIG_INET_UDP_DIAG) += udp_diag.o obj-$(CONFIG_INET_RAW_DIAG) += raw_diag.o obj-$(CONFIG_TCP_CONG_BBR) += tcp_bbr.o +obj-$(CONFIG_TCP_CONG_ROCCET) += tcp_roccet.o obj-$(CONFIG_TCP_CONG_BIC) += tcp_bic.o obj-$(CONFIG_TCP_CONG_CDG) += tcp_cdg.o obj-$(CONFIG_TCP_CONG_CUBIC) += tcp_cubic.o
net/ipv4/tcp_roccet.c new file +1233 -0
1,233 lines not on this page · raw message
base-commit: 87b80c2f6b05cad9f0ff9136709c62a0f59923e3
Signature · 1 line
2.43.0
[email protected]
AI reviewThank you for your contribution! Sashiko AI review found 11 potential issue(s) to consider: - [High] tcp_roccet: ignores packet loss during LAUNCH slow-start phase - [High] tcp_roccet: timer…
Thank you for your contribution! Sashiko AI review found 11 potential issue(s) to consider:
- [High] tcp_roccet: ignores packet loss during LAUNCH slow-start phase
- [High] tcp_roccet: timer wraparound to exactly 0 causes permanent DRAIN state lockup
- [High] tcp_roccet: double cwnd reduction and last_max_cwnd corruption during TCP_CA_CWR
- [High] tcp_roccet: ACK rate metric 16-bit saturation falsely aborts slow start
- [High] tcp_roccet: forces TCP_INIT_CWND on RTO, injecting severe bursts
- [Medium] tcp_roccet: ack_carry_over from slow start exit is lost to tcp_cong_avoid_ai
- [Medium] tcp_roccet: 32-bit usecs timestamp wraps after 35 minutes, freezing idle connections
- [Medium] tcp_roccet: zero-microsecond RTT samples are discarded
- [Medium] tcp_roccet: 32-bit sequence tracking wraps in 5 RTT measurement window
- [Medium] tcp_roccet: writable sysfs parameters lack runtime bounds validation
- [Low] tcp_roccet: struct roccettcp exceeds ICSK_CA_PRIV_SIZE limit
--
commit e3cfdf8398561ab8f09dd45039b191e26c9801e8
Author: Tim Fuechsel <[email protected]>
Subject: tcp: Add TCP ROCCET congestion control module.
TCP ROCCET is a new congestion control algorithm based on TCP CUBIC that improves performance in cellular networks by mitigating bufferbloat. It adds a modified slow start and monitors RTT and acknowledgment arrival rates to adjust the send rate.
Quoted 5 lines
net/ipv4/tcp_roccet.c new file +0
-0
[ ... ]
Quoted 2 lines
net/ipv4/tcp_roccet.c new file +2
-0+struct roccettcp { +u32 cnt;/* increase cwnd by 1 after ACKs */
[ ... ]
net/ipv4/tcp_roccet.c line 121
Quoted 3 lines
net/ipv4/tcp_roccet.c new file +3
-0+u16 ack_rate_cnt;/* Used for counting acks */ + + enum roccet_state state : 3; /* Current operating state of roccet */
[Severity: Low]
Does the transition from u16 to an enum bitfield force a 4-byte alignment here?
It appears this introduces 2 bytes of padding, expanding the struct beyond 104 bytes and causing the BUILD_BUG_ON(sizeof(struct roccettcp) > ICSK_CA_PRIV_SIZE) assertion in roccet_register() to fail at compile time.
[ ... ]
Quoted 1 line
+static int beta
__read_mostly= 717; /* = 717/1024 (BICTCP_BETA_SCALE) */
[ ... ]
Quoted 1 line
+module_param(beta, int, 0644);
[Severity: Medium]
Since beta is writable by root at runtime (0644), shouldn't there be a runtime bounds validation check?
The current param_check() is only executed during initialization. If beta is written to a negative value at runtime, it could cause an unsigned integer overflow in roccet_congestion_event() during congestion window scaling calculations.
[ ... ]
Quoted 2 lines
net/ipv4/tcp_roccet.c new file +2
-0+static void update_ack_rate (struct roccettcp *ca,u32 acked,u32 now) +{
[ ... ]
net/ipv4/tcp_roccet.c line 340
Quoted 13 lines
net/ipv4/tcp_roccet.c new file +13
-0+ }else { +/* start counting for the new interval */ + 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 =min_t (u32, acked,U16_MAX ); + } + + ca->was_idle = false; + }else { +/* Cap the ack count to avoid overflow */ + ca->ack_rate_cnt =min_t (u32, ca->ack_rate_cnt + acked, +U16_MAX );
[Severity: High]
Does artificially capping this measurement to U16_MAX flatten the tracked rate on high-speed networks?
On networks exceeding ~7.8 Gbps, the connection will receive more than 65535 ACKs per 100ms interval. This saturation would constantly yield a rate difference of 0, falsely signaling that throughput has plateaued and causing roccet_launch_update() to prematurely abort slow start.
[ ... ]
Quoted 2 lines
net/ipv4/tcp_roccet.c new file +2
-0+static void roccet_congestion_event (struct sock *sk,u32 now) +{
[ ... ]
net/ipv4/tcp_roccet.c
Quoted 2 lines
net/ipv4/tcp_roccet.c new file +2
-0+ ca->epoch_start =0 ; + ca->roccet_last_event_time_us = now;
[Severity: High]
Can this assign a wrapping microsecond timestamp to a field that later treats 0 as a special uninitialized sentinel?
The 32-bit microsecond timestamp generated in roccet_control() will wrap to exactly 0 every ~6.2 days. If a congestion event occurs at that precise millisecond, roccet_last_event_time_us becomes 0.
This would cause roccet_drain_update() to bypass its interval check permanently, trapping the connection in the DRAIN state.
[ ... ]
Quoted 2 lines
net/ipv4/tcp_roccet.c new file +2
-0+static void roccet_min_rtt_probe (struct sock *sk,u32 now) +{
[ ... ]
net/ipv4/tcp_roccet.c line 541
Quoted 3 lines
net/ipv4/tcp_roccet.c new file +3
-0+if (ca->probe_min_rtt_until ==0 ) +pr_warn_once ("ROCCET: Probing time should be set" ); +else if (time_before32 (now, ca->probe_min_rtt_until))
[Severity: High]
Could this sentinel check falsely trigger if the calculated deadline wrapped to exactly 0?
Similar to the issue above, because it treats 0 as uninitialized, a probe duration that naturally lands on 0 during wraparound will mysteriously abort the min RTT probing phase.
[ ... ]
Quoted 2 lines
net/ipv4/tcp_roccet.c new file +2
-0+static void roccet_launch_update (struct sock *sk,u32 acked) +{
[ ... ]
net/ipv4/tcp_roccet.c line 815
Quoted 10 lines
net/ipv4/tcp_roccet.c new file +10
-0+/* If cwnd hits ssthresh, go to ORBITER and if any ACKs are +* leftover save them for ORBITER. +* Check via tcp_in_slow_start() in case no ACKs are left. +*/ +if (!tcp_in_slow_start (tp)) { + ca->state =ORBITER ; + ca->epoch_start =0 ; + ca->ack_carry_over = acked; + } +}
[Severity: Medium]
When leftover ACKs are saved in ack_carry_over, are they ever applied to the actual congestion window growth?
Looking at roccet_orbiter_update(), it calls bictcp_update() which adds the carryover to ack_cnt, but that is only used for TCP friendliness fallback calculations. The subsequent call to tcp_cong_avoid_ai() only takes the current round's acked value, meaning the carried-over ACKs are permanently discarded.
[ ... ]
Quoted 2 lines
net/ipv4/tcp_roccet.c new file +2
-0+static void roccet_orbiter_update (struct sock *sk,u32 acked) +{
[ ... ]
net/ipv4/tcp_roccet.c line 870
Quoted 9 lines
net/ipv4/tcp_roccet.c new file +9
-0+/* Calculate if more bytes were sent than received +* in the time interval. +* +* Handle wrap arounds by relying on unsigned subtraction. +* e.g. if snd_nxt wraps to 10 and seq_start is U32_MAX - 10, +* the subtraction will result in the value of 21. +*/ + sent = tp->snd_nxt - ca->interval_snd_seq_start; + received = tp->snd_una - ca->interval_una_seq_start;
[Severity: Medium]
Will this calculation be subject to catastrophic 32-bit sequence wrap-around on networks operating at 100+ Gbps?
Since this measures bytes transferred over a 5 RTT measurement window, transferring more than 4 Gigabytes in that time will wrap the 32-bit sequence subtraction and break the sent_more_than_acked evaluation.
[ ... ]
Quoted 2 lines
net/ipv4/tcp_roccet.c new file +2
-0+static u32 roccet_recalc_ssthresh (struct sock *sk) +{
[ ... ]
net/ipv4/tcp_roccet.c line 937
Quoted 5 lines
net/ipv4/tcp_roccet.c new file +5
-0+/* In LAUNCH, we want no reduction on loss/ECN. +* On ECN this is set later on in roccet_state() +*/ +if (ca->state ==LAUNCH ) +return cwnd;
[Severity: High]
Does forcing ssthresh to remain at the full congestion window size despite an active congestion event bypass standard congestion control safeguards?
Ignoring packet loss during the LAUNCH slow-start phase entirely prevents the sender from backing off during severe congestion, which can lead to persistent buffer overflow.
[ ... ]
Quoted 2 lines
net/ipv4/tcp_roccet.c new file +2
-0+static u32 roccet_handle_recovery (struct sock *sk) +{
[ ... ]
net/ipv4/tcp_roccet.c line 981
Quoted 5 lines
net/ipv4/tcp_roccet.c new file +5
-0+/* On loss in LAUNCH, enter ORBITER without a cwnd reduction. */ +if (ca->state ==LAUNCH ) { + ca->state =ORBITER ; +return cwnd; + }
[Severity: High]
Does returning the unreduced cwnd intentionally circumvent standard cwnd reduction upon Fast Recovery during the initial growth phase?
As above, this completely ignores network congestion signals and violates TCP congestion control safety invariants.
[ ... ]
Quoted 2 lines
net/ipv4/tcp_roccet.c new file +2
-0+static void roccet_state (struct sock *sk,u8 new_state) +{
[ ... ]
net/ipv4/tcp_roccet.c line 1036
Quoted 3 lines
net/ipv4/tcp_roccet.c new file +3
-0+if (new_state == TCP_CA_Loss) { +roccet_reset (sk, ca); +tcp_snd_cwnd_set (tp,TCP_INIT_CWND );
[Severity: High]
Does explicitly overriding the TCP core's safe cwnd reduction of 1 with TCP_INIT_CWND cause severe congestion bursts?
Injecting up to 10 packets immediately following a full Retransmission Timeout violently floods an already deeply congested network.
net/ipv4/tcp_roccet.c line 1044
Quoted 8 lines
net/ipv4/tcp_roccet.c new file +8
-0+ }else if (new_state ==TCP_CA_CWR ) { +/* Handle CWR as ROCCET congestion event, +* however afterwards always set Wmax to the current cwnd. +*/ + cwnd =tcp_snd_cwnd (tp); +roccet_congestion_event (sk, now); + ca->last_max_cwnd = cwnd; + }else if (new_state == TCP_CA_Recovery) {
[Severity: High]
Is the congestion window getting penalized twice for TCP_CA_CWR?
The TCP core already halves cwnd prior to calling this state callback. Passing the already-halved cwnd into roccet_congestion_event() triggers a redundant congestion event.
Additionally, blindly assigning this doubly-penalized value to last_max_cwnd permanently corrupts CUBIC's target growth metric.
[ ... ]
Quoted 2 lines
net/ipv4/tcp_roccet.c new file +2
-0+static void roccet_acked (struct sock *sk,const struct ack_sample *sample) +{
[ ... ]
net/ipv4/tcp_roccet.c line 1082
Quoted 6 lines
net/ipv4/tcp_roccet.c new file +6
-0+/* Get valid sample for roccet */ +if (sample->rtt_us >0 ) { + ca->last_rtt = ca->curr_rtt; + ca->curr_rtt = sample->rtt_us; + } +}
[Severity: Medium]
Does discarding valid 0-microsecond RTT samples completely disable ROCCET on ultra-low latency links?
On loopback or ultra-low latency interfaces, sub-microsecond RTTs are rounded down to 0. By ignoring them entirely instead of treating them as 1us, curr_rtt is prevented from ever initializing past U32_MAX, permanently breaking the congestion avoidance logic.
[ ... ]
net/ipv4/tcp_roccet.c line 1135
Quoted 8 lines
net/ipv4/tcp_roccet.c new file +8
-0+static void roccet_drain_update (struct sock *sk,u32 now) +{ +struct tcp_sock *tp =tcp_sk (sk); +struct roccettcp *ca =inet_csk_ca (sk); + +if (ca->roccet_last_event_time_us && +time_after32 (now, ca->roccet_last_event_time_us + +100 *USEC_PER_MSEC )) {
[Severity: Medium]
Can this condition incorrectly fail if the connection idles for more than 35.7 minutes?
Because now is a 32-bit microsecond timestamp, the subtraction in time_after32() will exceed its signed boundary if the time elapsed is greater than 2^31 microseconds. If an application idles and then resumes, it could be incorrectly trapped in the DRAIN state, freezing window growth until the timer wraps again.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/aqqGU0XfcfsI-OGy@volt-roccet-vm?part=1
Hello,
thanks for the review. We have several comments on the issues and kindly ask for clarification on some of the findings.
Quoted 8 lines
[ ... ]
Quoted 2 lines
+struct roccettcp {
+ u32 cnt; /* increase cwnd by 1 after ACKs */[ ... ]
Quoted 3 lines
+ u16 ack_rate_cnt; /* Used for counting acks */
+
+ enumroccet_statestate : 3; /* Current operating state of roccet */
Quoted 7 lines
[Severity: Low]
Does the transition from u16 to an enum bitfield force a 4-byte alignment here?It appears this introduces 2 bytes of padding, expanding the struct beyond 104 bytes and causing the
BUILD_BUG_ON(sizeof(struct roccettcp) > ICSK_CA_PRIV_SIZE)assertion inroccet_register()to fail at compile time.
We will fix this by switching some variables to do the alignment.
Quoted 5 lines
[ ... ]
Quoted 1 line
+static int beta
__read_mostly= 717; /* = 717/1024 (BICTCP_BETA_SCALE) */[ ... ]
Quoted 1 line
+module_param(beta, int, 0644);
Quoted 8 lines
[Severity: Medium]
Since beta is writable by root at runtime (0644), shouldn't there be a runtime bounds validation check?The current
param_check()is only executed during initialization. If beta is written to a negative value at runtime, it could cause an unsigned integer overflow inroccet_congestion_event()during congestion window scaling calculations.
This is part of CUBIC's code, but we will adjust it.
Quoted 18 lines
[ ... ]
Quoted 2 lines
(quoted diff) +2
-0+static void update_ack_rate(struct roccettcp *ca, u32 acked, u32 now) +{[ ... ]
Quoted 5 lines
+ } else {
+ /* start counting for the new interval */ +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;Quoted 8 lines
+
ca->ack_rate_cnt=min_t(u32, acked, U16_MAX); + }
+
+ca->was_idle= false;
+ } else {
+ /* Cap the ack count to avoid overflow */
+ca->ack_rate_cnt=min_t(u32, ca->ack_rate_cnt + acked, + U16_MAX);
Quoted 8 lines
[Severity: High]
Does artificially capping this measurement toU16_MAXflatten the tracked rate on high-speed networks?On networks exceeding ~7.8 Gbps, the connection will receive more than 65535 ACKs per 100ms interval. This saturation would constantly yield a rate difference of 0, falsely signaling that throughput has plateaued and causing
roccet_launch_update()to prematurely abort slow start.
We discussed this in a previous review. This case is unlikely for cellular links, but it might happen in other networks. Here, this causes no harm because ROCCET would exit LAUNCH, without fully utilising the pipe, and enter the congestion avoidance phase.
Quoted 11 lines
[ ... ]
Quoted 2 lines
(quoted diff) +2
-0+static void roccet_congestion_event(struct sock *sk, u32 now) +{[ ... ]
Quoted 2 lines
+
ca->epoch_start= 0;
+ca->roccet_last_event_time_us= now;[Severity: High]
Can this assign a wrapping microsecond timestamp to a field that later treats 0 as a special uninitialized sentinel?
Quoted 6 lines
The 32-bit microsecond timestamp generated in
roccet_control()will wrap to exactly 0 every ~6.2 days. If a congestion event occurs at that precise millisecond,roccet_last_event_time_usbecomes 0.This would cause
roccet_drain_update()to bypass its interval check permanently, trapping the connection in the DRAIN state.
We will investigate this and fix it, but we think it is really unlikely to occur. For this, the socket has to be a lifetime of ~6.2 days, and the event has a vulnerability period of 1ms.
Quoted 8 lines
[ ... ]
Quoted 2 lines
(quoted diff) +2
-0+static void roccet_min_rtt_probe(struct sock *sk, u32 now) +{[ ... ]
Quoted 3 lines
+ if (
ca->probe_min_rtt_until== 0)
+pr_warn_once("ROCCET: Probing time should be set"); + else if (time_before32(now, ca->probe_min_rtt_until))
Quoted 7 lines
[Severity: High]
Could this sentinel check falsely trigger if the calculated deadline wrapped to exactly 0?Similar to the issue above, because it treats 0 as uninitialized, a probe duration that naturally lands on 0 during wraparound will mysteriously abort the min RTT probing phase.
We will investigate this and fix it.
Quoted 19 lines
[ ... ]
Quoted 2 lines
(quoted diff) +2
-0+static void roccet_launch_update(struct sock *sk, u32 acked) +{[ ... ]
Quoted 10 lines
+ /* If cwnd hits ssthresh, go to ORBITER and if any ACKs are + * leftover save them for ORBITER.
+ * Check viatcp_in_slow_start()in case no ACKs are left. + */
+ if (!tcp_in_slow_start(tp)) {
+ca->state= ORBITER;
+ca->epoch_start= 0;
+ca->ack_carry_over= acked;
+ }
+}[Severity: Medium]
When leftover ACKs are saved inack_carry_over, are they ever applied to the actual congestion window growth?
Quoted 5 lines
Looking at
roccet_orbiter_update(), it callsbictcp_update()which adds the carryover toack_cnt, but that is only used for TCP friendliness fallback calculations. The subsequent call totcp_cong_avoid_ai()only takes the current round's acked value, meaning the carried-over ACKs are permanently discarded.
Yes, we will fix it.
Quoted 14 lines
[ ... ]
Quoted 2 lines
net/ipv4/tcp_roccet.c new file +2
-0+static void roccet_orbiter_update (struct sock *sk,u32 acked) +{[ ... ]
Quoted 9 lines
+ /* Calculate if more bytes were sent than received + * in the time interval.
+ *
+ * Handle wrap arounds by relying on unsigned subtraction. + * e.g. ifsnd_nxtwraps to 10 andseq_startisU32_MAX- 10, + * the subtraction will result in the value of 21. + */
+ sent =tp->snd_nxt-ca->interval_snd_seq_start;
+ received =tp->snd_una-ca->interval_una_seq_start;
Quoted 7 lines
[Severity: Medium]
Will this calculation be subject to catastrophic 32-bit sequence wrap-around on networks operating at 100+ Gbps?Since this measures bytes transferred over a 5 RTT measurement window, transferring more than 4 Gigabytes in that time will wrap the 32-bit sequence subtraction and break the
sent_more_than_ackedevaluation.
To be honest, I do not know what to answer here. The whole TCP stack is not suitable to maintain 100+ Gbps with a single flow.
For these data rates, other technologies are used. I am not even sure if it is possible at the moment to reach these data rates with a single sender.
Quoted 10 lines
[ ... ]
Quoted 2 lines
(quoted diff) +2
-0+static u32 roccet_recalc_ssthresh(struct sock *sk) +{[ ... ]
Quoted 5 lines
+ /* In LAUNCH, we want no reduction on loss/ECN. + * On ECN this is set later on in
roccet_state()+ */
+ if (ca->state== LAUNCH)
+ return cwnd;
Quoted 7 lines
[Severity: High]
Does forcing ssthresh to remain at the full congestion window size despite an active congestion event bypass standard congestion control safeguards?Ignoring packet loss during the LAUNCH slow-start phase entirely prevents the sender from backing off during severe congestion, which can lead to persistent buffer overflow.
That is quite correct. On packet loss in LAUNCH, we switch to the congestion avoidance phase, ORBITER. Packet loss is not ignored here.
Quoted 10 lines
[ ... ]
Quoted 2 lines
(quoted diff) +2
-0+static u32 roccet_handle_recovery(struct sock *sk) +{[ ... ]
Quoted 5 lines
+ /* On loss in LAUNCH, enter ORBITER without a cwnd reduction. */ + if (
ca->state== LAUNCH) {
+ca->state= ORBITER;
+ return cwnd;
+ }
Quoted 6 lines
[Severity: High]
Does returning the unreduced cwnd intentionally circumvent standard cwnd reduction upon Fast Recovery during the initial growth phase?As above, this completely ignores network congestion signals and violates TCP congestion control safety invariants.
That's incorrect. ROCCET is reacting to loss in LAUNCH and ORBITER. In LAUNCH, the slow start is aborted, and ROCCET enters the congestion avoidance phase. And in the congestion avoidance phase, ROCCET reacts to loss like CUBIC.
We do not reduce the cwnd on loss in LAUNCH because loss in the beginning of a TCP connection is mostly not related to congestion in cellular networks. The detected loss mostly comes from layer 2 mechanisms. In other networks, we would detect a loss instantly after leaving LAUNCH and adjust the cwnd.
Quoted 8 lines
[ ... ]
Quoted 2 lines
(quoted diff) +2
-0+static void roccet_state(struct sock *sk, u8 new_state) +{[ ... ]
Quoted 3 lines
+ if (
new_state==TCP_CA_Loss) {
+roccet_reset(sk, ca);
+tcp_snd_cwnd_set(tp, TCP_INIT_CWND);
Quoted 6 lines
[Severity: High]
Does explicitly overriding the TCP core's safe cwnd reduction of 1 withTCP_INIT_CWNDcause severe congestion bursts?Injecting up to 10 packets immediately following a full Retransmission Timeout violently floods an already deeply congested network.
Setting the cwnd on an RTO to the initial value of 10 segments is a common procedure.
Quoted 12 lines
Quoted 8 lines
+ } else if (
new_state==TCP_CA_CWR) {
+ /* Handle CWR as ROCCET congestion event,
+ * however afterwards always set Wmax to the current cwnd. + */
+ cwnd =tcp_snd_cwnd(tp);
+roccet_congestion_event(sk, now);
+ca->last_max_cwnd= cwnd;
+ } else if (new_state==TCP_CA_Recovery) {[Severity: High]
Is the congestion window getting penalized twice forTCP_CA_CWR?
Quoted 6 lines
The TCP core already halves cwnd prior to calling this state callback. Passing the already-halved cwnd into
roccet_congestion_event()triggers a redundant congestion event.Additionally, blindly assigning this doubly-penalized value to
last_max_cwndpermanently corrupts CUBIC's target growth metric.
We will investigate this. But in our understanding, the TCP stack is not reducing the cwnd because we are implementing the cong_control function.
Quoted 11 lines
[ ... ]
Quoted 2 lines
(quoted diff) +2
-0+static void roccet_acked(struct sock *sk, const struct ack_sample *sample) +{[ ... ]
Quoted 6 lines
+ /* Get valid sample for roccet */
+ if (sample->rtt_us> 0) {
+ca->last_rtt=ca->curr_rtt;
+ca->curr_rtt=sample->rtt_us;
+ }
+}
Quoted 8 lines
[Severity: Medium]
Does discarding valid 0-microsecond RTT samples completely disable ROCCET on ultra-low latency links?On loopback or ultra-low latency interfaces, sub-microsecond RTTs are rounded down to 0. By ignoring them entirely instead of treating them as 1us,
curr_rttis prevented from ever initializing pastU32_MAX, permanently breaking the congestion avoidance logic.
I do not know what to answer here. The TCP stack is not feasible to handle latency below 1us.
Even on an internet path or a 10 gigabit LAN, there is no such low latency.
Quoted 14 lines
[ ... ]
net/ipv4/tcp_roccet.c line 1135
Quoted 8 lines
net/ipv4/tcp_roccet.c new file +8
-0+static void roccet_drain_update (struct sock *sk,u32 now) +{ + struct tcp_sock *tp = tcp_sk(sk); + struct roccettcp *ca = inet_csk_ca(sk); + + if (ca->roccet_last_event_time_us && + time_after32(now, ca->roccet_last_event_time_us + + 100 * USEC_PER_MSEC)) {[Severity: Medium]
Can this condition incorrectly fail if the connection idles for more than 35.7 minutes?
Quoted 5 lines
Because now is a 32-bit microsecond timestamp, the subtraction in
time_after32()will exceed its signed boundary if the time elapsed is greater than 2^31 microseconds. If an application idles and then resumes, it could be incorrectly trapped in the DRAIN state, freezing window growth until the timer wraps again.
We will fix that.
[email protected]
AI reviewThank you for your contribution! Sashiko AI review found 17 potential issue(s) to consider.
Thank you for your contribution! Sashiko AI review found 17 potential issue(s) to consider.
Critical: 0 · High: 6 · Medium: 4 · Low: 7
If you believe none of the issues are legitimate, please reply to each issue of Medium severity or higher and explain why the AI is wrong. If any of them are legitimate and you plan to respin, please let patchwork know by including "pw-bot: cr" as a separate line at the end of your reply (one such reply per series is enough).
- [High] On every RTO, roccet_state() (net/ipv4/tcp_roccet.c:1034-1036) calls roccet_reset() and then tcp_snd_cwnd_set(tp, TCP_INIT_CWND),…
- [High] In the LAUNCH state ROCCET performs no congestion response to loss at all: roccet_recalc_ssthresh() returns the unmodified cwnd…
- [High] DRAIN can become a long-lived (~35.8 minute) or permanent dead end.
- [High] A ROCCET (srRTT-triggered) congestion event in ORBITER reduces cwnd by beta but never lowers tp->snd_ssthresh:…
- [High] roccet_control() is installed as .cong_control, so tcp_cong_control() returns immediately after calling it…
- [High] LAUNCH grows the congestion window without bound for connections that are never cwnd-limited.
- [Medium] param_check() (net/ipv4/tcp_roccet.c:173-188) enforces 0 < beta < BICTCP_BETA_SCALE and bic_scale > 0, but it runs exactly once…
- [Medium] roccet_enter_min_rtt_probe() computes the probe window as max(tcp_snd_cwnd(tp) / 3, TCP_INIT_CWND) or max(tcp_snd_cwnd(tp) / 2,…
- [Medium] roccet_handle_state_transitions() starts a fresh five-RTT evaluation deadline on every transition into ORBITER…
- [Medium] A CWR-to-Recovery transition applies a second multiplicative reduction within the same recovery window.
- [Low] param_check() (net/ipv4/tcp_roccet.c:182-188) rejects only non- positive bic_scale; there is no upper bound.
- [Low] The header comment of roccet_update_pacing_rate() (net/ipv4/tcp_roccet.c:1084-1089) states the 200%/100% pacing choice…
- [Low] Six of the eight printk call sites in the new file omit the trailing newline (net/ipv4/tcp_roccet.c:364, 407, 472, 478, 540, 553,…
- [Low] The file header (net/ipv4/tcp_roccet.c:44-45) says "Unless CUBIC is enabled and congestion window is large this behaves the same as the…
- [Low] roccet_launch_update() calls tcp_slow_start(tp, acked) (net/ipv4/tcp_roccet.c:805) without establishing that precondition…
- [Low] struct roccettcp documents curr_rtt as "last sample rtt of current round" and last_rtt as "sample rtt of previous round.
- [Low] The module writes tp->snd_ssthresh with plain (unannotated) stores in roccet_congestion_event() (net/ipv4/tcp_roccet.c:449) and…
--
tcp: Add TCP ROCCET congestion control module.
This adds a new congestion control module, roccet, derived from CUBIC. It adds an srRTT metric and an ACK-arrival-rate metric, replaces HyStart with its own LAUNCH slow start, and drives everything from a .cong_control callback plus a small state machine (LAUNCH, ORBITER, DRAIN and three min-RTT-probe states).
A few questions about the new module below.
Quoted 7 lines
net/ipv4/tcp_roccet.c new file +1
-0@@ -0,0 +1,1233 @@ +// SPDX-License-Identifier: GPL-2.0
[ ... ]
Quoted 3 lines
+ * Unless CUBIC is enabled and congestion window is large
+ * this behaves the same as the original Reno.
+ */
[Severity: Low]
This isn't a bug, but this sentence looks carried over from tcp_cubic.c. TCP_CONG_ROCCET neither depends on nor selects TCP_CONG_CUBIC, and bictcp_update() is compiled into tcp_roccet.o unconditionally, so nothing here is affected by whether CUBIC is enabled. Could the sentence be dropped or reworded?
[ ... ]
Quoted 5 lines
net/ipv4/tcp_roccet.c new file +5
-0+static int fast_convergence__read_mostly =1 ; +static int beta__read_mostly =717 ;/* = 717/1024 (BICTCP_BETA_SCALE) */ +static int initial_ssthresh__read_mostly ; +static int bic_scale__read_mostly =41 ; +static int tcp_friendliness__read_mostly =1 ;
net/ipv4/tcp_roccet.c line 160
Quoted 10 lines
net/ipv4/tcp_roccet.c new file +10
-0+ +static u32 cube_rtt_scale__read_mostly ; +static u32 beta_scale__read_mostly ; +static u64 cube_factor__read_mostly ; + +/* Note parameters that are used for precomputing scale factors are read-only */ +module_param (fast_convergence, int,0644 ); +MODULE_PARM_DESC (fast_convergence,"turn on/off fast convergence" ); +module_param (beta, int,0644 ); +MODULE_PARM_DESC (beta,"beta for multiplicative increase" );
[Severity: Medium]param_check() runs exactly once, from roccet_register(), but beta stays writable at runtime with the generic integer setter, so a write to /sys/module/tcp_roccet/parameters/beta installs values the module itself declares invalid. The comment directly above says parameters used for precomputing scale factors are read-only, yet only bic_scale is 0444 while beta feeds beta_scale in param_precompute().
With beta >= BICTCP_BETA_SCALE, does every reduction turn into an increase?
return max((cwnd * beta) / BICTCP_BETA_SCALE, 2U);
and with a negative beta the u32 product wraps to a large cwnd. roccet_state() installs that value directly for TCP_CA_Recovery:
cwnd = roccet_handle_recovery(sk); tcp_snd_cwnd_set(tp, cwnd);
with no min(tp->snd_cwnd_clamp, ...) bound, unlike roccet_congestion_event(). Should the recovery result be clamped, and should beta be either 0444 or validated in a set() handler?
beta_scale is also derived once from the init-time beta, so after a runtime write the TCP-friendliness code in bictcp_update() and the reduction paths use two different generations of beta. roccet_recalc_ssthresh() reads beta twice in one computation with plain loads.
Module parameter attributes are published by mod_sysfs_setup() before do_init_module() runs. Can a write of beta == 1024 landing between param_check() and param_precompute() make (BICTCP_BETA_SCALE - beta) a zero divisor here?
One more nit: MODULE_PARM_DESC calls beta "beta for multiplicative increase" while every use multiplies cwnd or ssthresh down.
Quoted 14 lines
net/ipv4/tcp_roccet.c new file +14
-0+module_param (initial_ssthresh, int,0644 ); +MODULE_PARM_DESC (initial_ssthresh,"initial value of slow start threshold" ); +module_param (bic_scale, int,0444 ); +MODULE_PARM_DESC (bic_scale, +"scale (scaled by 1024) value for bic function (bic_scale/1024)" ); +module_param (tcp_friendliness, int,0644 ); +MODULE_PARM_DESC (tcp_friendliness,"turn on/off tcp friendliness" ); + +/* Used to check certain roccet parameters used in `param_precompute` in order +* to avoid invalid scale-related calculations. This validates the specified +* parameters or rejects them. +*/ +static int param_check (void ) +{
[ ... ]
Quoted 8 lines
net/ipv4/tcp_roccet.c new file +8
-0+if (bic_scale <=0 ) { +pr_err_once ("TCP ROCCET: bic_scale must be positive\n" ); + +return -EINVAL ; + } + +return 0 ; +}
[ ... ]
Quoted 1 line
net/ipv4/tcp_roccet.c new file +1
-0+ cube_rtt_scale = (bic_scale *10 );/* 1024*c/rtt */
[ ... ]
net/ipv4/tcp_roccet.c line 224
Quoted 3 lines
net/ipv4/tcp_roccet.c new file +3
-0+/* divide by bic_scale and by constant srtt (100ms) */ +do_div (cube_factor, bic_scale *10 ); +}
[Severity: Low]param_check() rejects only non-positive bic_scale, with no upper bound. Does any bic_scale above INT_MAX/10 (214748364) overflow the int expression bic_scale * 10 in both cube_rtt_scale and the do_div() divisor, so the module registers with corrupted growth constants (cube_factor collapsing to 0, leaving bic_K always 0)? Would an upper bound match the stated purpose of avoiding invalid scale-related calculations?
[ ... ]
net/ipv4/tcp_roccet.c line 366
Quoted 4 lines
net/ipv4/tcp_roccet.c new file +4
-0+if (ca->curr_min_rtt ==0 ) { +pr_err_once ("TCP ROCCET: Recorded curr_min_rtt is 0" ); +return ;/* skip srRTT update */ + }
[Severity: Low]
This isn't a bug, but this message has no trailing newline, unlike the two in param_check(). The same applies to the pr_warn_once() calls in roccet_min_rtt_probe_ce(), roccet_enter_min_rtt_probe() and roccet_min_rtt_probe(), and to the pr_err_once() in the default case of roccet_control(). Without a newline the text stays in the log buffer as a continuation and can be merged with unrelated output. The prefix also alternates between "TCP ROCCET: " and "ROCCET: " with no pr_fmt() defined.
[ ... ]
net/ipv4/tcp_roccet.c line 450
Quoted 9 lines
net/ipv4/tcp_roccet.c new file +9
-0+/* Reduce cwnd by beta */ +tcp_snd_cwnd_set (tp,min (tp->snd_cwnd_clamp, +max ((curr_cwnd * beta) + /BICTCP_BETA_SCALE ,2U ))); + +if (ca->state ==LAUNCH ) { +/* Set ssthresh on ECN, so that roccet is not in slow-start */ + tp->snd_ssthresh =tcp_snd_cwnd (tp); + ca->state =ORBITER ;
[Severity: Low]
This isn't a behavioural bug, but snd_ssthresh has lockless readers - for instance tcp.c reads it with READ_ONCE(tp->snd_ssthresh) - and the stack plus the other CC modules all write it with WRITE_ONCE(). roccet_init() already uses WRITE_ONCE(); should this store and the one in roccet_launch_update() do the same?
net/ipv4/tcp_roccet.c line 456
Quoted 6 lines
net/ipv4/tcp_roccet.c new file +6
-0+ }else if (ca->state ==ORBITER || ca->state ==RTT_PROBE_REFILL ) { +/* If we are in orbiter or currently refilling the pipe, +* abort the refill. +*/ + ca->state =DRAIN ; + }
[Severity: High]
In the ORBITER and RTT_PROBE_REFILL case cwnd is reduced by beta but tp->snd_ssthresh is left untouched; only the LAUNCH branch above updates it.
100 ms later roccet_drain_update() picks the next state from tcp_in_slow_start(tp), i.e. cwnd < ssthresh, which is true precisely because of the 0.7x reduction just applied:
if (tcp_in_slow_start(tp)) ca->state = LAUNCH; else ca->state = ORBITER;
Does this send the flow back into LAUNCH, where tcp_slow_start() grows cwnd exponentially again and loss is deliberately ignored, undoing within about half an RTT the reduction that was meant to drain the bottleneck buffer?
The loss and ECN paths do not have this problem, because tcp_init_cwnd_reduction() and tcp_enter_loss() lower snd_ssthresh via the .ssthresh callback. Only this internally generated event leaves cwnd and ssthresh inconsistent.
[ ... ]
net/ipv4/tcp_roccet.c line 504
Quoted 10 lines
net/ipv4/tcp_roccet.c new file +10
-0+if (!tcp_is_cwnd_limited (sk)) + probe_cwnd =max (tcp_snd_cwnd (tp) /3 ,TCP_INIT_CWND ); +else + probe_cwnd =max (tcp_snd_cwnd (tp) /2 ,TCP_INIT_CWND ); + + ca->probe_min_rtt_until = now + interval; + ca->cwnd_before_min_rtt_probe =tcp_snd_cwnd (tp); + +/* Half the cwnd to drain the buffer for probing. */ +tcp_snd_cwnd_set (tp, probe_cwnd);
[Severity: Medium]
Both branches floor the probe window at TCP_INIT_CWND, and the result is installed unconditionally. When the pre-probe cwnd is already below 10, does this drain probe raise cwnd instead of lowering it?
That case is reachable: roccet_handle_recovery() and roccet_recalc_ssthresh() floor the window at 2 while the flow stays in ORBITER, so with cwnd == 2 the probe window becomes max(2/2, 10) == 10, i.e. five times more segments in flight for at least max(200 ms, RTT) right after a congestion episode.
[ ... ]
Quoted 7 lines
net/ipv4/tcp_roccet.c new file +7
-0+static void roccet_launch_update (struct sock *sk,u32 acked) +{ +struct tcp_sock *tp =tcp_sk (sk); +struct roccettcp *ca =inet_csk_ca (sk); + +u32 now =jiffies_to_usecs (tcp_jiffies32); +
net/ipv4/tcp_roccet.c line 783
Quoted 10 lines
net/ipv4/tcp_roccet.c new file +10
-0+/* LAUNCH: Detect an exit point for tcp slow start +* in networks with large buffers of multiple BDP +* Like in cellular networks (5G, ...). +* +* Or exit LAUNCH if cwnd is too large for application layer +* data rate (tcp cwnd validation). +*/ +if ((ca->curr_srrtt > sr_rtt_upper_bound && +get_ack_rate_diff (ca) <= ack_rate_diff_ss) || + (!tcp_is_cwnd_limited (sk) && ca->initial_limit_reached)) {
[Severity: High]
The application-limited exit requires ca->initial_limit_reached, but that flag is only ever set while tcp_is_cwnd_limited(sk) is true, at the end of roccet_control():
if (tcp_is_cwnd_limited(sk)) ca->initial_limit_reached = true;
For a flow that never fills the window - request/response or interactive traffic with gaps shorter than the RTO - can the flag stay false forever, so neither LAUNCH exit fires on an uncongested path while every ACK still runs tcp_slow_start() below and raises cwnd by acked toward TCP_INFINITE_SSTHRESH and snd_cwnd_clamp?
The usual correction is not available here: tcp_cwnd_validate() skips tcp_cwnd_application_limited() when ca_ops->cong_control is set. Does that leave cwnd, and the pacing rate that roccet_update_pacing_rate() derives from it, inflated without any path evidence, allowing a large burst once the application does have data to send?
[ ... ]
net/ipv4/tcp_roccet.c line 805
Quoted 2 lines
net/ipv4/tcp_roccet.c new file +2
-0+/* If not already exiting LAUNCH, grow cwnd similar to slow-start */ + acked =tcp_slow_start (tp, acked);
[Severity: Low]tcp_slow_start() is documented for the case where cwnd is no greater than snd_ssthresh, but tcp_in_slow_start() is only checked afterwards.
After roccet_state(TCP_CA_Loss) sets cwnd to TCP_INIT_CWND while snd_ssthresh keeps the reduced value from roccet_recalc_ssthresh() (as low as 2), the next ACK arrives with cwnd > ssthresh. tcp_slow_start() then pulls cwnd down to ssthresh and its leftover-ACK arithmetic
acked -= cwnd - tcp_snd_cwnd(tp);
underflows in u32, returning roughly acked + (old_cwnd - ssthresh). Does that inflated credit end up in ca->ack_carry_over and later in ca->ack_cnt in bictcp_update()?
[ ... ]
Quoted 7 lines
net/ipv4/tcp_roccet.c new file +7
-0+/* Enter DRAIN when roccet was recently triggered */ +if (ca->roccet_last_event_time_us && +time_before32 (now, ca->roccet_last_event_time_us + +100 *USEC_PER_MSEC )) { + ca->state =DRAIN ; +return ; + }
[ ... ]
net/ipv4/tcp_roccet.c line 891
Quoted 11 lines
net/ipv4/tcp_roccet.c new file +11
-0+/* Check if it's time to evaluate the srRTT */ +if (time_after32 (now, ca->next_srrtt_check_ts)) { + evaluate_srrtt = true; + +/* reset struct and set next end of period */ + ca->next_srrtt_check_ts = now +5 * ca->curr_rtt; + +/* Reset Rate calculation */ + ca->interval_snd_seq_start = tp->snd_nxt; + ca->interval_una_seq_start = tp->snd_una; + }
[Severity: Medium]
These baselines are refreshed only when a deadline expires here, but roccet_handle_state_transitions() restarts the deadline on every transition into ORBITER without touching them:
if (ca->state == ORBITER) ca->next_srrtt_check_ts = now + 5 * ca->curr_rtt;
Does that make the deadline and the byte deltas cover different intervals? The first evaluation after LAUNCH compares snd_nxt/snd_una against the values captured in roccet_reset() at connection start, where in-flight was 0, so sent - received equals the current in-flight bytes and sent_more_than_acked is true even on an idle path. That both blocks growth
if (!tcp_is_cwnd_limited(sk) || sent_more_than_acked) return;
and can drive a congestion event on stale data. After DRAIN or min-RTT probing the stale baseline biases the other way and masks real growth.
[ ... ]
net/ipv4/tcp_roccet.c line 937
Quoted 11 lines
net/ipv4/tcp_roccet.c new file +11
-0+static u32 roccet_recalc_ssthresh (struct sock *sk) +{ +const struct tcp_sock *tp =tcp_sk (sk); +struct roccettcp *ca =inet_csk_ca (sk); +u32 cwnd =tcp_snd_cwnd (tp); + +/* In LAUNCH, we want no reduction on loss/ECN. +* On ECN this is set later on in roccet_state() +*/ +if (ca->state ==LAUNCH ) +return cwnd;
[Severity: High]
In LAUNCH this returns cwnd unchanged, so tcp_init_cwnd_reduction() and tcp_enter_loss() store snd_ssthresh == cwnd, i.e. no multiplicative decrease. roccet_handle_recovery() below also returns cwnd unchanged for LAUNCH, and because .cong_control is set, tcp_cong_control() returns before tcp_cwnd_reduction():
if (icsk->icsk_ca_ops->cong_control) {
icsk->icsk_ca_ops->cong_control(sk, ack, flag, rs);
return;
}so PRR does not reduce cwnd either. Is the loss event then absorbed entirely, with tcp_slow_start() in roccet_launch_update() continuing to grow cwnd through the episode?
This is not confined to connection start: roccet_state(TCP_CA_Loss) calls roccet_reset(), which re-enters LAUNCH, and roccet_drain_update() can also return to LAUNCH, so the loss-insensitive window recurs.
The commit message says "In addition, ROCCET reacts to packet loss by adapting CUBIC's behavior". Could it state that losses are ignored in LAUNCH, given that the ignore_loss module parameter that used to gate this was removed?
[ ... ]
Quoted 8 lines
net/ipv4/tcp_roccet.c new file +8
-0+/* On loss in LAUNCH, enter ORBITER without a cwnd reduction. */ +if (ca->state ==LAUNCH ) { + ca->state =ORBITER ; +return cwnd; + } + +return max ((cwnd * beta) /BICTCP_BETA_SCALE ,2U ); +}
[ ... ]
net/ipv4/tcp_roccet.c line 1036
Quoted 3 lines
net/ipv4/tcp_roccet.c new file +3
-0+if (new_state == TCP_CA_Loss) { +roccet_reset (sk, ca); +tcp_snd_cwnd_set (tp,TCP_INIT_CWND );
[Severity: High]tcp_enter_loss() sets cwnd = tcp_packets_in_flight(tp) + 1 and only afterwards calls tcp_set_ca_state(sk, TCP_CA_Loss), so this write is the final value. Does the flow then restart with 10 segments instead of 1 after every timeout?
With .cong_control set, tcp_cong_control() returns before tcp_cwnd_reduction(), so nothing re-imposes packet conservation, and roccet_reset() restores the same state on each consecutive RTO. CUBIC's set_state only calls bictcp_reset() and never touches cwnd. Should the loss window installed by the stack be left alone, and should the changelog describe this deviation?
net/ipv4/tcp_roccet.c line 1048
Quoted 12 lines
net/ipv4/tcp_roccet.c new file +12
-0+ }else if (new_state ==TCP_CA_CWR ) { +/* Handle CWR as ROCCET congestion event, +* however afterwards always set Wmax to the current cwnd. +*/ + cwnd =tcp_snd_cwnd (tp); +roccet_congestion_event (sk, now); + ca->last_max_cwnd = cwnd; + }else if (new_state == TCP_CA_Recovery) { +/* Directly reduce cwnd and rely on pacing */ + cwnd =roccet_handle_recovery (sk); +tcp_snd_cwnd_set (tp, cwnd); + }
[Severity: Medium]tcp_enter_recovery() skips tcp_init_cwnd_reduction() when tcp_in_cwnd_reduction(sk) is already true, but still calls tcp_set_ca_state(sk, TCP_CA_Recovery). Since this branch is unconditional, does a CWR to Recovery transition apply a second beta reduction inside the same recovery window, for instance 100 to about 70 to 49, while snd_ssthresh stays at about 70?
If the transition happens during RTT_PROBE, roccet_min_rtt_probe_ce() reduces ca->cwnd_before_min_rtt_probe a second time as well.
The changelog lists "Fix potential double penalty when receiving ECN (ECE) mark" - is this path covered by that fix?
[ ... ]
net/ipv4/tcp_roccet.c line 1082
Quoted 6 lines
net/ipv4/tcp_roccet.c new file +6
-0+/* Get valid sample for roccet */ +if (sample->rtt_us >0 ) { + ca->last_rtt = ca->curr_rtt; + ca->curr_rtt = sample->rtt_us; + } +}
[Severity: Low]
struct roccettcp documents curr_rtt as the "last sample rtt of current round" and last_rtt as the "sample rtt of previous round", but this shifts curr_rtt into last_rtt on every accepted sample with no round boundary. With several samples per RTT, is the jitter term roccet_xj in roccet_orbiter_update() a per-ACK delta rather than the round-to-round comparison the comments describe?
Quoted 6 lines
+/* Custom Pacing Rate for ROCCET TCP.
+ * The code here is similar to the pacing rate adjustments intcp_input.c
+ *tcp_cong_control(). In LAUNCH (slow start) we want a pacing of 200% and
+ * in ORBITER (congestion avoidance) we adjust the pacing to 100% and do not
+ * use thesysctl_tcp_pacing_ca_ratio.
+ */
[ ... ]
net/ipv4/tcp_roccet.c line 1106
Quoted 1 line
net/ipv4/tcp_roccet.c new file +1
-0+if (tcp_snd_cwnd (tp) < tp->snd_ssthresh /2 )
[Severity: Low]
This isn't a bug, but the comment describes the 200%/100% choice as state-based while the function never reads ca->state; it branches on cwnd versus snd_ssthresh / 2 and multiplies by sysctl_tcp_pacing_ss_ratio rather than a literal 200.
Since roccet_launch_update() and roccet_congestion_event() set snd_ssthresh = tcp_snd_cwnd(tp) when leaving LAUNCH, and roccet_state(TCP_CA_Loss) sets cwnd = TCP_INIT_CWND while leaving a large snd_ssthresh, a flow can be in LAUNCH and paced at 100%, or in ORBITER/DRAIN and paced at ss_ratio. Could the comment be brought in line with the code?
Quoted 4 lines
net/ipv4/tcp_roccet.c new file +4
-0+static void roccet_drain_update (struct sock *sk,u32 now) +{ +struct tcp_sock *tp =tcp_sk (sk); +struct roccettcp *ca =inet_csk_ca (sk);
net/ipv4/tcp_roccet.c line 1141
Quoted 10 lines
net/ipv4/tcp_roccet.c new file +10
-0+ +if (ca->roccet_last_event_time_us && +time_after32 (now, ca->roccet_last_event_time_us + +100 *USEC_PER_MSEC )) { +if (tcp_in_slow_start (tp)) + ca->state =LAUNCH ; +else + ca->state =ORBITER ; + } +}
[Severity: High]
This DRAIN exit and the DRAIN entry in roccet_orbiter_update() are exact negations of each other over the same timestamp:
roccet_orbiter_update():
if (ca->roccet_last_event_time_us &&
time_before32(now, ca->roccet_last_event_time_us +
100 * USEC_PER_MSEC)) {
ca->state = DRAIN;roccet_drain_update():
if (ca->roccet_last_event_time_us &&
time_after32(now, ca->roccet_last_event_time_us +
100 * USEC_PER_MSEC)) {ca->roccet_last_event_time_us is refreshed only by roccet_congestion_event() and by the srRTT exit branch of roccet_launch_update(). Once now - roccet_last_event_time_us passes 2^31 us (about 35.8 minutes, with now = jiffies_to_usecs(tcp_jiffies32) wrapping every ~71.6 minutes), the signed comparison flips.
Does ORBITER then jump unconditionally to DRAIN while DRAIN can never exit, so cwnd growth is frozen for the rest of the wrap half-period on long-lived flows that saw one congestion event and then a quiet path?
Separately, jiffies_to_usecs(tcp_jiffies32) is exactly 0 periodically (every 536870912 jiffies at HZ=1000). If roccet_congestion_event() stores that 0, the ca->roccet_last_event_time_us && guard makes this exit unreachable until an unrelated RTO or CWR resets the state machine. Is 0 intended to double as an "unset" sentinel here?
[ ... ]
net/ipv4/tcp_roccet.c line 1168
Quoted 8 lines
net/ipv4/tcp_roccet.c new file +8
-0+/* Evaluate roccet state */ +switch (ca->state) { +case LAUNCH : +roccet_launch_update (sk, rs->acked_sacked); +break ; +case ORBITER : +roccet_orbiter_update (sk, rs->acked_sacked); +break ;
[Severity: High]roccet_control() is installed as .cong_control, so tcp_cong_control() returns right after calling it, bypassing tcp_cwnd_reduction()/PRR and the tcp_may_raise_cwnd() gate. This switch dispatches on ca->state only, and inet_csk(sk)->icsk_ca_state is not read anywhere in the module.
While the socket is in TCP_CA_Recovery, TCP_CA_CWR or TCP_CA_Loss, does every incoming ACK keep running the growth paths, i.e. tcp_slow_start() in roccet_launch_update() (and roccet_state(TCP_CA_Loss) explicitly resets the machine to LAUNCH) and bictcp_update() plus tcp_cong_avoid_ai() in roccet_orbiter_update()?
tp->is_cwnd_limited is set by tcp_cwnd_validate() during the retransmissions, so the tcp_is_cwnd_limited() gate in roccet_orbiter_update() does not block this. BBR handles the same problem explicitly in bbr_set_cwnd_to_recover_or_restore(); should ROCCET implement its own packet conservation while in recovery?
[ ... ]
Signature · 1 line
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/aqqGU0XfcfsI-OGy%40volt-roccet-vm