diff --git a/config/default.toml b/config/default.toml index 3dab300b..ecc97b5e 100644 --- a/config/default.toml +++ b/config/default.toml @@ -110,6 +110,13 @@ veth_cidr = "10.12.0.0/16" mem_size_mib = 1024 vcpu_count = 2 +[machine.disk_rate_limit] +# enabled = true +# bandwidth_bytes_per_sec = 104857600 # 100 MB/s sustained +# bandwidth_burst_bytes = 10485760 # 10 MB one-time burst at VM start (not replenished) +# iops = 3000 +# iops_burst = 500 # one-time IOPS burst at VM start (not replenished) + [envd] version = "0.5.15" init_timeout_secs = 60 diff --git a/src/cfg.rs b/src/cfg.rs index baf32196..fcc8cc2b 100644 --- a/src/cfg.rs +++ b/src/cfg.rs @@ -252,6 +252,32 @@ pub struct MachineConfig { pub vcpu_count: u32, #[config(default = 1024u32)] pub mem_size_mib: u32, + #[config(nested)] + pub disk_rate_limit: DiskRateLimitConfig, +} + +#[derive(Debug, Config, Clone, Default, serde::Serialize, serde::Deserialize)] +pub struct DiskRateLimitConfig { + /// Enable per-sandbox disk I/O rate limiting via Firecracker's virtio-blk rate limiter. + #[config(default = false)] + pub enabled: bool, + /// Sustained disk bandwidth limit in bytes per second (0 = unlimited). + #[config(default = 0u64)] + pub bandwidth_bytes_per_sec: u64, + /// One-time bandwidth burst in bytes, granted once when the VM starts (maps + /// to Firecracker's `one_time_burst`). It is a separate allowance consumed + /// before the sustained bucket and is not replenished after use, so it only + /// absorbs the initial I/O spike; it does not raise the steady-state rate. + #[config(default = 0u64)] + pub bandwidth_burst_bytes: u64, + /// Sustained IOPS limit (0 = unlimited). + #[config(default = 0u64)] + pub iops: u64, + /// One-time IOPS burst, granted once when the VM starts (maps to + /// Firecracker's `one_time_burst`). Consumed before the sustained bucket and + /// not replenished, so it only absorbs the initial spike, not steady state. + #[config(default = 0u64)] + pub iops_burst: u64, } #[derive(Debug, Config, Clone)] @@ -778,6 +804,52 @@ impl AppConfig { } self.validate_memory_snapshot_background_download()?; self.validate_overlaybd_global_config_paths()?; + self.validate_disk_rate_limit()?; + Ok(()) + } + + /// Reject internally inconsistent or out-of-range disk rate limit configs so + /// operator mistakes fail at load time. Disabled sections are skipped: both + /// the fresh-boot and snapshot-resume paths ignore all configured values when + /// disabled, so dormant/pre-staged values must not block startup. + /// + /// When enabled: a one-time burst is meaningless without a nonzero sustained + /// limit (`build_disk_rate_limiter` only creates a bucket when the sustained + /// value is > 0, so a burst paired with a zero sustained limit is silently + /// ignored), and every value must fit Firecracker's signed `i64` token-bucket + /// fields (the consumer converts with `i64::try_from`, so an out-of-range + /// value would otherwise only fail later at sandbox start). + fn validate_disk_rate_limit(&self) -> Result<()> { + let cfg = &self.machine.disk_rate_limit; + if !cfg.enabled { + return Ok(()); + } + if cfg.bandwidth_burst_bytes > 0 && cfg.bandwidth_bytes_per_sec == 0 { + bail!( + "machine.disk_rate_limit: bandwidth_burst_bytes is set but \ + bandwidth_bytes_per_sec is 0; a burst requires a nonzero sustained limit" + ); + } + if cfg.iops_burst > 0 && cfg.iops == 0 { + bail!( + "machine.disk_rate_limit: iops_burst is set but iops is 0; \ + a burst requires a nonzero sustained limit" + ); + } + for (name, value) in [ + ("bandwidth_bytes_per_sec", cfg.bandwidth_bytes_per_sec), + ("bandwidth_burst_bytes", cfg.bandwidth_burst_bytes), + ("iops", cfg.iops), + ("iops_burst", cfg.iops_burst), + ] { + if value > i64::MAX as u64 { + bail!( + "machine.disk_rate_limit.{name} ({value}) exceeds the maximum \ + supported value {}", + i64::MAX + ); + } + } Ok(()) } @@ -1110,6 +1182,69 @@ mod tests { ); } + #[test] + fn validate_rejects_disk_burst_without_sustained() { + let mut config = AppConfig::default(); + config.machine.disk_rate_limit.enabled = true; + config.machine.disk_rate_limit.bandwidth_bytes_per_sec = 0; + config.machine.disk_rate_limit.bandwidth_burst_bytes = 1024; + let err = config.validate().unwrap_err(); + assert!( + err.to_string().contains("bandwidth_burst_bytes is set but"), + "unexpected error: {err}" + ); + + let mut config = AppConfig::default(); + config.machine.disk_rate_limit.enabled = true; + config.machine.disk_rate_limit.iops = 0; + config.machine.disk_rate_limit.iops_burst = 500; + let err = config.validate().unwrap_err(); + assert!( + err.to_string().contains("iops_burst is set but"), + "unexpected error: {err}" + ); + } + + #[test] + fn validate_skips_disabled_disk_rate_limit() { + // A disabled section is ignored at runtime, so even internally + // inconsistent or out-of-range values must not block startup. + let mut config = AppConfig::default(); + config.machine.disk_rate_limit.enabled = false; + config.machine.disk_rate_limit.bandwidth_bytes_per_sec = 0; + config.machine.disk_rate_limit.bandwidth_burst_bytes = 1024; + config.machine.disk_rate_limit.iops = u64::MAX; + config + .validate() + .expect("disabled disk rate limit config is not validated"); + } + + #[test] + fn validate_rejects_disk_rate_limit_above_i64_max() { + let mut config = AppConfig::default(); + config.machine.disk_rate_limit.enabled = true; + config.machine.disk_rate_limit.bandwidth_bytes_per_sec = i64::MAX as u64 + 1; + let err = config.validate().unwrap_err(); + assert!( + err.to_string() + .contains("machine.disk_rate_limit.bandwidth_bytes_per_sec"), + "unexpected error: {err}" + ); + } + + #[test] + fn validate_accepts_consistent_disk_rate_limit() { + let mut config = AppConfig::default(); + config.machine.disk_rate_limit.enabled = true; + config.machine.disk_rate_limit.bandwidth_bytes_per_sec = 104_857_600; + config.machine.disk_rate_limit.bandwidth_burst_bytes = 10_485_760; + config.machine.disk_rate_limit.iops = 3000; + config.machine.disk_rate_limit.iops_burst = 500; + config + .validate() + .expect("consistent disk rate limit config passes"); + } + #[test] fn validate_rejects_shared_overlaybd_global_config_path() { let mut config = AppConfig::default(); diff --git a/src/sandbox/firecracker/config.rs b/src/sandbox/firecracker/config.rs index a18ac70e..0236446b 100644 --- a/src/sandbox/firecracker/config.rs +++ b/src/sandbox/firecracker/config.rs @@ -150,6 +150,12 @@ pub struct FirecrackerCommonConfig { /// start-fresh / start-resume hooks. Persisted with snapshot configs. #[serde(default, skip_serializing_if = "Option::is_none")] pub custom_extension_params: Option, + /// Effective disk I/O rate limit for the user rootfs drive. Carried in the + /// common config so fresh boot and snapshot resume apply the same config + /// and it persists across pause/resume, instead of each path reaching for + /// the process-global config. + #[serde(default)] + pub disk_rate_limit: crate::cfg::DiskRateLimitConfig, } impl FirecrackerCommonConfig { @@ -181,6 +187,7 @@ impl FirecrackerCommonConfig { cpu_config_json: None, network_policy: None, custom_extension_params: None, + disk_rate_limit: crate::cfg::DiskRateLimitConfig::default(), } } @@ -191,6 +198,7 @@ impl FirecrackerCommonConfig { let mut common = Self::new(firecracker_binary, tools_drive_version, runtime_policy); common.envd_version = config.envd.version.clone(); + common.disk_rate_limit = config.machine.disk_rate_limit.clone(); common.rootfs_allow_shrink = config.ublk.overlaybd.allow_shrink; common.control_plane_port = config.tools.control_plane_port; common.firecracker_work_base_dir = config.firecracker.work_dir.clone(); @@ -346,6 +354,7 @@ impl FirecrackerSandboxConfig { read_only: false, runtime_upper_mode: UpperMode::LogStructured, }); + common.disk_rate_limit = app_config.machine.disk_rate_limit; Self { common, kernel_image, diff --git a/src/sandbox/firecracker/instance.rs b/src/sandbox/firecracker/instance.rs index 88d8cec7..227a5782 100644 --- a/src/sandbox/firecracker/instance.rs +++ b/src/sandbox/firecracker/instance.rs @@ -8,7 +8,7 @@ use anyhow::{bail, Context, Result}; use firecracker_client::models::drive::IoEngine; use firecracker_client::models::instance_action_info::ActionType; use firecracker_client::models::vm::State as VmState; -use firecracker_client::models::{mmds_config::Version as MmdsVersion, MmdsConfig}; +use firecracker_client::models::{mmds_config::Version as MmdsVersion, MmdsConfig, PartialDrive}; use firecracker_client::models::{ Balloon, BootSource, DirtyMemoryRanges, Drive, InstanceActionInfo, Logger, MachineConfiguration, MemoryBackend, NetworkInterface, NetworkOverride, RateLimiter, @@ -311,7 +311,10 @@ impl FirecrackerInstance { /// /// `is_root_device` must be `true` for the boot rootfs drive and `false` /// for any non-root extra drive. This API is only used during the - /// pre-boot configuration phase. + /// pre-boot configuration phase. `rate_limiter` attaches a pre-boot limiter + /// so throttling is in force from the guest's first I/O (a post-start PATCH + /// would leave a brief unthrottled window). + #[allow(clippy::too_many_arguments)] pub async fn add_drive( &self, drive_id: &str, @@ -320,12 +323,14 @@ impl FirecrackerInstance { read_only: bool, direct: bool, io_engine: IoEngine, + rate_limiter: Option>, ) -> Result<()> { let mut drive = Drive::new(drive_id.to_string(), is_root_device); drive.path_on_host = Some(path_on_host.to_string_lossy().into_owned()); drive.is_read_only = Some(read_only); drive.direct = Some(direct); drive.io_engine = Some(io_engine); + drive.rate_limiter = rate_limiter; let path = format!("/drives/{}", drive_id); self.client .request_no_content(Method::PUT, &path, Some(&drive)) @@ -333,6 +338,21 @@ impl FirecrackerInstance { .with_context(|| format!("Failed to add/update drive {}", drive_id)) } + /// Updates the rate limiter on a running VM's drive via PATCH. + pub async fn patch_drive_rate_limiter( + &self, + drive_id: &str, + rate_limiter: Box, + ) -> Result<()> { + let mut partial = PartialDrive::new(drive_id.to_string()); + partial.rate_limiter = Some(rate_limiter); + let path = format!("/drives/{}", drive_id); + self.client + .request_no_content(Method::PATCH, &path, Some(&partial)) + .await + .with_context(|| format!("Failed to patch rate limiter on drive {}", drive_id)) + } + /// Adds a network interface. /// Pre-boot only. pub async fn add_network_interface( diff --git a/src/sandbox/firecracker/sandbox.rs b/src/sandbox/firecracker/sandbox.rs index 5adcaef0..88858271 100644 --- a/src/sandbox/firecracker/sandbox.rs +++ b/src/sandbox/firecracker/sandbox.rs @@ -56,6 +56,98 @@ const VM_STATE_FILE_NAME: &str = "vm_state.bin"; const ROOTFS_DRIVE_PATH: &str = "rootfs.ext4"; const USER_ROOTFS_DRIVE_PATH: &str = "user-rootfs"; +/// Firecracker's `TokenBucket::size` is the number of tokens replenished every +/// `refill_time`, not a per-second rate. Pinning the refill period to 1000 ms +/// makes the configured `*_per_sec` values equal the sustained per-second rate. +const RATE_LIMIT_REFILL_TIME_MS: i64 = 1000; + +fn bandwidth_bucket( + cfg: &crate::cfg::DiskRateLimitConfig, +) -> Result>> { + if cfg.bandwidth_bytes_per_sec == 0 { + return Ok(None); + } + let size = i64::try_from(cfg.bandwidth_bytes_per_sec) + .context("disk bandwidth_bytes_per_sec exceeds Firecracker's i64 range")?; + let mut bw = firecracker_client::models::TokenBucket::new(RATE_LIMIT_REFILL_TIME_MS, size); + if cfg.bandwidth_burst_bytes > 0 { + bw.one_time_burst = Some( + i64::try_from(cfg.bandwidth_burst_bytes) + .context("disk bandwidth_burst_bytes exceeds Firecracker's i64 range")?, + ); + } + Ok(Some(Box::new(bw))) +} + +fn ops_bucket( + cfg: &crate::cfg::DiskRateLimitConfig, +) -> Result>> { + if cfg.iops == 0 { + return Ok(None); + } + let size = i64::try_from(cfg.iops).context("disk iops exceeds Firecracker's i64 range")?; + let mut ops = firecracker_client::models::TokenBucket::new(RATE_LIMIT_REFILL_TIME_MS, size); + if cfg.iops_burst > 0 { + ops.one_time_burst = Some( + i64::try_from(cfg.iops_burst) + .context("disk iops_burst exceeds Firecracker's i64 range")?, + ); + } + Ok(Some(Box::new(ops))) +} + +/// Build the limiter attached to the user rootfs drive at fresh boot (pre-boot +/// `PUT /drives`). Returns `None` when limiting is disabled or no dimension is +/// configured, in which case the drive is added with no limiter. +fn build_disk_rate_limiter( + cfg: &crate::cfg::DiskRateLimitConfig, +) -> Result>> { + if !cfg.enabled { + return Ok(None); + } + let bandwidth = bandwidth_bucket(cfg)?; + let ops = ops_bucket(cfg)?; + if bandwidth.is_none() && ops.is_none() { + return Ok(None); + } + let mut rl = firecracker_client::models::RateLimiter::new(); + rl.bandwidth = bandwidth; + rl.ops = ops; + Ok(Some(Box::new(rl))) +} + +/// A token bucket Firecracker interprets as "disable this dimension". +/// +/// Firecracker's `PATCH /drives` maps an *absent* token bucket to +/// `BucketUpdate::None` (leave unchanged), so a snapshot-inherited limit cannot +/// be removed by omission. The explicit disable sentinel is a bucket with both +/// `size == 0` and `refill_time == 0`; a mixed bucket (e.g. `size == 0`, +/// `refill_time == 1`) is not the sentinel and can be rejected as an invalid +/// token bucket, failing the resume PATCH. Send both fields as zero. +fn disabled_bucket() -> Box { + Box::new(firecracker_client::models::TokenBucket::new(0, 0)) +} + +/// Build the limiter to PATCH on resume, reconciling a snapshot-inherited +/// limiter against the node's current config. BOTH buckets are always present: +/// a configured dimension uses its own bucket, an unset dimension is overwritten +/// with a disabled (`size == 0`) bucket so any inherited limit on that dimension +/// is cleared (an omitted bucket would instead be left unchanged; see +/// [`disabled_bucket`]). +fn reconcile_disk_rate_limiter( + cfg: &crate::cfg::DiskRateLimitConfig, +) -> Result> { + let (bandwidth, ops) = if cfg.enabled { + (bandwidth_bucket(cfg)?, ops_bucket(cfg)?) + } else { + (None, None) + }; + let mut rl = firecracker_client::models::RateLimiter::new(); + rl.bandwidth = Some(bandwidth.unwrap_or_else(disabled_bucket)); + rl.ops = Some(ops.unwrap_or_else(disabled_bucket)); + Ok(Box::new(rl)) +} + pub(super) fn managed_snapshot_base() -> PathBuf { ConfigManager::global_config() .firecracker @@ -1468,6 +1560,19 @@ impl FirecrackerSandbox { let mmds_metadata = self.mmds_metadata(&config.common); self.fc_instance.set_mmds(&mmds_metadata).await?; + + // A restored snapshot inherits whatever limiter was active when it was + // paused, so reconcile against the node's current config while the VM is + // still loaded-but-paused — before resume() lets the guest issue I/O. + // Both buckets are always overwritten (configured or unlimited) so an + // inherited dimension the current config leaves unset is cleared rather + // than left unchanged. + let reconciled = reconcile_disk_rate_limiter(&config.common.disk_rate_limit)?; + self.fc_instance + .patch_drive_rate_limiter(USER_ROOTFS_DRIVE_ID, reconciled) + .await + .context("reconcile disk rate limiter on snapshot resume")?; + self.fc_instance.resume().await?; debug!("sandbox restored from snapshot config"); @@ -1572,6 +1677,7 @@ impl FirecrackerSandbox { true, false, IoEngine::Sync, + None, ) .await .with_context(|| { @@ -1581,7 +1687,9 @@ impl FirecrackerSandbox { ) })?; - // Drive 1 (/dev/vdb): user image, writable. + // Drive 1 (/dev/vdb): user image, writable. The disk rate limiter is + // applied here as pre-boot drive config (rather than a post-start PATCH) + // so throttling is in force the instant the guest starts issuing I/O. self.fc_instance .add_drive( USER_ROOTFS_DRIVE_ID, @@ -1590,6 +1698,7 @@ impl FirecrackerSandbox { false, true, IoEngine::Async, + build_disk_rate_limiter(&config.common.disk_rate_limit)?, ) .await?; @@ -1631,6 +1740,7 @@ impl FirecrackerSandbox { drive.read_only, true, IoEngine::Async, + None, ) .await .with_context(|| format!("Failed to add extra drive {}", drive.drive_id))?; @@ -1791,6 +1901,131 @@ mod tests { config } + fn rate_limit_cfg() -> crate::cfg::DiskRateLimitConfig { + crate::cfg::DiskRateLimitConfig { + enabled: true, + bandwidth_bytes_per_sec: 0, + bandwidth_burst_bytes: 0, + iops: 0, + iops_burst: 0, + } + } + + #[test] + fn rate_limiter_disabled_returns_none() { + let mut cfg = rate_limit_cfg(); + cfg.enabled = false; + cfg.bandwidth_bytes_per_sec = 104_857_600; + assert!(build_disk_rate_limiter(&cfg).unwrap().is_none()); + } + + #[test] + fn rate_limiter_enabled_but_all_zero_returns_none() { + assert!(build_disk_rate_limiter(&rate_limit_cfg()) + .unwrap() + .is_none()); + } + + #[test] + fn rate_limiter_bandwidth_size_equals_per_second_rate() { + let mut cfg = rate_limit_cfg(); + cfg.bandwidth_bytes_per_sec = 104_857_600; // 100 MB/s + cfg.bandwidth_burst_bytes = 10_485_760; + let rl = build_disk_rate_limiter(&cfg) + .unwrap() + .expect("limiter present"); + let bw = rl.bandwidth.expect("bandwidth bucket"); + // With refill pinned to 1000 ms, bucket size == sustained bytes/sec. + assert_eq!(bw.refill_time, RATE_LIMIT_REFILL_TIME_MS); + assert_eq!(bw.size, 104_857_600); + assert_eq!(bw.one_time_burst, Some(10_485_760)); + assert!(rl.ops.is_none()); + } + + #[test] + fn rate_limiter_iops_bucket_populated() { + let mut cfg = rate_limit_cfg(); + cfg.iops = 3000; + cfg.iops_burst = 500; + let rl = build_disk_rate_limiter(&cfg) + .unwrap() + .expect("limiter present"); + let ops = rl.ops.expect("ops bucket"); + assert_eq!(ops.refill_time, RATE_LIMIT_REFILL_TIME_MS); + assert_eq!(ops.size, 3000); + assert_eq!(ops.one_time_burst, Some(500)); + assert!(rl.bandwidth.is_none()); + } + + #[test] + fn rate_limiter_rejects_values_beyond_i64_range() { + let mut cfg = rate_limit_cfg(); + cfg.bandwidth_bytes_per_sec = u64::MAX; + assert!(build_disk_rate_limiter(&cfg).is_err()); + } + + #[test] + fn reconcile_disabled_makes_both_buckets_disabled() { + // Firecracker treats an absent bucket in a PATCH as "leave unchanged", so + // clearing an inherited limiter requires overwriting BOTH buckets with a + // disabled (size == 0) bucket rather than sending an empty RateLimiter. + let mut cfg = rate_limit_cfg(); + cfg.enabled = false; + cfg.bandwidth_bytes_per_sec = 100 << 20; + cfg.iops = 3000; + let rl = reconcile_disk_rate_limiter(&cfg).unwrap(); + let bw = rl.bandwidth.expect("bandwidth bucket present"); + let ops = rl.ops.expect("ops bucket present"); + assert_eq!(bw.size, 0); + assert_eq!(ops.size, 0); + } + + #[test] + fn reconcile_bandwidth_only_clears_inherited_iops() { + // Enabled with bandwidth but no iops: bandwidth gets its configured + // bucket, while the unset iops dimension is overwritten with a disabled + // bucket so a snapshot-inherited IOPS limit does not survive the resume. + let mut cfg = rate_limit_cfg(); + cfg.enabled = true; + cfg.bandwidth_bytes_per_sec = 100 << 20; + cfg.iops = 0; + let rl = reconcile_disk_rate_limiter(&cfg).unwrap(); + let bw = rl.bandwidth.expect("bandwidth bucket present"); + let ops = rl.ops.expect("ops bucket present"); + assert_eq!(bw.refill_time, RATE_LIMIT_REFILL_TIME_MS); + assert_eq!(bw.size, 100 << 20); + assert_eq!(ops.size, 0); + } + + #[test] + fn reconcile_iops_only_clears_inherited_bandwidth() { + let mut cfg = rate_limit_cfg(); + cfg.enabled = true; + cfg.bandwidth_bytes_per_sec = 0; + cfg.iops = 3000; + let rl = reconcile_disk_rate_limiter(&cfg).unwrap(); + let bw = rl.bandwidth.expect("bandwidth bucket present"); + let ops = rl.ops.expect("ops bucket present"); + assert_eq!(bw.size, 0); + assert_eq!(ops.refill_time, RATE_LIMIT_REFILL_TIME_MS); + assert_eq!(ops.size, 3000); + } + + #[test] + fn reconcile_both_dimensions_use_configured_buckets() { + let mut cfg = rate_limit_cfg(); + cfg.enabled = true; + cfg.bandwidth_bytes_per_sec = 100 << 20; + cfg.iops = 3000; + let rl = reconcile_disk_rate_limiter(&cfg).unwrap(); + let bw = rl.bandwidth.expect("bandwidth bucket present"); + let ops = rl.ops.expect("ops bucket present"); + assert_eq!(bw.refill_time, RATE_LIMIT_REFILL_TIME_MS); + assert_eq!(bw.size, 100 << 20); + assert_eq!(ops.refill_time, RATE_LIMIT_REFILL_TIME_MS); + assert_eq!(ops.size, 3000); + } + #[test] fn paused_state_image_cache_paths_use_snapshot_artifact_config() { let mut common = fresh_config().common;