From 77f92efd16e523c41b0e01aa5a7e11e9aae3e795 Mon Sep 17 00:00:00 2001 From: zh522130 <717572325@qq.com> Date: Wed, 29 May 2024 20:51:43 +0800 Subject: [PATCH] fix(iroh-net): fix extra delay (#2330) ## Description Fix for https://github.com/n0-computer/iroh/issues/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 https://github.com/n0-computer/iroh/issues/2327. --------- Co-authored-by: Floris Bruynooghe --- Cargo.lock | 4 +- iroh-net/Cargo.toml | 2 +- iroh-net/src/magicsock.rs | 173 +++++++++++++------------ iroh-net/src/net/interfaces/windows.rs | 2 +- 4 files changed, 94 insertions(+), 87 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 61d548d54b..0a06fd14bf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3147,9 +3147,9 @@ dependencies = [ [[package]] name = "netdev" -version = "0.25.0" +version = "0.27.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fb353f5a5a852d5cc779c1c80bec0bd14a696ef832f3a761cb10091802c37109" +checksum = "e12d9f49b8d4b9d7f97525ce65f6527079e549e8b2f010f15b921764e73d4baa" dependencies = [ "dlopen2", "libc", diff --git a/iroh-net/Cargo.toml b/iroh-net/Cargo.toml index bedcf87f86..53b97a0377 100644 --- a/iroh-net/Cargo.toml +++ b/iroh-net/Cargo.toml @@ -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" diff --git a/iroh-net/src/magicsock.rs b/iroh-net/src/magicsock.rs index f5c745dbc7..080e99b985 100644 --- a/iroh-net/src/magicsock.rs +++ b/iroh-net/src/magicsock.rs @@ -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. diff --git a/iroh-net/src/net/interfaces/windows.rs b/iroh-net/src/net/interfaces/windows.rs index 06f118c33b..3b5480b695 100644 --- a/iroh-net/src/net/interfaces/windows.rs +++ b/iroh-net/src/net/interfaces/windows.rs @@ -15,7 +15,7 @@ struct Win32_IP4RouteTable { fn get_default_route() -> anyhow::Result { 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