Skip to content

Commit

Permalink
refactor(iroh-net): move all timeouts into one file (#2641)
Browse files Browse the repository at this point in the history
## Description

Co-locates all timeout defaults that have to do with making connections
or running netcheck.

closes #2602

## Notes & open questions

I nested some of the timeouts in another module (specifically the ones
pertaining to the relay) but I could do this more for other sections,
ie, the portmapper timeouts could be nested in a `portmapper` module.

## Change checklist

- [x] Self-review.
- [x] 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.

---------

Co-authored-by: Kasey Huizinga <[email protected]>
  • Loading branch information
ramfox and Kasey Huizinga authored Aug 19, 2024
1 parent 6354e04 commit bb808b4
Show file tree
Hide file tree
Showing 11 changed files with 89 additions and 49 deletions.
74 changes: 74 additions & 0 deletions iroh-net/src/defaults.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,3 +125,77 @@ pub mod staging {
}
}
}

/// Contains all timeouts that we use in `iroh-net`.
pub(crate) mod timeouts {
use std::time::Duration;

// Timeouts for netcheck

/// Maximum duration to wait for a netcheck report.
pub(crate) const NETCHECK_REPORT_TIMEOUT: Duration = Duration::from_secs(10);

/// The maximum amount of time netcheck will spend gathering a single report.
pub(crate) const OVERALL_REPORT_TIMEOUT: Duration = Duration::from_secs(5);

/// The total time we wait for all the probes.
///
/// This includes the STUN, ICMP and HTTPS probes, which will all
/// start at different times based on the ProbePlan.
pub(crate) const PROBES_TIMEOUT: Duration = Duration::from_secs(3);

/// How long to await for a captive-portal result.
///
/// This delay is chosen so it starts after good-working STUN probes
/// would have finished, but not too long so the delay is bearable if
/// STUN is blocked.
pub(crate) const CAPTIVE_PORTAL_DELAY: Duration = Duration::from_millis(200);

/// Timeout for captive portal checks
///
/// Must be lower than [`OVERALL_REPORT_TIMEOUT`] minus
/// [`CAPTIVE_PORTAL_DELAY`].
pub(crate) const CAPTIVE_PORTAL_TIMEOUT: Duration = Duration::from_secs(2);

pub(crate) const DNS_TIMEOUT: Duration = Duration::from_secs(3);

/// The amount of time we wait for a hairpinned packet to come back.
pub(crate) const HAIRPIN_CHECK_TIMEOUT: Duration = Duration::from_millis(100);

/// Maximum duration a UPnP search can take before timing out.
pub(crate) const UPNP_SEARCH_TIMEOUT: Duration = Duration::from_secs(1);

/// Timeout to receive a response from a PCP server.
pub(crate) const PCP_RECV_TIMEOUT: Duration = Duration::from_millis(500);

/// Default Pinger timeout
pub(crate) const DEFAULT_PINGER_TIMEOUT: Duration = Duration::from_secs(5);

/// Timeout to receive a response from a NAT-PMP server.
pub(crate) const NAT_PMP_RECV_TIMEOUT: Duration = Duration::from_millis(500);

/// Timeouts specifically used in the iroh-relay
pub(crate) mod relay {
use super::*;

/// Timeout used by the relay client while connecting to the relay server,
/// using `TcpStream::connect`
pub(crate) const DIAL_NODE_TIMEOUT: Duration = Duration::from_millis(1500);
/// Timeout for expecting a pong from the relay server
pub(crate) const PING_TIMEOUT: Duration = Duration::from_secs(5);
/// Timeout for the entire relay connection, which includes dns, dialing
/// the server, upgrading the connection, and completing the handshake
pub(crate) const CONNECT_TIMEOUT: Duration = Duration::from_secs(10);
/// Timeout for our async dns resolver
pub(crate) const DNS_TIMEOUT: Duration = Duration::from_secs(1);

/// Maximum time the client will wait to receive on the connection, since
/// the last message. Longer than this time and the client will consider
/// the connection dead.
pub(crate) const CLIENT_RECV_TIMEOUT: Duration = Duration::from_secs(120);

/// Maximum time the server will attempt to get a successful write to the connection.
#[cfg(feature = "iroh-relay")]
pub(crate) const SERVER_WRITE_TIMEOUT: Duration = Duration::from_secs(2);
}
}
4 changes: 1 addition & 3 deletions iroh-net/src/magicsock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ use url::Url;
use watchable::Watchable;

