simplify QAD addresses in NodeMap

Author: “ramfox”

iroh-net-report/Cargo.toml

@@ -49,7 +49,6 @@ tokio = { version = "1", default-features = false, features = ["test-util"] }
 [features]
 default = ["metrics"]
 metrics = ["iroh-metrics/metrics", "portmapper/metrics"]
-iroh = []
 
 [package.metadata.docs.rs]
 all-features = true

iroh-net-report/src/lib.rs

@@ -38,7 +38,6 @@ mod ping;
 mod reportgen;
 
 pub use metrics::Metrics;
-pub use reportgen::MappedAddr;
 use reportgen::ProbeProto;
 pub use reportgen::QuicConfig;
 

iroh-net-report/src/reportgen.rs

@@ -700,25 +700,8 @@ pub struct QuicConfig {
     pub ipv4: bool,
     /// Enable ipv6 QUIC address discovery probes
     pub ipv6: bool,
-    /// A map of RelayUrls to addresses
-    ///
-    /// Only makes sense in the context of iroh, which uses QuicMappedAddrs
-    /// to allow dialing by NodeId
-    #[cfg(feature = "iroh")]
-    pub mapped_addrs: MappedRelayAddrs,
 }
 
-/// Holds a QuicMappedAddr
-#[cfg(feature = "iroh")]
-#[derive(Debug, Clone, Copy)]
-pub struct MappedAddr(pub SocketAddr);
-
-/// A relationship between a socket address and it's QUIC mapped address
-///
-/// Only relevant when using the net-report with iroh.
-#[cfg(feature = "iroh")]
-type MappedRelayAddrs = std::collections::HashMap<SocketAddr, MappedAddr>;
-
 /// Executes a particular [`Probe`], including using a delayed start if needed.
 ///
 /// If *stun_sock4* and *stun_sock6* are `None` the STUN probes are disabled.
@@ -941,13 +924,6 @@ async fn run_quic_probe(
         }
     };
 
-    // if we are using net-report with iroh, we must dial using the mapped address
-    #[cfg(feature = "iroh")]
-    let relay_addr = quic_config
-        .mapped_addrs
-        .get(&relay_addr)
-        .map_or(relay_addr, |mapped_addr| mapped_addr.0.clone());
-
     let quic_client = iroh_relay::quic::QuicClient::new(quic_config.ep, quic_config.client_config)
         .map_err(|e| ProbeError::Error(e, probe.clone()))?;
     let (addr, latency) = quic_client
@@ -1617,7 +1593,6 @@ mod tests {
             client_config,
             ipv4: true,
             ipv6: true,
-            mapped_addrs: std::collections::HashMap::new(),
         };
         let url = relay.url.clone();
         let port = server.quic_addr().unwrap().port();

iroh/Cargo.toml

@@ -106,7 +106,7 @@ webpki = { package = "rustls-webpki", version = "0.102" }
 webpki-roots = "0.26"
 x509-parser = "0.16"
 z32 = "1.0.3"
-net-report = { package = "iroh-net-report", path = "../iroh-net-report", version = "0.29", default-features = false, features = ["iroh"] }
+net-report = { package = "iroh-net-report", path = "../iroh-net-report", version = "0.29", default-features = false }
 
 # metrics
 iroh-metrics = { version = "0.29", default-features = false }

iroh/examples/listen.rs

