Tune overly aggressive discovery rate limiting

The default 30s post-failure backoff (300s cap, doubling per
consecutive failure) was set to bound traffic from chatty apps
looking up unreachable targets, but in practice it dominates
cold-start mesh convergence: a single timed-out lookup during
initial bloom-filter propagation suppresses any retry for 30s, and
the existing reset triggers (parent change, new peer, first RTT,
reconnection) don't fire on a stable post-handshake topology. The
suppression window winds up dictating the protocol's 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`,
defaulting to `[1, 2, 4, 8]`. Each attempt sends a fresh LookupRequest
with a new random request_id so successive attempts can take different
forwarding paths as the bloom and tree state evolve. The destination
is declared unreachable only after the sequence is exhausted (15s
total at the default).

Disables post-failure suppression by default (`backoff_base_secs`/
`backoff_max_secs` now `0`/`0`). The `DiscoveryBackoff` machinery
stays in tree (inert at zero base/cap); operators with chatty apps
generating repeat lookups against unreachable destinations can opt
back in.

`PendingLookup` field shape unchanged so the control-socket
`show_routing` JSON (`pending_lookups[].attempt`/`initiated_ms`/
`last_sent_ms`) keeps the same schema for fipstop and external
consumers; `last_sent_ms` now means "current-attempt start" under
the new state machine.
This commit is contained in:
Johnathan Corgan
2026-04-26 19:12:19 +00:00
parent cbc78091ab
commit f16b837a12
7 changed files with 116 additions and 95 deletions
+20
View File
@@ -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 updated. In-tree `fipstop` is adjusted to the new schema. The
control socket interface is still pre-1.0 and not covered by control socket interface is still pre-1.0 and not covered by
stability guarantees 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. - Validate bloom filter fill ratio on FilterAnnounce ingress.
Inbound FilterAnnounce messages whose derived false-positive Inbound FilterAnnounce messages whose derived false-positive
rate exceeds `node.bloom.max_inbound_fpr` (new config field, rate exceeds `node.bloom.max_inbound_fpr` (new config field,
+6 -10
View File
@@ -165,12 +165,10 @@ Controls bloom-guided node discovery (LookupRequest/LookupResponse).
| Parameter | Type | Default | Description | | Parameter | Type | Default | Description |
|-----------|------|---------|-------------| |-----------|------|---------|-------------|
| `node.discovery.ttl` | u8 | `64` | Hop limit for LookupRequest forwarding | | `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.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.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.max_attempts` | u8 | `2` | Max attempts per lookup (1 = no retry, 2 = one retry) | | `node.discovery.backoff_max_secs` | u64 | `0` | Cap on optional post-failure backoff |
| `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.forward_min_interval_secs` | u64 | `2` | Transit-side rate limiting: minimum interval between forwarded lookups for the same target | | `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.*`) ### Spanning Tree (`node.tree.*`)
@@ -707,12 +705,10 @@ node:
identity_size: 10000 identity_size: 10000
discovery: discovery:
ttl: 64 ttl: 64
timeout_secs: 10 attempt_timeouts_secs: [1, 2, 4, 8]
recent_expiry_secs: 10 recent_expiry_secs: 10
retry_interval_secs: 5 backoff_base_secs: 0
max_attempts: 2 backoff_max_secs: 0
backoff_base_secs: 30
backoff_max_secs: 300
forward_min_interval_secs: 2 forward_min_interval_secs: 2
tree: tree:
announce_min_interval_ms: 500 announce_min_interval_ms: 500
+22 -10
View File
@@ -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 fresh `request_id` and re-evaluates bloom filter matches, so it can take
a different path if the tree has restructured. 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 Each discovery is a sequence of attempts with growing per-attempt timeouts.
originator enters **exponential backoff** before re-attempting discovery for Default sequence is `[1s, 2s, 4s, 8s]` (configurable via
the same target: `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`) ### Originator Backoff (optional, off by default)
- **Multiplier**: 2x per consecutive failure
- **Cap**: 300s (configurable via `backoff_max_secs`)
Backoff is **reset on topology changes** that might make previously After the per-attempt sequence is exhausted, the originator can additionally
unreachable targets reachable: parent switch, new peer connection, first suppress further fresh lookups for the same target with exponential
RTT measurement from MMP, or peer reconnection. 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 ### Bloom Filter Pre-Check
+18 -29
View File
@@ -192,14 +192,20 @@ pub struct DiscoveryConfig {
/// Hop limit for LookupRequest flood (`node.discovery.ttl`). /// Hop limit for LookupRequest flood (`node.discovery.ttl`).
#[serde(default = "DiscoveryConfig::default_ttl")] #[serde(default = "DiscoveryConfig::default_ttl")]
pub ttl: u8, pub ttl: u8,
/// Lookup completion timeout in seconds (`node.discovery.timeout_secs`). /// Per-attempt timeouts in seconds (`node.discovery.attempt_timeouts_secs`).
#[serde(default = "DiscoveryConfig::default_timeout_secs")] /// Each entry is the time to wait for a response before sending the next
pub timeout_secs: u64, /// 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<u64>,
/// Dedup cache expiry in seconds (`node.discovery.recent_expiry_secs`). /// Dedup cache expiry in seconds (`node.discovery.recent_expiry_secs`).
#[serde(default = "DiscoveryConfig::default_recent_expiry_secs")] #[serde(default = "DiscoveryConfig::default_recent_expiry_secs")]
pub recent_expiry_secs: u64, pub recent_expiry_secs: u64,
/// Base backoff after first lookup failure in seconds (`node.discovery.backoff_base_secs`). /// Base backoff after lookup failure in seconds (`node.discovery.backoff_base_secs`).
/// Doubles per consecutive failure up to `backoff_max_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")] #[serde(default = "DiscoveryConfig::default_backoff_base_secs")]
pub backoff_base_secs: u64, pub backoff_base_secs: u64,
/// Maximum backoff cap in seconds (`node.discovery.backoff_max_secs`). /// Maximum backoff cap in seconds (`node.discovery.backoff_max_secs`).
@@ -210,28 +216,17 @@ pub struct DiscoveryConfig {
/// Defense-in-depth against misbehaving nodes. /// Defense-in-depth against misbehaving nodes.
#[serde(default = "DiscoveryConfig::default_forward_min_interval_secs")] #[serde(default = "DiscoveryConfig::default_forward_min_interval_secs")]
pub forward_min_interval_secs: u64, 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 { impl Default for DiscoveryConfig {
fn default() -> Self { fn default() -> Self {
Self { Self {
ttl: 64, ttl: 64,
timeout_secs: 10, attempt_timeouts_secs: vec![1, 2, 4, 8],
recent_expiry_secs: 10, recent_expiry_secs: 10,
backoff_base_secs: 30, backoff_base_secs: 0,
backoff_max_secs: 300, backoff_max_secs: 0,
forward_min_interval_secs: 2, forward_min_interval_secs: 2,
retry_interval_secs: 5,
max_attempts: 2,
} }
} }
} }
@@ -240,27 +235,21 @@ impl DiscoveryConfig {
fn default_ttl() -> u8 { fn default_ttl() -> u8 {
64 64
} }
fn default_timeout_secs() -> u64 { fn default_attempt_timeouts_secs() -> Vec<u64> {
10 vec![1, 2, 4, 8]
} }
fn default_recent_expiry_secs() -> u64 { fn default_recent_expiry_secs() -> u64 {
10 10
} }
fn default_backoff_base_secs() -> u64 { fn default_backoff_base_secs() -> u64 {
30 0
} }
fn default_backoff_max_secs() -> u64 { fn default_backoff_max_secs() -> u64 {
300 0
} }
fn default_forward_min_interval_secs() -> u64 { fn default_forward_min_interval_secs() -> u64 {
2 2
} }
fn default_retry_interval_secs() -> u64 {
5
}
fn default_max_attempts() -> u8 {
2
}
} }
/// Spanning tree (`node.tree.*`). /// Spanning tree (`node.tree.*`).
+14 -11
View File
@@ -2,10 +2,12 @@
//! //!
//! Two complementary mechanisms: //! Two complementary mechanisms:
//! //!
//! - **`DiscoveryBackoff`** (originator-side): Exponential backoff for failed //! - **`DiscoveryBackoff`** (originator-side, optional): Exponential
//! lookups. After a lookup times out, suppresses re-initiation with //! suppression of fresh lookups after the per-attempt sequence in
//! increasing delays (30s → 60s → 300s cap). Reset on topology changes //! `node.discovery.attempt_timeouts_secs` has been exhausted.
//! (parent change, new peer, first RTT, reconnection). //! **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 //! - **`DiscoveryForwardRateLimiter`** (transit-side): Per-target minimum
//! interval for forwarded requests. Defense-in-depth against misbehaving //! interval for forwarded requests. Defense-in-depth against misbehaving
@@ -19,11 +21,11 @@ use std::time::{Duration, Instant};
// Originator-side: Discovery Backoff // Originator-side: Discovery Backoff
// ============================================================================ // ============================================================================
/// Default base backoff after first lookup failure. /// Default base backoff after first lookup failure. `0` = disabled.
const DEFAULT_BACKOFF_BASE_SECS: u64 = 30; const DEFAULT_BACKOFF_BASE_SECS: u64 = 0;
/// Default maximum backoff cap. /// Default maximum backoff cap. `0` = disabled.
const DEFAULT_BACKOFF_MAX_SECS: u64 = 300; const DEFAULT_BACKOFF_MAX_SECS: u64 = 0;
/// Backoff multiplier per consecutive failure. /// Backoff multiplier per consecutive failure.
const BACKOFF_MULTIPLIER: u64 = 2; const BACKOFF_MULTIPLIER: u64 = 2;
@@ -49,7 +51,7 @@ struct BackoffEntry {
} }
impl DiscoveryBackoff { impl DiscoveryBackoff {
/// Create with default parameters (30s base, 300s cap). /// Create with default parameters (disabled — base/cap = 0).
pub fn new() -> Self { pub fn new() -> Self {
Self::with_params(DEFAULT_BACKOFF_BASE_SECS, DEFAULT_BACKOFF_MAX_SECS) Self::with_params(DEFAULT_BACKOFF_BASE_SECS, DEFAULT_BACKOFF_MAX_SECS)
} }
@@ -245,7 +247,8 @@ mod tests {
#[test] #[test]
fn test_backoff_suppressed_after_failure() { 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)); backoff.record_failure(&addr(1));
assert!(backoff.is_suppressed(&addr(1))); assert!(backoff.is_suppressed(&addr(1)));
// Different target not affected // Different target not affected
@@ -254,7 +257,7 @@ mod tests {
#[test] #[test]
fn test_backoff_cleared_on_success() { fn test_backoff_cleared_on_success() {
let mut backoff = DiscoveryBackoff::new(); let mut backoff = DiscoveryBackoff::with_params(30, 300);
backoff.record_failure(&addr(1)); backoff.record_failure(&addr(1));
assert!(backoff.is_suppressed(&addr(1))); assert!(backoff.is_suppressed(&addr(1)));
+35 -34
View File
@@ -423,31 +423,26 @@ impl Node {
/// Initiate a discovery lookup if one is not already pending for this target. /// Initiate a discovery lookup if one is not already pending for this target.
/// ///
/// Checks: pending dedup, backoff, bloom filter pre-check. If all pass, /// Checks: pending dedup, post-failure backoff (off by default), bloom
/// initiates the lookup. If no tree peers have the target in their bloom /// filter pre-check. If all pass, sends the first attempt's LookupRequest.
/// filter, the lookup is skipped (bloom miss) and recorded as a failure /// Subsequent attempts (with fresh request_ids) are scheduled by
/// for backoff purposes. /// [`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) { pub(in crate::node) async fn maybe_initiate_lookup(&mut self, dest: &NodeAddr) {
let now_ms = Self::now_ms(); let now_ms = Self::now_ms();
let lookup_timeout_ms = self.config.node.discovery.timeout_secs * 1000;
// Check pending lookup dedup (in-flight) // Dedup: any pending lookup means we are already trying.
if let Some(entry) = self.pending_lookups.get(dest) { if self.pending_lookups.contains_key(dest) {
let age_ms = now_ms.saturating_sub(entry.initiated_ms); self.stats_mut().discovery.req_deduplicated += 1;
let attempt = entry.attempt; debug!(
if age_ms < lookup_timeout_ms { target_node = %self.peer_display_name(dest),
self.stats_mut().discovery.req_deduplicated += 1; "Discovery lookup deduplicated, already pending"
debug!( );
target_node = %self.peer_display_name(dest), return;
age_ms = age_ms,
attempt = attempt,
"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) { if self.discovery_backoff.is_suppressed(dest) {
self.stats_mut().discovery.req_backoff_suppressed += 1; self.stats_mut().discovery.req_backoff_suppressed += 1;
debug!( 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: /// Called periodically from the tick handler. The lookup state machine
/// - If retry interval elapsed and attempts remain: resend /// runs through `node.discovery.attempt_timeouts_secs` (default
/// - If total timeout elapsed: fail, record backoff, send ICMP unreachable /// `[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) { 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 timeouts = self.config.node.discovery.attempt_timeouts_secs.clone();
let retry_ms = self.config.node.discovery.retry_interval_secs * 1000; let max_attempts = timeouts.len() as u8;
// Collect targets needing action // Collect targets needing action
let mut to_retry: Vec<NodeAddr> = Vec::new(); let mut to_retry: Vec<NodeAddr> = Vec::new();
let mut to_timeout: Vec<NodeAddr> = Vec::new(); let mut to_timeout: Vec<NodeAddr> = Vec::new();
for (&target, entry) in &self.pending_lookups { for (&target, entry) in &self.pending_lookups {
let age = now_ms.saturating_sub(entry.initiated_ms); let attempt_idx = (entry.attempt as usize).saturating_sub(1);
if age >= timeout_ms { let attempt_timeout_ms = timeouts.get(attempt_idx).copied().unwrap_or(0) * 1000;
to_timeout.push(target); if now_ms.saturating_sub(entry.last_sent_ms) >= attempt_timeout_ms {
} else if entry.attempt < self.config.node.discovery.max_attempts if entry.attempt >= max_attempts {
&& now_ms.saturating_sub(entry.last_sent_ms) >= retry_ms to_timeout.push(target);
{ } else {
to_retry.push(target); to_retry.push(target);
}
} }
} }
@@ -535,7 +536,7 @@ impl Node {
self.stats_mut().discovery.resp_timed_out += 1; self.stats_mut().discovery.resp_timed_out += 1;
self.pending_lookups.remove(&addr); self.pending_lookups.remove(&addr);
// Record failure for backoff // Record failure for optional backoff
self.discovery_backoff.record_failure(&addr); self.discovery_backoff.record_failure(&addr);
let failures = self.discovery_backoff.failure_count(&addr); let failures = self.discovery_backoff.failure_count(&addr);
+1 -1
View File
@@ -260,7 +260,7 @@ struct PendingConnect {
/// ///
/// The `addr_to_link` map enables dispatching incoming packets to the right /// The `addr_to_link` map enables dispatching incoming packets to the right
/// connection before authentication completes. /// 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 { pub struct Node {
// === Identity === // === Identity ===
/// This node's cryptographic identity. /// This node's cryptographic identity.