Retry container-side tap device setup, external process only

A user's log from the real target device showed nsenter'd `ip addr
add ... dev eth0` failing with "Cannot find device" immediately after
network_tap_relay.h's relay had already created it -- confirmed by
hand that retrying the whole session a few times eventually worked.

join_one_network() now retries (bounded, ~500ms, quiet until final
success/give-up) the three steps that touch the just-created container
interface -- IPv4 address, IPv6 address, bringing it up -- via a new
run_with_retry() instead of plain run().

A tempting first fix was investigated and ruled out by direct A/B
testing, not just reasoned about: having the relay itself self-verify
the device is visible (a same-process check, immediately after
creating it, before ever reporting success) was tried first, in
relay_child_main(). It made things categorically worse: the
container-side device became permanently invisible to every external
nsenter afterward, 100% reproducibly (confirmed with a 10-second retry
budget -- never once became visible), on a mechanism that had
otherwise worked correctly and instantly, zero retries needed, on
every real session tested earlier the same day -- including a
from-scratch self-test reproduction that had passed reliably many
times before this one change, and immediately went back to passing
once it was reverted. Root cause not fully understood (something about
forking a subprocess that inherits the tap fd -- deliberately not
O_CLOEXEC -- while still holding it open, immediately after device
creation, appears to corrupt the device's external visibility on this
kernel specifically), but the fix is unambiguous: never add an
internal, same-process/fd-holding self-check to the relay; only the
external, separate-process retry is safe.

network_tap_relay.cpp ends up completely unchanged -- the actual fix
lives entirely in network_join.cpp's own retry. self_test.cpp's own
container-visibility check needed the same external retry treatment,
for the same underlying reason.

Verified as root via the doas rule: three separate real --no-veth
sessions all succeeded getting eth0 on the first attempt (no retries
triggered), and the self-test passes reliably across repeated runs.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gv3s5jckJKzh6JkMoi2Akz

CLAUDE.md

@@ -333,14 +333,23 @@ Source layout (all under `src/`):
   trusting its pid. Confirms the host-side tap gets created and attached to
   the bridge (`ip link show` output contains `master <bridge>`), the
   container-side tap gets created with the requested name inside the target
