Zepp-Hanzj opened a new pull request, #18993:
URL: https://github.com/apache/nuttx/pull/18993

   ## 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
   ```c
   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
   ```c
   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
   ```bash
   $ ./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
   
   - Issue: #13161
   - RFC 6298: Computing TCP's Retransmission Timer
   - Karn's Algorithm: Don't use retransmitted segments for RTT estimation
   
   ### Signed-off-by
   
   hanzj <[email protected]>


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to