From beead5a30377dc28eb6665f250e354f72546f671 Mon Sep 17 00:00:00 2001 From: Herbert Wolverson Date: Mon, 20 Mar 2023 17:40:03 +0000 Subject: [PATCH] Backport the TC_HANDLE parser to main. Replace C code with native Rust, including unit tests to find the edge-cases. --- src/rust/Cargo.lock | 1 - src/rust/lqos_bus/Cargo.toml | 3 -- src/rust/lqos_bus/build.rs | 3 -- src/rust/lqos_bus/src/tc_handle.rs | 56 ++++++++++++------------ src/rust/lqos_bus/src/tc_handle_parser.c | 46 ------------------- src/rust/lqos_utils/src/hex_string.rs | 3 ++ 6 files changed, 30 insertions(+), 82 deletions(-) delete mode 100644 src/rust/lqos_bus/build.rs delete mode 100644 src/rust/lqos_bus/src/tc_handle_parser.c diff --git a/src/rust/Cargo.lock b/src/rust/Cargo.lock index 951c1825..f8835901 100644 --- a/src/rust/Cargo.lock +++ b/src/rust/Cargo.lock @@ -1327,7 +1327,6 @@ name = "lqos_bus" version = "0.1.0" dependencies = [ "bincode", - "cc", "criterion", "log", "lqos_config", diff --git a/src/rust/lqos_bus/Cargo.toml b/src/rust/lqos_bus/Cargo.toml index 345675cf..bb3fd25b 100644 --- a/src/rust/lqos_bus/Cargo.toml +++ b/src/rust/lqos_bus/Cargo.toml @@ -17,9 +17,6 @@ tokio = { version = "1", features = [ "rt", "macros", "net", "io-util", "time" ] log = "0" nix = "0" -[build-dependencies] -cc = "1.0" - [dev-dependencies] criterion = { version = "0", features = [ "html_reports", "async_tokio"] } diff --git a/src/rust/lqos_bus/build.rs b/src/rust/lqos_bus/build.rs deleted file mode 100644 index 5e414178..00000000 --- a/src/rust/lqos_bus/build.rs +++ /dev/null @@ -1,3 +0,0 @@ -fn main() { - cc::Build::new().file("src/tc_handle_parser.c").compile("tc_handle_parse.o"); -} diff --git a/src/rust/lqos_bus/src/tc_handle.rs b/src/rust/lqos_bus/src/tc_handle.rs index 20585c42..dbea21de 100644 --- a/src/rust/lqos_bus/src/tc_handle.rs +++ b/src/rust/lqos_bus/src/tc_handle.rs @@ -1,6 +1,6 @@ use log::error; +use lqos_utils::hex_string::read_hex_string; use serde::{Deserialize, Serialize}; -use std::ffi::CString; use thiserror::Error; /// Provides consistent handling of TC handle types. @@ -9,20 +9,9 @@ use thiserror::Error; )] pub struct TcHandle(u32); -#[allow(non_camel_case_types)] -type __u32 = ::std::os::raw::c_uint; -#[allow(dead_code)] const TC_H_ROOT: u32 = 4294967295; -#[allow(dead_code)] const TC_H_UNSPEC: u32 = 0; -extern "C" { - pub fn get_tc_classid( - h: *mut __u32, - str_: *const ::std::os::raw::c_char, - ) -> ::std::os::raw::c_int; -} - impl TcHandle { /// Returns the TC handle as two values, indicating major and minor /// TC handle values. @@ -36,23 +25,26 @@ impl TcHandle { /// Build a TC handle from a string. This is actually a complicated /// operation, since it has to handle "root" and other strings as well /// as simple "1:2" mappings. Calls a C function to handle this gracefully. - pub fn from_string( - handle: S, + pub fn from_string( + handle: &str, ) -> Result { - let mut tc_handle: __u32 = 0; - let str = CString::new(handle.to_string()); - if str.is_err() { - error!("Unable to convert {} to a C-String.", handle.to_string()); - return Err(TcHandleParseError::CString); - } - let str = str.unwrap(); - let handle_pointer: *mut __u32 = &mut tc_handle; - let result = unsafe { get_tc_classid(handle_pointer, str.as_ptr()) }; - if result != 0 { - error!("Unable to parse {} as a valid TC handle", handle.to_string()); - Err(TcHandleParseError::InvalidInput) - } else { - Ok(Self(tc_handle)) + let handle = handle.trim(); + match handle { + "root" => Ok(Self(TC_H_ROOT)), + "none" => Ok(Self(TC_H_UNSPEC)), + _ => { + if !handle.contains(':') { + error!("Unable to parse TC handle {handle}. Must contain a colon."); + return Err(TcHandleParseError::InvalidInput(handle.to_string())); + } + let parts: Vec<&str> = handle.split(':').collect(); + let major = read_hex_string(parts[0]).map_err(|_| TcHandleParseError::InvalidInput(handle.to_string()))?; + let minor = read_hex_string(parts[1]).map_err(|_| TcHandleParseError::InvalidInput(handle.to_string()))?; + if major >= (1<<16) || minor >= (1<<16) { + return Err(TcHandleParseError::InvalidInput(handle.to_string())); + } + Ok(Self((major << 16) | minor)) + } } } @@ -86,7 +78,7 @@ pub enum TcHandleParseError { )] CString, #[error("Invalid input")] - InvalidInput, + InvalidInput(String), } #[cfg(test)] @@ -151,4 +143,10 @@ mod test { } } } + + #[test] + fn blank_minor() { + let tc = TcHandle::from_string("7FFF:").unwrap(); + assert_eq!(tc.to_string().to_uppercase(), "7FFF:0"); + } } diff --git a/src/rust/lqos_bus/src/tc_handle_parser.c b/src/rust/lqos_bus/src/tc_handle_parser.c deleted file mode 100644 index e0412254..00000000 --- a/src/rust/lqos_bus/src/tc_handle_parser.c +++ /dev/null @@ -1,46 +0,0 @@ -// Imported from https://github.com/thebracket/cpumap-pping/blob/master/src/xdp_iphash_to_cpu_cmdline.c -// Because it uses strtoul and is based on the TC source, including it directly -// seemed like the path of least resistance. - -#include -#include -#include -#include -#include /* TC macros */ - -/* Handle classid parsing based on iproute source */ -int get_tc_classid(__u32 *h, const char *str) -{ - __u32 major, minor; - char *p; - - major = TC_H_ROOT; - if (strcmp(str, "root") == 0) - goto ok; - major = TC_H_UNSPEC; - if (strcmp(str, "none") == 0) - goto ok; - major = strtoul(str, &p, 16); - if (p == str) { - major = 0; - if (*p != ':') - return -1; - } - if (*p == ':') { - if (major >= (1<<16)) - return -1; - major <<= 16; - str = p+1; - minor = strtoul(str, &p, 16); - if (*p != 0) - return -1; - if (minor >= (1<<16)) - return -1; - major |= minor; - } else if (*p != 0) - return -1; - -ok: - *h = major; - return 0; -} \ No newline at end of file diff --git a/src/rust/lqos_utils/src/hex_string.rs b/src/rust/lqos_utils/src/hex_string.rs index ed78ee93..7e47ceb8 100644 --- a/src/rust/lqos_utils/src/hex_string.rs +++ b/src/rust/lqos_utils/src/hex_string.rs @@ -19,6 +19,9 @@ use thiserror::Error; /// assert_eq!(read_hex_string("0x12AD").unwrap(), 4781); /// ``` pub fn read_hex_string(s: &str) -> Result { + if s.is_empty() { + return Ok(0); + } let result = u32::from_str_radix(&s.replace("0x", ""), 16); match result { Ok(data) => Ok(data),