fix(quic): improve listener socket selection when dialing
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 5.6k
- Forks
- 1.3k
- Avg merge
- 8h 47m
- Merged PRs (30d)
- 19
Description
Problem
Say the local node listens on /ip4/0.0.0.0/udp/0/quic-v1. One would expect a consecutive dial to /ip4/127.0.0.1/udp/42/quic-v1 to use one of the listen addresses as the source address of the dial.
That is not the case today. Instead libp2p-quic will create a new quinn endpoint and thus a new UDP socket for the outgoing dial.
Why?
In TCP we track the specific listen addresses, even if the user calls listen_on with an unspecified address (e.g. 0.0.0.0). If the user calls listen_on with a specific IP address i.e. not a wildcard like 0.0.0.0 but e.g. 127.0.0.1 we track (here register) the specific address right away.
In case where the user calls listen_on with an unspecified address (e.g. 0.0.0.0) we depend on if-watch, more specifically poll_if_watch to provide us with the concrete addresses:
Either way, we end up tracking specific listen addresses with port-reuse enabled and can thus choose the right listener in local_dial_addr:
In libp2p-quic we only track the initial address that the user provided on the listen_on call.
This might be a wildcard address. Later on, when dialing a 127.0.0.1 address, the check listen_addr.ip().is_loopback() == socket_addr.ip().is_loopback() discards the listener with the unspecified listen address, thus a new dial-only socket is used, even though one of the existing listener sockets is available.
This is e.g. problematic when trying to dial a node on 127.0.0.1 with the expectation that the dial uses one of the listen addresses as a source address.
How does go-libp2p solve this:
go-libp2p asks the OS for the correct interface given a destination address. It uses Google's routing package. See https://github.com/libp2p/go-libp2p/blob/260b9695cafdd8e35ec65b30ef153f0c15549c72/p2p/transport/quicreuse/reuse.go#L209 for concrete usage and implementation.
How to move forward?
I am not sure simply removing the listen_addr.ip().is_loopback() == socket_addr.ip().is_loopback() check is the way to go. This check is still helpful. E.g. say we have two listeners, one on localhost, one not on localhost. Ideally we would use the former when dialing.
We could mirror what libp2p-tcp does. That is, track the specified listen addresses reported by if-watch. Later, when dialing, choose the listener with the right address.
Ideally I would like to have the operating system choose. It has the most information about all available routes to each interface. In other words ideally we would do what go-libp2p does.
For now, I suggest we go with the libp2p-tcp approach. A second iteration would mirror what go-libp2p does.
Originally posted by @mxinden in https://github.com/libp2p/rust-libp2p/pull/3454#discussion_r1276338263
Thank you @kpp for surfacing this bug.
Thank you @marten-seemann for the quick help, pointing me to how go-libp2p solves this issue.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in transports/quic/src/endpoint.rs around listen address tracking, then compare transports/tcp/src/lib.rs for registration, poll_if_watch, and local_dial_addr behavior. Follow how concrete addresses are selected and verify that dialing a loopback destination from a wildcard listener reuses the appropriate listener socket instead of creating a dial-only socket.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- networking
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100