Skip to content

fast retransmit fix#1155

Open
JPDye wants to merge 7 commits into
smoltcp-rs:mainfrom
JPDye:fast-retransmit-fix
Open

fast retransmit fix#1155
JPDye wants to merge 7 commits into
smoltcp-rs:mainfrom
JPDye:fast-retransmit-fix

Conversation

@JPDye

@JPDye JPDye commented May 28, 2026

Copy link
Copy Markdown
Contributor

Stacked PR. Requires #1156 and CUBIC. Will rebase once that merges.

Previously, the fast retransmit would send the entire buffer. Now, it only sends the oldest segment. On the single flow netsim, performance regresses and on the multiflow netsim (#1153) performance is significantly improved. This is because fast-retransmit works better with "realistic" tail drop loss better than it does random loss.

These performance losses are somewhat regained by changing the RTTEstimator to handle RTO differently to individual retransmissions, but still leave the single flow reasonably worse off when random loss occurs.

Single flow:

╭───┬───────┬───────┬───────┬───────┬────────┬────────┬────────┬────────┬────────╮
│ # │  buf  │ 0.000 │ 0.001 │ 0.010 │ 0.020  │ 0.050  │ 0.100  │ 0.200  │ 0.300  │
├───┼───────┼───────┼───────┼───────┼────────┼────────┼────────┼────────┼────────┤
│ 0 │   128 │ 0%    │ 0%    │ 0%    │ 0%     │ 0%     │ 0%     │ 0%     │ 0%     │
│ 1 │   256 │ 0%    │ 0%    │ 0%    │ 0%     │ 0%     │ 0%     │ 0%     │ 0%     │
│ 2 │   512 │ 0%    │ 0%    │ 0%    │ 0%     │ 0%     │ 0%     │ 0%     │ 0%     │
│ 3 │  1024 │ 0%    │ 0%    │ 0%    │ 0%     │ 0%     │ 0%     │ 0%     │ 0%     │
│ 4 │  2048 │ 0%    │ 0%    │ 0%    │ 0%     │ 0%     │ -0.3%  │ 0%     │ +1%    │
│ 5 │  4096 │ 0%    │ 0%    │ 0%    │ -0.1%  │ +1.3%  │ -3.4%  │ +9.5%  │ +14.8% │
│ 6 │  8192 │ 0%    │ -0.6% │ -2.8% │ +0.2%  │ -2.9%  │ +11.3% │ +27.1% │ +5.5%  │
│ 7 │ 16384 │ 0%    │ -0.1% │ -8%   │ -16.9% │ -27.1% │ -35.3% │ +0.1%  │ -10.1% │
│ 8 │ 32768 │ 0%    │ +0.1% │ -16%  │ -33.7% │ -40.9% │ -49%   │ -4.4%  │ -41.4% │
╰───┴───────┴───────┴───────┴───────┴────────┴────────┴────────┴────────┴────────╯

Multiflow:

╭───┬───────┬──────────┬──────────┬──────────┬──────────┬────────╮
│ # │ flows │ agg_thru │ min_thru │ max_thru │ fairness │ drops  │
├───┼───────┼──────────┼──────────┼──────────┼──────────┼────────┤
│ 0 │     1 │ 0%       │ 0%       │ 0%       │ 0%       │ 0%     │
│ 1 │     2 │ 0%       │ 0%       │ 0%       │ 0%       │ 0%     │
│ 2 │     4 │ 0%       │ 0%       │ 0%       │ 0%       │ 0%     │
│ 3 │    16 │ +8.7%    │ +20.9%   │ +9.8%    │ +5.9%    │ -74.8% │
│ 4 │    32 │ +7.5%    │ +61.5%   │ -15.9%   │ +11.4%   │ -67.9% │
│ 5 │    64 │ +7.1%    │ +41.8%   │ -24.5%   │ +10.9%   │ -58.7% │
╰───┴───────┴──────────┴──────────┴──────────┴──────────┴────────╯

@codecov

codecov Bot commented May 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.52%. Comparing base (e347a1e) to head (235c8c7).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1155      +/-   ##
==========================================
+ Coverage   81.48%   81.52%   +0.03%     
==========================================
  Files          81       81              
  Lines       25007    25045      +38     
==========================================
+ Hits        20378    20417      +39     
+ Misses       4629     4628       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@JPDye JPDye force-pushed the fast-retransmit-fix branch from d14a8bf to 54b8fec Compare May 28, 2026 20:10
@JPDye JPDye force-pushed the fast-retransmit-fix branch from 54b8fec to 235c8c7 Compare May 28, 2026 20:16
Comment thread src/socket/tcp.rs
let offset = self.remote_last_seq - self.local_seq_no;
repr.payload = self.tx_buffer.get_allocated(offset, size);
let offset = if self.pending_fast_retransmit {
let size = effective_mss.min(self.tx_buffer.len());

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

it's possible self.cwnd_remaining() is 0, which would cause effective_mss to be 0. I think fast retransmit needs to be exempted from cc?

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants