feat(sandbox): bind running Firecracker threads to host CPUs - #238
feat(sandbox): bind running Firecracker threads to host CPUs#238emailcannotbeblank wants to merge 1 commit into
Conversation
Add an admin-only CPU-affinity API and the `aenv cpu-bind` command. Bind selected vCPU threads, or all Firecracker threads with `*`, to host logical CPUs. Validate bounded CPU lists with ranges and strides, ignore offline CPUs, verify each update, and roll back earlier updates on failure.
|
🔍 OpenCodeReview found 4 issue(s) in this PR.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| let resp = handle_status( | ||
| self.post(&format!("/sandboxes/{id}/cpu-affinity")) | ||
| .send_json(&body), | ||
| )?; |
There was a problem hiding this comment.
This uses Client::post, which authenticates with X-API-Key, but the new OpenAPI operation declares only AdminApiKeyAuth (X-Admin-Token). Consequently, aenv cpu-bind created via Client::from_env() will receive 401 even with the normal saved API key. Either expose this operation under the intended regular/team authentication with the necessary ownership checks, or add an explicit admin credential/header path to the client and command.
| let pid = sandbox.runtime_process_id().map_err(|source| { | ||
| OrchestratorError::SandboxOperationFailed { | ||
| sandbox_id, | ||
| operation, | ||
| source, | ||
| } | ||
| })?; |
There was a problem hiding this comment.
The handle lock prevents orchestrator-driven replacement, but it does not establish that this numeric PID still belongs to this Firecracker process. If Firecracker exits naturally and the PID is reused before bind_process_cpu_affinity scans /proc, a wildcard request can enumerate and change every thread of an unrelated host process. The per-thread starttime checks only preserve the identity discovered during that scan; they do not compare it with the original Firecracker process identity. Keep a stable process reference (for example, a pidfd) or capture and validate the Firecracker leader's start time before applying any affinity changes.
| fn runtime_process_id(&self) -> Result<i32> { | ||
| Ok(self.fc_instance.pid()?.as_raw()) | ||
| } |
There was a problem hiding this comment.
This exposes only the numeric PID from Child::id(), which can remain available after Firecracker exits. If the PID is reused before the blocking worker scans /proc, affinity can be applied to an unrelated host process; the per-thread start-time checks only establish identities after that stale process PID has been accepted. Return a stable runtime identity (for example, retain a pidfd plus the process start time captured at spawn) and verify the original process is still alive/owns this PID for the duration of binding, rather than passing a bare i32 across the operation.
| fn runtime_process_id(&self) -> Result<i32> { | ||
| i32::try_from(std::process::id()).context("mock runtime pid does not fit in i32") | ||
| } |
There was a problem hiding this comment.
A successful vcpu="*" request against this mock binds every thread discovered in the test runner process, and the orchestrator path does not restore affinity after success. This violates the mock backend's no-op isolation and can permanently constrain unrelated concurrent tests. The mock should report that runtime CPU affinity is unsupported, or own a disposable helper process whose PID can safely be returned.
1 participant
What
Add runtime CPU-affinity control for running Firecracker sandboxes.
POST /sandboxes/{sandboxID}/cpu-affinityAPI.aenv cpu-bind <sandbox-id> --vcpu <list|*> --core <list>.0-10:2.*, toonline host logical CPUs.
fails.
Why
Hardware architects use AgentENV sandboxes for processor benchmarking and
workload characterization. Runtime CPU affinity supports three main use cases:
to LLC, NUMA, and SMT topology to improve long-running workload performance.
host CPUs, reducing run-to-run noise in PMU counters, IPC, cache-miss rates,
and latency measurements.
CPU, or on SMT siblings of one physical core, to evaluate oversubscription
behavior.
Related issue
Closes #228
Scope and non-goals
Included:
public API.
user documentation.
Non-goals:
abstractions introduced here in a follow-up.
Design and behavior changes
The orchestrator accepts only running sandboxes, locks the backend while the
blocking affinity operation runs, and keeps the operation cancellation-safe so
pause, snapshot, or delete cannot replace the Firecracker process midway.
The sandbox layer scans
/proc/<pid>/task, identifies vCPU threads by theirfc_vcpu Nnames, and checks thread start times around numeric-TID syscalls tonarrow the TID-reuse window. Input length, expansion, value count, and CPU IDs
are bounded before allocation. Requested offline CPUs are ignored; an empty
online intersection is rejected before any affinity change.
Before applying changes, the implementation records every target's original
affinity. It then updates and reads back each thread one at a time. A failure or
kernel mismatch triggers best-effort rollback of all threads already changed.
Compatibility and operations
OpenAPI-generated server types. Existing endpoints are unchanged.
by default and protected by the deployment API key; there is no separate
feature flag.
kernel state and is not persisted.
API, but an already-applied affinity remains until changed or the Firecracker
process exits.
/procandpermission to call
sched_setaffinityon Firecracker threads. No new port orruntime dependency is introduced. Supported logical CPU IDs are 0-1023.
Validation
make fmtmake clippymake test-unitmake -C services test(required whenservices/changes)maketargetCommands and results:
Skipped checks and reasons:
make agentenv-server: generated server changes and their OpenAPI source arecommitted, but final regeneration could not be rerun because Java is not
installed in the validation environment.
than changing a request-processing hot path.
environment.
Risks and reviewer notes
the endpoint is therefore admin-only.
exclusive access. All selected threads receive the same host CPU set.
*selects threads present when the request is handled; it is not a policyfor threads created later.
Firecracker process.
checks before and after affinity reads narrow that window.
Suggested review order:
src/sandbox/cpu_affinity.rsfor parsing, thread selection, verification,and rollback.
src/orchestrator/service.rsfor lifecycle locking and PID handling.src/api/openapi.ymlandsrc/api/impls/admin.rsfor the admin API.services/gateway/internal/server.goand theaenvclient/command forrouting and user-facing behavior.
Checklist