[PR #313] [CLOSED] fix(setup): suppress ifupdown hotplug ifquery storm for aenv-* NICs #339

New issue

Closed

opened 2026-10-29 14:22:23 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-29 14:22:23 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/kvcache-ai/AgentENV/pull/313
Author: @helloC-A
Created: 9/22/2026
Status: ❌ Closed

Base: main ← Head: fix/suppress-ifquery-hotplug-storm

📝 Commits (1)

f492af8 fix(setup): suppress ifupdown hotplug ifquery storm for aenv-* NICs

📊 Changes

6 files changed (+152 additions, -16 deletions)

View changed files

📝 docs/src/internals/networking.md (+8 -8)
📝 src/sandbox/network/manager.rs (+6 -6)
📝 src/sandbox/network/mod.rs (+1 -1)
📝 src/sandbox/network/slot.rs (+1 -1)
➕ src/setup/hotplug.rs (+129 -0)
📝 src/setup/mod.rs (+7 -0)

📄 Description

What

Suppress the ifquery process storm triggered by AENV NIC events on Debian hosts.

Rename the host-side veth prefix from veth- to aenv- so AENV NICs are uniquely identifiable
Add aenv-* to the early-exit whitelists of /lib/udev/ifupdown-hotplug (both add and remove paths), applied via --setup-host
Guard the patched script with dpkg-divert (no --rename) so ifupdown package upgrades cannot overwrite it
Update docs/src/internals/networking.md for the renamed host-side interface

Why

On Debian-based hosts, /lib/udev/rules.d/85-ifupdown.rules invokes /lib/udev/ifupdown-hotplug for every net device add/remove event. Its net_ifup() unconditionally runs ifquery --allow hotplug to decide whether the interface is hotplug-managed. AENV NICs are never listed in /etc/network/interfaces, so for them the answer is always "not managed" — yet each check pays a full process spawn (udevd worker + hotplug shell + ifquery, ~3 forks per event).

Under large-scale sandbox churn this produces hundreds of concurrent ifquery processes (observed 110→342, monotonically growing, indicating arrival rate > service rate). The storm starves host CPUs and inflates sandbox pause/resume latency, which are short, host-CPU-mediated operations.

Load test results (n=22434 pause/resume cycles, same workload before/after):

Metric	Before	After	Change
pause_ms (mean / min)	1595.2 / 421.3	940.3 / 56.9	-41% / -86%
resume_ms (mean / p95)	1493.4 / 2469.4	662.1 / 1114.7	-56% / -55%
pause+resume per cycle (ms)	3088.7	1602.4	-48%
exec_ms / think_ms	—	—	unchanged

exec/think being unchanged confirms the gain is host-noise removal rather than workload drift. After the fix, concurrent ifquery counts stay in single digits during sandbox churn.

Closes #312

Scope and non-goals

Included:

Host-side NIC prefix rename (veth- → aenv-) and the unit tests that reference it
ifupdown-hotplug patching module wired into ensure_host()
Internals networking docs sync

Non-goals:

No changes to udev rules, NetworkManager, egress policy, or any services/ code
No attempt to eliminate the residual udev worker + hotplug shell spawn per event (the script now exits before forking ifquery; a single early-exit shell per event is acceptable and avoids the fragile alternatives evaluated below)
Docker/K8s veth-* NICs are explicitly unaffected

Rejected alternatives (for context): OPTIONS+="ignore_device" (removed in udev 148, silently ignored), GOTO skip rules (requires overriding system rule files, upgrade conflicts), disabling ifupdown hotplug globally (breaks other services), whitelisting veth-* (collides with Docker/K8s).

Design and behavior changes

patch_ifupdown_hotplug() in src/setup/hotplug.rs runs at the end of ensure_host():

Locates the script (/lib/udev/ifupdown-hotplug or /usr/lib/udev/ifupdown-hotplug); skips silently if absent (non-Debian hosts)
Idempotency: returns early if the content already contains aenv-*
Reads the original content first, writes a one-time backup at <script>.aenv-backup
Registers dpkg-divert --add (deliberately without --rename: --rename moves the file immediately and breaks the subsequent write; without it the original path stays under AENV control and package upgrades write to .aenv-orig)
Text replacement adds aenv-* to both existing case whitelists; bails loudly if the pattern is not matched (guards against ifupdown versions with a different script layout)
Writes the patched script and reloads udev rules (udevadm control --reload-rules, no --trigger to avoid replaying events for all NICs)

The whitelist is the mechanism's intended use: existing entries (ppp*, lo, ipsec*, …) already mark NICs managed by other software. AENV configures its NICs itself (netlink for host-side address/route setup, ip= kernel boot arg for the guest), so ifupdown behavior for aenv-* is unchanged — it does nothing either way; only the wasteful fork is skipped.

Compatibility and operations

Public API or generated protocol: N/A — no API or schema changes.
Configuration or defaults: N/A — the [network.internal] config keys (incl. veth_cidr) are unchanged; only the host-side interface name prefix changes from veth-{slot} to aenv-{slot}.
Snapshot manifest, artifact layout, or storage format: N/A.
Upgrade and rollback: Patch is idempotent (safe to re-run --setup-host); original script preserved at <script>.aenv-backup for manual rollback; the diversion is listed via dpkg-divert --list. Note for existing deployments: NICs created by older builds keep the veth- name until their slot is recycled.
Host requirements, permissions, ports, or dependencies: --setup-host already requires root; on Debian, dpkg-divert is used if present (falls back to direct write otherwise); non-Debian hosts are skipped with a log line.

Validation

make fmt
make clippy
make test-unit
Relevant Rust integration tests
make -C services test (required when services/ changes)
Generated clients/server regenerated with the documented make target (N/A — no schema/generated-code changes)
Documentation updated (docs/src/internals/networking.md)
Benchmarks or performance comparison completed (see Why section)

Commands and results:

# On the production Debian host after --setup-host:
grep -n 'aenv' /lib/udev/ifupdown-hotplug
ppp*|ippp*|isdn*|plip*|lo|irda*|ipsec*|aenv-*)
        ppp*|aenv-*)