@@ -28,7 +28,7 @@ async fn main() -> anyhow::Result<()> {
         // Use `RelayMode::Custom` to pass in a `RelayMap` with custom relay urls.
         // Use `RelayMode::Disable` to disable holepunching and relaying over HTTPS
         // If you want to experiment with relaying using your own relay server, you must pass in the same custom relay url to both the `listen` code AND the `connect` code
-        .relay_mode(RelayMode::Staging)
+        .relay_mode(RelayMode::Default)
         // you can choose a port to bind to, but passing in `0` will bind the socket to a random available port
         .bind()
         .await?;

iroh/src/magicsock.rs

@@ -565,8 +565,8 @@ impl MagicSock {
                 }
             }
             None => {
-                // Check if this is a QUIC address discovery packet
-                if let Some(addr) = self.node_map.get_qad_addr(&dest) {
+                // Check if this is addr is used to perform QUIC Address Discovery
+                if let Some(addr) = self.node_map.qad_addr_for_send(&dest.0) {
                     // send udp
                     // rewrite target address
                     transmit.destination = addr;
@@ -828,16 +828,16 @@ impl MagicSock {
                 // Update the NodeMap and remap RecvMeta to the QuicMappedAddr.
                 match self.node_map.receive_udp(meta.addr) {
                     None => {
-                        // Check if this is QUIC address discovery response
-                        if let Some(quic_mapped_addr) = self.node_map.receive_qad(meta.addr) {
+                        // Check if this address is used for QUIC address discovery
+                        if let Some(addr) = self.node_map.qad_addr_for_recv(&meta.addr) {
                             trace!(
                                 src = ?meta.addr,
                                 count = %quic_datagram_count,
                                 len = meta.len,
                                 "UDP recv QUIC address discovery packets",
                             );
                             quic_packets_total += quic_datagram_count;
-                            meta.addr = quic_mapped_addr.0;
+                            meta.addr = addr;
                         } else {
                             warn!(
                                 src = ?meta.addr,
@@ -2049,6 +2049,9 @@ impl Actor {
             }
         }
 
+        // kick off resolving the URLs for relay addresses
+        self.resolve_qad_addrs(Duration::from_millis(10)).await;
+
         let mut receiver_closed = false;
         let mut portmap_watcher_closed = false;
         let mut link_change_closed = false;
@@ -2429,8 +2432,7 @@ impl Actor {
 
         // Need to add Quic Mapped Addrs for the relay nodes to use for
         // QUIC Address Discovery
-        let mapped_addrs = self
-            .resolve_qad_addrs(std::time::Duration::from_millis(500))
+        self.resolve_qad_addrs(std::time::Duration::from_millis(300))
             .await;
         // create a client config for the endpoint to
         // use for QUIC address discovery
@@ -2444,7 +2446,6 @@ impl Actor {
             client_config,
             ipv4: true,
             ipv6: self.pconn6.is_some(),
-            mapped_addrs,
         });
         opts = opts.quic_config(quic_config);
 
@@ -2546,7 +2547,7 @@ impl Actor {
 
         match relay_node.url.host() {
             Some(url::Host::Domain(hostname)) => {
-                error!(%hostname, "Performing DNS A lookup for relay addr");
+                debug!(%hostname, "Performing DNS A lookup for relay addr");
                 let mut set = JoinSet::new();
                 let resolver = dns_resolver.clone();
                 let ipv4_resolver = resolver.clone();
@@ -2566,13 +2567,13 @@ impl Actor {
                         Err(_) => {}
                         Ok(resolved_addrs) => {
                             for addr in resolved_addrs {
-                                addrs.insert(SocketAddr::new(addr.into(), port));
+                                addrs.insert(SocketAddr::new(addr, port));
                             }
                         }
                     }
                 }
                 if addrs.is_empty() {
-                    error!(%hostname, "Unable to resolve ip addresses for relay node");
+                    debug!(%hostname, "Unable to resolve ip addresses for relay node");
                 }
             }
             Some(url::Host::Ipv4(addr)) => {
@@ -2585,15 +2586,11 @@ impl Actor {
                 error!(?relay_node.url, "No hostname for relay node, cannot resolve ip");
             }
         }
-        return addrs;
+        addrs
     }
 
     /// Resolve the relay addresses used for QUIC address discovery.
-    async fn resolve_qad_addrs(
-        &mut self,
-        duration: Duration,
-    ) -> HashMap<SocketAddr, net_report::MappedAddr> {
-        let mut mapped_addrs = HashMap::new();
+    async fn resolve_qad_addrs(&mut self, duration: Duration) {
         let mut set = JoinSet::new();
         let resolver = self.msock.dns_resolver();
         for relay_node in self.msock.relay_map.nodes() {
@@ -2606,11 +2603,9 @@ impl Actor {
         let res = set.join_all().await;
         for addrs in res {
             addrs.iter().for_each(|addr| {
-                let mapped_addr = self.msock.node_map.add_qad_addr(*addr);
-                mapped_addrs.insert(*addr, net_report::MappedAddr(mapped_addr.0));
+                self.msock.node_map.add_qad_addr(*addr);
             });
         }
-        mapped_addrs
     }
 
     fn set_nearest_relay(&mut self, relay_url: Option<RelayUrl>) -> bool {

iroh/src/magicsock/node_map.rs

@@ -66,7 +66,7 @@ pub(super) struct NodeMapInner {
     by_id: HashMap<usize, NodeState>,
     next_id: usize,
     // special mapping for relay socket addresses so that we can do quic address discovery
-    qad_mapped_addrs: HashMap<SocketAddr, QuicMappedAddr>,
+    qad_addrs: BTreeSet<SocketAddr>,
 }
 
 /// Identifier to look up a [`NodeState`] in the [`NodeMap`].
@@ -145,46 +145,65 @@ impl NodeMap {
     }
 
     /// Add a the SocketAddr used to preform QUIC Address Discovery to the nodemap
-    pub(super) fn add_qad_addr(&self, udp_addr: SocketAddr) -> QuicMappedAddr {
-        let quic_mapped_addr = QuicMappedAddr::generate();
+    pub(super) fn add_qad_addr(&self, udp_addr: SocketAddr) {
         self.inner
             .lock()
             .expect("poisoned")
-            .qad_mapped_addrs
-            .insert(udp_addr, quic_mapped_addr);
-        quic_mapped_addr
+            .qad_addrs
+            .insert(udp_addr);
     }
 
-    /// Get the socket address used to preform QUIC Address Discovery
-    pub(super) fn get_qad_addr(&self, addr: &QuicMappedAddr) -> Option<SocketAddr> {
-        self.inner
+    /// Return a correctly canonicalized SocketAddr if this address is one
+    /// used to perform QUIC Address Discovery
+    pub(super) fn qad_addr_for_send(&self, addr: &SocketAddr) -> Option<SocketAddr> {
+        // all mapped addresses are Ipv6 addresses,
+        // so we need to canonicalize before check
+        let canonicalized_addr = SocketAddr::new(addr.ip().to_canonical(), addr.port());
+        if self
+            .inner
+            .lock()
+            .expect("poisoned")
+            .qad_addrs
+            .contains(&canonicalized_addr)
+        {
+            Some(canonicalized_addr)
+        } else {
+            None
+        }
+    }
+
+    /// Return a correctly formed SocketAddr if this adddress is one used to
+    /// perform QUIC Address Discovery
+    pub(super) fn qad_addr_for_recv(&self, addr: &SocketAddr) -> Option<SocketAddr> {
+        if self
+            .inner
             .lock()
             .expect("poisoned")
-            .qad_mapped_addrs
-            .iter()
-            .find_map(|(udp_addr, quic_mapped_addr)| {
-                if addr == quic_mapped_addr {
-                    Some(*udp_addr)
-                } else {
-                    None
+            .qad_addrs
+            .contains(addr)
+        {
+            match addr.ip() {
+                IpAddr::V4(ipv4_addr) => {
+                    // if this is an ipv4 addr, we need to map it back to
+                    // an ipv6 addr, since all addresses we use to dial on
+                    // the underlying quinn endpoint are mapped ipv6 addrs
+                    Some(SocketAddr::new(
+                        ipv4_addr.to_ipv6_mapped().into(),
+                        addr.port(),
+                    ))
                 }
-            })
+                IpAddr::V6(_) => Some(*addr),
+            }
+        } else {
+            None
+        }
     }
 
     /// Number of nodes currently listed.
     pub(super) fn node_count(&self) -> usize {
         self.inner.lock().expect("poisoned").node_count()
     }
 
-    pub(super) fn receive_qad(&self, udp_addr: SocketAddr) -> Option<QuicMappedAddr> {
-        self.inner
-            .lock()
-            .expect("poisoned")
-            .qad_mapped_addrs
-            .get(&udp_addr)
-            .map(|addr| *addr)
-    }
-
     pub(super) fn receive_udp(&self, udp_addr: SocketAddr) -> Option<(PublicKey, QuicMappedAddr)> {
         self.inner.lock().expect("poisoned").receive_udp(udp_addr)
     }

iroh/src/magicsock/node_map/node_state.rs

@@ -1671,7 +1671,7 @@ mod tests {
                 (d_endpoint.id, d_endpoint),
             ]),
             next_id: 5,
-            qad_mapped_addrs: HashMap::new(),
+            qad_addrs: BTreeSet::new(),
         });
         let mut got = node_map.list_remote_infos(later);
         got.sort_by_key(|p| p.node_id);