Skip to content

perf(orchestrator): replace ip netns exec with nsenter to halve mount namespace copies on sandbox startup - #3037

Open
AdaAibaby wants to merge 10 commits into
e2b-dev:mainfrom
AdaAibaby:perf/replace-ip-netns-exec-with-nsenter
Open

AdaAibaby wants to merge 10 commits into
e2b-dev:mainfrom
AdaAibaby:perf/replace-ip-netns-exec-with-nsenter

Conversation

@AdaAibaby

@AdaAibaby AdaAibaby commented Jun 18, 2026 •

Copy link
Copy Markdown
Contributor

#3012
p.configure() latency under 100+ concurrent sandboxes:
image

image

/cc @jakubno @dobrac @ValentaTomas Would appreciate your review on this change, thanks!

… namespace copies

Use CLONE_NEWNS cloneflags + nsenter --net= instead of unshare -m + ip netns exec.
This eliminates the second mount tree copy per sandbox, reducing namespace_sem
kernel lock contention under high concurrency (100+ sandboxes).
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request replaces the use of external unshare and ip netns exec commands with syscall.CLONE_NEWNS and nsenter --net to optimize performance by avoiding redundant mount namespace copies. It also updates the start script templates and adds comprehensive unit tests to verify the new execution path and command formatting. I have no feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@dobrac dobrac left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the contribution!

Can you please verify the mount propagation is still set to private as with unshare -m? I believe the syscall.CLONE_NEWNS does not carry the same isolation.

AdaAibaby and others added 2 commits July 1, 2026 15:31
…EWNS

Cloneflags does not automatically set MS_REC|MS_PRIVATE mount propagation
after creating the new mount namespace. Only Unshareflags triggers Go's
built-in mount('none', '/', MS_REC|MS_PRIVATE) call in the fork/exec path
(see syscall/exec_linux.go:473), which matches the behavior of 'unshare -m'.

This closes the window between clone() and the script's
'mount --make-rprivate /' where mounts could still propagate.
@AdaAibaby
AdaAibaby requested a review from dobrac July 1, 2026 07:58
AdaAibaby and others added 2 commits July 2, 2026 16:56
…ount --make-rprivate dependency

Reviewer correctly noted that syscall.CLONE_NEWNS does not carry the same
automatic MS_PRIVATE propagation that unshare -m applies (util-linux >= 2.27
auto-sets private before exec).

Verified on this host (util-linux 2.39.3):
  - unshare --propagation unchanged -m: 80 shared mount points inherited
  - After mount --make-rprivate /: 0 shared mount points
  - unshare -m (auto-private): 0 shared mount points
Result is identical; the first script command is the equivalent.

Fix the misleading comment on Unshareflags and add an explanatory block
comment above the script templates documenting this invariant.
@AdaAibaby

AdaAibaby commented Jul 3, 2026 •

Copy link
Copy Markdown
Contributor Author

@dobrac — verified on the orchestrator host (kernel 6.8.0-55-generic, util-linux 2.39.3)
using bpftrace to trace the mount tracepoint at the kernel syscall boundary:

sudo bpftrace -e '
tracepoint:syscalls:sys_enter_unshare /args->unshare_flags & 131072/ {
    printf("[%s pid=%d] unshare(CLONE_NEWNS=0x%x)\n", comm, pid, args->unshare_flags);
}
tracepoint:syscalls:sys_enter_mount /(args->flags & 278528) == 278528/ {
    printf("[%s pid=%d] mount(target=%s, MS_REC|MS_PRIVATE=0x%x)\n",
           comm, pid, str(args->dir_name), args->flags);
}
'

Running both paths concurrently:

=== path A: old — unshare -m ===
[unshare pid=3291031] unshare(CLONE_NEWNS=0x20000)
[unshare pid=3291031] mount(target=/, MS_REC|MS_PRIVATE=0x44000)

=== path B: new — CLONE_NEWNS via SysProcAttr + script-first mount --make-rprivate / ===
[unshare pid=3291033] unshare(CLONE_NEWNS=0x20000)
[mount   pid=3291033] mount(target=/, MS_REC|MS_PRIVATE=0x44000)

Both paths produce the identical kernel syscall sequence — unshare(CLONE_NEWNS=0x20000)
followed immediately by mount(MS_REC|MS_PRIVATE=0x44000, target=/). The only difference
is which binary issues the second call: in the old path the unshare binary does it
implicitly before exec; in the new path the mount --make-rprivate / at the top of the
start script does it explicitly. The resulting propagation state is identical.

