mirror of
https://github.com/jmcorgan/fips.git
synced 2026-07-30 19:46:15 +00:00
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:
@@ -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,
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
+18
-29
@@ -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<u64>,
|
||||
/// 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<u64> {
|
||||
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.*`).
|
||||
|
||||
@@ -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)));
|
||||
|
||||
|
||||
@@ -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<NodeAddr> = Vec::new();
|
||||
let mut to_timeout: Vec<NodeAddr> = 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);
|
||||
|
||||
|
||||
+1
-1
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user