Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

swarm: implement Happy Eyeballs ranking #2365

Merged
merged 1 commit into from
Jun 28, 2023

Conversation

sukunrt
Copy link
Member

@sukunrt sukunrt commented Jun 14, 2023

Depends on #2320

Ignores #2349 We can take that up after this.

@sukunrt sukunrt changed the title swarm: implement blackhole detection swarm: implement Happy Eyeballs ranking Jun 14, 2023
@marten-seemann marten-seemann added the status/blocked Unable to be worked further until needs are met label Jun 15, 2023
@marten-seemann marten-seemann removed the status/blocked Unable to be worked further until needs are met label Jun 27, 2023
Copy link
Contributor

@marten-seemann marten-seemann left a comment

Choose a reason for hiding this comment

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

lgtm, modulo the question regarding swapping vs shifting

if isQUICAddr(addrs[0]) && isProtocolAddr(addrs[0], ma.P_IP6) {
for i := 1; i < len(addrs); i++ {
if isQUICAddr(addrs[i]) && isProtocolAddr(addrs[i], ma.P_IP4) {
addrs[i], addrs[1] = addrs[1], addrs[i]
Copy link
Contributor

Choose a reason for hiding this comment

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

Is swapping the right thing to do here, or should the address be inserted at position 1, shifting all other addresses?

Copy link
Member Author

Choose a reason for hiding this comment

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

There's no impact because we have three tiers:
IPv6 dialed first, IPv4 dialed second, rest of the addresses.
I'll change it to shifting though because that seems more correct in case we eventually want to dial only a single address at a time.

Copy link
Member Author

Choose a reason for hiding this comment

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

updated

p2p/net/swarm/dial_ranker.go Outdated Show resolved Hide resolved
p2p/net/swarm/dial_ranker.go Outdated Show resolved Hide resolved
p2p/net/swarm/dial_ranker.go Outdated Show resolved Hide resolved
@marten-seemann marten-seemann merged commit 71432c9 into master Jun 28, 2023
@marten-seemann marten-seemann mentioned this pull request Jun 28, 2023
29 tasks
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.

2 participants