Skip to content

feat: Detect and recover from TCP Simultaneous open#2950

Closed
MarcoPolo wants to merge 3 commits intomasterfrom
marco/detect-tcp-simopen
Closed

feat: Detect and recover from TCP Simultaneous open#2950
MarcoPolo wants to merge 3 commits intomasterfrom
marco/detect-tcp-simopen

Conversation

@MarcoPolo
Copy link
Copy Markdown
Collaborator

Closes #2925

This touches one of the trickiest parts of the codebase, so please review carefully @sukunrt.

Requires multiformats/go-multistream#114 and a new release from that dep.

@gdsoumya
Copy link
Copy Markdown

Interested in trying this out for my issue, any updates on this ^^

@MarcoPolo MarcoPolo mentioned this pull request Oct 9, 2024
30 tasks
@MarcoPolo
Copy link
Copy Markdown
Collaborator Author

@gdsoumya, could you try this branch to see if it solves your problem?

@MarcoPolo MarcoPolo requested a review from sukunrt October 15, 2024 18:34
@sukunrt sukunrt self-assigned this Nov 1, 2024
@p-shahi p-shahi mentioned this pull request Dec 3, 2024
26 tasks
Comment thread p2p/net/swarm/dial_worker.go
Comment thread core/sec/security.go
Comment thread p2p/security/noise/transport.go
Comment thread p2p/security/tls/transport.go
Comment thread p2p/net/swarm/dial_worker.go
Comment thread p2p/net/swarm/dial_worker.go
Comment on lines +143 to +144
top := dq.top()
if !top.ForceDelay && dialsInFlight == 0 && !w.connected {
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.

Instead of this, how about we introduce another queue:
fixedDelayQueue.

  1. This construct forces us to expose ForceRetry, which external users never need to use.
  2. Makes the semantics slightly incorrect for dials that are behind in the queue but don't have the forceRetryFlag.

I suggest introducing:

  • a separate fixed Delay Queue with its separate fixedDelayTimer
  • adding a now time.Time argument to NextBatch that'll return elements that have expired by now
  • Have two blocks for handling dials, 1 for dialQueue and 1 for fixedDelayDialQueue.

I don't feel strongly about the suggestion. Anything that fixes 2 would be great.

@p-shahi p-shahi mentioned this pull request Dec 26, 2024
20 tasks
@MarcoPolo MarcoPolo mentioned this pull request Jan 29, 2025
20 tasks
@MarcoPolo MarcoPolo mentioned this pull request Feb 14, 2025
19 tasks
@MarcoPolo MarcoPolo mentioned this pull request Feb 22, 2025
19 tasks
@MarcoPolo
Copy link
Copy Markdown
Collaborator Author

Automatically closing old open PRs. If you're still interested in merging this please open the PR again and rebase from the latest master, otherwise do nothing.

@MarcoPolo MarcoPolo closed this Sep 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Detect failures related to TCP simultaneous open and retry

3 participants