From b6517f0a7cb09f03828b0b900bd951855263bd02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9Cramfox=E2=80=9D?= <“kasey@n0.computer”> Date: Wed, 11 Dec 2024 19:58:47 -0500 Subject: [PATCH 1/7] add `net_report::Options` to allow specifying which probes you want to run --- iroh-net-report/src/lib.rs | 165 +++++++++++++++-------- iroh-net-report/src/reportgen.rs | 15 ++- iroh-net-report/src/reportgen/probes.rs | 170 ++++++++++++++++++++---- iroh/src/magicsock.rs | 9 +- 4 files changed, 276 insertions(+), 83 deletions(-) diff --git a/iroh-net-report/src/lib.rs b/iroh-net-report/src/lib.rs index 4379590ac5..aa584d5a6e 100644 --- a/iroh-net-report/src/lib.rs +++ b/iroh-net-report/src/lib.rs @@ -10,7 +10,7 @@ #![cfg_attr(not(test), deny(clippy::unwrap_used))] use std::{ - collections::{BTreeMap, HashMap}, + collections::{BTreeMap, BTreeSet, HashMap}, fmt::{self, Debug}, net::{SocketAddr, SocketAddrV4, SocketAddrV6}, sync::Arc, @@ -38,6 +38,7 @@ mod ping; mod reportgen; pub use metrics::Metrics; +pub use reportgen::ProbeProto as ProbeProtocol; pub use reportgen::QuicConfig; const FULL_REPORT_INTERVAL: Duration = Duration::from_secs(5 * 60); @@ -206,6 +207,59 @@ impl Default for Reports { } } +#[derive(Debug)] +/// Options for running a report +pub struct Options { + /// The relay configuration. + pub relay_map: RelayMap, + /// Socket to send IPv4 STUN probes from. + /// + /// Responses are never read from this socket, they must be passed in via internal + /// messaging since, when used internally in iroh, the socket is also used to receive + /// other packets from in the magicsocket (`MagicSock`). + /// + /// If not provided and: + /// - no probes are indicated in the *probes* field, + /// - or [`ProbeProto::StunIpv4`] probes are explicitly added to the *probes* list, + /// then the client will attempt to bind a suitable socket itself. + pub stun_sock_v4: Option>, + /// Socket to send IPv6 STUN probes from. + /// + /// Responses are never read from this socket, they must be passed in via internal + /// messaging since, when used internally in iroh, the socket is also used to receive + /// other packets from in the magicsocket (`MagicSock`). + /// + /// If not provided and: + /// - no probes are indicated in the *probes* field, + /// - or [`ProbeProtocol::StunIpv6`] probes are explicitly added to the *probes* list, + /// then the client will attempt to bind a suitable socket itself. + pub stun_sock_v6: Option>, + /// Endpoint and client configuration to create a QUIC + /// connection to do QUIC address discovery. + /// + /// If not provided, will not do QUIC address discovery, even if indicated in the *probes* set. + pub quic_config: Option, + /// An empty protocols list indicates running all possible probe protocols. + /// + /// Otherwise, *protocols* is a set of [`ProbeProtocol`]s that should be run in the report. + pub protocols: BTreeSet, +} + +impl Options { + /// The default probe protocols used in a report + pub fn default_protocols() -> BTreeSet { + BTreeSet::from([ + ProbeProtocol::StunIpv4, + ProbeProtocol::StunIpv6, + ProbeProtocol::Https, + ProbeProtocol::IcmpV4, + ProbeProtocol::IcmpV6, + ProbeProtocol::QuicIpv4, + ProbeProtocol::QuicIpv6, + ]) + } +} + impl Client { /// Creates a new net_report client. /// @@ -252,15 +306,23 @@ impl Client { /// using QUIC address discovery. /// /// When `None`, it will disable the QUIC address discovery probes. + /// + /// This will attempt to use *all* probe protocols. pub async fn get_report( &mut self, - dm: RelayMap, - stun_conn4: Option>, - stun_conn6: Option>, + relay_map: RelayMap, + stun_sock_v4: Option>, + stun_sock_v6: Option>, quic_config: Option, ) -> Result> { let rx = self - .get_report_channel(dm, stun_conn4, stun_conn6, quic_config) + .get_report_channel(Options { + relay_map, + stun_sock_v4, + stun_sock_v6, + quic_config, + protocols: Options::default_protocols(), + }) .await?; match rx.await { Ok(res) => res, @@ -268,23 +330,31 @@ impl Client { } } + /// Runs a net_report, returning the report. + /// + /// It may not be called concurrently with itself, `&mut self` takes care of that. + /// + /// Look at [`Options`] for the different configuration options. Use the + /// `Options::protocols` field to specify which protocols to attempt to run. + pub async fn get_report_with_opts(&mut self, opts: Options) -> Result> { + let rx = self.get_report_channel(opts).await?; + match rx.await { + Ok(res) => res, + Err(_) => Err(anyhow!("channel closed, actor awol")), + } + } + /// Get report with channel pub async fn get_report_channel( &mut self, - dm: RelayMap, - stun_conn4: Option>, - stun_conn6: Option>, - quic_config: Option, + opts: Options, ) -> Result>>> { // TODO: consider if RelayMap should be made to easily clone? It seems expensive // right now. let (tx, rx) = oneshot::channel(); self.addr .send(Message::RunCheck { - relay_map: dm, - stun_sock_v4: stun_conn4, - stun_sock_v6: stun_conn6, - quic_config, + opts, response_tx: tx, }) .await?; @@ -310,25 +380,8 @@ pub(crate) enum Message { /// Only one net_report can be run at a time, trying to run multiple concurrently will /// fail. RunCheck { - /// The relay configuration. - relay_map: RelayMap, - /// Socket to send IPv4 STUN probes from. - /// - /// Responses are never read from this socket, they must be passed in via the - /// [`Message::StunPacket`] message since the socket is also used to receive - /// other packets from in the magicsocket (`MagicSock`). - /// - /// If not provided this will attempt to bind a suitable socket itself. - stun_sock_v4: Option>, - /// Socket to send IPv6 STUN probes from. - /// - /// Like `stun_sock_v4` but for IPv6. - stun_sock_v6: Option>, - /// Endpoint and client configuration to create a QUIC - /// connection to do QUIC address discovery. - /// - /// If not provided, will not do QUIC address discovery. - quic_config: Option, + /// Options for the report + opts: Options, /// Channel to receive the response. response_tx: oneshot::Sender>>, }, @@ -464,20 +517,8 @@ impl Actor { while let Some(msg) = self.receiver.recv().await { trace!(?msg, "handling message"); match msg { - Message::RunCheck { - relay_map, - stun_sock_v4, - stun_sock_v6, - quic_config, - response_tx, - } => { - self.handle_run_check( - relay_map, - stun_sock_v4, - stun_sock_v6, - quic_config, - response_tx, - ); + Message::RunCheck { opts, response_tx } => { + self.handle_run_check(opts, response_tx); } Message::ReportReady { report } => { self.handle_report_ready(report); @@ -502,12 +543,21 @@ impl Actor { /// sockets you will be using. fn handle_run_check( &mut self, - relay_map: RelayMap, - stun_sock_v4: Option>, - stun_sock_v6: Option>, - quic_config: Option, + opts: Options, response_tx: oneshot::Sender>>, ) { + let Options { + relay_map, + stun_sock_v4, + stun_sock_v6, + quic_config, + protocols, + } = opts; + let protos = if protocols.is_empty() { + Options::default_protocols() + } else { + protocols + }; if self.current_report_run.is_some() { response_tx .send(Err(anyhow!( @@ -520,13 +570,15 @@ impl Actor { let now = Instant::now(); let cancel_token = CancellationToken::new(); - let stun_sock_v4 = match stun_sock_v4 { - Some(sock) => Some(sock), - None => bind_local_stun_socket(IpFamily::V4, self.addr(), cancel_token.clone()), + let stun_sock_v4 = match (stun_sock_v4, protos.contains(&ProbeProtocol::StunIpv4)) { + (Some(sock), true) => Some(sock), + (None, true) => bind_local_stun_socket(IpFamily::V4, self.addr(), cancel_token.clone()), + _ => None, }; - let stun_sock_v6 = match stun_sock_v6 { - Some(sock) => Some(sock), - None => bind_local_stun_socket(IpFamily::V6, self.addr(), cancel_token.clone()), + let stun_sock_v6 = match (stun_sock_v6, protos.contains(&ProbeProtocol::StunIpv6)) { + (Some(sock), true) => Some(sock), + (None, true) => bind_local_stun_socket(IpFamily::V6, self.addr(), cancel_token.clone()), + _ => None, }; let mut do_full = self.reports.next_full || now.duration_since(self.reports.last_full) > FULL_REPORT_INTERVAL; @@ -558,6 +610,7 @@ impl Actor { stun_sock_v6, quic_config, self.dns_resolver.clone(), + protos, ); self.current_report_run = Some(ReportRun { diff --git a/iroh-net-report/src/reportgen.rs b/iroh-net-report/src/reportgen.rs index 67491edcac..0fe7505d04 100644 --- a/iroh-net-report/src/reportgen.rs +++ b/iroh-net-report/src/reportgen.rs @@ -17,6 +17,7 @@ //! - Sends the completed report to the net_report actor. use std::{ + collections::BTreeSet, future::Future, net::{IpAddr, SocketAddr}, pin::Pin, @@ -59,7 +60,8 @@ use crate::{ mod hairpin; mod probes; -use probes::{Probe, ProbePlan, ProbeProto}; +pub use probes::ProbeProto; +use probes::{Probe, ProbePlan}; use crate::defaults::timeouts::{ CAPTIVE_PORTAL_DELAY, CAPTIVE_PORTAL_TIMEOUT, OVERALL_REPORT_TIMEOUT, PROBES_TIMEOUT, @@ -91,6 +93,7 @@ impl Client { stun_sock6: Option>, quic_config: Option, dns_resolver: DnsResolver, + protocols: BTreeSet, ) -> Self { let (msg_tx, msg_rx) = mpsc::channel(32); let addr = Addr { @@ -110,6 +113,7 @@ impl Client { hairpin_actor: hairpin::Client::new(net_report, addr), outstanding_tasks: OutstandingTasks::default(), dns_resolver, + protocols, }; let task = tokio::spawn( async move { actor.run().await }.instrument(info_span!("reportgen.actor")), @@ -193,6 +197,9 @@ struct Actor { outstanding_tasks: OutstandingTasks, /// The DNS resolver to use for probes that need to resolve DNS records. dns_resolver: DnsResolver, + /// Protocols we should attempt to create probes for, if we have the correct + /// configuration for that protocol. + protocols: BTreeSet, } impl Actor { @@ -536,8 +543,10 @@ impl Actor { let if_state = interfaces::State::new().await; debug!(%if_state, "Local interfaces"); let plan = match self.last_report { - Some(ref report) => ProbePlan::with_last_report(&self.relay_map, &if_state, report), - None => ProbePlan::initial(&self.relay_map, &if_state), + Some(ref report) => { + ProbePlan::with_last_report(&self.relay_map, &if_state, report, &self.protocols) + } + None => ProbePlan::initial(&self.relay_map, &if_state, &self.protocols), }; trace!(%plan, "probe plan"); diff --git a/iroh-net-report/src/reportgen/probes.rs b/iroh-net-report/src/reportgen/probes.rs index ac27bef23b..5c971d6578 100644 --- a/iroh-net-report/src/reportgen/probes.rs +++ b/iroh-net-report/src/reportgen/probes.rs @@ -45,7 +45,7 @@ const NUM_INCREMENTAL_RELAYS: usize = 3; /// The protocol used to time a node's latency. #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, derive_more::Display)] #[repr(u8)] -pub(super) enum ProbeProto { +pub enum ProbeProto { /// STUN IPv4 StunIpv4, /// STUN IPv6 @@ -218,7 +218,11 @@ pub(super) struct ProbePlan(BTreeSet); impl ProbePlan { /// Creates an initial probe plan. - pub(super) fn initial(relay_map: &RelayMap, if_state: &interfaces::State) -> Self { + pub(super) fn initial( + relay_map: &RelayMap, + if_state: &interfaces::State, + protocols: &BTreeSet, + ) -> Self { let mut plan = Self(BTreeSet::new()); // The first time we need add probes after the STUN we record this delay, so that @@ -263,19 +267,30 @@ impl ProbePlan { }) .expect("adding QuicIpv6 probe to a QuicAddrIpv6 probe set"); } - plan.add(stun_ipv4_probes); - plan.add(stun_ipv6_probes); - plan.add(quic_ipv4_probes); - plan.add(quic_ipv6_probes); + plan.add_probes( + &protocols, + vec![ + stun_ipv4_probes, + stun_ipv6_probes, + quic_ipv4_probes, + quic_ipv6_probes, + ], + ); // The HTTP and ICMP probes only start after the STUN probes have had a chance. let mut https_probes = ProbeSet::new(ProbeProto::Https); let mut icmp_probes_ipv4 = ProbeSet::new(ProbeProto::IcmpV4); let mut icmp_probes_ipv6 = ProbeSet::new(ProbeProto::IcmpV6); + + let has_priority_probes = plan.has_priority_probes(); for attempt in 0..3 { - let start = *max_stun_delay.get_or_insert_with(|| plan.max_delay()) - + DEFAULT_INITIAL_RETRANSMIT; - let delay = start + DEFAULT_INITIAL_RETRANSMIT * attempt as u32; + let delay = if !has_priority_probes { + DEFAULT_INITIAL_RETRANSMIT * attempt as u32 + } else { + let start = *max_stun_delay.get_or_insert_with(|| plan.max_delay()) + + DEFAULT_INITIAL_RETRANSMIT; + start + DEFAULT_INITIAL_RETRANSMIT * attempt as u32 + }; https_probes .push(Probe::Https { @@ -300,9 +315,11 @@ impl ProbePlan { .expect("adding IcmpIpv6 probe to and IcmpIpv6 probe set"); } } - plan.add(https_probes); - plan.add(icmp_probes_ipv4); - plan.add(icmp_probes_ipv6); + + plan.add_probes( + &protocols, + vec![https_probes, icmp_probes_ipv4, icmp_probes_ipv6], + ); } plan } @@ -312,9 +329,10 @@ impl ProbePlan { relay_map: &RelayMap, if_state: &interfaces::State, last_report: &Report, + protocols: &BTreeSet, ) -> Self { if last_report.relay_latency.is_empty() { - return Self::initial(relay_map, if_state); + return Self::initial(relay_map, if_state, protocols); } let mut plan = Self(Default::default()); @@ -400,10 +418,15 @@ impl ProbePlan { .expect("adding QuicIpv6 probe to a QuicAddrIpv6 probe set"); } } - plan.add(stun_ipv4_probes); - plan.add(stun_ipv6_probes); - plan.add(quic_ipv4_probes); - plan.add(quic_ipv6_probes); + plan.add_probes( + protocols, + vec![ + stun_ipv4_probes, + stun_ipv6_probes, + quic_ipv4_probes, + quic_ipv6_probes, + ], + ); // The HTTP and ICMP probes only start after the STUN probes have had a chance. let mut https_probes = ProbeSet::new(ProbeProto::Https); @@ -437,9 +460,11 @@ impl ProbePlan { .expect("Pusying IcmpV6 Probe to an IcmpV6 ProbeSet"); } } - plan.add(https_probes); - plan.add(icmp_v4_probes); - plan.add(icmp_v6_probes); + + plan.add_probes( + protocols, + vec![https_probes, icmp_v4_probes, icmp_v6_probes], + ); } plan } @@ -465,6 +490,31 @@ impl ProbePlan { .max() .unwrap_or_default() } + + /// Adds [`ProbeSet`]s, if the probe set has been indicated in the set of protocols we want to attempt + fn add_probes(&mut self, protocols: &BTreeSet, probes: Vec) { + for probe in probes.into_iter() { + if protocols.contains(&probe.proto) { + self.add(probe); + } + } + } + + /// Stun & Quic probes are "priority" probes + fn has_priority_probes(&self) -> bool { + for probe in &self.0 { + if matches!( + probe.proto, + ProbeProto::StunIpv4 + | ProbeProto::StunIpv6 + | ProbeProto::QuicIpv4 + | ProbeProto::QuicIpv6 + ) { + return true; + } + } + return false; + } } impl fmt::Display for ProbePlan { @@ -527,7 +577,7 @@ mod tests { use pretty_assertions::assert_eq; use super::*; - use crate::{test_utils, RelayLatencies}; + use crate::{test_utils, Options, RelayLatencies}; /// Shorthand which declares a new ProbeSet. /// @@ -555,7 +605,7 @@ mod tests { let relay_node_1 = relay_map.nodes().next().unwrap(); let relay_node_2 = relay_map.nodes().nth(1).unwrap(); let if_state = interfaces::State::fake(); - let plan = ProbePlan::initial(&relay_map, &if_state); + let plan = ProbePlan::initial(&relay_map, &if_state, &Options::default_protocols()); let expected_plan: ProbePlan = [ probeset! { @@ -670,6 +720,75 @@ mod tests { assert_eq!(plan, expected_plan); } + #[tokio::test] + async fn test_initial_probeplan_some_protocols() { + let (_servers, relay_map) = test_utils::relay_map(2).await; + let relay_node_1 = relay_map.nodes().next().unwrap(); + let relay_node_2 = relay_map.nodes().nth(1).unwrap(); + let if_state = interfaces::State::fake(); + let plan = ProbePlan::initial( + &relay_map, + &if_state, + &BTreeSet::from([ProbeProto::Https, ProbeProto::IcmpV4, ProbeProto::IcmpV6]), + ); + + let expected_plan: ProbePlan = [ + probeset! { + proto: ProbeProto::Https, + relay: relay_node_1.clone(), + delays: [Duration::ZERO, + Duration::from_millis(100), + Duration::from_millis(200)], + }, + probeset! { + proto: ProbeProto::IcmpV4, + relay: relay_node_1.clone(), + delays: [Duration::ZERO, + Duration::from_millis(100), + Duration::from_millis(200)], + }, + probeset! { + proto: ProbeProto::IcmpV6, + relay: relay_node_1.clone(), + delays: [Duration::ZERO, + Duration::from_millis(100), + Duration::from_millis(200)], + }, + probeset! { + proto: ProbeProto::Https, + relay: relay_node_2.clone(), + delays: [Duration::ZERO, + Duration::from_millis(100), + Duration::from_millis(200)], + }, + probeset! { + proto: ProbeProto::IcmpV4, + relay: relay_node_2.clone(), + delays: [Duration::ZERO, + Duration::from_millis(100), + Duration::from_millis(200)], + }, + probeset! { + proto: ProbeProto::IcmpV6, + relay: relay_node_2.clone(), + delays: [Duration::ZERO, + Duration::from_millis(100), + Duration::from_millis(200)], + }, + ] + .into_iter() + .collect(); + + println!("expected:"); + println!("{expected_plan}"); + println!("actual:"); + println!("{plan}"); + // The readable error: + assert_eq!(plan.to_string(), expected_plan.to_string()); + // Just in case there's a bug in the Display impl: + assert_eq!(plan, expected_plan); + } + #[tokio::test] async fn test_plan_with_report() { let _logging = iroh_test::logging::setup(); @@ -704,7 +823,12 @@ mod tests { global_v6: None, captive_portal: None, }; - let plan = ProbePlan::with_last_report(&relay_map, &if_state, &last_report); + let plan = ProbePlan::with_last_report( + &relay_map, + &if_state, + &last_report, + &Options::default_protocols(), + ); let expected_plan: ProbePlan = [ probeset! { proto: ProbeProto::StunIpv4, diff --git a/iroh/src/magicsock.rs b/iroh/src/magicsock.rs index 23d24461d0..8c086107fe 100644 --- a/iroh/src/magicsock.rs +++ b/iroh/src/magicsock.rs @@ -37,6 +37,7 @@ use futures_util::{stream::BoxStream, task::AtomicWaker}; use iroh_base::{NodeAddr, NodeId, PublicKey, RelayUrl, SecretKey, SharedSecret}; use iroh_metrics::{inc, inc_by}; use iroh_relay::{protos::stun, RelayMap}; +use net_report::Options as ReportOptions; use netwatch::{interfaces, ip::LocalAddresses, netmon, UdpSocket}; use quinn::AsyncUdpSocket; use rand::{seq::SliceRandom, Rng, SeedableRng}; @@ -2347,7 +2348,13 @@ impl Actor { debug!("requesting net_report report"); match self .net_reporter - .get_report_channel(relay_map, pconn4, pconn6, quic_config) + .get_report_channel(ReportOptions { + relay_map, + stun_sock_v4: pconn4, + stun_sock_v6: pconn6, + quic_config, + protocols: ReportOptions::default_protocols(), + }) .await { Ok(rx) => { From c9760ff614ed8e7a0d4a4618b000f6c439cb1775 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9Cramfox=E2=80=9D?= <“kasey@n0.computer”> Date: Thu, 12 Dec 2024 20:15:31 -0500 Subject: [PATCH 2/7] fix: add quic probes results --- iroh-net-report/src/reportgen.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/iroh-net-report/src/reportgen.rs b/iroh-net-report/src/reportgen.rs index 0fe7505d04..0b63e7c317 100644 --- a/iroh-net-report/src/reportgen.rs +++ b/iroh-net-report/src/reportgen.rs @@ -799,7 +799,7 @@ async fn run_probe( let url = node.url.clone(); match quic_config { Some(quic_config) => { - run_quic_probe(quic_config, url, relay_addr, probe).await?; + result = run_quic_probe(quic_config, url, relay_addr, probe).await?; } None => { return Err(ProbeError::AbortSet( From 194d4a0e2402215e99c0e162190aca23a48b3097 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9Cramfox=E2=80=9D?= <“kasey@n0.computer”> Date: Fri, 13 Dec 2024 20:19:08 -0500 Subject: [PATCH 3/7] fix: always canonicalize the SocketAddr --- iroh-relay/src/quic.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/iroh-relay/src/quic.rs b/iroh-relay/src/quic.rs index 760bbda5ca..81c22a1572 100644 --- a/iroh-relay/src/quic.rs +++ b/iroh-relay/src/quic.rs @@ -260,10 +260,7 @@ impl QuicClient { let mut observed_addr = res.expect("checked"); // if we've sent to an ipv4 address, but received an observed address // that is ivp6 then the address is an [IPv4-Mapped IPv6 Addresses](https://doc.rust-lang.org/beta/std/net/struct.Ipv6Addr.html#ipv4-mapped-ipv6-addresses) - if server_addr.is_ipv4() && observed_addr.is_ipv6() { - observed_addr = - SocketAddr::new(observed_addr.ip().to_canonical(), observed_addr.port()); - } + observed_addr = SocketAddr::new(observed_addr.ip().to_canonical(), observed_addr.port()); let latency = conn.rtt() / 2; // gracefully close the connections conn.close(QUIC_ADDR_DISC_CLOSE_CODE, QUIC_ADDR_DISC_CLOSE_REASON); From f04c62bc97e72a3e356a884a6b97f9360a83d354 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9Cramfox=E2=80=9D?= <“kasey@n0.computer”> Date: Fri, 13 Dec 2024 22:05:04 -0500 Subject: [PATCH 4/7] docs, clippy, and fmt --- iroh-dns-server/src/store.rs | 2 +- iroh-net-report/src/lib.rs | 7 ++++--- iroh-net-report/src/reportgen/probes.rs | 6 +++--- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/iroh-dns-server/src/store.rs b/iroh-dns-server/src/store.rs index f5e156ff40..8ee036f7f5 100644 --- a/iroh-dns-server/src/store.rs +++ b/iroh-dns-server/src/store.rs @@ -34,7 +34,7 @@ pub enum PacketSource { /// A store for pkarr signed packets. /// -/// Packets are stored in the persistent [`SignedPacketStore`], and cached on-demand in an in-memory LRU +/// Packets are stored in the persistent `SignedPacketStore`, and cached on-demand in an in-memory LRU /// cache used for resolving DNS queries. #[derive(Debug, Clone)] pub struct ZoneStore { diff --git a/iroh-net-report/src/lib.rs b/iroh-net-report/src/lib.rs index aa584d5a6e..0c377371ee 100644 --- a/iroh-net-report/src/lib.rs +++ b/iroh-net-report/src/lib.rs @@ -38,8 +38,7 @@ mod ping; mod reportgen; pub use metrics::Metrics; -pub use reportgen::ProbeProto as ProbeProtocol; -pub use reportgen::QuicConfig; +pub use reportgen::{ProbeProto as ProbeProtocol, QuicConfig}; const FULL_REPORT_INTERVAL: Duration = Duration::from_secs(5 * 60); @@ -220,7 +219,8 @@ pub struct Options { /// /// If not provided and: /// - no probes are indicated in the *probes* field, - /// - or [`ProbeProto::StunIpv4`] probes are explicitly added to the *probes* list, + /// - or [`ProbeProtocol::StunIpv4`] probes are explicitly added to the *probes* list, + /// /// then the client will attempt to bind a suitable socket itself. pub stun_sock_v4: Option>, /// Socket to send IPv6 STUN probes from. @@ -232,6 +232,7 @@ pub struct Options { /// If not provided and: /// - no probes are indicated in the *probes* field, /// - or [`ProbeProtocol::StunIpv6`] probes are explicitly added to the *probes* list, + /// /// then the client will attempt to bind a suitable socket itself. pub stun_sock_v6: Option>, /// Endpoint and client configuration to create a QUIC diff --git a/iroh-net-report/src/reportgen/probes.rs b/iroh-net-report/src/reportgen/probes.rs index 5c971d6578..e398ab16e2 100644 --- a/iroh-net-report/src/reportgen/probes.rs +++ b/iroh-net-report/src/reportgen/probes.rs @@ -268,7 +268,7 @@ impl ProbePlan { .expect("adding QuicIpv6 probe to a QuicAddrIpv6 probe set"); } plan.add_probes( - &protocols, + protocols, vec![ stun_ipv4_probes, stun_ipv6_probes, @@ -317,7 +317,7 @@ impl ProbePlan { } plan.add_probes( - &protocols, + protocols, vec![https_probes, icmp_probes_ipv4, icmp_probes_ipv6], ); } @@ -513,7 +513,7 @@ impl ProbePlan { return true; } } - return false; + false } } From 6a7928a56d7f4ef0491e7d4864be153b26e64b75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9Cramfox=E2=80=9D?= <“kasey@n0.computer”> Date: Fri, 13 Dec 2024 22:36:46 -0500 Subject: [PATCH 5/7] clean up from PR review --- iroh-net-report/src/reportgen/probes.rs | 113 +++++++++++------------- 1 file changed, 52 insertions(+), 61 deletions(-) diff --git a/iroh-net-report/src/reportgen/probes.rs b/iroh-net-report/src/reportgen/probes.rs index e398ab16e2..190ff55e06 100644 --- a/iroh-net-report/src/reportgen/probes.rs +++ b/iroh-net-report/src/reportgen/probes.rs @@ -214,7 +214,10 @@ impl fmt::Display for ProbeSet { /// /// [`reportgen`]: crate::reportgen #[derive(Debug, PartialEq, Eq)] -pub(super) struct ProbePlan(BTreeSet); +pub(super) struct ProbePlan { + set: BTreeSet, + protocols: BTreeSet, +} impl ProbePlan { /// Creates an initial probe plan. @@ -223,11 +226,14 @@ impl ProbePlan { if_state: &interfaces::State, protocols: &BTreeSet, ) -> Self { - let mut plan = Self(BTreeSet::new()); + let mut plan = Self { + set: BTreeSet::new(), + protocols: protocols.clone(), + }; // The first time we need add probes after the STUN we record this delay, so that // further relay server can reuse this delay. - let mut max_stun_delay: Option = None; + let mut max_high_prio_delay: Option = None; for relay_node in relay_map.nodes() { let mut stun_ipv4_probes = ProbeSet::new(ProbeProto::StunIpv4); @@ -267,31 +273,25 @@ impl ProbePlan { }) .expect("adding QuicIpv6 probe to a QuicAddrIpv6 probe set"); } - plan.add_probes( - protocols, - vec![ - stun_ipv4_probes, - stun_ipv6_probes, - quic_ipv4_probes, - quic_ipv6_probes, - ], - ); + plan.add_if_enabled(stun_ipv4_probes); + plan.add_if_enabled(stun_ipv6_probes); + plan.add_if_enabled(quic_ipv4_probes); + plan.add_if_enabled(quic_ipv6_probes); // The HTTP and ICMP probes only start after the STUN probes have had a chance. let mut https_probes = ProbeSet::new(ProbeProto::Https); - let mut icmp_probes_ipv4 = ProbeSet::new(ProbeProto::IcmpV4); - let mut icmp_probes_ipv6 = ProbeSet::new(ProbeProto::IcmpV6); + let mut icmp_v4_probes = ProbeSet::new(ProbeProto::IcmpV4); + let mut icmp_v6_probes = ProbeSet::new(ProbeProto::IcmpV6); - let has_priority_probes = plan.has_priority_probes(); for attempt in 0..3 { - let delay = if !has_priority_probes { - DEFAULT_INITIAL_RETRANSMIT * attempt as u32 - } else { - let start = *max_stun_delay.get_or_insert_with(|| plan.max_delay()) - + DEFAULT_INITIAL_RETRANSMIT; - start + DEFAULT_INITIAL_RETRANSMIT * attempt as u32 - }; - + let mut start = *max_high_prio_delay.get_or_insert_with(|| plan.max_delay()); + // if there are high priority probes, ensure there is a buffer between + // the highest probe delay and the next probes we create + // if there are no high priority probes, we don't need a buffer + if plan.has_priority_probes() { + start = start + DEFAULT_INITIAL_RETRANSMIT; + } + let delay = start + DEFAULT_INITIAL_RETRANSMIT * attempt as u32; https_probes .push(Probe::Https { delay, @@ -299,7 +299,7 @@ impl ProbePlan { }) .expect("adding Https probe to a Https probe set"); if if_state.have_v4 { - icmp_probes_ipv4 + icmp_v4_probes .push(Probe::IcmpV4 { delay, node: relay_node.clone(), @@ -307,7 +307,7 @@ impl ProbePlan { .expect("adding Icmp probe to an Icmp probe set"); } if if_state.have_v6 { - icmp_probes_ipv6 + icmp_v6_probes .push(Probe::IcmpV6 { delay, node: relay_node.clone(), @@ -316,10 +316,9 @@ impl ProbePlan { } } - plan.add_probes( - protocols, - vec![https_probes, icmp_probes_ipv4, icmp_probes_ipv6], - ); + plan.add_if_enabled(https_probes); + plan.add_if_enabled(icmp_v4_probes); + plan.add_if_enabled(icmp_v6_probes); } plan } @@ -334,7 +333,10 @@ impl ProbePlan { if last_report.relay_latency.is_empty() { return Self::initial(relay_map, if_state, protocols); } - let mut plan = Self(Default::default()); + let mut plan = Self { + set: Default::default(), + protocols: protocols.clone(), + }; // The first time we need add probes after the STUN we record this delay, so that // further relay servers can reuse this delay. @@ -418,15 +420,10 @@ impl ProbePlan { .expect("adding QuicIpv6 probe to a QuicAddrIpv6 probe set"); } } - plan.add_probes( - protocols, - vec![ - stun_ipv4_probes, - stun_ipv6_probes, - quic_ipv4_probes, - quic_ipv6_probes, - ], - ); + plan.add_if_enabled(stun_ipv4_probes); + plan.add_if_enabled(stun_ipv6_probes); + plan.add_if_enabled(quic_ipv4_probes); + plan.add_if_enabled(quic_ipv6_probes); // The HTTP and ICMP probes only start after the STUN probes have had a chance. let mut https_probes = ProbeSet::new(ProbeProto::Https); @@ -461,29 +458,29 @@ impl ProbePlan { } } - plan.add_probes( - protocols, - vec![https_probes, icmp_v4_probes, icmp_v6_probes], - ); + plan.add_if_enabled(https_probes); + plan.add_if_enabled(icmp_v4_probes); + plan.add_if_enabled(icmp_v6_probes); } plan } /// Returns an iterator over the [`ProbeSet`]s in this plan. pub(super) fn iter(&self) -> impl Iterator { - self.0.iter() + self.set.iter() } - /// Adds a [`ProbeSet`] if it contains probes. - fn add(&mut self, set: ProbeSet) { - if !set.is_empty() { - self.0.insert(set); + /// Adds a [`ProbeSet`] if it contains probes and the protocol indicated in + /// the [`ProbeSet] matches a protocol in our set of [`ProbeProto`]s. + fn add_if_enabled(&mut self, set: ProbeSet) { + if !set.is_empty() && self.protocols.contains(&set.proto) { + self.set.insert(set); } } /// Returns the delay of the last probe in the probe plan. fn max_delay(&self) -> Duration { - self.0 + self.set .iter() .flatten() .map(|probe| probe.delay()) @@ -491,18 +488,9 @@ impl ProbePlan { .unwrap_or_default() } - /// Adds [`ProbeSet`]s, if the probe set has been indicated in the set of protocols we want to attempt - fn add_probes(&mut self, protocols: &BTreeSet, probes: Vec) { - for probe in probes.into_iter() { - if protocols.contains(&probe.proto) { - self.add(probe); - } - } - } - /// Stun & Quic probes are "priority" probes fn has_priority_probes(&self) -> bool { - for probe in &self.0 { + for probe in &self.set { if matches!( probe.proto, ProbeProto::StunIpv4 @@ -520,7 +508,7 @@ impl ProbePlan { impl fmt::Display for ProbePlan { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { writeln!(f, "ProbePlan {{")?; - for probe_set in self.0.iter() { + for probe_set in self.set.iter() { writeln!(f, r#" ProbeSet("{}") {{"#, probe_set.proto)?; for probe in probe_set.probes.iter() { writeln!(f, " {probe},")?; @@ -533,7 +521,10 @@ impl fmt::Display for ProbePlan { impl FromIterator for ProbePlan { fn from_iter>(iter: T) -> Self { - Self(iter.into_iter().collect()) + Self { + set: iter.into_iter().collect(), + protocols: BTreeSet::new(), + } } } From 9fa1b792be0bd2d58f2733d3390e9df4cc37ae21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9Cramfox=E2=80=9D?= <“kasey@n0.computer”> Date: Sat, 14 Dec 2024 00:31:31 -0500 Subject: [PATCH 6/7] clippy --- iroh-net-report/src/reportgen/probes.rs | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/iroh-net-report/src/reportgen/probes.rs b/iroh-net-report/src/reportgen/probes.rs index 190ff55e06..2c968d9333 100644 --- a/iroh-net-report/src/reportgen/probes.rs +++ b/iroh-net-report/src/reportgen/probes.rs @@ -289,7 +289,7 @@ impl ProbePlan { // the highest probe delay and the next probes we create // if there are no high priority probes, we don't need a buffer if plan.has_priority_probes() { - start = start + DEFAULT_INITIAL_RETRANSMIT; + start += DEFAULT_INITIAL_RETRANSMIT; } let delay = start + DEFAULT_INITIAL_RETRANSMIT * attempt as u32; https_probes @@ -568,7 +568,7 @@ mod tests { use pretty_assertions::assert_eq; use super::*; - use crate::{test_utils, Options, RelayLatencies}; + use crate::{test_utils, RelayLatencies}; /// Shorthand which declares a new ProbeSet. /// @@ -590,13 +590,25 @@ mod tests { }; } + fn default_protocols() -> BTreeSet { + BTreeSet::from([ + ProbeProto::StunIpv4, + ProbeProto::StunIpv6, + ProbeProto::QuicIpv4, + ProbeProto::QuicIpv6, + ProbeProto::IcmpV4, + ProbeProto::IcmpV6, + ProbeProto::Https, + ]) + } + #[tokio::test] async fn test_initial_probeplan() { let (_servers, relay_map) = test_utils::relay_map(2).await; let relay_node_1 = relay_map.nodes().next().unwrap(); let relay_node_2 = relay_map.nodes().nth(1).unwrap(); let if_state = interfaces::State::fake(); - let plan = ProbePlan::initial(&relay_map, &if_state, &Options::default_protocols()); + let plan = ProbePlan::initial(&relay_map, &if_state, &default_protocols()); let expected_plan: ProbePlan = [ probeset! { @@ -818,7 +830,7 @@ mod tests { &relay_map, &if_state, &last_report, - &Options::default_protocols(), + &default_protocols(), ); let expected_plan: ProbePlan = [ probeset! { From 5d991ff2ee8e25008f03cbc92c87154770732345 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9Cramfox=E2=80=9D?= <“kasey@n0.computer”> Date: Sat, 14 Dec 2024 00:31:50 -0500 Subject: [PATCH 7/7] maybe this is better for options? --- iroh-net-report/src/lib.rs | 224 ++++++++++++++++-------- iroh-net-report/src/reportgen.rs | 6 + iroh-net-report/src/reportgen/probes.rs | 10 +- iroh/src/magicsock.rs | 20 +-- 4 files changed, 165 insertions(+), 95 deletions(-) diff --git a/iroh-net-report/src/lib.rs b/iroh-net-report/src/lib.rs index 0c377371ee..1f622247c7 100644 --- a/iroh-net-report/src/lib.rs +++ b/iroh-net-report/src/lib.rs @@ -38,7 +38,8 @@ mod ping; mod reportgen; pub use metrics::Metrics; -pub use reportgen::{ProbeProto as ProbeProtocol, QuicConfig}; +use reportgen::ProbeProto; +pub use reportgen::QuicConfig; const FULL_REPORT_INTERVAL: Duration = Duration::from_secs(5 * 60); @@ -206,58 +207,137 @@ impl Default for Reports { } } +/// Options for running probes +/// +/// By default, will run icmp over IPv4, icmp over IPv6, and Https probes. +/// +/// Use [`Options::stun_v4`], [`Options::stun_v6`], and [`Options::quic_config`] +/// to enable STUN over IPv4, STUN over IPv6, and QUIC address discovery. #[derive(Debug)] -/// Options for running a report pub struct Options { - /// The relay configuration. - pub relay_map: RelayMap, /// Socket to send IPv4 STUN probes from. /// /// Responses are never read from this socket, they must be passed in via internal - /// messaging since, when used internally in iroh, the socket is also used to receive + /// messaging since, when used internally in iroh, the socket is also used to receive /// other packets from in the magicsocket (`MagicSock`). /// - /// If not provided and: - /// - no probes are indicated in the *probes* field, - /// - or [`ProbeProtocol::StunIpv4`] probes are explicitly added to the *probes* list, - /// - /// then the client will attempt to bind a suitable socket itself. - pub stun_sock_v4: Option>, + /// If not provided, STUN probes will not be sent over IPv4. + stun_sock_v4: Option>, /// Socket to send IPv6 STUN probes from. /// /// Responses are never read from this socket, they must be passed in via internal - /// messaging since, when used internally in iroh, the socket is also used to receive + /// messaging since, when used internally in iroh, the socket is also used to receive /// other packets from in the magicsocket (`MagicSock`). /// - /// If not provided and: - /// - no probes are indicated in the *probes* field, - /// - or [`ProbeProtocol::StunIpv6`] probes are explicitly added to the *probes* list, + /// If not provided, STUN probes will not be sent over IPv6. + stun_sock_v6: Option>, + /// The configuration needed to launch QUIC address discovery probes. + /// + /// If not provided, will not run QUIC address discovery. + quic_config: Option, + /// Enable icmp_v4 probes /// - /// then the client will attempt to bind a suitable socket itself. - pub stun_sock_v6: Option>, - /// Endpoint and client configuration to create a QUIC - /// connection to do QUIC address discovery. + /// On by default + icmp_v4: bool, + /// Enable icmp_v6 probes /// - /// If not provided, will not do QUIC address discovery, even if indicated in the *probes* set. - pub quic_config: Option, - /// An empty protocols list indicates running all possible probe protocols. + /// On by default + icmp_v6: bool, + /// Enable https probes /// - /// Otherwise, *protocols* is a set of [`ProbeProtocol`]s that should be run in the report. - pub protocols: BTreeSet, + /// On by default + https: bool, +} + +impl Default for Options { + fn default() -> Self { + Self { + stun_sock_v4: None, + stun_sock_v6: None, + quic_config: None, + icmp_v4: true, + icmp_v6: true, + https: true, + } + } } impl Options { - /// The default probe protocols used in a report - pub fn default_protocols() -> BTreeSet { - BTreeSet::from([ - ProbeProtocol::StunIpv4, - ProbeProtocol::StunIpv6, - ProbeProtocol::Https, - ProbeProtocol::IcmpV4, - ProbeProtocol::IcmpV6, - ProbeProtocol::QuicIpv4, - ProbeProtocol::QuicIpv6, - ]) + /// Create an empty Options that enables no probes + pub fn empty() -> Self { + Self { + stun_sock_v4: None, + stun_sock_v6: None, + quic_config: None, + icmp_v4: false, + icmp_v6: false, + https: false, + } + } + + /// Set the ipv4 stun socket and enable ipv4 stun probes + pub fn stun_v4(mut self, sock: Option>) -> Self { + self.stun_sock_v4 = sock; + self + } + + /// Set the ipv6 stun socket and enable ipv6 stun probes + pub fn stun_v6(mut self, sock: Option>) -> Self { + self.stun_sock_v6 = sock; + self + } + + /// Enable quic probes + pub fn quic_config(mut self, quic_config: Option) -> Self { + self.quic_config = quic_config; + self + } + + /// Enable icmp_v4 probe + pub fn enable_icmp_v4(mut self) -> Self { + self.icmp_v4 = true; + self + } + + /// Enable icmp_v6 probe + pub fn enable_icmp_v6(mut self) -> Self { + self.icmp_v6 = true; + self + } + + /// Enable https probe + pub fn enable_https(mut self) -> Self { + self.https = true; + self + } + + /// Turn the options into set of valid protocols + fn to_protocols(&self) -> BTreeSet { + let mut protocols = BTreeSet::new(); + if self.stun_sock_v4.is_some() { + protocols.insert(ProbeProto::StunIpv4); + } + if self.stun_sock_v6.is_some() { + protocols.insert(ProbeProto::StunIpv6); + } + if let Some(ref quic) = self.quic_config { + if quic.ipv4 { + protocols.insert(ProbeProto::QuicIpv4); + } + if quic.ipv6 { + protocols.insert(ProbeProto::QuicIpv6); + } + } + if self.icmp_v4 { + protocols.insert(ProbeProto::IcmpV4); + } + if self.icmp_v6 { + protocols.insert(ProbeProto::IcmpV6); + } + if self.https { + protocols.insert(ProbeProto::Https); + } + protocols } } @@ -316,15 +396,11 @@ impl Client { stun_sock_v6: Option>, quic_config: Option, ) -> Result> { - let rx = self - .get_report_channel(Options { - relay_map, - stun_sock_v4, - stun_sock_v6, - quic_config, - protocols: Options::default_protocols(), - }) - .await?; + let opts = Options::default() + .stun_v4(stun_sock_v4) + .stun_v6(stun_sock_v6) + .quic_config(quic_config); + let rx = self.get_report_channel(relay_map.clone(), opts).await?; match rx.await { Ok(res) => res, Err(_) => Err(anyhow!("channel closed, actor awol")), @@ -335,10 +411,13 @@ impl Client { /// /// It may not be called concurrently with itself, `&mut self` takes care of that. /// - /// Look at [`Options`] for the different configuration options. Use the - /// `Options::protocols` field to specify which protocols to attempt to run. - pub async fn get_report_with_opts(&mut self, opts: Options) -> Result> { - let rx = self.get_report_channel(opts).await?; + /// Look at [`Options`] for the different configuration options. + pub async fn get_report_with_opts( + &mut self, + relay_map: RelayMap, + opts: Options, + ) -> Result> { + let rx = self.get_report_channel(relay_map, opts).await?; match rx.await { Ok(res) => res, Err(_) => Err(anyhow!("channel closed, actor awol")), @@ -346,15 +425,17 @@ impl Client { } /// Get report with channel + /// + /// Look at [`Options`] for the different configuration options. pub async fn get_report_channel( &mut self, + relay_map: RelayMap, opts: Options, ) -> Result>>> { - // TODO: consider if RelayMap should be made to easily clone? It seems expensive - // right now. let (tx, rx) = oneshot::channel(); self.addr .send(Message::RunCheck { + relay_map, opts, response_tx: tx, }) @@ -381,6 +462,8 @@ pub(crate) enum Message { /// Only one net_report can be run at a time, trying to run multiple concurrently will /// fail. RunCheck { + /// The map of relays we want to probe + relay_map: RelayMap, /// Options for the report opts: Options, /// Channel to receive the response. @@ -518,8 +601,12 @@ impl Actor { while let Some(msg) = self.receiver.recv().await { trace!(?msg, "handling message"); match msg { - Message::RunCheck { opts, response_tx } => { - self.handle_run_check(opts, response_tx); + Message::RunCheck { + relay_map, + opts, + response_tx, + } => { + self.handle_run_check(relay_map, opts, response_tx); } Message::ReportReady { report } => { self.handle_report_ready(report); @@ -544,21 +631,17 @@ impl Actor { /// sockets you will be using. fn handle_run_check( &mut self, + relay_map: RelayMap, opts: Options, response_tx: oneshot::Sender>>, ) { + let protocols = opts.to_protocols(); let Options { - relay_map, stun_sock_v4, stun_sock_v6, quic_config, - protocols, + .. } = opts; - let protos = if protocols.is_empty() { - Options::default_protocols() - } else { - protocols - }; if self.current_report_run.is_some() { response_tx .send(Err(anyhow!( @@ -570,17 +653,6 @@ impl Actor { let now = Instant::now(); - let cancel_token = CancellationToken::new(); - let stun_sock_v4 = match (stun_sock_v4, protos.contains(&ProbeProtocol::StunIpv4)) { - (Some(sock), true) => Some(sock), - (None, true) => bind_local_stun_socket(IpFamily::V4, self.addr(), cancel_token.clone()), - _ => None, - }; - let stun_sock_v6 = match (stun_sock_v6, protos.contains(&ProbeProtocol::StunIpv6)) { - (Some(sock), true) => Some(sock), - (None, true) => bind_local_stun_socket(IpFamily::V6, self.addr(), cancel_token.clone()), - _ => None, - }; let mut do_full = self.reports.next_full || now.duration_since(self.reports.last_full) > FULL_REPORT_INTERVAL; @@ -611,12 +683,11 @@ impl Actor { stun_sock_v6, quic_config, self.dns_resolver.clone(), - protos, + protocols, ); self.current_report_run = Some(ReportRun { _reportgen: actor, - _drop_guard: cancel_token.drop_guard(), report_tx: response_tx, }); } @@ -783,8 +854,6 @@ impl Actor { struct ReportRun { /// The handle of the [`reportgen`] actor, cancels the actor on drop. _reportgen: reportgen::Client, - /// Drop guard to optionally kill workers started by net_report to support reportgen. - _drop_guard: tokio_util::sync::DropGuard, /// Where to send the completed report. report_tx: oneshot::Sender>>, } @@ -794,7 +863,7 @@ struct ReportRun { /// If successful this returns the bound socket and will forward STUN responses to the /// provided *actor_addr*. The *cancel_token* serves to stop the packet forwarding when the /// socket is no longer needed. -fn bind_local_stun_socket( +pub fn bind_local_stun_socket( network: IpFamily, actor_addr: Addr, cancel_token: CancellationToken, @@ -1057,8 +1126,10 @@ mod tests { // Note that the ProbePlan will change with each iteration. for i in 0..5 { + let cancel = CancellationToken::new(); + let sock = bind_local_stun_socket(IpFamily::V4, client.addr(), cancel.clone()); println!("--round {}", i); - let r = client.get_report(dm.clone(), None, None, None).await?; + let r = client.get_report(dm.clone(), sock, None, None).await?; assert!(r.udp, "want UDP"); assert_eq!( @@ -1074,6 +1145,7 @@ mod tests { ); assert!(r.global_v4.is_some(), "expected globalV4 set"); assert!(r.preferred_relay.is_some(),); + cancel.cancel(); } assert!( diff --git a/iroh-net-report/src/reportgen.rs b/iroh-net-report/src/reportgen.rs index 0b63e7c317..fb144cdc25 100644 --- a/iroh-net-report/src/reportgen.rs +++ b/iroh-net-report/src/reportgen.rs @@ -696,6 +696,10 @@ pub struct QuicConfig { pub ep: quinn::Endpoint, /// A client config. pub client_config: rustls::ClientConfig, + /// Enable ipv4 QUIC address discovery probes + pub ipv4: bool, + /// Enable ipv6 QUIC address discovery probes + pub ipv6: bool, } /// Executes a particular [`Probe`], including using a delayed start if needed. @@ -1586,6 +1590,8 @@ mod tests { let quic_addr_disc = QuicConfig { ep: ep.clone(), client_config, + ipv4: true, + ipv6: true, }; let url = relay.url.clone(); let port = server.quic_addr().unwrap().port(); diff --git a/iroh-net-report/src/reportgen/probes.rs b/iroh-net-report/src/reportgen/probes.rs index 2c968d9333..ba5f9ce22e 100644 --- a/iroh-net-report/src/reportgen/probes.rs +++ b/iroh-net-report/src/reportgen/probes.rs @@ -610,7 +610,7 @@ mod tests { let if_state = interfaces::State::fake(); let plan = ProbePlan::initial(&relay_map, &if_state, &default_protocols()); - let expected_plan: ProbePlan = [ + let mut expected_plan: ProbePlan = [ probeset! { proto: ProbeProto::StunIpv4, relay: relay_node_1.clone(), @@ -712,6 +712,7 @@ mod tests { ] .into_iter() .collect(); + expected_plan.protocols = default_protocols(); println!("expected:"); println!("{expected_plan}"); @@ -735,7 +736,7 @@ mod tests { &BTreeSet::from([ProbeProto::Https, ProbeProto::IcmpV4, ProbeProto::IcmpV6]), ); - let expected_plan: ProbePlan = [ + let mut expected_plan: ProbePlan = [ probeset! { proto: ProbeProto::Https, relay: relay_node_1.clone(), @@ -781,6 +782,8 @@ mod tests { ] .into_iter() .collect(); + expected_plan.protocols = + BTreeSet::from([ProbeProto::Https, ProbeProto::IcmpV4, ProbeProto::IcmpV6]); println!("expected:"); println!("{expected_plan}"); @@ -832,7 +835,7 @@ mod tests { &last_report, &default_protocols(), ); - let expected_plan: ProbePlan = [ + let mut expected_plan: ProbePlan = [ probeset! { proto: ProbeProto::StunIpv4, relay: relay_node_1.clone(), @@ -934,6 +937,7 @@ mod tests { ] .into_iter() .collect(); + expected_plan.protocols = default_protocols(); println!("{} round", i); println!("expected:"); diff --git a/iroh/src/magicsock.rs b/iroh/src/magicsock.rs index 8c086107fe..31ac05c8b6 100644 --- a/iroh/src/magicsock.rs +++ b/iroh/src/magicsock.rs @@ -37,7 +37,6 @@ use futures_util::{stream::BoxStream, task::AtomicWaker}; use iroh_base::{NodeAddr, NodeId, PublicKey, RelayUrl, SecretKey, SharedSecret}; use iroh_metrics::{inc, inc_by}; use iroh_relay::{protos::stun, RelayMap}; -use net_report::Options as ReportOptions; use netwatch::{interfaces, ip::LocalAddresses, netmon, UdpSocket}; use quinn::AsyncUdpSocket; use rand::{seq::SliceRandom, Rng, SeedableRng}; @@ -2340,23 +2339,12 @@ impl Actor { } let relay_map = self.msock.relay_map.clone(); - let pconn4 = Some(self.pconn4.clone()); - let pconn6 = self.pconn6.clone(); - - let quic_config = None; + let opts = net_report::Options::default() + .stun_v4(Some(self.pconn4.clone())) + .stun_v6(self.pconn6.clone()); debug!("requesting net_report report"); - match self - .net_reporter - .get_report_channel(ReportOptions { - relay_map, - stun_sock_v4: pconn4, - stun_sock_v6: pconn6, - quic_config, - protocols: ReportOptions::default_protocols(), - }) - .await - { + match self.net_reporter.get_report_channel(relay_map, opts).await { Ok(rx) => { let msg_sender = self.msg_sender.clone(); tokio::task::spawn(async move {