Wire tap+relay fallback into join_one_network()
join_one_network() now branches on should_use_veth(network) (network_bridge.h): the existing veth-pair path when the kernel supports veth and the network wasn't created with --no-veth, or create_tap_relay() (network_tap_relay.h) otherwise -- both produce the same postcondition (a ready eth<N> in the container's namespace) before the unchanged IP-assignment/route code runs. JoinedNetwork gains an optional `relay` field; run_container() (commands.cpp) collects these into active_relays (mirroring active_port_forwards) and calls stop_tap_relay() for each after run_bwrap() returns. Two real bugs caught by testing while verifying this end-to-end: 1. The relay child, unlike every other forked child in this project, never exec()s, so O_CLOEXEC on fds created before its fork (like daemonize.cpp's own report pipe) never takes effect -- it only closes fds across exec(), not across a fork that never execs. The relay inherited a live copy of that pipe's write end and kept it open forever, so `-D` combined with `-n <no-veth network>` hung indefinitely (the daemonize handshake's read-until-EOF never saw EOF). Fixed with close_inherited_fds(), scanning /proc/self/fd and closing everything except stdio and the relay's own report pipe, as the first thing relay_child_main() does. 2. A first-attempt companion fix -- adding the relay's pid to the session's own cgroup so --kill would reach it directly -- was tried and reverted: remove_session_cgroup() runs inside run_bwrap(), before run_container() gets to call stop_tap_relay(), so the cgroup was still non-empty at removal time and every such session left a stray cgroup directory behind (EBUSY, confirmed by testing). The ordinary flow already stops the relay correctly (killing bwrap lets run_container() reach its own cleanup), so this wasn't worth the added complexity. Verified end-to-end as root via the doas rule, using a --no-veth extern network on this dev machine specifically to exercise the fallback: two containers joined the same network, got distinct addresses via two tap devices + relays (no veth at all), and pinged each other with 0% packet loss, repeatably. Known gap, confirmed by testing, not yet root-caused: neither container could reach the network's own gateway IP (ICMP or TCP), despite ARP resolving correctly -- ruling out an L2/relay framing problem. The identical bridge/subnet/host reached via veth instead works perfectly, ruling out every environment-level explanation that would affect both paths equally. rp_filter=0 (host-tap, bridge, and `all` scope) was tried and confirmed not to fix it. Diagnosing further needs host tools (tcpdump, direct iptables/sysctl inspection) this session's doas access doesn't permit. Peer-to-peer connectivity (an intern network's whole purpose) is solid; gateway/outside reachability through this fallback needs re-verification, ideally on the actual veth-less target device, before being relied on. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gv3s5jckJKzh6JkMoi2Akz
This commit is contained in:
@@ -271,7 +271,14 @@ Source layout (all under `src/`):
|
||||
`remove_port_forward_record(container_name, bwrap_pid)` (`bwrap_pid`
|
||||
captured from the same callback, in a variable declared in
|
||||
`run_container()`'s own scope) so a cleanly-exiting session's own record
|
||||
doesn't linger for `--clean-processes` to find later.
|
||||
doesn't linger for `--clean-processes` to find later. The same callback
|
||||
also collects each `JoinedNetwork::relay` (`network_join.h`) returned by
|
||||
`join_networks()` into a `std::vector<TapRelayHandle> active_relays`
|
||||
declared in `run_container()`'s own scope — the direct tap+relay
|
||||
(`network_tap_relay.h`) analog of `active_port_forwards` above, since a
|
||||
relay process is likewise independent host-global state (unlike a veth
|
||||
pair) that needs an explicit `stop_tap_relay()` call for each, made right
|
||||
alongside the `remove_port_forward()` loop after `run_bwrap()` returns.
|
||||
- `self_test.{h,cpp}` — `run_self_tests()` implements `-t/--test`, this
|
||||
project's own built-in self-test mode (distinct from the Meson-driven
|
||||
fixture smoke test under `tests/`, described in "Build & test commands"
|
||||
@@ -690,12 +697,29 @@ Source layout (all under `src/`):
|
||||
networked services are unaffected — confirmed by testing (see below).
|
||||
For each named network: looked up in `config.networks` (an unknown name is
|
||||
a per-network error, not fatal to the others); `ensure_network_provisioned()`
|
||||
(`network_bridge.h`) covers post-reboot recreation; a veth pair is created
|
||||
wherever that network's bridge lives (`wrap_for_network()`, reused from
|
||||
`network_bridge.h`), the bridge-side end attached and brought up, the
|
||||
container-side end moved into the session's own namespace (`ip link set
|
||||
... netns <ns_pid>`) and renamed `eth<N>` (`N` = the network's position in
|
||||
the `-n` list, so multiple joins each get a distinct interface). **Address
|
||||
(`network_bridge.h`) covers post-reboot recreation; then, per
|
||||
`should_use_veth(network)` (`network_bridge.h` — combines the network's own
|
||||
`veth` policy flag with a kernel-capability probe, see that file's own
|
||||
entry above), either a veth pair is created wherever that network's bridge
|
||||
lives (`wrap_for_network()`, reused from `network_bridge.h`), the
|
||||
bridge-side end attached and brought up, the container-side end moved into
|
||||
the session's own namespace (`ip link set ... netns <ns_pid>`) and renamed
|
||||
`eth<N>`, **or**, when veth isn't available or the network was created
|
||||
with `--no-veth`, `create_tap_relay()` (`network_tap_relay.h`, see below)
|
||||
is used instead, producing the same end state (a ready `eth<N>` in the
|
||||
container's namespace) via two tap devices and a relay process rather than
|
||||
a kernel veth pair. `eth<N>`'s `N` is the network's position in the `-n`
|
||||
list, so multiple joins each get a distinct interface, regardless of which
|
||||
strategy created it — everything downstream (IP assignment, routes, the
|
||||
address returned to `port_forward.h`) is identical either way, since it
|
||||
only ever operates on `eth<N>` by name. Unlike a veth pair (torn down by
|
||||
the kernel automatically once the session's namespace goes away, whatever
|
||||
else fails), a tap relay is an independent process with no such automatic
|
||||
cleanup — if any step after `create_tap_relay()` succeeds fails later in
|
||||
`join_one_network()` (address exhaustion, a failed `ip addr add`/route
|
||||
command), a `fail()` helper (`.cpp`-local, only present when a relay was
|
||||
actually created) calls `stop_tap_relay()` before returning `nullopt`, so
|
||||
a partial failure doesn't leak the relay process. **Address
|
||||
allocation, `pick_free_address()`, needed a real fix during testing, not
|
||||
just design**: an interface's actual assigned IP lives inside its own
|
||||
private per-container namespace, invisible from the bridge's own namespace
|
||||
@@ -721,23 +745,29 @@ Source layout (all under `src/`):
|
||||
route command needed for same-bridge reachability regardless of kind).
|
||||
Every step failure is logged specifically (which command, which network)
|
||||
and best-effort: `join_networks()` returns one `JoinedNetwork {network,
|
||||
container_ip}` per network that actually joined (in `-n` order, so shorter
|
||||
than the request list on any partial failure), never fatal to the
|
||||
container_ip, relay}` per network that actually joined (in `-n` order, so
|
||||
shorter than the request list on any partial failure), never fatal to the
|
||||
already-running session (network setup can only happen after `bwrap`'s own
|
||||
namespace exists, i.e. potentially after the sandboxed command is already
|
||||
running) — this return value exists specifically for `port_forward.h`
|
||||
(see below) to resolve a `-p` spec against which networks/IPs are actually
|
||||
usable, not as a pass/fail signal on its own. An empty `network_names`
|
||||
returns immediately (no namespace wait at all), so callers that always
|
||||
invoke this once `on_bwrap_pid_known` fires for any reason (`commands.cpp`
|
||||
also fires it for `-D/--daemonize` alone, with no `-n`) don't pay for a
|
||||
wait that has nothing to do. Veth teardown
|
||||
usable, not as a pass/fail signal on its own; `relay` (`nullopt` for a
|
||||
veth-joined network) is what `run_container()` (`commands.cpp`) collects
|
||||
to call `stop_tap_relay()` on after `run_bwrap()` returns, mirroring how it
|
||||
already collects `active_port_forwards` for `-p`'s own cleanup. An empty
|
||||
`network_names` returns immediately (no namespace wait at all), so callers
|
||||
that always invoke this once `on_bwrap_pid_known` fires for any reason
|
||||
(`commands.cpp` also fires it for `-D/--daemonize` alone, with no `-n`)
|
||||
don't pay for a wait that has nothing to do. Veth teardown
|
||||
needs no explicit code: the kernel destroys an entire veth pair (both
|
||||
ends, including the one still attached to the bridge) the instant *either*
|
||||
end's owning namespace is destroyed, so a session's veths disappear on
|
||||
their own once its namespace does — only the bridge/iptables/persistent-
|
||||
namespace state is deliberately left behind (`network_bridge.h`'s
|
||||
reboot-reconciliation design). **Verified end-to-end on this dev machine
|
||||
reboot-reconciliation design); a tap relay instead needs the explicit
|
||||
`stop_tap_relay()` call described above, since it's an independent process
|
||||
with no namespace of its own to be torn down by. **Verified end-to-end on
|
||||
this dev machine
|
||||
(root, via a scoped `doas` rule)**: two concurrently-running containers on
|
||||
the same `intern` network got distinct addresses and could ping each
|
||||
other; an `intern`-joined container could not reach the outside
|
||||
@@ -907,14 +937,74 @@ Source layout (all under `src/`):
|
||||
`container_if_name` is a completely ordinary interface from the
|
||||
container's own point of view — `join_one_network()`'s existing IP
|
||||
assignment/route/DNAT-target-address code (unchanged, not yet wired to
|
||||
call this) needs no changes at all. **Not yet wired into
|
||||
`join_one_network()`/`run_container()`** — see `self_test.{h,cpp}` above
|
||||
for how this file's own create/attach/teardown cycle was verified
|
||||
end-to-end in isolation first; full traffic-plane verification (does
|
||||
`-p`/inter-container connectivity actually work end-to-end through this
|
||||
path) is deferred to the commit that does that wiring, where two real
|
||||
`--no-veth`-joined containers pinging/curling each other is a more natural
|
||||
and simpler proof than hand-rolled raw-socket test traffic.
|
||||
call this) needs no changes at all. See `self_test.{h,cpp}` above for how
|
||||
this file's own create/attach/teardown cycle was verified end-to-end in
|
||||
isolation first, before being wired into `join_one_network()`.
|
||||
|
||||
**Real fd-leak bug caught by direct testing, not assumed**: the relay
|
||||
child, unlike every other forked child elsewhere in this project, never
|
||||
`exec()`s — so `O_CLOEXEC` on fds created *before* this fork (e.g.
|
||||
`daemonize.cpp`'s own report-pipe write end, still open in the forking
|
||||
process at this point since `report_daemon_started()` — which closes it —
|
||||
hasn't run yet when `join_networks()` is called) never takes effect,
|
||||
since it only closes fds *across `exec()`*, not across a fork that never
|
||||
execs. Without a fix, the relay child inherited a live copy of that pipe's
|
||||
write end and never closed it, so `daemonize()`'s read-until-EOF in the
|
||||
*original, pre-fork* process blocked forever, even after
|
||||
`report_daemon_started()` closed its own copy — a pipe only reports EOF
|
||||
once *every* copy of its write end, across every process, is closed.
|
||||
Confirmed directly: `-r -D -n <no-veth network> -- sleep 600` hung
|
||||
indefinitely; killing the session (which reaches `stop_tap_relay()` via
|
||||
`run_container()`'s own post-`run_bwrap()` cleanup, closing the leaked
|
||||
copy) immediately unblocked the original process. Fixed by
|
||||
`close_inherited_fds()` (`.cpp`-local): scans `/proc/self/fd` and closes
|
||||
everything except stdin/stdout/stderr and the report pipe's own write
|
||||
end, called as the very first thing in `relay_child_main()`. A
|
||||
first-attempt companion fix — adding the relay's pid to the session's own
|
||||
cgroup (`session_cgroup.h`) so `--kill` would reach it directly, since a
|
||||
relay is a *sibling* of bwrap rather than a descendant and so would never
|
||||
inherit cgroup membership on its own — was tried and then **reverted**:
|
||||
`remove_session_cgroup()` runs *inside* `run_bwrap()`, before
|
||||
`run_container()` ever gets to call `stop_tap_relay()`, so the cgroup was
|
||||
still non-empty (the relay still in it) at removal time, and every
|
||||
session using this fallback left a stray, never-removed cgroup directory
|
||||
behind (`rmdir` failing with `EBUSY`, confirmed by testing). Since the
|
||||
ordinary flow already stops the relay correctly on its own (killing bwrap
|
||||
unblocks `run_bwrap()`'s own `waitpid()`, letting `run_container()`
|
||||
finish its normal cleanup, `stop_tap_relay()` included) and the only gap
|
||||
left by *not* doing this is a benign, self-resolving race in `--kill`'s
|
||||
own "has it fully stopped" check (a genuine crash of the whole session
|
||||
process, not just bwrap, is a separate, already-scoped concern — see the
|
||||
crash-orphan sweep below), the added complexity wasn't worth it.
|
||||
|
||||
**Verified end-to-end on this dev machine (root, via the scoped `doas`
|
||||
rule), using a `--no-veth` `extern` network specifically to exercise this
|
||||
path**: two real containers joined the same network, each getting a
|
||||
distinct address (`10.168.0.2`/`10.168.0.3`) via the tap+relay path with
|
||||
no veth involved at all, and pinged each other successfully (0% packet
|
||||
loss, confirmed repeatably). **Known gap, confirmed by testing, root
|
||||
cause not yet identified**: neither container could reach the network's
|
||||
own gateway IP (the address `provision_bridge()` assigns directly to the
|
||||
bridge itself) — not with ICMP, not with a TCP `wget` — despite ARP
|
||||
resolving correctly (confirmed via the container's own `ip neigh`/
|
||||
`/proc/net/arp` showing a `REACHABLE` entry for the gateway's real MAC),
|
||||
meaning L2 delivery through the relay is not at fault. The *exact same*
|
||||
bridge, same subnet, same host, same MASQUERADE/`ip_forward` setup, but
|
||||
joined via veth instead of this fallback, reaches the same gateway IP
|
||||
perfectly (0% loss) — ruling out every environment-level explanation
|
||||
(host firewall, `rp_filter`, this dev sandbox's own networking) that would
|
||||
otherwise affect both paths identically. `net.ipv4.conf.<host-tap>
|
||||
.rp_filter=0` (and broader `all`/bridge-scoped variants) was tried and
|
||||
confirmed *not* to fix it. Root cause undetermined — diagnosing further
|
||||
needs host-level tools (`tcpdump`, direct `iptables`/`sysctl` inspection)
|
||||
this session's `doas` access doesn't permit (restricted to running
|
||||
`slocker-lite` itself only). **This means gateway/outside reachability
|
||||
through the tap+relay fallback is unconfirmed** — peer-to-peer
|
||||
connectivity (an `intern` network's whole purpose, and half of what an
|
||||
`extern` network offers) is solid; reaching the real internet through one
|
||||
specifically needs re-verification, ideally on the actual veth-less
|
||||
target device (a different kernel/environment where this dev sandbox's
|
||||
own unidentified cause may not even apply) before being relied on.
|
||||
- `session_cgroup.{h,cpp}` — gives `--kill` (`kill_session.{h,cpp}`, see
|
||||
below) a reliable way to find every process a session ever started, however
|
||||
deeply forked/daemonized/reparented, by putting it in a dedicated cgroup v2
|
||||
|
||||
Reference in New Issue
Block a user