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.

Sign up for GitHub

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

Jump to bottom

{core,swarm}: Allow configuring dial concurrency factor per dial #2385

Closed
mxinden opened this issue Dec 15, 2021 · 0 comments · Fixed by #2404
Closed

{core,swarm}: Allow configuring dial concurrency factor per dial #2385

mxinden opened this issue Dec 15, 2021 · 0 comments · Fixed by #2404
Labels
difficulty:easy getting-started Issues that can be tackled if you don't know the internals of libp2p very well help wanted

Comments

@mxinden
Copy link
Member

mxinden commented Dec 15, 2021

Today the dial concurrency factor can be set global and for once.

rust-libp2p/swarm/src/lib.rs

Lines 1241 to 1245 in b6c82a4

/// Number of addresses concurrently dialed for a single outbound connection attempt.
pub fn dial_concurrency_factor(mut self, factor: NonZeroU8) -> Self {
self.network_config = self.network_config.with_dial_concurrency_factor(factor);
self
}

I argue a NetworkBehaviour should be able to configure the factor for each dial individually.

While it is necessary to dial concurrently when hole punching to increase success rate, I don't think this should be the case for libp2p-autonat when probing someones addresses. In other words, to reduce the amount of work a remote node can make the local node do at a time, I think the local node should probe the remotes addresses in sequence and not in parallel. In order to be able to do so, libp2p-autonat needs to be able to set the dial concurrency factor to 0 for probe dials.

@elenaf9 would you agree with the above?

#2363 already adds a new parameter to Network::dial, namely role_override. Instead of adding yet another one, it might be time to have Network::dial take a DialOpts like e.g. Swarm::dial does.
@mxinden mxinden added difficulty:easy help wanted getting-started Issues that can be tackled if you don't know the internals of libp2p very well labels Dec 15, 2021
@mxinden mxinden changed the title {core/swarm}: Allow configuring dial concurrency factor per dial {core,swarm}: Allow configuring dial concurrency factor per dial Dec 15, 2021
mxinden added a commit to mxinden/rust-libp2p that referenced this issue Dec 28, 2021
@mxinden
{core,swarm}/: Allow configuring dial concurrency factor per dial
cec8f38 
Enable a `NetworkBehaviour` or a user via `Swarm::dial` to override the
dial concurrency factor per dial.

To enable the above, this commit:

- Introduces `libp2p_core::DialOpts` mirroring `libp2p_swarm::DialOpts`.
  Passed as an argument to `Network::dial`.
- Removes `Peer::dial` in favor of `Network::dial`.
- Simplifies `Swarm::dial_with_handler`.

Fixes libp2p#2385.
mxinden added a commit to mxinden/rust-libp2p that referenced this issue Dec 28, 2021
@mxinden
{core,swarm}/: Allow configuring dial concurrency factor per dial
5ba1124 
Enable a `NetworkBehaviour` or a user via `Swarm::dial` to override the
dial concurrency factor per dial. This is especially relevant in the
case of libp2p-autonat where one wants to probe addresses in sequence to
reduce the amount of work a remote peer can force onto the local node.

To enable the above, this commit also:

- Introduces `libp2p_core::DialOpts` mirroring `libp2p_swarm::DialOpts`.
  Passed as an argument to `Network::dial`.
- Removes `Peer::dial` in favor of `Network::dial`.
- Simplifies `Swarm::dial_with_handler`.

The introduction of `libp2p_core::DialOpts` will be useful beyond this
feature, e.g. for libp2p#2363.

In the long run I would like to move and merge `libp2p_core::Network`
and `libp2p_core::Pool` into `libp2p_swarm::Swarm` thus deduplicating
`libp2p_core::DialOpts` and `libp2p_swarm::DialOpts`.

Fixes libp2p#2385.
mxinden added a commit to mxinden/rust-libp2p that referenced this issue Dec 28, 2021
@mxinden
{core,swarm}/: Allow configuring dial concurrency factor per dial
d7ddd0d 
Enable a `NetworkBehaviour` or a user via `Swarm::dial` to override the
dial concurrency factor per dial. This is especially relevant in the
case of libp2p-autonat where one wants to probe addresses in sequence to
reduce the amount of work a remote peer can force onto the local node.

To enable the above, this commit also:

- Introduces `libp2p_core::DialOpts` mirroring `libp2p_swarm::DialOpts`.
  Passed as an argument to `Network::dial`.
- Removes `Peer::dial` in favor of `Network::dial`.
- Simplifies `Swarm::dial_with_handler`.

The introduction of `libp2p_core::DialOpts` will be useful beyond this
feature, e.g. for libp2p#2363.

In the long run I would like to move and merge `libp2p_core::Network`
and `libp2p_core::Pool` into `libp2p_swarm::Swarm` thus deduplicating
`libp2p_core::DialOpts` and `libp2p_swarm::DialOpts`.

Fixes libp2p#2385.
@mxinden mxinden mentioned this issue Dec 28, 2021
Merged
mxinden added a commit that referenced this issue Jan 13, 2022
@mxinden
{core,swarm}/: Allow configuring dial concurrency factor per dial (#2404
74f31f1 
)

Enable a `NetworkBehaviour` or a user via `Swarm::dial` to override the
dial concurrency factor per dial. This is especially relevant in the
case of libp2p-autonat where one wants to probe addresses in sequence to
reduce the amount of work a remote peer can force onto the local node.

To enable the above, this commit also:

- Introduces `libp2p_core::DialOpts` mirroring `libp2p_swarm::DialOpts`.
  Passed as an argument to `Network::dial`.
- Removes `Peer::dial` in favor of `Network::dial`.
- Simplifies `Swarm::dial_with_handler`.

The introduction of `libp2p_core::DialOpts` will be useful beyond this
feature, e.g. for #2363.

In the long run I would like to move and merge `libp2p_core::Network`
and `libp2p_core::Pool` into `libp2p_swarm::Swarm` thus deduplicating
`libp2p_core::DialOpts` and `libp2p_swarm::DialOpts`.

Fixes #2385.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
difficulty:easy getting-started Issues that can be tackled if you don't know the internals of libp2p very well help wanted
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

{core,swarm}/: Allow configuring dial concurrency factor per dial mxinden/rust-libp2p
1 participant
@mxinden