Skip to content

Recovery: fix spurious retransmit path reference - #249

Open
rpaulo wants to merge 1 commit into
apple:mainfrom
rpaulo:recovery-spurious
Open

rpaulo wants to merge 1 commit into
apple:mainfrom
rpaulo:recovery-spurious

Conversation

@rpaulo

@rpaulo rpaulo commented Oct 13, 2026

Copy link
Copy Markdown
Contributor
  • Report spurious retransmits and loss-related congestion events to the path a packet was sent on, not the path its ACK arrived on
  • Retain lost packets for up to one RTT so a late ACK is recognized as a spurious loss and undoes the congestion window reduction
  • Pass the correct largest-lost sent time so a new congestion event isn't hidden on fresh connections
  • Rename path/pathID to sentPath/sentPathID for clarity and add tests covering single- and multi-path spurious loss recovery

@rpaulo rpaulo added the 🔨 semver/patch No public API change. label Oct 13, 2026
}
if removedEntry.lostTime != .zero, lostPacketCount > 0 {
lostPacketCount &-= 1
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

So this missing branch looks like it was throwing our book-keeping off?

if removedEntry.lostTime != .zero, lostPacketCount > 0 {
    lostPacketCount &-= 1
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The main problem is that we were not keeping packets in the list when they were declared lost, so there was no way to detect spurious losses.

- Report spurious retransmits and loss-related congestion events to the
  path a packet was sent on, not the path its ACK arrived on
- Retain lost packets for up to one RTT so a late ACK is recognized as a
  spurious loss and undoes the congestion window reduction
- Pass the correct largest-lost sent time so a new congestion event
  isn't hidden on fresh connections
- Rename `path`/`pathID` to `sentPath`/`sentPathID` for clarity and add
  tests covering single- and multi-path spurious loss recovery
@rpaulo
rpaulo force-pushed the recovery-spurious branch from 08efc6d to 377adf7 Compare October 13, 2026 20:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🔨 semver/patch No public API change.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants