From d388c709c38a408f75d14b51d16986eb68354f0d Mon Sep 17 00:00:00 2001 From: Martti Malmi Date: Mon, 28 Sep 2026 10:49:46 +0300 Subject: [PATCH 1/2] fix(gateway): preserve the DNS TTL when renewing a draining mapping A DNS refresh updated last_referenced but left the mapping on its old drain timer, allowing reclamation while the renewed answer was still valid. Restore draining mappings to Allocated and clear the old grace window, using the supplied allocation time for renewals too. Cover address renewal at the admission ceiling, refreshes without an address answer, and eventual reclamation after the renewed TTL and grace period. --- CHANGELOG.md | 4 +++ src/gateway/pool.rs | 81 ++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 81 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 40db0a79..7813d910 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -312,6 +312,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 mapping, and one whose client did not re-query DNS was reclaimed about two minutes after its last DNS reference while its traffic was still flowing. Each `dst=` value is now parsed as an address and compared as one. +- A DNS query that refreshes a draining mapping now cancels its old grace + period. Previously the address could be reclaimed while the client's + renewed DNS answer was still valid. The mapping now survives the full + renewed TTL and a fresh grace period before it can be reused. - The conntrack table is read once per tick instead of once per mapping, and the read happens off the runtime thread. The whole file was read and scanned for each mapping in turn, while the pool lock was held, on the same diff --git a/src/gateway/pool.rs b/src/gateway/pool.rs index bac11373..834dbc39 100644 --- a/src/gateway/pool.rs +++ b/src/gateway/pool.rs @@ -509,11 +509,20 @@ impl VirtualIpPool { /// /// Returns whether a mapping for `node_addr` existed. A query the gateway /// answers without an address still says the client is using the name, so - /// it must keep the mapping alive without minting one. + /// it must keep the mapping alive without minting one. Refreshing a + /// draining mapping cancels reclamation for the renewed TTL. pub fn refresh_if_present(&mut self, node_addr: NodeAddr) -> bool { + self.refresh_at(node_addr, Instant::now()) + } + + fn refresh_at(&mut self, node_addr: NodeAddr, now: Instant) -> bool { match self.mappings.get_mut(&node_addr) { Some(mapping) => { - mapping.last_referenced = Instant::now(); + mapping.last_referenced = now; + if mapping.state == MappingState::Draining { + mapping.state = MappingState::Allocated; + mapping.drain_start = None; + } true } None => false, @@ -532,7 +541,7 @@ impl VirtualIpPool { } /// `allocate` at a given instant, which drives the rate limit's refill - /// and stamps a new mapping. + /// and stamps a new or refreshed mapping. /// /// An existing mapping is returned before either limit is consulted, so a /// name already in use keeps resolving when new names are refused. @@ -544,7 +553,7 @@ impl VirtualIpPool { now: Instant, ) -> Result<(Ipv6Addr, bool), PoolError> { // Idempotent: return existing mapping, refreshed. - if self.refresh_if_present(node_addr) + if self.refresh_at(node_addr, now) && let Some(mapping) = self.mappings.get(&node_addr) { return Ok((mapping.virtual_ip, false)); @@ -979,6 +988,70 @@ mod tests { assert_eq!(pool.available.len(), 255); // returned to pool } + #[test] + fn dns_renewal_preserves_the_full_ttl_after_draining() { + let t0 = Instant::now(); + let mut pool = VirtualIpPool::with_limits("fd01::/120", 60, 60, 1, 1, 1).unwrap(); + let ct = ConntrackSnapshot::default(); + let node = make_node_addr(1); + let mesh = make_mesh_addr(1); + let (vip, _) = pool.allocate_at(node, mesh, "test.fips", t0).unwrap(); + + pool.tick(t0 + Duration::from_secs(61), &ct); + assert_eq!(pool.mappings[&node].state, MappingState::Draining); + + // Renew just before the old grace period ends, with admission full. + // The answer reuses the same address and promises another 60s TTL. + let renewed = t0 + Duration::from_secs(120); + assert_eq!( + pool.allocate_at(node, mesh, "test.fips", renewed).unwrap(), + (vip, false) + ); + assert_eq!(pool.bucket.tokens(), 0); + assert!(pool.tick(t0 + Duration::from_secs(122), &ct).is_empty()); + assert!(pool.tick(renewed + Duration::from_secs(60), &ct).is_empty()); + assert_eq!(pool.lookup_virtual_ip(&vip).unwrap().node_addr, node); + assert_eq!(pool.mappings[&node].state, MappingState::Allocated); + + // An idle mapping still expires after its renewed TTL and a fresh + // grace period; renewal must not make addresses immortal. + let drained = renewed + Duration::from_secs(61); + assert!(pool.tick(drained, &ct).is_empty()); + assert_eq!(pool.mappings[&node].state, MappingState::Draining); + assert!(pool.tick(drained + Duration::from_secs(60), &ct).is_empty()); + let events = pool.tick(drained + Duration::from_secs(61), &ct); + assert!(matches!( + events.as_slice(), + [PoolEvent::MappingRemoved { .. }] + )); + assert!(pool.lookup_virtual_ip(&vip).is_none()); + } + + #[test] + fn dns_refresh_without_an_address_cancels_draining() { + let now = Instant::now(); + let mut pool = VirtualIpPool::new("fd01::/120", 60, 10).unwrap(); + let ct = ConntrackSnapshot::default(); + let node = make_node_addr(1); + pool.allocate_at( + node, + make_mesh_addr(1), + "test.fips", + now - Duration::from_secs(62), + ) + .unwrap(); + pool.tick(now - Duration::from_secs(1), &ct); + assert_eq!(pool.mappings[&node].state, MappingState::Draining); + + // A/other queries refresh existing mappings without creating one. + assert!(pool.refresh_if_present(node)); + assert!(!pool.refresh_if_present(make_node_addr(2))); + assert_eq!(pool.mappings.len(), 1); + assert!(pool.tick(now + Duration::from_secs(11), &ct).is_empty()); + assert_eq!(pool.mappings[&node].state, MappingState::Allocated); + assert!(pool.mappings[&node].drain_start.is_none()); + } + #[test] fn test_mapping_lifecycle_active_draining_free() { let mut pool = VirtualIpPool::new("fd01::/120", 1, 1).unwrap(); From 94a16ae19b81a946e65c69d6b2756e66a37fe603 Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Mon, 28 Sep 2026 14:35:50 +0000 Subject: [PATCH 2/2] docs(gateway): describe the virtual-IP pool state machine as the code runs it A DNS query for a draining mapping now returns it to Allocated with a fresh TTL, so the design document gains that transition. The rest of the section is brought in line with the pool's tick at the same time: an allocated mapping that times out drains before it is freed rather than being freed directly, traffic that resumes during the grace period returns a draining mapping to Active, and a mapping only drains once it has no sessions, since sessions refresh it at every tick. --- docs/design/fips-gateway.md | 36 +++++++++++++++++++++++------------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/docs/design/fips-gateway.md b/docs/design/fips-gateway.md index 47165abb..50adeb45 100644 --- a/docs/design/fips-gateway.md +++ b/docs/design/fips-gateway.md @@ -231,34 +231,44 @@ The pool tracks state per address: ```text Allocated ──→ Active ──→ Draining ──→ Free - │ ▲ - └──────────────────────────────────┘ - (TTL expired, no sessions) + │ ▲ + └───────────────────────┘ + (TTL expired, no sessions) + +Draining ──→ Active traffic resumes before the grace period ends +Draining ──→ Allocated a DNS query for the name ``` | State | Meaning | | ----- | ------- | -| Allocated | DNS query created the mapping; no NAT sessions yet. | +| Allocated | DNS query created or renewed the mapping; no NAT sessions yet. | | Active | Conntrack reports at least one session for this virtual IP. | -| Draining | TTL has expired; sessions may still be in progress, or grace period is running after sessions ended. | +| Draining | TTL has expired with no sessions; the grace period is running. | | Free | Reclaimed and available for new allocations. | Transitions: - **Allocated → Active**: conntrack sessions count goes above zero. -- **Allocated → Free**: TTL expires before any session is ever +- **Allocated → Draining**: TTL expires before any session is observed. -- **Active → Draining**: TTL expires (sessions may or may not still - be present). -- **Draining → Free**: session count is zero and the grace period - has elapsed since draining began. +- **Active → Draining**: TTL expires after the last session ends. + Sessions refresh the mapping at every tick, so a mapping in use + does not drain. +- **Draining → Active**: conntrack reports a session again before + the grace period ends. The next drain starts a fresh grace period. +- **Draining → Allocated**: a DNS query for the name, with or + without an address in the answer. The client may now hold a fresh + TTL, so reclamation is cancelled: the mapping gets the full TTL + and, if it stays idle, a fresh grace period. +- **Draining → Free**: the grace period has elapsed since draining + began with no session seen. Timing: - **TTL** (`gateway.dns.ttl`, default 60 s) is both the DNS TTL - returned to the client and the mapping's idle lifetime. Repeated - DNS queries for the same destination refresh the - `last_referenced` timestamp. + returned to the client and the mapping's idle lifetime. A DNS + query for a mapped name refreshes the mapping's idle clock, and + so do conntrack sessions at each tick. - **Grace period** (`gateway.pool_grace_period`, default 60 s) is the dwell time after the last session ends before the address is recycled. It prevents immediate reuse from confusing hosts with