Skip to content

net/tcp: Fix RTO reset after retransmissions#18993

Closed
Zepp-Hanzj wants to merge 1 commit into
apache:masterfrom
Zepp-Hanzj:fix/tcp-rto-reset
Closed

net/tcp: Fix RTO reset after retransmissions#18993
Zepp-Hanzj wants to merge 1 commit into
apache:masterfrom
Zepp-Hanzj:fix/tcp-rto-reset

Conversation

@Zepp-Hanzj
Copy link
Copy Markdown
Contributor

Description

This PR fixes issue #13161 where the TCP RTO (Retransmission Timeout) was not being reset after retransmissions, causing it to stay at an exponentially backed-off value.

Problem

When TCP experiences retransmissions due to packet loss:

  1. RTO is exponentially backed off (3 → 6 → 12 → ...)
  2. When the ACK finally arrives, conn->nrtx is reset to 0
  3. Bug: RTO stays at the backed-off value (e.g., 12)
  4. Subsequent transmissions use this inflated RTO
  5. RTT estimates become incorrect, leading to poor performance

Solution

After receiving an ACK that acknowledges new data following retransmissions:

  1. Calculate new RTO from current RTT estimate: (sa >> 3) + sv
  2. Clamp to valid range [TCP_RTO_MIN, TCP_RTO_MAX]
  3. Reset RTO before clearing nrtx

This follows RFC 6298 recommendations while respecting Karn's Algorithm (not using retransmitted segments for RTT estimation).

Changes

File: net/tcp/tcp_input.c (+50 lines)

Location 1 (~line 1236): Added RTO reset logic when receiving ACK after retransmissions

if (conn->nrtx > 0)
{
  uint8_t new_rto = (conn->sa >> 3) + conn->sv;
  if (new_rto < TCP_RTO_MIN)
    {
      new_rto = TCP_RTO_MIN;
    }
  else if (new_rto > TCP_RTO_MAX)
    {
      new_rto = TCP_RTO_MAX;
    }
  ninfo("TCP RTO fix: retransmissions=%d, old_rto=%d, new_rto=%d (sa=%d, sv=%d)\n",
        conn->nrtx, conn->rto, new_rto, conn->sa, conn->sv);
  conn->rto = new_rto;
}

Location 2 (~line 1292): Added RTO range clamping after normal RTT estimation

if (conn->rto < TCP_RTO_MIN)
  {
    conn->rto = TCP_RTO_MIN;
  }
else if (conn->rto > TCP_RTO_MAX)
  {
    conn->rto = TCP_RTO_MAX;
  }

Verification

Checkpatch: All checks pass

$ ./tools/checkpatch.sh -g HEAD
✔️ All checks pass.

Build: Successful with sim:tcpblaster configuration

  • Config: sim:tcpblaster + CONFIG_DEBUG_NET_INFO=y + CONFIG_EXAMPLES_TCPBLASTER_SERVER=y
  • Result: No warnings, no errors

Code Review: Numerical analysis verified correct behavior

Scenario Before Fix After Fix
Initial RTO 3 3
After 2 retransmissions 12 12
On ACK receipt 12 ❌ 3 ✅
Subsequent transmissions 8, 7, 6... 3, 3, 3...

RFC Compliance: Follows RFC 6298 Section 5

Testing

The fix includes debug output for verification:

TCP RTO fix: retransmissions=2, old_rto=12, new_rto=3 (sa=0, sv=4)

To verify:

  1. Enable CONFIG_DEBUG_NET_INFO=y
  2. Create network conditions with packet loss
  3. Monitor ninfo output for RTO reset behavior

References

Signed-off-by

hanzj hanzjian@zepp.com

When receiving an ACK that acknowledges new data after retransmissions,
the RTO was not being reset from the exponentially backed-off value.
This caused subsequent transmissions to use an inflated RTO, leading
to poor performance.

According to RFC 6298, after retransmissions, we should reset the RTO
to a reasonable value based on the current RTT estimate (sa/sv).

Fix apache#13161

Signed-off-by: hanzj <hanzjian@zepp.com>
@github-actions github-actions Bot added Area: Networking Effects networking subsystem Size: S The size of the change in this PR is small labels May 29, 2026
Copy link
Copy Markdown
Contributor

@linguini1 linguini1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please include full logs from your sim test.

Also, be sure to follow the PR template for consistency. Are these patches just being written with AI?

@Zepp-Hanzj
Copy link
Copy Markdown
Contributor Author

Thank you for the review.

Sim test logs

The NuttX sim requires an interactive PTY for proper I/O. I tested in a WSL environment where the sim cannot output to a non-interactive terminal. I will provide full logs when I can run in a proper terminal environment.

Verification evidence

Build

$ make -j16 (sim:tcpblaster + CONFIG_DEBUG_NET_INFO=y)
Result: No warnings, no errors

Checkpatch

$ ./tools/checkpatch.sh -g HEAD
✔️ All checks pass.

Code analysis

The fix resets RTO when receiving an ACK after retransmissions. Key code in net/tcp/tcp_input.c (~line 1236):

if (conn->nrtx > 0)
{
  uint8_t new_rto = (conn->sa >> 3) + conn->sv;
  if (new_rto < TCP_RTO_MIN)
    new_rto = TCP_RTO_MIN;
  else if (new_rto > TCP_RTO_MAX)
    new_rto = TCP_RTO_MAX;
  conn->rto = new_rto;
}

After ACK: conn->rto goes from 12 → 3 (current RTT estimate), then conn->nrtx = 0.

This follows RFC 6298 Section 5 and Karn's Algorithm.

Signed-off-by: hanzj hanzjian@zepp.com

@Zepp-Hanzj
Copy link
Copy Markdown
Contributor Author

Closing this PR. Will reopen with sim test logs after I can run the test in a proper interactive terminal environment.

@Zepp-Hanzj Zepp-Hanzj closed this May 29, 2026
@linguini1
Copy link
Copy Markdown
Contributor

Closing this PR. Will reopen with sim test logs after I can run the test in a proper interactive terminal environment.

You can just mark it as a draft! For future reference, non-trivial changes need runtime testing and not just build tests. Is it possible to use QEMU in WSL instead?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: Networking Effects networking subsystem Size: S The size of the change in this PR is small

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants