diff --git a/AGENTS.md b/AGENTS.md index 2376a88..ba245ac 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -14,6 +14,8 @@ Cursor, Gemini CLI, …). Read this before starting work, and follow the gates i real API source (not memory, not docs summaries) and confirm the approach before writing code. - **No code duplicates.** Re-use existing code and logic where applicable. +- **Protect latency.** Keep database and external I/O outside shared locks unless ordering requires it; preserve lifecycle serialization and bound concurrent work. +- **Check performance regressions.** For routing, storage or lifecycle changes, compare encrypted cold/warm concurrent load and teardown timings, including errors and cleanup, before declaring ready. ## Submitting a PR @@ -46,7 +48,8 @@ Every change ships only after it has run end-to-end on real hosts. - The gRPC control channel is TLS. Each host needs `JWT_SIGNING_KEY`, `MFA_ENCRYPTION_KEY`, `CONTROL_SERVICE_TLS_SAN`, and `ca-cert.pem` present in `/root/nullnet/`, plus a rebuilt proxy. See SETUP.md for the full setup. -- Containers need a restart after nullnet restarts. +- Verify client restart recovery without restarting application containers; + startup cleanup must preserve Docker-owned interfaces and routes. - To restart a Swarm stack, use `docker service update --force`. - The default-deny eBPF firewall blocks Swarm ports. `2377`, `7946`, and `4789` must be in the `.env` allowlists on strict nodes, or the worker goes Down. diff --git a/CHANGELOG.md b/CHANGELOG.md index 2adab0d..77a23c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ All Nullnet releases with the relative changes are documented in this file. - Per-service egress/ingress traffic filters: arbitrary AND/OR/group combinations of Country, Organization, Src IP (ingress), and Dst IP (egress) conditions, evaluated via `rpn-predicate-interpreter` — replaces the country-only egress/ingress policy ([#171](https://github.com/NullNet-ai/nullnet/pull/171) — fixes [#143](https://github.com/NullNet-ai/nullnet/issues/143)) - Persist ingress and egress sessions to SQLite and show the full history on the Sessions page, filterable by status, service, direction, and policy verdict, with its own retention window ([#170](https://github.com/NullNet-ai/nullnet/pull/170) — fixes [#156](https://github.com/NullNet-ai/nullnet/issues/156)) ### Changed +- Improve routing performance and capped-network balancing, reduce storage contention, and harden tunnel setup, teardown, and recovery under concurrent load ([#199](https://github.com/NullNet-ai/nullnet/pull/199)) - Simplify the dashboard to linked session, service, and node count cards ([#194](https://github.com/NullNet-ai/nullnet/pull/194)) - Unify live and historical session views ([#191](https://github.com/NullNet-ai/nullnet/pull/191)) - Simplify backend configuration to a `backends` array of service names and derive trigger ports from peer declarations ([#190](https://github.com/NullNet-ai/nullnet/pull/190)) @@ -24,8 +25,8 @@ All Nullnet releases with the relative changes are documented in this file. - Install BPF linker as a prebuilt binary rather than compiling it from source ([#158](https://github.com/NullNet-ai/nullnet/pull/158)) ### Removed ### Fixed -- Route same-host egress directly with container-scoped forwarding and NAT ([#196](https://github.com/NullNet-ai/nullnet/pull/196)) -- Track container overlay addresses when deciding whether backend connections are idle ([#195](https://github.com/NullNet-ai/nullnet/pull/195)) +- Route same-host egress directly with container-scoped forwarding and NAT ([#197](https://github.com/NullNet-ai/nullnet/pull/197)) +- Track container overlay addresses when deciding whether backend connections are idle ([#196](https://github.com/NullNet-ai/nullnet/pull/196)) - Make backend and egress trigger claims atomic, preserve liveness during setup, and report setup failures instead of transient packet-wait timeouts ([#186](https://github.com/NullNet-ai/nullnet/pull/186)) - Set both same-host veth MAC addresses at creation to prevent udev races from breaking encrypted connections ([#185](https://github.com/NullNet-ai/nullnet/pull/185)) - Stop a teardown from corrupting a chain that is still being set up ([#167](https://github.com/NullNet-ai/nullnet/pull/167) — fixes [#166](https://github.com/NullNet-ai/nullnet/issues/166)) diff --git a/Cargo.lock b/Cargo.lock index f5f6795..631e576 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -424,7 +424,7 @@ dependencies = [ "aws-smithy-runtime-api", "aws-smithy-types", "h2 0.3.27", - "h2 0.4.16", + "h2 0.4.19", "http 0.2.12", "http 1.5.0", "http-body 0.4.6", @@ -2015,9 +2015,9 @@ dependencies = [ [[package]] name = "h2" -version = "0.4.16" +version = "0.4.19" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a9f37a958b41b3b19ee2707c06439c0e9e547e847223eb791ecb0cb821c65e27" +checksum = "ef8e5e5a340588f4452631496976cf8636d4a7ecf600239fdc27615d2530bc16" dependencies = [ "atomic-waker", "bytes", @@ -2229,7 +2229,7 @@ dependencies = [ "bytes", "futures-channel", "futures-core", - "h2 0.4.16", + "h2 0.4.19", "http 1.5.0", "http-body 1.1.0", "httparse", @@ -3690,7 +3690,7 @@ dependencies = [ "derivative", "flate2", "futures", - "h2 0.4.16", + "h2 0.4.19", "http 1.5.0", "httparse", "httpdate", @@ -3807,7 +3807,7 @@ dependencies = [ "bytes", "clap", "futures", - "h2 0.4.16", + "h2 0.4.19", "http 1.5.0", "log", "once_cell", @@ -4281,7 +4281,7 @@ dependencies = [ "bytes", "encoding_rs", "futures-core", - "h2 0.4.16", + "h2 0.4.19", "http 1.5.0", "http-body 1.1.0", "http-body-util", @@ -4321,7 +4321,7 @@ dependencies = [ "bytes", "encoding_rs", "futures-core", - "h2 0.4.16", + "h2 0.4.19", "http 1.5.0", "http-body 1.1.0", "http-body-util", @@ -5414,7 +5414,7 @@ dependencies = [ "axum", "base64 0.22.1", "bytes", - "h2 0.4.16", + "h2 0.4.19", "http 1.5.0", "http-body 1.1.0", "http-body-util", diff --git a/SETUP.md b/SETUP.md index d18aeb2..c1ecfb8 100644 --- a/SETUP.md +++ b/SETUP.md @@ -31,6 +31,7 @@ plus the shared gRPC interface. The repository should be cloned under `/root` so the provided `setup-*.sh` scripts and `.service` units work without changes. +Run the setup scripts as root. The supplied systemd services also run as root; privileged commands execute directly without sudo. ## Usage @@ -125,6 +126,11 @@ The repository should be cloned under `/root` so the provided `setup-*.sh` scrip EVENT_RETENTION_DAYS=7 # how long an event is kept EVENT_RETENTION_SWEEP_INTERVAL_SECS=3600 # how often the deletion sweep runs (1h) ``` + Writes use a bounded queue and batched transactions; normal events appear after commit. + Storage failures retain accepted events and retry, reporting failure/recovery in Events + and stderr. A full queue drops new events instead of blocking routing; Events reports + the dropped count when storage catches up. Shutdown drains for up to five seconds; + a longer outage, forced kill or power loss can lose events still queued in memory. - sessions (UI: *Sessions* page) are persisted the same way: every ingress session and every external destination reached over an egress edge is stored, so the page shows the full @@ -137,6 +143,14 @@ The repository should be cloned under `/root` so the provided `setup-*.sh` scrip An active session is never pruned, however old. Sessions still marked active are closed at startup, since the state they described died with the previous process. +- Upgrade server, proxy and clients together: VXLAN backend/egress packet release uses + an acknowledged `NetReady` command after both endpoints finish setup. Nullnet reserves + Linux interface groups `0x4e000000`–`0x4e400000` for tunnel cleanup; do not assign + unrelated interfaces to that range. + Failed setup is not acknowledged as ready. Failed teardown keeps its ID reserved; + after correcting the reported error, restart that client to purge stale state and + release the reservation. Application containers do not need restarting. + - the gRPC control channel itself (nullnet-client/nullnet-proxy ↔ nullnet-server) is TLS-only, authenticated by a private CA. On first boot the server generates its own CA (`members/nullnet-server/grpc-tls/ca-cert.pem` + `ca-key.pem`, created once and never @@ -369,7 +383,15 @@ The repository should be cloned under `/root` so the provided `setup-*.sh` scrip ``` ./setup-client.sh ``` - + Setup applies and persists a conntrack limit of at least 1,048,576 entries, + preserving a higher existing limit. No manual sysctl configuration is needed. + It also keeps NetworkManager and udev hotplug helpers from managing Nullnet-owned + interfaces, avoiding competing network-configuration work during tunnel bursts. + +On upgrades, deploy the server, proxy and all clients from the same revision. +Client startup cleans stale Nullnet network state while preserving Docker interfaces; +routine container restarts are unnecessary. Containers already damaged by an older +client's cleanup need a one-time restart (Swarm: `docker service update --force SERVICE`). ### Observation mode diff --git a/docs/nullnet-performance-2026-09-21.md b/docs/nullnet-performance-2026-09-21.md new file mode 100644 index 0000000..a883519 --- /dev/null +++ b/docs/nullnet-performance-2026-09-21.md @@ -0,0 +1,162 @@ +# Nullnet performance — 21 September 2026 + +Compared **main** (`28a4c2e`), **perf** and **capped-perf** (same code, `max_networks = 10` for every service). Two Linux hosts, each with 8 vCPUs and 16 GiB RAM; 12 services, 24 replicas and 19 backend relationships. Requests exercised those backends. ESP/AES-GCM, MACsec AES-256-GCM and control-channel TLS stayed enabled. + +The comparison below predates the HTTP/2 update. Uncapped single/large-load results use `03a4fd6`; the warm test was repeated after the balancing fix. + +## Results + +| Measurement | Main | Perf | Capped-perf | +|---|---:|---:|---:| +| Single cold request, average of 20 isolated rounds | 211.9 ms | 82.0 ms | 76.9 ms | +| Setup beside Net ID in the UI, same rounds | 186.4 ms | 68.3 ms | 64.1 ms | +| One endpoint’s setup / teardown, same rounds | 171.3 / 212.2 ms | 61.9 / 119.3 ms | 57.3 / 102.0 ms | +| 2,048 new clients, 64 concurrent: successful requests/second | 0.15 | 13.69 | 136.22 | +| Same cold workload: requests served successfully | 7.11% | 100% | 100% | +| Warm requests from one IP, 32 concurrent: average / successful requests/second | 287.9 ms / 110 | 9.1 ms / 3,522 | 9.0 ms / 3,553 | +| 2,000 distinct clients together: average first-request time | Not measured | 44.26 s | 18.37 s | +| Same 10-minute test, final 5 minutes: average / successful requests/second | Not measured | 1.27 s / 1,570 | 0.71 s / 2,829 | + +Main’s cold-client test stopped at its time limit after 2,026 attempts. Individual request latency includes queueing; burst duration divided by request count is **not** individual latency. Endpoint teardown excludes idle timeout and includes kernel work. These are individual lab trials; host load varies. The cap does not affect isolated setup, so small differences there are normal variation. + +## Where cold requests spend time + +A separate encrypted, capped 60-second profiling run served **138,223 requests, all successful**. The first request from each of 2,000 simultaneous clients averaged **12.40 seconds**, broken down as follows (a fresh trial, separate from the 10-minute comparison above): + +| Part of the first request | Average | +|---|---:| +| Waiting for an available gRPC call slot | 11.98 s (96.6%) | +| Executing policy and routing calls, including setup when needed | 0.28 s | +| Connecting to the proxy and remaining request work | 0.14 s | + +Slow cold setups hold slots while other requests queue; most clients subsequently reuse networks. Endpoint setup averaged 2.04 s on the busier host and 0.41 s on the other. Kernel samples showed network-configuration and firewall waits, but do not establish an unavoidable kernel limit. + +The old HTTP/2 dependency disconnected twice with `too_many_data_frames` when the shared call limit was raised from 32 to 64. Updating `h2` from 0.4.16 to 0.4.19 brings upstream fixes for frame accounting ([release notes](https://github.com/hyperium/h2/blob/v0.4.19/CHANGELOG.md)). Removing all request limits still overloaded tunnel creation. A replacement server-side limit was slower and was discarded. The existing 32-call limit remains; only the extra transport receipt protocol is removed. Setup, readiness and teardown acknowledgements remain intact. + +The updated build's capped burst served **124,742 requests successfully**, but one initial request reached the **180-second timeout**. Its cause remains unresolved; this is **not a clean validation pass**. The other 1,999 first requests averaged **10.29 s**. A subsequent warm test served **47,053 requests**, all successful, averaging **13.60 ms / 2,351 requests per second**. No HTTP/2 disconnect or daemon restart occurred during these tests. These individual trials do not prove a speed improvement from the update. A short uncapped test also passed 64/64 cold requests (1.64 s average), using a 5-second idle timeout to shorten cleanup. Both hosts returned to zero owned interfaces, namespaces and matching IPsec entries; graph and active sessions also reached zero. Original configuration and routes were restored, with all 24 application processes unchanged. Full Linux CI passed; this run left end-to-end verification incomplete. The later cap-5 repeat below passed, but does not explain that stall. + +## 100 clients — four cases, 22 September + +Two trials per case, each with **100 distinct IPs and 100 concurrent requests total across 12 services**, followed by 15 seconds of repeated requests from those same clients. Encryption stayed enabled. Main uses the original `28a4c2e` release binaries; all perf cases use `9691900`. Averages below are individual successful HTTP request durations, including waiting and backend calls; throughput includes draining outstanding requests. + +| Measurement, both trials combined | Main, uncapped | Perf, uncapped | Perf, cap 10 | Perf, cap 5 | +|---|---:|---:|---:|---:| +| Cold requests served / attempted | 0 / 200 | 200 / 200 | 200 / 200 | 200 / 200 | +| Cold average | No successes | 5.14 s | 3.70 s | 2.21 s | +| Subsequent load: requests served / attempted | 852 / 2,134 | 81,211 / 81,211 | 59,310 / 59,310 | 88,307 / 88,307 | +| Subsequent load: average successful request | 2.49 s | 37.0 ms | 50.6 ms | 34.0 ms | +| Subsequent load: successful requests/second | 16.8 | 2,703 | 1,972 | 2,939 | + +Main's cold burst failed in both trials, with proxy restarts and HTTP/2 watch-stream errors. Its subsequent load was **not fully warm** and succeeded only 39.9% of the time. The perf cases served every request successfully. + +Cap 5 used 60 ingress networks, versus 100 with cap 10; each had 38 backend networks. There were only 8–9 clients per service, so cap 10 never forced sharing. Its timing difference from uncapped perf cannot be credited to the cap; these short trials show substantial variation. Cap 5's cold averages were 2.28 / 2.14 s, versus 5.04 / 5.24 s uncapped and 3.23 / 4.16 s at cap 10. It reduced cold work at this load; the larger-test replica imbalance still matters for production. + +All trials began with empty networks. Main's Docker host interfaces were temporarily renamed to protect them from its broad startup cleanup; its binaries were unchanged. Final recovery restored perf, original interface names, configuration and routes. Application processes and container routes remained intact. These short comparisons do not prove natural cleanup or long-term reliability. + +## Cap 5 versus 10 — 22 September + +Two short trials per cap on the updated build: 2,000 simultaneous new clients, then 15 seconds of warm traffic at the same concurrency. Same encrypted 12-service topology and two replicas per service. All **206,228 requests succeeded**. + +| Measurement | Cap 10 | Cap 5 | +|---|---:|---:| +| Cold average, combined trials | 15.07 s | 13.70 s | +| Cold average in each trial | 10.46 / 19.68 s | 16.02 / 11.37 s | +| Warm average | 646 ms | 590 ms | +| Warm successful requests/second | 3,021 | 3,317 | +| Ingress networks / backend networks | 120 / 38 | 60 / 38 | +| Clients sharing each ingress network | 16–17 | 33–34 | + +Cap 5 worked and improved warm throughput by about 10%, but cold variation exceeded the apparent improvement. Network sharing remained even; replica usage did not: in the second cap-5 trial, two services placed about 80% of ingress clients on one replica, versus at most about 60% with cap 10. Lightweight lab backends do not measure the production cost of concentrating application work. **Keep 10 as the production starting point.** + +Each trial started with empty networks. Nullnet daemons were reset between later trials to avoid long cleanup waits; these short comparisons do not establish natural cleanup or soak reliability. Original lab configuration/routes were restored, and final cleanup and application continuity were checked. Production configuration remains at 10. + +## Cap 5 production check — 22 September + +Repeated on `9691900` with **2,000 distinct IPs and 2,000 concurrent requests**, cap 5 on all 12 services, encryption enabled, and the normal 60-second idle timeout. One cold request per client, then 60 seconds of repeated requests from those clients: + +| Measurement | Result | +|---|---:| +| Cold requests served | 2,000 / 2,000 | +| Cold average / slowest request | 20.62 / 25.58 s | +| Warm requests served | 211,034 / 211,034 | +| Warm average / 99th percentile | 571 / 704 ms | +| Warm successful requests/second | 3,481 | + +No HTTP/2 disconnect or daemon restart occurred. The earlier 180-second stall did not recur; this does not establish its cause or prove it fixed. There were 60 ingress and 38 backend networks, with 33–34 clients per ingress network. Two services again placed 133 clients on one replica and 34 on the other: cap 5 limits network creation but does not guarantee balanced application work. + +Automatic cleanup returned graph entries, active sessions, owned interfaces, namespaces and matching IPsec entries to zero on both hosts within three minutes after traffic stopped, without restarting daemons or shortening the timeout. All 196 endpoint setups had matching completed teardowns. Conntrack notification overflows occurred, but cleanup recovered. All 24 test application processes and container routes remained intact. Original lab configuration and test routes were restored. This repeat supports a monitored production rollout with cap 5, subject to the replica imbalance and the earlier unexplained stall; it is not a long-duration soak. + +## Static-review fixes — 22 September + +Verified the working tree on `9691900` after fixing failed-operation acknowledgements, +lifecycle RPC admission, host-mapping publication locks and event-storage backpressure. +Each original failure was reproduced before its fix. Full Linux CI passed again: +gRPC, server, proxy, client, eBPF and UI checks, including the privileged failed-setup/ +teardown acknowledgement regression test. + +The final comparison with the cap-5 production check above used **2,000 distinct IPs +and 2,000 concurrent requests**, followed by 60 seconds of repeated requests. +All 12 services had `max_networks = 5` and the normal 60-second idle timeout. +The 24 replicas exercised their dependencies through `/fanout`; `/` only serves a +simple response and the earlier ingress-only trials from this review are not mesh +validation. TLS, ESP/AES-GCM and MACsec AES-256-GCM remained enabled. The load +generator and lab daemons used a 65,536-descriptor soft limit. + +| Measurement | Earlier documented cap-5 check | Fixed working tree | +|---|---:|---:| +| Cold requests served | 2,000 / 2,000 | 2,000 / 2,000 | +| Cold average / slowest request | 20.62 / 25.58 s | 17.04 / 22.96 s | +| Warm requests served | 211,034 / 211,034 | 179,912 / 179,912 | +| Warm average / 99th percentile | 571 / 704 ms | 670 / 814 ms | +| Warm successful requests/second | 3,481 | 2,968 | + +All **181,912 requests succeeded**. Warm throughput was approximately 15% below +the earlier documented trial; the user accepted this difference and declined an +additional comparison. These individual trials do not establish the cause of the +difference or statistical performance equivalence. + +`/api/graph/latency` showed 2,000 ingress clients sharing **60 ingress networks**, +plus **38 backend edges**. All **196 endpoint setups had matching completed +teardowns**. Natural cleanup returned graph edges, active sessions, owned links, +namespaces and matching IPsec state to zero on both hosts without daemon restarts. +Endpoint setup averaged 435 / 944 ms on hosts 103 / 104; teardown averaged +807 / 402 ms, with a maximum of 1,851 / 1,632 ms. Conntrack notification overflows +still occurred, but reconciliation and cleanup recovered. Replica imbalance also +remains: one service placed about 80% of its ingress clients on one replica. + +A separate five-second event-storage failure test used 100 concurrent ingress +clients. The old writer produced a 90-second request timeout; the fixed writer +served **102,939 / 102,939 requests** and persisted an explicit overflow count of +**47,228 dropped events** after storage recovered. Accepted events were retried; +new events beyond queue capacity were deliberately dropped to preserve routing. + +An aggressive five-second idle-timeout trial also exposed a host setup gap: +Debian's `ifupdown` hotplug probes accumulated on Nullnet interfaces and competed +with the client for RTNL. The stalled host had over 100 probes and over 13,000 +interfaces. Client setup now installs a udev rule for Nullnet-owned interface +names; verification confirmed the physical NIC retains its normal hotplug rules. +The ingress-only repeat with that exclusion served **38,249 / 38,249 requests**. + +Local and cross-host egress both returned HTTP 301 from the external test endpoint. +An injected XFRM deletion failure logged `VXLAN teardown FAILED` for network 131; +Events recorded `vxlan_teardown_failed`, then `net_teardown_unconfirmed` 30 seconds +later, confirming the failed cleanup was not acknowledged. Removing the fault and +restarting only that client purged the stale state; all 12 subsequent `/fanout` +requests succeeded. All 34 running containers retained their processes, Docker +addresses and routes through recovery. The lab's original configuration and test +routes were restored afterward; verified binaries remain active through runtime +service overrides. No commit, push or production deployment was performed. + +## Correctness and remaining limits + +The capped 10-minute run served **1,615,122 requests, all successful**. Each service used 10 networks with **16–17 clients each**; previously one network could hold 158 clients while the others held one. New clients now select and join the least-shared network atomically; existing sessions stay sticky. + +All 12 tested shared networks survived until their final client expired, about **60.25 seconds after its last response**. A 15-second open request also survived a 5-second idle timeout; teardown followed 5.05 seconds after its response. Each of those 12 received exactly one teardown per endpoint. Cleanup returned graph entries, active sessions, owned interfaces and namespaces to zero without restarting. + +Perf fixes control-message overload, unnecessary lock holding and database work, premature tunnel-ID reuse, and unsafe startup cleanup. Event storage remains enabled; average configuration-save time fell from 155.1 to 10.1 ms. Setup configures conntrack automatically. Restart recovery preserved all 24 application containers and their interfaces/routes; no Docker restart was required. + +Cold bursts can temporarily exceed the cap while networks are being created; this behavior is unchanged. Kernel contention and queued teardown still limit large uncapped cold bursts. Connection-tracking notification overflows still occurred, but cleanup recovered automatically. Main left 1,152 owned interfaces and 278 namespaces after its overload test; perf cleaned up automatically. + +Full Linux CI passed, including 275 server tests plus client, proxy, gRPC, eBPF and UI checks. The new concurrent test spreads 200 clients evenly across 10 networks. + +The incident included **121–151-second routing lookups on established sessions**, versus 5.6 ms for direct application responses. Main’s sustained failure was reproduced, but that exact production episode and its initiating cause remain unconfirmed. Real application/database work and frontend HTTPS were not reproduced; no production deployment was performed. diff --git a/members/nullnet-client/nullnet-networkmanager.conf b/members/nullnet-client/nullnet-networkmanager.conf new file mode 100644 index 0000000..a3704ae --- /dev/null +++ b/members/nullnet-client/nullnet-networkmanager.conf @@ -0,0 +1,2 @@ +[keyfile] +unmanaged-devices+=interface-name:br_*;interface-name:ns_*;interface-name:vxlan-ns_*;interface-name:veth-*;interface-name:macsec-* diff --git a/members/nullnet-client/nullnet-udev.rules b/members/nullnet-client/nullnet-udev.rules new file mode 100644 index 0000000..d17d0ab --- /dev/null +++ b/members/nullnet-client/nullnet-udev.rules @@ -0,0 +1,3 @@ +# Nullnet configures these transient links itself. Hotplug helpers such as +# ifupdown's ifquery otherwise contend with tunnel setup for the RTNL lock. +SUBSYSTEM=="net", KERNEL=="br_[0-9]*|ns_[0-9]*|vxlan-ns_[0-9]*|veth-[0-9]*-[sc]|macsec-[0-9]*-[sc]", RUN:="/usr/bin/true" diff --git a/members/nullnet-client/src/commands/dnat.rs b/members/nullnet-client/src/commands/dnat.rs index 46b704a..c06ee76 100644 --- a/members/nullnet-client/src/commands/dnat.rs +++ b/members/nullnet-client/src/commands/dnat.rs @@ -9,20 +9,20 @@ const PROTOS: [&str; 2] = ["tcp", "udp"]; /// inherits no stale state from a previous run. Idempotent. pub(crate) fn init() { // create our chain (no-op if it already exists) - let _ = sudo(&["iptables", "-t", "nat", "-N", CHAIN]); + let _ = privileged(&["iptables", "-t", "nat", "-N", CHAIN]); // flush any rules left over from a previous run - let _ = sudo(&["iptables", "-t", "nat", "-F", CHAIN]); + let _ = privileged(&["iptables", "-t", "nat", "-F", CHAIN]); // hook the chain from PREROUTING (idempotent via -C check). The OUTPUT // hook is gone with the NFQUEUE migration — initiators are always // containers entering the host stack via PREROUTING. - let already = sudo(&["iptables", "-t", "nat", "-C", HOOK_CHAIN, "-j", CHAIN]) + let already = privileged(&["iptables", "-t", "nat", "-C", HOOK_CHAIN, "-j", CHAIN]) .map(|s| s.success()) .unwrap_or(false); if !already { - let _ = sudo(&["iptables", "-t", "nat", "-A", HOOK_CHAIN, "-j", CHAIN]); + let _ = privileged(&["iptables", "-t", "nat", "-A", HOOK_CHAIN, "-j", CHAIN]); } // drop any conntrack flows that may have been NAT'd through stale rules - let _ = sudo(&["conntrack", "-F"]); + let _ = privileged(&["conntrack", "-F"]); println!("[dnat] init: chain {CHAIN} ready, conntrack flushed"); } @@ -73,7 +73,13 @@ fn run_iptables( "--to-destination", &target, ]); - let status = sudo(&args); + let status = privileged(&args); + if action == "-D" && !status.as_ref().is_ok_and(std::process::ExitStatus::success) { + args[3] = "-C"; + if privileged(&args).is_ok_and(|s| s.code() == Some(1)) { + return true; + } + } let src = if container_ip.is_unspecified() { "any".to_string() } else { @@ -113,7 +119,7 @@ fn run_iptables( fn flush_conntrack(port: u16, container_ip: Ipv4Addr) { for proto in PROTOS { let args = flush_conntrack_args(proto, port, container_ip); - let _ = sudo(&args.iter().map(String::as_str).collect::>()); + let _ = privileged(&args.iter().map(String::as_str).collect::>()); } } @@ -168,6 +174,10 @@ mod tests { } } -fn sudo(args: &[&str]) -> std::io::Result { - Command::new("sudo").args(args).status() +fn privileged(args: &[&str]) -> std::io::Result { + let mut command = Command::new(args[0]); + if args[0] == "iptables" { + command.arg("-w"); + } + command.args(&args[1..]).status() } diff --git a/members/nullnet-client/src/commands/egress.rs b/members/nullnet-client/src/commands/egress.rs index 7dafb08..1a49e1e 100644 --- a/members/nullnet-client/src/commands/egress.rs +++ b/members/nullnet-client/src/commands/egress.rs @@ -62,7 +62,7 @@ fn cidr_contains(cidr: &str, ip: Ipv4Addr) -> bool { (u32::from(ip) & mask) == (u32::from(base) & mask) } -fn sudo(args: &[&str]) -> std::io::Result { +fn privileged(args: &[&str]) -> std::io::Result { // For iptables, inject `-w` (wait for the xtables lock) right after the // binary so concurrent per-edge VxlanSetup tasks don't fail on lock // contention. Other commands (ip, sysctl) are passed through unchanged. @@ -71,14 +71,14 @@ fn sudo(args: &[&str]) -> std::io::Result { v.push("iptables"); v.push("-w"); v.extend_from_slice(&args[1..]); - return Command::new("sudo").args(&v).status(); + return Command::new(v[0]).args(&v[1..]).status(); } - Command::new("sudo").args(args).status() + Command::new(args[0]).args(&args[1..]).status() } /// Run a rule that must apply; log on failure. Returns success. -fn sudo_ok(label: &str, args: &[&str]) -> bool { - match sudo(args) { +fn privileged_ok(label: &str, args: &[&str]) -> bool { + match privileged(args) { Ok(s) if s.success() => true, Ok(s) => { eprintln!("[egress] {label} exited {s}"); @@ -97,7 +97,7 @@ fn sudo_ok(label: &str, args: &[&str]) -> bool { /// where unused. pub(crate) fn init() { // internal-destination ipset: create + repopulate. - sudo_ok( + privileged_ok( "ipset create internal", &[ "ipset", @@ -109,9 +109,9 @@ pub(crate) fn init() { "inet", ], ); - sudo_ok("ipset flush internal", &["ipset", "flush", INTERNAL_SET]); + privileged_ok("ipset flush internal", &["ipset", "flush", INTERNAL_SET]); for range in INTERNAL_RANGES { - sudo_ok( + privileged_ok( "ipset add internal", &["ipset", "add", "-exist", INTERNAL_SET, range], ); @@ -144,10 +144,10 @@ pub(crate) fn init() { ]; let mut del = vec!["iptables", "-D", "PREROUTING"]; del.extend_from_slice(&rule); - let _ = sudo(&del); + let _ = privileged(&del); let mut add = vec!["iptables", "-A", "PREROUTING"]; add.extend_from_slice(&rule); - sudo_ok( + privileged_ok( &format!("iptables -A PREROUTING egress NFQUEUE {proto}"), &add, ); @@ -157,7 +157,7 @@ pub(crate) fn init() { // can be routed from the overlay bridge out the real NIC. Harmless on // non-gateway nodes. Per-edge MASQUERADE is added on demand (see // `install_gateway_forward`). - sudo_ok( + privileged_ok( "sysctl ip_forward", &["sysctl", "-w", "net.ipv4.ip_forward=1"], ); @@ -216,7 +216,9 @@ pub(crate) fn install_steer( return false; } - remove_steer(net_id, br_dev, snat_src, container_ip); + if !remove_steer(net_id, br_dev, snat_src, container_ip) { + return false; + } let table = table_for(net_id).to_string(); let base = prio_base(net_id); @@ -227,7 +229,7 @@ pub(crate) fn install_steer( // service-to-service and control-plane routing is unchanged. for (i, range) in INTERNAL_RANGES.iter().enumerate() { let prio = (base + i as u32).to_string(); - ok &= sudo_ok( + ok &= privileged_ok( "ip rule add internal bypass", &[ "ip", "rule", "add", "from", &cip, "to", range, "lookup", "main", "priority", &prio, @@ -236,14 +238,14 @@ pub(crate) fn install_steer( } // Catch-all: everything else from this container → the egress table. let catch_all = (base + 15).to_string(); - ok &= sudo_ok( + ok &= privileged_ok( "ip rule add egress catch-all", &[ "ip", "rule", "add", "from", &cip, "lookup", &table, "priority", &catch_all, ], ); // The egress table's only route: default via the proxy overlay IP. - ok &= sudo_ok( + ok &= privileged_ok( "ip route add egress default", &[ "ip", @@ -260,7 +262,7 @@ pub(crate) fn install_steer( ); // SNAT the container's traffic leaving the overlay bridge to the overlay // source, so the proxy routes replies back through the tunnel. - ok &= sudo_ok( + ok &= privileged_ok( "iptables SNAT egress", &[ "iptables", @@ -283,27 +285,47 @@ pub(crate) fn install_steer( "[egress] steer net {net_id}: {cip} -> via {proxy_gw} dev {br_dev} (snat {snat_src})" ); } else { - // Partial install (e.g. a transient failure mid-sequence): roll back - // whatever applied so we don't leak policy rules / SNAT for a net that - // may never be reused. Teardown otherwise keys on EgressState, which the - // caller only records on success. + // Roll back partial installation now; recorded state also lets the + // eventual teardown retry any cleanup that fails here. eprintln!("[egress] steer net {net_id} partial install; rolling back"); remove_steer(net_id, br_dev, snat_src, container_ip); } ok } -/// Reverse of `install_steer`. Best-effort; ignores rules that don't exist. -pub(crate) fn remove_steer(net_id: u32, br_dev: &str, snat_src: Ipv4Addr, container_ip: Ipv4Addr) { +/// Reverse steering, accepting already-absent rules but reporting cleanup failures. +pub(crate) fn remove_steer( + net_id: u32, + br_dev: &str, + snat_src: Ipv4Addr, + container_ip: Ipv4Addr, +) -> bool { let table = table_for(net_id).to_string(); let base = prio_base(net_id); let cip = container_ip.to_string(); - for i in 0..16u32 { - let prio = (base + i).to_string(); - let _ = sudo(&["ip", "rule", "del", "priority", &prio]); + let Some(rules) = privileged_output(&["ip", "rule", "show"]) else { + return false; + }; + let mut ok = true; + for priority in rules + .lines() + .filter_map(|line| line.split_once(':')?.0.trim().parse::().ok()) + { + if (base..base + 16).contains(&priority) { + ok &= privileged_ok( + "remove egress rule", + &["ip", "rule", "del", "priority", &priority.to_string()], + ); + } } - let _ = sudo(&["ip", "route", "flush", "table", &table]); - let _ = sudo(&[ + ok &= Command::new("ip") + .args(["route", "flush", "table", &table]) + .output() + .is_ok_and(|out| { + out.status.success() + || String::from_utf8_lossy(&out.stderr).contains("FIB table does not exist") + }); + ok &= remove_iptables_rule(&[ "iptables", "-t", "nat", @@ -318,6 +340,7 @@ pub(crate) fn remove_steer(net_id: u32, br_dev: &str, snat_src: Ipv4Addr, contai "--to-source", &snat_src.to_string(), ]); + ok } /// Drop policy-routing state left behind by a previous run. @@ -341,9 +364,9 @@ pub(crate) fn purge_stale_steers() { // Most of the band is legitimately absent — a steer only ever uses // the internal bypasses plus the catch-all — so the misses are the // expected case, not something to spell out on every startup. - let _ = sudo_quiet(&["ip", "rule", "del", "priority", &(base + i).to_string()]); + let _ = privileged_quiet(&["ip", "rule", "del", "priority", &(base + i).to_string()]); } - let _ = sudo(&["ip", "route", "flush", "table", &table]); + let _ = privileged(&["ip", "route", "flush", "table", &table]); tables += 1; } @@ -351,7 +374,7 @@ pub(crate) fn purge_stale_steers() { for spec in stale_overlay_snat_rules() { let mut args: Vec<&str> = vec!["iptables", "-t", "nat"]; args.extend(spec.iter().map(String::as_str)); - if sudo(&args).map(|s| s.success()).unwrap_or(false) { + if privileged(&args).map(|s| s.success()).unwrap_or(false) { snats += 1; } } @@ -363,7 +386,7 @@ pub(crate) fn purge_stale_steers() { /// internal-bypass rules use `lookup main`, so the catch-all is what identifies /// the band — a partial install missing it is left for `install_steer` to heal. fn stale_steer_net_ids() -> Vec { - parse_steer_net_ids(&sudo_output(&["ip", "rule", "show"]).unwrap_or_default()) + parse_steer_net_ids(&privileged_output(&["ip", "rule", "show"]).unwrap_or_default()) } fn parse_steer_net_ids(out: &str) -> Vec { @@ -389,7 +412,7 @@ fn parse_steer_net_ids(out: &str) -> Vec { /// verb yields the matching delete. fn stale_overlay_snat_rules() -> Vec> { parse_overlay_snat_rules( - &sudo_output(&["iptables", "-t", "nat", "-S", "POSTROUTING"]).unwrap_or_default(), + &privileged_output(&["iptables", "-t", "nat", "-S", "POSTROUTING"]).unwrap_or_default(), ) } @@ -411,17 +434,20 @@ fn parse_overlay_snat_rules(out: &str) -> Vec> { .collect() } -fn sudo_output(args: &[&str]) -> Option { - let out = Command::new("sudo").args(args).output().ok()?; +fn privileged_output(args: &[&str]) -> Option { + let out = Command::new(args[0]).args(&args[1..]).output().ok()?; out.status .success() .then(|| String::from_utf8_lossy(&out.stdout).into_owned()) } -/// `sudo` with stderr captured rather than inherited, for calls whose failure +/// Privileged command with stderr captured rather than inherited, for calls whose failure /// is the expected steady state (deleting a rule that isn't there). -fn sudo_quiet(args: &[&str]) -> std::io::Result { - Command::new("sudo").args(args).output().map(|o| o.status) +fn privileged_quiet(args: &[&str]) -> std::io::Result { + Command::new(args[0]) + .args(&args[1..]) + .output() + .map(|o| o.status) } /// Gateway side: forward decapsulated external-bound packets arriving on @@ -429,7 +455,9 @@ fn sudo_quiet(args: &[&str]) -> std::io::Result { /// to that NIC's address (MASQUERADE). netfilter conntrack de-SNATs the replies /// back over the tunnel. Replaces the old TPROXY-to-forward-proxy path. pub(crate) fn install_gateway_forward(br_dev: &str, br_net: &str) -> bool { - remove_gateway_forward(br_dev, br_net); + if !remove_gateway_forward(br_dev, br_net) { + return false; + } let Some(nic) = default_route_iface() else { eprintln!("[egress] gateway forward: no default-route NIC found; not installed"); return false; @@ -441,11 +469,26 @@ pub(crate) fn install_gateway_forward(br_dev: &str, br_net: &str) -> bool { ok } -/// Reverse of `install_gateway_forward`. Best-effort; recomputes the NIC. -pub(crate) fn remove_gateway_forward(br_dev: &str, br_net: &str) { - if let Some(nic) = default_route_iface() { - apply_forward_rules(&forward_rules(Some(br_dev), br_net, Some(&nic)), "-D"); - } +/// Discover the installed NIC rather than assuming the default route is unchanged. +pub(crate) fn remove_gateway_forward(br_dev: &str, br_net: &str) -> bool { + let Ok(network) = br_net.parse::() else { + return false; + }; + let subnet = format!("{}/{}", network.network(), network.prefix()); + remove_forward_rules(|output| { + output + .lines() + .filter_map(|line| { + let parts: Vec<_> = line.split_whitespace().collect(); + let owns = parts.windows(2).any(|p| { + (p[0] == "-s" && p[1] == subnet) + || (matches!(p[0], "-i" | "-o") && p[1] == br_dev) + }); + (parts.first() == Some(&"-A") && owns) + .then(|| parts.into_iter().skip(1).map(String::from).collect()) + }) + .collect() + }) } type ForwardRules = Vec<(&'static str, Vec)>; @@ -490,11 +533,25 @@ fn apply_forward_rules(rules: &ForwardRules, operation: &str) -> bool { for (table, rule) in rules { let mut args = vec!["iptables", "-t", table, operation]; args.extend(rule.iter().map(String::as_str)); - ok &= sudo_ok("gateway forwarding rule", &args); + ok &= if operation == "-D" { + remove_iptables_rule(&args) + } else { + privileged_ok("gateway forwarding rule", &args) + }; } ok } +// A failed delete is benign only if a successful check confirms absence. +fn remove_iptables_rule(args: &[&str]) -> bool { + if privileged(args).is_ok_and(|status| status.success()) { + return true; + } + let mut check = args.to_vec(); + check[3] = "-C"; + privileged(&check).is_ok_and(|status| status.code() == Some(1)) +} + const LOCAL_FORWARD_COMMENT: &str = "nullnet-local-egress-"; /// Same-host egress needs one NAT pass, scoped to the initiator's address. @@ -524,17 +581,21 @@ pub(crate) fn install_local_forward(net_id: u32, container_ips: &[Ipv4Addr]) -> /// Discover exact installed rules so cleanup also survives a NIC change/restart. pub(crate) fn remove_local_forwards(net_id: Option) -> bool { + remove_forward_rules(|output| parse_local_forward_rules(output, net_id)) +} + +fn remove_forward_rules(select: impl Fn(&str) -> Vec>) -> bool { let mut ok = true; for (table, chain) in [("nat", "POSTROUTING"), ("filter", "FORWARD")] { - let Some(output) = sudo_output(&["iptables", "-t", table, "-S", chain]) else { - eprintln!("[egress] could not inspect {table}/{chain} for local gateway cleanup"); + let Some(output) = privileged_output(&["iptables", "-t", table, "-S", chain]) else { + eprintln!("[egress] could not inspect {table}/{chain} for gateway cleanup"); ok = false; continue; }; - for rule in parse_local_forward_rules(&output, net_id) { + for rule in select(&output) { let mut args = vec!["iptables", "-t", table, "-D"]; args.extend(rule.iter().map(String::as_str)); - ok &= sudo_ok("local gateway cleanup", &args); + ok &= privileged_ok("gateway cleanup", &args); } } ok diff --git a/members/nullnet-client/src/commands/mod.rs b/members/nullnet-client/src/commands/mod.rs index 6e31654..f67d94c 100644 --- a/members/nullnet-client/src/commands/mod.rs +++ b/members/nullnet-client/src/commands/mod.rs @@ -11,6 +11,7 @@ mod netlink; pub(crate) mod nfqueue; mod ovs; pub(crate) mod vxlan; +mod vxlan_cleanup; pub(crate) async fn setup_br0(rtnetlink_handle: &RtNetLinkHandle) { // create the bridge @@ -112,6 +113,13 @@ pub(crate) struct RtNetLinkHandle { } impl RtNetLinkHandle { + #[cfg(test)] + pub(crate) fn disconnected() -> Self { + let (connection, handle, _) = new_connection().unwrap(); + drop(connection); + Self { handle } + } + pub(crate) fn new() -> Result { let (rtnetlink_conn, rtnetlink_handle, _) = new_connection().handle_err(location!())?; tokio::spawn(rtnetlink_conn); @@ -160,15 +168,18 @@ const XFRM_SPI_MAX: u32 = 1_000 + 2_097_151; /// the *old* key — both ends then disagree and the tunnel black-holes. Scoped /// by SPI so unrelated IPsec on the host is left alone. fn purge_stale_xfrm() { - let states_out = sudo_output(&["ip", "xfrm", "state", "show"]).unwrap_or_default(); - let policies_out = sudo_output(&["ip", "xfrm", "policy", "show"]).unwrap_or_default(); + let states_out = privileged_output(&["ip", "xfrm", "state", "show"]).unwrap_or_default(); + let policies_out = privileged_output(&["ip", "xfrm", "policy", "show"]).unwrap_or_default(); let mut states = 0usize; for (src, dst, spi) in parse_xfrm_states(&states_out) { let args = [ "ip", "xfrm", "state", "delete", "src", &src, "dst", &dst, "proto", "esp", "spi", &spi, ]; - if sudo_quiet(&args).map(|s| s.success()).unwrap_or(false) { + if privileged_quiet(&args) + .map(|s| s.success()) + .unwrap_or(false) + { states += 1; } } @@ -178,7 +189,10 @@ fn purge_stale_xfrm() { let mut args: Vec<&str> = vec!["ip", "xfrm", "policy", "delete"]; args.extend(selector.iter().map(String::as_str)); args.extend_from_slice(&["dir", &dir]); - if sudo_quiet(&args).map(|s| s.success()).unwrap_or(false) { + if privileged_quiet(&args) + .map(|s| s.success()) + .unwrap_or(false) + { policies += 1; } } @@ -246,9 +260,9 @@ fn parse_our_spi(line: &str) -> Option { .then(|| raw.to_string()) } -fn sudo_output(args: &[&str]) -> Option { - let out = std::process::Command::new("sudo") - .args(args) +fn privileged_output(args: &[&str]) -> Option { + let out = std::process::Command::new(args[0]) + .args(&args[1..]) .output() .ok()?; out.status @@ -260,6 +274,16 @@ fn sudo_output(args: &[&str]) -> Option { mod xfrm_tests { use super::{parse_xfrm_policies, parse_xfrm_states}; + #[test] + fn orphan_namespace_cleanup_is_scoped_to_nullnet_names() { + for name in ["ns_101_s", "ns_101_c"] { + assert!(super::is_nullnet_namespace(name)); + } + for name in ["ns_custom", "ns_101_backup", "ns_01_c", "ns_+1_c", "ns__s"] { + assert!(!super::is_nullnet_namespace(name), "{name}"); + } + } + /// One of ours (net id 101 → spi 0x0000046d = 1101) and one unrelated SA /// whose SPI sits outside the pool range. const STATE_SHOW: &str = "\ @@ -375,12 +399,12 @@ fn install_mss_clamp() -> Option { ]; let mut check = vec!["iptables", "-t", "mangle", "-C", "FORWARD"]; check.extend_from_slice(&rule); - if sudo(&check).map(|s| s.success()).unwrap_or(false) { + if privileged(&check).map(|s| s.success()).unwrap_or(false) { return None; } let mut add = vec!["iptables", "-t", "mangle", "-A", "FORWARD"]; add.extend_from_slice(&rule); - match sudo(&add) { + match privileged(&add) { Ok(s) if s.success() => { println!("[mss] clamp installed on mangle/FORWARD: --set-mss {MSS}"); None @@ -438,7 +462,7 @@ fn prune_superseded_mss_rules() { for _ in 0..8 { let mut del = vec!["iptables", "-t", "mangle", "-D", "FORWARD"]; del.extend_from_slice(spec); - match sudo_quiet(&del) { + match privileged_quiet(&del) { Ok(s) if s.success() => { println!("[mss] removed superseded clamp: {}", spec.join(" ")); } @@ -448,16 +472,18 @@ fn prune_superseded_mss_rules() { } } -fn sudo(args: &[&str]) -> std::io::Result { - std::process::Command::new("sudo").args(args).status() +fn privileged(args: &[&str]) -> std::io::Result { + std::process::Command::new(args[0]) + .args(&args[1..]) + .status() } -/// `sudo` with stdout/stderr captured rather than inherited. For calls whose +/// Privileged command with stdout/stderr captured rather than inherited. For calls whose /// failure is the expected steady state — deleting a rule that isn't there — /// so iptables' "Bad rule" complaint doesn't reach the log on every startup. -fn sudo_quiet(args: &[&str]) -> std::io::Result { - std::process::Command::new("sudo") - .args(args) +fn privileged_quiet(args: &[&str]) -> std::io::Result { + std::process::Command::new(args[0]) + .args(&args[1..]) .output() .map(|o| o.status) } @@ -487,10 +513,10 @@ fn vxlan_cleanup_network() { .map(|mut c| c.wait()) .handle_err(location!()); } - } else if device.name.starts_with("veth-") { + } else if netlink::is_nullnet_veth(&device.name) { println!("Cleaning up existing same-host veth pair: {}", device.name); - let _ = std::process::Command::new("sudo") - .args(["ip", "link", "del", &device.name]) + let _ = std::process::Command::new("ip") + .args(["link", "del", &device.name]) .spawn() .map(|mut c| c.wait()) .handle_err(location!()); @@ -512,6 +538,34 @@ fn vxlan_cleanup_network() { } } } + + // Interrupted setup can leave a namespace with no host-side interface. + if let Ok(namespaces) = std::fs::read_dir("/var/run/netns") { + for entry in namespaces.flatten() { + let name = entry.file_name(); + if let Some(name) = name.to_str() + && is_nullnet_namespace(name) + { + let _ = std::process::Command::new("ip") + .args(["netns", "del", name]) + .status() + .handle_err(location!()); + } + } + } +} + +fn is_nullnet_namespace(name: &str) -> bool { + let Some(suffix) = name.strip_prefix("ns_") else { + return false; + }; + let Some(id) = suffix + .strip_suffix("_s") + .or_else(|| suffix.strip_suffix("_c")) + else { + return false; + }; + id.parse::().is_ok_and(|value| value.to_string() == id) } /// Cleanup existing veth and VLANs diff --git a/members/nullnet-client/src/commands/netlink.rs b/members/nullnet-client/src/commands/netlink.rs index 5a623f9..7f9346f 100644 --- a/members/nullnet-client/src/commands/netlink.rs +++ b/members/nullnet-client/src/commands/netlink.rs @@ -99,7 +99,7 @@ async fn delete_all_veths(handle: &Handle) { if let Ok(link) = link_res && link.attributes.iter().any(|attr| { if let LinkAttribute::IfName(name) = attr - && name.starts_with("veth") + && is_nullnet_veth(name) { true } else { @@ -112,6 +112,18 @@ async fn delete_all_veths(handle: &Handle) { } } +pub(super) fn is_nullnet_veth(name: &str) -> bool { + let Some(suffix) = name.strip_prefix("veth-") else { + return false; + }; + let id = suffix + .strip_suffix("-s") + .or_else(|| suffix.strip_suffix("-c")) + .or_else(|| suffix.strip_suffix('p')) + .unwrap_or(suffix); + id.parse::().is_ok_and(|value| value.to_string() == id) +} + async fn delete_veth(handle: &Handle, vlan_id: u16) { let mut links = handle.link().get().execute(); while let Some(link_res) = links.next().await { @@ -252,3 +264,26 @@ pub(super) async fn delete_link(handle: &Handle, link: LinkMessage) -> Result<() Ok(()) } + +#[cfg(test)] +mod ownership_tests { + use super::is_nullnet_veth; + + #[test] + fn startup_preserves_docker_and_unrelated_veths() { + for name in [ + "veth0a3f12", + "veth123456", + "veth-custom", + "veth-12-backup", + "veth-01", + "veth-+1", + "veth-", + ] { + assert!(!is_nullnet_veth(name), "{name}"); + } + for name in ["veth-101", "veth-101p", "veth-101-s", "veth-101-c"] { + assert!(is_nullnet_veth(name), "{name}"); + } + } +} diff --git a/members/nullnet-client/src/commands/nfqueue.rs b/members/nullnet-client/src/commands/nfqueue.rs index 6dcfee9..809a2e9 100644 --- a/members/nullnet-client/src/commands/nfqueue.rs +++ b/members/nullnet-client/src/commands/nfqueue.rs @@ -22,7 +22,7 @@ pub(crate) fn init() { // ipset: create if missing, flush so we start with no ports. Both must // succeed — without them the iptables rules below match an empty/missing // set and queue nothing. - sudo_must( + privileged_must( "ipset create", &[ "ipset", @@ -34,11 +34,11 @@ pub(crate) fn init() { "0-65535", ], ); - sudo_must("ipset flush", &["ipset", "flush", SET_NAME]); + privileged_must("ipset flush", &["ipset", "flush", SET_NAME]); // ESTABLISHED,RELATED bypass at top of mangle PREROUTING. Pre-delete is // intentional idempotency — silently ignore its result. - let _ = sudo(&[ + let _ = privileged(&[ "iptables", "-t", "mangle", @@ -51,7 +51,7 @@ pub(crate) fn init() { "-j", "ACCEPT", ]); - sudo_must( + privileged_must( "iptables -I PREROUTING ESTABLISHED bypass", &[ "iptables", @@ -72,7 +72,7 @@ pub(crate) fn init() { // NFQUEUE rules for tcp + udp. Pre-deletes stay quiet; appends must // succeed or new flows on watched ports won't be queued. for proto in PROTOS { - let _ = sudo(&[ + let _ = privileged(&[ "iptables", "-t", "mangle", @@ -91,7 +91,7 @@ pub(crate) fn init() { QUEUE_NUM, "--queue-bypass", ]); - sudo_must( + privileged_must( &format!("iptables -A PREROUTING NFQUEUE {proto}"), &[ "iptables", @@ -118,11 +118,11 @@ pub(crate) fn init() { println!("[nfqueue] init: ipset {SET_NAME} ready, mangle PREROUTING rules installed"); } -/// Run a `sudo` command that must succeed for the NFQUEUE plumbing to be +/// Run a privileged command that must succeed for the NFQUEUE plumbing to be /// usable, logging any failure. Used for ipset create/flush and iptables /// add operations; pre-delete idempotency calls bypass this and stay quiet. -fn sudo_must(label: &str, args: &[&str]) { - match sudo(args) { +fn privileged_must(label: &str, args: &[&str]) { + match privileged(args) { Ok(s) if s.success() => {} Ok(s) => eprintln!("[nfqueue] init: {label} exited {s}"), Err(e) => eprintln!("[nfqueue] init: {label}: {e}"), @@ -136,7 +136,7 @@ pub(crate) fn apply_ports_diff(old: &HashSet, new: &HashSet) { for &port in old { if !new.contains(&port) { let port_s = port.to_string(); - match sudo(&["ipset", "del", SET_NAME, &port_s]) { + match privileged(&["ipset", "del", SET_NAME, &port_s]) { Ok(s) if s.success() => println!("[nfqueue] unwatched port {port}"), Ok(s) => eprintln!("[nfqueue] ipset del {port} exited {s}"), Err(e) => eprintln!("[nfqueue] ipset del {port}: {e}"), @@ -146,7 +146,7 @@ pub(crate) fn apply_ports_diff(old: &HashSet, new: &HashSet) { for &port in new { if !old.contains(&port) { let port_s = port.to_string(); - match sudo(&["ipset", "add", "-exist", SET_NAME, &port_s]) { + match privileged(&["ipset", "add", "-exist", SET_NAME, &port_s]) { Ok(s) if s.success() => println!("[nfqueue] watching port {port}"), Ok(s) => eprintln!("[nfqueue] ipset add {port} exited {s}"), Err(e) => eprintln!("[nfqueue] ipset add {port}: {e}"), @@ -155,6 +155,6 @@ pub(crate) fn apply_ports_diff(old: &HashSet, new: &HashSet) { } } -fn sudo(args: &[&str]) -> std::io::Result { - Command::new("sudo").args(args).status() +fn privileged(args: &[&str]) -> std::io::Result { + Command::new(args[0]).args(&args[1..]).status() } diff --git a/members/nullnet-client/src/commands/vxlan.rs b/members/nullnet-client/src/commands/vxlan.rs index 2678527..b7a973f 100644 --- a/members/nullnet-client/src/commands/vxlan.rs +++ b/members/nullnet-client/src/commands/vxlan.rs @@ -24,9 +24,9 @@ use std::collections::HashMap; use std::fs::File; use std::net::{IpAddr, Ipv4Addr}; use std::os::fd::AsRawFd; -use std::process::Command; use std::sync::{Arc, LazyLock, Mutex as StdMutex, PoisonError}; -use tokio::sync::{Mutex as AsyncMutex, OwnedMutexGuard}; +use tokio::process::Command; +use tokio::sync::{Mutex as AsyncMutex, OwnedMutexGuard, Semaphore}; /// Matches `vxlan-setup.sh`'s `OVERLAY_MTU` — see that script's comment for /// how the 1080-byte ceiling was measured against this underlay. @@ -37,6 +37,12 @@ const OVERLAY_MTU: u32 = 1080; /// can still carry a full `OVERLAY_MTU`-sized frame. const MACSEC_VETH_MTU: u32 = OVERLAY_MTU + 32; +// Reserved link groups batch one tunnel side's deletion in a single RTNL +// operation. Net ids use 21 bits; the low bit distinguishes the two sides. +fn link_group(vxlan_id: u32, client_side: bool) -> u32 { + 0x4e00_0000 | (vxlan_id << 1) | u32::from(client_side) +} + pub(crate) struct VxlanSetupParams { pub(crate) vxlan_id: u32, pub(crate) ns_name: String, @@ -56,7 +62,6 @@ pub(crate) struct VxlanTeardownParams { pub(crate) ns_name: String, pub(crate) br_name: String, pub(crate) dstport: u16, - pub(crate) docker_container: Option, } /// Per-net-id serialization: setup runs once per side (`_s`/`_c`) of the same @@ -69,6 +74,8 @@ pub(crate) struct VxlanTeardownParams { static VXLAN_LOCKS: LazyLock>>>> = LazyLock::new(|| StdMutex::new(HashMap::new())); +static COMMAND_SLOTS: Semaphore = Semaphore::const_new(8); + async fn lock(vxlan_id: u32) -> OwnedMutexGuard<()> { let entry = VXLAN_LOCKS .lock() @@ -85,6 +92,7 @@ pub(crate) async fn setup( cache: &BridgeIpCache, ) -> Result<(), Error> { let _guard = lock(params.vxlan_id).await; + let _setup = super::vxlan_cleanup::track_setup(); // Publish ownership before the interface can carry its first packet. if let Some(container) = ¶ms.docker_container { cache.add_overlay(¶ms.ns_name, params.ns_net.ip(), container); @@ -101,81 +109,58 @@ async fn setup_locked( params: &VxlanSetupParams, ) -> Result<(), Error> { let handle = &rtnetlink_handle.handle; + let group = link_group(params.vxlan_id, params.br_name.ends_with("_c")); // Namespace: join the docker container's, or create a fresh standalone // one. Creating/joining a namespace isn't an rtnetlink (RTM_*) operation // at all — it's `unshare(CLONE_NEWNET)` plus a bind mount — so this stays // a CLI call. let ns_pid = match ¶ms.docker_container { - Some(container) => Some(docker_pid(container)?), + Some(container) => Some(docker_pid(container).await?), None => { // Idempotent, like the script: a leftover namespace from a // previous run is reused rather than treated as fatal. - let _ = Command::new("sudo") - .args(["ip", "netns", "add", ¶ms.ns_name]) - .status(); + let _ = + command_status(Command::new("ip").args(["netns", "add", ¶ms.ns_name])).await; None } }; - // Veth pair connecting the namespace (`-in`) to the bridge (`-out`). + // Create the peer in its final namespace to avoid a second registration + // and the kernel synchronization required by moving an existing device. let veth_in = format!("{}-in", params.ns_name); let veth_out = format!("{}-out", params.ns_name); - delete_if_exists(handle, &veth_in).await?; + delete_if_exists(handle, &veth_out).await?; + let peer = LinkUnspec::new_with_name(&veth_in).mtu(OVERLAY_MTU); + let (peer, netns_file) = match ns_pid { + Some(pid) => (peer.setns_by_pid(pid).build(), None), + None => { + let file = + File::open(format!("/var/run/netns/{}", params.ns_name)).handle_err(location!())?; + (peer.setns_by_fd(file.as_raw_fd()).build(), Some(file)) + } + }; handle .link() - .add(LinkVeth::new(&veth_in, &veth_out).build()) + .add( + LinkVeth::new(&veth_out, &veth_in) + .mtu(OVERLAY_MTU) + .link_group(group) + .set_info_data(InfoData::Veth(InfoVeth::Peer(peer))) + .build(), + ) .execute() .await .handle_err(location!())?; - let link_in = get_link_by_name(handle, &veth_in).await?; - - // Move the namespace end into its target namespace. This is a normal - // RTM_NEWLINK from the root namespace (IFLA_NET_NS_PID/FD) — no need to - // actually enter the target namespace ourselves. - match ns_pid { - Some(pid) => { - let setns_req = LinkUnspec::new_with_index(link_in.header.index) - .setns_by_pid(pid) - .build(); - handle - .link() - .set(setns_req) - .execute() - .await - .handle_err(location!())?; - } - None => { - // Kept alive across the `.execute().await` below: it's only the - // raw fd *number* that goes into the netlink message, so if the - // `File` were dropped (closing the fd) before the message is - // actually sent, the kernel would read a closed/reused fd — - // "Bad file descriptor" (EBADF) — instead of the namespace. - let netns_file = - File::open(format!("/var/run/netns/{}", params.ns_name)).handle_err(location!())?; - let setns_req = LinkUnspec::new_with_index(link_in.header.index) - .setns_by_fd(netns_file.as_raw_fd()) - .build(); - handle - .link() - .set(setns_req) - .execute() - .await - .handle_err(location!())?; - } - } + drop(netns_file); - // Once moved, the veth's ifindex only exists inside the target - // namespace's own link table — configuring it needs a netlink socket - // bound to that namespace, which rtnetlink has no way to reach from - // here. Stays a couple of `ip netns exec`/`nsenter` calls. - configure_ns_in(params, ns_pid)?; + configure_ns_in(params, ns_pid).await?; // Bridge, with its own address, carrying the namespace's traffic. delete_if_exists(handle, ¶ms.br_name).await?; handle .link() - .add(LinkBridge::new(¶ms.br_name).build()) + .add(LinkBridge::new(¶ms.br_name).link_group(group).build()) .execute() .await .handle_err(location!())?; @@ -203,12 +188,8 @@ async fn setup_locked( } // Enable forwarding (Docker sets FORWARD policy to DROP). - let _ = Command::new("sudo") - .args(["sysctl", "-w", "net.ipv4.ip_forward=1"]) - .status(); - let _ = Command::new("sudo") - .args(["iptables", "-P", "FORWARD", "ACCEPT"]) - .status(); + let _ = command_status(Command::new("sysctl").args(["-w", "net.ipv4.ip_forward=1"])).await; + let _ = command_status(Command::new("iptables").args(["-P", "FORWARD", "ACCEPT"])).await; Ok(()) } @@ -226,7 +207,7 @@ async fn setup_same_host( // Drop artifacts left by a previous cross-host incarnation of this net // id — an edge switches branch when its peer relocates onto this host. delete_if_exists(handle, &format!("vxlan-{}", params.ns_name)).await?; - purge_xfrm_spi(params.vxlan_id); + purge_xfrm_spi(params.vxlan_id).await; let veth_s = format!("veth-{}-s", params.vxlan_id); let veth_c = format!("veth-{}-c", params.vxlan_id); @@ -248,11 +229,13 @@ async fn setup_same_host( .add( LinkVeth::new(&veth_s, &veth_c) .address(mac_s.clone()) + .link_group(link_group(params.vxlan_id, false)) // Set both MACs at creation so udev never sees a random peer // address and races us with MACAddressPolicy=persistent. .set_info_data(InfoData::Veth(InfoVeth::Peer( LinkUnspec::new_with_name(&veth_c) .address(mac_c.clone()) + .link_group(link_group(params.vxlan_id, true)) .build(), ))) .build(), @@ -282,7 +265,7 @@ async fn setup_same_host( // rtnetlink's `.port(1)` builds an SCI on port 256 while the `ip macsec // rx port 1` calls below key on port 1 — no frame ever matches. // `port` must precede `cipher`: iproute2's parser is positional here. - sudo_checked(&[ + privileged_checked(&[ "ip", "link", "add", @@ -297,14 +280,15 @@ async fn setup_same_host( "gcm-aes-256", "encrypt", "on", - ])?; + ]) + .await?; // SA/key installation is a separate genl family ("macsec"), not // covered by rtnetlink — stays the same `ip macsec` calls the script // used. Left unsuppressed (a real failure here should be loud). let key_id = format!("{:032x}", params.vxlan_id); let peer_mac_str = format_mac(&peer_mac); - sudo_checked(&[ + privileged_checked(&[ "ip", "macsec", "add", @@ -318,8 +302,9 @@ async fn setup_same_host( "key", &key_id, ¶ms.key_hex, - ])?; - sudo_checked(&[ + ]) + .await?; + privileged_checked(&[ "ip", "macsec", "add", @@ -330,8 +315,9 @@ async fn setup_same_host( "address", &peer_mac_str, "on", - ])?; - sudo_checked(&[ + ]) + .await?; + privileged_checked(&[ "ip", "macsec", "add", @@ -349,7 +335,8 @@ async fn setup_same_host( "key", &key_id, ¶ms.key_hex, - ])?; + ]) + .await?; let macsec_link = get_link_by_name(handle, &macsec_if).await?; attach_and_size(handle, &macsec_link, br_index, OVERLAY_MTU).await?; @@ -381,6 +368,7 @@ async fn setup_cross_host( .link() .add( LinkVxlan::new(&vxlan_name, params.vxlan_id) + .link_group(link_group(params.vxlan_id, params.br_name.ends_with("_c"))) .local(params.local_ip) .remote(params.remote_ip) .port(params.dstport) @@ -393,7 +381,7 @@ async fn setup_cross_host( attach_and_size(handle, &vxlan_link, br_index, OVERLAY_MTU).await?; if params.encrypted { - install_xfrm(params)?; + install_xfrm(params).await?; } Ok(()) @@ -404,7 +392,7 @@ pub(crate) async fn teardown( params: &VxlanTeardownParams, cache: &BridgeIpCache, ) -> Result<(), Error> { - let _guard = lock(params.vxlan_id).await; + let guard = Arc::new(lock(params.vxlan_id).await); let handle = &rtnetlink_handle.handle; // This tunnel's XFRM state/policy pair, if any was installed. Matched on @@ -415,7 +403,7 @@ pub(crate) async fn teardown( let spi = xfrm_spi(params.vxlan_id); if params.dstport != crate::DEFAULT_VXLAN_DSTPORT { let dstport = params.dstport.to_string(); - let _ = sudo_quiet(&[ + privileged_checked(&[ "ip", "xfrm", "policy", @@ -426,8 +414,9 @@ pub(crate) async fn teardown( &dstport, "dir", "out", - ]); - let _ = sudo_quiet(&[ + ]) + .await?; + privileged_checked(&[ "ip", "xfrm", "policy", @@ -438,9 +427,10 @@ pub(crate) async fn teardown( &dstport, "dir", "in", - ]); + ]) + .await?; } - let _ = sudo_quiet(&[ + privileged_checked(&[ "ip", "xfrm", "state", @@ -449,36 +439,53 @@ pub(crate) async fn teardown( "esp", "spi", &spi, - ]); - - // Remove the VXLAN tunnel or same-host veth/macsec pair. Both are swept - // unconditionally — whichever branch setup took, only one set exists, - // and the other's absence is expected rather than an error. - delete_if_exists(handle, &format!("macsec-{}-s", params.vxlan_id)).await?; - delete_if_exists(handle, &format!("macsec-{}-c", params.vxlan_id)).await?; - delete_if_exists(handle, &format!("vxlan-{}", params.ns_name)).await?; - delete_if_exists(handle, &format!("veth-{}-s", params.vxlan_id)).await?; + ]) + .await?; + + // Parent deletion also removes veth peers and their MACsec children. + // Side-specific groups preserve the other side's bridge until its cleanup. + super::vxlan_cleanup::remove( + handle, + link_group(params.vxlan_id, params.br_name.ends_with("_c")), + guard.clone(), + ) + .await + .handle_err(location!())?; - // Remove the namespace veth pair. - delete_if_exists(handle, &format!("{}-out", params.ns_name)).await?; cache.remove_overlay(¶ms.ns_name); - if params.docker_container.is_none() { - // Standalone mode: delete the namespace we created (its `-in` end - // goes with it). Docker mode: nothing to do, Docker manages its own - // namespace. - let _ = Command::new("sudo") - .args(["ip", "netns", "del", ¶ms.ns_name]) - .status(); + // Egress steering creates a named namespace even for a Docker initiator. + // Delete the namespace we actually own, independent of the message's owner. + if std::path::Path::new("/var/run/netns") + .join(¶ms.ns_name) + .exists() + { + privileged_checked(&["ip", "netns", "del", ¶ms.ns_name]).await?; } - delete_if_exists(handle, ¶ms.br_name).await?; - Ok(()) } // helpers ----------------------------------------------------------------------------------------- +// Commands may wait on Docker or the kernel; keep that work off runtime +// workers and cap child processes independently of the number of tunnel tasks. +async fn command_status(command: &mut Command) -> std::io::Result { + let _permit = COMMAND_SLOTS + .acquire() + .await + .expect("command admission stays open"); + command.kill_on_drop(true).status().await +} + +async fn command_output(command: &mut Command) -> std::io::Result { + let _permit = COMMAND_SLOTS + .acquire() + .await + .expect("command admission stays open"); + command.kill_on_drop(true).output().await +} + /// Deletes `name` if it exists; a no-op otherwise. Called on essentially /// every setup — "doesn't exist yet" is the routine, expected case (this is /// a purge-before-create, not a lookup of something that's supposed to be @@ -527,28 +534,27 @@ async fn attach_and_size( Ok(()) } -fn docker_pid(container: &str) -> Result { - let out = Command::new("docker") - .args(["inspect", "-f", "{{.State.Pid}}", container]) - .output() - .handle_err(location!())?; +async fn docker_pid(container: &str) -> Result { + let out = + command_output(Command::new("docker").args(["inspect", "-f", "{{.State.Pid}}", container])) + .await + .handle_err(location!())?; String::from_utf8_lossy(&out.stdout) .trim() .parse::() .handle_err(location!()) } -/// Configures the namespace's own end of the veth pair (address, mtu/up, -/// default route) from inside the namespace it was just moved into. -fn configure_ns_in(params: &VxlanSetupParams, ns_pid: Option) -> Result<(), Error> { +/// Configure the peer address, bring it up, and add a standalone default route. +async fn configure_ns_in(params: &VxlanSetupParams, ns_pid: Option) -> Result<(), Error> { let veth_in = format!("{}-in", params.ns_name); let prefix: Vec = match ns_pid { Some(pid) => vec!["nsenter".into(), "-t".into(), pid.to_string(), "-n".into()], + // Only network state changes here; avoid cloning the mount namespace. None => vec![ - "ip".into(), - "netns".into(), - "exec".into(), - params.ns_name.clone(), + "nsenter".into(), + format!("--net=/var/run/netns/{}", params.ns_name), + "--".into(), ], }; @@ -562,19 +568,9 @@ fn configure_ns_in(params: &VxlanSetupParams, ns_pid: Option) -> Result<(), "dev", &veth_in, ], - )?; - ns_exec( - &prefix, - &[ - "ip", - "link", - "set", - &veth_in, - "mtu", - &OVERLAY_MTU.to_string(), - "up", - ], - )?; + ) + .await?; + ns_exec(&prefix, &["ip", "link", "set", &veth_in, "up"]).await?; if ns_pid.is_none() { // Standalone mode only: docker mode leaves routing to the container. ns_exec( @@ -587,22 +583,21 @@ fn configure_ns_in(params: &VxlanSetupParams, ns_pid: Option) -> Result<(), "via", ¶ms.br_net.ip().to_string(), ], - )?; + ) + .await?; } Ok(()) } -fn ns_exec(prefix: &[String], extra: &[&str]) -> Result<(), Error> { - let status = Command::new("sudo") - .args(prefix) - .args(extra) - .status() +async fn ns_exec(prefix: &[String], extra: &[&str]) -> Result<(), Error> { + let status = command_status(Command::new(&prefix[0]).args(&prefix[1..]).args(extra)) + .await .handle_err(location!())?; if status.success() { Ok(()) } else { Err(format!( - "`sudo {} {}` failed: {status}", + "`{} {}` failed: {status}", prefix.join(" "), extra.join(" ") )) @@ -610,22 +605,23 @@ fn ns_exec(prefix: &[String], extra: &[&str]) -> Result<(), Error> { } } -fn sudo_checked(args: &[&str]) -> Result<(), Error> { - let status = Command::new("sudo") - .args(args) - .status() +async fn privileged_checked(args: &[&str]) -> Result<(), Error> { + let status = command_status(Command::new(args[0]).args(&args[1..])) + .await .handle_err(location!())?; if status.success() { Ok(()) } else { - Err(format!("`sudo {}` failed: {status}", args.join(" "))).handle_err(location!()) + Err(format!("`{}` failed: {status}", args.join(" "))).handle_err(location!()) } } -/// `sudo` with output captured rather than inherited, for calls whose failure +/// Privileged command with captured output, for calls whose failure /// is the expected steady state (deleting state that isn't there). -fn sudo_quiet(args: &[&str]) -> std::io::Result { - Command::new("sudo").args(args).output().map(|o| o.status) +async fn privileged_quiet(args: &[&str]) -> std::io::Result { + command_output(Command::new(args[0]).args(&args[1..])) + .await + .map(|o| o.status) } /// SPI values 1-255 are IANA-reserved (RFC 4301) and the kernel's XFRM code @@ -635,9 +631,9 @@ fn xfrm_spi(vxlan_id: u32) -> String { format!("0x{:08x}", vxlan_id + 1_000) } -fn purge_xfrm_spi(vxlan_id: u32) { +async fn purge_xfrm_spi(vxlan_id: u32) { let spi = xfrm_spi(vxlan_id); - let _ = sudo_quiet(&[ + let _ = privileged_quiet(&[ "ip", "xfrm", "state", @@ -646,18 +642,19 @@ fn purge_xfrm_spi(vxlan_id: u32) { "esp", "spi", &spi, - ]); + ]) + .await; } /// Installs this tunnel's IPsec/ESP state+policy (AES-256-GCM, transport /// mode) between the two hosts' physical IPs, scoped to this tunnel's /// dstport. A separate netlink protocol family (`NETLINK_XFRM`), not covered /// by rtnetlink — stays CLI, ported 1:1 from `vxlan-setup.sh`. -fn install_xfrm(params: &VxlanSetupParams) -> Result<(), Error> { +async fn install_xfrm(params: &VxlanSetupParams) -> Result<(), Error> { // RFC4106 GCM keys are "AES key || 4-byte salt". The server only hands // out a 32-byte AES key, so the salt is derived here, identically on // both ends, from that same key. - let salt = sha256sum_prefix(¶ms.key_hex)?; + let salt = sha256sum_prefix(¶ms.key_hex).await?; let aead_key = format!("0x{}{salt}", params.key_hex); let spi = xfrm_spi(params.vxlan_id); let dstport = params.dstport.to_string(); @@ -667,7 +664,7 @@ fn install_xfrm(params: &VxlanSetupParams) -> Result<(), Error> { // Outbound: this host -> remote. Inbound: remote -> this host. Argument // order matters to `ip xfrm`'s positional parser — see the script this // was ported from for the (extensively tested) details. - sudo_checked(&[ + privileged_checked(&[ "ip", "xfrm", "state", @@ -686,8 +683,9 @@ fn install_xfrm(params: &VxlanSetupParams) -> Result<(), Error> { "128", "mode", "transport", - ])?; - sudo_checked(&[ + ]) + .await?; + privileged_checked(&[ "ip", "xfrm", "policy", @@ -713,8 +711,9 @@ fn install_xfrm(params: &VxlanSetupParams) -> Result<(), Error> { &spi, "mode", "transport", - ])?; - sudo_checked(&[ + ]) + .await?; + privileged_checked(&[ "ip", "xfrm", "state", @@ -733,8 +732,9 @@ fn install_xfrm(params: &VxlanSetupParams) -> Result<(), Error> { "128", "mode", "transport", - ])?; - sudo_checked(&[ + ]) + .await?; + privileged_checked(&[ "ip", "xfrm", "policy", @@ -760,13 +760,19 @@ fn install_xfrm(params: &VxlanSetupParams) -> Result<(), Error> { &spi, "mode", "transport", - ])?; + ]) + .await?; Ok(()) } -fn sha256sum_prefix(key_hex: &str) -> Result { - use std::io::Write; +async fn sha256sum_prefix(key_hex: &str) -> Result { + use tokio::io::AsyncWriteExt; + let _permit = COMMAND_SLOTS + .acquire() + .await + .expect("command admission stays open"); let mut child = Command::new("sha256sum") + .kill_on_drop(true) .stdin(std::process::Stdio::piped()) .stdout(std::process::Stdio::piped()) .spawn() @@ -777,8 +783,9 @@ fn sha256sum_prefix(key_hex: &str) -> Result { .ok_or("sha256sum stdin unavailable") .handle_err(location!())? .write_all(key_hex.as_bytes()) + .await .handle_err(location!())?; - let out = child.wait_with_output().handle_err(location!())?; + let out = child.wait_with_output().await.handle_err(location!())?; let digest = String::from_utf8_lossy(&out.stdout); let hex = digest .split_whitespace() @@ -805,6 +812,25 @@ fn format_mac(mac: &[u8]) -> String { #[cfg(test)] mod tests { + + #[tokio::test] + async fn command_wait_yields_and_preserves_output_and_status() { + let mut command = tokio::process::Command::new("sleep"); + command.arg("2"); + tokio::select! { + biased; + _ = super::command_status(&mut command) => panic!("child wait blocked the runtime"), + () = tokio::time::sleep(std::time::Duration::from_millis(20)) => {} + } + let out = super::command_output( + tokio::process::Command::new("sh").args(["-c", "printf diagnostic; exit 7"]), + ) + .await + .unwrap(); + assert_eq!(out.stdout, b"diagnostic"); + assert_eq!(out.status.code(), Some(7)); + } + use super::{format_mac, veth_mac, xfrm_spi}; /// Net id 101 -> spi 1101 = 0x44d, clear of the 1-255 IANA-reserved band. diff --git a/members/nullnet-client/src/commands/vxlan_cleanup.rs b/members/nullnet-client/src/commands/vxlan_cleanup.rs new file mode 100644 index 0000000..140e98f --- /dev/null +++ b/members/nullnet-client/src/commands/vxlan_cleanup.rs @@ -0,0 +1,154 @@ +use futures::TryStreamExt; +use rtnetlink::packet_route::link::LinkAttribute; +use rtnetlink::{Handle, LinkUnspec}; +use std::collections::HashSet; +use std::sync::atomic::{AtomicUsize, Ordering}; +use std::sync::{Arc, OnceLock}; +use tokio::sync::{OwnedMutexGuard, mpsc, oneshot}; + +// A single worker owns this temporary group; only retiring links enter it. +const RETIRING_GROUP: u32 = 0x4e40_0000; +const DELETE_BATCH: usize = 128; +static ACTIVE_SETUPS: AtomicUsize = AtomicUsize::new(0); +type Request = ( + u32, + Arc>, + oneshot::Sender>, +); +static CLEANUP: OnceLock> = OnceLock::new(); + +pub(super) struct SetupGuard; + +pub(super) fn track_setup() -> SetupGuard { + ACTIVE_SETUPS.fetch_add(1, Ordering::Relaxed); + SetupGuard +} + +impl Drop for SetupGuard { + fn drop(&mut self) { + ACTIVE_SETUPS.fetch_sub(1, Ordering::Relaxed); + } +} + +pub(super) async fn remove( + handle: &Handle, + group: u32, + guard: Arc>, +) -> Result<(), String> { + let sender = CLEANUP.get_or_init(|| { + let (sender, mut receiver) = mpsc::channel::(1024); + let handle = handle.clone(); + tokio::spawn(async move { + let mut batch = Vec::with_capacity(DELETE_BATCH); + while let Some(first) = receiver.recv().await { + batch.push(first); + // Deletion holds RTNL. Yield between smaller batches while setup + // is active; the counter is only a scheduling hint, never a lock. + let limit = if ACTIVE_SETUPS.load(Ordering::Relaxed) == 0 { + DELETE_BATCH + } else { + 16 + }; + while batch.len() < limit { + match receiver.try_recv() { + Ok(request) => batch.push(request), + Err(_) => break, + } + } + let groups = batch.iter().map(|(group, _, _)| *group).collect(); + let result = remove_groups(&handle, groups).await; + for (_, _guard, response) in batch.drain(..) { + let _ = response.send(result.clone()); + } + } + }); + sender + }); + let (response, received) = oneshot::channel(); + sender + .send((group, guard, response)) + .await + .map_err(|e| e.to_string())?; + received.await.map_err(|e| e.to_string())? +} + +async fn remove_groups(handle: &Handle, groups: HashSet) -> Result<(), String> { + if groups.len() == 1 { + return delete_group(handle, *groups.iter().next().unwrap()).await; + } + let links: Vec<_> = handle + .link() + .get() + .execute() + .try_collect() + .await + .map_err(|e| e.to_string())?; + for link in links { + if link + .attributes + .iter() + .any(|attr| matches!(attr, LinkAttribute::Group(group) if groups.contains(group))) + { + let result = handle + .link() + .set( + LinkUnspec::new_with_index(link.header.index) + .link_group(RETIRING_GROUP) + .build(), + ) + .execute() + .await; + if let Err(error) = result { + // Finish both assigned and unassigned groups on partial failure. + let mut cleanup = delete_group(handle, RETIRING_GROUP).await; + for group in &groups { + if let Err(error) = delete_group(handle, *group).await { + cleanup = Err(error); + } + } + return cleanup.map_err(|cleanup| format!("{error}; cleanup: {cleanup}")); + } + } + } + delete_group(handle, RETIRING_GROUP).await +} + +async fn delete_group(handle: &Handle, group: u32) -> Result<(), String> { + let mut request = handle.link().del(0); + request + .message_mut() + .attributes + .push(LinkAttribute::Group(group)); + match request.execute().await { + Ok(()) => Ok(()), + Err(rtnetlink::Error::NetlinkError(e)) + if e.code.map(std::num::NonZeroI32::get) == Some(-libc::ENODEV) => + { + Ok(()) + } + Err(error) => Err(error.to_string()), + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[tokio::test] + async fn cancelled_setup_releases_its_scheduling_hint() { + let first = track_setup(); + let (started, received) = oneshot::channel(); + let setup = tokio::spawn(async move { + let _guard = track_setup(); + started.send(()).unwrap(); + std::future::pending::<()>().await; + }); + received.await.unwrap(); + assert_eq!(ACTIVE_SETUPS.load(Ordering::Relaxed), 2); + drop(first); + assert_eq!(ACTIVE_SETUPS.load(Ordering::Relaxed), 1); + setup.abort(); + assert!(setup.await.unwrap_err().is_cancelled()); + assert_eq!(ACTIVE_SETUPS.load(Ordering::Relaxed), 0); + } +} diff --git a/members/nullnet-client/src/control_channel.rs b/members/nullnet-client/src/control_channel.rs index 968e46d..7804c41 100644 --- a/members/nullnet-client/src/control_channel.rs +++ b/members/nullnet-client/src/control_channel.rs @@ -1,10 +1,12 @@ +mod lifecycle; + use crate::commands::{RtNetLinkHandle, configure_access_port, dnat, egress, remove_vlan}; use crate::conntrack::LivenessSets; use crate::ebpf::{FirewallPeers, FirewallVxlanPorts, NetId}; use crate::egress_policy::{FLUSH_SUPPRESSION, PolicyVerdicts, flush_container_conntrack}; use crate::egress_state::{EgressRecord, EgressState}; use crate::host_mappings::{ - HOSTS_MARKER, HostMappingsState, edit_container_hosts, hosts_file_lock, + HOSTS_MARKER, HostMappingsState, edit_container_hosts, hosts_file_lock, write_hosts_file, }; use crate::nfqueue::BridgeIpCache; use crate::peers::peer::{Peers, VethKey}; @@ -37,14 +39,9 @@ fn fire_event(grpc: &NullnetGrpcInterface, kind: AgentEventKind) { }); } -/// Confirm a completed teardown so the server can return the net id to its -/// pool. Must be called only once the teardown has actually run — the whole -/// point of the ack is that the id stays out of circulation until this edge's -/// kernel state is gone. -/// -/// `msg_id` is absent when the server predates the ack field; there is then -/// nothing to confirm and the server falls back to its grace timer. -async fn ack_teardown( +/// Confirm a completed control operation. Teardown acknowledgements follow +/// all kernel cleanup so the server can safely reuse the net id. +async fn ack_control( outbound: &Sender, msg_id: Option, grpc: &NullnetGrpcInterface, @@ -89,7 +86,10 @@ pub(crate) async fn control_channel( AgentEventKind::ControlChannelEstablished(AgentControlChannelEstablished {}), ); - while let Ok(Some(message)) = inbound.message().await { + let lifecycle = lifecycle::LifecycleTasks::default(); + while let Ok(Some(message)) = inbound.message().await.inspect_err(|error| { + eprintln!("Control channel from server failed: {error:?}"); + }) { let rtnetlink_handle = rtnetlink_handle.clone(); let peers = peers.clone(); let outbound = outbound.clone(); @@ -132,7 +132,7 @@ pub(crate) async fn control_channel( let triggers_state = triggers_state.clone(); let egress_state = egress_state.clone(); let sets = sets.clone(); - tokio::spawn(async move { + lifecycle.spawn(vxlan_setup.vxlan_id, async move { let _ = handle_vxlan_setup( vxlan_setup, rtnetlink_handle, @@ -155,7 +155,7 @@ pub(crate) async fn control_channel( let triggers_state = triggers_state.clone(); let egress_state = egress_state.clone(); let sets = sets.clone(); - tokio::spawn(async move { + lifecycle.spawn(vxlan_teardown.vxlan_id, async move { handle_vxlan_teardown( vxlan_teardown, rtnetlink_handle, @@ -172,6 +172,47 @@ pub(crate) async fn control_channel( .await; }); } + Some(net_message::Message::NetReady(ready)) => { + let triggers_state = triggers_state.clone(); + lifecycle.spawn(ready.net_id, async move { + let grpc = server.clone(); + let published = tokio::task::spawn_blocking(move || { + let mut published = true; + host_mappings_state.with_vxlan(ready.net_id, |mapping, container| { + if add_host_mapping(mapping, container).is_err() { + published = false; + fire_event( + &grpc, + AgentEventKind::HostMappingFailed(AgentHostMappingFailed { + hostname: mapping.name.clone(), + ip: mapping.ip.clone(), + docker_container: container.map(String::from), + }), + ); + } + }); + published + }) + .await + .expect("host mapping publication task panicked"); + if !published { + return; + } + if triggers_state.mark_ready(ready.net_id) { + ack_control(&outbound, ready.msg_id, &server, "net_ready").await; + } else { + fire_event( + &server, + AgentEventKind::VxlanSetupFailed(AgentVxlanSetupFailed { + vxlan_id: ready.net_id, + ns_name: format!("ns_{}_c", ready.net_id), + error_code: -1, + }), + ); + eprintln!("NetReady for {} has no installed steering", ready.net_id); + } + }); + } Some(net_message::Message::ContainerSuspend(container_suspend)) => { tokio::spawn(async move { handle_container_suspend(container_suspend, server); @@ -379,7 +420,7 @@ async fn handle_vlan_teardown( // Acked last: the server frees the net id on this, so everything above must // already be undone. - ack_teardown(&outbound, ack_id, &grpc, "vlan_teardown").await; + ack_control(&outbound, ack_id, &grpc, "vlan_teardown").await; Ok(()) } @@ -520,6 +561,7 @@ async fn handle_vxlan_setup( init_t.elapsed().as_millis(), message.docker_container.as_deref().unwrap_or("none"), ); + setup_result?; // Egress edges install steering (initiator) or interception (proxy) instead // of the host-mapping + DNAT path used by proxy/backend edges. @@ -561,25 +603,23 @@ async fn handle_vxlan_setup( } else { egress::install_steer(vxlan_id, &br_name, gw, snat_src, cip) }; + if local_gateway { + egress_state.record(vxlan_id, EgressRecord::Local); + } else { + egress_state.record( + vxlan_id, + EgressRecord::Steer { + br_name: br_name.clone(), + snat_src, + container_ip: cip, + }, + ); + } if installed { - if local_gateway { - egress_state.record(vxlan_id, EgressRecord::Local); - } else { - egress_state.record( - vxlan_id, - EgressRecord::Steer { - br_name: br_name.clone(), - snat_src, - container_ip: cip, - }, - ); - } - // Steering is live — release any egress SYN the NFQUEUE - // listener is holding for this initiator. Mirrors the - // backend DNAT→mark_active ordering (install first, then - // wake, so the freed packet finds the rule in place). + // Record local steering; NetReady releases packets only + // after the gateway also acknowledges setup. if let Some(container) = message.docker_container.as_deref() { - triggers_state.mark_active( + triggers_state.mark_prepared( container, crate::triggers::EGRESS_TRIGGER_PORT, vxlan_id, @@ -598,6 +638,7 @@ async fn handle_vxlan_setup( error_message: "steer rules failed to install".to_string(), }), ); + return Err("egress steering installation failed").handle_err(location!()); } } _ => { @@ -615,18 +656,18 @@ async fn handle_vxlan_setup( ), }), ); + return Err("egress steering has no gateway or source").handle_err(location!()); } } } else if egress_intercept && local_ip != remote_ip { - if egress::install_gateway_forward(&br_name, &message.br_net) { - egress_state.record( - vxlan_id, - EgressRecord::Gateway { - br_name: br_name.clone(), - br_net: message.br_net.clone(), - }, - ); - } else { + egress_state.record( + vxlan_id, + EgressRecord::Gateway { + br_name: br_name.clone(), + br_net: message.br_net.clone(), + }, + ); + if !egress::install_gateway_forward(&br_name, &message.br_net) { fire_event( &grpc, AgentEventKind::GatewayForwardInstallFailed(AgentGatewayForwardInstallFailed { @@ -634,9 +675,19 @@ async fn handle_vxlan_setup( br_net: message.br_net.clone(), }), ); + return Err("gateway forwarding installation failed").handle_err(location!()); } } else if let Some(host_mapping) = &message.host_mapping { - if add_host_mapping(host_mapping, message.docker_container.as_deref()).is_err() { + host_mappings_state.record_vxlan( + vxlan_id, + host_mapping.clone(), + message.docker_container.clone(), + ); + // Backend names stay on their trigger address until both endpoints + // are ready; publishing early lets container ARP bypass NFQUEUE. + if message.dnat_port.is_none() + && add_host_mapping(host_mapping, message.docker_container.as_deref()).is_err() + { fire_event( &grpc, AgentEventKind::HostMappingFailed(AgentHostMappingFailed { @@ -645,25 +696,11 @@ async fn handle_vxlan_setup( docker_container: message.docker_container.clone(), }), ); + return Err("host mapping installation failed").handle_err(location!()); } - host_mappings_state.record_vxlan( - vxlan_id, - host_mapping.clone(), - message.docker_container.clone(), - ); - // backend-entry edge: install DNAT(dnat_port -> overlay_ip) so the - // initiator's traffic on that local port is steered into the new - // VXLAN. - // - // Order matters. The NFQUEUE listener is parked on a `Notify` that - // `mark_active` fires; once woken it verdicts ACCEPT and the held - // packet traverses `nat PREROUTING`. The DNAT rule MUST already be - // installed by then, so we: - // 1. peek the initiator's bridge IP (stashed at `mark_pending`) - // 2. install DNAT with `-s ` - // 3. mark_active → wakes the waiter, packet released into the new - // rule + // Install scoped DNAT before acknowledging local setup. NetReady + // releases held packets once the receiving endpoint is also ready. if let Some(dnat_port) = message.dnat_port && let Ok(dnat_port) = u16::try_from(dnat_port) && let Ok(overlay_ip) = host_mapping.ip.parse::() @@ -676,12 +713,12 @@ async fn handle_vxlan_setup( // untrustworthy first — otherwise a rebuild under open connections // reads as 1->0 and decrements the chain out from under them. sets.suppress_trigger(container_key, dnat_port, FLUSH_SUPPRESSION); - // Only promote to Active if the DNAT rule is actually live. Waking + // Only record prepared steering if the DNAT rule is actually live. Waking // the held packet without it would release the SYN into a missing // rule (→ misroute to the original dest); instead leave it Pending // so the listener drops at ACTIVE_TIMEOUT. if dnat::install(dnat_port, overlay_ip, container_ip) { - triggers_state.mark_active( + triggers_state.mark_prepared( container_key, dnat_port, vxlan_id, @@ -689,6 +726,14 @@ async fn handle_vxlan_setup( container_ip, ); } else { + // Retain partial-rule ownership for the server's rollback. + triggers_state.mark_prepared( + container_key, + dnat_port, + vxlan_id, + overlay_ip, + container_ip, + ); fire_event( &grpc, AgentEventKind::DnatInstallFailed(AgentDnatInstallFailed { @@ -696,6 +741,7 @@ async fn handle_vxlan_setup( overlay_ip: overlay_ip.to_string(), }), ); + return Err("backend DNAT installation failed").handle_err(location!()); } } else if message.dnat_port.is_some() { // Backend-entry edge with a malformed port or host IP — DNAT @@ -715,6 +761,7 @@ async fn handle_vxlan_setup( overlay_ip: host_mapping.ip.clone(), }), ); + return Err("invalid backend DNAT mapping").handle_err(location!()); } } @@ -765,9 +812,12 @@ async fn handle_vxlan_teardown( br_name, snat_src, container_ip, - } => egress::remove_steer(message.vxlan_id, &br_name, snat_src, container_ip), + } => { + local_cleanup_ok = + egress::remove_steer(message.vxlan_id, &br_name, snat_src, container_ip); + } EgressRecord::Gateway { br_name, br_net } => { - egress::remove_gateway_forward(&br_name, &br_net) + local_cleanup_ok = egress::remove_gateway_forward(&br_name, &br_net); } } } @@ -796,6 +846,7 @@ async fn handle_vxlan_teardown( // issues the flush. sets.suppress_trigger(&container, port, FLUSH_SUPPRESSION); if !dnat::remove(port, overlay_ip, container_ip) { + local_cleanup_ok = false; fire_event( &grpc, AgentEventKind::DnatRemovalFailed(AgentDnatRemovalFailed { @@ -807,10 +858,16 @@ async fn handle_vxlan_teardown( } // remove host mapping if one was installed at setup - if let Some((host_mapping, docker_container)) = host_mappings_state.take_vxlan(message.vxlan_id) - { - let _ = remove_host_mapping(&host_mapping, docker_container.as_deref()); - } + let net_id = message.vxlan_id; + local_cleanup_ok &= tokio::task::spawn_blocking(move || { + host_mappings_state + .take_vxlan(net_id) + .is_none_or(|(mapping, container)| { + remove_host_mapping(&mapping, container.as_deref()).is_ok() + }) + }) + .await + .expect("host mapping cleanup task panicked"); // teardown VXLAN on this machine let init_t = std::time::Instant::now(); @@ -826,7 +883,6 @@ async fn handle_vxlan_teardown( // A malformed dstport just means the (already-idempotent) XFRM // policy cleanup below is skipped — everything else still tears down. dstport: u16::try_from(message.dstport).unwrap_or(crate::DEFAULT_VXLAN_DSTPORT), - docker_container: message.docker_container.clone(), }; let teardown_result = crate::commands::vxlan::teardown(&rtnetlink_handle, &vxlan_params, &bridge_cache).await; @@ -857,7 +913,9 @@ async fn handle_vxlan_teardown( // Acked last: the server frees the net id on this, so every kernel object // named after it — bridge, veth/macsec pair, XFRM SA, DNAT — must already // be gone. - ack_teardown(&outbound, ack_id, &grpc, "vxlan_teardown").await; + if local_cleanup_ok && teardown_result.is_ok() { + ack_control(&outbound, ack_id, &grpc, "vxlan_teardown").await; + } } /// Pause an idle container. Fire-and-forget: the server marks the replica @@ -1007,8 +1065,11 @@ fn add_host_mapping(hm: &HostMapping, docker_container: Option<&str>) -> Result< } else { // host-targeted: upsert into the host's /etc/hosts let content = std::fs::read_to_string(path).handle_err(location!())?; - std::fs::write(path, upsert_hosts_entry(&content, &hm.name, &entry)) - .handle_err(location!())?; + write_hosts_file( + std::path::Path::new(path), + &upsert_hosts_entry(&content, &hm.name, &entry), + ) + .handle_err(location!())?; } Ok(()) @@ -1049,8 +1110,11 @@ fn remove_host_mapping(hm: &HostMapping, docker_container: Option<&str>) -> Resu } else { // host-targeted: drop this net's line from the host file let content = std::fs::read_to_string(path).handle_err(location!())?; - std::fs::write(path, remove_hosts_entry(&content, &hm.name, &hm.ip)) - .handle_err(location!())?; + write_hosts_file( + std::path::Path::new(path), + &remove_hosts_entry(&content, &hm.name, &hm.ip), + ) + .handle_err(location!())?; } Ok(()) @@ -1082,3 +1146,94 @@ fn remove_hosts_entry(content: &str, name: &str, ip: &str) -> String { fn hex_encode(bytes: &[u8]) -> String { bytes.iter().map(|b| format!("{b:02x}")).collect() } + +#[cfg(test)] +mod failure_tests { + use super::*; + + #[tokio::test] + #[ignore = "requires root, nnreview0 dummy interface, and the lab TLS server"] + async fn failed_kernel_teardown_is_not_acknowledged() { + let grpc = NullnetGrpcInterface::new( + &crate::env::CONTROL_SERVICE_ADDR, + *crate::env::CONTROL_SERVICE_PORT, + std::path::Path::new(&*crate::env::CONTROL_SERVICE_CA_CERT), + ) + .await + .unwrap(); + let firewall = crate::ebpf::enable( + "nnreview0", + &crate::ebpf::FirewallConfig { + server_ip: crate::env::CONTROL_SERVICE_ADDR.parse().unwrap(), + control_port: *crate::env::CONTROL_SERVICE_PORT, + egress_gateway: false, + ingress_tcp: vec![], + ingress_udp: vec![], + egress_tcp: vec![], + egress_udp: vec![], + }, + ) + .unwrap(); + let (outbound, mut acks) = mpsc::channel(2); + let setup = handle_vxlan_setup( + VxlanSetup { + msg_id: Some(MsgId { + id: "review-failed-setup".into(), + }), + vxlan_id: 2_000_000, + ns_name: "ns_2000000_c".into(), + br_name: "br_2000000_c".into(), + ns_net: "10.250.0.2/29".into(), + br_net: "10.250.0.1/29".into(), + local_ip: "127.0.0.1".into(), + remote_ip: "127.0.0.1".into(), + dstport: u32::from(crate::DEFAULT_VXLAN_DSTPORT), + docker_container: Some("nullnet-review-deliberately-absent".into()), + ..Default::default() + }, + RtNetLinkHandle::disconnected(), + outbound.clone(), + Arc::default(), + Arc::default(), + grpc.clone(), + firewall.peers.clone(), + firewall.vxlan_ports.clone(), + Arc::default(), + LivenessSets::new(), + BridgeIpCache::new(), + ) + .await; + assert!(setup.is_err()); + assert!( + acks.try_recv().is_err(), + "failed setup was acknowledged as completed" + ); + handle_vxlan_teardown( + VxlanTeardown { + msg_id: Some(MsgId { + id: "review-failed-teardown".into(), + }), + vxlan_id: 2_000_000, + ns_name: "ns_2000000_c".into(), + br_name: "br_2000000_c".into(), + dstport: u32::from(crate::DEFAULT_VXLAN_DSTPORT), + ..Default::default() + }, + RtNetLinkHandle::disconnected(), + Arc::default(), + outbound, + Arc::default(), + grpc, + firewall.peers.clone(), + firewall.vxlan_ports.clone(), + Arc::default(), + LivenessSets::new(), + BridgeIpCache::new(), + ) + .await; + assert!( + acks.try_recv().is_err(), + "failed kernel cleanup was acknowledged as completed" + ); + } +} diff --git a/members/nullnet-client/src/control_channel/lifecycle.rs b/members/nullnet-client/src/control_channel/lifecycle.rs new file mode 100644 index 0000000..3f5d9b7 --- /dev/null +++ b/members/nullnet-client/src/control_channel/lifecycle.rs @@ -0,0 +1,74 @@ +use std::collections::HashMap; +use std::future::Future; +use std::sync::{Arc, Mutex}; +use tokio::sync::oneshot; + +type Tail = (Arc<()>, oneshot::Receiver<()>); + +#[derive(Default)] +pub(super) struct LifecycleTasks { + tails: Arc>>, +} + +impl LifecycleTasks { + // Enqueue before spawning so scheduling cannot reorder a tunnel's commands. + pub(super) fn spawn(&self, id: u32, work: impl Future + Send + 'static) { + let token = Arc::new(()); + let (done, finished) = oneshot::channel(); + let previous = self + .tails + .lock() + .unwrap() + .insert(id, (token.clone(), finished)); + let tails = self.tails.clone(); + tokio::spawn(async move { + if let Some((_, previous)) = previous { + let _ = previous.await; + } + work.await; + let mut tails = tails.lock().unwrap(); + if tails + .get(&id) + .is_some_and(|(tail, _)| Arc::ptr_eq(tail, &token)) + { + tails.remove(&id); + } + let _ = done.send(()); + }); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[tokio::test] + async fn orders_one_tunnel_without_blocking_another_and_releases_state() { + let tasks = LifecycleTasks::default(); + let (release, held) = oneshot::channel(); + let (events, mut observed) = tokio::sync::mpsc::unbounded_channel(); + let first = events.clone(); + tasks.spawn(1, async move { + held.await.unwrap(); + first.send("setup").unwrap(); + }); + let second = events.clone(); + tasks.spawn(1, async move { + second.send("ready").unwrap(); + }); + let third = events.clone(); + tasks.spawn(1, async move { + third.send("teardown").unwrap(); + }); + tasks.spawn(2, async move { + events.send("other").unwrap(); + }); + assert_eq!(observed.recv().await, Some("other")); + assert!(observed.try_recv().is_err()); + release.send(()).unwrap(); + for expected in ["setup", "ready", "teardown"] { + assert_eq!(observed.recv().await, Some(expected)); + } + assert!(tasks.tails.lock().unwrap().is_empty()); + } +} diff --git a/members/nullnet-client/src/host_mappings.rs b/members/nullnet-client/src/host_mappings.rs index dab57bc..07f64c8 100644 --- a/members/nullnet-client/src/host_mappings.rs +++ b/members/nullnet-client/src/host_mappings.rs @@ -1,5 +1,6 @@ use nullnet_grpc_lib::nullnet_grpc::HostMapping; use std::collections::HashMap; +use std::io::Write; use std::path::{Path, PathBuf}; use std::process::Command; use std::sync::{Arc, LazyLock, Mutex, PoisonError}; @@ -45,9 +46,11 @@ pub fn hosts_file_lock(docker_container: Option<&str>) -> Arc> { #[derive(Default)] pub struct HostMappingsState { by_vlan: Mutex>, - by_vxlan: Mutex)>>, + by_vxlan: Mutex>>>>, } +type VxlanMapping = (HostMapping, Option); + impl HostMappingsState { pub fn record_vlan(&self, vlan_id: u16, hm: HostMapping) { self.by_vlan.lock().unwrap().insert(vlan_id, hm); @@ -61,11 +64,22 @@ impl HostMappingsState { self.by_vxlan .lock() .unwrap() - .insert(vxlan_id, (hm, docker_container)); + .insert(vxlan_id, Arc::new(Mutex::new(Some((hm, docker_container))))); } pub fn take_vxlan(&self, vxlan_id: u32) -> Option<(HostMapping, Option)> { - self.by_vxlan.lock().unwrap().remove(&vxlan_id) + let entry = self.by_vxlan.lock().unwrap().remove(&vxlan_id)?; + entry.lock().unwrap().take() + } + + // Keep publication ordered before teardown takes and removes the mapping. + pub fn with_vxlan(&self, vxlan_id: u32, apply: impl FnOnce(&HostMapping, Option<&str>)) { + let entry = self.by_vxlan.lock().unwrap().get(&vxlan_id).cloned(); + let Some(entry) = entry else { return }; + let mapping = entry.lock().unwrap(); + if let Some((mapping, container)) = mapping.as_ref() { + apply(mapping, container.as_deref()); + } } } @@ -86,7 +100,7 @@ pub fn purge_stale_mappings() { if let Ok(content) = std::fs::read_to_string(HOSTS_PATH) { let cleaned = strip_marked(&content); if cleaned != content { - match std::fs::write(HOSTS_PATH, &cleaned) { + match write_hosts_file(Path::new(HOSTS_PATH), &cleaned) { Ok(()) => swept += 1, Err(e) => eprintln!("[hosts] purge: writing {HOSTS_PATH} failed: {e}"), } @@ -192,11 +206,19 @@ fn edit_hosts_file(path: &Path, edit: impl FnOnce(&str) -> String) -> Result std::io::Result<()> { + let mut file = std::fs::OpenOptions::new().write(true).open(path)?; + file.write_all(content.as_bytes())?; + file.set_len(content.len() as u64) +} + #[cfg(test)] mod edit_tests { use super::{HOSTS_MARKER, edit_hosts_file, strip_marked}; @@ -257,6 +279,79 @@ mod edit_tests { mod tests { use super::{HOSTS_MARKER, strip_marked}; + #[test] + fn slow_publication_does_not_block_unrelated_tunnels() { + use super::*; + use std::sync::mpsc; + use std::time::Duration; + let state = Arc::new(HostMappingsState::default()); + let mapping = HostMapping { + name: "backend".into(), + ip: "10.0.0.1".into(), + }; + state.record_vxlan(101, mapping.clone(), None); + let (entered, started) = mpsc::channel(); + let (release, wait) = mpsc::channel(); + let publisher = state.clone(); + let publish = std::thread::spawn(move || { + publisher.with_vxlan(101, |_, _| { + entered.send(()).unwrap(); + wait.recv().unwrap(); + }) + }); + started.recv().unwrap(); + let (done, finished) = mpsc::channel(); + let unrelated = state.clone(); + let record = std::thread::spawn(move || { + unrelated.record_vxlan(102, mapping, None); + done.send(()).unwrap(); + }); + let result = finished.recv_timeout(Duration::from_millis(200)); + release.send(()).unwrap(); + publish.join().unwrap(); + record.join().unwrap(); + assert!( + result.is_ok(), + "one publication blocked an unrelated tunnel" + ); + } + + #[test] + fn teardown_waits_for_publication_and_prevents_late_republication() { + use super::*; + let state = Arc::new(HostMappingsState::default()); + state.record_vxlan( + 101, + HostMapping { + name: "backend".into(), + ip: "10.0.0.1".into(), + }, + None, + ); + let (started, ready) = std::sync::mpsc::channel(); + let (release, wait) = std::sync::mpsc::channel(); + let publisher = state.clone(); + let publication = std::thread::spawn(move || { + publisher.with_vxlan(101, |_, _| { + started.send(()).unwrap(); + wait.recv().unwrap(); + }) + }); + ready.recv().unwrap(); + let entry = state.by_vxlan.lock().unwrap().get(&101).cloned().unwrap(); + assert!(entry.try_lock().is_err()); + let remover = state.clone(); + let teardown = std::thread::spawn(move || remover.take_vxlan(101)); + release.send(()).unwrap(); + publication.join().unwrap(); + assert!(teardown.join().unwrap().is_some()); + assert!( + entry.lock().unwrap().is_none(), + "an earlier snapshot must be invalidated too" + ); + state.with_vxlan(101, |_, _| panic!("removed mapping was republished")); + } + #[test] fn strips_only_marked_lines() { let content = format!( diff --git a/members/nullnet-client/src/nfqueue/egress_listener.rs b/members/nullnet-client/src/nfqueue/egress_listener.rs index c798f3a..5bca751 100644 --- a/members/nullnet-client/src/nfqueue/egress_listener.rs +++ b/members/nullnet-client/src/nfqueue/egress_listener.rs @@ -1,14 +1,8 @@ //! Egress-trigger NFQUEUE listener (queue 1) — hold-until-steered. //! -//! The kernel queues the first packet of each NEW flow to a non-internal -//! destination (see `commands::egress`). For a packet whose source maps to a -//! registered service container we fire `egress_trigger` so the server builds the -//! per-initiator egress edge to the gateway, and — unlike the old accept-and-retry -//! model — we **hold** the packet (defer the verdict) until steering is installed. -//! `control_channel` calls `TriggersState::mark_active` right after -//! `egress::install_steer` succeeds; that wakes the held handler, which verdicts -//! ACCEPT. The released packet then hits the freshly-installed `ip rule` and is -//! policy-routed into the tunnel, so the original SYN is not dropped. +//! Hold the first packet while the server builds the initiator's gateway edge. +//! VxlanSetup installs local steering; NetReady releases packets only after +//! both endpoints acknowledge setup, so SYNs cannot enter an unfinished peer. //! //! This reuses the backend-trigger machinery wholesale: the shared //! `recv_loop::spawn_queue_loop` plumbing and the `TriggersState` lifecycle, @@ -46,7 +40,7 @@ const QUEUE_MAX_LEN: u32 = 4096; /// the RPC cancels the server's handler mid-build. const TRIGGER_TIMEOUT: Duration = Duration::from_secs(30); /// How long the handler holds the packet waiting for steering to be installed -/// (`mark_active`) before giving up. +/// (`mark_ready`) before giving up. const STEER_TIMEOUT: Duration = Duration::from_secs(5); /// How long the handler waits for the server's egress policy verdict. const POLICY_TIMEOUT: Duration = Duration::from_secs(5); @@ -264,7 +258,7 @@ async fn decide_verdict(ctx: &EgressCtx, flow: Option) -> Decision { TriggerClaim::Pending(notify) => tracked(wait_for_steer(ctx, &container, notify).await), TriggerClaim::Start(notify) => { // Register the waiter BEFORE the gRPC round-trip: the server can - // dispatch the egress `VxlanSetup` (→ `mark_active`) faster than its + // dispatch the egress `NetReady` (→ `mark_ready`) faster than its // reply to `egress_trigger` returns, and `notify_waiters` only wakes // already-registered futures (see the backend listener for the race). let notified = notify.notified(); @@ -366,7 +360,7 @@ async fn policy_allows(ctx: &EgressCtx, container: &str, dst_ip: Ipv4Addr) -> bo /// Verdict for a held packet whose `egress_trigger` errored or timed out. /// /// The RPC failing says nothing about the `VxlanSetup`: it can land, install -/// steering and `mark_active` while the RPC is still in flight (observed in +/// steering and `mark_ready` while the RPC is still in flight (observed in /// production — steer live at +2s, RPC abandoned at +5s). So clear the entry /// only while it is still `Pending`, and release the packet if steering did go /// live after all. diff --git a/members/nullnet-client/src/nfqueue/listener.rs b/members/nullnet-client/src/nfqueue/listener.rs index 895bc9e..50e0540 100644 --- a/members/nullnet-client/src/nfqueue/listener.rs +++ b/members/nullnet-client/src/nfqueue/listener.rs @@ -191,16 +191,8 @@ async fn decide_verdict( match ctx.triggers_state.claim(container, dst_port, src_ip) { TriggerClaim::Active => Verdict::Accept, TriggerClaim::Pending(notify) => { - // `mark_active` wakes us with `Notify::notify_waiters()`, which - // only delivers to currently-registered futures — there is no - // stored-permit fallback. So we must `.enable()` the Notified - // future BEFORE awaiting, and then re-check state synchronously - // to close the window between the `state()` call above and our - // registration. Without this, `mark_active` firing in that - // window is a silently-lost wake-up and the held packet drops - // 5 s later for no reason — the visible symptom is the - // "[nfqueue] no VxlanSetup …" / "timeout waiting for active - // state …" log line on a chain that demonstrably came up. + // Register before checking readiness so a concurrent NetReady + // notification cannot be lost between the state check and wait. let notified = notify.notified(); tokio::pin!(notified); if notified.as_mut().enable() @@ -222,12 +214,8 @@ async fn decide_verdict( } } TriggerClaim::Start(notify) => { - // Register BEFORE the gRPC round-trip: the server can dispatch - // `VxlanSetup` (→ `mark_active` here) faster than its reply to - // `backend_trigger` arrives back, especially on multi-edge - // chains where `net_chain_setup` returns only after the slowest - // edge finishes. Without pre-registration the early - // `mark_active`'s wake fires to zero waiters and is lost. + // NetReady can precede the unary response. Register first so its + // notification remains visible while that response is in flight. let notified = notify.notified(); tokio::pin!(notified); if notified.as_mut().enable() @@ -251,13 +239,13 @@ async fn decide_verdict( Ok(Ok(())) => match timeout(ACTIVE_TIMEOUT, notified).await { Ok(_) => Verdict::Accept, Err(_) => { - // No `forget`: the trigger was accepted, so a VxlanSetup + // No `forget`: the trigger was accepted, so activation // is just slow. Forgetting wipes the stashed container_ip, // making the late setup install an unscoped DNAT. Keeping // Pending lets it peek the real IP; entry ages out at // PENDING_TIMEOUT so re-trigger still works. eprintln!( - "[nfqueue] no VxlanSetup for '{service}' port {dst_port} container {container}" + "[nfqueue] tunnel not ready for '{service}' port {dst_port} container {container}" ); Verdict::Drop } diff --git a/members/nullnet-client/src/triggers.rs b/members/nullnet-client/src/triggers.rs index ab43e08..ae5178c 100644 --- a/members/nullnet-client/src/triggers.rs +++ b/members/nullnet-client/src/triggers.rs @@ -31,7 +31,8 @@ pub enum Lifecycle { notify: Arc, container_ip: Ipv4Addr, }, - Active { + Installed { + ready: bool, vxlan_id: u32, overlay_ip: Ipv4Addr, container_ip: Ipv4Addr, @@ -67,7 +68,8 @@ impl TriggersState { pub fn state(&self, container: &str, port: u16) -> TriggerState { let by_key = self.by_key.lock().unwrap(); match by_key.get(&(container.to_string(), port)) { - Some(Lifecycle::Active { .. }) => TriggerState::Active, + Some(Lifecycle::Installed { ready: true, .. }) => TriggerState::Active, + Some(Lifecycle::Installed { ready: false, .. }) => TriggerState::Pending, Some(Lifecycle::Pending { since, .. }) if since.elapsed() < PENDING_TIMEOUT => { TriggerState::Pending } @@ -80,7 +82,14 @@ impl TriggersState { let mut by_key = self.by_key.lock().unwrap(); let key = (container.to_string(), port); match by_key.get(&key) { - Some(Lifecycle::Active { .. }) => return TriggerClaim::Active, + Some(Lifecycle::Installed { ready: true, .. }) => return TriggerClaim::Active, + Some(Lifecycle::Installed { + ready: false, + notify, + .. + }) => { + return TriggerClaim::Pending(notify.clone()); + } Some(Lifecycle::Pending { since, notify, .. }) if since.elapsed() < PENDING_TIMEOUT => { return TriggerClaim::Pending(notify.clone()); } @@ -106,26 +115,20 @@ impl TriggersState { } } - /// Read the `container_ip` stashed at claim time without - /// mutating state. Used by `control_channel` to install DNAT *before* - /// promoting to `Active` — installing first ensures the held packet - /// (which wakes on `mark_active`'s `notify_waiters`) finds the DNAT rule - /// live by the time it traverses `nat PREROUTING`. Returns - /// `Ipv4Addr::UNSPECIFIED` if no entry exists. + /// Read the claimed container IP before installing scoped steering. + /// Installed entries retain it until teardown, including before NetReady. pub fn peek_container_ip(&self, container: &str, port: u16) -> Ipv4Addr { let by_key = self.by_key.lock().unwrap(); match by_key.get(&(container.to_string(), port)) { Some(Lifecycle::Pending { container_ip, .. }) - | Some(Lifecycle::Active { container_ip, .. }) => *container_ip, + | Some(Lifecycle::Installed { container_ip, .. }) => *container_ip, None => Ipv4Addr::UNSPECIFIED, } } - /// Promote to `Active` (storing `container_ip` so teardown can match the - /// DNAT rule's `-s`) and wake every handler awaiting the transition. - /// The caller is expected to install DNAT *before* calling this — the - /// wake-up is what allows the held packet to traverse the chain. - pub fn mark_active( + /// Record installed local steering without releasing packets. The server + /// sends NetReady only after both endpoints acknowledge setup. + pub fn mark_prepared( &self, container: &str, port: u16, @@ -136,32 +139,55 @@ impl TriggersState { let mut by_key = self.by_key.lock().unwrap(); let key = (container.to_string(), port); let notify = match by_key.remove(&key) { - Some(Lifecycle::Pending { notify, .. } | Lifecycle::Active { notify, .. }) => notify, + Some(Lifecycle::Pending { notify, .. } | Lifecycle::Installed { notify, .. }) => notify, None => Arc::new(Notify::new()), }; by_key.insert( key, - Lifecycle::Active { + Lifecycle::Installed { + ready: false, vxlan_id, overlay_ip, container_ip, - notify: notify.clone(), + notify, }, ); - // Hold the lock while waking so concurrent `state()` callers see - // `Active` before they have a chance to register a new waiter that - // would never get woken. - notify.notify_waiters(); } - /// Drop only the pending generation owned by the failed caller. - /// - /// A trigger RPC that failed or timed out is not evidence that setup - /// failed: the `VxlanSetup` can land, install steering/DNAT and - /// `mark_active` while that RPC is still in flight. Dropping an `Active` - /// entry there discards the only record that the datapath is live, and the - /// server sends no second setup for an edge it already considers up — so - /// every later packet is held and dropped forever. + pub fn mark_ready(&self, vxlan_id: u32) -> bool { + let mut by_key = self.by_key.lock().unwrap(); + for lifecycle in by_key.values_mut() { + if let Lifecycle::Installed { + vxlan_id: id, + ready, + notify, + .. + } = lifecycle + && *id == vxlan_id + { + *ready = true; + notify.notify_waiters(); + return true; + } + } + false + } + + #[cfg(test)] + pub fn mark_active( + &self, + container: &str, + port: u16, + vxlan_id: u32, + overlay_ip: Ipv4Addr, + container_ip: Ipv4Addr, + ) { + self.mark_prepared(container, port, vxlan_id, overlay_ip, container_ip); + assert!(self.mark_ready(vxlan_id)); + } + + /// Drop only the failed caller's pending claim. Installed steering must + /// survive RPC failure so teardown can still remove the exact DNAT rule. pub fn forget_pending(&self, container: &str, port: u16, owner: &Arc) { let mut by_key = self.by_key.lock().unwrap(); let key = (container.to_string(), port); @@ -171,27 +197,27 @@ impl TriggersState { } } - /// Find the `Active` entry for `vxlan_id`, remove it, and return + /// Find the installed entry for `vxlan_id`, remove it, and return /// `(container, port, overlay_ip, container_ip)` so the caller can tear /// down DNAT with the matching `-s`. pub fn remove_by_vxlan(&self, vxlan_id: u32) -> Option<(String, u16, Ipv4Addr, Ipv4Addr)> { let mut by_key = self.by_key.lock().unwrap(); let key = by_key.iter().find_map(|((c, p), lc)| match lc { - Lifecycle::Active { vxlan_id: v, .. } if *v == vxlan_id => Some((c.clone(), *p)), + Lifecycle::Installed { vxlan_id: v, .. } if *v == vxlan_id => Some((c.clone(), *p)), _ => None, })?; // The lock is held across the `iter().find_map` and the `remove` - // below, so the removed entry is guaranteed to be the same `Active` + // below, so the removed entry is guaranteed to be the same installed // one we just matched on. A `Pending` here would mean the lock // protection was broken. match by_key.remove(&key) { - Some(Lifecycle::Active { + Some(Lifecycle::Installed { overlay_ip, container_ip, .. }) => Some((key.0, key.1, overlay_ip, container_ip)), Some(Lifecycle::Pending { .. }) | None => { - unreachable!("find_map matched Active for {key:?}; lock held across remove") + unreachable!("find_map matched Installed for {key:?}; lock held across remove") } } } @@ -199,6 +225,43 @@ impl TriggersState { #[cfg(test)] mod tests { + + #[tokio::test] + async fn installed_trigger_waits_for_both_endpoints() { + let state = TriggersState::default(); + let owner = state.mark_pending("c1", 80, IP); + let notified = owner.notified(); + tokio::pin!(notified); + assert!(!notified.as_mut().enable()); + state.mark_prepared("c1", 80, 42, OVERLAY, IP); + assert!(matches!(state.state("c1", 80), TriggerState::Pending)); + assert!(matches!( + state.claim("c1", 80, IP), + TriggerClaim::Pending(_) + )); + assert!(!notified.as_mut().enable()); + assert!(state.mark_ready(42)); + notified.await; + assert!(matches!(state.claim("c1", 80, IP), TriggerClaim::Active)); + } + + #[test] + fn prepared_steering_survives_rpc_failure_and_is_cleaned_before_activation() { + let state = TriggersState::default(); + let owner = state.mark_pending("c1", 80, IP); + state.mark_prepared("c1", 80, 42, OVERLAY, IP); + state.forget_pending("c1", 80, &owner); + assert_eq!( + state.remove_by_vxlan(42), + Some(("c1".into(), 80, OVERLAY, IP)) + ); + assert!(!state.mark_ready(42)); + state.mark_prepared("c1", 80, 43, OVERLAY, IP); + assert!(!state.mark_ready(42)); + assert!(matches!(state.state("c1", 80), TriggerState::Pending)); + assert!(state.mark_ready(43)); + } + use super::*; use std::sync::Arc; use std::time::Duration; diff --git a/members/nullnet-client/vxlan_scripts/br-teardown.sh b/members/nullnet-client/vxlan_scripts/br-teardown.sh index d96f5de..789d77e 100755 --- a/members/nullnet-client/vxlan_scripts/br-teardown.sh +++ b/members/nullnet-client/vxlan_scripts/br-teardown.sh @@ -9,4 +9,4 @@ fi BR_NAME=$1 -sudo ip link set $BR_NAME down && sudo ip link del $BR_NAME +ip link set $BR_NAME down && ip link del $BR_NAME diff --git a/members/nullnet-client/vxlan_scripts/ns-teardown.sh b/members/nullnet-client/vxlan_scripts/ns-teardown.sh index a43ade1..acb326f 100755 --- a/members/nullnet-client/vxlan_scripts/ns-teardown.sh +++ b/members/nullnet-client/vxlan_scripts/ns-teardown.sh @@ -9,9 +9,9 @@ fi NS_NAME=$1 -sudo ip link set vxlan-$NS_NAME down && sudo ip link del vxlan-$NS_NAME -sudo ip link set $NS_NAME-out down && sudo ip link del $NS_NAME-out +ip link set vxlan-$NS_NAME down && ip link del vxlan-$NS_NAME +ip link set $NS_NAME-out down && ip link del $NS_NAME-out # Delete the namespace if it exists (standalone mode). # In Docker mode there's no namespace to delete — ip netns del will simply fail silently. -sudo ip netns del $NS_NAME 2>/dev/null +ip netns del $NS_NAME 2>/dev/null diff --git a/members/nullnet-grpc-lib/proto/nullnet_grpc.proto b/members/nullnet-grpc-lib/proto/nullnet_grpc.proto index b988518..b41c945 100644 --- a/members/nullnet-grpc-lib/proto/nullnet_grpc.proto +++ b/members/nullnet-grpc-lib/proto/nullnet_grpc.proto @@ -129,9 +129,16 @@ message NetMessage { ContainerResume container_resume = 6; // Egress country-policy changed on the server (fire-and-forget) EgressPolicyChanged egress_policy_changed = 7; + NetReady net_ready = 8; } } +// Both endpoints are prepared; the initiator may release held packets. +message NetReady { + MsgId msg_id = 1; + uint32 net_id = 2; +} + // A service's egress country lists changed on a config reload. The client // drops its cached policy verdicts and flushes conntrack for its registered // containers' external flows, so every live flow re-enters the NFQUEUE as NEW diff --git a/members/nullnet-grpc-lib/src/lib.rs b/members/nullnet-grpc-lib/src/lib.rs index 9e66163..90df108 100644 --- a/members/nullnet-grpc-lib/src/lib.rs +++ b/members/nullnet-grpc-lib/src/lib.rs @@ -11,13 +11,18 @@ use crate::nullnet_grpc::{ ProxyConnectionEnd, ProxyRequest, ServiceReport, ServicesListResponse, Upstream, }; pub use proto::*; -use std::path::Path; -use tokio::sync::mpsc; +use std::{future::Future, path::Path, sync::Arc}; +use tokio::sync::{Semaphore, mpsc}; use tonic::Request; pub use tonic::Streaming; use tonic::codegen::tokio_stream::wrappers::ReceiverStream; use tonic::transport::{Channel, ClientTlsConfig}; +// Bound concurrent RPC work, including cold tunnel creation. Removing this +// limit overloads setup during large bursts even with corrected h2 accounting. +const MAX_IN_FLIGHT_UNARY: usize = 32; +const MAX_IN_FLIGHT_LIFECYCLE: usize = 8; + /// Why a `proxy` lookup failed. #[derive(Debug)] pub enum ProxyLookupError { @@ -31,6 +36,8 @@ pub enum ProxyLookupError { #[derive(Clone)] pub struct NullnetGrpcInterface { client: NullnetGrpcClient, + unary_slots: Arc, + lifecycle_slots: Arc, } impl NullnetGrpcInterface { @@ -60,6 +67,8 @@ impl NullnetGrpcInterface { if let Ok(channel) = endpoint.connect().await { return Ok(Self { client: NullnetGrpcClient::new(channel), + unary_slots: Arc::new(Semaphore::new(MAX_IN_FLIGHT_UNARY)), + lifecycle_slots: Arc::new(Semaphore::new(MAX_IN_FLIGHT_LIFECYCLE)), }); } @@ -71,11 +80,35 @@ impl NullnetGrpcInterface { } } + // Streams stay independent so setup acknowledgements can always progress. + async fn unary( + &self, + request: impl Future, tonic::Status>>, + ) -> Result, tonic::Status> { + let _permit = self + .unary_slots + .acquire() + .await + .expect("RPC limiter stays open"); + request.await + } + + // Renewals and closes must progress while cold setups occupy unary slots. + async fn lifecycle( + &self, + request: impl Future, tonic::Status>>, + ) -> Result, tonic::Status> { + let _permit = self + .lifecycle_slots + .acquire() + .await + .expect("RPC limiter stays open"); + request.await + } + #[allow(clippy::missing_errors_doc)] pub async fn network_type(&self) -> Result { - self.client - .clone() - .network_type(Request::new(Empty {})) + self.unary(self.client.clone().network_type(Request::new(Empty {}))) .await .map(tonic::Response::into_inner) .map_err(|e| e.to_string()) @@ -99,9 +132,7 @@ impl NullnetGrpcInterface { #[allow(clippy::missing_errors_doc)] pub async fn proxy(&self, message: ProxyRequest) -> Result { - self.client - .clone() - .proxy(Request::new(message)) + self.unary(self.client.clone().proxy(Request::new(message))) .await .map(tonic::Response::into_inner) .map_err(|e| { @@ -120,12 +151,14 @@ impl NullnetGrpcInterface { /// pin the edge until the node disconnects. #[allow(clippy::missing_errors_doc)] pub async fn proxy_connection_closed(&self, message: ProxyConnectionEnd) -> Result<(), String> { - self.client - .clone() - .proxy_connection_closed(Request::new(message)) - .await - .map(|_| ()) - .map_err(|e| e.to_string()) + self.lifecycle( + self.client + .clone() + .proxy_connection_closed(Request::new(message)), + ) + .await + .map(|_| ()) + .map_err(|e| e.to_string()) } /// Report an egress open-connection transition (0<->1) for one container. @@ -135,15 +168,17 @@ impl NullnetGrpcInterface { initiator_container: String, active: bool, ) -> Result<(), String> { - self.client - .clone() - .egress_liveness(Request::new(EgressLivenessReport { - initiator_container, - active, - })) - .await - .map(|_| ()) - .map_err(|e| e.to_string()) + self.lifecycle( + self.client + .clone() + .egress_liveness(Request::new(EgressLivenessReport { + initiator_container, + active, + })), + ) + .await + .map(|_| ()) + .map_err(|e| e.to_string()) } /// Report a trigger-built chain's open-connection transition (0<->1). @@ -158,17 +193,19 @@ impl NullnetGrpcInterface { initiator_container: String, active: bool, ) -> Result<(), String> { - self.client - .clone() - .backend_liveness(Request::new(BackendLivenessReport { - service_name, - port, - initiator_container, - active, - })) - .await - .map(|_| ()) - .map_err(|e| e.to_string()) + self.lifecycle( + self.client + .clone() + .backend_liveness(Request::new(BackendLivenessReport { + service_name, + port, + initiator_container, + active, + })), + ) + .await + .map(|_| ()) + .map_err(|e| e.to_string()) } #[allow(clippy::missing_errors_doc)] @@ -176,9 +213,7 @@ impl NullnetGrpcInterface { &self, message: ServiceReport, ) -> Result { - self.client - .clone() - .services_list(Request::new(message)) + self.unary(self.client.clone().services_list(Request::new(message))) .await .map(tonic::Response::into_inner) .map_err(|e| e.to_string()) @@ -191,16 +226,18 @@ impl NullnetGrpcInterface { port: u32, initiator_container: String, ) -> Result<(), String> { - self.client - .clone() - .backend_trigger(Request::new(BackendTriggerRequest { - service_name, - port, - initiator_container, - })) - .await - .map(|_| ()) - .map_err(|e| e.to_string()) + self.unary( + self.client + .clone() + .backend_trigger(Request::new(BackendTriggerRequest { + service_name, + port, + initiator_container, + })), + ) + .await + .map(|_| ()) + .map_err(|e| e.to_string()) } /// Fire an egress trigger: the sending host observed a registered service @@ -214,17 +251,19 @@ impl NullnetGrpcInterface { dst_ip: String, dst_port: u32, ) -> Result<(), String> { - self.client - .clone() - .egress_trigger(Request::new(EgressTriggerRequest { - service_name, - initiator_container, - dst_ip, - dst_port, - })) - .await - .map(|_| ()) - .map_err(|e| e.to_string()) + self.unary( + self.client + .clone() + .egress_trigger(Request::new(EgressTriggerRequest { + service_name, + initiator_container, + dst_ip, + dst_port, + })), + ) + .await + .map(|_| ()) + .map_err(|e| e.to_string()) } #[allow(clippy::missing_errors_doc)] @@ -232,12 +271,14 @@ impl NullnetGrpcInterface { &self, entries: Vec, ) -> Result<(), String> { - self.client - .clone() - .report_egress_destination(Request::new(EgressDestinationReport { entries })) - .await - .map(|_| ()) - .map_err(|e| e.to_string()) + self.unary( + self.client + .clone() + .report_egress_destination(Request::new(EgressDestinationReport { entries })), + ) + .await + .map(|_| ()) + .map_err(|e| e.to_string()) } /// Ask whether the egress country policy allows `initiator_container` to @@ -248,15 +289,17 @@ impl NullnetGrpcInterface { initiator_container: String, dst_ip: String, ) -> Result { - self.client - .clone() - .check_egress_destination(Request::new(EgressPolicyCheck { - initiator_container, - dst_ip, - })) - .await - .map(|resp| resp.into_inner().allowed) - .map_err(|e| e.to_string()) + self.unary( + self.client + .clone() + .check_egress_destination(Request::new(EgressPolicyCheck { + initiator_container, + dst_ip, + })), + ) + .await + .map(|resp| resp.into_inner().allowed) + .map_err(|e| e.to_string()) } /// Ask whether the ingress country policy allows an external client at @@ -268,22 +311,22 @@ impl NullnetGrpcInterface { service_name: String, client_ip: String, ) -> Result { - self.client - .clone() - .check_ingress(Request::new(IngressPolicyCheck { - service_name, - client_ip, - })) - .await - .map(|resp| resp.into_inner().allowed) - .map_err(|e| e.to_string()) + self.unary( + self.client + .clone() + .check_ingress(Request::new(IngressPolicyCheck { + service_name, + client_ip, + })), + ) + .await + .map(|resp| resp.into_inner().allowed) + .map_err(|e| e.to_string()) } #[allow(clippy::missing_errors_doc)] pub async fn report_event(&self, event: AgentEvent) -> Result<(), String> { - self.client - .clone() - .report_event(Request::new(event)) + self.unary(self.client.clone().report_event(Request::new(event))) .await .map(|_| ()) .map_err(|e| e.to_string()) @@ -330,3 +373,74 @@ impl NullnetGrpcInterface { .into_inner()) } } + +#[cfg(test)] +mod admission_tests { + use super::*; + + #[tokio::test] + async fn liveness_progresses_when_setup_admission_is_full() { + let interface = NullnetGrpcInterface { + client: NullnetGrpcClient::new( + Channel::from_static("http://127.0.0.1:1").connect_lazy(), + ), + unary_slots: Arc::new(Semaphore::new(MAX_IN_FLIGHT_UNARY)), + lifecycle_slots: Arc::new(Semaphore::new(MAX_IN_FLIGHT_LIFECYCLE)), + }; + let _busy = interface + .unary_slots + .acquire_many(MAX_IN_FLIGHT_UNARY as u32) + .await + .unwrap(); + let result = tokio::time::timeout( + std::time::Duration::from_millis(200), + interface.backend_liveness("backend".into(), 80, "container".into(), true), + ) + .await; + assert!( + result.is_ok(), + "liveness was never admitted while setup occupied the slots" + ); + assert!( + result.unwrap().is_err(), + "the intentionally absent server should reject the call" + ); + } + + #[tokio::test] + async fn clones_share_admission_and_cancellation_releases_capacity() { + let interface = NullnetGrpcInterface { + client: NullnetGrpcClient::new( + Channel::from_static("http://127.0.0.1:1").connect_lazy(), + ), + unary_slots: Arc::new(Semaphore::new(2)), + lifecycle_slots: Arc::new(Semaphore::new(MAX_IN_FLIGHT_LIFECYCLE)), + }; + let (entered, mut received) = mpsc::unbounded_channel(); + let mut calls = tokio::task::JoinSet::new(); + for _ in 0..3 { + let interface = interface.clone(); + let entered = entered.clone(); + calls.spawn(async move { + interface + .unary(async { + entered.send(()).unwrap(); + std::future::pending::, tonic::Status>>() + .await + }) + .await + }); + } + received.recv().await.unwrap(); + received.recv().await.unwrap(); + assert_eq!(interface.unary_slots.available_permits(), 0); + assert!(received.try_recv().is_err()); + calls.shutdown().await; + assert_eq!(interface.unary_slots.available_permits(), 2); + let result: Result, _> = interface + .unary(async { Err(tonic::Status::unavailable("disconnected")) }) + .await; + assert!(result.is_err()); + assert_eq!(interface.unary_slots.available_permits(), 2); + } +} diff --git a/members/nullnet-grpc-lib/src/proto/nullnet_grpc.rs b/members/nullnet-grpc-lib/src/proto/nullnet_grpc.rs index da0cdab..1064e0f 100644 --- a/members/nullnet-grpc-lib/src/proto/nullnet_grpc.rs +++ b/members/nullnet-grpc-lib/src/proto/nullnet_grpc.rs @@ -21,7 +21,7 @@ pub struct NetType { } #[derive(Clone, PartialEq, Eq, Hash, ::prost::Message)] pub struct NetMessage { - #[prost(oneof = "net_message::Message", tags = "1, 2, 3, 4, 5, 6, 7")] + #[prost(oneof = "net_message::Message", tags = "1, 2, 3, 4, 5, 6, 7, 8")] pub message: ::core::option::Option, } /// Nested message and enum types in `NetMessage`. @@ -46,8 +46,18 @@ pub mod net_message { /// Egress country-policy changed on the server (fire-and-forget) #[prost(message, tag = "7")] EgressPolicyChanged(super::EgressPolicyChanged), + #[prost(message, tag = "8")] + NetReady(super::NetReady), } } +/// Both endpoints are prepared; the initiator may release held packets. +#[derive(Clone, PartialEq, Eq, Hash, ::prost::Message)] +pub struct NetReady { + #[prost(message, optional, tag = "1")] + pub msg_id: ::core::option::Option, + #[prost(uint32, tag = "2")] + pub net_id: u32, +} /// A service's egress country lists changed on a config reload. The client /// drops its cached policy verdicts and flushes conntrack for its registered /// containers' external flows, so every live flow re-enters the NFQUEUE as NEW diff --git a/members/nullnet-server/src/db/events.rs b/members/nullnet-server/src/db/events.rs index 6386b3e..24944a6 100644 --- a/members/nullnet-server/src/db/events.rs +++ b/members/nullnet-server/src/db/events.rs @@ -8,6 +8,13 @@ use nullnet_liberror::{Error, ErrorHandler, Location, location}; use std::sync::Arc; use tokio::sync::Mutex; +pub(crate) struct EventInsert { + pub(crate) kind: &'static str, + pub(crate) severity: &'static str, + pub(crate) timestamp: i64, + pub(crate) payload: String, +} + /// Typed access to the `events` table: durable storage for `crate::events::Event`, /// with time-based deletion so volume never grows unbounded (see `events_retention.rs`). pub(crate) struct EventRepository { @@ -19,6 +26,29 @@ impl EventRepository { Self { conn } } + pub(crate) async fn insert_batch(&self, rows: Vec) -> Result<(), Error> { + let mut conn = self.conn.lock().await; + conn.spawn_blocking(move |conn| { + conn.transaction::<(), diesel::result::Error, _>(|conn| { + for row in &rows { + let values = NewEventRow { + kind: row.kind, + severity: row.severity, + timestamp: row.timestamp, + payload: &row.payload, + }; + diesel::RunQueryDsl::execute( + diesel::insert_into(events::table).values(values), + conn, + )?; + } + Ok(()) + }) + }) + .await + .handle_err(location!()) + } + pub(crate) async fn insert( &self, kind: &str, @@ -198,4 +228,194 @@ mod tests { assert_eq!(remaining.len(), 1); assert_eq!(remaining[0].timestamp, 500); } + + #[tokio::test] + async fn a_failed_batch_rolls_back_every_row() { + use super::*; + let db = test_db().await; + let repo = db.events(); + diesel::sql_query("CREATE TRIGGER reject_event BEFORE INSERT ON events WHEN NEW.payload = 'reject' BEGIN SELECT RAISE(ABORT, 'injected write failure'); END;") + .execute(&mut *repo.conn.lock().await).await.unwrap(); + let rows = vec![ + EventInsert { + kind: "node_connected", + severity: "info", + timestamp: 100, + payload: "{}".into(), + }, + EventInsert { + kind: "node_connected", + severity: "info", + timestamp: 100, + payload: "reject".into(), + }, + ]; + assert!(repo.insert_batch(rows).await.is_err()); + assert!( + repo.query(None, None, None, None, None, 10) + .await + .unwrap() + .is_empty() + ); + } + + #[tokio::test] + async fn event_enqueue_does_not_wait_for_storage_and_shutdown_drains() { + use crate::events::{Event, EventStore}; + let db = test_db().await; + let store = EventStore::new(); + store.attach_db(db.clone()); + let mut live = store.subscribe(); + let repo = db.events(); + let guard = repo.conn.lock().await; + for i in 0..512 { + store + .emit(Event::node_connected(format!("192.0.2.{i}"))) + .await; + } + assert!(live.try_recv().is_err()); + drop(guard); + store.flush().await; + assert_eq!( + repo.query(None, None, None, None, None, 1000) + .await + .unwrap() + .len(), + 512 + ); + store + .emit(Event::node_connected("198.51.100.1".into())) + .await; + store.shutdown().await; + let rows = repo + .query(None, None, None, None, None, 1000) + .await + .unwrap(); + assert_eq!(rows.len(), 513); + assert!(rows[0].payload.contains("198.51.100.1")); + } + + #[tokio::test] + async fn unavailable_event_storage_does_not_block_producers() { + use super::*; + use crate::events::{Event, EventStore}; + let db = test_db().await; + let repo = db.events(); + diesel::sql_query("CREATE TRIGGER reject_event BEFORE INSERT ON events BEGIN SELECT RAISE(ABORT, 'injected persistent failure'); END;") + .execute(&mut *repo.conn.lock().await).await.unwrap(); + let store = EventStore::new(); + store.attach_db(db.clone()); + let mut live = store.subscribe(); + store.emit(Event::node_connected("sentinel".into())).await; + assert!(matches!( + live.recv().await.unwrap(), + Event::PersistenceFailed { .. } + )); + let result = tokio::time::timeout(std::time::Duration::from_secs(2), async { + for i in 0..6000 { + store.emit(Event::node_connected(format!("test-{i}"))).await; + } + }) + .await; + diesel::sql_query("DROP TRIGGER reject_event") + .execute(&mut *repo.conn.lock().await) + .await + .unwrap(); + store.shutdown().await; + assert!( + result.is_ok(), + "event persistence failure blocked its producers" + ); + let rows = repo + .query(None, None, None, None, None, 10_000) + .await + .unwrap(); + let persisted = rows.iter().filter(|r| r.kind == "node_connected").count() as u64; + let lost: u64 = rows + .iter() + .filter(|r| r.kind == "event_persistence_overflow") + .map(|r| { + serde_json::from_str::(&r.payload).unwrap()["dropped_events"] + .as_u64() + .unwrap() + }) + .sum(); + assert!(lost > 0); + assert_eq!( + persisted + lost, + 6001, + "every accepted or dropped event must be accounted for" + ); + } + + #[tokio::test] + async fn event_shutdown_is_bounded_during_persistent_failure() { + use super::*; + use crate::events::{Event, EventStore}; + let db = test_db().await; + let repo = db.events(); + diesel::sql_query("CREATE TRIGGER reject_event BEFORE INSERT ON events BEGIN SELECT RAISE(ABORT, 'injected persistent failure'); END;") + .execute(&mut *repo.conn.lock().await).await.unwrap(); + let store = EventStore::new(); + store.attach_db(db); + store.emit(Event::node_connected("sentinel".into())).await; + tokio::time::timeout(std::time::Duration::from_secs(6), store.shutdown()) + .await + .unwrap(); + } + + #[tokio::test] + async fn event_writer_retries_without_losing_events_and_reports_recovery() { + use super::*; + use crate::events::{Event, EventStore}; + let db = test_db().await; + let repo = db.events(); + diesel::sql_query("CREATE TRIGGER reject_event BEFORE INSERT ON events BEGIN SELECT RAISE(ABORT, 'injected write failure'); END;") + .execute(&mut *repo.conn.lock().await).await.unwrap(); + let store = EventStore::new(); + store.attach_db(db.clone()); + let mut live = store.subscribe(); + store.emit(Event::node_connected("192.0.2.1".into())).await; + let failure = tokio::time::timeout(std::time::Duration::from_secs(5), live.recv()) + .await + .unwrap() + .unwrap(); + assert!(matches!(failure, Event::PersistenceFailed { .. })); + assert!( + repo.query(None, None, None, None, None, 10) + .await + .unwrap() + .is_empty() + ); + store.emit(Event::node_connected("192.0.2.2".into())).await; + diesel::sql_query("DROP TRIGGER reject_event") + .execute(&mut *repo.conn.lock().await) + .await + .unwrap(); + tokio::time::timeout(std::time::Duration::from_secs(5), store.shutdown()) + .await + .unwrap(); + let rows = repo.query(None, None, None, None, None, 10).await.unwrap(); + assert_eq!(rows.len(), 4); + for row in &rows { + let payload: serde_json::Value = serde_json::from_str(&row.payload).unwrap(); + assert_eq!(payload["type"], row.kind); + } + assert_eq!( + rows.iter().filter(|r| r.kind == "node_connected").count(), + 2 + ); + assert_eq!( + rows.iter() + .filter(|r| r.kind == "event_persistence_failed") + .count(), + 1 + ); + assert_eq!( + rows.iter() + .filter(|r| r.kind == "event_persistence_recovered") + .count(), + 1 + ); + } } diff --git a/members/nullnet-server/src/db/mod.rs b/members/nullnet-server/src/db/mod.rs index a1cdd4e..3cd0c08 100644 --- a/members/nullnet-server/src/db/mod.rs +++ b/members/nullnet-server/src/db/mod.rs @@ -31,7 +31,7 @@ mod user_scopes; mod users; pub(crate) use certs::CertRepository; -pub(crate) use events::EventRepository; +pub(crate) use events::{EventInsert, EventRepository}; pub(crate) use login_attempts::LoginAttemptRepository; /// Only the session-history tests read rows back as a typed struct; production /// code goes through `SessionStore`, which hands out `SessionRecordJson`. @@ -146,6 +146,11 @@ impl Db { pub(crate) fn sessions(&self) -> SessionRepository { SessionRepository::new(self.conn.clone()) } + + #[cfg(test)] + pub(crate) async fn hold_connection(&self) -> impl Send + '_ { + self.conn.lock().await + } } /// Unix seconds, for `updated_at` columns. diff --git a/members/nullnet-server/src/db/models.rs b/members/nullnet-server/src/db/models.rs index 6a4ecf8..a62cc41 100644 --- a/members/nullnet-server/src/db/models.rs +++ b/members/nullnet-server/src/db/models.rs @@ -86,17 +86,17 @@ pub(crate) struct ServiceRow { #[derive(Insertable, Debug, Clone)] #[diesel(table_name = services)] #[diesel(check_for_backend(diesel::sqlite::Sqlite))] -pub(crate) struct NewServiceRow<'a> { - pub(crate) stack: &'a str, - pub(crate) name: &'a str, - pub(crate) docker_container: Option<&'a str>, - pub(crate) process_path: Option<&'a str>, - pub(crate) host_ip: Option<&'a str>, +pub(crate) struct NewServiceRow { + pub(crate) stack: String, + pub(crate) name: String, + pub(crate) docker_container: Option, + pub(crate) process_path: Option, + pub(crate) host_ip: Option, pub(crate) pausable: bool, pub(crate) port: Option, pub(crate) timeout: Option, pub(crate) max_networks: Option, - pub(crate) protocol: Option<&'a str>, + pub(crate) protocol: Option, pub(crate) listen_port: Option, pub(crate) egress_filter: Option, pub(crate) ingress_filter: Option, @@ -161,14 +161,14 @@ pub(crate) struct RouteRow { #[derive(Insertable, Debug, Clone)] #[diesel(table_name = routes)] #[diesel(check_for_backend(diesel::sqlite::Sqlite))] -pub(crate) struct NewRouteRow<'a> { - pub(crate) stack: &'a str, - pub(crate) host: &'a str, - pub(crate) path: &'a str, - pub(crate) target_kind: &'a str, - pub(crate) target_service: Option<&'a str>, +pub(crate) struct NewRouteRow { + pub(crate) stack: String, + pub(crate) host: String, + pub(crate) path: String, + pub(crate) target_kind: String, + pub(crate) target_service: Option, pub(crate) strip_prefix: bool, - pub(crate) redirect_to: Option<&'a str>, + pub(crate) redirect_to: Option, pub(crate) redirect_status: Option, pub(crate) preserve_path: bool, pub(crate) preserve_query: bool, diff --git a/members/nullnet-server/src/db/stacks.rs b/members/nullnet-server/src/db/stacks.rs index fe237f2..ca5eb92 100644 --- a/members/nullnet-server/src/db/stacks.rs +++ b/members/nullnet-server/src/db/stacks.rs @@ -46,13 +46,7 @@ pub(crate) struct RouteInsert<'a> { pub(crate) preserve_query: bool, } -/// Typed access to a stack's normalized config: `stacks` (existence), -/// `services`/`service_triggers`/`service_dependencies`, and `routes`. A -/// stack is the atomic unit the rest of the app always loads/saves as a -/// whole (mirrors `ParsedStack`/the whole-list-replace HTTP contract), so -/// `put_services`/`put_routes` each replace their stack's full list in one -/// held connection lock — the same "acquire once, do several statements" -/// pattern `RefreshTokenRepository::rotate` already uses. +/// Atomic replacement of a stack's services, routes, or both. pub(crate) struct StackRepository { conn: Arc>, } @@ -83,25 +77,6 @@ impl StackRepository { Ok(found.is_some()) } - /// Insert the stack if it's new, otherwise just bump `updated_at` — this - /// runs on every services/routes save (not only on first creation), so - /// it doubles as the stack's "last modified" timestamp. - async fn ensure_exists(conn: &mut AsyncSqlite, stack: &str) -> Result<(), Error> { - let new_row = NewStack { - name: stack, - updated_at: super::now(), - }; - diesel::insert_into(stacks::table) - .values(&new_row) - .on_conflict(stacks::name) - .do_update() - .set(&new_row) - .execute(conn) - .await - .handle_err(location!())?; - Ok(()) - } - pub(crate) async fn services_for(&self, stack: &str) -> Result, Error> { let mut conn = self.conn.lock().await; services::table @@ -148,107 +123,139 @@ impl StackRepository { .handle_err(location!()) } - /// Whole-list replace: ensure the stack exists (so this also doubles as - /// "create a stack"), delete its current `services` rows (cascades - /// their `service_triggers`/`service_dependencies` rows), and insert - /// `new_services`. pub(crate) async fn put_services( &self, stack: &str, new_services: &[ServiceInsert<'_>], ) -> Result<(), Error> { - let mut conn = self.conn.lock().await; - Self::ensure_exists(&mut conn, stack).await?; - diesel::delete(services::table.filter(services::stack.eq(stack))) - .execute(&mut *conn) + self.put_configuration(stack, Some(new_services), None) .await - .handle_err(location!())?; - - for s in new_services { - let new_row = NewServiceRow { - stack, - name: s.name, - docker_container: s.docker_container, - process_path: s.process_path, - host_ip: s.host_ip, - pausable: s.pausable, - port: s.port, - timeout: s.timeout, - max_networks: s.max_networks, - protocol: s.protocol, - listen_port: s.listen_port, - egress_filter: s.egress_filter.clone(), - ingress_filter: s.ingress_filter.clone(), - }; - let service_id: i32 = diesel::insert_into(services::table) - .values(&new_row) - .returning(services::id) - .get_result(&mut *conn) - .await - .handle_err(location!())?; - - // SQLite doesn't support diesel's multi-row batch insert (a - // single INSERT with several VALUES tuples) the way Postgres - // does, so each row is its own statement — fine at this scale - // (a handful of triggers/branches per service, an admin-time - // operation, not a hot path). - for peer in &s.backends { - diesel::insert_into(service_triggers::table) - .values(NewServiceTriggerRow { - service_id, - peer: peer.clone(), - }) - .execute(&mut *conn) - .await - .handle_err(location!())?; - } - for (i, chain) in s.dependencies.iter().enumerate() { - diesel::insert_into(service_dependencies::table) - .values(NewServiceDependencyRow { - service_id, - branch_index: i32::try_from(i).unwrap_or(i32::MAX), - chain: chain.clone(), - }) - .execute(&mut *conn) - .await - .handle_err(location!())?; - } - } - Ok(()) } - /// Whole-list replace for `routes`, same shape as `put_services`. pub(crate) async fn put_routes( &self, stack: &str, new_routes: &[RouteInsert<'_>], ) -> Result<(), Error> { - let mut conn = self.conn.lock().await; - Self::ensure_exists(&mut conn, stack).await?; - diesel::delete(routes::table.filter(routes::stack.eq(stack))) - .execute(&mut *conn) - .await - .handle_err(location!())?; + self.put_configuration(stack, None, Some(new_routes)).await + } - for r in new_routes { - diesel::insert_into(routes::table) - .values(NewRouteRow { - stack, - host: r.host, - path: r.path, - target_kind: r.target_kind, - target_service: r.target_service, + pub(crate) async fn put_configuration( + &self, + stack: &str, + new_services: Option<&[ServiceInsert<'_>]>, + new_routes: Option<&[RouteInsert<'_>]>, + ) -> Result<(), Error> { + let service_rows = new_services.map(|items| { + items + .iter() + .map(|s| { + ( + NewServiceRow { + stack: stack.to_string(), + name: s.name.to_string(), + docker_container: s.docker_container.map(str::to_string), + process_path: s.process_path.map(str::to_string), + host_ip: s.host_ip.map(str::to_string), + pausable: s.pausable, + port: s.port, + timeout: s.timeout, + max_networks: s.max_networks, + protocol: s.protocol.map(str::to_string), + listen_port: s.listen_port, + egress_filter: s.egress_filter.clone(), + ingress_filter: s.ingress_filter.clone(), + }, + s.backends.clone(), + s.dependencies.clone(), + ) + }) + .collect::>() + }); + let route_rows = new_routes.map(|items| { + items + .iter() + .map(|r| NewRouteRow { + stack: stack.to_string(), + host: r.host.to_string(), + path: r.path.to_string(), + target_kind: r.target_kind.to_string(), + target_service: r.target_service.map(str::to_string), strip_prefix: r.strip_prefix, - redirect_to: r.redirect_to, + redirect_to: r.redirect_to.map(str::to_string), redirect_status: r.redirect_status, preserve_path: r.preserve_path, preserve_query: r.preserve_query, }) - .execute(&mut *conn) - .await - .handle_err(location!())?; - } - Ok(()) + .collect::>() + }); + let stack = stack.to_string(); + let mut conn = self.conn.lock().await; + // One blocking transaction also completes safely if the caller disconnects. + conn.spawn_blocking(move |conn| { + conn.transaction::<(), diesel::result::Error, _>(|conn| { + let row = NewStack { + name: &stack, + updated_at: super::now(), + }; + diesel::RunQueryDsl::execute( + diesel::insert_into(stacks::table) + .values(&row) + .on_conflict(stacks::name) + .do_update() + .set(&row), + conn, + )?; + if let Some(items) = service_rows { + diesel::RunQueryDsl::execute( + diesel::delete(services::table.filter(services::stack.eq(&stack))), + conn, + )?; + for (row, backends, dependencies) in items { + let service_id: i32 = diesel::RunQueryDsl::get_result( + diesel::insert_into(services::table) + .values(row) + .returning(services::id), + conn, + )?; + for peer in backends { + diesel::RunQueryDsl::execute( + diesel::insert_into(service_triggers::table) + .values(NewServiceTriggerRow { service_id, peer }), + conn, + )?; + } + for (i, chain) in dependencies.into_iter().enumerate() { + diesel::RunQueryDsl::execute( + diesel::insert_into(service_dependencies::table).values( + NewServiceDependencyRow { + service_id, + branch_index: i32::try_from(i).unwrap_or(i32::MAX), + chain, + }, + ), + conn, + )?; + } + } + } + if let Some(items) = route_rows { + diesel::RunQueryDsl::execute( + diesel::delete(routes::table.filter(routes::stack.eq(&stack))), + conn, + )?; + for row in items { + diesel::RunQueryDsl::execute( + diesel::insert_into(routes::table).values(row), + conn, + )?; + } + } + Ok(()) + }) + }) + .await + .handle_err(location!()) } /// Delete the stack; cascades to its services (and their @@ -300,6 +307,65 @@ mod tests { } } + #[tokio::test] + async fn failed_service_replace_preserves_previous_configuration() { + let db = test_db().await; + let repo = db.stacks(); + repo.put_services("alpha", &[service("original")]) + .await + .unwrap(); + let original = repo.services_for("alpha").await.unwrap()[0].id; + assert!( + repo.put_services("alpha", &[service("duplicate"), service("duplicate")]) + .await + .is_err() + ); + let rows = repo.services_for("alpha").await.unwrap(); + assert_eq!(rows.len(), 1); + assert_eq!(rows[0].id, original); + assert_eq!(rows[0].name, "original"); + assert_eq!(repo.triggers_for(&[original]).await.unwrap().len(), 1); + assert_eq!(repo.dependencies_for(&[original]).await.unwrap().len(), 1); + } + + #[tokio::test] + async fn failed_import_rolls_back_services_and_routes_together() { + let db = test_db().await; + let repo = db.stacks(); + repo.put_configuration( + "alpha", + Some(&[service("web")]), + Some(&[route("old.example.com")]), + ) + .await + .unwrap(); + let original = repo.services_for("alpha").await.unwrap()[0].id; + assert!( + repo.put_configuration( + "alpha", + Some(&[service("replacement")]), + Some(&[ + route("duplicate.example.com"), + route("duplicate.example.com") + ]), + ) + .await + .is_err() + ); + assert_eq!(repo.services_for("alpha").await.unwrap()[0].id, original); + assert_eq!( + repo.routes_for("alpha").await.unwrap()[0].host, + "old.example.com" + ); + assert_eq!(repo.triggers_for(&[original]).await.unwrap().len(), 1); + assert!( + repo.put_services("new", &[service("duplicate"), service("duplicate")]) + .await + .is_err() + ); + assert!(!repo.exists("new").await.unwrap()); + } + #[tokio::test] async fn put_services_round_trips_service_triggers_and_dependencies() { let db = test_db().await; @@ -345,13 +411,9 @@ mod tests { assert_eq!(triggers.len(), 1); // re-created for the new row, not doubled } - #[tokio::test] - async fn put_routes_round_trips_and_replaces() { - let db = test_db().await; - let repo = db.stacks(); - - let route = RouteInsert { - host: "ops.example.com", + fn route(host: &str) -> RouteInsert<'_> { + RouteInsert { + host, path: "/", target_kind: "service", target_service: Some("web"), @@ -360,7 +422,14 @@ mod tests { redirect_status: None, preserve_path: false, preserve_query: false, - }; + } + } + + #[tokio::test] + async fn put_routes_round_trips_and_replaces() { + let db = test_db().await; + let repo = db.stacks(); + let route = route("ops.example.com"); repo.put_routes("alpha", &[route]).await.unwrap(); let routes = repo.routes_for("alpha").await.unwrap(); assert_eq!(routes.len(), 1); diff --git a/members/nullnet-server/src/events.rs b/members/nullnet-server/src/events.rs index 3c15123..d69c27e 100644 --- a/members/nullnet-server/src/events.rs +++ b/members/nullnet-server/src/events.rs @@ -1,3 +1,5 @@ +mod persistence; + use std::sync::{Arc, OnceLock}; use std::time::{SystemTime, UNIX_EPOCH}; @@ -42,6 +44,22 @@ pub(crate) struct EventEnvelope<'a> { #[derive(Debug, Clone, Serialize)] #[serde(tag = "type", rename_all = "snake_case")] pub(crate) enum Event { + #[serde(rename = "event_persistence_overflow")] + PersistenceOverflow { + dropped_events: u64, + timestamp: u64, + }, + #[serde(rename = "event_persistence_failed")] + PersistenceFailed { + error_message: String, + queued_events: usize, + timestamp: u64, + }, + #[serde(rename = "event_persistence_recovered")] + PersistenceRecovered { + queued_events: usize, + timestamp: u64, + }, NodeConnected { ip: String, timestamp: u64, @@ -124,9 +142,8 @@ pub(crate) enum Event { client_ip: String, timestamp: u64, }, - /// An endpoint never confirmed a teardown, so the net id went back to the - /// pool unverified. Its kernel state may still exist on that node, and a - /// later edge reusing the id would collide with it. + /// Cleanup exceeded its deadline. Keep the id reserved until completion + /// or disconnect rather than colliding with the endpoint's old state. NetTeardownUnconfirmed { net_id: u32, node_ip: String, @@ -513,6 +530,9 @@ pub(crate) enum Event { impl Event { pub(crate) fn kind(&self) -> &'static str { match self { + Self::PersistenceOverflow { .. } => "event_persistence_overflow", + Self::PersistenceFailed { .. } => "event_persistence_failed", + Self::PersistenceRecovered { .. } => "event_persistence_recovered", Self::NodeConnected { .. } => "node_connected", Self::NodeDisconnected { .. } => "node_disconnected", Self::ServiceRegistered { .. } => "service_registered", @@ -606,7 +626,8 @@ impl Event { Severity::Error } } - Self::NodeConnected { .. } + Self::PersistenceRecovered { .. } + | Self::NodeConnected { .. } | Self::ServiceRegistered { .. } | Self::SetupStarted { .. } | Self::SetupAck { .. } @@ -642,7 +663,9 @@ impl Event { | Self::ProxyClientNotInet { .. } | Self::ProxyDisconnected { .. } => Severity::Warning, - Self::SetupTimeout { .. } + Self::PersistenceOverflow { .. } + | Self::PersistenceFailed { .. } + | Self::SetupTimeout { .. } | Self::EdgePromotionLost { .. } | Self::ChainOwnerLost { .. } | Self::NetTeardownUnconfirmed { .. } @@ -1444,22 +1467,18 @@ pub(crate) struct EventPage { pub(crate) next_before_id: Option, } -/// Shared event store: durably backed by the `events` DB table (pruned on a -/// retention timer — see `events_retention.rs`), plus a broadcast channel for -/// live SSE subscribers. `db` is filled in once via [`Self::attach_db`] — -/// the many in-process unit tests that build an `Orchestrator` directly never -/// call it, so their events still broadcast live but aren't persisted, which -/// is all those tests need. +/// Bounded persistence queue with batched commits and live SSE after commit. +/// Unattached stores only broadcast, as used by in-process orchestrator tests. #[derive(Clone)] pub(crate) struct EventStore { - db: Arc>, + writer: Arc>, tx: broadcast::Sender, } impl std::fmt::Debug for EventStore { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("EventStore") - .field("db_attached", &self.db.get().is_some()) + .field("db_attached", &self.writer.get().is_some()) .finish() } } @@ -1468,42 +1487,36 @@ impl EventStore { pub(crate) fn new() -> Self { let (tx, _) = broadcast::channel(512); Self { - db: Arc::new(OnceLock::new()), + writer: Arc::new(OnceLock::new()), tx, } } - /// Wire in DB-backed persistence. A no-op after the first call. + /// Attach one bounded, batching writer shared by every store clone. pub(crate) fn attach_db(&self, db: Db) { - let _ = self.db.set(db); + self.writer + .get_or_init(|| persistence::Writer::start(db, self.tx.clone())); } pub(crate) async fn emit(&self, event: Event) { - if let Some(db) = self.db.get() { - let kind = event.kind(); - match serde_json::to_value(&event) { - Ok(payload) => { - let timestamp = payload - .get("timestamp") - .and_then(serde_json::Value::as_i64) - .unwrap_or_else(|| now_secs() as i64); - if let Err(e) = db - .events() - .insert( - kind, - event.severity().as_str(), - timestamp, - &payload.to_string(), - ) - .await - { - eprintln!("Failed to persist event '{kind}': {e:?}"); - } - } - Err(e) => eprintln!("Failed to serialize event '{kind}' for persistence: {e:#}"), - } + if let Some(writer) = self.writer.get() { + writer.emit(event).await; + } else { + let _ = self.tx.send(event); + } + } + + pub(crate) async fn shutdown(&self) { + if let Some(writer) = self.writer.get() { + writer.shutdown().await; + } + } + + #[cfg(test)] + pub(crate) async fn flush(&self) { + if let Some(writer) = self.writer.get() { + writer.flush().await; } - let _ = self.tx.send(event); } /// Most-recent-first page of persisted events, optionally filtered by @@ -1519,14 +1532,15 @@ impl EventStore { before_id: Option, limit: i64, ) -> EventPage { - let Some(db) = self.db.get() else { + let Some(writer) = self.writer.get() else { return EventPage { events: vec![], next_before_id: None, }; }; let severity_str = severity.map(Severity::as_str); - let rows = db + let rows = writer + .db .events() .query(kind, severity_str, since, until, before_id, limit) .await diff --git a/members/nullnet-server/src/events/persistence.rs b/members/nullnet-server/src/events/persistence.rs new file mode 100644 index 0000000..da45e1d --- /dev/null +++ b/members/nullnet-server/src/events/persistence.rs @@ -0,0 +1,197 @@ +use super::{Event, now_secs}; +use crate::db::{Db, EventInsert}; +use std::sync::{ + Arc, + atomic::{AtomicU64, Ordering}, +}; +use std::time::Duration; +use tokio::sync::{broadcast, mpsc, oneshot}; + +const QUEUE_CAPACITY: usize = 4096; +const BATCH_SIZE: usize = 512; + +enum Command { + Event(Event), + #[cfg(test)] + Flush(oneshot::Sender<()>), + Shutdown(oneshot::Sender<()>), +} + +pub(super) struct Writer { + pub(super) db: Db, + queue: mpsc::Sender, + dropped: Arc, + live: broadcast::Sender, +} + +impl Writer { + pub(super) fn start(db: Db, live: broadcast::Sender) -> Self { + let (queue, receiver) = mpsc::channel(QUEUE_CAPACITY); + let dropped = Arc::new(AtomicU64::new(0)); + tokio::spawn(run(db.clone(), live.clone(), receiver, dropped.clone())); + Self { + db, + queue, + dropped, + live, + } + } + + pub(super) async fn emit(&self, event: Event) { + match self.queue.try_send(Command::Event(event)) { + Ok(()) => {} + Err(mpsc::error::TrySendError::Full(_)) => { + if self.dropped.fetch_add(1, Ordering::Relaxed) == 0 { + eprintln!( + "Event persistence queue is full; dropping new events to keep routing available" + ); + let _ = self.live.send(Event::PersistenceOverflow { + dropped_events: 1, + timestamp: now_secs(), + }); + } + } + Err(mpsc::error::TrySendError::Closed(_)) => { + eprintln!("Event persistence is closed; event was not accepted"); + } + } + } + + #[cfg(test)] + pub(super) async fn flush(&self) { + let (tx, rx) = oneshot::channel(); + self.queue.send(Command::Flush(tx)).await.unwrap(); + rx.await.unwrap(); + } + + pub(super) async fn shutdown(&self) { + let (tx, rx) = oneshot::channel(); + let drain = async { + if self.queue.send(Command::Shutdown(tx)).await.is_ok() { + let _ = rx.await; + } + }; + if tokio::time::timeout(Duration::from_secs(5), drain) + .await + .is_err() + { + eprintln!( + "Event persistence shutdown exceeded 5s; remaining queued events may be lost" + ); + } + } +} + +async fn run( + db: Db, + live: broadcast::Sender, + mut queue: mpsc::Receiver, + dropped: Arc, +) { + let mut commands = Vec::with_capacity(BATCH_SIZE); + let mut shutdown = Vec::new(); + while queue.recv_many(&mut commands, BATCH_SIZE).await != 0 { + let mut events = Vec::with_capacity(commands.len()); + #[cfg(test)] + let mut flush = Vec::new(); + for command in commands.drain(..) { + match command { + Command::Event(event) => events.push(event), + #[cfg(test)] + Command::Flush(ack) => flush.push(ack), + Command::Shutdown(ack) => { + queue.close(); + shutdown.push(ack); + } + } + } + let lost = dropped.swap(0, Ordering::Relaxed); + if lost > 0 { + events.push(Event::PersistenceOverflow { + dropped_events: lost, + timestamp: now_secs(), + }); + } + if !events.is_empty() { + persist(&db, &live, &events, &queue).await; + } + #[cfg(test)] + for ack in flush { + let _ = ack.send(()); + } + } + for ack in shutdown { + let _ = ack.send(()); + } +} + +async fn persist( + db: &Db, + live: &broadcast::Sender, + events: &[Event], + queue: &mpsc::Receiver, +) { + let mut failure = None; + loop { + let recovered = failure.as_ref().map(|_| Event::PersistenceRecovered { + queued_events: events.len() + queue.len(), + timestamp: now_secs(), + }); + let all: Vec<_> = events + .iter() + .chain(failure.iter()) + .chain(recovered.iter()) + .collect(); + let payloads: Vec<_> = all + .iter() + .map(|event| { + let value = serde_json::to_value(event).expect("event fields are JSON values"); + let timestamp = value + .get("timestamp") + .and_then(serde_json::Value::as_i64) + .unwrap_or_else(|| now_secs() as i64); + (timestamp, value.to_string()) + }) + .collect(); + let rows: Vec<_> = all + .iter() + .zip(payloads) + .map(|(event, (timestamp, payload))| EventInsert { + kind: event.kind(), + severity: event.severity().as_str(), + timestamp, + payload, + }) + .collect(); + match db.events().insert_batch(rows).await { + Ok(()) => { + for event in events { + let _ = live.send(event.clone()); + } + if let Some(recovered) = recovered { + eprintln!( + "Event persistence recovered; committed {} queued events", + events.len() + ); + let _ = live.send(recovered); + } + return; + } + Err(error) => { + if failure.is_none() { + eprintln!( + "Event persistence failed; retaining the batch and retrying: {error:?}" + ); + let event = Event::PersistenceFailed { + error_message: error.to_str().to_owned(), + queued_events: events.len() + queue.len(), + timestamp: now_secs(), + }; + let _ = live.send(event.clone()); + failure = Some(event); + } + tokio::time::sleep(Duration::from_secs(1)).await; + } + } + } +} diff --git a/members/nullnet-server/src/http_server/service_config.rs b/members/nullnet-server/src/http_server/service_config.rs index 968428c..2b6eeec 100644 --- a/members/nullnet-server/src/http_server/service_config.rs +++ b/members/nullnet-server/src/http_server/service_config.rs @@ -271,15 +271,9 @@ pub(super) async fn import_handler( if state .db .stacks() - .put_services(&stack, &service_inserts) + .put_configuration(&stack, Some(&service_inserts), Some(&route_inserts)) .await .is_err() - || state - .db - .stacks() - .put_routes(&stack, &route_inserts) - .await - .is_err() { return rejected( StatusCode::INTERNAL_SERVER_ERROR, diff --git a/members/nullnet-server/src/main.rs b/members/nullnet-server/src/main.rs index e57bf35..d210511 100644 --- a/members/nullnet-server/src/main.rs +++ b/members/nullnet-server/src/main.rs @@ -34,6 +34,9 @@ const DEFAULT_DATABASE_URL: &str = "/var/nullnet/data/nullnet.db"; #[tokio::main] async fn main() -> Result<(), Error> { + let shutdown = std::sync::Arc::new(tokio::sync::Notify::new()); + let shutdown_signal = shutdown.clone(); + ctrlc::set_handler(move || shutdown_signal.notify_one()).handle_err(location!())?; // let _gag1: gag::Redirect; // let _gag2: gag::Redirect; // if let Some((gag1, gag2)) = redirect_stdout_stderr_to_file() { @@ -125,17 +128,21 @@ async fn main() -> Result<(), Error> { // prune persisted events + ended sessions past their retention windows (issue #151) retention::start(app_state.db.clone(), retention::RetentionConfig::from_env()); - tokio::select! { + let events = app_state.events.clone(); + let result = tokio::select! { result = server .add_service( NullnetGrpcServer::new(nullnet) .max_decoding_message_size(50 * 1024 * 1024), ) .serve(addr) => { - result.handle_err(location!())?; + result.handle_err(location!()) } - () = http_server::serve(app_state, certificates) => {} - } + () = http_server::serve(app_state, certificates) => Ok(()), + () = shutdown.notified() => Ok(()), + }; + events.shutdown().await; + result?; Ok(()) } @@ -151,12 +158,6 @@ async fn init_nullnet(db: db::Db) -> Result { })); } - // handle termination signals: SIGINT, SIGTERM, SIGHUP - ctrlc::set_handler(move || { - process::exit(1); - }) - .handle_err(location!())?; - NullnetGrpcImpl::new(db).await } diff --git a/members/nullnet-server/src/nullnet_grpc_impl.rs b/members/nullnet-server/src/nullnet_grpc_impl.rs index a6bd123..57fc0f4 100644 --- a/members/nullnet-server/src/nullnet_grpc_impl.rs +++ b/members/nullnet-server/src/nullnet_grpc_impl.rs @@ -424,12 +424,13 @@ impl NullnetGrpcImpl { request: Request>, ) -> Result::ControlChannelStream>, Error> { let (outbound, receiver) = mpsc::channel(64); + let stream = ReceiverStream::new(receiver); self.orchestrator .add_client(request, outbound, self.services.clone()) .await?; - Ok(Response::new(ReceiverStream::new(receiver))) + Ok(Response::new(stream)) } // Concurrent first-time setup is race-safe for single-hop deps (check-and- @@ -633,51 +634,30 @@ impl NullnetGrpcImpl { registered = reg.clone(); } - // Max-networks: if the limit is reached, reuse the least-used existing - // network on the same proxy instead of creating a new one. - if let Some(max) = registered.max_networks() - && registered.proxy_clients_count() >= max as usize - && let Some(( - upstream, - client_net, - server_net, - net_id, - replica_ip, - replica_docker, - setup_ms, - )) = registered.find_reusable_network_on_proxy(proxy_ip) - { - println!( - "Max networks ({max}) reached for '{service_name}', \ - reusing network on proxy {proxy_ip}" - ); - self.orchestrator - .events - .emit(Event::max_networks_limit_enforced( - service_name.to_string(), - proxy_ip.to_string(), - net_id, - max, - )) - .await; - let mut services_mut = self.services.write().await; - if let Some(stack_map) = services_mut.get_mut(&stack) { - if let Some(ServiceInfo::Registered(reg)) = stack_map.get_mut(service_name) { - // Create a new Client entry sharing the existing network - let new_ci = - ClientInfo::new(proxy_ip, client_net, server_net, net_id, setup_ms, None); - reg.add_client_to_replica( - replica_ip, - replica_docker.as_deref(), - proxy_client.clone(), - new_ci, - ); - reg.add_chain(&proxy_client); + // Select and attach under one lock so concurrent clients see current usage. + let reused = if registered.max_networks().is_some() { + let mut services = self.services.write().await; + let reuse = services.get_mut(&stack).and_then(|stack_map| { + let ServiceInfo::Registered(reg) = stack_map.get_mut(service_name)? else { + return None; + }; + let max = reg.max_networks()?; + if reg.proxy_clients_count() < max as usize { + return None; } - // Increment chains on each dependency edge (intra-stack) + let network = reg.find_reusable_network_on_proxy(proxy_ip)?; + let (_, client_net, server_net, net_id, replica_ip, replica_docker, setup_ms) = + &network; + reg.add_client_to_replica( + *replica_ip, + replica_docker.as_deref(), + proxy_client.clone(), + ClientInfo::new(proxy_ip, *client_net, *server_net, *net_id, *setup_ms, None), + ); + reg.add_chain(&proxy_client); let dep_edges = collect_dep_chain_edges( service_name, - replica_ip, + *replica_ip, replica_docker.as_deref(), stack_map, ); @@ -686,8 +666,23 @@ impl NullnetGrpcImpl { dep_reg.add_chain(&dep_client); } } - } - drop(services_mut); + Some((max, network)) + }); + drop(services); + reuse + } else { + None + }; + if let Some((max, (upstream, client_net, server_net, net_id, _, _, setup_ms))) = reused { + self.orchestrator + .events + .emit(Event::max_networks_limit_enforced( + service_name.to_string(), + proxy_ip.to_string(), + net_id, + max, + )) + .await; // Reusing a network still starts a session for *this* client: it // gets its own client entry and its own teardown, so it needs its @@ -2045,9 +2040,9 @@ async fn run_net_chain_setup( // built is holding a refcount nobody will ever spend. Hand them back // instead of stranding them and their net ids. let mut services_mut = services.write().await; - let backend_lost = if let Some((key, generation)) = &owner { + let backend_history = if let Some((key, generation)) = &owner { if any_failure { - false + None } else { let source = Client::new_service(key.0.clone(), key.1, key.2.clone()); let first = successful @@ -2062,7 +2057,7 @@ async fn run_net_chain_setup( _ => None, }) .map(ClientInfo::time_ms); - !orchestrator + orchestrator .finish_backend_session( key, *generation, @@ -2073,8 +2068,9 @@ async fn run_net_chain_setup( .await } } else { - false + None }; + let backend_lost = owner.is_some() && !any_failure && backend_history.is_none(); let orphaned = backend_lost || { let guard = &services_mut; successful @@ -2111,6 +2107,10 @@ async fn run_net_chain_setup( drop(services_mut); + if let Some(history) = backend_history { + orchestrator.persist_backend_session(history).await; + } + // Past the unwind check, so every branch is up and these sessions are real. for edge in &successful { let (Some(ingress), Some(proxy_ip)) = (&edge.ingress, edge.client.is_proxy()) else { @@ -2367,7 +2367,13 @@ async fn setup_edge( let (server_ok, client_ok) = tokio::join!(server_res, client_res); - if server_ok.is_none() || client_ok.is_none() { + // Local steering must not release packets into an unfinished peer. + let activate = *NET_TYPE == Net::Vxlan + && (backend_entry_port.is_some() || client_egress == EgressRole::Steer); + if server_ok.is_none() + || client_ok.is_none() + || (activate && !orchestrator.send_net_ready(client_ethernet, net_id).await) + { if client.is_proxy().is_some() { orchestrator .events diff --git a/members/nullnet-server/src/orchestrator.rs b/members/nullnet-server/src/orchestrator.rs index 88112af..4660def 100644 --- a/members/nullnet-server/src/orchestrator.rs +++ b/members/nullnet-server/src/orchestrator.rs @@ -8,7 +8,8 @@ use crate::services::changes::{apply_changes, detect_node_disconnect_changes}; use crate::services::input::StackMap; use crate::sessions::SessionStore; use nullnet_grpc_lib::nullnet_grpc::{ - ContainerResume, ContainerSuspend, EgressPolicyChanged, MsgId, Net, NetMessage, net_message, + ContainerResume, ContainerSuspend, EgressPolicyChanged, MsgId, Net, NetMessage, NetReady, + net_message, }; use nullnet_liberror::{Error, ErrorHandler, Location, location}; use std::collections::HashMap; @@ -50,15 +51,22 @@ struct BackendSession { idle_since: Option, } +pub(crate) struct PendingBackendHistory { + key: BackendKey, + generation: Uuid, + stack: String, + net_id: u32, + destination: String, + setup_ms: Option, +} + /// Cap on distinct external destinations tracked per egress edge. When full, the /// least-recently-contacted destination is evicted. Bounds memory for a service /// that contacts a very large set of hosts (e.g. a crawler). const MAX_DESTS_PER_EDGE: usize = 256; -/// How long to wait for an endpoint to confirm a teardown before returning the -/// net id to the pool unconfirmed. Matches `send_container_resume`'s 30s ack -/// window; teardown is a short local operation, so reaching this means the -/// endpoint is wedged or gone rather than slow. +/// Warn when cleanup exceeds its deadline; only completion or a closed +/// connection can release the id. A connected endpoint may still be deleting. const TEARDOWN_ACK_GRACE: Duration = Duration::from_secs(30); /// Per-destination stats on an egress edge, reported by the client (which owns @@ -215,7 +223,9 @@ impl Orchestrator { let mut inbound = request.into_inner(); let orchestrator = self.clone(); tokio::spawn(async move { - while let Ok(Some(msg_id)) = inbound.message().await { + while let Ok(Some(msg_id)) = inbound.message().await.inspect_err(|error| { + eprintln!("Control channel from '{client_ip}' failed: {error:?}"); + }) { if let Some(tx) = orchestrator.pending.lock().await.remove(&msg_id.id) { let _ = tx.send(()); } @@ -396,7 +406,10 @@ impl Orchestrator { ); let (proxy_ok, init_ok) = tokio::join!(proxy_res, init_res); - if proxy_ok.is_none() || init_ok.is_none() { + if proxy_ok.is_none() + || init_ok.is_none() + || !self.send_net_ready(initiator_ip, net_id).await + { self.events .emit(Event::setup_timeout(net_id, service.to_string())) .await; @@ -788,7 +801,7 @@ impl Orchestrator { Some(generation) } - /// Preserve liveness received during setup; never promote a replacement. + /// Called under the services lock; storage is deferred until that lock drops. pub(crate) async fn finish_backend_session( &self, key: &BackendKey, @@ -796,29 +809,58 @@ impl Orchestrator { net_id: u32, destination: &str, setup_ms: Option, - ) -> bool { + ) -> Option { + let mut sessions = self.backend_sessions.write().await; + let session = sessions + .get_mut(key) + .filter(|s| s.generation == generation)?; + session.building = false; + Some(PendingBackendHistory { + key: key.clone(), + generation, + stack: session.stack.clone(), + net_id, + destination: destination.to_string(), + setup_ms, + }) + } + + pub(crate) async fn persist_backend_session(&self, pending: PendingBackendHistory) { + let history_id = self + .sessions + .open_backend( + &pending.stack, + &pending.key, + pending.net_id, + &pending.destination, + pending.setup_ms, + ) + .await; let mut sessions = self.backend_sessions.write().await; - if let Some(session) = sessions.get_mut(key) - && session.generation == generation + if let Some(session) = sessions + .get_mut(&pending.key) + .filter(|s| s.generation == pending.generation) { - session.history_id = self - .sessions - .open_backend(&session.stack, key, net_id, destination, setup_ms) - .await; - session.building = false; - true + session.history_id = history_id; } else { - false + drop(sessions); + self.sessions.close_backend(history_id).await; } } pub(crate) async fn cancel_backend_session(&self, key: &BackendKey, generation: Uuid) { - let mut sessions = self.backend_sessions.write().await; - if sessions - .get(key) - .is_some_and(|s| s.generation == generation) - && let Some(session) = sessions.remove(key) - { + let removed = { + let mut sessions = self.backend_sessions.write().await; + if sessions + .get(key) + .is_some_and(|s| s.generation == generation) + { + sessions.remove(key) + } else { + None + } + }; + if let Some(session) = removed { self.sessions.close_backend(session.history_id).await; } } @@ -853,7 +895,7 @@ impl Orchestrator { pub(crate) async fn take_due_backend_sessions( &self, debounce: Duration, - ) -> Vec<(BackendKey, String)> { + ) -> Vec<(BackendKey, String, Option)> { let now = Instant::now(); let mut sessions = self.backend_sessions.write().await; let due: Vec = sessions @@ -868,11 +910,14 @@ impl Orchestrator { let mut expired = Vec::new(); for key in due { if let Some(session) = sessions.remove(&key) { - self.sessions.close_backend(session.history_id).await; - expired.push((key, session.stack)); + expired.push((key, session)); } } + drop(sessions); expired + .into_iter() + .map(|(key, session)| (key, session.stack, session.history_id)) + .collect() } /// How long until the nearest trigger chain becomes reapable, if any. @@ -907,13 +952,18 @@ impl Orchestrator { ports: &[u16], ) { let mut sessions = self.backend_sessions.write().await; + let mut removed = Vec::new(); for port in ports { if let Some(session) = sessions.remove(&(service.to_string(), ip, docker.map(String::from), *port)) { - self.sessions.close_backend(session.history_id).await; + removed.push(session); } } + drop(sessions); + for session in removed { + self.sessions.close_backend(session.history_id).await; + } } /// Tear down egress edges on `node_ip` whose initiator container is no longer @@ -965,14 +1015,9 @@ impl Orchestrator { encrypted: bool, egress: EgressRole, ) -> Option { - let outbound = self.clients.read().await.get(&dest).cloned(); - if let Some(outbound) = outbound { - let (tx, rx) = oneshot::channel(); - let msg_id = Uuid::new_v4().to_string(); - self.pending.lock().await.insert(msg_id.clone(), tx); - - let (server_net, message) = NET_TYPE.setup( - msg_id.clone(), + self.send_control(dest, |msg_id| { + NET_TYPE.setup( + msg_id.id, dest, remote_server_name, net_id, @@ -983,24 +1028,50 @@ impl Orchestrator { dstport, encrypted, egress, - )?; - - if outbound.send(Ok(message)).await.is_err() { - self.pending.lock().await.remove(&msg_id); - return None; - } + ) + }) + .await + } - if let Ok(result) = tokio::time::timeout(Duration::from_secs(30), rx).await { - result.ok().map(|()| server_net) - } else { - self.pending.lock().await.remove(&msg_id); + async fn send_control( + &self, + dest: IpAddr, + message: impl FnOnce(MsgId) -> Option<(T, NetMessage)>, + ) -> Option { + let outbound = self.clients.read().await.get(&dest).cloned()?; + let id = Uuid::new_v4().to_string(); + let (value, message) = message(MsgId { id: id.clone() })?; + let (tx, rx) = oneshot::channel(); + self.pending.lock().await.insert(id.clone(), tx); + if outbound.send(Ok(message)).await.is_err() { + self.pending.lock().await.remove(&id); + return None; + } + match tokio::time::timeout(Duration::from_secs(30), rx).await { + Ok(Ok(())) => Some(value), + _ => { + self.pending.lock().await.remove(&id); None } - } else { - None } } + pub(crate) async fn send_net_ready(&self, dest: IpAddr, net_id: u32) -> bool { + self.send_control(dest, |msg_id| { + Some(( + (), + NetMessage { + message: Some(net_message::Message::NetReady(NetReady { + msg_id: Some(msg_id), + net_id, + })), + }, + )) + }) + .await + .is_some() + } + /// Fire-and-forget: tell the host running `docker_container` to `docker pause` it. /// Mirrors `send_net_teardown` — no ack; the caller marks the replica suspended. pub(crate) async fn send_container_suspend(&self, dest: IpAddr, docker_container: String) { @@ -1047,34 +1118,20 @@ impl Orchestrator { dest: IpAddr, docker_container: String, ) -> bool { - let outbound = self.clients.read().await.get(&dest).cloned(); - let Some(outbound) = outbound else { - return false; - }; - - let (tx, rx) = oneshot::channel(); - let msg_id = Uuid::new_v4().to_string(); - self.pending.lock().await.insert(msg_id.clone(), tx); - println!("Resuming container '{docker_container}' on {dest}"); - let message = NetMessage { - message: Some(net_message::Message::ContainerResume(ContainerResume { - msg_id: Some(MsgId { id: msg_id.clone() }), - docker_container, - })), - }; - - if outbound.send(Ok(message)).await.is_err() { - self.pending.lock().await.remove(&msg_id); - return false; - } - - if let Ok(result) = tokio::time::timeout(Duration::from_secs(30), rx).await { - result.is_ok() - } else { - self.pending.lock().await.remove(&msg_id); - false - } + self.send_control(dest, |msg_id| { + Some(( + (), + NetMessage { + message: Some(net_message::Message::ContainerResume(ContainerResume { + msg_id: Some(msg_id), + docker_container, + })), + }, + )) + }) + .await + .is_some() } pub(crate) async fn allocate_net_id(&self) -> Option { @@ -1150,10 +1207,8 @@ impl Orchestrator { /// turn a multi-edge chain teardown into a serial walk of ack timeouts. /// Caller-visible latency is unchanged. /// - /// The id is still freed if an ack never arrives (`TEARDOWN_ACK_GRACE`), - /// because the endpoint being gone is the *normal* case on this path — - /// `teardown_egress_edges_for_node` runs precisely when a node has - /// disconnected — and refusing to free would leak every id that node held. + /// A disconnected endpoint purges old state before registering again. + /// A connected endpoint must finish cleanup even if its deadline expires. pub(crate) async fn send_net_teardown( &self, client: IpAddr, @@ -1197,7 +1252,7 @@ impl Orchestrator { // Nothing will ever ack a message that was never sent. self.pending.lock().await.remove(&msg_id); } else { - acks.push((dest, msg_id, rx)); + acks.push((dest, msg_id, rx, outbound)); } } } @@ -1216,26 +1271,22 @@ impl Orchestrator { /// Return a net id and its VXLAN dstport to their pools. async fn free_net_id_and_port(&self, net_id: u32) { - self.net_id_pool.lock().await.free(net_id); if let Some((pair, port)) = self.net_id_ports.lock().await.remove(&net_id) && let Some(pool) = self.udp_port_pools.lock().await.get_mut(&pair) { pool.free(port); } + self.net_id_pool.lock().await.free(net_id); } - /// Wait for every endpoint that was actually sent a teardown to ack it, - /// then return the net id and its dstport to the pools. See - /// `send_net_teardown` for why this is detached and why it frees anyway on - /// timeout. + /// Keep ids and ports reserved until every sent teardown is complete or + /// its connection closes. A deadline only raises an operator warning. fn spawn_deferred_net_id_free( &self, net_id: u32, - acks: Vec<(IpAddr, String, oneshot::Receiver<()>)>, + acks: Vec<(IpAddr, String, oneshot::Receiver<()>, OutboundStream)>, ) { - let net_id_pool = self.net_id_pool.clone(); - let net_id_ports = self.net_id_ports.clone(); - let udp_port_pools = self.udp_port_pools.clone(); + let orchestrator = self.clone(); let pending = self.pending.clone(); let events = self.events.clone(); let inflight = self.inflight_teardowns.clone(); @@ -1245,30 +1296,33 @@ impl Orchestrator { inflight.fetch_add(1, Ordering::SeqCst); tokio::spawn(async move { - for (dest, msg_id, rx) in acks { - match tokio::time::timeout(TEARDOWN_ACK_GRACE, rx).await { - Ok(Ok(())) => {} - // Timed out, or the sender was dropped without acking. - _ => { - pending.lock().await.remove(&msg_id); - println!( - "Network {net_id} teardown was not acked by {dest} within {}s; \ - freeing the id anyway", - TEARDOWN_ACK_GRACE.as_secs() - ); - events - .emit(Event::net_teardown_unconfirmed(net_id, dest.to_string())) - .await; + for (dest, msg_id, rx, outbound) in acks { + let completed = async { + tokio::select! { + result = rx => { + if result.is_err() { outbound.closed().await; } + } + () = outbound.closed() => {} } + }; + tokio::pin!(completed); + if tokio::time::timeout(TEARDOWN_ACK_GRACE, &mut completed) + .await + .is_err() + { + println!( + "Network {net_id} teardown was not acked by {dest} within {}s; keeping the id reserved", + TEARDOWN_ACK_GRACE.as_secs() + ); + events + .emit(Event::net_teardown_unconfirmed(net_id, dest.to_string())) + .await; + completed.await; } + pending.lock().await.remove(&msg_id); } - net_id_pool.lock().await.free(net_id); - if let Some((pair, port)) = net_id_ports.lock().await.remove(&net_id) - && let Some(pool) = udp_port_pools.lock().await.get_mut(&pair) - { - pool.free(port); - } + orchestrator.free_net_id_and_port(net_id).await; inflight.fetch_sub(1, Ordering::SeqCst); }); } @@ -1294,10 +1348,7 @@ impl Orchestrator { /// Production code never needs to observe the deferred free; assertions /// about pool state do, and must not race the detached task. pub(crate) async fn settle_teardowns(&self) { - // Bounded: a teardown whose ack never arrives resolves via - // TEARDOWN_ACK_GRACE, which no test should be waiting on. If this cap - // is ever hit, the assertion that follows will fail loudly rather than - // hang. + // Tests must explicitly ack or disconnect every endpoint before settling. for _ in 0..10_000 { if self.inflight_teardowns.load(Ordering::SeqCst) == 0 { return; @@ -1342,6 +1393,7 @@ impl Orchestrator { | Some(net_message::Message::VxlanSetup( nullnet_grpc_lib::nullnet_grpc::VxlanSetup { msg_id, .. }, )) + | Some(net_message::Message::NetReady(NetReady { msg_id, .. })) | Some(net_message::Message::ContainerResume(ContainerResume { msg_id, .. })) @@ -1353,9 +1405,10 @@ impl Orchestrator { )) => msg_id.clone(), _ => None, }; + let activation = matches!(msg.message, Some(net_message::Message::NetReady(_))); log_task.lock().await.push(msg); if let Some(msg_id) = ack_id { - if let Ok(permit) = gate_task.acquire().await { + if !activation && let Ok(permit) = gate_task.acquire().await { permit.forget(); } if let Some(tx) = pending.lock().await.remove(&msg_id.id) { @@ -1393,6 +1446,7 @@ impl Orchestrator { | Some(net_message::Message::VxlanSetup( nullnet_grpc_lib::nullnet_grpc::VxlanSetup { msg_id, .. }, )) + | Some(net_message::Message::NetReady(NetReady { msg_id, .. })) | Some(net_message::Message::ContainerResume(ContainerResume { msg_id, .. })) => msg_id.clone(), @@ -1460,7 +1514,7 @@ mod teardown_ack_tests { orch.send_net_teardown(a, None, b, None, id).await; // Give the detached task every chance to run; it must still be parked - // on the ack rather than freeing (it frees only after the grace). + // on the ack rather than freeing. for _ in 0..256 { tokio::task::yield_now().await; } @@ -1474,6 +1528,73 @@ mod teardown_ack_tests { assert_ne!(orch.allocate_net_id().await.unwrap(), id); } + #[tokio::test] + async fn a_late_teardown_ack_keeps_the_id_reserved_until_completion() { + let orch = Orchestrator::new(); + let (a, b) = (ip(10, 0, 0, 1), ip(10, 0, 0, 2)); + let (tx, mut rx) = mpsc::channel(64); + orch.clients.write().await.insert(a, tx); + let id = orch.allocate_net_id().await.unwrap(); + orch.send_net_teardown(a, None, b, None, id).await; + let message = rx.recv().await.unwrap().unwrap(); + let msg_id = match message.message.unwrap() { + net_message::Message::VlanTeardown(m) => m.msg_id.unwrap(), + net_message::Message::VxlanTeardown(m) => m.msg_id.unwrap(), + _ => panic!("expected teardown"), + }; + tokio::time::sleep(TEARDOWN_ACK_GRACE + Duration::from_millis(100)).await; + assert_eq!( + orch.net_ids_in_use().await, + 1, + "an expired deadline is not completed cleanup" + ); + orch.pending + .lock() + .await + .remove(&msg_id.id) + .unwrap() + .send(()) + .unwrap(); + orch.settle_teardowns().await; + assert_eq!(orch.net_ids_in_use().await, 0); + } + + #[tokio::test] + async fn a_closed_connection_releases_its_pending_teardown() { + let orch = Orchestrator::new(); + let (a, b) = (ip(10, 0, 0, 1), ip(10, 0, 0, 2)); + let (tx, rx) = mpsc::channel(64); + orch.clients.write().await.insert(a, tx); + let id = orch.allocate_net_id().await.unwrap(); + orch.send_net_teardown(a, None, b, None, id).await; + drop(rx); + orch.settle_teardowns().await; + assert_eq!(orch.net_ids_in_use().await, 0); + assert!(orch.pending.lock().await.is_empty()); + } + + #[tokio::test] + async fn returning_an_id_waits_for_its_old_port_record_to_be_removed() { + let orch = Orchestrator::new(); + let id = orch.allocate_net_id().await.unwrap(); + let ports = orch.net_id_ports.lock().await; + let copy = orch.clone(); + let cleanup = tokio::spawn(async move { + copy.free_net_id_and_port(id).await; + }); + for _ in 0..256 { + tokio::task::yield_now().await; + } + assert_eq!( + orch.net_ids_in_use().await, + 1, + "id must not be reused while its old port record is being removed" + ); + drop(ports); + cleanup.await.unwrap(); + assert_eq!(orch.net_ids_in_use().await, 0); + } + /// Neither endpoint is connected, so nothing was sent and there is nothing /// to wait for — the id comes back immediately, as before this change. /// `teardown_egress_edges_for_node` runs in exactly this state. @@ -1804,6 +1925,125 @@ mod session_history_tests { (orch, db) } + async fn finish_backend( + orch: &Orchestrator, + key: &BackendKey, + generation: Uuid, + net_id: u32, + destination: &str, + setup_ms: Option, + ) -> bool { + let Some(pending) = orch + .finish_backend_session(key, generation, net_id, destination, setup_ms) + .await + else { + return false; + }; + orch.persist_backend_session(pending).await; + true + } + + #[tokio::test] + async fn backend_history_wait_does_not_block_liveness() { + let (orch, db) = orch_with_db().await; + let key = ("api".to_string(), ip(10, 0, 0, 1), None, 8080); + let generation = orch + .claim_backend_session(key.clone(), "prod") + .await + .unwrap(); + let db_guard = db.hold_connection().await; + let finish = finish_backend(&orch, &key, generation, 42, "db", Some(7)); + tokio::pin!(finish); + assert!(futures::poll!(&mut finish).is_pending()); + let liveness = orch.set_backend_liveness(&key, false); + tokio::pin!(liveness); + assert!(futures::poll!(&mut liveness).is_ready()); + drop(db_guard); + assert!(finish.await); + assert!( + orch.backend_sessions.read().await[&key] + .idle_since + .is_some() + ); + } + + #[tokio::test] + async fn backend_history_late_open_cannot_overwrite_replacement() { + let (orch, db) = orch_with_db().await; + let key = ("api".to_string(), ip(10, 0, 0, 1), None, 8080); + let generation = orch + .claim_backend_session(key.clone(), "prod") + .await + .unwrap(); + let db_guard = db.hold_connection().await; + let finish = finish_backend(&orch, &key, generation, 42, "db", Some(7)); + tokio::pin!(finish); + assert!(futures::poll!(&mut finish).is_pending()); + orch.cancel_backend_session(&key, generation).await; + let replacement = orch + .claim_backend_session(key.clone(), "prod") + .await + .unwrap(); + drop(db_guard); + assert!(finish.await); + assert!(orch.backend_sessions.read().await[&key].building); + assert_eq!(db.sessions().count_active("prod").await.unwrap(), 0); + assert!(finish_backend(&orch, &key, replacement, 43, "db", Some(8)).await); + assert_eq!(db.sessions().count_active("prod").await.unwrap(), 1); + } + + #[tokio::test] + async fn backend_history_close_wait_does_not_block_other_generations() { + for mode in 0..3 { + let (orch, db) = orch_with_db().await; + let key = ("api".to_string(), ip(10, 0, 0, 1), None, 8080); + let other = (key.0.clone(), key.1, None, 9090); + let generation = orch + .claim_backend_session(key.clone(), "prod") + .await + .unwrap(); + let other_generation = orch + .claim_backend_session(other.clone(), "prod") + .await + .unwrap(); + assert!(finish_backend(&orch, &key, generation, 42, "db", Some(7)).await); + assert!(finish_backend(&orch, &other, other_generation, 42, "db", Some(7)).await); + orch.set_backend_liveness(&key, false).await; + let db_guard = db.hold_connection().await; + let close = async { + match mode { + 0 => orch.cancel_backend_session(&key, generation).await, + 1 => { + let expired = orch.take_due_backend_sessions(Duration::ZERO).await; + assert_eq!(expired.len(), 1); + orch.sessions.close_backend(expired[0].2).await; + } + _ => { + orch.forget_backend_sessions("api", key.1, None, &[8080]) + .await + } + } + }; + tokio::pin!(close); + assert!(futures::poll!(&mut close).is_pending()); + let liveness = orch.set_backend_liveness(&other, false); + tokio::pin!(liveness); + assert!(futures::poll!(&mut liveness).is_ready()); + let replacement = orch + .claim_backend_session(key.clone(), "prod") + .await + .unwrap(); + drop(db_guard); + close.await; + assert!(finish_backend(&orch, &key, replacement, 43, "db", Some(8)).await); + assert_eq!(db.sessions().count_active("prod").await.unwrap(), 2); + assert_eq!( + orch.backend_sessions.read().await[&key].generation, + replacement + ); + } + } + #[tokio::test] async fn backend_history_tracks_independent_chains_and_generations() { let (orch, db) = orch_with_db().await; @@ -1823,14 +2063,8 @@ mod session_history_tests { .await .unwrap(); assert_eq!(db.sessions().count_active("prod").await.unwrap(), 0); - assert!( - orch.finish_backend_session(&key, generation, 42, "db", Some(7)) - .await - ); - assert!( - orch.finish_backend_session(&other, other_generation, 42, "db", Some(7)) - .await - ); + assert!(finish_backend(&orch, &key, generation, 42, "db", Some(7)).await); + assert!(finish_backend(&orch, &other, other_generation, 42, "db", Some(7)).await); assert_eq!(db.sessions().count_active("prod").await.unwrap(), 2); let rows = db .sessions() @@ -1874,19 +2108,15 @@ mod session_history_tests { ); } orch.set_backend_liveness(&key, false).await; - assert_eq!( - orch.take_due_backend_sessions(Duration::ZERO).await.len(), - 1 - ); + let expired = orch.take_due_backend_sessions(Duration::ZERO).await; + assert_eq!(expired.len(), 1); + orch.sessions.close_backend(expired[0].2).await; assert_eq!(db.sessions().count_active("prod").await.unwrap(), 1); let replacement = orch .claim_backend_session(key.clone(), "prod") .await .unwrap(); - assert!( - orch.finish_backend_session(&key, replacement, 42, "db", Some(7)) - .await - ); + assert!(finish_backend(&orch, &key, replacement, 42, "db", Some(7)).await); orch.cancel_backend_session(&key, generation).await; assert_eq!(db.sessions().count_active("prod").await.unwrap(), 2); orch.forget_backend_sessions("api", key.1, key.2.as_deref(), &[8080]) @@ -1921,11 +2151,7 @@ mod session_history_tests { .await .unwrap(); assert!(orch.cancel_backend_build(&key).await); - assert!( - !orch - .finish_backend_session(&key, generation, 42, "db", Some(7)) - .await - ); + assert!(!finish_backend(&orch, &key, generation, 42, "db", Some(7)).await); assert!( db.sessions() .query("prod", None, None, None, None, None, None, None, 10) diff --git a/members/nullnet-server/src/services/clients.rs b/members/nullnet-server/src/services/clients.rs index 296f754..ad65611 100644 --- a/members/nullnet-server/src/services/clients.rs +++ b/members/nullnet-server/src/services/clients.rs @@ -265,6 +265,12 @@ impl ClientInfo { self.latest } + pub(super) fn idle_expired(&self, now: Instant, timeout: std::time::Duration) -> bool { + !self.is_pending() + && self.open_connections() == 0 + && now.duration_since(self.latest()) >= timeout + } + pub(crate) fn created_at(&self) -> SystemTime { self.created_at } diff --git a/members/nullnet-server/src/services/service_info.rs b/members/nullnet-server/src/services/service_info.rs index 9d06174..45338ab 100644 --- a/members/nullnet-server/src/services/service_info.rs +++ b/members/nullnet-server/src/services/service_info.rs @@ -670,10 +670,8 @@ impl RegisteredServiceInfo { .count() } - /// Find the least-used proxy client on the given proxy IP. - /// Returns the upstream, network IPs/ID, and replica identity — - /// everything the caller needs to create a new Client entry that - /// shares the same physical network. + /// Find the network with the fewest clients on this proxy. + /// Returns its upstream, addresses and replica identity for sharing. #[allow(clippy::type_complexity)] pub(crate) fn find_reusable_network_on_proxy( &self, @@ -687,28 +685,27 @@ impl RegisteredServiceInfo { Option, u128, )> { - let best = self - .replicas - .iter() - .flat_map(|r| { - r.clients.clients().iter().filter_map(move |(c, ci)| { - if c.is_proxy() == Some(proxy_ip) && ci.server_net() != Ipv4Addr::UNSPECIFIED { - Some(( - ci.active_chains(), - ci.client_net(), - ci.server_net(), - ci.net_id(), - r, - ci.time_ms(), - )) - } else { - None - } - }) - }) - .min_by_key(|(chains, _, _, _, _, _)| *chains); - - let (_, client_net, server_net, net_id, replica, setup_ms) = best?; + let mut networks = HashMap::new(); + for replica in &self.replicas { + for (client, info) in replica.clients.clients() { + if client.is_proxy() == Some(proxy_ip) && info.server_net() != Ipv4Addr::UNSPECIFIED + { + let entry = networks + .entry(info.net_id()) + .or_insert((0usize, info, replica)); + entry.0 += 1; + } + } + } + let (_, (_, info, replica)) = networks + .into_iter() + .min_by_key(|(net_id, (users, _, _))| (*users, *net_id))?; + let (client_net, server_net, net_id, setup_ms) = ( + info.client_net(), + info.server_net(), + info.net_id(), + info.time_ms(), + ); Some(( Upstream { ip: server_net.to_string(), @@ -826,17 +823,19 @@ impl RegisteredServiceInfo { .clients .clients() .iter() - .filter(|(c, ci)| { - c.is_proxy().is_some() - && !ci.is_pending() - && ci.open_connections() == 0 - && now.duration_since(ci.latest()) >= timeout - }) + .filter(|(c, ci)| c.is_proxy().is_some() && ci.idle_expired(now, timeout)) .map(|(c, _)| c.clone()) }) .collect() } + pub(crate) fn proxy_client_expired(&self, client: &Client, timeout: Duration) -> bool { + client.is_proxy().is_some() + && self + .client_info(client) + .is_some_and(|ci| ci.idle_expired(Instant::now(), timeout)) + } + pub(crate) fn nearest_proxy_expiry(&self, timeout: Duration) -> Option { let now = Instant::now(); self.replicas diff --git a/members/nullnet-server/src/tests.rs b/members/nullnet-server/src/tests.rs index 5157e89..5b67665 100644 --- a/members/nullnet-server/src/tests.rs +++ b/members/nullnet-server/src/tests.rs @@ -721,7 +721,7 @@ async fn node_disconnected_proxy1() { const PROXY_TIMEOUT: &str = "proxy_timeout"; -async fn proxy_timeout_setup() -> NullnetGrpcImpl { +pub(crate) async fn proxy_timeout_setup() -> NullnetGrpcImpl { let services = load_fixture(PROXY_TIMEOUT).await; let server = NullnetGrpcImpl::new_for_test(services); @@ -750,6 +750,38 @@ async fn proxy_timeout_setup() -> NullnetGrpcImpl { server } +#[tokio::test] +async fn mass_expiry_allows_reads_between_client_teardowns() { + let server = proxy_timeout_setup().await; + let proxy = ip(5, 5, 5, 5); + for i in 3..131 { + setup_proxy_chain(&server, "A", proxy, &format!("10.0.0.{i}")).await; + } + tokio::time::sleep(std::time::Duration::from_millis(1100)).await; + let (log, gate) = server.orchestrator().register_gated_client(proxy).await; + let worker = server.clone(); + let reap = tokio::spawn(async move { + crate::timeout::reap_ingress_timeouts(worker.services(), worker.orchestrator()).await; + }); + wait_for_log(&log, |messages| !messages.is_empty()).await; + let reader = server.clone(); + let read = tokio::spawn(async move { + let _guard = reader.services().read().await; + }); + tokio::task::yield_now().await; + // Unblock one send if the bounded control queue is already full. + gate.add_permits(1); + let result = tokio::time::timeout(std::time::Duration::from_secs(1), read).await; + gate.add_permits(1000); + reap.await.unwrap(); + assert!(result.is_ok(), "bulk expiry monopolized the services lock"); + let guard = server.services().read().await; + let ServiceInfo::Registered(reg) = &stack_view(&guard)["A"] else { + unreachable!() + }; + assert_eq!(reg.client_count(), 0); +} + /// After A's timeout (1s), both proxy clients on A expire. B's proxy client /// survives (timeout=5). A→C→D edges removed (no more proxy clients on A). /// B→D edge survives. @@ -1755,6 +1787,71 @@ async fn multi_replica_first_step_container_disconnect() { const MAX_NETWORKS: &str = "max_networks"; +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn max_networks_balances_shared_networks_under_concurrent_requests() { + let config: ServicesToml = toml::from_str( + r#"[[services]] +name = "A" +timeout = 60 +max_networks = 10 +proxy_dependencies = [["B"]] +"#, + ) + .unwrap(); + let server = std::sync::Arc::new(NullnetGrpcImpl::new_for_test(into_stack_map( + config.services_map().unwrap(), + ))); + let proxy = ip(5, 5, 5, 5); + register_services( + &server, + &HashMap::from([("A", ip(1, 1, 1, 1)), ("B", ip(2, 2, 2, 2))]), + 8080, + ) + .await; + server.orchestrator().register_fake_client(proxy).await; + for i in 1..=10 { + setup_proxy_chain(&server, "A", proxy, &format!("10.0.0.{i}")).await; + } + assert_net_ids_in_use(&server, 11).await; + + let mut requests = tokio::task::JoinSet::new(); + for i in 11..=200 { + let server = server.clone(); + requests.spawn(async move { + setup_proxy_chain(&server, "A", proxy, &format!("10.0.0.{i}")).await; + }); + } + while let Some(result) = requests.join_next().await { + result.unwrap(); + } + assert_net_ids_in_use(&server, 11).await; + let guard = server.services().read().await; + let ServiceInfo::Registered(reg) = &stack_view(&guard)["A"] else { + panic!("A should be registered"); + }; + let mut users = HashMap::::new(); + for replica in reg.replicas() { + for ci in replica.clients().values() { + *users.entry(ci.net_id()).or_default() += 1; + } + } + assert_eq!(users.len(), 10); + assert!(users.values().all(|count| *count == 20), "{users:?}"); + let ServiceInfo::Registered(dep) = &stack_view(&guard)["B"] else { + panic!("B should be registered"); + }; + assert_eq!(dep.client_count(), 1); + assert_eq!( + dep.replicas()[0] + .clients() + .values() + .next() + .unwrap() + .active_chains(), + 200 + ); +} + /// Full lifecycle: /// 1. First client creates network (2 net IDs: proxy→A, A→B) /// 2. Second client reuses (still 2 net IDs, but A has 2 proxy clients) @@ -3327,6 +3424,130 @@ async fn node_hosting_no_trigger_service_gets_nothing() { const BACKEND_LIVENESS: &str = "backend_liveness"; +#[tokio::test] +async fn backend_reap_history_wait_releases_topology() { + let server = NullnetGrpcImpl::new_for_test(load_fixture(BACKEND_LIVENESS).await); + register_services( + &server, + &HashMap::from([("A", ip(1, 1, 1, 1)), ("B", ip(2, 2, 2, 2))]), + 8080, + ) + .await; + let db = attach_session_db(&server).await; + server + .handle_backend_trigger("A", 5555, ip(1, 1, 1, 1), None) + .await + .unwrap(); + assert_eq!(db.sessions().count_active(TEST_STACK).await.unwrap(), 1); + let key = ("A".to_string(), ip(1, 1, 1, 1), None, 5555); + server + .orchestrator() + .set_backend_liveness(&key, false) + .await; + let db_guard = db.hold_connection().await; + let reap = reap_idle_backend_chains( + server.services(), + server.orchestrator(), + std::time::Duration::ZERO, + ); + tokio::pin!(reap); + assert!(futures::poll!(&mut reap).is_pending()); + assert!( + server.services().try_read().is_ok(), + "history close holds the global topology lock" + ); + assert_net_ids_in_use(&server, 0).await; + let task_server = server.clone(); + let replacement = tokio::spawn(async move { + task_server + .handle_backend_trigger("A", 5555, ip(1, 1, 1, 1), None) + .await + }); + tokio::time::timeout(std::time::Duration::from_secs(5), async { + while !server.orchestrator().holds_backend_session(&key).await { + tokio::task::yield_now().await; + } + }) + .await + .unwrap(); + drop(db_guard); + reap.await; + replacement.await.unwrap().unwrap(); + assert_net_ids_in_use(&server, 1).await; + assert_eq!(db.sessions().count_active(TEST_STACK).await.unwrap(), 1); + report_backend_idle_and_reap(&server, "A", key.1, 5555).await; + assert_net_ids_in_use(&server, 0).await; + assert_eq!(db.sessions().count_active(TEST_STACK).await.unwrap(), 0); +} + +#[tokio::test] +async fn backend_pending_history_allows_teardown_and_replacement() { + let server = NullnetGrpcImpl::new_for_test(load_fixture(BACKEND_LIVENESS).await); + register_services( + &server, + &HashMap::from([("A", ip(1, 1, 1, 1)), ("B", ip(2, 2, 2, 2))]), + 8080, + ) + .await; + let db = attach_session_db(&server).await; + let db_guard = db.hold_connection().await; + let key = ("A".to_string(), ip(1, 1, 1, 1), None, 5555); + let task_server = server.clone(); + let first = tokio::spawn(async move { + task_server + .handle_backend_trigger("A", 5555, ip(1, 1, 1, 1), None) + .await + }); + tokio::time::timeout(std::time::Duration::from_secs(5), async { + while !server.orchestrator().holds_backend_session(&key).await { + tokio::task::yield_now().await; + } + server + .orchestrator() + .set_backend_liveness(&key, false) + .await; + while server + .orchestrator() + .nearest_backend_expiry(std::time::Duration::ZERO) + .await + .is_none() + { + tokio::task::yield_now().await; + } + reap_idle_backend_chains( + server.services(), + server.orchestrator(), + std::time::Duration::ZERO, + ) + .await; + }) + .await + .expect("history storage blocked topology or teardown"); + assert!(!first.is_finished()); + assert_net_ids_in_use(&server, 0).await; + let task_server = server.clone(); + let replacement = tokio::spawn(async move { + task_server + .handle_backend_trigger("A", 5555, ip(1, 1, 1, 1), None) + .await + }); + tokio::time::timeout(std::time::Duration::from_secs(5), async { + while !server.orchestrator().holds_backend_session(&key).await { + tokio::task::yield_now().await; + } + }) + .await + .unwrap(); + drop(db_guard); + first.await.unwrap().unwrap(); + replacement.await.unwrap().unwrap(); + assert_net_ids_in_use(&server, 1).await; + assert_eq!(db.sessions().count_active(TEST_STACK).await.unwrap(), 1); + report_backend_idle_and_reap(&server, "A", key.1, 5555).await; + assert_net_ids_in_use(&server, 0).await; + assert_eq!(db.sessions().count_active(TEST_STACK).await.unwrap(), 0); +} + #[tokio::test] async fn trigger_atomic_concurrent_backend_claims_release_once() { let server = NullnetGrpcImpl::new_for_test(load_fixture(BACKEND_LIVENESS).await); @@ -3378,9 +3599,12 @@ async fn trigger_atomic_backend_idle_during_setup_is_preserved() { gate.add_permits(64); build.await.unwrap().unwrap(); { - let mut guard = server.services().write().await; - reap_idle_backend_chains(&mut guard, server.orchestrator(), std::time::Duration::ZERO) - .await; + reap_idle_backend_chains( + server.services(), + server.orchestrator(), + std::time::Duration::ZERO, + ) + .await; } assert_net_ids_in_use(&server, 0).await; } @@ -3484,8 +3708,12 @@ async fn report_backend_idle_and_reap( .orchestrator() .set_backend_liveness(&(service.to_string(), ip, None, port), false) .await; - let mut guard = server.services().write().await; - reap_idle_backend_chains(&mut guard, server.orchestrator(), std::time::Duration::ZERO).await; + reap_idle_backend_chains( + server.services(), + server.orchestrator(), + std::time::Duration::ZERO, + ) + .await; } /// The Step 4 path: once the initiator's connections on the trigger port are @@ -3557,9 +3785,12 @@ async fn a_second_reap_pass_releases_nothing_more() { .set_backend_liveness(&("A".to_string(), ip(1, 1, 1, 1), None, 5555), false) .await; for _ in 0..3 { - let mut guard = server.services().write().await; - reap_idle_backend_chains(&mut guard, server.orchestrator(), std::time::Duration::ZERO) - .await; + reap_idle_backend_chains( + server.services(), + server.orchestrator(), + std::time::Duration::ZERO, + ) + .await; } assert_eq!( @@ -4004,7 +4235,7 @@ async fn a_failed_edge_rollback_keeps_a_concurrent_holder() { teardowns sent: {torn:?})" ); // No assertion on the pool here: host b never acks its teardown either, so - // the id is deliberately held until `TEARDOWN_ACK_GRACE` expires. What must + // the id is deliberately held until cleanup or disconnect. What must // hold immediately is that no entry is left behind for it. let _ = in_use; assert_eq!( @@ -4614,9 +4845,12 @@ port = 6666 .orchestrator() .set_backend_liveness(&("source".into(), source_ip, None, 5555), false) .await; - let mut guard = server.services().write().await; - reap_idle_backend_chains(&mut guard, server.orchestrator(), std::time::Duration::ZERO).await; - drop(guard); + reap_idle_backend_chains( + server.services(), + server.orchestrator(), + std::time::Duration::ZERO, + ) + .await; assert_net_ids_in_use(&server, 0).await; } @@ -4687,3 +4921,50 @@ port = 5555 assert_net_ids_in_use(&server, 0).await; assert_eq!(db.sessions().count_active(TEST_STACK).await.unwrap(), 0); } + +#[tokio::test] +async fn backend_packets_are_released_only_after_both_setup_acks() { + let services = load_fixture(BACKEND_LIVENESS).await; + let server = NullnetGrpcImpl::new_for_test(services); + register_services( + &server, + &HashMap::from([("A", ip(1, 1, 1, 1)), ("B", ip(2, 2, 2, 2))]), + 8080, + ) + .await; + let initiator = server + .orchestrator() + .register_recording_client(ip(1, 1, 1, 1)) + .await; + let (receiver, gate) = server + .orchestrator() + .register_gated_client(ip(2, 2, 2, 2)) + .await; + let task_server = server.clone(); + let build = tokio::spawn(async move { + task_server + .handle_backend_trigger("A", 5555, ip(1, 1, 1, 1), None) + .await + }); + wait_for_log(&receiver, |messages| !setup_net_ids(messages).is_empty()).await; + assert!( + !initiator + .lock() + .await + .iter() + .any(|message| matches!(message.message, Some(net_message::Message::NetReady(_)))) + ); + gate.add_permits(1); + build.await.unwrap().unwrap(); + let messages = initiator.lock().await; + let setup = setup_net_ids(&messages); + let ready: Vec<_> = messages + .iter() + .filter_map(|message| match &message.message { + Some(net_message::Message::NetReady(ready)) => Some(ready.net_id), + _ => None, + }) + .collect(); + assert_eq!(ready.len(), 1); + assert!(setup.contains(&ready[0])); +} diff --git a/members/nullnet-server/src/timeout.rs b/members/nullnet-server/src/timeout.rs index 8c43642..1ab1842 100644 --- a/members/nullnet-server/src/timeout.rs +++ b/members/nullnet-server/src/timeout.rs @@ -2,6 +2,7 @@ use crate::orchestrator::Orchestrator; use crate::services::changes::{ ServiceChange, apply_changes, dep_chain_has_pending, release_backend_chain, }; +use crate::services::clients::Client; use crate::services::input::StackMap; use crate::services::service_info::ServiceInfo; use std::collections::HashMap; @@ -65,19 +66,63 @@ pub(crate) async fn check_timeouts( .reap_idle_egress_edges(LIVENESS_REAP_DEBOUNCE) .await; + reap_idle_backend_chains(&services, &orchestrator, LIVENESS_REAP_DEBOUNCE).await; + reap_ingress_timeouts(&services, &orchestrator).await; let mut services_mut = services.write().await; - reap_idle_backend_chains(&mut services_mut, &orchestrator, LIVENESS_REAP_DEBOUNCE).await; - let stack_names: Vec = services_mut.keys().cloned().collect(); - for stack in stack_names { - if let Some(stack_map) = services_mut.get_mut(&stack) { - apply_timeouts(stack_map, &orchestrator, &stack).await; - } - } crate::services::service_info::reconcile_container_pauses(&mut services_mut, &orchestrator) .await; } } +pub(crate) async fn reap_ingress_timeouts( + services: &RwLock, + orchestrator: &Orchestrator, +) { + let candidates: Vec<_> = services + .read() + .await + .iter() + .flat_map(|(stack, services)| { + collect_timed_out_clients(services) + .into_iter() + .map(|change| (stack.clone(), change)) + }) + .collect(); + for (stack, change) in candidates { + // Keep each chain transaction ordered, but let routing run between clients. + let mut guard = services.write().await; + if let Some(services) = guard.get_mut(&stack) { + apply_timeout_candidate(change, services, orchestrator, &stack).await; + } + } +} + +async fn apply_timeout_candidate( + change: ServiceChange, + services: &mut HashMap, + orchestrator: &Orchestrator, + stack: &str, +) { + if let ServiceChange::ProxyClientTimedOut { name, client } = &change + && client_timed_out(services, name, client) + { + apply_changes(vec![change], services, None, orchestrator, stack).await; + } +} + +fn client_timed_out(services: &HashMap, name: &str, client: &Client) -> bool { + let Some(ServiceInfo::Registered(reg)) = services.get(name) else { + return false; + }; + let Some(timeout) = services[name].timeout().filter(|timeout| *timeout != 0) else { + return false; + }; + reg.proxy_client_expired(client, Duration::from_secs(timeout)) + && !reg.client_replica(client).is_some_and(|(ip, docker)| { + dep_chain_has_pending(name, ip, docker.as_deref(), services) + }) +} + /// Release the hold of every trigger chain whose connections are provably gone. /// /// Each session holds exactly one refcount, so this decrements by one — never a @@ -85,38 +130,46 @@ pub(crate) async fn check_timeouts( /// same edges. Taking the session out of the map is what makes it happen /// exactly once, however many duplicate close reports arrive. pub(crate) async fn reap_idle_backend_chains( - services: &mut StackMap, + services: &RwLock, orchestrator: &Orchestrator, debounce: Duration, ) { - for (key, stack) in orchestrator.take_due_backend_sessions(debounce).await { + let mut services = services.write().await; + let expired = orchestrator.take_due_backend_sessions(debounce).await; + for (key, stack, _) in &expired { let (service, ip, docker, port) = key; - let Some(stack_map) = services.get_mut(&stack) else { + let Some(stack_map) = services.get_mut(stack) else { continue; }; println!( "Trigger chain '{service}' ({ip}) port {port} idle past the debounce; releasing its hold" ); release_backend_chain( - &service, - ip, + service, + *ip, docker.as_deref(), - port, + *port, stack_map, orchestrator, ) .await; } + // Topology is settled; row IDs keep delayed closes scoped to old generations. + drop(services); + for (_, _, history_id) in expired { + orchestrator.sessions.close_backend(history_id).await; + } } +#[cfg(test)] pub(crate) async fn apply_timeouts( services: &mut HashMap, orchestrator: &Orchestrator, stack: &str, ) { let changes = collect_timed_out_clients(services); - if !changes.is_empty() { - apply_changes(changes, services, None, orchestrator, stack).await; + for change in changes { + apply_timeout_candidate(change, services, orchestrator, stack).await; } } @@ -178,3 +231,36 @@ fn nearest_timeout(services: &HashMap) -> Duration { nearest } + +#[cfg(test)] +mod tests { + use super::*; + + #[tokio::test] + async fn stale_expiry_candidates_preserve_renewed_connections_and_grace() { + let server = crate::tests::proxy_timeout_setup().await; + tokio::time::sleep(Duration::from_millis(1100)).await; + let candidates = collect_timed_out_clients(&server.services().read().await["default"]); + assert_eq!(candidates.len(), 2); + let proxy1 = "5.5.5.5".parse().unwrap(); + let proxy2 = "6.6.6.6".parse().unwrap(); + server + .handle_proxy_request("A", proxy1, "10.0.0.1") + .await + .unwrap(); + server + .handle_proxy_request("A", proxy2, "10.0.0.2") + .await + .unwrap(); + server.mark_connection_closed("A", proxy2, "10.0.0.2").await; + let mut guard = server.services().write().await; + let services = guard.get_mut("default").unwrap(); + for candidate in candidates { + apply_timeout_candidate(candidate, services, server.orchestrator(), "default").await; + } + let ServiceInfo::Registered(reg) = &services["A"] else { + unreachable!() + }; + assert_eq!(reg.client_count(), 2); + } +} diff --git a/members/nullnet-server/ui/src/pages/Events.tsx b/members/nullnet-server/ui/src/pages/Events.tsx index 586a26a..56a8db4 100644 --- a/members/nullnet-server/ui/src/pages/Events.tsx +++ b/members/nullnet-server/ui/src/pages/Events.tsx @@ -13,6 +13,9 @@ const SEVERITY_COLOR: Record = { }; const KIND_LABELS: Record = { + event_persistence_overflow: 'event_persistence_overflow', + event_persistence_failed: 'event_persistence_failed', + event_persistence_recovered: 'event_persistence_recovered', // Server events node_connected: 'node_connected', node_disconnected: 'node_disconnected', @@ -102,6 +105,12 @@ const ALL_KINDS = Object.keys(KIND_LABELS); function eventDetail(e: EventJson): string { switch (e.type) { + case 'event_persistence_overflow': + return `${e.dropped_events} events dropped because persistence could not keep up`; + case 'event_persistence_failed': + return `${e.queued_events} queued · ${e.error_message}`; + case 'event_persistence_recovered': + return `Persistence recovered · ${e.queued_events} events draining`; case 'node_connected': case 'node_disconnected': return e.ip; @@ -127,7 +136,7 @@ function eventDetail(e: EventJson): string { case 'session_torn_down': return `net ${e.net_id} · ${e.service} · ${e.client_ip}`; case 'net_teardown_unconfirmed': - return `net ${e.net_id} · ${e.node_ip} never confirmed teardown`; + return `net ${e.net_id} · ${e.node_ip} teardown overdue; ID kept reserved`; case 'observation_changed': return `${e.stack} · observation #${e.observation_id} ${e.action}`; case 'config_reloaded': diff --git a/members/nullnet-server/ui/src/types.ts b/members/nullnet-server/ui/src/types.ts index 804bb98..38ed6e2 100644 --- a/members/nullnet-server/ui/src/types.ts +++ b/members/nullnet-server/ui/src/types.ts @@ -128,6 +128,9 @@ export type Severity = 'info' | 'warning' | 'error'; type WithSeverity = { severity: Severity; timestamp: number }; export type EventJson = + | WithSeverity & { type: 'event_persistence_overflow'; dropped_events: number } + | WithSeverity & { type: 'event_persistence_failed'; error_message: string; queued_events: number } + | WithSeverity & { type: 'event_persistence_recovered'; queued_events: number } // Existing server events | WithSeverity & { type: 'node_connected'; ip: string } | WithSeverity & { type: 'node_disconnected'; ip: string } diff --git a/setup-client.sh b/setup-client.sh index aff5604..bc6b078 100755 --- a/setup-client.sh +++ b/setup-client.sh @@ -1,17 +1,44 @@ #!/bin/bash -apt install sudo -sudo apt-get update && \ -sudo apt-get install -y iptables conntrack ipset openvswitch-switch unzip build-essential && \ +configure_conntrack() { + local conntrack_limit + modprobe nf_conntrack && \ + conntrack_limit=$(sysctl -n net.netfilter.nf_conntrack_max) && \ + conntrack_limit=$((conntrack_limit > 1048576 ? conntrack_limit : 1048576)) && \ + printf 'nf_conntrack\n' | tee /etc/modules-load.d/nullnet-conntrack.conf >/dev/null && \ + printf 'net.netfilter.nf_conntrack_max = %s\n' "$conntrack_limit" | \ + tee /etc/sysctl.d/99-nullnet-conntrack.conf >/dev/null && \ + sysctl -p /etc/sysctl.d/99-nullnet-conntrack.conf +} + +configure_network_manager() { + install -D -m 644 members/nullnet-client/nullnet-networkmanager.conf \ + /etc/NetworkManager/conf.d/90-nullnet.conf && \ + if systemctl is-active --quiet NetworkManager; then + nmcli general reload conf + fi +} + +configure_hotplug() { + install -D -m 644 members/nullnet-client/nullnet-udev.rules \ + /etc/udev/rules.d/90-nullnet.rules && \ + udevadm control --reload +} + +apt-get update && \ +apt-get install -y iptables conntrack ipset openvswitch-switch unzip build-essential kmod procps udev && \ +configure_conntrack && \ +configure_network_manager && \ +configure_hotplug && \ { command -v protoc >/dev/null || { \ curl -OL https://github.com/google/protobuf/releases/download/v3.20.3/protoc-3.20.3-linux-x86_64.zip && \ unzip -o protoc-3.20.3-linux-x86_64.zip -d protoc3 && \ - sudo mv protoc3/bin/* /usr/local/bin/ && \ - sudo mv protoc3/include/* /usr/local/include/ ; }; } && \ + mv protoc3/bin/* /usr/local/bin/ && \ + mv protoc3/include/* /usr/local/include/ ; }; } && \ cargo install cargo-binstall && \ cargo binstall -y bpf-linker && \ git pull && \ cargo xtask build --release && \ -sudo cp members/nullnet-client/nullnet-client.service /etc/systemd/system/ && \ -sudo systemctl enable nullnet-client && \ -sudo systemctl restart nullnet-client +cp members/nullnet-client/nullnet-client.service /etc/systemd/system/ && \ +systemctl enable nullnet-client && \ +systemctl restart nullnet-client diff --git a/setup-proxy.sh b/setup-proxy.sh index 53cac17..db8872f 100755 --- a/setup-proxy.sh +++ b/setup-proxy.sh @@ -1,15 +1,14 @@ #!/bin/bash -apt install sudo -sudo apt-get update && \ -sudo apt-get install -y unzip pkg-config libssl-dev build-essential && \ +apt-get update && \ +apt-get install -y unzip pkg-config libssl-dev build-essential && \ { command -v protoc >/dev/null || { \ curl -OL https://github.com/google/protobuf/releases/download/v3.20.3/protoc-3.20.3-linux-x86_64.zip && \ unzip -o protoc-3.20.3-linux-x86_64.zip -d protoc3 && \ - sudo mv protoc3/bin/* /usr/local/bin/ && \ - sudo mv protoc3/include/* /usr/local/include/ ; }; } && \ + mv protoc3/bin/* /usr/local/bin/ && \ + mv protoc3/include/* /usr/local/include/ ; }; } && \ git pull && \ cargo build -p nullnet-proxy --release && \ -sudo cp members/nullnet-proxy/nullnet-proxy.service /etc/systemd/system/ && \ -sudo systemctl enable nullnet-proxy && \ -sudo systemctl restart nullnet-proxy +cp members/nullnet-proxy/nullnet-proxy.service /etc/systemd/system/ && \ +systemctl enable nullnet-proxy && \ +systemctl restart nullnet-proxy diff --git a/setup-server.sh b/setup-server.sh index 1ec6f47..31b5835 100755 --- a/setup-server.sh +++ b/setup-server.sh @@ -1,18 +1,17 @@ #!/bin/bash -apt install sudo -sudo apt-get update && \ -sudo apt-get install -y unzip pkg-config libssl-dev cmake build-essential && \ +apt-get update && \ +apt-get install -y unzip pkg-config libssl-dev cmake build-essential && \ { command -v protoc >/dev/null || { \ curl -OL https://github.com/google/protobuf/releases/download/v3.20.3/protoc-3.20.3-linux-x86_64.zip && \ unzip -o protoc-3.20.3-linux-x86_64.zip -d protoc3 && \ - sudo mv protoc3/bin/* /usr/local/bin/ && \ - sudo mv protoc3/include/* /usr/local/include/ ; }; } && \ + mv protoc3/bin/* /usr/local/bin/ && \ + mv protoc3/include/* /usr/local/include/ ; }; } && \ git pull && \ { command -v npm >/dev/null || { \ - curl -fsSL https://deb.nodesource.com/setup_20.x | sudo -E bash - && \ - sudo apt-get install -y nodejs ; }; } && \ + curl -fsSL https://deb.nodesource.com/setup_20.x | bash - && \ + apt-get install -y nodejs ; }; } && \ cargo build -p nullnet-server --release && \ -sudo cp members/nullnet-server/nullnet-server.service /etc/systemd/system/ && \ -sudo systemctl enable nullnet-server && \ -sudo systemctl restart nullnet-server +cp members/nullnet-server/nullnet-server.service /etc/systemd/system/ && \ +systemctl enable nullnet-server && \ +systemctl restart nullnet-server