diff --git a/CHANGELOG.md b/CHANGELOG.md index 9874212..ee9b82d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -138,6 +138,26 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 updated. In-tree `fipstop` is adjusted to the new schema. The control socket interface is still pre-1.0 and not covered by stability guarantees +- Discovery rate limiting retuned to be less aggressive at cold start. + The previous defaults (30s base post-failure suppression, doubling + to a 300s cap, with reset only on parent change / new peer / first + RTT / reconnection) reliably outlasted initial mesh convergence: a + single timed-out lookup during bloom-filter propagation suppressed + any retry for 30s while none of the reset triggers fired on a + stable post-handshake topology. The suppression window dictated + effective time-to-converge instead of bounding repeat traffic. + Replaces the single-lookup-with-internal-retry model + (`timeout_secs`/`retry_interval_secs`/`max_attempts`) with a + per-attempt timeout sequence in + `node.discovery.attempt_timeouts_secs` (default `[1, 2, 4, 8]`). + Each attempt sends a fresh `LookupRequest` with a new `request_id`, + which lets successive attempts take different forwarding paths as + the bloom and tree state evolve. The destination is declared + unreachable only after the full sequence is exhausted (15s total + at the default). Disables post-failure suppression by default + (`backoff_base_secs`/`backoff_max_secs` now both `0`); operators + with chatty apps generating repeat lookups against unreachable + destinations can opt back in - Validate bloom filter fill ratio on FilterAnnounce ingress. Inbound FilterAnnounce messages whose derived false-positive rate exceeds `node.bloom.max_inbound_fpr` (new config field, diff --git a/docs/design/fips-configuration.md b/docs/design/fips-configuration.md index 2bcb0d9..3374f39 100644 --- a/docs/design/fips-configuration.md +++ b/docs/design/fips-configuration.md @@ -165,12 +165,10 @@ Controls bloom-guided node discovery (LookupRequest/LookupResponse). | Parameter | Type | Default | Description | |-----------|------|---------|-------------| | `node.discovery.ttl` | u8 | `64` | Hop limit for LookupRequest forwarding | -| `node.discovery.timeout_secs` | u64 | `10` | Lookup completion timeout | +| `node.discovery.attempt_timeouts_secs` | array<u64> | `[1, 2, 4, 8]` | Per-attempt timeouts. Each entry is the deadline for one `LookupRequest` before sending the next attempt with a fresh `request_id`. Length determines total attempt count; default gives 4 attempts and a 15s total budget | | `node.discovery.recent_expiry_secs` | u64 | `10` | Dedup cache expiry for recent request IDs | -| `node.discovery.retry_interval_secs` | u64 | `5` | Retry interval within the timeout window; after this interval without a response, resend the lookup | -| `node.discovery.max_attempts` | u8 | `2` | Max attempts per lookup (1 = no retry, 2 = one retry) | -| `node.discovery.backoff_base_secs` | u64 | `30` | Base for exponential backoff after lookup failure; doubles per consecutive failure | -| `node.discovery.backoff_max_secs` | u64 | `300` | Cap on exponential backoff (5 minutes) | +| `node.discovery.backoff_base_secs` | u64 | `0` | Optional post-failure suppression base in seconds; doubles per consecutive failure. `0` disables (default) — the per-attempt sequence is the only retry pacing | +| `node.discovery.backoff_max_secs` | u64 | `0` | Cap on optional post-failure backoff | | `node.discovery.forward_min_interval_secs` | u64 | `2` | Transit-side rate limiting: minimum interval between forwarded lookups for the same target | ### Spanning Tree (`node.tree.*`) @@ -707,12 +705,10 @@ node: identity_size: 10000 discovery: ttl: 64 - timeout_secs: 10 + attempt_timeouts_secs: [1, 2, 4, 8] recent_expiry_secs: 10 - retry_interval_secs: 5 - max_attempts: 2 - backoff_base_secs: 30 - backoff_max_secs: 300 + backoff_base_secs: 0 + backoff_max_secs: 0 forward_min_interval_secs: 2 tree: announce_min_interval_ms: 500 diff --git a/docs/design/fips-mesh-operation.md b/docs/design/fips-mesh-operation.md index 1a14725..ebefc35 100644 --- a/docs/design/fips-mesh-operation.md +++ b/docs/design/fips-mesh-operation.md @@ -385,19 +385,31 @@ The default `max_attempts` is 2 (initial + one retry). Each retry generates a fresh `request_id` and re-evaluates bloom filter matches, so it can take a different path if the tree has restructured. -### Originator Backoff +### Per-Attempt Timeouts -After a lookup times out or no peer's bloom filter contains the target, the -originator enters **exponential backoff** before re-attempting discovery for -the same target: +Each discovery is a sequence of attempts with growing per-attempt timeouts. +Default sequence is `[1s, 2s, 4s, 8s]` (configurable via +`node.discovery.attempt_timeouts_secs`). When the current attempt's deadline +elapses without a `LookupResponse`, the originator sends another +`LookupRequest` with a **fresh `request_id`** and the next entry in the +sequence as its deadline. Fresh request_ids let each attempt take a +different forwarding path as the bloom and tree state evolve, which is +particularly useful during cold-start convergence. The destination is +declared unreachable only after the full sequence is exhausted (15s total +with the default). -- **Base delay**: 30s (configurable via `backoff_base_secs`) -- **Multiplier**: 2x per consecutive failure -- **Cap**: 300s (configurable via `backoff_max_secs`) +### Originator Backoff (optional, off by default) -Backoff is **reset on topology changes** that might make previously -unreachable targets reachable: parent switch, new peer connection, first -RTT measurement from MMP, or peer reconnection. +After the per-attempt sequence is exhausted, the originator can additionally +suppress further fresh lookups for the same target with exponential +post-failure backoff. This is **disabled by default** (`backoff_base_secs: +0`); the per-attempt sequence is the only retry pacing in the standard +configuration. Operators may opt in via `node.discovery.backoff_base_secs` +and `node.discovery.backoff_max_secs` if their deployment has chatty apps +generating repeated lookups for genuinely unreachable destinations. When +enabled, backoff is **reset on topology changes** that might make +previously unreachable targets reachable: parent switch, new peer +connection, first RTT measurement from MMP, or peer reconnection. ### Bloom Filter Pre-Check diff --git a/src/config/node.rs b/src/config/node.rs index 2aef794..5155652 100644 --- a/src/config/node.rs +++ b/src/config/node.rs @@ -192,14 +192,20 @@ pub struct DiscoveryConfig { /// Hop limit for LookupRequest flood (`node.discovery.ttl`). #[serde(default = "DiscoveryConfig::default_ttl")] pub ttl: u8, - /// Lookup completion timeout in seconds (`node.discovery.timeout_secs`). - #[serde(default = "DiscoveryConfig::default_timeout_secs")] - pub timeout_secs: u64, + /// Per-attempt timeouts in seconds (`node.discovery.attempt_timeouts_secs`). + /// Each entry is the time to wait for a response before sending the next + /// LookupRequest (with a fresh request_id). Sequence length determines the + /// total number of attempts before declaring the destination unreachable. + /// Default `[1, 2, 4, 8]` gives 4 attempts and a 15s total budget. + #[serde(default = "DiscoveryConfig::default_attempt_timeouts_secs")] + pub attempt_timeouts_secs: Vec, /// Dedup cache expiry in seconds (`node.discovery.recent_expiry_secs`). #[serde(default = "DiscoveryConfig::default_recent_expiry_secs")] pub recent_expiry_secs: u64, - /// Base backoff after first lookup failure in seconds (`node.discovery.backoff_base_secs`). - /// Doubles per consecutive failure up to `backoff_max_secs`. + /// Base backoff after lookup failure in seconds (`node.discovery.backoff_base_secs`). + /// Doubles per consecutive failure up to `backoff_max_secs`. Defaults to 0 + /// (no post-failure suppression); the per-attempt sequence in + /// `attempt_timeouts_secs` provides the only retry pacing. #[serde(default = "DiscoveryConfig::default_backoff_base_secs")] pub backoff_base_secs: u64, /// Maximum backoff cap in seconds (`node.discovery.backoff_max_secs`). @@ -210,28 +216,17 @@ pub struct DiscoveryConfig { /// Defense-in-depth against misbehaving nodes. #[serde(default = "DiscoveryConfig::default_forward_min_interval_secs")] pub forward_min_interval_secs: u64, - /// Retry interval within the timeout window in seconds - /// (`node.discovery.retry_interval_secs`). - /// After this interval without a response, resend the lookup. - #[serde(default = "DiscoveryConfig::default_retry_interval_secs")] - pub retry_interval_secs: u64, - /// Maximum attempts per lookup (`node.discovery.max_attempts`). - /// 1 = no retry, 2 = one retry, etc. - #[serde(default = "DiscoveryConfig::default_max_attempts")] - pub max_attempts: u8, } impl Default for DiscoveryConfig { fn default() -> Self { Self { ttl: 64, - timeout_secs: 10, + attempt_timeouts_secs: vec![1, 2, 4, 8], recent_expiry_secs: 10, - backoff_base_secs: 30, - backoff_max_secs: 300, + backoff_base_secs: 0, + backoff_max_secs: 0, forward_min_interval_secs: 2, - retry_interval_secs: 5, - max_attempts: 2, } } } @@ -240,27 +235,21 @@ impl DiscoveryConfig { fn default_ttl() -> u8 { 64 } - fn default_timeout_secs() -> u64 { - 10 + fn default_attempt_timeouts_secs() -> Vec { + vec![1, 2, 4, 8] } fn default_recent_expiry_secs() -> u64 { 10 } fn default_backoff_base_secs() -> u64 { - 30 + 0 } fn default_backoff_max_secs() -> u64 { - 300 + 0 } fn default_forward_min_interval_secs() -> u64 { 2 } - fn default_retry_interval_secs() -> u64 { - 5 - } - fn default_max_attempts() -> u8 { - 2 - } } /// Spanning tree (`node.tree.*`). diff --git a/src/node/discovery_rate_limit.rs b/src/node/discovery_rate_limit.rs index 6fb9294..ba1106d 100644 --- a/src/node/discovery_rate_limit.rs +++ b/src/node/discovery_rate_limit.rs @@ -2,10 +2,12 @@ //! //! Two complementary mechanisms: //! -//! - **`DiscoveryBackoff`** (originator-side): Exponential backoff for failed -//! lookups. After a lookup times out, suppresses re-initiation with -//! increasing delays (30s → 60s → 300s cap). Reset on topology changes -//! (parent change, new peer, first RTT, reconnection). +//! - **`DiscoveryBackoff`** (originator-side, optional): Exponential +//! suppression of fresh lookups after the per-attempt sequence in +//! `node.discovery.attempt_timeouts_secs` has been exhausted. +//! **Disabled by default** (base/cap = 0); the per-attempt sequence +//! is the only retry pacing in the standard configuration. Reset on +//! topology changes (parent change, new peer, first RTT, reconnection). //! //! - **`DiscoveryForwardRateLimiter`** (transit-side): Per-target minimum //! interval for forwarded requests. Defense-in-depth against misbehaving @@ -19,11 +21,11 @@ use std::time::{Duration, Instant}; // Originator-side: Discovery Backoff // ============================================================================ -/// Default base backoff after first lookup failure. -const DEFAULT_BACKOFF_BASE_SECS: u64 = 30; +/// Default base backoff after first lookup failure. `0` = disabled. +const DEFAULT_BACKOFF_BASE_SECS: u64 = 0; -/// Default maximum backoff cap. -const DEFAULT_BACKOFF_MAX_SECS: u64 = 300; +/// Default maximum backoff cap. `0` = disabled. +const DEFAULT_BACKOFF_MAX_SECS: u64 = 0; /// Backoff multiplier per consecutive failure. const BACKOFF_MULTIPLIER: u64 = 2; @@ -49,7 +51,7 @@ struct BackoffEntry { } impl DiscoveryBackoff { - /// Create with default parameters (30s base, 300s cap). + /// Create with default parameters (disabled — base/cap = 0). pub fn new() -> Self { Self::with_params(DEFAULT_BACKOFF_BASE_SECS, DEFAULT_BACKOFF_MAX_SECS) } @@ -245,7 +247,8 @@ mod tests { #[test] fn test_backoff_suppressed_after_failure() { - let mut backoff = DiscoveryBackoff::new(); + // Backoff is opt-in; exercise the suppression path with explicit params. + let mut backoff = DiscoveryBackoff::with_params(30, 300); backoff.record_failure(&addr(1)); assert!(backoff.is_suppressed(&addr(1))); // Different target not affected @@ -254,7 +257,7 @@ mod tests { #[test] fn test_backoff_cleared_on_success() { - let mut backoff = DiscoveryBackoff::new(); + let mut backoff = DiscoveryBackoff::with_params(30, 300); backoff.record_failure(&addr(1)); assert!(backoff.is_suppressed(&addr(1))); diff --git a/src/node/handlers/discovery.rs b/src/node/handlers/discovery.rs index dd051cf..cb478a3 100644 --- a/src/node/handlers/discovery.rs +++ b/src/node/handlers/discovery.rs @@ -423,31 +423,26 @@ impl Node { /// Initiate a discovery lookup if one is not already pending for this target. /// - /// Checks: pending dedup, backoff, bloom filter pre-check. If all pass, - /// initiates the lookup. If no tree peers have the target in their bloom - /// filter, the lookup is skipped (bloom miss) and recorded as a failure - /// for backoff purposes. + /// Checks: pending dedup, post-failure backoff (off by default), bloom + /// filter pre-check. If all pass, sends the first attempt's LookupRequest. + /// Subsequent attempts (with fresh request_ids) are scheduled by + /// [`Self::check_pending_lookups`] when each attempt's per-attempt timeout + /// expires, using the sequence in `node.discovery.attempt_timeouts_secs`. pub(in crate::node) async fn maybe_initiate_lookup(&mut self, dest: &NodeAddr) { let now_ms = Self::now_ms(); - let lookup_timeout_ms = self.config.node.discovery.timeout_secs * 1000; - // Check pending lookup dedup (in-flight) - if let Some(entry) = self.pending_lookups.get(dest) { - let age_ms = now_ms.saturating_sub(entry.initiated_ms); - let attempt = entry.attempt; - if age_ms < lookup_timeout_ms { - self.stats_mut().discovery.req_deduplicated += 1; - debug!( - target_node = %self.peer_display_name(dest), - age_ms = age_ms, - attempt = attempt, - "Discovery lookup deduplicated, already pending" - ); - return; - } + // Dedup: any pending lookup means we are already trying. + if self.pending_lookups.contains_key(dest) { + self.stats_mut().discovery.req_deduplicated += 1; + debug!( + target_node = %self.peer_display_name(dest), + "Discovery lookup deduplicated, already pending" + ); + return; } - // Check backoff from previous failures + // Optional post-failure suppression. Defaults are 0/0 (inert); + // operators can opt in by setting `node.discovery.backoff_*_secs`. if self.discovery_backoff.is_suppressed(dest) { self.stats_mut().discovery.req_backoff_suppressed += 1; debug!( @@ -487,27 +482,33 @@ impl Node { } } - /// Check pending lookups for retry or timeout. + /// Check pending lookups for next-attempt or final timeout. /// - /// Called periodically from the tick handler. For each pending lookup: - /// - If retry interval elapsed and attempts remain: resend - /// - If total timeout elapsed: fail, record backoff, send ICMP unreachable + /// Called periodically from the tick handler. The lookup state machine + /// runs through `node.discovery.attempt_timeouts_secs` (default + /// `[1, 2, 4, 8]`): each entry is the deadline for one attempt. When the + /// current attempt's deadline elapses: + /// - If more entries remain: send the next attempt with a fresh + /// `request_id`. + /// - Otherwise: declare the destination unreachable, drop queued packets, + /// and emit ICMPv6 destination-unreachable for each. pub(in crate::node) async fn check_pending_lookups(&mut self, now_ms: u64) { - let timeout_ms = self.config.node.discovery.timeout_secs * 1000; - let retry_ms = self.config.node.discovery.retry_interval_secs * 1000; + let timeouts = self.config.node.discovery.attempt_timeouts_secs.clone(); + let max_attempts = timeouts.len() as u8; // Collect targets needing action let mut to_retry: Vec = Vec::new(); let mut to_timeout: Vec = Vec::new(); for (&target, entry) in &self.pending_lookups { - let age = now_ms.saturating_sub(entry.initiated_ms); - if age >= timeout_ms { - to_timeout.push(target); - } else if entry.attempt < self.config.node.discovery.max_attempts - && now_ms.saturating_sub(entry.last_sent_ms) >= retry_ms - { - to_retry.push(target); + let attempt_idx = (entry.attempt as usize).saturating_sub(1); + let attempt_timeout_ms = timeouts.get(attempt_idx).copied().unwrap_or(0) * 1000; + if now_ms.saturating_sub(entry.last_sent_ms) >= attempt_timeout_ms { + if entry.attempt >= max_attempts { + to_timeout.push(target); + } else { + to_retry.push(target); + } } } @@ -535,7 +536,7 @@ impl Node { self.stats_mut().discovery.resp_timed_out += 1; self.pending_lookups.remove(&addr); - // Record failure for backoff + // Record failure for optional backoff self.discovery_backoff.record_failure(&addr); let failures = self.discovery_backoff.failure_count(&addr); diff --git a/src/node/mod.rs b/src/node/mod.rs index f16c7fc..a213673 100644 --- a/src/node/mod.rs +++ b/src/node/mod.rs @@ -260,7 +260,7 @@ struct PendingConnect { /// /// The `addr_to_link` map enables dispatching incoming packets to the right /// connection before authentication completes. -// Discovery lookup constants moved to config: node.discovery.timeout_secs, node.discovery.ttl +// Discovery lookup constants moved to config: node.discovery.attempt_timeouts_secs, node.discovery.ttl pub struct Node { // === Identity === /// This node's cryptographic identity.