Feature gap: address-persistent SNAT pool mode silently degrades to round-robin in userspace-dp

[psaab]

Gap

The eBPF dataplane implements address-persistent SNAT pool mode (Junos pool-name { address-persistent; }): the pool address selected for a given internal source IP is sticky — re-NAT'd flows from the same internal source always pick the same pool address. The userspace dataplane parses the same pool_addresses snapshot field but its PortAllocator does round-robin only — the addr_persistent semantic is silently lost. This is a CORRECTNESS regression for any deployment relying on sticky external-IP affinity (e.g. SIP, stateful upstream auth tied to client IP).

eBPF implementation (source of truth)

• bpf/xdp/xdp_policy.c:442-445 — if (cfg->addr_persistent) ip_idx = ((__u32)src_ip) % cfg->num_ips; (IPv4 sticky path)
• bpf/xdp/xdp_policy.c:527-535 — same for IPv6: hash last 4 bytes of source for sticky pool index
• pkg/dataplane/compiler_nat.go:306, 419 — if natCfg.AddressPersistent { ... } propagates the flag into BPF nat_pool_config
• bpf/headers/xpf_nat.h (struct nat_pool_config { __u8 addr_persistent; ... })

Userspace-dp gap

userspace-dp/src/nat.rs:107-114 PortAllocator::next_address_index():

pub(crate) fn next_address_index(&self) -> usize {
    if self.counters.is_empty() { return 0; }
    let idx = self.addr_counter.fetch_add(1, Ordering::Relaxed);
    (idx as usize) % self.counters.len()
}

grep -rn 'address.persistent\\|address_persistent\\|AddressPersistent' userspace-dp/src/ — zero matches. grep -rn 'address_persistent\\|AddressPersistent\\|addr_persistent' pkg/dataplane/userspace/snapshot.go — zero matches. The Go-side snapshot does not even carry the flag to Rust.

Recommended fix

1. Plumb AddressPersistent bool through SourceNATRuleSnapshot in pkg/dataplane/userspace/protocol.go + matching field on the Rust SourceNATRuleSnapshot
2. In userspace-dp/src/nat.rs::PortAllocator, add next_address_index_for(src_ip: IpAddr) that returns hash(src_ip) % num_ips when sticky, else falls back to round-robin
3. Add capability gate or test that address-persistent config actually round-trips correctly under userspace mode

Blocker for #1373

This must land before Phase 4 (BPF source removal) of #1373. Today the eBPF dataplane preserves correctness for these configs; once eBPF is removed there is no fallback path and the silent regression becomes permanent.

---

