Skip to content

Commit eae2da1

Browse files
ZhiXiao-LinRoy Lin
andauthored
fix(cgroup): enforce --memory-reservation/--memory-swap in-guest (#35 Part B) (#114)
Completes #35: with the dead host-side cgroup path removed (#113), these two flags were left unenforced. Apply them in the in-guest per-container cgroup, the same way #10 did for the CPU caps: - ContainerCgroup::create gains memory_low (memory.low) + memory_swap_max (memory.swap.max) params, written best-effort like the cpu limits. - Run path: spec.rs emits A3S_SEC_MEM_LOW (from --memory-reservation) and A3S_SEC_MEM_SWAP (from --memory-swap); guest main.rs reads them. - CRI path: exec_server passes them through (parsed from the env; absent for CRI, which has no soft-reservation concept, so they no-op there). The hard --memory limit stays VM-sized (no A3S_SEC_MEM_LIMIT on the run path). All cgroup resource limits are now enforced in ONE place (the in-guest cgroup). Test: test_run_path_plumbs_memory_reservation_and_swap_to_guest asserts the two A3S_SEC_MEM_* vars are emitted and A3S_SEC_MEM_LIMIT is not. Co-authored-by: Roy Lin <roylin@a3s.box>
1 parent 936635a commit eae2da1

4 files changed

Lines changed: 81 additions & 3 deletions

File tree

src/guest/init/src/cgroup.rs

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,18 +109,32 @@ impl ContainerCgroup {
109109
/// (max process count, `--pids-limit`). Returns `None` when no limit is
110110
/// requested or cgroup v2 is unavailable, in which case the caller proceeds
111111
/// without enforcement.
112+
#[allow(clippy::too_many_arguments)]
112113
pub fn create(
113114
memory_max: Option<u64>,
115+
memory_low: Option<u64>,
116+
memory_swap_max: Option<i64>,
114117
cpu_quota: Option<i64>,
115118
cpu_period: Option<u64>,
116119
cpu_shares: Option<u64>,
117120
pids_max: Option<u64>,
118121
) -> Option<Self> {
119122
let want_memory = memory_max.is_some_and(|m| m > 0);
123+
let want_memory_low = memory_low.is_some_and(|m| m > 0);
124+
// memory.swap.max accepts a byte value or -1 (unlimited); any explicit
125+
// value means the limit was requested.
126+
let want_memory_swap = memory_swap_max.is_some();
120127
let want_cpu = cpu_quota.is_some_and(|q| q > 0);
121128
let want_weight = cpu_shares.is_some_and(|s| s > 0);
122129
let want_pids = pids_max.is_some_and(|p| p > 0);
123-
if (!want_memory && !want_cpu && !want_weight && !want_pids) || !ensure_cgroup2_ready() {
130+
if (!want_memory
131+
&& !want_memory_low
132+
&& !want_memory_swap
133+
&& !want_cpu
134+
&& !want_weight
135+
&& !want_pids)
136+
|| !ensure_cgroup2_ready()
137+
{
124138
return None;
125139
}
126140
let seq = CGROUP_SEQ.fetch_add(1, Ordering::Relaxed);
@@ -141,6 +155,27 @@ impl ContainerCgroup {
141155
// even if the over-allocating process was a child (best-effort).
142156
let _ = write_cgroup_file(&format!("{path}/memory.oom.group"), "1");
143157
}
158+
if want_memory_low {
159+
// memory.low = best-effort soft reservation (--memory-reservation):
160+
// the kernel reclaims from this cgroup only after unprotected memory
161+
// is exhausted. Non-fatal so other limits still apply.
162+
let low = memory_low.unwrap_or(0);
163+
if let Err(error) = write_cgroup_file(&format!("{path}/memory.low"), &low.to_string()) {
164+
warn!(error = %error, low, "cgroup: failed to set memory.low");
165+
}
166+
}
167+
if want_memory_swap {
168+
// memory.swap.max (--memory-swap): a byte cap, or "max" for -1
169+
// (unlimited swap). Non-fatal.
170+
let value = match memory_swap_max {
171+
Some(v) if v < 0 => "max".to_string(),
172+
Some(v) => v.to_string(),
173+
None => "max".to_string(),
174+
};
175+
if let Err(error) = write_cgroup_file(&format!("{path}/memory.swap.max"), &value) {
176+
warn!(error = %error, value, "cgroup: failed to set memory.swap.max");
177+
}
178+
}
144179
if want_cpu {
145180
// cgroup v2 `cpu.max` = "<quota_us> <period_us>"; CRI defaults the
146181
// period to 100ms when unset.

src/guest/init/src/exec_server.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1243,6 +1243,8 @@ fn execute_command_streaming(
12431243
#[cfg(target_os = "linux")]
12441244
let container_cgroup = crate::cgroup::ContainerCgroup::create(
12451245
parse_sec_mem_limit(spec.env),
1246+
parse_sec_int(spec.env, "A3S_SEC_MEM_LOW=").map(|value| value as u64),
1247+
parse_sec_int(spec.env, "A3S_SEC_MEM_SWAP="),
12461248
parse_sec_int(spec.env, "A3S_SEC_CPU_QUOTA="),
12471249
parse_sec_int(spec.env, "A3S_SEC_CPU_PERIOD=").map(|value| value as u64),
12481250
parse_sec_int(spec.env, "A3S_SEC_CPU_SHARES=").map(|value| value as u64),

src/guest/init/src/main.rs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -608,11 +608,18 @@ fn run_init() -> Result<(), Box<dyn std::error::Error>> {
608608
// Build the per-container cgroup from the runtime's A3S_SEC_* control vars.
609609
// memory_max stays None on the boot path: `--memory` is enforced by sizing
610610
// the microVM RAM, not an in-guest cgroup (so the runtime emits no
611-
// A3S_SEC_MEM_LIMIT here). The CPU/pids caps DO have to be applied in-guest
612-
// and are mirrored from the same env vars the CRI exec path consumes.
611+
// A3S_SEC_MEM_LIMIT here). The CPU/pids caps and the memory soft-reservation
612+
// (--memory-reservation) / swap cap (--memory-swap) DO have to be applied
613+
// in-guest, mirrored from the same A3S_SEC_* env vars.
613614
#[cfg(target_os = "linux")]
614615
let container_cgroup = a3s_box_guest_init::cgroup::ContainerCgroup::create(
615616
None,
617+
std::env::var("A3S_SEC_MEM_LOW")
618+
.ok()
619+
.and_then(|value| value.parse::<u64>().ok()),
620+
std::env::var("A3S_SEC_MEM_SWAP")
621+
.ok()
622+
.and_then(|value| value.parse::<i64>().ok()),
616623
std::env::var("A3S_SEC_CPU_QUOTA")
617624
.ok()
618625
.and_then(|value| value.parse::<i64>().ok()),

src/runtime/src/vm/spec.rs

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,20 @@ impl VmManager {
278278
}
279279
}
280280

281+
// Memory soft-reservation (--memory-reservation → memory.low) and
282+
// swap cap (--memory-swap → memory.swap.max). Like the CPU caps these
283+
// are enforced by the in-guest per-container cgroup (the broken
284+
// host-side path was removed); the hard --memory limit stays
285+
// VM-sized, so no A3S_SEC_MEM_LIMIT is emitted here.
286+
if let Some(reservation) = self.config.resource_limits.memory_reservation {
287+
if reservation > 0 {
288+
env.push(("A3S_SEC_MEM_LOW".to_string(), reservation.to_string()));
289+
}
290+
}
291+
if let Some(swap) = self.config.resource_limits.memory_swap {
292+
env.push(("A3S_SEC_MEM_SWAP".to_string(), swap.to_string()));
293+
}
294+
281295
// Signal guest init to remount rootfs read-only after all setup
282296
if self.config.read_only {
283297
env.push(("BOX_READONLY".to_string(), "1".to_string()));
@@ -871,6 +885,26 @@ mod tests {
871885
assert_eq!(env_value(&spec, "A3S_SEC_PIDS_LIMIT"), Some("100"));
872886
}
873887

888+
#[test]
889+
fn test_run_path_plumbs_memory_reservation_and_swap_to_guest() {
890+
// --memory-reservation (memory.low) and --memory-swap (memory.swap.max)
891+
// must reach guest-init as A3S_SEC_MEM_LOW / A3S_SEC_MEM_SWAP so the
892+
// in-guest cgroup enforces them (the broken host path was removed).
893+
let temp = tempdir().unwrap();
894+
let mut config = BoxConfig::default();
895+
config.resource_limits.memory_reservation = Some(256 * 1024 * 1024);
896+
config.resource_limits.memory_swap = Some(-1);
897+
898+
let mut vm = test_vm_manager(config);
899+
let layout = test_layout(temp.path(), Some(test_oci_config(None, None)), true);
900+
let spec = vm.build_instance_spec(&layout).unwrap();
901+
902+
assert_eq!(env_value(&spec, "A3S_SEC_MEM_LOW"), Some("268435456"));
903+
assert_eq!(env_value(&spec, "A3S_SEC_MEM_SWAP"), Some("-1"));
904+
// The hard --memory limit is VM-sized, not an in-guest memory.max.
905+
assert_eq!(env_value(&spec, "A3S_SEC_MEM_LIMIT"), None);
906+
}
907+
874908
#[test]
875909
fn test_run_path_omits_cpu_limits_when_unset_or_unlimited() {
876910
// No limits set, plus an explicit unlimited quota (-1): nothing should be

0 commit comments

Comments
 (0)