use crate::{
defaults::timeouts::NETCHECK_REPORT_TIMEOUT,
disco::{self, SendAddr},
discovery::Discovery,
dns::DnsResolver,
Expand Down Expand Up @@ -90,9 +91,6 @@ const ENDPOINTS_FRESH_ENOUGH_DURATION: Duration = Duration::from_secs(27);

const HEARTBEAT_INTERVAL: Duration = Duration::from_secs(5);

/// Maximum duration to wait for a netcheck report.
const NETCHECK_REPORT_TIMEOUT: Duration = Duration::from_secs(10);

/// Contains options for `MagicSock::listen`.
#[derive(derive_more::Debug)]
pub(crate) struct Options {
Expand Down
27 changes: 4 additions & 23 deletions iroh-net/src/netcheck/reportgen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,32 +46,13 @@ mod probes;

use probes::{Probe, ProbePlan, ProbeProto};

/// The maximum amount of time netcheck will spend gathering a single report.
const OVERALL_REPORT_TIMEOUT: Duration = Duration::from_secs(5);

/// The total time we wait for all the probes.
///
/// This includes the STUN, ICMP and HTTPS probes, which will all
/// start at different times based on the [`ProbePlan`].
const PROBES_TIMEOUT: Duration = Duration::from_secs(3);

/// How long to await for a captive-portal result.
///
/// This delay is chosen so it starts after good-working STUN probes
/// would have finished, but not too long so the delay is bearable if
/// STUN is blocked.
const CAPTIVE_PORTAL_DELAY: Duration = Duration::from_millis(200);

/// Timeout for captive portal checks
///
/// Must be lower than [`OVERALL_REPORT_TIMEOUT`] minus
/// [`CAPTIVE_PORTAL_DELAY`].
const CAPTIVE_PORTAL_TIMEOUT: Duration = Duration::from_secs(2);
use crate::defaults::timeouts::{
CAPTIVE_PORTAL_DELAY, CAPTIVE_PORTAL_TIMEOUT, DNS_TIMEOUT, OVERALL_REPORT_TIMEOUT,
PROBES_TIMEOUT,
};

const ENOUGH_NODES: usize = 3;

const DNS_TIMEOUT: Duration = Duration::from_secs(3);

/// Delay used to perform staggered dns queries.
const DNS_STAGGERING_MS: &[u64] = &[200, 300];

Expand Down
5 changes: 2 additions & 3 deletions iroh-net/src/netcheck/reportgen/hairpin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
//! requests to it will fail which is intentional.
use std::net::{Ipv4Addr, SocketAddr, SocketAddrV4};
use std::time::Duration;

use anyhow::{bail, Context, Result};
use tokio::sync::oneshot;
Expand All @@ -25,8 +24,7 @@ use crate::netcheck::{self, reportgen, Inflight};
use crate::stun;
use crate::util::CancelOnDrop;

/// The amount of time we wait for a hairpinned packet to come back.
const HAIRPIN_CHECK_TIMEOUT: Duration = Duration::from_millis(100);
use crate::defaults::timeouts::HAIRPIN_CHECK_TIMEOUT;

/// Handle to the hairpin actor.
///
Expand Down Expand Up @@ -179,6 +177,7 @@ impl Actor {
#[cfg(test)]
mod tests {
use bytes::BytesMut;
use std::time::Duration;
use tokio::sync::mpsc;
use tracing::info;

Expand Down
3 changes: 1 addition & 2 deletions iroh-net/src/ping.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use std::{
time::Duration,
};

use crate::defaults::timeouts::DEFAULT_PINGER_TIMEOUT as DEFAULT_TIMEOUT;
use anyhow::{Context, Result};
use surge_ping::{Client, Config, IcmpPacket, PingIdentifier, PingSequence, ICMP};
use tracing::debug;
Expand Down Expand Up @@ -39,8 +40,6 @@ struct Inner {
client_v4: Mutex<Option<Client>>,
}

const DEFAULT_TIMEOUT: Duration = Duration::from_secs(5);

impl Pinger {
/// Create a new [Pinger].
pub fn new() -> Self {
Expand Down
4 changes: 1 addition & 3 deletions iroh-net/src/portmapper/nat_pmp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,13 @@ use std::{net::Ipv4Addr, num::NonZeroU16, time::Duration};

use tracing::{debug, trace};

use crate::defaults::timeouts::NAT_PMP_RECV_TIMEOUT as RECV_TIMEOUT;
use crate::net::UdpSocket;

use self::protocol::{MapProtocol, Request, Response};

mod protocol;

/// Timeout to receive a response from a NAT-PMP server.
const RECV_TIMEOUT: Duration = Duration::from_millis(500);

/// Recommended lifetime is 2 hours. See [RFC 6886 Requesting a
/// Mapping](https://datatracker.ietf.org/doc/html/rfc6886#section-3.3).
const MAPPING_REQUESTED_LIFETIME_SECONDS: u32 = 60 * 60 * 2;
Expand Down
4 changes: 1 addition & 3 deletions iroh-net/src/portmapper/pcp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,11 @@ use std::{net::Ipv4Addr, num::NonZeroU16, time::Duration};
use rand::RngCore;
use tracing::{debug, trace};

use crate::defaults::timeouts::PCP_RECV_TIMEOUT as RECV_TIMEOUT;
use crate::net::UdpSocket;

mod protocol;

/// Timeout to receive a response from a PCP server.
const RECV_TIMEOUT: Duration = Duration::from_millis(500);

/// Use the recommended port mapping lifetime for PMP, which is 2 hours. See
/// <https://datatracker.ietf.org/doc/html/rfc6886#section-3.3>
const MAPPING_REQUESTED_LIFETIME_SECONDS: u32 = 60 * 60;
Expand Down
5 changes: 2 additions & 3 deletions iroh-net/src/portmapper/upnp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,11 @@ use super::Metrics;

pub type Gateway = aigd::Gateway<aigd::tokio::Tokio>;

use crate::defaults::timeouts::UPNP_SEARCH_TIMEOUT as SEARCH_TIMEOUT;

/// Seconds we ask the router to maintain the port mapping. 0 means infinite.
const PORT_MAPPING_LEASE_DURATION_SECONDS: u32 = 0;

/// Maximum duration a UPnP search can take before timing out.
const SEARCH_TIMEOUT: Duration = Duration::from_secs(1);

/// Tailscale uses the recommended port mapping lifetime for PMP, which is 2 hours. So we assume a
/// half lifetime of 1h. See <https://datatracker.ietf.org/doc/html/rfc6886#section-3.3>
const HALF_LIFETIME: Duration = Duration::from_secs(60 * 60);
Expand Down
6 changes: 1 addition & 5 deletions iroh-net/src/relay/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ use conn::{
};
use streams::{downcast_upgrade, MaybeTlsStream, ProxyStream};

use crate::defaults::timeouts::relay::*;
use crate::dns::{DnsResolver, ResolverExt};
use crate::key::{PublicKey, SecretKey};
use crate::relay::codec::DerpCodec;
Expand All @@ -43,11 +44,6 @@ use crate::util::AbortingJoinHandle;
pub(crate) mod conn;
pub(crate) mod streams;

const DIAL_NODE_TIMEOUT: Duration = Duration::from_millis(1500);
const PING_TIMEOUT: Duration = Duration::from_secs(5);
const CONNECT_TIMEOUT: Duration = Duration::from_secs(10);
const DNS_TIMEOUT: Duration = Duration::from_secs(1);

/// Possible connection errors on the [`Client`]
#[derive(Debug, thiserror::Error)]
pub enum ClientError {
Expand Down
3 changes: 1 addition & 2 deletions iroh-net/src/relay/client/conn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ use tokio_tungstenite_wasm::WebSocketStream;
use tokio_util::codec::{FramedRead, FramedWrite};
use tracing::{debug, info_span, trace, Instrument};

use crate::defaults::timeouts::relay::CLIENT_RECV_TIMEOUT;
use crate::key::{PublicKey, SecretKey};
use crate::relay::client::streams::{MaybeTlsStreamReader, MaybeTlsStreamWriter};
use crate::relay::codec::{
Expand All @@ -28,8 +29,6 @@ use crate::relay::codec::{
use crate::relay::codec::{ClientInfo, PER_CLIENT_READ_QUEUE_DEPTH};
use crate::util::AbortingJoinHandle;

const CLIENT_RECV_TIMEOUT: Duration = Duration::from_secs(120);

impl PartialEq for Conn {
fn eq(&self, other: &Self) -> bool {
Arc::ptr_eq(&self.inner, &other.inner)
Expand Down
3 changes: 1 addition & 2 deletions iroh-net/src/relay/server/actor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use tokio_util::sync::CancellationToken;
use tracing::{info_span, trace, Instrument};
use tungstenite::protocol::Role;

use crate::defaults::timeouts::relay::SERVER_WRITE_TIMEOUT as WRITE_TIMEOUT;
use crate::key::{PublicKey, SecretKey};
use crate::relay::http::Protocol;
use crate::relay::server::streams::{MaybeTlsStream, RelayIo};
Expand All @@ -41,8 +42,6 @@ fn new_conn_num() -> usize {
CONN_NUM.fetch_add(1, Ordering::Relaxed)
}

pub(crate) const WRITE_TIMEOUT: Duration = Duration::from_secs(2);

/// The task for a running server actor.
///
/// Will forcefully abort the server actor loop when dropped.
Expand Down

0 comments on commit bb808b4

Please sign in to comment.