From eff99222cb07a225802bc17336cce28662852097 Mon Sep 17 00:00:00 2001 From: tsturzl Date: Mon, 24 May 2021 10:52:34 -0600 Subject: [PATCH 1/2] network controllers added and passing tests locally --- .github/workflows/main.yml | 2 +- src/cgroups/controller_type.rs | 4 ++ src/cgroups/devices.rs | 1 + src/cgroups/hugetlb.rs | 47 ++++++++---- src/cgroups/manager.rs | 10 ++- src/cgroups/memory.rs | 5 +- src/cgroups/mod.rs | 2 + src/cgroups/network_classifier.rs | 98 +++++++++++++++++++++++++ src/cgroups/network_priority.rs | 116 ++++++++++++++++++++++++++++++ src/cgroups/pids.rs | 8 +-- 10 files changed, 268 insertions(+), 25 deletions(-) create mode 100644 src/cgroups/network_classifier.rs create mode 100644 src/cgroups/network_priority.rs diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 66f7fb36..15ba4eae 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -45,7 +45,7 @@ jobs: expect_err_num=8 act_err_num=0 cd $(go env GOPATH)/src/github.com/opencontainers/runtime-tools - test_cases=("default/default.t" "linux_cgroups_devices/linux_cgroups_devices.t" "linux_cgroups_hugetlb/linux_cgroups_hugetlb.t" "linux_cgroups_pids/linux_cgroups_pids.t", "linux_cgroups_memory/linux_cgroups_memory.t") + test_cases=("default/default.t" "linux_cgroups_devices/linux_cgroups_devices.t" "linux_cgroups_hugetlb/linux_cgroups_hugetlb.t" "linux_cgroups_pids/linux_cgroups_pids.t", "linux_cgroups_memory/linux_cgroups_memory.t", "linux_cgroups_network/linux_cgroups_network.t") for case in "${test_cases[@]}"; do title="Running $case" if [ 0 -ne $(sudo RUNTIME=$GITHUB_WORKSPACE/target/x86_64-unknown-linux-gnu/debug/youki ./validation/$case | grep "not ok" | wc -l) ]; then diff --git a/src/cgroups/controller_type.rs b/src/cgroups/controller_type.rs index c4959414..933c593b 100644 --- a/src/cgroups/controller_type.rs +++ b/src/cgroups/controller_type.rs @@ -6,6 +6,8 @@ pub enum ControllerType { Pids, Memory, Blkio, + NetworkPriority, + NetworkClassifier, } impl ToString for ControllerType { @@ -16,6 +18,8 @@ impl ToString for ControllerType { Self::Pids => "pids".into(), Self::Memory => "memory".into(), Self::Blkio => "blkio".into(), + Self::NetworkPriority => "net_prio".into(), + Self::NetworkClassifier => "net_cls".into(), } } } diff --git a/src/cgroups/devices.rs b/src/cgroups/devices.rs index cdfee8bd..eb5d4b40 100644 --- a/src/cgroups/devices.rs +++ b/src/cgroups/devices.rs @@ -37,6 +37,7 @@ pub struct Devices {} impl Controller for Devices { fn apply(linux_resources: &LinuxResources, cgroup_root: &Path, pid: Pid) -> Result<()> { + log::debug!("Apply Devices cgroup config"); create_dir_all(&cgroup_root)?; for d in &linux_resources.devices { diff --git a/src/cgroups/hugetlb.rs b/src/cgroups/hugetlb.rs index 33bd9a82..0f9f60c8 100644 --- a/src/cgroups/hugetlb.rs +++ b/src/cgroups/hugetlb.rs @@ -1,14 +1,26 @@ -use std::{fs::{self, OpenOptions}, io::Write, path::Path}; +use std::{ + fs::{self, OpenOptions}, + io::Write, + path::Path, +}; -use regex::Regex; use anyhow::anyhow; +use regex::Regex; -use crate::{cgroups::Controller, spec::{ LinuxHugepageLimit, LinuxResources}}; +use crate::{ + cgroups::Controller, + spec::{LinuxHugepageLimit, LinuxResources}, +}; pub struct Hugetlb {} impl Controller for Hugetlb { - fn apply(linux_resources: &LinuxResources, cgroup_root: &std::path::Path, pid: nix::unistd::Pid) -> anyhow::Result<()> { + fn apply( + linux_resources: &LinuxResources, + cgroup_root: &std::path::Path, + pid: nix::unistd::Pid, + ) -> anyhow::Result<()> { + log::debug!("Apply Hugetlb cgroup config"); fs::create_dir_all(cgroup_root)?; for hugetlb in &linux_resources.hugepage_limits { @@ -32,18 +44,21 @@ impl Hugetlb { match caps { None => return Err(anyhow!("page size must be in the format [0-9]+[KMG]B")), Some(caps) => { - let page_size:u64 = caps["pagesize"].parse()?; + let page_size: u64 = caps["pagesize"].parse()?; if !Self::is_power_of_two(page_size) { - return Err(anyhow!("page size must be in the format of 2^(integer)")) + return Err(anyhow!("page size must be in the format of 2^(integer)")); } } } - Self::write_file(&root_path.join(format!("hugetlb.{}.limit_in_bytes", hugetlb.page_size)), &hugetlb.limit.to_string())?; + Self::write_file( + &root_path.join(format!("hugetlb.{}.limit_in_bytes", hugetlb.page_size)), + &hugetlb.limit.to_string(), + )?; Ok(()) } - fn write_file(file_path: &Path, data: &str) -> anyhow::Result<()> { + fn write_file(file_path: &Path, data: &str) -> anyhow::Result<()> { fs::OpenOptions::new() .create(false) .write(true) @@ -55,7 +70,7 @@ impl Hugetlb { } fn is_power_of_two(number: u64) -> bool { - (number != 0) && (number & (number -1)) == 0 + (number != 0) && (number & (number - 1)) == 0 } } @@ -91,16 +106,17 @@ mod tests { let hugetlb = LinuxHugepageLimit { page_size: "2MB".to_owned(), limit: 16384, - }; Hugetlb::apply(&tmp, &hugetlb).expect("apply hugetlb"); - let content = std::fs::read_to_string(tmp.join(page_file_name)).expect("Read hugetlb file content"); + let content = + std::fs::read_to_string(tmp.join(page_file_name)).expect("Read hugetlb file content"); assert_eq!(hugetlb.limit.to_string(), content); } #[test] fn test_set_hugetlb_with_invalid_page_size() { - let tmp = create_temp_dir("test_set_hugetlb_with_invalid_page_size").expect("create temp directory for test"); + let tmp = create_temp_dir("test_set_hugetlb_with_invalid_page_size") + .expect("create temp directory for test"); let hugetlb = LinuxHugepageLimit { page_size: "3MB".to_owned(), @@ -108,6 +124,9 @@ mod tests { }; let result = Hugetlb::apply(&tmp, &hugetlb); - assert!(result.is_err(), "page size that is not a power of two should be an error"); + assert!( + result.is_err(), + "page size that is not a power of two should be an error" + ); } -} \ No newline at end of file +} diff --git a/src/cgroups/manager.rs b/src/cgroups/manager.rs index 7a511a34..1fb2d6db 100644 --- a/src/cgroups/manager.rs +++ b/src/cgroups/manager.rs @@ -7,7 +7,11 @@ use procfs::process::Process; use crate::{cgroups::ControllerType, spec::LinuxResources, utils::PathBufExt}; -use super::{devices::Devices, hugetlb::Hugetlb, memory::Memory, pids::Pids, blkio::Blkio, Controller}; +use super::{ + blkio::Blkio, devices::Devices, hugetlb::Hugetlb, memory::Memory, + network_classifier::NetworkClassifier, network_priority::NetworkPriority, pids::Pids, + Controller, +}; const CONTROLLERS: &[ControllerType] = &[ ControllerType::Devices, @@ -15,6 +19,8 @@ const CONTROLLERS: &[ControllerType] = &[ ControllerType::Memory, ControllerType::Pids, ControllerType::Blkio, + ControllerType::NetworkPriority, + ControllerType::NetworkClassifier, ]; pub struct Manager { @@ -42,6 +48,8 @@ impl Manager { "memory" => Memory::apply(linux_resources, &subsys.1, pid)?, "pids" => Pids::apply(linux_resources, &subsys.1, pid)?, "blkio" => Blkio::apply(linux_resources, &subsys.1, pid)?, + "net_prio" => NetworkPriority::apply(linux_resources, &subsys.1, pid)?, + "net_cls" => NetworkClassifier::apply(linux_resources, &subsys.1, pid)?, _ => continue, } } diff --git a/src/cgroups/memory.rs b/src/cgroups/memory.rs index 8cc5805e..3490f4fe 100644 --- a/src/cgroups/memory.rs +++ b/src/cgroups/memory.rs @@ -27,10 +27,7 @@ pub struct Memory {} impl Controller for Memory { fn apply(linux_resources: &LinuxResources, cgroup_root: &Path, pid: Pid) -> Result<()> { - log::info!( - "Memory controller path: {}", - cgroup_root.to_str().unwrap_or("") - ); + log::debug!("Apply Memory cgroup config"); create_dir_all(&cgroup_root)?; if let Some(memory) = &linux_resources.memory { diff --git a/src/cgroups/mod.rs b/src/cgroups/mod.rs index fb5a9637..9eb044b8 100644 --- a/src/cgroups/mod.rs +++ b/src/cgroups/mod.rs @@ -5,6 +5,8 @@ mod hugetlb; mod blkio; mod manager; mod memory; +mod network_classifier; +mod network_priority; mod pids; pub use controller::Controller; pub use controller_type::ControllerType; diff --git a/src/cgroups/network_classifier.rs b/src/cgroups/network_classifier.rs new file mode 100644 index 00000000..6b9e1fa3 --- /dev/null +++ b/src/cgroups/network_classifier.rs @@ -0,0 +1,98 @@ +use std::io::Write; +use std::{ + fs::{create_dir_all, OpenOptions}, + path::Path, +}; + +use anyhow::Result; +use nix::unistd::Pid; + +use crate::{ + cgroups::Controller, + spec::{LinuxNetwork, LinuxResources}, +}; + +pub struct NetworkClassifier {} + +impl Controller for NetworkClassifier { + fn apply(linux_resources: &LinuxResources, cgroup_root: &Path, pid: Pid) -> Result<()> { + log::debug!("Apply NetworkClassifier cgroup config"); + create_dir_all(&cgroup_root)?; + + if let Some(network) = linux_resources.network.as_ref() { + Self::apply(cgroup_root, network)?; + + OpenOptions::new() + .create(false) + .write(true) + .truncate(true) + .open(cgroup_root.join("cgroup.procs"))? + .write_all(pid.to_string().as_bytes())?; + } + + Ok(()) + } +} + +impl NetworkClassifier { + fn apply(root_path: &Path, network: &LinuxNetwork) -> Result<()> { + if let Some(class_id) = network.class_id { + Self::write_file(&root_path.join("net_cls.classid"), &class_id.to_string())?; + } + + Ok(()) + } + + fn write_file(file_path: &Path, data: &str) -> Result<()> { + OpenOptions::new() + .create(false) + .write(true) + .truncate(true) + .open(file_path)? + .write_all(data.as_bytes())?; + + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use std::path::PathBuf; + + use super::*; + + fn set_fixture(temp_dir: &std::path::Path, filename: &str, val: &str) -> Result<()> { + std::fs::OpenOptions::new() + .create(true) + .write(true) + .truncate(true) + .open(temp_dir.join(filename))? + .write_all(val.as_bytes())?; + + Ok(()) + } + + fn create_temp_dir(test_name: &str) -> Result { + std::fs::create_dir_all(std::env::temp_dir().join(test_name))?; + Ok(std::env::temp_dir().join(test_name)) + } + + #[test] + fn test_apply_network_classifier() { + let tmp = create_temp_dir("test_apply_network_classifier") + .expect("create temp directory for test"); + set_fixture(&tmp, "net_cls.classid", "0").expect("set fixture for classID"); + + let id = 0x100001; + let network = LinuxNetwork { + class_id: Some(id), + priorities: vec![], + }; + + NetworkClassifier::apply(&tmp, &network).expect("apply network classID"); + + let content = + std::fs::read_to_string(tmp.join("net_cls.classid")).expect("Read classID contents"); + assert_eq!(id.to_string(), content); + } +} diff --git a/src/cgroups/network_priority.rs b/src/cgroups/network_priority.rs new file mode 100644 index 00000000..a895e379 --- /dev/null +++ b/src/cgroups/network_priority.rs @@ -0,0 +1,116 @@ +use std::io::Write; +use std::{ + fs::{create_dir_all, OpenOptions}, + path::Path, +}; + +use anyhow::Result; +use nix::unistd::Pid; + +use crate::{ + cgroups::Controller, + spec::{LinuxInterfacePriority, LinuxNetwork, LinuxResources}, +}; + +impl ToString for LinuxInterfacePriority { + fn to_string(&self) -> String { + format!("{} {}\n", self.name, self.priority) + } +} + +pub struct NetworkPriority {} + +impl Controller for NetworkPriority { + fn apply(linux_resources: &LinuxResources, cgroup_root: &Path, pid: Pid) -> Result<()> { + log::debug!("Apply NetworkPriority cgroup config"); + create_dir_all(&cgroup_root)?; + + if let Some(network) = linux_resources.network.as_ref() { + Self::apply(cgroup_root, network)?; + + OpenOptions::new() + .create(false) + .write(true) + .truncate(true) + .open(cgroup_root.join("cgroup.procs"))? + .write_all(pid.to_string().as_bytes())?; + } + + Ok(()) + } +} + +impl NetworkPriority { + fn apply(root_path: &Path, network: &LinuxNetwork) -> Result<()> { + let priorities: String = network.priorities.iter().map(|p| p.to_string()).collect(); + Self::write_file(&root_path.join("net_prio.ifpriomap"), &priorities.trim())?; + + Ok(()) + } + + fn write_file(file_path: &Path, data: &str) -> Result<()> { + OpenOptions::new() + .create(false) + .write(true) + .truncate(true) + .open(file_path)? + .write_all(data.as_bytes())?; + + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use std::path::PathBuf; + + use super::*; + + fn set_fixture(temp_dir: &std::path::Path, filename: &str, val: &str) -> Result<()> { + std::fs::OpenOptions::new() + .create(true) + .write(true) + .truncate(true) + .open(temp_dir.join(filename))? + .write_all(val.as_bytes())?; + + Ok(()) + } + + fn create_temp_dir(test_name: &str) -> Result { + std::fs::create_dir_all(std::env::temp_dir().join(test_name))?; + Ok(std::env::temp_dir().join(test_name)) + } + + #[test] + fn test_apply_network_priorites() { + let tmp = create_temp_dir("test_apply_network_priorites") + .expect("create temp directory for test"); + set_fixture(&tmp, "net_prio.ifpriomap", "").expect("set fixture for priority map"); + let priorities = vec![ + LinuxInterfacePriority { + name: "a".to_owned(), + priority: 1, + }, + LinuxInterfacePriority { + name: "b".to_owned(), + priority: 2, + }, + ]; + let priorities_string = priorities + .clone() + .iter() + .map(|p| p.to_string()) + .collect::(); + let network = LinuxNetwork { + class_id: None, + priorities, + }; + + NetworkPriority::apply(&tmp, &network).expect("apply network priorities"); + + let content = + std::fs::read_to_string(tmp.join("net_prio.ifpriomap")).expect("Read classID contents"); + assert_eq!(priorities_string.trim(), content); + } +} diff --git a/src/cgroups/pids.rs b/src/cgroups/pids.rs index e8926080..f8670964 100644 --- a/src/cgroups/pids.rs +++ b/src/cgroups/pids.rs @@ -61,8 +61,6 @@ impl Pids { #[cfg(test)] mod tests { - use std::path::PathBuf; - use super::*; use crate::spec::LinuxPids; @@ -77,7 +75,7 @@ mod tests { Ok(()) } - fn create_temp_dir(test_name: &str) -> Result { + fn create_temp_dir(test_name: &str) -> Result { std::fs::create_dir_all(std::env::temp_dir().join(test_name))?; Ok(std::env::temp_dir().join(test_name)) } @@ -85,7 +83,7 @@ mod tests { #[test] fn test_set_pids() { let pids_file_name = "pids.max"; - let tmp = create_temp_dir("pids").expect("create temp directory for test"); + let tmp = create_temp_dir("test_set_pids").expect("create temp directory for test"); set_fixture(&tmp, pids_file_name, "1000").expect("Set fixture for 1000 pids"); let pids = LinuxPids { limit: 1000 }; @@ -99,7 +97,7 @@ mod tests { #[test] fn test_set_pids_max() { let pids_file_name = "pids.max"; - let tmp = create_temp_dir("pids").expect("create temp directory for test"); + let tmp = create_temp_dir("test_set_pids_max").expect("create temp directory for test"); set_fixture(&tmp, pids_file_name, "0").expect("set fixture for 0 pids"); let pids = LinuxPids { limit: 0 }; From 8ca20a3244d2a7787ed1c316471786029260693a Mon Sep 17 00:00:00 2001 From: tsturzl Date: Tue, 25 May 2021 11:45:49 -0600 Subject: [PATCH 2/2] network controller should work on all systems --- .github/workflows/main.yml | 2 +- src/cgroups/manager.rs | 14 +++++++++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 15ba4eae..2aa736f2 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -48,7 +48,7 @@ jobs: test_cases=("default/default.t" "linux_cgroups_devices/linux_cgroups_devices.t" "linux_cgroups_hugetlb/linux_cgroups_hugetlb.t" "linux_cgroups_pids/linux_cgroups_pids.t", "linux_cgroups_memory/linux_cgroups_memory.t", "linux_cgroups_network/linux_cgroups_network.t") for case in "${test_cases[@]}"; do title="Running $case" - if [ 0 -ne $(sudo RUNTIME=$GITHUB_WORKSPACE/target/x86_64-unknown-linux-gnu/debug/youki ./validation/$case | grep "not ok" | wc -l) ]; then + if [ 0 -ne $(sudo RUST_BACKTRACE=1 RUNTIME=$GITHUB_WORKSPACE/target/x86_64-unknown-linux-gnu/debug/youki ./validation/$case | grep "not ok" | wc -l) ]; then exit 1 fi done diff --git a/src/cgroups/manager.rs b/src/cgroups/manager.rs index 1fb2d6db..b6d59523 100644 --- a/src/cgroups/manager.rs +++ b/src/cgroups/manager.rs @@ -69,10 +69,22 @@ impl Manager { } fn get_subsystem_path(cgroup_path: &Path, subsystem: &str) -> anyhow::Result { + log::debug!("Get path for subsystem: {}", subsystem); let mount = Process::myself()? .mountinfo()? .into_iter() - .find(|m| m.fs_type == "cgroup" && m.mount_point.ends_with(subsystem)) + .find(|m| { + if m.fs_type == "cgroup" { + // Some systems mount net_prio and net_cls in the same directory + // other systems mount them in their own diretories. This + // should handle both cases. + if subsystem == "net_cls" || subsystem == "net_prio" { + return m.mount_point.ends_with("net_cls,net_prio") + || m.mount_point.ends_with("net_prio,net_cls"); + } + } + m.mount_point.ends_with(subsystem) + }) .unwrap(); let cgroup = Process::myself()?