Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(iroh-net): move all timeouts into one file #2641

Merged
merged 7 commits into from
Aug 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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];
matheus23 marked this conversation as resolved.
Show resolved Hide resolved

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
Loading