Refined contract (added 2026-05-17 after triple-review of #1384 and code review of #1385)

See docs/pr/1373-retire-ebpf-dataplane/plan-1377-snat-pools.md for the full implementation contract. New since the original issue body:

• Cross-backend algorithm divergence pinned — legacy eBPF IPv4 uses src_ip % num_ips; legacy eBPF IPv6 XORs the four 32-bit source-address lanes mod pool size; current userspace uses xpf-userspace-snat-address-persistent-v1 SHA-256 over (family_tag, src_octets); DPDK has allocator-local internals. Feature gap: address-persistent SNAT pool mode silently degrades to round-robin in userspace-dp #1377 must either standardize a shared algorithm or document a compatibility break + constrain mixed-backend failover/rollback tests. Until then, "address-persistent" means stable within one backend's pool order/size only.
• Userspace address-persistent SNAT pools #1385 (merged) is the prerequisite fail-closed fix for missing/malformed pool admission, wrong-family pool shadowing, and uint16 port-range bounds — full address-persistent semantics still owed by Feature gap: address-persistent SNAT pool mode silently degrades to round-robin in userspace-dp #1377.
• Distinction surfaced: Junos global source address-persistent (per-source-IP sticky pool address) ≠ per-pool persistent-nat (per-flow sticky NAT mapping). Feature gap: address-persistent SNAT pool mode silently degrades to round-robin in userspace-dp #1377 must implement both, not just the first.
• Hot-path invariant: address-persistent pool choice must be deterministic for (backend, source_address, pool_family, pool_order); any cross-backend divergence captured in tests and docs.

[psaab]

Implementation contract

Priority: silent correctness regression in production today. Anyone running pool-mode SNAT with `address-persistent` on the userspace cluster is getting silent round-robin selection, breaking sticky external-IP affinity (SIP, stateful upstream auth, etc.).

Acceptance criteria

1. `SourceNATRuleSnapshot` in `pkg/dataplane/userspace/protocol.go` gains `AddressPersistent bool` field.
2. Matching Rust field in `userspace-dp/src/nat.rs::SourceNATRuleSnapshot`.
3. `PortAllocator` adds method `next_address_index_for(src_ip: IpAddr) -> usize` that returns `hash(src_ip) % num_ips` when sticky, falls back to `fetch_add` round-robin otherwise.
4. All call sites that allocate a pool address pass the source IP (or sticky context).
5. The capability gate in `pkg/dataplane/userspace/snapshot.go` no longer rejects (or no longer silently downgrades) configs with `address-persistent`.
6. Same internal src_ip → same external IP across multiple flows. Different src_ips spread across the pool.

Test plan

• Cargo: `userspace-dp/src/nat.rs` test `port_allocator_sticky_returns_same_address_for_same_src_ip` over 100 flows from 5 different src_ips against a 4-IP pool; assert each src_ip uses exactly 1 external IP.
• Cargo: `port_allocator_sticky_distributes_across_pool` over 1000 distinct src_ips → assert ≥80% pool address utilization (no hash collision pathology).
• Go: `pkg/dataplane/userspace/snapshot_test.go` adds case verifying `AddressPersistent` flag round-trips correctly through the protocol.
• Integration: cluster smoke with pool-mode SNAT + `address-persistent` config + 10 different src_ips × 3 flows each → each src_ip uses 1 external IP (verified via NAT session table inspection).

Dependencies

None — can land independently of #1381. Pure plumbing fix.

Size estimate

S (<2 days). Small protocol field + small allocator method + 2 tests.

[psaab]

Plan-review round 1 — PLAN-NEEDS-MAJOR

Reviewer	Verdict
Codex	PLAN-NEEDS-MAJOR (8 concrete gaps)
Gemini Pro 3	PLAN-NEEDS-MINOR (validates the bug exists today; aligns on hash + HA seed)

Convergent findings — bug confirmed

Gemini validated the production bug: current SNAT path uses meta->src_ip.v4 % num_ips. Because src_ip.v4 is network byte order, on little-endian x86 the lowest bits are the FIRST OCTET. With pool size 2, any IP starting with an even number always maps to index 0 — complete spatial collapse. This isn't just round-robin; it's a partition collapse hiding behind round-robin appearance.

Codex MAJOR findings

1. Hash choice is not optional — must be explicit + deterministic. Rust DefaultHasher/random SipHash is broken across daemon restarts AND HA peers. sha2 is already in userspace-dp/Cargo.toml; SHA-256 over (domain_tag, addr_family, canonical_ip_bytes) works.
2. IPv6 must hash all 128 bits with address-family tag — distinguish 192.0.2.1 from ::ffff:192.0.2.1 from ::192.0.2.1 from link-local. Current eBPF v6 uses XOR of 32-bit words (xdp_policy.c:441) — weak; userspace plan should be stricter.
3. Pool resizing is not stable under % num_ips. 4→8 addresses remaps ~50% of sources. Plan must define semantic: "sticky within pool-size window" vs "sticky across resize". Junos address-persistent docs explicitly note pool rebuilds affect address picking.
4. HA seed agreement is a HARD blocker. Process-local random key = peers disagree post-failover. Must be config-synchronized deterministic seed OR fixed domain-separated constant. Without this, sticky NAT silently breaks after failover.
5. Source-zone semantics unresolved. Same private IP in two zones using same pool → same external IP unless zone is part of key. Plan must pick.
6. Statistical test in contract is weak. "100 src_ips × 4-IP pool" can look fine under both round-robin and hash. Need deterministic per-source assertion (single src_ip across N flows → exactly 1 external IP).
7. One-address pool + AddressPersistent=true must be no-op success, not error.
8. Existing Go SourceNATRuleSnapshot lacks Rust's pool_addresses, port_low, port_high fields (protocol.go:232 vs protocol.rs:393). Plumb fix may be a prerequisite — verify.

Size revision

Codex M (3-5 days), not S if HA-configurable seeds + zone scoping included. S (<2 days) only if you commit to: fixed seed constant, no zone scoping, sticky-within-pool-window semantics, no pool-resize persistence.

Recommended plan v2

• Pick hash: SHA-256 over (domain_tag, addr_family, canonical_ip_bytes) — deterministic, HA-safe, IPv6-correct
• Use a fixed HA_SNAT_SEED constant in the binary (not config-synced) — simplest path that survives failover
• Document the trade-offs explicitly: sticky only within pool-size window; pool resize breaks sticky
• Plumb missing Rust fields (pool_addresses, port_low, port_high) through Go protocol FIRST if they're not already there
• Strengthen the regression test: 1 src_ip × 5 sequential flows → assert single external IP

This is the highest-priority of the 8 — it's a silent correctness regression in production. Get plan v2 right and ship.

Codex task: `task-mp8rfgvo-smp0gj`. Gemini Pro 3 task: `task-mp8rfr7q-krkt45`.

[psaab]

Update after #1385: keep open, but narrow away from the original silent round-robin bug. Landed: userspace pool address/port snapshotting and userspace-v1 SHA-256 address-persistent selection. Remaining: decide/document cross-backend compatibility, implement/define persistent-NAT mapping semantics, failover/rollback constraints, and persistence/exhaustion counters.