From 118d909143222390f7a646d573ea6c66860b6424 Mon Sep 17 00:00:00 2001 From: Justin Schilleman Date: Fri, 8 May 2026 22:17:57 -0400 Subject: [PATCH 1/2] refactor: migrate to more permissive/accurate string types The API exclusively accepted `String` parameters before. This is forcing an unergonmic API for users by forcing them to allocate a `String` for an internal detail. --- examples/add_route_pref_src.rs | 3 +- examples/create_macvlan.rs | 6 ++-- examples/create_macvtap.rs | 6 ++-- examples/create_vlan.rs | 8 ++--- examples/create_vxlan.rs | 8 +++-- examples/create_xfrm.rs | 9 +++-- examples/del_link.rs | 8 +++-- examples/flush_addresses.rs | 8 +++-- examples/get_address.rs | 8 +++-- examples/get_bond_port_settings.rs | 9 ++--- examples/get_links.rs | 8 +++-- examples/get_links_async.rs | 8 +++-- examples/get_links_thread_builder.rs | 8 +++-- examples/set_link_down.rs | 8 +++-- src/link/builder.rs | 4 +-- src/link/get.rs | 6 ++-- src/link/test.rs | 2 +- src/ns.rs | 54 ++++++++++++++++------------ src/rule/add.rs | 12 ++++--- 19 files changed, 117 insertions(+), 66 deletions(-) diff --git a/examples/add_route_pref_src.rs b/examples/add_route_pref_src.rs index ef9c7cd..49bab9b 100644 --- a/examples/add_route_pref_src.rs +++ b/examples/add_route_pref_src.rs @@ -38,10 +38,11 @@ async fn main() -> Result<(), ()> { async fn add_route( dest: &Ipv4Network, - iface: String, + iface: impl Into, source: Ipv4Addr, handle: Handle, ) -> Result<(), Error> { + let iface = iface.into(); let iface_idx = handle .link() .get() diff --git a/examples/create_macvlan.rs b/examples/create_macvlan.rs index 9ccfb78..0f32fd0 100644 --- a/examples/create_macvlan.rs +++ b/examples/create_macvlan.rs @@ -35,11 +35,11 @@ async fn main() -> Result<(), String> { async fn create_macvlan( handle: Handle, - link_name: String, + link_name: impl Into, mac_address: Option>, ) -> Result<(), Error> { - let mut parent_links = - handle.link().get().match_name(link_name.clone()).execute(); + let link_name = link_name.into(); + let mut parent_links = handle.link().get().match_name(&link_name).execute(); if let Some(parent) = parent_links.try_next().await? { let mut builder = LinkMacVlan::new( "my-macvlan", diff --git a/examples/create_macvtap.rs b/examples/create_macvtap.rs index 1307897..ac69570 100644 --- a/examples/create_macvtap.rs +++ b/examples/create_macvtap.rs @@ -26,10 +26,10 @@ async fn main() -> Result<(), String> { async fn create_macvtap( handle: Handle, - link_name: String, + link_name: impl Into, ) -> Result<(), Error> { - let mut parent_links = - handle.link().get().match_name(link_name.clone()).execute(); + let link_name = link_name.into(); + let mut parent_links = handle.link().get().match_name(&link_name).execute(); if let Some(parent) = parent_links.try_next().await? { let message = LinkMacVtap::new( "test_macvtap", diff --git a/examples/create_vlan.rs b/examples/create_vlan.rs index 668ada0..79f1b17 100644 --- a/examples/create_vlan.rs +++ b/examples/create_vlan.rs @@ -42,7 +42,7 @@ async fn main() -> Result<(), String> { let mut mode = ParsingMode::None; for argument in args.drain(1..) { - fn match_argument(argument: String) -> Result { + fn match_argument(argument: &str) -> Result { match argument.to_lowercase().as_str() { ARG_BASE => Ok(ParsingMode::Base), ARG_NAME => Ok(ParsingMode::Name), @@ -57,7 +57,7 @@ async fn main() -> Result<(), String> { } mode = match mode { - ParsingMode::None => match_argument(argument)?, + ParsingMode::None => match_argument(&argument)?, ParsingMode::Base => { base_interface = u32::from_str(&argument).ok(); ParsingMode::None @@ -75,14 +75,14 @@ async fn main() -> Result<(), String> { ingress.push(mapping); mode } - Err(_) => match_argument(argument)?, + Err(_) => match_argument(&argument)?, }, mode @ ParsingMode::Egress => match parse_mapping(&argument) { Ok(mapping) => { egress.push(mapping); mode } - Err(_) => match_argument(argument)?, + Err(_) => match_argument(&argument)?, }, } } diff --git a/examples/create_vxlan.rs b/examples/create_vxlan.rs index e7f56c3..2ccb2d1 100644 --- a/examples/create_vxlan.rs +++ b/examples/create_vxlan.rs @@ -22,8 +22,12 @@ async fn main() -> Result<(), String> { .map_err(|e| format!("{e}")) } -async fn create_vxlan(handle: Handle, name: String) -> Result<(), Error> { - let mut links = handle.link().get().match_name(name.clone()).execute(); +async fn create_vxlan( + handle: Handle, + name: impl Into, +) -> Result<(), Error> { + let name = name.into(); + let mut links = handle.link().get().match_name(&name).execute(); if let Some(link) = links.try_next().await? { let message = LinkVxlan::new("vxlan0", 10) .dev(link.header.index) diff --git a/examples/create_xfrm.rs b/examples/create_xfrm.rs index 1476304..8a977f2 100644 --- a/examples/create_xfrm.rs +++ b/examples/create_xfrm.rs @@ -20,9 +20,12 @@ async fn main() -> Result<(), String> { .map_err(|e| format!("{e}")) } -async fn create_xfrm(handle: Handle, link_name: String) -> Result<(), Error> { - let mut parent_links = - handle.link().get().match_name(link_name.clone()).execute(); +async fn create_xfrm( + handle: Handle, + link_name: impl Into, +) -> Result<(), Error> { + let link_name = link_name.into(); + let mut parent_links = handle.link().get().match_name(&link_name).execute(); if let Some(parent) = parent_links.try_next().await? { let request = handle .link() diff --git a/examples/del_link.rs b/examples/del_link.rs index c53570d..0cec747 100644 --- a/examples/del_link.rs +++ b/examples/del_link.rs @@ -23,8 +23,12 @@ async fn main() -> Result<(), ()> { Ok(()) } -async fn del_link(handle: Handle, name: String) -> Result<(), Error> { - let mut links = handle.link().get().match_name(name.clone()).execute(); +async fn del_link( + handle: Handle, + name: impl Into, +) -> Result<(), Error> { + let name = name.into(); + let mut links = handle.link().get().match_name(&name).execute(); if let Some(link) = links.try_next().await? { handle.link().del(link.header.index).execute().await } else { diff --git a/examples/flush_addresses.rs b/examples/flush_addresses.rs index cf9bb1d..681b1ff 100644 --- a/examples/flush_addresses.rs +++ b/examples/flush_addresses.rs @@ -24,8 +24,12 @@ async fn main() -> Result<(), ()> { Ok(()) } -async fn flush_addresses(handle: Handle, link: String) -> Result<(), Error> { - let mut links = handle.link().get().match_name(link.clone()).execute(); +async fn flush_addresses( + handle: Handle, + link: impl Into, +) -> Result<(), Error> { + let link = link.into(); + let mut links = handle.link().get().match_name(&link).execute(); if let Some(link) = links.try_next().await? { // We should have received only one message assert!(links.try_next().await?.is_none()); diff --git a/examples/get_address.rs b/examples/get_address.rs index ccfd017..012ab98 100644 --- a/examples/get_address.rs +++ b/examples/get_address.rs @@ -18,8 +18,12 @@ async fn main() -> Result<(), ()> { Ok(()) } -async fn dump_addresses(handle: Handle, link: String) -> Result<(), Error> { - let mut links = handle.link().get().match_name(link.clone()).execute(); +async fn dump_addresses( + handle: Handle, + link: impl Into, +) -> Result<(), Error> { + let link = link.into(); + let mut links = handle.link().get().match_name(&link).execute(); if let Some(link) = links.try_next().await? { let mut addresses = handle .address() diff --git a/examples/get_bond_port_settings.rs b/examples/get_bond_port_settings.rs index 7b39714..cf38dcb 100644 --- a/examples/get_bond_port_settings.rs +++ b/examples/get_bond_port_settings.rs @@ -31,12 +31,13 @@ async fn main() -> Result<(), ()> { async fn dump_bond_port_settings( handle: Handle, - linkname: String, + link_name: impl Into, ) -> Result<(), Error> { - let mut links = handle.link().get().match_name(linkname.clone()).execute(); + let link_name = link_name.into(); + let mut links = handle.link().get().match_name(&link_name).execute(); if let Some(_link) = links.try_next().await? { let mut link_messgage = - handle.link().get().match_name(linkname).execute(); + handle.link().get().match_name(link_name).execute(); while let Some(msg) = link_messgage.try_next().await? { for nla in msg.attributes { if let LinkAttribute::LinkInfo(i) = &nla { @@ -46,7 +47,7 @@ async fn dump_bond_port_settings( } Ok(()) } else { - eprintln!("link {linkname} not found"); + eprintln!("link {link_name} not found"); Ok(()) } } diff --git a/examples/get_links.rs b/examples/get_links.rs index 08cf08b..e63bd3d 100644 --- a/examples/get_links.rs +++ b/examples/get_links.rs @@ -68,8 +68,12 @@ async fn get_link_by_index(handle: Handle, index: u32) -> Result<(), Error> { Ok(()) } -async fn get_link_by_name(handle: Handle, name: String) -> Result<(), Error> { - let mut links = handle.link().get().match_name(name.clone()).execute(); +async fn get_link_by_name( + handle: Handle, + name: impl Into, +) -> Result<(), Error> { + let name = name.into(); + let mut links = handle.link().get().match_name(&name).execute(); if (links.try_next().await?).is_some() { println!("found link {name}"); // We should only have one link with that name diff --git a/examples/get_links_async.rs b/examples/get_links_async.rs index 5392f3b..f35a6b4 100644 --- a/examples/get_links_async.rs +++ b/examples/get_links_async.rs @@ -68,8 +68,12 @@ async fn get_link_by_index(handle: Handle, index: u32) -> Result<(), Error> { Ok(()) } -async fn get_link_by_name(handle: Handle, name: String) -> Result<(), Error> { - let mut links = handle.link().get().match_name(name.clone()).execute(); +async fn get_link_by_name( + handle: Handle, + name: impl Into, +) -> Result<(), Error> { + let name = name.into(); + let mut links = handle.link().get().match_name(&name).execute(); if (links.try_next().await?).is_some() { println!("found link {name}"); // We should only have one link with that name diff --git a/examples/get_links_thread_builder.rs b/examples/get_links_thread_builder.rs index dac183a..2a19b85 100644 --- a/examples/get_links_thread_builder.rs +++ b/examples/get_links_thread_builder.rs @@ -66,8 +66,12 @@ async fn get_link_by_index(handle: Handle, index: u32) -> Result<(), Error> { Ok(()) } -async fn get_link_by_name(handle: Handle, name: String) -> Result<(), Error> { - let mut links = handle.link().get().match_name(name.clone()).execute(); +async fn get_link_by_name( + handle: Handle, + name: impl Into, +) -> Result<(), Error> { + let name = name.into(); + let mut links = handle.link().get().match_name(&name).execute(); if (links.try_next().await?).is_some() { println!("found link {name}"); // We should only have one link with that name diff --git a/examples/set_link_down.rs b/examples/set_link_down.rs index 898d80e..cf4696c 100644 --- a/examples/set_link_down.rs +++ b/examples/set_link_down.rs @@ -22,8 +22,12 @@ async fn main() -> Result<(), String> { .map_err(|e| format!("{e}")) } -async fn set_link_down(handle: Handle, name: String) -> Result<(), Error> { - let mut links = handle.link().get().match_name(name.clone()).execute(); +async fn set_link_down( + handle: Handle, + name: impl Into, +) -> Result<(), Error> { + let name = name.into(); + let mut links = handle.link().get().match_name(&name).execute(); if let Some(link) = links.try_next().await? { handle .link() diff --git a/src/link/builder.rs b/src/link/builder.rs index 09d4235..9a2bfee 100644 --- a/src/link/builder.rs +++ b/src/link/builder.rs @@ -149,8 +149,8 @@ impl LinkMessageBuilder { ret } - pub fn name(self, name: String) -> Self { - self.append_extra_attribute(LinkAttribute::IfName(name)) + pub fn name(self, name: impl Into) -> Self { + self.append_extra_attribute(LinkAttribute::IfName(name.into())) } /// Set the mtu of the link with the given index (equivalent to diff --git a/src/link/get.rs b/src/link/get.rs index 3a41462..b06024c 100644 --- a/src/link/get.rs +++ b/src/link/get.rs @@ -90,9 +90,11 @@ impl LinkGetRequest { /// /// This function requires support from your kernel (>= 2.6.33). If yours is /// older, consider filtering the resulting stream of links. - pub fn match_name(mut self, name: String) -> Self { + pub fn match_name(mut self, name: impl Into) -> Self { self.dump = false; - self.message.attributes.push(LinkAttribute::IfName(name)); + self.message + .attributes + .push(LinkAttribute::IfName(name.into())); self } } diff --git a/src/link/test.rs b/src/link/test.rs index e34fb07..3be0abc 100644 --- a/src/link/test.rs +++ b/src/link/test.rs @@ -127,7 +127,7 @@ async fn _create_wg() -> Result { async fn _get_iface( handle: &mut LinkHandle, - iface_name: String, + iface_name: impl Into, ) -> Result { let mut links = handle.get().match_name(iface_name).execute(); let msg = links.try_next().await?; diff --git a/src/ns.rs b/src/ns.rs index cc34f13..03b1886 100644 --- a/src/ns.rs +++ b/src/ns.rs @@ -1,6 +1,9 @@ // SPDX-License-Identifier: MIT -use std::{path::Path, process::exit}; +use std::{ + path::{Path, PathBuf}, + process::exit, +}; use nix::{ fcntl::OFlag, @@ -10,6 +13,7 @@ use nix::{ wait::{waitpid, WaitStatus}, }, unistd::{fork, ForkResult}, + NixPath, }; use crate::Error; @@ -60,7 +64,7 @@ pub struct NetworkNamespace(); impl NetworkNamespace { /// Add a new network namespace. /// This is equivalent to `ip netns add NS_NAME`. - pub async fn add(ns_name: String) -> Result<(), Error> { + pub async fn add(ns_name: impl AsRef) -> Result<(), Error> { // Forking process to avoid moving caller into new namespace NetworkNamespace::prep_for_fork()?; log::trace!("Forking..."); @@ -69,7 +73,7 @@ impl NetworkNamespace { NetworkNamespace::parent_process(child) } Ok(ForkResult::Child) => { - NetworkNamespace::child_process(ns_name); + NetworkNamespace::child_process(ns_name.as_ref()); } Err(e) => { let err_msg = format!("Fork failed: {e}"); @@ -80,15 +84,15 @@ impl NetworkNamespace { /// Remove a network namespace /// This is equivalent to `ip netns del NS_NAME`. - pub async fn del(ns_name: String) -> Result<(), Error> { - try_spawn_blocking(move || { - let mut netns_path = String::new(); - netns_path.push_str(NETNS_PATH); - netns_path.push_str(&ns_name); - let ns_path = Path::new(&netns_path); + pub async fn del(ns_name: impl AsRef) -> Result<(), Error> { + let netns_path = Path::new(NETNS_PATH).join(&ns_name); - if nix::mount::umount2(ns_path, nix::mount::MntFlags::MNT_DETACH) - .is_err() + try_spawn_blocking(move || { + if nix::mount::umount2( + &netns_path, + nix::mount::MntFlags::MNT_DETACH, + ) + .is_err() { let err_msg = String::from( "Namespace unmount failed (are you running as root?)", @@ -96,7 +100,7 @@ impl NetworkNamespace { return Err(Error::NamespaceError(err_msg)); } - if nix::unistd::unlink(ns_path).is_err() { + if nix::unistd::unlink(&netns_path).is_err() { let err_msg = String::from( "Namespace file remove failed (are you running as root?)", ); @@ -150,7 +154,7 @@ impl NetworkNamespace { } } - fn child_process(ns_name: String) -> ! { + fn child_process(ns_name: &Path) -> ! { let res = std::panic::catch_unwind(|| -> Result<(), Error> { let netns_path = NetworkNamespace::child_process_create_ns(ns_name)?; @@ -174,10 +178,13 @@ impl NetworkNamespace { /// This is the child process, it will actually create the namespace /// resources. It creates the folder and namespace file. /// Returns the namespace file path - pub fn child_process_create_ns(ns_name: String) -> Result { + pub fn child_process_create_ns( + ns_name: impl AsRef, + ) -> Result { log::trace!("child_process will create the namespace"); - let mut netns_path = String::new(); + let mut netns_path = + PathBuf::with_capacity(NETNS_PATH.len() + ns_name.as_ref().len()); let dir_path = Path::new(NETNS_PATH); let mut mkdir_mode = Mode::empty(); @@ -197,8 +204,8 @@ impl NetworkNamespace { open_flags.insert(OFlag::O_CREAT); open_flags.insert(OFlag::O_EXCL); - netns_path.push_str(NETNS_PATH); - netns_path.push_str(&ns_name); + netns_path.push(NETNS_PATH); + netns_path.push(&ns_name); // creating namespaces folder if not exists #[allow(clippy::collapsible_if)] @@ -256,10 +263,9 @@ impl NetworkNamespace { return Err(Error::NamespaceError(err_msg)); } - let ns_path = Path::new(&netns_path); - // creating the netns file - let fd = match nix::fcntl::open(ns_path, open_flags, Mode::empty()) { + let fd = match nix::fcntl::open(&netns_path, open_flags, Mode::empty()) + { Ok(raw_fd) => raw_fd, Err(e) => { log::error!("open error: {}", e); @@ -271,7 +277,7 @@ impl NetworkNamespace { if let Err(e) = nix::unistd::close(fd) { log::error!("close error: {}", e); let err_msg = format!("close error: {e}"); - let _ = nix::unistd::unlink(ns_path); + let _ = nix::unistd::unlink(&netns_path); return Err(Error::NamespaceError(err_msg)); } @@ -281,10 +287,12 @@ impl NetworkNamespace { /// This function unshare the calling process and move into /// the given network namespace #[allow(unused)] - pub fn unshare_processing(netns_path: String) -> Result<(), Error> { + pub fn unshare_processing( + netns_path: impl AsRef, + ) -> Result<(), Error> { let mut setns_flags = CloneFlags::empty(); let mut open_flags = OFlag::empty(); - let ns_path = Path::new(&netns_path); + let ns_path = netns_path.as_ref(); let none_fs = Path::new(&NONE_FS); let none_p4: Option<&Path> = None; diff --git a/src/rule/add.rs b/src/rule/add.rs index ef9b511..ba508fe 100644 --- a/src/rule/add.rs +++ b/src/rule/add.rs @@ -44,14 +44,18 @@ impl RuleAddRequest { } /// Sets the input interface name. - pub fn input_interface(mut self, ifname: String) -> Self { - self.message.attributes.push(RuleAttribute::Iifname(ifname)); + pub fn input_interface(mut self, ifname: impl Into) -> Self { + self.message + .attributes + .push(RuleAttribute::Iifname(ifname.into())); self } /// Sets the output interface name. - pub fn output_interface(mut self, ifname: String) -> Self { - self.message.attributes.push(RuleAttribute::Oifname(ifname)); + pub fn output_interface(mut self, ifname: impl Into) -> Self { + self.message + .attributes + .push(RuleAttribute::Oifname(ifname.into())); self } From 89f8463125ef784241cb55d61ee93e0978e152c8 Mon Sep 17 00:00:00 2001 From: Justin Schilleman Date: Fri, 8 May 2026 22:31:34 -0400 Subject: [PATCH 2/2] fix: fix init. of PathBuf to be more concise and fix off by 1 in allocation --- src/ns.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/ns.rs b/src/ns.rs index 03b1886..fa86a5e 100644 --- a/src/ns.rs +++ b/src/ns.rs @@ -183,8 +183,7 @@ impl NetworkNamespace { ) -> Result { log::trace!("child_process will create the namespace"); - let mut netns_path = - PathBuf::with_capacity(NETNS_PATH.len() + ns_name.as_ref().len()); + let netns_path = Path::new(NETNS_PATH).join(ns_name); let dir_path = Path::new(NETNS_PATH); let mut mkdir_mode = Mode::empty(); @@ -204,9 +203,6 @@ impl NetworkNamespace { open_flags.insert(OFlag::O_CREAT); open_flags.insert(OFlag::O_EXCL); - netns_path.push(NETNS_PATH); - netns_path.push(&ns_name); - // creating namespaces folder if not exists #[allow(clippy::collapsible_if)] if nix::sys::stat::stat(dir_path).is_err() {