Also updated 5b55eed to correct the Unshareflags comment (CLONE_NEWNS alone does not
carry MS_PRIVATE; private isolation depends on the explicit mount --make-rprivate / as
the invariant first command of both script templates).

tiago-mitralab added a commit to mitralab-dev/e2b-infra that referenced this pull request Jul 9, 2026
… ns copies (cherry-pick e2b-dev#3037)

Upstream e2b-dev/infra e2b-dev#3037. Conflict in process.go resolved: upstream already dropped unshare -m, so kept our structure and added Unshareflags CLONE_NEWNS + nsenter --net= in script_builder (NamespaceID -> NetNamespacePath).
tiago-mitralab added a commit to mitralab-dev/e2b-infra that referenced this pull request Jul 9, 2026
tiago-mitralab added a commit to mitralab-dev/e2b-infra that referenced this pull request Jul 9, 2026
…dening) (#3)

* fix(orchestrator): prevent nil panic when finding default gateway

In Linux, the default route (0.0.0.0/0) has a nil Dst field in the
netlink.Route struct. The previous code called route.Dst.String() without
a nil check, which would panic on systems where the default route's Dst
is nil.

Fix by checking route.Dst == nil directly, which is the canonical way to
identify the default route.

(cherry picked from commit a2304dc)

* fix(envd): use constant-time comparison for signature validation

Replace string inequality operator (!=) with crypto/subtle.ConstantTimeCompare
for signature validation in auth.go. The previous implementation was vulnerable
to timing attacks, where an attacker could potentially determine the correct
signature byte-by-byte by measuring response times.

This is a standard security best practice for comparing cryptographic values
such as HMAC signatures, tokens, and hashes.

(cherry picked from commit 85cce6c)

* test(envd): add validateSigning tests for signature validation

Add 6 test cases covering validateSigning:
- Accepts correct signature with expiration
- Rejects wrong signature
- Rejects expired signature
- Rejects missing signature parameter
- Accepts valid access token from header
- Rejects invalid access token from header

Also refactor existing tests to use a shared helper.

(cherry picked from commit 69cc233)

* fix(uffd): log error and remove stale TODO on handle failure

The UFFD handle goroutine had a TODO comment stating that the sandbox
should be killed when handle() fails. In fact, this is already
implemented: u.exit.SetError() triggers the sandbox exit watcher in
sandbox.go (line ~1093) which calls sbx.Stop().

- Remove the stale TODO comment
- Add error logging when handle() fails for better observability
- Add unit tests covering handle failure behavior (exit error
  propagation, readyCh closure, handler error state, initial state,
  socket creation)

(cherry picked from commit 3e30b03)

* fix(orchestrator): fix typo in TODO comment (to to → to)

(cherry picked from commit c87310c)

* fix: add t.Parallel() to all test functions (paralleltest)

(cherry picked from commit 7b54ed7)

* fix(orchestrator): fix race condition in Cleanup causing missed cleanup functions

There was a TOCTOU race between Add()/AddPriority() and run():

1. Add() checks hasRun (false) outside the lock
2. Add() blocks waiting for mu.Lock()
3. run() acquires lock, sets hasRun=true, executes all cleanups, unlocks
4. Add() acquires lock, appends f — but run() already finished

Result: f is never executed, potentially leaking resources (network
namespaces, cgroups, file descriptors, etc.).

Fix:
- Move hasRun.Store(true) inside the lock in run()
- Add double-checked locking in Add()/AddPriority(): re-check hasRun
  after acquiring the lock and execute f inline if cleanup already ran

Add race-condition tests that reliably reproduce the bug with -race.

(cherry picked from commit d7b2551)

* refactor: release lock before executing cleanup in double-check path

Avoid holding the mutex while executing cleanup functions in the
double-check branch. This prevents potential deadlocks if a cleanup
function performs blocking operations or re-enters the lock.

(cherry picked from commit 97336fd)

* fix: add t.Parallel() to all test functions (paralleltest)

(cherry picked from commit 619dd88)

* feat(orchestrator): add capacity limit and memory-pressure eviction to template mmap cache

(cherry picked from commit 1f1a5a1)

* fix: use MemAvailable instead of Freeram, evict one entry per tick, add unit tests

- Replace unix.Sysinfo Freeram with /proc/meminfo MemAvailable to get
  true available memory (includes reclaimable page cache)
- Evict only one LRU entry per 1s tick instead of looping until threshold
  is met; mmap.Unmap is not instantaneous so the OS stats lag behind
- Add 6 unit tests covering WithCapacity LRU eviction, MemAvailable
  parser correctness/error handling, and pressure-eviction logic

(cherry picked from commit cab034c)

* fix(orchestrator): enforce sandbox TTL on the node via WaitForExit

WaitForExit was defined but never called, leaving the API-layer evictor as
the sole mechanism to kill expired sandboxes.  If the API service restarts
the evictor goroutine stops and VMs accumulate indefinitely on nodes.

Two changes:

1. setupSandboxLifecycle (sandboxes.go): replace sbx.Wait with
   sbx.WaitForExit so the lifecycle goroutine races against the sandbox TTL.
   On timeout, Stop() is called explicitly before the normal Close/cleanup
   path runs.

2. WaitForExit (sandbox.go): replace the one-shot time.After with a
   time.NewTimer loop that re-checks endAt after each fire.  This means a
   KeepAlive that calls SetEndAt mid-flight correctly resets the node-side
   deadline instead of being silently ignored.

Fixes e2b-dev#3193

(cherry picked from commit 0a823f1)

* test(orchestrator): unit tests for WaitForExit TTL enforcement

5 cases covering the two code changes in the parent commit:

- AlreadyExpired: endAt in the past returns error immediately
- ExitBeforeTTL: clean FC exit before TTL returns nil
- ExitWithError: FC exits with error, error is wrapped and returned
- KeepAliveExtendsTTL: SetEndAt extension resets the timer loop;
  sandbox is NOT killed at the original deadline
- ContextCancelled: ctx cancel returns nil without killing

All pass with -race.

(cherry picked from commit 4826a8e)

* fix: propagate Docker image ENV vars to runtime sandbox processes

When creating a sandbox, merge the template metadata's Context.EnvVars
(which contains Docker image ENV directives) into the sandbox config's
Envd.Vars before sending them to envd via POST /init.

User-provided env vars from the SDK create() call take precedence over
template defaults. Per-command env vars from process.start() still
override both.

This fixes both the standard resume path and the filesystem-only cold
boot (reboot) path, where DefaultUser and DefaultWorkdir were already
restored from template metadata but EnvVars was not.

Closes e2b-dev#2268

(cherry picked from commit 5bfd55e)

* perf(orchestrator): replace ip-netns-exec with nsenter (cherry-pick e2b-dev#3037, process.go conflict resolved)

* fix(envd): correct tag-based process lookup (cherry-pick e2b-dev#3099, version bump deferred)

* fix(orchestrator): accept nil or 0.0.0.0/0 default route + bump envd 0.6.9

e2b-dev#3146 as-is panics at boot on our host (default route has non-nil Dst=0.0.0.0/0). Bump envd for cherry-picked e2b-dev#3099/e2b-dev#3145 envd changes.

---------

Co-authored-by: AdaAibaby <shaolila@buaa.edu.cn>
adababys and others added 2 commits July 16, 2026 21:54
- Replace hardcoded "/var/run/netns" in buildArgs with network.NetNamespacesDir
  (the authoritative constant already exported from the network package)
- Add BenchmarkBuild (sequential, v1+v2 sub-benchmarks) and
  BenchmarkSandboxConcurrentStart (RunParallel) to quantify and guard
  script-generation throughput; comments document the run commands and
  what the benchmarks do and do not measure (kernel-level namespace_sem
  contention requires root + real netns files and is out of scope here)
tiago-mitralab added a commit to mitralab-dev/e2b-infra that referenced this pull request Jul 21, 2026
@ValentaTomas
ValentaTomas force-pushed the main branch 2 times, most recently from 5aad415 to d71980e Compare July 25, 2026 22:53
tiago-mitralab added a commit to mitralab-dev/e2b-infra that referenced this pull request Jul 25, 2026
tiago-mitralab added a commit to mitralab-dev/e2b-infra that referenced this pull request Jul 26, 2026
tiago-mitralab added a commit to mitralab-dev/e2b-infra that referenced this pull request Aug 14, 2026
tiago-mitralab added a commit to mitralab-dev/e2b-infra that referenced this pull request Aug 31, 2026
tiago-mitralab added a commit to mitralab-dev/e2b-infra that referenced this pull request Aug 31, 2026
tiago-mitralab added a commit to mitralab-dev/e2b-infra that referenced this pull request Sep 15, 2026
tiago-mitralab added a commit to mitralab-dev/e2b-infra that referenced this pull request Sep 15, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants