Add --delete-network-full to fully tear down a network's live state

--delete-network only ever removed the config entry, leaving the
bridge/iptables/persistent-namespace state behind. Since
ensure_network_provisioned() treats "bridge exists" as "already fully
provisioned" and skips re-adding anything, a network recreated with
the same name after a plain --delete-network silently never got a
fresh MASQUERADE rule if the old one had been removed separately by
hand -- the bridge itself was still there the whole time.

teardown_network_state() (network_bridge.{h,cpp}) is the reverse of
ensure_network_provisioned(): for extern, removes the MASQUERADE
rule(s) then deletes the bridge; for intern, removes the whole
persistent namespace in one step (destroys the bridge inside it too,
no separate ip link del needed). Deliberately leaves the IPv4/IPv6
forwarding sysctls alone -- those are global host state shared across
every extern network, not per-network. Each step is best-effort
(teardown_step(), logging a warning not an error on failure) since a
step "failing" because that piece was already gone by hand is the
expected case this exists to handle, not a reason to abort --
delete_network_command() doesn't gate the config removal on any of
this succeeding, unlike delete_volume_command()'s own -full variant.

--delete-network-full wired into cli_args.{h,cpp} the same way
--delete-volume-full is.

Verified as root via the doas rule: an extern network's bridge and
MASQUERADE rule were both confirmed gone after --delete-network-full,
and recreating a network with the same name went through
provision_bridge() fresh instead of short-circuiting on a stale
bridge_exists() check -- fixing exactly the gap reported (a manually
removed MASQUERADE rule never came back on delete+recreate). An intern
network's persistent namespace was likewise confirmed fully removed
and recreatable without conflict.

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

CLAUDE.md

@@ -182,10 +182,20 @@ Source layout (all under `src/`):
   it's already a pure deterministic function of the name — then the IPv6
   subnet — or `"(no ipv6)"` — appended unaligned as the trailing column,
   nothing follows it).
