Skip to content

Commit

Permalink
feat(relay)!: relay only mode now configurable (#3056)
Browse files Browse the repository at this point in the history
## Description

This will allow us to configure relay only mode at runtime. ~~Useful for
easy testing without having to rebuild with the `DEV_RELAY_ONLY`
environment variable set.~~

The `DEV_RELAY_ONLY` compile time env var has been completely dropped
and a `relay_only` option has been threaded through the entire stack
when `test-utils` is enabled. An example of it can be followed with the
`iroh/examples/transfer.rs` where you can set up a provide and fetch
node with `--relay-only`.

## Breaking Changes

<!-- Optional, if there are any breaking changes document them,
including how to migrate older code. -->

## Notes & open questions

<!-- Any notes, remarks or open questions you have to make about the PR.
-->

## Change checklist

- [ ] Self-review.
- [ ] Documentation updates following the [style
guide](https://rust-lang.github.io/rfcs/1574-more-api-documentation-conventions.html#appendix-a-full-conventions-text),
if relevant.
- [ ] Tests if relevant.
- [ ] All breaking changes documented.
  • Loading branch information
Arqu authored Jan 3, 2025
1 parent f50db17 commit 5aba17e
Show file tree
Hide file tree
Showing 9 changed files with 158 additions and 49 deletions.
13 changes: 3 additions & 10 deletions iroh/bench/src/bin/bulk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,24 +39,17 @@ pub fn run_iroh(opt: Opt) -> Result<()> {
metrics.insert(::iroh::metrics::NetReportMetrics::new(reg));
metrics.insert(::iroh::metrics::PortmapMetrics::new(reg));
#[cfg(feature = "local-relay")]
if opt.with_relay {
if opt.only_relay {
metrics.insert(::iroh::metrics::RelayMetrics::new(reg));
}
})?;
}

#[cfg(not(feature = "local-relay"))]
if opt.with_relay {
anyhow::bail!(
"Must compile the benchmark with the `local-relay` feature flag to use this option"
);
}

let server_span = tracing::error_span!("server");
let runtime = rt();

#[cfg(feature = "local-relay")]
let (relay_url, _guard) = if opt.with_relay {
let (relay_url, _guard) = if opt.only_relay {
let (_, relay_url, _guard) = runtime.block_on(::iroh::test_utils::run_relay_server())?;

(Some(relay_url), Some(_guard))
Expand Down Expand Up @@ -120,7 +113,7 @@ pub fn run_iroh(opt: Opt) -> Result<()> {
"PortmapMetrics",
core.get_collector::<::iroh::metrics::PortmapMetrics>(),
);
// if None, (this is the case if opt.with_relay is false), then this is skipped internally:
// if None, (this is the case if opt.only_relay is false), then this is skipped internally:
#[cfg(feature = "local-relay")]
collect_and_print(
"RelayMetrics",
Expand Down
14 changes: 12 additions & 2 deletions iroh/bench/src/iroh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,12 @@ pub fn server_endpoint(
let mut builder = Endpoint::builder();
#[cfg(feature = "local-relay")]
{
builder = builder.insecure_skip_relay_cert_verify(relay_url.is_some())
builder = builder.insecure_skip_relay_cert_verify(relay_url.is_some());
let path_selection = match opt.only_relay {
true => iroh::endpoint::PathSelection::RelayOnly,
false => iroh::endpoint::PathSelection::default(),
};
builder = builder.path_selection(path_selection);
}
let ep = builder
.alpns(vec![ALPN.to_vec()])
Expand Down Expand Up @@ -89,7 +94,12 @@ pub async fn connect_client(
let mut builder = Endpoint::builder();
#[cfg(feature = "local-relay")]
{
builder = builder.insecure_skip_relay_cert_verify(relay_url.is_some())
builder = builder.insecure_skip_relay_cert_verify(relay_url.is_some());
let path_selection = match opt.only_relay {
true => iroh::endpoint::PathSelection::RelayOnly,
false => iroh::endpoint::PathSelection::default(),
};
builder = builder.path_selection(path_selection);
}
let endpoint = builder
.alpns(vec![ALPN.to_vec()])
Expand Down
9 changes: 4 additions & 5 deletions iroh/bench/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,12 +69,11 @@ pub struct Opt {
#[clap(long, default_value = "1200")]
pub initial_mtu: u16,
/// Whether to run a local relay and have the server and clients connect to that.
///
/// Can be combined with the `DEV_RELAY_ONLY` environment variable (at compile time)
/// to test throughput for relay-only traffic locally.
/// (e.g. `DEV_RELAY_ONLY=true cargo run --release -- iroh --with-relay`)
/// This will force all traffic over the relay and can be used to test
/// throughput for relay-only traffic.
#[cfg(feature = "local-relay")]
#[clap(long, default_value_t = false)]
pub with_relay: bool,
pub only_relay: bool,
}

pub enum EndpointSelector {
Expand Down
33 changes: 28 additions & 5 deletions iroh/examples/transfer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ use bytes::Bytes;
use clap::{Parser, Subcommand};
use indicatif::HumanBytes;
use iroh::{
endpoint::ConnectionError, Endpoint, NodeAddr, RelayMap, RelayMode, RelayUrl, SecretKey,
endpoint::{ConnectionError, PathSelection},
Endpoint, NodeAddr, RelayMap, RelayMode, RelayUrl, SecretKey,
};
use iroh_base::ticket::NodeTicket;
use tracing::info;
Expand All @@ -29,12 +30,16 @@ enum Commands {
size: u64,
#[clap(long)]
relay_url: Option<String>,
#[clap(long, default_value = "false")]
relay_only: bool,
},
Fetch {
#[arg(index = 1)]
ticket: String,
#[clap(long)]
relay_url: Option<String>,
#[clap(long, default_value = "false")]
relay_only: bool,
},
}

Expand All @@ -44,14 +49,22 @@ async fn main() -> anyhow::Result<()> {
let cli = Cli::parse();

match &cli.command {
Commands::Provide { size, relay_url } => provide(*size, relay_url.clone()).await?,
Commands::Fetch { ticket, relay_url } => fetch(ticket, relay_url.clone()).await?,
Commands::Provide {
size,
relay_url,
relay_only,
} => provide(*size, relay_url.clone(), *relay_only).await?,
Commands::Fetch {
ticket,
relay_url,
relay_only,
} => fetch(ticket, relay_url.clone(), *relay_only).await?,
}

Ok(())
}

async fn provide(size: u64, relay_url: Option<String>) -> anyhow::Result<()> {
async fn provide(size: u64, relay_url: Option<String>, relay_only: bool) -> anyhow::Result<()> {
let secret_key = SecretKey::generate(rand::rngs::OsRng);
let relay_mode = match relay_url {
Some(relay_url) => {
Expand All @@ -61,10 +74,15 @@ async fn provide(size: u64, relay_url: Option<String>) -> anyhow::Result<()> {
}
None => RelayMode::Default,
};
let path_selection = match relay_only {
true => PathSelection::RelayOnly,
false => PathSelection::default(),
};
let endpoint = Endpoint::builder()
.secret_key(secret_key)
.alpns(vec![TRANSFER_ALPN.to_vec()])
.relay_mode(relay_mode)
.path_selection(path_selection)
.bind()
.await?;

Expand Down Expand Up @@ -142,7 +160,7 @@ async fn provide(size: u64, relay_url: Option<String>) -> anyhow::Result<()> {
Ok(())
}

async fn fetch(ticket: &str, relay_url: Option<String>) -> anyhow::Result<()> {
async fn fetch(ticket: &str, relay_url: Option<String>, relay_only: bool) -> anyhow::Result<()> {
let ticket: NodeTicket = ticket.parse()?;
let secret_key = SecretKey::generate(rand::rngs::OsRng);
let relay_mode = match relay_url {
Expand All @@ -153,10 +171,15 @@ async fn fetch(ticket: &str, relay_url: Option<String>) -> anyhow::Result<()> {
}
None => RelayMode::Default,
};
let path_selection = match relay_only {
true => PathSelection::RelayOnly,
false => PathSelection::default(),
};
let endpoint = Endpoint::builder()
.secret_key(secret_key)
.alpns(vec![TRANSFER_ALPN.to_vec()])
.relay_mode(relay_mode)
.path_selection(path_selection)
.bind()
.await?;

Expand Down
26 changes: 26 additions & 0 deletions iroh/src/endpoint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,18 @@ const DISCOVERY_WAIT_PERIOD: Duration = Duration::from_millis(500);

type DiscoveryBuilder = Box<dyn FnOnce(&SecretKey) -> Option<Box<dyn Discovery>> + Send + Sync>;

/// Defines the mode of path selection for all traffic flowing through
/// the endpoint.
#[cfg(any(test, feature = "test-utils"))]
#[derive(Debug, Default, Copy, Clone, PartialEq, Eq)]
pub enum PathSelection {
/// Uses all available paths
#[default]
All,
/// Forces all traffic to go exclusively through relays
RelayOnly,
}

/// Builder for [`Endpoint`].
///
/// By default the endpoint will generate a new random [`SecretKey`], which will result in a
Expand All @@ -97,6 +109,8 @@ pub struct Builder {
insecure_skip_relay_cert_verify: bool,
addr_v4: Option<SocketAddrV4>,
addr_v6: Option<SocketAddrV6>,
#[cfg(any(test, feature = "test-utils"))]
path_selection: PathSelection,
}

impl Default for Builder {
Expand All @@ -115,6 +129,8 @@ impl Default for Builder {
insecure_skip_relay_cert_verify: false,
addr_v4: None,
addr_v6: None,
#[cfg(any(test, feature = "test-utils"))]
path_selection: PathSelection::default(),
}
}
}
Expand Down Expand Up @@ -160,6 +176,8 @@ impl Builder {
dns_resolver,
#[cfg(any(test, feature = "test-utils"))]
insecure_skip_relay_cert_verify: self.insecure_skip_relay_cert_verify,
#[cfg(any(test, feature = "test-utils"))]
path_selection: self.path_selection,
};
Endpoint::bind(static_config, msock_opts, self.alpn_protocols).await
}
Expand Down Expand Up @@ -417,6 +435,14 @@ impl Builder {
self.insecure_skip_relay_cert_verify = skip_verify;
self
}

/// This implies we only use the relay to communicate
/// and do not attempt to do any hole punching.
#[cfg(any(test, feature = "test-utils"))]
pub fn path_selection(mut self, path_selection: PathSelection) -> Self {
self.path_selection = path_selection;
self
}
}

/// Configuration for a [`quinn::Endpoint`] that cannot be changed at runtime.
Expand Down
25 changes: 17 additions & 8 deletions iroh/src/magicsock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@
//!
//! Based on tailscale/wgengine/magicsock
//!
//! ### `DEV_RELAY_ONLY` env var:
//! When present at *compile time*, this env var will force all packets
//! to be sent over the relay connection, regardless of whether or
//! ### `RelayOnly` path selection:
//! When set this will force all packets to be sent over
//! the relay connection, regardless of whether or
//! not we have a direct UDP address for the given node.
//!
//! The intended use is for testing the relay protocol inside the MagicSock
Expand Down Expand Up @@ -61,6 +61,8 @@ use self::{
relay_actor::{RelayActor, RelayActorMessage, RelayRecvDatagram},
udp_conn::UdpConn,
};
#[cfg(any(test, feature = "test-utils"))]
use crate::endpoint::PathSelection;
use crate::{
defaults::timeouts::NET_REPORT_TIMEOUT,
disco::{self, CallMeMaybe, SendAddr},
Expand Down Expand Up @@ -128,6 +130,10 @@ pub(crate) struct Options {
/// May only be used in tests.
#[cfg(any(test, feature = "test-utils"))]
pub(crate) insecure_skip_relay_cert_verify: bool,

/// Configuration for what path selection to use
#[cfg(any(test, feature = "test-utils"))]
pub(crate) path_selection: PathSelection,
}

impl Default for Options {
Expand All @@ -143,6 +149,8 @@ impl Default for Options {
dns_resolver: crate::dns::default_resolver().clone(),
#[cfg(any(test, feature = "test-utils"))]
insecure_skip_relay_cert_verify: false,
#[cfg(any(test, feature = "test-utils"))]
path_selection: PathSelection::default(),
}
}
}
Expand Down Expand Up @@ -1493,11 +1501,6 @@ impl Handle {
/// Creates a magic [`MagicSock`] listening on [`Options::addr_v4`] and [`Options::addr_v6`].
async fn new(opts: Options) -> Result<Self> {
let me = opts.secret_key.public().fmt_short();
if crate::util::relay_only_mode() {
warn!(
"creating a MagicSock that will only send packets over a relay relay connection."
);
}

Self::with_name(me, opts)
.instrument(error_span!("magicsock"))
Expand All @@ -1518,6 +1521,8 @@ impl Handle {
proxy_url,
#[cfg(any(test, feature = "test-utils"))]
insecure_skip_relay_cert_verify,
#[cfg(any(test, feature = "test-utils"))]
path_selection,
} = opts;

let relay_datagram_recv_queue = Arc::new(RelayDatagramRecvQueue::new());
Expand Down Expand Up @@ -1548,6 +1553,9 @@ impl Handle {

// 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);
#[cfg(not(any(test, feature = "test-utils")))]
let node_map = NodeMap::load_from_vec(node_map);

let secret_encryption_key = secret_ed_box(secret_key.secret());
Expand Down Expand Up @@ -3815,6 +3823,7 @@ mod tests {
dns_resolver: crate::dns::default_resolver().clone(),
proxy_url: None,
insecure_skip_relay_cert_verify: true,
path_selection: PathSelection::default(),
};
let msock = MagicSock::spawn(opts).await?;
let server_config = crate::endpoint::make_server_config(
Expand Down
Loading

0 comments on commit 5aba17e

Please sign in to comment.