[PR] feat(instance): per-instance identity + reject placeholder next-hop + router-id defaulting #233

New issue

Closed

opened 2026-07-31 07:01:36 +00:00 by vxfemboy · 2 comments

vxfemboy commented

2026-07-31 07:01:36 +00:00

Owner

Mirrored from GitHub — originally by @vxfemboy. https://github.com/femboyisp/blackwall/pull/199

Why

Two arming defects surfaced during the 2026-07-31 box work.

1. A second blackwalld instance can't coexist with the first. Covering a second ingress path (e.g. a separate IX-peering NIC) with its own daemon was blocked: both instances share three global kernel resources and clobber each other — the nft table (every apply does add+flush, so they flush-wipe each other's rules), the deception TPROXY fwmark, and its policy route table (teardown runs ip rule del + ip route flush, so one stopping rips out the other's plumbing).

2. Arming with a placeholder blackhole next-hop fails silently. An rtbh next-hop left at a documentation/discard template value (192.0.2.1, 100::1, …) never resolves to a real discard route, so BIRD leaves the /32 unreachable and never exports it — the daemon shows Established yet announces nothing.

What

instance=<name> (optional top-level directive) → blackwall_core::InstanceIds derives all three resources per-instance: table blackwall_<name>, and a fwmark + route-table id taken from independent 32-bit halves of a 64-bit hash of the name (mark bit31 set → never 0x1, never == route id; route id ≥ 0x4000_0000 → never 100). Absent = today's exact identity (blackwall / 0x1 / 100) — fully backward-compatible. Threaded through nft render + apply/teardown (now takes &Policy), plus a new blackwalld teardown --config subcommand so the runit crash backstop cleans up only its own instance.

Placeholder next-hop guard — RtbhPolicy::placeholder_next_hops() flags RFC 5737 / 3849 / 6666 ranges; the flow daemon refuses to start armed (not-shadow) if any rtbh next-hop is one. Shadow is exempt.

Review

An independent review verified both Rust goals correct end-to-end (all three resources namespaced, stamp-mark == match-mark, guard on the live path only, range checks correct, all 14 mechanical instance: None insertions landed right) and caught two issues, both fixed in 33e004e:

CRITICAL: the runit finish crash script still hardcoded the default identity → a named instance's crash would wipe the default instance's live dataplane. Now delegates to blackwalld teardown.
MEDIUM: mark + route-table shared one 16-bit hash slot (~1/65536 silent collision) → now independent 64-bit halves.

Tests

InstanceIds derivation (defaults / namespacing / distinctness / stability), instance= parse (set/default/empty/duplicate), named-instance table rename, placeholder-next-hop detection. Workspace clippy -D warnings + fmt clean.

Still the operator's decision (not in this PR)

The real fleet blackhole next-hop value (#4) — this PR only stops the placeholder shipping silently. Router-id validation in bird-config (#232) is a noted fast-follow.

> _Mirrored from GitHub — originally by `@vxfemboy`._ [https://github.com/femboyisp/blackwall/pull/199](https://github.com/femboyisp/blackwall/pull/199) ## Why Two arming defects surfaced during the 2026-07-31 box work. **1. A second `blackwalld` instance can't coexist with the first.** Covering a second ingress path (e.g. a separate IX-peering NIC) with its own daemon was blocked: both instances share three global kernel resources and clobber each other — the nft table (every apply does `add`+`flush`, so they flush-wipe each other's rules), the deception TPROXY fwmark, and its policy route table (teardown runs `ip rule del` + `ip route flush`, so one stopping rips out the other's plumbing). **2. Arming with a placeholder blackhole next-hop fails silently.** An `rtbh` next-hop left at a documentation/discard template value (`192.0.2.1`, `100::1`, …) never resolves to a real discard route, so BIRD leaves the `/32` `unreachable` and never exports it — the daemon shows `Established` yet announces nothing. ## What **`instance=<name>`** (optional top-level directive) → `blackwall_core::InstanceIds` derives all three resources per-instance: table `blackwall_<name>`, and a fwmark + route-table id taken from independent 32-bit halves of a 64-bit hash of the name (mark bit31 set → never `0x1`, never == route id; route id ≥ `0x4000_0000` → never `100`). **Absent = today's exact identity (`blackwall` / `0x1` / `100`)** — fully backward-compatible. Threaded through nft `render` + `apply`/`teardown` (now takes `&Policy`), plus a new `blackwalld teardown --config` subcommand so the runit crash backstop cleans up **only its own** instance. **Placeholder next-hop guard** — `RtbhPolicy::placeholder_next_hops()` flags RFC 5737 / 3849 / 6666 ranges; the flow daemon refuses to start armed (not-shadow) if any rtbh next-hop is one. Shadow is exempt. ## Review An independent review verified both Rust goals correct end-to-end (all three resources namespaced, stamp-mark == match-mark, guard on the live path only, range checks correct, all 14 mechanical `instance: None` insertions landed right) and caught two issues, both fixed in `33e004e`: - **CRITICAL**: the runit `finish` crash script still hardcoded the default identity → a named instance's crash would wipe the *default* instance's live dataplane. Now delegates to `blackwalld teardown`. - **MEDIUM**: mark + route-table shared one 16-bit hash slot (~1/65536 silent collision) → now independent 64-bit halves. ## Tests InstanceIds derivation (defaults / namespacing / distinctness / stability), `instance=` parse (set/default/empty/duplicate), named-instance table rename, placeholder-next-hop detection. Workspace `clippy -D warnings` + `fmt` clean. ## Still the operator's decision (not in this PR) The real fleet blackhole next-hop value (#4) — this PR only stops the placeholder shipping silently. Router-id validation in `bird-config` (#232) is a noted fast-follow.

vxfemboy added the

pull-request

label

2026-07-31 07:01:36 +00:00

vxfemboy changed title from ~~[PR] feat(instance): per-instance kernel identity + reject placeholder RTBH next-hop~~ to [PR] feat(instance): per-instance identity + reject placeholder next-hop + router-id defaulting

2026-07-31 07:17:29 +00:00

vxfemboy commented

2026-07-31 07:17:37 +00:00

Author

Owner

Mirrored from GitHub — originally by @vxfemboy. https://github.com/femboyisp/blackwall/pull/199#issuecomment-5140362742

Added a third arming fix (femboyisp/blackwall#232, commit 0acfc5c): router-id is now optional and defaults to the IPv4 local-addr (blackwall's own address, distinct from the BIRD peer's router-id by construction), and router-id == peer is rejected at parse time. This is the code hardening for the Bad BGP identifier collision hit at arming — same fix the box applied live (router-id → container IP). Unit tests: default-from-local-addr, reject-equal-to-peer, error-when-undeterminable. Workspace clippy -D warnings + fmt clean.

> _Mirrored from GitHub — originally by `@vxfemboy`._ [https://github.com/femboyisp/blackwall/pull/199#issuecomment-5140362742](https://github.com/femboyisp/blackwall/pull/199#issuecomment-5140362742) Added a third arming fix (femboyisp/blackwall#232, commit 0acfc5c): `router-id` is now optional and defaults to the IPv4 `local-addr` (blackwall's own address, distinct from the BIRD peer's router-id by construction), and `router-id == peer` is rejected at parse time. This is the code hardening for the `Bad BGP identifier` collision hit at arming — same fix the box applied live (router-id → container IP). Unit tests: default-from-local-addr, reject-equal-to-peer, error-when-undeterminable. Workspace clippy -D warnings + fmt clean.

vxfemboy commented

2026-07-31 18:54:25 +00:00

Author

Owner

Mirrored from GitHub — originally by @vxfemboy. https://github.com/femboyisp/blackwall/pull/199#issuecomment-5146427871

Moved to Forgejo — superseded by vxug.dev PR #234 (we're off GitHub Actions for CI cost). Branch unchanged.

> _Mirrored from GitHub — originally by `@vxfemboy`._ [https://github.com/femboyisp/blackwall/pull/199#issuecomment-5146427871](https://github.com/femboyisp/blackwall/pull/199#issuecomment-5146427871) Moved to Forgejo — superseded by vxug.dev PR #234 (we're off GitHub Actions for CI cost). Branch unchanged.

vxfemboy closed this issue

2026-07-31 18:54:26 +00:00