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

transports/tcp: Bind to port, not local IP, when dialing #2382

Merged
merged 3 commits into from
Jan 1, 2022

Conversation

rkuhn
Copy link
Contributor

@rkuhn rkuhn commented Dec 14, 2021

fixes #2152

I tried this out on Linux only, it works perfectly here.

Copy link
Member

@mxinden mxinden left a comment

Choose a reason for hiding this comment

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

This looks reasonable to me. Definitely in favor of not re-implementing the kernel's routing logic.

If I am not mistaken, go-libp2p does the same, i.e. go-libp2p uses an unspecified IP as well:

https://github.com/libp2p/go-reuseport-transport/blob/0eb62d386464f17a45160fa09241c7639b5afe62/dial.go#L71-L113

Before we proceed here @rubdos could you test whether this resolves your issue as well?

@rubdos
Copy link
Contributor

rubdos commented Dec 18, 2021

Before we proceed here @rubdos could you test whether this resolves your issue as well?

I should be able to find some time before Christmas, feel free to tickle me during the week if I'm slow.

@mxinden
Copy link
Member

mxinden commented Dec 27, 2021

Friendly ping @rubdos :)

@rubdos
Copy link
Contributor

rubdos commented Dec 28, 2021

Looks like this is working well indeed! Had to struggle a bit with the 0.40 and 0.41 patches to NetworkBehaviour before stuff compiled again, sorry for the delay.

Next grievance: libp2p seems to want to connect to non-public addresses that are not in the routing tables. That's probably a bit more difficult to fix.

@rkuhn
Copy link
Contributor Author

rkuhn commented Dec 28, 2021

libp2p can’t really know which address is how public, and it can’t know which of the “less public” addresses are actually useful (e.g. in your home network). Are these connection attempts presenting a problem for you?

@rubdos
Copy link
Contributor

rubdos commented Dec 28, 2021

libp2p can’t really know which address is how public, and it can’t know which of the “less public” addresses are actually useful (e.g. in your home network).

No, indeed, but my gut says it should be possible to make a smarter attempt at order and maybe concurrency in IP address attempts.

Are these connection attempts presenting a problem for you?

They are presenting a slow down for bootstraps.

Anyway, that's completely aside from this PR!

@dvc94ch
Copy link
Contributor

dvc94ch commented Dec 28, 2021

One way to handle this would be to not allow private networks into the dht. Mdns should manage to supply the local view of the network and when dialing addresses_of_peer should join the two views

@rubdos
Copy link
Contributor

rubdos commented Dec 28, 2021

MDNS is not always allowed on certain semi-public networks. Maybe it's even application-specific, the handling of such IP ranges. I was thinking in the line of only using private ranges if and only if they appear directly in the routing tables, but sometimes a private range is also handled in the default routes (nested NAT/CGNAT/...).

Shall I move this to a new issue?

@thomaseizinger
Copy link
Contributor

I think libp2p's solution for this is the AutoNAT protocol (https://github.com/libp2p/specs/pull/362/files) currently in development for rust-libp2p here: #2262

@mxinden
Copy link
Member

mxinden commented Dec 29, 2021

I was thinking in the line of only using private ranges if and only if they appear directly in the routing tables,

As you suggested, I think this is best discussed in a new issue @rubdos.

One way to handle this would be to not allow private networks into the dht.

This is the case for go-libp2p and the plan for rust-libp2p. A node would use #2262 to determine whether it is publicly reachable. If not, it would run in libp2p-kad client mode #2032 and thus won't be included in the Kademlia routing tables.

Copy link
Member

@mxinden mxinden left a comment

Choose a reason for hiding this comment

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

@rkuhn would you mind including a changelog entry in transports/tcp/CHANGELOG.md? Otherwise this is good to merge from my side.

@rubdos thank you for testing!

@rkuhn
Copy link
Contributor Author

rkuhn commented Dec 31, 2021

done

@mxinden mxinden changed the title only bind port, not local IP, when dialing TCP addresses transports/tcp: Bind to port, not local IP, when dialing Jan 1, 2022
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.

(Tokio) Tcp port reuse: picking unconnectable interfaces?
5 participants