Skip to content

Commit

Permalink
fix(iroh-net): fix extra delay (#2330)
Browse files Browse the repository at this point in the history
## Description

Fix for #2328: Enumerating
network devices is slow
on windows and causes delays in the magicsock actor. This makes sure
that this operation
does not block the remainder of the actor.

Also bumps netdev to v0.27.0 , to fix
#2327.

---------

Co-authored-by: Floris Bruynooghe <[email protected]>
  • Loading branch information
zh522130 and flub authored May 29, 2024
1 parent 23e8c7b commit 77f92ef
Showing 4 changed files with 94 additions and 87 deletions.
4 changes: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion iroh-net/Cargo.toml
Original file line number Diff line number Diff line change
@@ -22,7 +22,7 @@ anyhow = { version = "1" }
base64 = "0.22.1"
backoff = "0.4.0"
bytes = "1"
netdev = "0.25"
netdev = "0.27.0"
der = { version = "0.7", features = ["alloc", "derive"] }
derive_more = { version = "1.0.0-beta.6", features = ["debug", "display", "from", "try_into", "deref"] }
flume = "0.11"
173 changes: 90 additions & 83 deletions iroh-net/src/magicsock.rs
Original file line number Diff line number Diff line change
@@ -2000,102 +2000,109 @@ impl Actor {
.map(|a| a.ip().is_unspecified())
.unwrap_or(false);

let LocalAddresses {
regular: mut ips,
loopback,
} = LocalAddresses::new();

if is_unspecified_v4 || is_unspecified_v6 {
if ips.is_empty() && eps.is_empty() {
// Only include loopback addresses if we have no
// interfaces at all to use as endpoints and don't
// have a public IPv4 or IPv6 address. This allows
// for localhost testing when you're on a plane and
// offline, for example.
ips = loopback;
}
let v4_port = local_addr_v4.and_then(|addr| {
if addr.ip().is_unspecified() {
Some(addr.port())
} else {
None
let msock = self.msock.clone();

tokio::spawn(async move {
// Depending on the OS and network interfaces attached and their state enumerating
// the local interfaces can take a long time. Especially Windows is very slow.
let LocalAddresses {
regular: mut ips,
loopback,
} = tokio::task::spawn_blocking(LocalAddresses::new)
.await
.unwrap();

if is_unspecified_v4 || is_unspecified_v6 {
if ips.is_empty() && eps.is_empty() {
// Only include loopback addresses if we have no
// interfaces at all to use as endpoints and don't
// have a public IPv4 or IPv6 address. This allows
// for localhost testing when you're on a plane and
// offline, for example.
ips = loopback;
}
});
let v4_port = local_addr_v4.and_then(|addr| {
if addr.ip().is_unspecified() {
Some(addr.port())
} else {
None
}
});

let v6_port = local_addr_v6.and_then(|addr| {
if addr.ip().is_unspecified() {
Some(addr.port())
} else {
None
}
});
let v6_port = local_addr_v6.and_then(|addr| {
if addr.ip().is_unspecified() {
Some(addr.port())
} else {
None
}
});

for ip in ips {
match ip {
IpAddr::V4(_) => {
if let Some(port) = v4_port {
add_addr!(
already,
eps,
SocketAddr::new(ip, port),
config::EndpointType::Local
);
for ip in ips {
match ip {
IpAddr::V4(_) => {
if let Some(port) = v4_port {
add_addr!(
already,
eps,
SocketAddr::new(ip, port),
config::EndpointType::Local
);
}
}
}
IpAddr::V6(_) => {
if let Some(port) = v6_port {
add_addr!(
already,
eps,
SocketAddr::new(ip, port),
config::EndpointType::Local
);
IpAddr::V6(_) => {
if let Some(port) = v6_port {
add_addr!(
already,
eps,
SocketAddr::new(ip, port),
config::EndpointType::Local
);
}
}
}
}
}
}

if !is_unspecified_v4 {
if let Some(addr) = local_addr_v4 {
// Our local endpoint is bound to a particular address.
// Do not offer addresses on other local interfaces.
add_addr!(already, eps, addr, config::EndpointType::Local);
if !is_unspecified_v4 {
if let Some(addr) = local_addr_v4 {
// Our local endpoint is bound to a particular address.
// Do not offer addresses on other local interfaces.
add_addr!(already, eps, addr, config::EndpointType::Local);
}
}
}

if !is_unspecified_v6 {
if let Some(addr) = local_addr_v6 {
// Our local endpoint is bound to a particular address.
// Do not offer addresses on other local interfaces.
add_addr!(already, eps, addr, config::EndpointType::Local);
if !is_unspecified_v6 {
if let Some(addr) = local_addr_v6 {
// Our local endpoint is bound to a particular address.
// Do not offer addresses on other local interfaces.
add_addr!(already, eps, addr, config::EndpointType::Local);
}
}
}

// Note: the endpoints are intentionally returned in priority order,
// from "farthest but most reliable" to "closest but least
// reliable." Addresses returned from STUN should be globally
// addressable, but might go farther on the network than necessary.
// Local interface addresses might have lower latency, but not be
// globally addressable.
//
// The STUN address(es) are always first.
// Despite this sorting, clients are not relying on this sorting for decisions;

let updated = self
.msock
.endpoints
.update(DiscoveredEndpoints::new(eps))
.is_ok();
if updated {
let eps = self.msock.endpoints.read();
eps.log_endpoint_change();
self.msock.publish_my_addr();
}

// Regardless of whether our local endpoints changed, we now want to send any queued
// call-me-maybe messages.
self.msock.send_queued_call_me_maybes();
// Note: the endpoints are intentionally returned in priority order,
// from "farthest but most reliable" to "closest but least
// reliable." Addresses returned from STUN should be globally
// addressable, but might go farther on the network than necessary.
// Local interface addresses might have lower latency, but not be
// globally addressable.
//
// The STUN address(es) are always first.
// Despite this sorting, clients are not relying on this sorting for decisions;

let updated = msock
.endpoints
.update(DiscoveredEndpoints::new(eps))
.is_ok();
if updated {
let eps = msock.endpoints.read();
eps.log_endpoint_change();
msock.publish_my_addr();
}

// Regardless of whether our local endpoints changed, we now want to send any queued
// call-me-maybe messages.
msock.send_queued_call_me_maybes();
});
}

/// Called when an endpoints update is done, no matter if it was successful or not.
2 changes: 1 addition & 1 deletion iroh-net/src/net/interfaces/windows.rs
Original file line number Diff line number Diff line change
@@ -15,7 +15,7 @@ struct Win32_IP4RouteTable {

fn get_default_route() -> anyhow::Result<DefaultRouteDetails> {
let com_con = COMLibrary::new()?;
let wmi_con = WMIConnection::new(com_con.into())?;
let wmi_con = WMIConnection::new(com_con)?;

let query: HashMap<_, _> = [("Destination".into(), FilterValue::Str("0.0.0.0"))].into();
let route: Win32_IP4RouteTable = wmi_con

0 comments on commit 77f92ef

Please sign in to comment.