dpkg-divert --list | grep ifupdown-hotplug
diversion of /lib/udev/ifupdown-hotplug to /lib/udev/ifupdown-hotplug.aenv-orig by aenv

# During sandbox churn: watch -n 1 'ps -eo comm | grep -c "^ifquery$"'
# Before: 110 → 342 (growing)   After: single digits (stable)

Skipped checks and reasons: make fmt / make clippy / make test-unit were not run locally (development machine is Windows without a Rust toolchain; the project builds only on Linux). Behavioral verification was performed on the Linux test host as documented above.

Risks and reviewer notes

Modifies a distro-owned script (/lib/udev/ifupdown-hotplug); mitigated by dpkg-divert registration, a one-time backup, and idempotent re-runs.
Pattern-match failure on future ifupdown versions fails loudly (bail!) rather than silently mispatching; the content check guards double-patching.
The prefix rename is user-visible (ip link shows aenv-*); docs updated accordingly.
Residual cost: one early-exiting hotplug shell per NIC event remains (evaluated as negligible in benchmarks).
Key files for review: src/setup/hotplug.rs (new), src/setup/mod.rs (wiring), src/sandbox/network/mod.rs (prefix constant), src/sandbox/network/manager.rs (test updates).

Checklist

The PR contains one coherent change and no unrelated formatting or refactoring.
New behavior is covered by tests, or I explained why testing is impractical.
Logs and examples contain no credentials, tokens, or private registry information.
I did not manually edit generated code without updating its source and regenerating it.