-  namespace (checked via `nsenter --net=/proc/<pid>/ns/net -- ip link show`),
-  and — the biggest previously-unverified assumption from
-  `docs/networking-design.md`'s tap+relay addendum — **both devices actually
-  disappear on their own once `stop_tap_relay()` stops the relay process,
-  with no explicit `ip link del` needed** (neither was created with
-  `IFF_PERSIST`) — confirmed directly, twice, on this dev machine (root, via
-  the scoped `doas` rule). Deliberately its own small file since more real
-  tests are expected here as more of the networking feature lands.
+  namespace (checked via `nsenter --net=/proc/<pid>/ns/net -- ip link show`,
+  wrapped in `wait_for_container_device_visible()` — a `.cpp`-local bounded
+  retry, same shape as `network_join.cpp`'s own `run_with_retry()`, added
+  for the same real device-reported reason — see that function's own doc
+  comment for the full story, including the tempting-but-harmful internal-
+  self-check "fix" that was tried and ruled out first, confirmed by this
+  exact self-test: it started reliably failing, 100% of the time, the moment
+  that harmful check was added — despite having passed reliably many times
+  earlier the same session before that change — and immediately went back to
+  passing reliably once the harmful check was removed again), and — the
+  biggest previously-unverified assumption from `docs/networking-design.md`'s
+  tap+relay addendum — **both devices actually disappear on their own once
+  `stop_tap_relay()` stops the relay process, with no explicit `ip link del`
+  needed** (neither was created with `IFF_PERSIST`) — confirmed directly,
+  many times over, on this dev machine (root, via the scoped `doas` rule).
+  Deliberately its own small file since more real tests are expected here as
+  more of the networking feature lands.
 - `env_spec.{h,cpp}` — `resolve_env_specs()` turns an ordered list of
   `EnvSpec {is_file, value}` (see `cli_args.{h,cpp}` above) into a flat, ordered list of
   `(key, value)` pairs. A literal (`--env`) is split at its *first* `=` (the
@@ -853,6 +862,47 @@ Source layout (all under `src/`):
   unaffected (still just loopback, whether or not the kernel happened to
   give it its own otherwise-empty net namespace via the default
   `global.unshare-net` policy).
+
+  **Real bug reported from the real target device (`-n <extern network> --
+  /bin/sh`, tap+relay fallback): the container-side tap device wasn't always
+  immediately visible.** The user's own log showed `nsenter --net=/proc/<ns_pid>
+  /ns/net -- ip addr add 10.168.0.2/24 dev eth0` failing with `"Cannot find
+  device \"eth0\""` right after `network_tap_relay.h`'s relay had already
+  created it — and confirmed by hand that simply retrying the whole session a
+  few times eventually worked. `run_with_retry()` (`.cpp`-local, same
+  shape/spirit as `wait_for_isolated_net_namespace()` below) replaces `run()`
+  for the three steps that touch the just-created `container_if` (IPv4
+  address, IPv6 address, bringing it up) — retries quietly (no per-attempt
+  log spam) for up to 500ms before giving up loudly, same as `run()`'s own
+  single-attempt error.
+
+  **A tempting "fix" investigated and ruled out by direct A/B testing on this
+  dev machine, not just reasoned about**: the obvious first instinct — have
+  the relay *itself* self-verify the device is visible (a same-process check
+  via its own `run_process()` call, immediately after `open_tap()`, before
+  ever reporting success) — was tried first, in `network_tap_relay.cpp`'s
+  `relay_child_main()`. It made things *worse*, not better: it made the
+  container-side device **permanently invisible to every external `nsenter`
+  afterward, 100% reproducibly** (confirmed with a 10-second retry budget —
+  never once became visible), on a mechanism that had otherwise worked
+  correctly and instantly on every single real session tested earlier this
+  same day, with no retries ever needed. Root cause not fully understood
+  (something about forking a subprocess that inherits the tap fd — opened
+  via `open("/dev/net/tun", O_RDWR)`, deliberately *not* `O_CLOEXEC` — while
+  still holding it open, immediately after device creation, appears to
+  corrupt the device's *external* visibility specifically on this kernel;
+  the *same* process's own view of the device it just created stayed correct
+  throughout). Confirmed via direct A/B testing, not just correlation:
+  removing that internal check immediately restored 100%-reliable, zero-retry
+  first-try success on three separate real sessions; re-adding it reliably
+  broke it again, including for a from-scratch self-test reproduction (see
+  `self_test.{h,cpp}` below) that had passed reliably many times earlier in
+  the same session before this specific change. The fix that shipped is
+  unambiguous: never add an internal, same-process/fd-holding self-check to
+  the relay; only the *external*, separate-process retry above
+  (`run_with_retry()`) is safe, and is sufficient on its own — every retried
+  real-device call this session succeeded on the very first attempt once the
+  harmful internal check was gone.
 - `port_forward.{h,cpp}` — implements `-p`. `parse_port_forward_spec()`
   splits `"[<network>:]<host-port>:<container-port>"` on `':'` (2 or 3
   fields; the network name is deliberately restricted to excluding `':'` --

docs/networking-design.md

@@ -524,6 +524,40 @@ the `doas` rule: two freshly created `extern` networks got
 `fdf0:f243:f06f:168::/64` and `fdf0:f243:f06f:169::/64` exactly as
 expected, correctly paired with `10.168.0.0/24`/`10.168.1.0/24`.
 
+## Resolved: container-side tap device intermittently not immediately visible
+
+**Trigger**: a user's own log from the real target device showed
+`nsenter --net=/proc/<ns_pid>/ns/net -- ip addr add 10.168.0.2/24 dev eth0`
+failing with `"Cannot find device \"eth0\""` immediately after the relay had
+already created it — confirmed by hand that retrying the whole session a few
+times eventually let it succeed.
+
+**Fix**: `network_join.cpp`'s `join_one_network()` now retries (bounded,
+~500ms, quiet until final success/give-up) the three steps that touch the
+just-created container interface — IPv4 address, IPv6 address, bringing it
+up — via a new `run_with_retry()` instead of the plain `run()` used
+elsewhere.
+
+**A tempting "fix" investigated and ruled out by direct A/B testing on the
+dev machine, not just reasoned about**: the obvious first instinct — have
+the relay *itself* self-verify the device is visible (a same-process check,
+immediately after creating it, before ever reporting success back) — was
+tried first, in `network_tap_relay.cpp`. It made things categorically worse:
+it made the container-side device **permanently invisible to every external
+`nsenter` afterward, 100% reproducibly** (confirmed with a 10-second retry
+budget — never once became visible), on a mechanism that had otherwise
+worked correctly and instantly, with zero retries needed, on every real
+session tested earlier the same day — including a from-scratch self-test
+reproduction (`self_test.cpp`) that had passed reliably many times before
+this one change, and immediately went back to passing reliably once the
+change was reverted. Root cause not fully understood (something about
+forking a subprocess that inherits the tap fd — deliberately not
+`O_CLOEXEC` — while still holding it open, immediately after device
+creation, appears to corrupt the device's *external* visibility on this
+kernel specifically), but the fix that shipped is unambiguous: never add an
+internal, same-process/fd-holding self-check to the relay; the external,
+separate-process retry above is safe and sufficient on its own.
+
 ## Explicitly out of scope for now
 
 - **Rootless networking.** An earlier draft of this design considered a

src/network_join.cpp

@@ -92,6 +92,50 @@ bool run(const std::vector<std::string>& argv, std::string_view what, std::strin
     return true;
 }
 
+// Same as run() above, but retries (nanosleep, EINTR-retry -- the same
+// shape wait_for_isolated_net_namespace() below already uses) on failure
+// for up to timeout_ms before giving up, quietly (no per-attempt error
+// spam) -- only the final give-up is logged as an error, same as run()'s
+// own single-attempt message. **Real bug reported from the real target
+// device**: right after network_tap_relay.h's relay creates the
+// container-side tap device (via `ioctl(TUNSETIFF)`, reporting success),
+// this function's own caller -- a completely separate process, joining the
+// same namespace fresh via `nsenter` -- could briefly fail to find that
+// device with "Cannot find device"; retrying the whole session by hand a
+// few times let it eventually succeed. This retries automatically instead.
+// **A tempting "fix" investigated and ruled out, not just assumed safe**:
+// having the relay *itself* self-verify visibility (a same-process,
+// fd-holding check, before ever reporting success) was tried first and
+// found to be actively harmful -- confirmed by direct A/B testing on this
+// dev machine, it made the container-side device *permanently* invisible
+// to every external `nsenter` afterward, 100% reproducibly, where the
+// mechanism had otherwise always worked instantly and reliably. That
+// self-check was removed entirely; only this external, unrelated-process
+// retry remains. Harmless when nothing is actually wrong (e.g. on the veth
+// path, where the device is already proven to exist by the time this
+// runs, or on a device where the relay's device is already visible
+// immediately) -- the very first attempt succeeding costs nothing extra.
+bool run_with_retry(const std::vector<std::string>& argv, std::string_view what, std::string_view network_name) {
+    constexpr int interval_ms = 25;
+    constexpr int timeout_ms = 500;
+    for (int elapsed = 0; elapsed <= timeout_ms; elapsed += interval_ms) {
+        if (run_process(argv).exit_code == 0) {
+            if (elapsed > 0) {
+                spdlog::debug("succeeded to {} for network '{}' after ~{}ms of retrying", what, network_name,
+                              elapsed);
+            }
+            return true;
+        }
+        struct timespec ts {
+            0, static_cast<long>(interval_ms) * 1000000L
+        };
+        while (nanosleep(&ts, &ts) != 0 && errno == EINTR) {
+        }
+    }
+    spdlog::error("failed to {} for network '{}' (gave up after ~{}ms of retrying)", what, network_name, timeout_ms);
+    return false;
+}
+
 // Address allocation can't be checked by inspecting live interface state the
 // way session/cgroup liveness can: a container's actual assigned IP lives on
 // its own `eth<N>` inside its own private namespace, invisible from the
@@ -219,8 +263,8 @@ std::optional<JoinedNetwork> join_one_network(pid_t ns_pid, const NetworkEntry&
         spdlog::error("no free IPv4 address available on network '{}'", network.name);
         return fail();
     }
-    if (!run(wrap_in_container(ns_pid, {"ip", "addr", "add", *container_ip, "dev", container_if}),
-             "assign the container's IPv4 address", network.name)) {
+    if (!run_with_retry(wrap_in_container(ns_pid, {"ip", "addr", "add", *container_ip, "dev", container_if}),
+                         "assign the container's IPv4 address", network.name)) {
         return fail();
     }
 
@@ -230,14 +274,14 @@ std::optional<JoinedNetwork> join_one_network(pid_t ns_pid, const NetworkEntry&
             spdlog::error("no free IPv6 address available on network '{}'", network.name);
             return fail();
         }
-        if (!run(wrap_in_container(ns_pid, {"ip", "-6", "addr", "add", *container_ip6, "dev", container_if}),
-                 "assign the container's IPv6 address", network.name)) {
+        if (!run_with_retry(wrap_in_container(ns_pid, {"ip", "-6", "addr", "add", *container_ip6, "dev", container_if}),
+                             "assign the container's IPv6 address", network.name)) {
             return fail();
         }
     }
 
-    if (!run(wrap_in_container(ns_pid, {"ip", "link", "set", container_if, "up"}), "bring the container's interface up",
-             network.name)) {
+    if (!run_with_retry(wrap_in_container(ns_pid, {"ip", "link", "set", container_if, "up"}),
+                         "bring the container's interface up", network.name)) {
         return fail();
     }
 

src/self_test.cpp

@@ -36,6 +36,46 @@
 
 namespace {
 
+// Polls (nanosleep, EINTR-retry -- same shape as this file's own isolation
+// wait below, and network_join.cpp's own run_with_retry()) until `nsenter
+// --net=/proc/<container_pid>/ns/net -- ip link show <container_if>`
+// succeeds, or timeout_ms elapses. Mirrors join_one_network()'s own
+// run_with_retry() (network_join.cpp) -- see that function's doc comment
+// for the full story: a user's real target device reported the
+// container-side tap device intermittently not yet visible immediately
+// after creation. **A tempting but actively harmful fix, ruled out by
+// direct testing on this dev machine, not just reasoned about**: having
+// the relay itself (network_tap_relay.cpp) self-verify visibility (via its
+// own `run_process()` call) before ever reporting success made the
+// container-side device *permanently* invisible to every external
+// `nsenter`, reproducibly, 100% of the time -- confirmed by direct A/B
+// testing (adding that internal check broke a previously 100%-reliable
+// real session; removing it again immediately restored first-try success,
+// no retries ever needed on this machine). Root cause not fully understood
+// (something about forking a subprocess that inherits the tap fd -- opened
+// without `O_CLOEXEC` -- immediately after device creation, while still
+// holding it open, appears to corrupt the device's external visibility on
+// this kernel), but the fix is unambiguous: never add an internal,
+// same-process/fd-holding self-check to the relay; an external,
+// unrelated-process retry (this function, and join_one_network()'s own) is
+// safe and sufficient.
+bool wait_for_container_device_visible(pid_t container_pid, const std::string& container_if, int timeout_ms) {
+    constexpr int interval_ms = 25;
+    for (int elapsed = 0; elapsed <= timeout_ms; elapsed += interval_ms) {
+        if (run_process({"nsenter", fmt::format("--net=/proc/{}/ns/net", container_pid), "--", "ip", "link", "show",
+                          container_if})
+                .exit_code == 0) {
+            return true;
+        }
+        struct timespec ts {
+            0, static_cast<long>(interval_ms) * 1000000L
+        };
+        while (nanosleep(&ts, &ts) != 0 && errno == EINTR) {
+        }
+    }
+    return false;
+}
+
 bool test_persistent_netns() {
     constexpr std::string_view test_netns_name = "selftest";
 
@@ -160,9 +200,7 @@ bool test_tap_relay() {
     }
 
     if (ok) {
-        auto container_check = run_process(
-            {"nsenter", fmt::format("--net=/proc/{}/ns/net", container_pid), "--", "ip", "link", "show", container_if});
-        if (container_check.exit_code != 0) {
+        if (!wait_for_container_device_visible(container_pid, container_if, 500)) {
             spdlog::error("self-test: container-side tap device missing inside the target namespace");
             ok = false;
         }