From 99269e7f06982ddb45fe3f99afc8d73c96922074 Mon Sep 17 00:00:00 2001 From: Kasey Date: Thu, 23 May 2024 16:05:29 -0400 Subject: [PATCH 1/2] return `Poll::Pending` when we have no relay URL or direct addresses to send on --- iroh-net/src/magicsock.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/iroh-net/src/magicsock.rs b/iroh-net/src/magicsock.rs index 080e99b985..814a79ea6c 100644 --- a/iroh-net/src/magicsock.rs +++ b/iroh-net/src/magicsock.rs @@ -531,12 +531,10 @@ impl MagicSock { } if udp_addr.is_none() && relay_url.is_none() { - // Handle no addresses being available - warn!(node = %public_key.fmt_short(), "failed to send: no UDP or relay addr"); - return Poll::Ready(Err(io::Error::new( - io::ErrorKind::NotConnected, - "no UDP or relay address available for node", - ))); + // Returning an error here would lock up the entire `Endpoint`. + // Instead, log an error and return `Poll::Pending`, the connection will timeout. + error!(node = %public_key.fmt_short(), "failed to send: no UDP or relay addr"); + return Poll::Pending; } if (udp_addr.is_none() || udp_pending) && (relay_url.is_none() || relay_pending) { @@ -549,14 +547,16 @@ impl MagicSock { } if !relay_sent && !udp_sent && !pings_sent { - warn!(node = %public_key.fmt_short(), "failed to send: no UDP or relay addr"); + // Returning an error here would lock up the entire `Endpoint`. + // Instead, log an error and return `Poll::Pending`, the connection will timeout. let err = udp_error.unwrap_or_else(|| { io::Error::new( io::ErrorKind::NotConnected, "no UDP or relay address available for node", ) }); - return Poll::Ready(Err(err)); + error!(node = %public_key.fmt_short(), "{err:?}"); + return Poll::Pending; } trace!( From cb6d75bb2ded5f791f97abc82c6ca2bdc41c17ee Mon Sep 17 00:00:00 2001 From: Kasey Date: Wed, 5 Jun 2024 14:22:14 -0400 Subject: [PATCH 2/2] fix(iroh-net): return `Poll::Ready(Ok(n))` when we do not have an address for a node id --- iroh-net/src/magicsock.rs | 14 ++++++++++++-- iroh-net/src/net/interfaces/bsd.rs | 2 +- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/iroh-net/src/magicsock.rs b/iroh-net/src/magicsock.rs index 814a79ea6c..7437c87694 100644 --- a/iroh-net/src/magicsock.rs +++ b/iroh-net/src/magicsock.rs @@ -532,9 +532,19 @@ impl MagicSock { if udp_addr.is_none() && relay_url.is_none() { // Returning an error here would lock up the entire `Endpoint`. - // Instead, log an error and return `Poll::Pending`, the connection will timeout. + // + // If we returned `Poll::Pending`, the waker driving the `poll_send` will never get woken up. + // + // Our best bet here is to log an error and return `Poll::Ready(Ok(n))`. + // + // `n` is the number of consecutive transmits in this batch that are meant for the same destination (a destination that we have no addresses for, and so we can never actually send). + // + // When we return `Poll::Ready(Ok(n))`, we are effectively dropping those n messages, by lying to QUIC and saying they were sent. + // (If we returned `Poll::Ready(Ok(0))` instead, QUIC would loop to attempt to re-send those messages, blocking other traffic.) + // + // When `QUIC` gets no `ACK`s for those messages, the connection will eventually timeout. error!(node = %public_key.fmt_short(), "failed to send: no UDP or relay addr"); - return Poll::Pending; + return Poll::Ready(Ok(n)); } if (udp_addr.is_none() || udp_pending) && (relay_url.is_none() || relay_pending) { diff --git a/iroh-net/src/net/interfaces/bsd.rs b/iroh-net/src/net/interfaces/bsd.rs index dd6ca7e3ca..7ef0cd1eb0 100644 --- a/iroh-net/src/net/interfaces/bsd.rs +++ b/iroh-net/src/net/interfaces/bsd.rs @@ -300,7 +300,7 @@ impl WireFormat { Ok(Some(WireMessage::Route(m))) } - #[cfg(any(target_os = "openbsd",))] + #[cfg(target_os = "openbsd")] MessageType::Route => { if data.len() < self.body_off { return Err(RouteError::MessageTooShort);