libp2p / libp2p/rust-libp2p

fix(quic): improve listener socket selection when dialing

Open
#4,259 8 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug difficulty:moderate help wanted
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.

https://github.com/libp2p/rust-libp2p/blob/f10f1a274abe6e5193de653aa4dd951ae56c775e/transports/tcp/src/lib.rs#L394

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:

https://github.com/libp2p/rust-libp2p/blob/f10f1a274abe6e5193de653aa4dd951ae56c775e/transports/tcp/src/lib.rs#L666-L682

Either way, we end up tracking specific listen addresses with port-reuse enabled and can thus choose the right listener in local_dial_addr:

https://github.com/libp2p/rust-libp2p/blob/f10f1a274abe6e5193de653aa4dd951ae56c775e/transports/tcp/src/lib.rs#L122-L151

In libp2p-quic we only track the initial address that the user provided on the listen_on call.

https://github.com/libp2p/rust-libp2p/blob/f10f1a274abe6e5193de653aa4dd951ae56c775e/transports/quic/src/endpoint.rs#L170-L178

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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.