Testing notes: the prefix rename is covered by the existing unit tests in
src/sandbox/network/manager.rs (slot_index_from_host_veth_name_*), updated
for aenv-*. The hotplug patch operates on the distro-owned
/lib/udev/ifupdown-hotplug and requires root plus the host's actual
udev/ifupdown installation, so unit testing is impractical; it is verified
behaviorally after --setup-host by grepping the patched script and observing
ifquery counts during sandbox churn (see Validation).

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/kvcache-ai/AgentENV/pull/313 **Author:** [@helloC-A](https://github.com/helloC-A) **Created:** 9/22/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/suppress-ifquery-hotplug-storm` --- ### 📝 Commits (1) - [`f492af8`](https://github.com/kvcache-ai/AgentENV/commit/f492af8b3bcdd50adffc67f5ca639fe2ff6274d3) fix(setup): suppress ifupdown hotplug ifquery storm for aenv-* NICs ### 📊 Changes **6 files changed** (+152 additions, -16 deletions) <details> <summary>View changed files</summary> 📝 `docs/src/internals/networking.md` (+8 -8) 📝 `src/sandbox/network/manager.rs` (+6 -6) 📝 `src/sandbox/network/mod.rs` (+1 -1) 📝 `src/sandbox/network/slot.rs` (+1 -1) ➕ `src/setup/hotplug.rs` (+129 -0) 📝 `src/setup/mod.rs` (+7 -0) </details> ### 📄 Description ## What Suppress the `ifquery` process storm triggered by AENV NIC events on Debian hosts. - Rename the host-side veth prefix from `veth-` to `aenv-` so AENV NICs are uniquely identifiable - Add `aenv-*` to the early-exit whitelists of `/lib/udev/ifupdown-hotplug` (both add and remove paths), applied via `--setup-host` - Guard the patched script with `dpkg-divert` (no `--rename`) so ifupdown package upgrades cannot overwrite it - Update `docs/src/internals/networking.md` for the renamed host-side interface ## Why On Debian-based hosts, `/lib/udev/rules.d/85-ifupdown.rules` invokes `/lib/udev/ifupdown-hotplug` for every net device add/remove event. Its `net_ifup()` unconditionally runs `ifquery --allow hotplug` to decide whether the interface is hotplug-managed. AENV NICs are never listed in `/etc/network/interfaces`, so for them the answer is always "not managed" — yet each check pays a full process spawn (udevd worker + hotplug shell + ifquery, ~3 forks per event). Under large-scale sandbox churn this produces hundreds of concurrent ifquery processes (observed 110→342, monotonically growing, indicating arrival rate > service rate). The storm starves host CPUs and inflates sandbox pause/resume latency, which are short, host-CPU-mediated operations. Load test results (n=22434 pause/resume cycles, same workload before/after): | Metric | Before | After | Change | | --- | --- | --- | --- | | pause_ms (mean / min) | 1595.2 / 421.3 | 940.3 / 56.9 | -41% / -86% | | resume_ms (mean / p95) | 1493.4 / 2469.4 | 662.1 / 1114.7 | -56% / -55% | | pause+resume per cycle (ms) | 3088.7 | 1602.4 | -48% | | exec_ms / think_ms | — | — | unchanged | exec/think being unchanged confirms the gain is host-noise removal rather than workload drift. After the fix, concurrent ifquery counts stay in single digits during sandbox churn. ## Related issue Closes #312  ## Scope and non-goals Included: - Host-side NIC prefix rename (`veth-` → `aenv-`) and the unit tests that reference it - `ifupdown-hotplug` patching module wired into `ensure_host()` - Internals networking docs sync Non-goals: - No changes to udev rules, NetworkManager, egress policy, or any `services/` code - No attempt to eliminate the residual udev worker + hotplug shell spawn per event (the script now exits before forking ifquery; a single early-exit shell per event is acceptable and avoids the fragile alternatives evaluated below) - Docker/K8s `veth-*` NICs are explicitly unaffected Rejected alternatives (for context): `OPTIONS+="ignore_device"` (removed in udev 148, silently ignored), `GOTO` skip rules (requires overriding system rule files, upgrade conflicts), disabling ifupdown hotplug globally (breaks other services), whitelisting `veth-*` (collides with Docker/K8s). ## Design and behavior changes `patch_ifupdown_hotplug()` in `src/setup/hotplug.rs` runs at the end of `ensure_host()`: 1. Locates the script (`/lib/udev/ifupdown-hotplug` or `/usr/lib/udev/ifupdown-hotplug`); skips silently if absent (non-Debian hosts) 2. Idempotency: returns early if the content already contains `aenv-*` 3. Reads the original content first, writes a one-time backup at `<script>.aenv-backup` 4. Registers `dpkg-divert --add` (deliberately without `--rename`: `--rename` moves the file immediately and breaks the subsequent write; without it the original path stays under AENV control and package upgrades write to `.aenv-orig`) 5. Text replacement adds `aenv-*` to both existing `case` whitelists; bails loudly if the pattern is not matched (guards against ifupdown versions with a different script layout) 6. Writes the patched script and reloads udev rules (`udevadm control --reload-rules`, no `--trigger` to avoid replaying events for all NICs) The whitelist is the mechanism's intended use: existing entries (`ppp*`, `lo`, `ipsec*`, …) already mark NICs managed by other software. AENV configures its NICs itself (netlink for host-side address/route setup, `ip=` kernel boot arg for the guest), so ifupdown behavior for `aenv-*` is unchanged — it does nothing either way; only the wasteful fork is skipped. ## Compatibility and operations - Public API or generated protocol: N/A — no API or schema changes. - Configuration or defaults: N/A — the `[network.internal]` config keys (incl. `veth_cidr`) are unchanged; only the host-side interface name prefix changes from `veth-{slot}` to `aenv-{slot}`. - Snapshot manifest, artifact layout, or storage format: N/A. - Upgrade and rollback: Patch is idempotent (safe to re-run `--setup-host`); original script preserved at `<script>.aenv-backup` for manual rollback; the diversion is listed via `dpkg-divert --list`. Note for existing deployments: NICs created by older builds keep the `veth-` name until their slot is recycled. - Host requirements, permissions, ports, or dependencies: `--setup-host` already requires root; on Debian, `dpkg-divert` is used if present (falls back to direct write otherwise); non-Debian hosts are skipped with a log line. ## Validation - [ ] `make fmt` - [ ] `make clippy` - [ ] `make test-unit` - [ ] Relevant Rust integration tests - [ ] `make -C services test` (required when `services/` changes) - [x] Generated clients/server regenerated with the documented `make` target (N/A — no schema/generated-code changes) - [x] Documentation updated (`docs/src/internals/networking.md`) - [x] Benchmarks or performance comparison completed (see Why section) Commands and results: ```text # On the production Debian host after --setup-host: grep -n 'aenv' /lib/udev/ifupdown-hotplug ppp*|ippp*|isdn*|plip*|lo|irda*|ipsec*|aenv-*) ppp*|aenv-*) dpkg-divert --list | grep ifupdown-hotplug diversion of /lib/udev/ifupdown-hotplug to /lib/udev/ifupdown-hotplug.aenv-orig by aenv # During sandbox churn: watch -n 1 'ps -eo comm | grep -c "^ifquery$"' # Before: 110 → 342 (growing) After: single digits (stable) ``` Skipped checks and reasons: `make fmt` / `make clippy` / `make test-unit` were not run locally (development machine is Windows without a Rust toolchain; the project builds only on Linux). Behavioral verification was performed on the Linux test host as documented above.  ## Risks and reviewer notes - Modifies a distro-owned script (`/lib/udev/ifupdown-hotplug`); mitigated by dpkg-divert registration, a one-time backup, and idempotent re-runs. - Pattern-match failure on future ifupdown versions fails loudly (`bail!`) rather than silently mispatching; the content check guards double-patching. - The prefix rename is user-visible (`ip link` shows `aenv-*`); docs updated accordingly. - Residual cost: one early-exiting hotplug shell per NIC event remains (evaluated as negligible in benchmarks). - Key files for review: `src/setup/hotplug.rs` (new), `src/setup/mod.rs` (wiring), `src/sandbox/network/mod.rs` (prefix constant), `src/sandbox/network/manager.rs` (test updates). ## Checklist - [x] The PR contains one coherent change and no unrelated formatting or refactoring. - [x] New behavior is covered by tests, or I explained why testing is impractical. - [x] Logs and examples contain no credentials, tokens, or private registry information. - [x] I did not manually edit generated code without updating its source and regenerating it. Testing notes: the prefix rename is covered by the existing unit tests in `src/sandbox/network/manager.rs` (`slot_index_from_host_veth_name_*`), updated for `aenv-*`. The hotplug patch operates on the distro-owned `/lib/udev/ifupdown-hotplug` and requires root plus the host's actual udev/ifupdown installation, so unit testing is impractical; it is verified behaviorally after `--setup-host` by grepping the patched script and observing ifquery counts during sandbox churn (see Validation). --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-29 14:22:23 +00:00