-  `delete_network_command()` currently only removes the config entry, the
+  `delete_network_command()` takes a `delete_full` bool, same shape as
-  same as `delete_volume_command()`'s default (non-`-full`) behavior — it
+  `delete_volume_command()`'s own: `--delete-network` (`false`) only removes
-  does not tear down the network's live bridge/namespace/iptables state (no
+  the config entry, leaving the network's live host-side state untouched;
-  `--delete-network-full` analog exists yet).
+  `--delete-network-full` (`true`) additionally calls
+  `teardown_network_state()` (`network_bridge.h`, see below) first. Unlike
+  `delete_volume_command()`'s `-full` variant (which bails out *before*
+  touching the config if its single `remove_all()` call fails),
+  `delete_network_command()` doesn't gate the config removal on
+  `teardown_network_state()`'s success at all — that function is
+  deliberately best-effort/non-fatal per-step (see its own doc comment), so
+  a step "failing" because that piece was already gone by hand (exactly
+  `ensure_network_provisioned()`'s own existence-check caveat, above) is
+  expected, not a reason to leave a network the user explicitly asked to
+  delete sitting in the config.
   `write_config_command()` implements `-w/--write-config`: unlike
   `create_volume_command()`/`delete_volume_command()`'s use of
   `write_config_file()` (which only ever persists `AppConfig` fields that are
@@ -662,6 +672,35 @@ Source layout (all under `src/`):
   all came up correctly; a real `intern` network's bridge came up inside its
   own dedicated namespace with neither forwarding nor a NAT rule, confirming
   the structural (not merely policy) isolation the design calls for.
+  `teardown_network_state()` is the reverse: `--delete-network-full`
+  (`commands.cpp`'s `delete_network_command()`) calls it to tear down
+  exactly what `ensure_network_provisioned()` stood up. For `extern`:
+  removes the `ip6tables` MASQUERADE rule (if `ipv6`) then the `iptables`
+  one, then deletes the bridge itself — via a new `.cpp`-local
+  `teardown_step()`, the same shape as `run_admin_command()` but logging a
+  *warning*, not an error, on failure, since a step failing because that
+  piece was already gone by hand is the expected, common case this exists
+  to handle (exactly `ensure_network_provisioned()`'s own existence-check
+  caveat above — this is precisely how a manually-removed MASQUERADE rule
+  can go from "won't come back on recreate" to "cleanly torn down and
+  recreated" once `--delete-network-full` exists at all). For `intern`:
+  removes the whole persistent namespace (`persistent_netns.h`) in one
+  step, which destroys everything inside it — the bridge included — with
+  no separate `ip link del` needed. Deliberately never touches the
+  IPv4/IPv6 forwarding sysctls `provision_bridge()` enables for `extern` —
+  those are global host state shared across every `extern` network, not
+  per-network, so turning them off here could break others still relying
+  on them. **Verified end-to-end on this dev machine (root, via the scoped
+  `doas` rule)**: an `extern` network's bridge and MASQUERADE rule were
+  both confirmed gone after `--delete-network-full` (`ip link show`
+  reporting "Device does not exist"), and recreating a network with the
+  *same name* afterward correctly went through `provision_bridge()` again
+  from scratch (confirmed via the debug log) instead of short-circuiting
+  on a stale `bridge_exists()` check — fixing exactly the gap a user
+  reported (a manually-removed MASQUERADE rule never came back on
+  `--delete-network` + recreate, since the old bridge was silently still
+  there); an `intern` network's persistent namespace was similarly
+  confirmed fully removed and recreatable without conflict.
   `probe_veth_support()` (added for the tap+relay fallback, see
   `docs/networking-design.md`'s addendum and `network_join.{h,cpp}` below):
   the real target device supports `tun`/`tap` but not `veth`

README.md

@@ -94,7 +94,8 @@ slocker-lite -V|--version
 | `--delete-volume-full <name>` | Like `--delete-volume`, but also recursively deletes the volume's host directory. |
 | `-n, --network <name>` | Create/manage a persistent named network: requires exactly one of `--extern` (a real Linux bridge in the host's own namespace, with NAT/forwarding set up so containers on it reach the host's real network) or `--intern` (a bridge inside its own dedicated, routeless namespace, only reachable by other containers on the same network). `--subnet <cidr>` overrides the auto-allocated IPv4 range (`10.168.0.0/24`, incrementing per network); `--no-ipv6` disables (and `--subnet6 <cidr>` overrides) the auto-allocated IPv6 range, on by default. `--no-veth` forces the tap+relay join fallback even on a kernel that supports veth (useful for testing that path; it's otherwise chosen automatically whenever the running kernel lacks veth support). With `--run`, instead joins `<name>` to the container as its own `eth<N>` interface with an address from the network's subnet; repeatable, no membership limit. Root-only for now. **Known gap**: on a network joined via the tap+relay fallback, peer-to-peer connectivity works, but reaching the network's own gateway (and thus the outside, for `extern`) does not yet — root cause unconfirmed, see [`docs/networking-design.md`](docs/networking-design.md). |
 | `--list-networks` | List all named networks (see `-n/--network`) with their kind, IPv4 subnet, bridge name, and IPv6 subnet (or `(no ipv6)`). |
-| `--delete-network <name>` | Remove a named network from the config. |
+| `--delete-network <name>` | Remove a named network from the config. Its live bridge/namespace/iptables state is left untouched. |
+| `--delete-network-full <name>` | Like `--delete-network`, but also tears down its live state (MASQUERADE rule(s), the bridge itself for `extern`, or the whole persistent namespace for `intern`), so a network with the same name can be created fresh afterward. |
 | `-p, --port-forward [<network>:]<host-port>:<container-port>` | With `--run`, forward a TCP port from the host into the container. `<network>` is optional, defaulting to the container's sole `--extern` network (an error if it joined more than one without specifying). Repeatable. Reachable via the host's real, externally-facing IP; `localhost`/loopback access has a known NAT-hairpinning limitation (see [`docs/networking-design.md`](docs/networking-design.md)). |
 | `--list-processes` | List running `--run` sessions found by their pid files under `$XDG_STATE_HOME/slocker-lite/run/`, with their pid, container name, and status (`running` or `exited`). |
 | `--clean-processes` | Remove stale pid files (see `--list-processes`), and any `-p`/`--port-forward` iptables rules, left behind by sessions that are no longer running (e.g. after a crash). |

src/cli_args.cpp

@@ -63,9 +63,10 @@ constexpr int network_subnet6 = 273;
 constexpr int list_networks = 274;
 constexpr int delete_network = 275;
 constexpr int network_no_veth = 276;
+constexpr int delete_network_full = 277;
 }  // namespace options
 
-constexpr std::array<struct option, 37> long_options = {{
+constexpr std::array<struct option, 38> long_options = {{
     {"help", no_argument, nullptr, 'h'},
     {"version", no_argument, nullptr, 'V'},
     {"test", no_argument, nullptr, 't'},
@@ -101,6 +102,7 @@ constexpr std::array<struct option, 37> long_options = {{
     {"no-veth", no_argument, nullptr, options::network_no_veth},
     {"list-networks", no_argument, nullptr, options::list_networks},
     {"delete-network", required_argument, nullptr, options::delete_network},
+    {"delete-network-full", required_argument, nullptr, options::delete_network_full},
     {"port-forward", required_argument, nullptr, 'p'},
     {nullptr, 0, nullptr, 0},
 }};
@@ -125,6 +127,7 @@ void print_usage(const char* prog) {
         "                 [--no-ipv6] [--subnet6 <cidr>] [--no-veth]\n"
         "       {0} --list-networks\n"
         "       {0} --delete-network <name>\n"
+        "       {0} --delete-network-full <name>\n"
         "       {0} --list-processes\n"
         "       {0} --clean-processes\n"
         "       {0} -w|--write-config\n"
@@ -236,7 +239,15 @@ void print_usage(const char* prog) {
         "                            with their kind, IPv4 subnet, bridge name,\n"
         "                            and IPv6 subnet (or \"(no ipv6)\")\n"
         "      --delete-network <name>\n"
-        "                            remove a named network from the config\n"
+        "                            remove a named network from the config (its\n"
+        "                            live bridge/namespace/iptables state is left\n"
+        "                            untouched)\n"
+        "      --delete-network-full <name>\n"
+        "                            like --delete-network, but also tears down its\n"
+        "                            live state: MASQUERADE rule(s), the bridge\n"
+        "                            itself (extern), or the whole persistent\n"
+        "                            namespace (intern) -- so a network with the\n"
+        "                            same name can be created fresh afterward\n"
         "  -p, --port-forward [<network>:]<host-port>:<container-port>\n"
         "                            with --run, forward a TCP port from the host\n"
         "                            into the container. <network> is optional --\n"
@@ -325,6 +336,7 @@ std::optional<int> parse_args(int argc, char* argv[], ParsedArgs& out) {
             case options::clean_processes:
             case options::list_networks:
             case options::delete_network:
+            case options::delete_network_full:
             case options::kill: {
                 Mode requested;
                 switch (opt) {
@@ -376,6 +388,9 @@ std::optional<int> parse_args(int argc, char* argv[], ParsedArgs& out) {
                     case options::delete_network:
                         requested = Mode::delete_network;
                         break;
+                    case options::delete_network_full:
+                        requested = Mode::delete_network_full;
+                        break;
                     default:
                         requested = Mode::clean_processes;
                         break;

src/cli_args.h

@@ -46,7 +46,8 @@ enum class Mode {
     write_config,
     network,
     list_networks,
-    delete_network
+    delete_network,
+    delete_network_full
 };
 
 // Everything parse_args() extracts from argv, ready to hand to

src/commands.cpp

@@ -510,7 +510,8 @@ int list_networks_command(const AppConfig& config) {
     return 0;
 }
 
-int delete_network_command(const std::string& name, const std::filesystem::path& config_path, AppConfig& config) {
+int delete_network_command(const std::string& name, const std::filesystem::path& config_path, AppConfig& config,
+                            bool delete_full) {
     auto it = std::find_if(config.networks.begin(), config.networks.end(),
                             [&](const NetworkEntry& network) { return network.name == name; });
     if (it == config.networks.end()) {
@@ -518,6 +519,14 @@ int delete_network_command(const std::string& name, const std::filesystem::path&
         return 1;
     }
 
+    // Best-effort, not gating the config removal below on success -- see
+    // teardown_network_state()'s own doc comment (network_bridge.h) for why
+    // a step "failing" (e.g. a rule already removed by hand) is expected,
+    // not a reason to leave the config entry in place.
+    if (delete_full) {
+        teardown_network_state(*it);
+    }
+
     config.networks.erase(it);
     if (!write_config_file(config_path, config)) {
         return 1;
@@ -844,7 +853,9 @@ int dispatch_command(const ParsedArgs& args, const std::filesystem::path& config
         case Mode::list_networks:
             return list_networks_command(config);
         case Mode::delete_network:
-            return delete_network_command(args.mode_arg, config_path, config);
+            return delete_network_command(args.mode_arg, config_path, config, false);
+        case Mode::delete_network_full:
+            return delete_network_command(args.mode_arg, config_path, config, true);
         case Mode::run: {
             // As root, containers-storage mount doesn't need to reexec into a private
             // user namespace to gain privilege, so the mount is already directly

src/network_bridge.cpp

@@ -71,6 +71,19 @@ bool run_admin_command(const NetworkEntry& network, std::vector<std::string> arg
     return true;
 }
 
+// Same shape as run_admin_command() above, but a failure is logged as a
+// warning, not an error, and never treated as fatal by its caller -- unlike
+// provisioning (where a failure is a real problem), a teardown step failing
+// because that piece was already removed by hand, or never existed, is the
+// expected common case (see ensure_network_provisioned()'s own
+// existence-check caveat, network_bridge.h), not something worth alarming
+// about.
+void teardown_step(const NetworkEntry& network, std::vector<std::string> argv, std::string_view what) {
+    if (run_process(wrap_for_network(network, std::move(argv))).exit_code != 0) {
+        spdlog::warn("failed to {} for network '{}' (already gone, or never existed)", what, network.name);
+    }
+}
+
 bool bridge_exists(const NetworkEntry& network, const std::string& bridge) {
     return run_process(wrap_for_network(network, {"ip", "link", "show", bridge})).exit_code == 0;
 }
@@ -203,3 +216,31 @@ bool ensure_network_provisioned(const NetworkEntry& network) {
 
     return provision_bridge(network, bridge);
 }
+
+void teardown_network_state(const NetworkEntry& network) {
+    std::string bridge = bridge_name(network.name);
+
+    if (network.kind == NetworkKind::extern_) {
+        // Remove the MASQUERADE rule(s) before the bridge itself -- purely
+        // for tidiness, iptables doesn't require the referenced interface
+        // to still exist for the rule to be removable.
+        if (network.ipv6) {
+            teardown_step(network,
+                          {"ip6tables", "-t", "nat", "-D", "POSTROUTING", "-s", network.subnet6, "!", "-o", bridge,
+                           "-j", "MASQUERADE"},
+                          "remove IPv6 MASQUERADE rule");
+        }
+        teardown_step(network,
+                      {"iptables", "-t", "nat", "-D", "POSTROUTING", "-s", network.subnet, "!", "-o", bridge, "-j",
+                       "MASQUERADE"},
+                      "remove IPv4 MASQUERADE rule");
+        teardown_step(network, {"ip", "link", "del", bridge}, "delete bridge");
+    } else {
+        // Removing the whole persistent namespace destroys everything
+        // inside it -- the bridge included -- in one step; no separate
+        // `ip link del` needed.
+        if (persistent_netns_exists(network.name) && !remove_persistent_netns(network.name)) {
+            spdlog::warn("failed to remove persistent namespace for network '{}'", network.name);
+        }
+    }
+}

src/network_bridge.h

@@ -87,3 +87,22 @@ bool should_use_veth(const NetworkEntry& network);
 // entry) -- there's no separate "reconcile" path; calling this again just
 // recreates whatever's missing.
 bool ensure_network_provisioned(const NetworkEntry& network);
+
+// Tears down `network`'s live host-side state -- the reverse of
+// ensure_network_provisioned(). For `extern`: removes the MASQUERADE
+// rule(s) (`ip6tables` first, if `ipv6`, then `iptables` -- purely for
+// tidiness, since iptables doesn't require the interface a rule references
+// to still exist), then deletes the bridge itself. For `intern`: removes
+// the whole persistent namespace (persistent_netns.h) in one step, which
+// destroys everything inside it -- the bridge included -- with no separate
+// `ip link del` needed. Deliberately does **not** touch the IPv4/IPv6
+// forwarding sysctls `provision_bridge()` enables for `extern` -- those are
+// global host state shared across every `extern` network, not per-network,
+// so disabling them here could break others still relying on them.
+// Best-effort, like every other host-global teardown in this project
+// (`remove_session_cgroup()`, `remove_port_forward()`): logs a warning and
+// keeps going past any individual step that fails, rather than stopping --
+// a step failing because that piece was already removed by hand (see
+// ensure_network_provisioned()'s own existence-check caveat above) is the
+// common case this exists to handle gracefully, not an error to abort on.
+void teardown_network_state(const NetworkEntry& network);