From 1ae677bdbd9a56bf128d329107954dc712a1b96d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philipp=20Kr=C3=BCger?= Date: Thu, 31 Jul 2025 15:20:29 +0200 Subject: [PATCH 1/3] fix(iroh): Always update the best addr after changes --- iroh/src/magicsock/node_map/node_state.rs | 105 +++++++++++----------- iroh/src/magicsock/node_map/udp_paths.rs | 41 ++++++++- iroh/src/magicsock/transports.rs | 4 +- 3 files changed, 92 insertions(+), 58 deletions(-) diff --git a/iroh/src/magicsock/node_map/node_state.rs b/iroh/src/magicsock/node_map/node_state.rs index 5a84f5ae37..e9ece2f501 100644 --- a/iroh/src/magicsock/node_map/node_state.rs +++ b/iroh/src/magicsock/node_map/node_state.rs @@ -214,7 +214,7 @@ impl NodeState { let latency = match conn_type { ConnectionType::Direct(addr) => self .udp_paths - .paths + .paths() .get(&addr.into()) .and_then(|state| state.latency()), ConnectionType::Relay(ref url) => self @@ -225,7 +225,7 @@ impl NodeState { ConnectionType::Mixed(addr, ref url) => { let addr_latency = self .udp_paths - .paths + .paths() .get(&addr.into()) .and_then(|state| state.latency()); let relay_latency = self @@ -240,7 +240,7 @@ impl NodeState { let addrs = self .udp_paths - .paths + .paths() .iter() .map(|(addr, path_state)| DirectAddrInfo { addr: SocketAddr::from(*addr), @@ -373,7 +373,7 @@ impl NodeState { /// /// If this is also the best address, it will be cleared as well. pub(super) fn remove_direct_addr(&mut self, ip_port: &IpPort, now: Instant, why: &'static str) { - let Some(state) = self.udp_paths.paths.remove(ip_port) else { + let Some(state) = self.udp_paths.access_mut(now).paths().remove(ip_port) else { return; }; @@ -381,8 +381,6 @@ impl NodeState { Some(last_alive) => debug!(%ip_port, ?last_alive, why, "pruning address"), None => debug!(%ip_port, last_seen=%"never", why, "pruning address"), } - - self.udp_paths.update_to_best_addr(now); } /// Whether we need to send another call-me-maybe to the endpoint. @@ -412,7 +410,7 @@ impl NodeState { UdpSendAddr::Valid(addr) => { let latency = self .udp_paths - .paths + .paths() .get(&(*addr).into()) .expect("send path not tracked?") .latency() @@ -444,7 +442,9 @@ impl NodeState { debug!(tx = %HEXLOWER.encode(&txid), addr = %sp.to, "pong not received in timeout"); match sp.to { SendAddr::Udp(addr) => { - if let Some(path_state) = self.udp_paths.paths.get_mut(&addr.into()) { + if let Some(path_state) = + self.udp_paths.access_mut(now).paths().get_mut(&addr.into()) + { path_state.last_ping = None; let consider_alive = path_state .last_alive() @@ -456,12 +456,7 @@ impl NodeState { // pong. Both are used to select this path again, but we know // it's not a usable path now. path_state.validity = PathValidity::empty(); - self.udp_paths.update_to_best_addr(now); } - } else { - // If we have no state for the best addr it should have been cleared - // anyway. - self.udp_paths.update_to_best_addr(now); } } SendAddr::Relay(ref url) => { @@ -523,7 +518,7 @@ impl NodeState { let mut path_found = false; match to { SendAddr::Udp(addr) => { - if let Some(st) = self.udp_paths.paths.get_mut(&addr.into()) { + if let Some(st) = self.udp_paths.access_mut(now).paths().get_mut(&addr.into()) { st.last_ping.replace(now); path_found = true } @@ -617,7 +612,7 @@ impl NodeState { #[must_use = "actions must be handled"] fn send_pings(&mut self, now: Instant) -> Vec { // We allocate +1 in case the caller wants to add a call-me-maybe message. - let mut ping_msgs = Vec::with_capacity(self.udp_paths.paths.len() + 1); + let mut ping_msgs = Vec::with_capacity(self.udp_paths.paths().len() + 1); if let Some((url, state)) = self.relay_url.as_ref() { if state.needs_ping(&now) { @@ -639,7 +634,7 @@ impl NodeState { self.prune_direct_addresses(now); let mut ping_dsts = String::from("["); self.udp_paths - .paths + .paths() .iter() .filter_map(|(ipp, state)| state.needs_ping(&now).then_some(*ipp)) .filter_map(|ipp| { @@ -654,7 +649,7 @@ impl NodeState { debug!( %ping_dsts, dst = %self.node_id.fmt_short(), - paths = %summarize_node_paths(&self.udp_paths.paths), + paths = %summarize_node_paths(self.udp_paths.paths()), "sending pings to node", ); self.last_full_ping.replace(now); @@ -698,9 +693,10 @@ impl NodeState { }); } + let mut access = self.udp_paths.access_mut(now); for &addr in new_addrs.iter() { - self.udp_paths - .paths + access + .paths() .entry(addr.into()) .and_modify(|path_state| { path_state.add_source(source.clone(), now); @@ -709,7 +705,8 @@ impl NodeState { PathState::new(self.node_id, SendAddr::from(addr), source.clone(), now) }); } - let paths = summarize_node_paths(&self.udp_paths.paths); + drop(access); + let paths = summarize_node_paths(self.udp_paths.paths()); debug!(new = ?new_addrs , %paths, "added new direct paths for endpoint"); } @@ -730,20 +727,22 @@ impl NodeState { let now = Instant::now(); let role = match path { - SendAddr::Udp(addr) => match self.udp_paths.paths.entry(addr.into()) { - Entry::Occupied(mut occupied) => occupied.get_mut().handle_ping(tx_id, now), - Entry::Vacant(vacant) => { - info!(%addr, "new direct addr for node"); - vacant.insert(PathState::with_ping( - self.node_id, - path.clone(), - tx_id, - Source::Udp, - now, - )); - PingRole::NewPath + SendAddr::Udp(addr) => { + match self.udp_paths.access_mut(now).paths().entry(addr.into()) { + Entry::Occupied(mut occupied) => occupied.get_mut().handle_ping(tx_id, now), + Entry::Vacant(vacant) => { + info!(%addr, "new direct addr for node"); + vacant.insert(PathState::with_ping( + self.node_id, + path.clone(), + tx_id, + Source::Udp, + now, + )); + PingRole::NewPath + } } - }, + } SendAddr::Relay(ref url) => { match self.relay_url.as_mut() { Some((home_url, _state)) if home_url != url => { @@ -812,7 +811,7 @@ impl NodeState { debug!( ?role, needs_ping_back = ?needs_ping_back.is_some(), - paths = %summarize_node_paths(&self.udp_paths.paths), + paths = %summarize_node_paths(self.udp_paths.paths()), "endpoint handled ping", ); PingHandled { @@ -829,7 +828,7 @@ impl NodeState { // prune candidates are addresses that are not active let mut prune_candidates: Vec<_> = self .udp_paths - .paths + .paths() .iter() .filter(|(_ip_port, state)| !state.is_active()) .map(|(ip_port, state)| (*ip_port, state.last_alive())) @@ -844,7 +843,7 @@ impl NodeState { if prune_count == 0 { // nothing to do, within limits debug!( - paths = %summarize_node_paths(&self.udp_paths.paths), + paths = %summarize_node_paths(self.udp_paths.paths()), "prune addresses: {prune_count} pruned", ); return; @@ -858,7 +857,7 @@ impl NodeState { self.remove_direct_addr(&ip_port, now, "inactive"); } debug!( - paths = %summarize_node_paths(&self.udp_paths.paths), + paths = %summarize_node_paths(self.udp_paths.paths()), "prune addresses: {prune_count} pruned", ); } @@ -867,10 +866,10 @@ impl NodeState { /// assumptions about which paths work. #[instrument("disco", skip_all, fields(node = %self.node_id.fmt_short()))] pub(super) fn note_connectivity_change(&mut self, now: Instant) { - for es in self.udp_paths.paths.values_mut() { + let mut guard = self.udp_paths.access_mut(now); + for es in guard.paths().values_mut() { es.clear(); } - self.udp_paths.update_to_best_addr(now); } /// Handles a Pong message (a reply to an earlier ping). @@ -916,7 +915,7 @@ impl NodeState { match src { SendAddr::Udp(addr) => { - match self.udp_paths.paths.get_mut(&addr.into()) { + match self.udp_paths.access_mut(now).paths().get_mut(&addr.into()) { None => { warn!("ignoring pong: no state for src addr"); // This is no longer an endpoint we care about. @@ -933,7 +932,7 @@ impl NodeState { } } debug!( - paths = %summarize_node_paths(&self.udp_paths.paths), + paths = %summarize_node_paths(self.udp_paths.paths()), "handled pong", ); } @@ -964,7 +963,6 @@ impl NodeState { // TODO(bradfitz): decide how latency vs. preference order affects decision if let SendAddr::Udp(_to) = sp.to { debug_assert!(!is_relay, "mismatching relay & udp"); - self.udp_paths.update_to_best_addr(now); } node_map_insert @@ -986,6 +984,8 @@ impl NodeState { let now = Instant::now(); let mut call_me_maybe_ipps = BTreeSet::new(); + let mut guard = self.udp_paths.access_mut(now); + for peer_sockaddr in &m.my_numbers { if let IpAddr::V6(ip) = peer_sockaddr.ip() { if netwatch::ip::is_unicast_link_local(ip) { @@ -996,8 +996,8 @@ impl NodeState { } let ipp = IpPort::from(*peer_sockaddr); call_me_maybe_ipps.insert(ipp); - self.udp_paths - .paths + guard + .paths() .entry(ipp) .or_insert_with(|| { PathState::new( @@ -1014,7 +1014,7 @@ impl NodeState { // Zero out all the last_ping times to force send_pings to send new ones, even if // it's been less than 5 seconds ago. Also clear pongs for direct addresses not // included in the updated set. - for (ipp, st) in self.udp_paths.paths.iter_mut() { + for (ipp, st) in guard.paths().iter_mut() { st.last_ping = None; if !call_me_maybe_ipps.contains(ipp) { // TODO: This seems like a weird way to signal that the endpoint no longer @@ -1026,13 +1026,12 @@ impl NodeState { } } // Clear trust on our best_addr if it is not included in the updated set. - let changed = self.udp_paths.update_to_best_addr(now); - if changed { + if guard.has_best_addr_changed() { // Clear the last call-me-maybe send time so we will send one again. self.last_call_me_maybe = None; } debug!( - paths = %summarize_node_paths(&self.udp_paths.paths), + paths = %summarize_node_paths(self.udp_paths.paths()), "updated endpoint paths from call-me-maybe", ); self.send_pings(now) @@ -1041,13 +1040,13 @@ impl NodeState { /// Marks this node as having received a UDP payload message. #[cfg(not(wasm_browser))] pub(super) fn receive_udp(&mut self, addr: IpPort, now: Instant) { - let Some(state) = self.udp_paths.paths.get_mut(&addr) else { + let mut guard = self.udp_paths.access_mut(now); + let Some(state) = guard.paths().get_mut(&addr) else { debug_assert!(false, "node map inconsistency by_ip_port <-> direct addr"); return; }; state.receive_payload(now); self.last_used = Some(now); - self.udp_paths.update_to_best_addr(now); } pub(super) fn receive_relay(&mut self, url: &RelayUrl, src: NodeId, now: Instant) { @@ -1078,7 +1077,7 @@ impl NodeState { match addr { SendAddr::Udp(addr) => self .udp_paths - .paths + .paths() .get(&(*addr).into()) .and_then(|ep| ep.last_ping), SendAddr::Relay(url) => self @@ -1175,12 +1174,12 @@ impl NodeState { /// Get the direct addresses for this endpoint. pub(super) fn direct_addresses(&self) -> impl Iterator + '_ { - self.udp_paths.paths.keys().copied() + self.udp_paths.paths().keys().copied() } #[cfg(test)] pub(super) fn direct_address_states(&self) -> impl Iterator + '_ { - self.udp_paths.paths.iter() + self.udp_paths.paths().iter() } pub(super) fn last_used(&self) -> Option { diff --git a/iroh/src/magicsock/node_map/udp_paths.rs b/iroh/src/magicsock/node_map/udp_paths.rs index 29c43ccc99..fcf92b4481 100644 --- a/iroh/src/magicsock/node_map/udp_paths.rs +++ b/iroh/src/magicsock/node_map/udp_paths.rs @@ -76,7 +76,7 @@ impl UdpSendAddr { #[derive(Debug, Default)] pub(super) struct NodeUdpPaths { /// The state for each of this node's direct paths. - pub(super) paths: BTreeMap, + paths: BTreeMap, /// The current address we use to send on. /// /// This is *almost* the same as going through `paths` and finding @@ -89,7 +89,30 @@ pub(super) struct NodeUdpPaths { /// The current best address to send on from all IPv4 addresses we have available. /// /// Follows the same logic as `best` above, but doesn't include any IPv6 addresses. - pub(super) best_ipv4: UdpSendAddr, + best_ipv4: UdpSendAddr, +} + +pub(super) struct MutAccess<'a> { + now: Instant, + inner: &'a mut NodeUdpPaths, +} + +impl<'a> MutAccess<'a> { + pub fn paths(&mut self) -> &mut BTreeMap { + &mut self.inner.paths + } + + pub fn has_best_addr_changed(self) -> bool { + let changed = self.inner.update_to_best_addr(self.now); + std::mem::forget(self); // don't run drop + changed + } +} + +impl Drop for MutAccess<'_> { + fn drop(&mut self) { + self.inner.update_to_best_addr(self.now); + } } impl NodeUdpPaths { @@ -114,12 +137,24 @@ impl NodeUdpPaths { &self.best } + /// Returns a guard for accessing the inner paths mutably. + /// + /// This guard ensures that [`Self::send_addr`] will be updated on drop. + pub(super) fn access_mut(&mut self, now: Instant) -> MutAccess<'_> { + MutAccess { now, inner: self } + } + + /// Returns immutable access to the inner paths. + pub(super) fn paths(&self) -> &BTreeMap { + &self.paths + } + /// Changes the current best address(es) to ones chosen as described in [`Self::best_addr`] docs. /// /// Returns whether one of the best addresses had to change. /// /// This should be called any time that `paths` is modified. - pub(super) fn update_to_best_addr(&mut self, now: Instant) -> bool { + fn update_to_best_addr(&mut self, now: Instant) -> bool { let best_ipv4 = self.best_addr(false, now); let best = self.best_addr(true, now); let mut changed = false; diff --git a/iroh/src/magicsock/transports.rs b/iroh/src/magicsock/transports.rs index c4703b303d..485dbb832d 100644 --- a/iroh/src/magicsock/transports.rs +++ b/iroh/src/magicsock/transports.rs @@ -513,7 +513,7 @@ impl quinn::UdpSender for UdpSender { // come back at any time or missing should be transient so chooses to let // these kind of errors time out. See test_try_send_no_send_addr to try // this out. - error!("no paths available for node, voiding transmit"); + error!("poll_send: no paths available for node, voiding transmit"); return Poll::Ready(Ok(())); } @@ -564,7 +564,7 @@ impl quinn::UdpSender for UdpSender { // come back at any time or missing should be transient so chooses to let // these kind of errors time out. See test_try_send_no_send_addr to try // this out. - error!("no paths available for node, voiding transmit"); + error!("try_send: no paths available for node, voiding transmit"); return Ok(()); } From b4fb12c8f904e1aab903c1adf931281ec1522dc1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philipp=20Kr=C3=BCger?= Date: Thu, 31 Jul 2025 15:46:25 +0200 Subject: [PATCH 2/3] refactor: Never access `NodeUdpState::best` directly --- iroh/src/magicsock.rs | 23 +++++--- iroh/src/magicsock/node_map.rs | 67 +++++++++++++---------- iroh/src/magicsock/node_map/node_state.rs | 17 +++--- iroh/src/magicsock/node_map/udp_paths.rs | 2 +- 4 files changed, 64 insertions(+), 45 deletions(-) diff --git a/iroh/src/magicsock.rs b/iroh/src/magicsock.rs index ba6f50f507..42cb4aace7 100644 --- a/iroh/src/magicsock.rs +++ b/iroh/src/magicsock.rs @@ -393,8 +393,9 @@ impl MagicSock { } } if !addr.is_empty() { + let have_ipv6 = self.ipv6_reported.load(Ordering::Relaxed); self.node_map - .add_node_addr(addr, source, &self.metrics.magicsock); + .add_node_addr(addr, source, have_ipv6, &self.metrics.magicsock); Ok(()) } else if pruned != 0 { Err(EmptyPrunedSnafu { pruned }.build()) @@ -1254,15 +1255,20 @@ impl Handle { let (actor_sender, actor_receiver) = mpsc::channel(256); + let ipv6_reported = false; + // load the node data let node_map = node_map.unwrap_or_default(); - #[cfg(any(test, feature = "test-utils"))] - let node_map = NodeMap::load_from_vec(node_map, path_selection, &metrics.magicsock); - #[cfg(not(any(test, feature = "test-utils")))] - let node_map = NodeMap::load_from_vec(node_map, &metrics.magicsock); + let node_map = NodeMap::load_from_vec( + node_map, + #[cfg(any(test, feature = "test-utils"))] + path_selection, + ipv6_reported, + &metrics.magicsock, + ); let my_relay = Watchable::new(None); - let ipv6_reported = Arc::new(AtomicBool::new(false)); + let ipv6_reported = Arc::new(AtomicBool::new(ipv6_reported)); let max_receive_segments = Arc::new(AtomicUsize::new(1)); let relay_transport = RelayTransport::new(RelayActorConfig { @@ -1861,7 +1867,8 @@ impl Actor { // TODO: this might trigger too many packets at once, pace this self.msock.node_map.prune_inactive(); - let msgs = self.msock.node_map.nodes_stayin_alive(); + let have_v6 = self.netmon_watcher.clone().get().have_v6; + let msgs = self.msock.node_map.nodes_stayin_alive(have_v6); self.handle_ping_actions(&sender, msgs).await; } } @@ -3183,6 +3190,7 @@ mod tests { Source::NamedApp { name: "test".into(), }, + true, &msock_1.metrics.magicsock, ); let addr_2 = msock_1.get_mapping_addr(node_id_2).unwrap(); @@ -3226,6 +3234,7 @@ mod tests { Source::NamedApp { name: "test".into(), }, + true, &msock_1.metrics.magicsock, ); diff --git a/iroh/src/magicsock/node_map.rs b/iroh/src/magicsock/node_map.rs index a67a570c3d..3b61744be2 100644 --- a/iroh/src/magicsock/node_map.rs +++ b/iroh/src/magicsock/node_map.rs @@ -118,20 +118,20 @@ pub enum Source { } impl NodeMap { - #[cfg(not(any(test, feature = "test-utils")))] - /// Create a new [`NodeMap`] from a list of [`NodeAddr`]s. - pub(super) fn load_from_vec(nodes: Vec, metrics: &Metrics) -> Self { - Self::from_inner(NodeMapInner::load_from_vec(nodes, metrics)) - } - - #[cfg(any(test, feature = "test-utils"))] /// Create a new [`NodeMap`] from a list of [`NodeAddr`]s. pub(super) fn load_from_vec( nodes: Vec, - path_selection: PathSelection, + #[cfg(any(test, feature = "test-utils"))] path_selection: PathSelection, + have_ipv6: bool, metrics: &Metrics, ) -> Self { - Self::from_inner(NodeMapInner::load_from_vec(nodes, path_selection, metrics)) + Self::from_inner(NodeMapInner::load_from_vec( + nodes, + #[cfg(any(test, feature = "test-utils"))] + path_selection, + have_ipv6, + metrics, + )) } fn from_inner(inner: NodeMapInner) -> Self { @@ -141,11 +141,17 @@ impl NodeMap { } /// Add the contact information for a node. - pub(super) fn add_node_addr(&self, node_addr: NodeAddr, source: Source, metrics: &Metrics) { + pub(super) fn add_node_addr( + &self, + node_addr: NodeAddr, + source: Source, + have_v6: bool, + metrics: &Metrics, + ) { self.inner .lock() .expect("poisoned") - .add_node_addr(node_addr, source, metrics) + .add_node_addr(node_addr, source, have_v6, metrics) } /// Number of nodes currently listed. @@ -270,11 +276,11 @@ impl NodeMap { } } - pub(super) fn nodes_stayin_alive(&self) -> Vec { + pub(super) fn nodes_stayin_alive(&self, have_ipv6: bool) -> Vec { let mut inner = self.inner.lock().expect("poisoned"); inner .node_states_mut() - .flat_map(|(_idx, node_state)| node_state.stayin_alive()) + .flat_map(|(_idx, node_state)| node_state.stayin_alive(have_ipv6)) .collect() } @@ -320,36 +326,33 @@ impl NodeMap { } impl NodeMapInner { - #[cfg(not(any(test, feature = "test-utils")))] - /// Create a new [`NodeMap`] from a list of [`NodeAddr`]s. - fn load_from_vec(nodes: Vec, metrics: &Metrics) -> Self { - let mut me = Self::default(); - for node_addr in nodes { - me.add_node_addr(node_addr, Source::Saved, metrics); - } - me - } - - #[cfg(any(test, feature = "test-utils"))] /// Create a new [`NodeMap`] from a list of [`NodeAddr`]s. fn load_from_vec( nodes: Vec, - path_selection: PathSelection, + #[cfg(any(test, feature = "test-utils"))] path_selection: PathSelection, + have_ipv6: bool, metrics: &Metrics, ) -> Self { let mut me = Self { + #[cfg(any(test, feature = "test-utils"))] path_selection, ..Default::default() }; for node_addr in nodes { - me.add_node_addr(node_addr, Source::Saved, metrics); + me.add_node_addr(node_addr, Source::Saved, have_ipv6, metrics); } me } /// Add the contact information for a node. #[instrument(skip_all, fields(node = %node_addr.node_id.fmt_short()))] - fn add_node_addr(&mut self, node_addr: NodeAddr, source: Source, metrics: &Metrics) { + fn add_node_addr( + &mut self, + node_addr: NodeAddr, + source: Source, + have_ipv6: bool, + metrics: &Metrics, + ) { let source0 = source.clone(); let node_id = node_addr.node_id; let relay_url = node_addr.relay_url.clone(); @@ -367,6 +370,7 @@ impl NodeMapInner { node_addr.relay_url.as_ref(), &node_addr.direct_addresses, source0, + have_ipv6, metrics, ); let id = node_state.id(); @@ -715,6 +719,7 @@ mod tests { Source::NamedApp { name: "test".into(), }, + true, &Default::default(), ) } @@ -761,8 +766,12 @@ mod tests { Some(addr) }) .collect(); - let loaded_node_map = - NodeMap::load_from_vec(addrs.clone(), PathSelection::default(), &Default::default()); + let loaded_node_map = NodeMap::load_from_vec( + addrs.clone(), + PathSelection::default(), + true, + &Default::default(), + ); let mut loaded: Vec = loaded_node_map .list_remote_infos(Instant::now()) diff --git a/iroh/src/magicsock/node_map/node_state.rs b/iroh/src/magicsock/node_map/node_state.rs index e9ece2f501..9226414b4f 100644 --- a/iroh/src/magicsock/node_map/node_state.rs +++ b/iroh/src/magicsock/node_map/node_state.rs @@ -392,13 +392,13 @@ impl NodeState { /// of the endpoint. The [`NodeState::send_call_me_maybe`] function takes care of this. #[cfg(not(wasm_browser))] #[instrument("want_call_me_maybe", skip_all)] - fn want_call_me_maybe(&self, now: &Instant) -> bool { + fn want_call_me_maybe(&self, now: &Instant, have_ipv6: bool) -> bool { trace!("full ping: wanted?"); let Some(last_full_ping) = self.last_full_ping else { debug!("no previous full ping: need full ping"); return true; }; - match &self.udp_paths.best { + match &self.udp_paths.send_addr(have_ipv6) { UdpSendAddr::None | UdpSendAddr::Unconfirmed(_) => { debug!("best addr not set: need full ping"); true @@ -661,10 +661,11 @@ impl NodeState { new_relay_url: Option<&RelayUrl>, new_addrs: &BTreeSet, source: super::Source, + have_ipv6: bool, metrics: &MagicsockMetrics, ) { if matches!( - self.udp_paths.best, + self.udp_paths.send_addr(have_ipv6), UdpSendAddr::None | UdpSendAddr::Unconfirmed(_) ) { // we do not have a direct connection, so changing the relay information may @@ -795,7 +796,7 @@ impl NodeState { // if the endpoint does not yet have a best_addr let needs_ping_back = if matches!(path, SendAddr::Udp(_)) && matches!( - self.udp_paths.best, + self.udp_paths.send_addr(true), UdpSendAddr::None | UdpSendAddr::Unconfirmed(_) | UdpSendAddr::Outdated(_) ) { // We also need to send a ping to make this path available to us as well. This @@ -1099,7 +1100,7 @@ impl NodeState { /// Send a heartbeat to the node to keep the connection alive, or trigger a full ping /// if necessary. #[instrument("stayin_alive", skip_all, fields(node = %self.node_id.fmt_short()))] - pub(super) fn stayin_alive(&mut self) -> Vec { + pub(super) fn stayin_alive(&mut self, have_ipv6: bool) -> Vec { trace!("stayin_alive"); let now = Instant::now(); if !self.is_active(&now) { @@ -1108,13 +1109,13 @@ impl NodeState { } // If we do not have an optimal addr, send pings to all known places. - if self.want_call_me_maybe(&now) { + if self.want_call_me_maybe(&now, have_ipv6) { debug!("sending a call-me-maybe"); return self.send_call_me_maybe(now, SendCallMeMaybe::Always); } // Send heartbeat ping to keep the current addr going as long as we need it. - if let Some(udp_addr) = self.udp_paths.best.get_addr() { + if let Some(udp_addr) = self.udp_paths.send_addr(have_ipv6).get_addr() { let elapsed = self.last_ping(&SendAddr::Udp(udp_addr)).map(|l| now - l); // Send a ping if the last ping is older than 2 seconds. let needs_ping = match elapsed { @@ -1158,7 +1159,7 @@ impl NodeState { } let (udp_addr, relay_url) = self.addr_for_send(have_ipv6, metrics); - let ping_msgs = if self.want_call_me_maybe(&now) { + let ping_msgs = if self.want_call_me_maybe(&now, have_ipv6) { self.send_call_me_maybe(now, SendCallMeMaybe::IfNoRecent) } else { Vec::new() diff --git a/iroh/src/magicsock/node_map/udp_paths.rs b/iroh/src/magicsock/node_map/udp_paths.rs index fcf92b4481..9953116d4d 100644 --- a/iroh/src/magicsock/node_map/udp_paths.rs +++ b/iroh/src/magicsock/node_map/udp_paths.rs @@ -85,7 +85,7 @@ pub(super) struct NodeUdpPaths { /// 2. Slightly sticky: It only changes when /// - the current send addr is not a validated path anymore or /// - we received a pong with lower latency. - pub(super) best: UdpSendAddr, + best: UdpSendAddr, /// The current best address to send on from all IPv4 addresses we have available. /// /// Follows the same logic as `best` above, but doesn't include any IPv6 addresses. From 893084a8bc600a43dc6c68ef730041c4c59961f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philipp=20Kr=C3=BCger?= Date: Thu, 31 Jul 2025 15:58:29 +0200 Subject: [PATCH 3/3] Fix wasm types, cleanup --- iroh-relay/src/protos/handshake.rs | 2 ++ iroh/src/magicsock/node_map/node_state.rs | 3 +-- iroh/src/magicsock/transports.rs | 4 ++-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/iroh-relay/src/protos/handshake.rs b/iroh-relay/src/protos/handshake.rs index 295e0ec1a6..e5bbdede1c 100644 --- a/iroh-relay/src/protos/handshake.rs +++ b/iroh-relay/src/protos/handshake.rs @@ -26,6 +26,7 @@ //! [RFC 5705]: https://datatracker.ietf.org/doc/html/rfc5705 use bytes::{BufMut, Bytes, BytesMut}; use data_encoding::BASE32HEX_NOPAD as HEX; +#[cfg(not(wasm_browser))] use http::HeaderValue; #[cfg(feature = "server")] use iroh_base::Signature; @@ -47,6 +48,7 @@ use crate::ExportKeyingMaterial; const DOMAIN_SEP_CHALLENGE: &str = "iroh-relay handshake v1 challenge signature"; /// Domain separation label for [`KeyMaterialClientAuth`]'s use of [`ExportKeyingMaterial`] +#[cfg(not(wasm_browser))] const DOMAIN_SEP_TLS_EXPORT_LABEL: &[u8] = b"iroh-relay handshake v1"; /// Authentication message from the client. diff --git a/iroh/src/magicsock/node_map/node_state.rs b/iroh/src/magicsock/node_map/node_state.rs index 9226414b4f..ec21c94d4f 100644 --- a/iroh/src/magicsock/node_map/node_state.rs +++ b/iroh/src/magicsock/node_map/node_state.rs @@ -430,7 +430,7 @@ impl NodeState { } #[cfg(wasm_browser)] - fn want_call_me_maybe(&self, _now: &Instant) -> bool { + fn want_call_me_maybe(&self, _now: &Instant, _have_ipv6: bool) -> bool { trace!("full ping: skipped in browser"); false } @@ -1026,7 +1026,6 @@ impl NodeState { } } } - // Clear trust on our best_addr if it is not included in the updated set. if guard.has_best_addr_changed() { // Clear the last call-me-maybe send time so we will send one again. self.last_call_me_maybe = None; diff --git a/iroh/src/magicsock/transports.rs b/iroh/src/magicsock/transports.rs index 485dbb832d..c4703b303d 100644 --- a/iroh/src/magicsock/transports.rs +++ b/iroh/src/magicsock/transports.rs @@ -513,7 +513,7 @@ impl quinn::UdpSender for UdpSender { // come back at any time or missing should be transient so chooses to let // these kind of errors time out. See test_try_send_no_send_addr to try // this out. - error!("poll_send: no paths available for node, voiding transmit"); + error!("no paths available for node, voiding transmit"); return Poll::Ready(Ok(())); } @@ -564,7 +564,7 @@ impl quinn::UdpSender for UdpSender { // come back at any time or missing should be transient so chooses to let // these kind of errors time out. See test_try_send_no_send_addr to try // this out. - error!("try_send: no paths available for node, voiding transmit"); + error!("no paths available for node, voiding transmit"); return Ok(()); }