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: add loopback to low timeout filter #2595

Merged
merged 1 commit into from
Oct 12, 2023

Conversation

dennis-tra
Copy link
Contributor

@dennis-tra dennis-tra commented Oct 6, 2023

@sukunrt suggested lowering swarm.WithDialTimeoutLocal to prevent our tests from timing out because of blocking dials. In our tests, we connect via loopback addresses, and I noticed that the swarm.WithDialTimeoutLocal wasn't applied.

In this PR, I added the loopback CIDR to the list of CIDRs to which the local timeout applies.

There are a few more ranges from http://www.iana.org/assignments/iana-ipv4-special-registry/iana-ipv4-special-registry.xhtml that aren't in that list. I'm not sure if there's a reason or if it was just forgotten. I'm happy to extend this PR to add the missing ones as well.

These are missing:

"/ip4/192.0.0.9/ipcidr/32"
"/ip4/192.0.0.10/ipcidr/32"
"/ip4/192.31.196.0/ipcidr/24"
"/ip4/192.52.193.0/ipcidr/24"
"/ip4/192.88.99.0/ipcidr/24"
"/ip4/192.175.48.0/ipcidr/24"
"/ip4/198.18.0.0/ipcidr/15"

Copy link
Member

@sukunrt sukunrt left a comment

Choose a reason for hiding this comment

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

I've changed my mind here considering this:

There are a few more ranges from http://www.iana.org/assignments/iana-ipv4-special-registry/iana-ipv4-special-registry.xhtml that aren't in that list. I'm not sure if there's a reason or if it was just forgotten. I'm happy to extend this PR to add the missing ones as well.

Let's remove this lowTimeoutFilters construct and use manet.IsPublicAddr or manet.IsPrivateAddr. That will keep this definition in one place.
https://pkg.go.dev/github.com/multiformats/go-multiaddr/net#IsPrivateAddr

@dennis-tra dennis-tra force-pushed the low-timeout-loopback branch from 464e330 to e3d6cbc Compare October 6, 2023 09:50
@dennis-tra
Copy link
Contributor Author

Changed to use manet.IsPrivateAddr 👍

dennis-tra added a commit to probe-lab/zikade that referenced this pull request Oct 6, 2023
Copy link
Member

@sukunrt sukunrt left a comment

Choose a reason for hiding this comment

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

This PR is fine. The flakiness is because of issues with webrtc-direct implementation. That will be handled in #2586

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.

Nice job deleting addrs.go!

@sukunrt sukunrt merged commit 1195bf5 into libp2p:master Oct 12, 2023
9 of 11 checks passed
@dennis-tra dennis-tra deleted the low-timeout-loopback branch October 12, 2023 10:13
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.

3 participants