From 6c73add95600719addc63ea542cb7e207cacdaec Mon Sep 17 00:00:00 2001 From: zhoujiahui Date: Tue, 7 Apr 2026 19:36:15 +0800 Subject: [PATCH] Refactor CLI error handling paths --- vtether-cli/src/gc.rs | 11 ++-- vtether-cli/src/helper.rs | 47 ++++++++++++++++- vtether-cli/src/inspect.rs | 34 ++++++++++-- vtether-cli/src/proxy.rs | 103 ++++++++++++++++++++++--------------- vtether-cli/src/setup.rs | 33 ++++++++---- 5 files changed, 167 insertions(+), 61 deletions(-) diff --git a/vtether-cli/src/gc.rs b/vtether-cli/src/gc.rs index 9d9f74f..12f9602 100644 --- a/vtether-cli/src/gc.rs +++ b/vtether-cli/src/gc.rs @@ -1,7 +1,7 @@ use aya::maps::{HashMap, Map, MapData}; use anyhow::Context as _; -use log::info; +use log::{info, warn}; pub const IPPROTO_TCP: u8 = 6; @@ -133,7 +133,9 @@ pub fn reap_conntrack(pin_path: &std::path::Path) -> anyhow::Result { ); } for key in &expired_keys { - let _ = ct4.remove(key); + ct4.remove(key) + .inspect_err(|error| warn!("failed to remove expired CT entry: {error}")) + .ok(); } // ---- Phase 2: Purge orphan SNAT entries ---- @@ -194,7 +196,10 @@ pub fn reap_conntrack(pin_path: &std::path::Path) -> anyhow::Result { info!("gc: purging {} orphan SNAT entries", orphan_keys.len()); } for key in &orphan_keys { - let _ = snat4.remove(key); + snat4 + .remove(key) + .inspect_err(|error| warn!("failed to remove orphan SNAT entry: {error}")) + .ok(); } snat_orphans = orphan_keys.len() as u64; } diff --git a/vtether-cli/src/helper.rs b/vtether-cli/src/helper.rs index 2be6d0c..bfe3459 100644 --- a/vtether-cli/src/helper.rs +++ b/vtether-cli/src/helper.rs @@ -1,7 +1,10 @@ +use std::io::ErrorKind; use std::net::Ipv4Addr; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; +use std::process::Command; use anyhow::Context as _; +use log::warn; const STATE_BASE_DIR: &str = "/run/vtether"; @@ -28,3 +31,45 @@ pub fn get_interface_ipv4(interface: &str) -> anyhow::Result { } anyhow::bail!("no IPv4 address found on interface '{interface}'") } + +pub fn best_effort_command(mut command: Command, description: &str) { + match command + .status() + .inspect_err(|error| warn!("{description}: {error}")) + { + Ok(status) if !status.success() => { + warn!("{description}: exited with status {status}"); + } + _ => {} + } +} + +pub fn best_effort_remove_file(path: &Path) { + std::fs::remove_file(path) + .inspect_err(|error| { + if error.kind() != ErrorKind::NotFound { + warn!("failed to remove {}: {error}", path.display()); + } + }) + .ok(); +} + +pub fn best_effort_remove_dir(path: &Path) { + std::fs::remove_dir(path) + .inspect_err(|error| { + if error.kind() != ErrorKind::NotFound { + warn!("failed to remove {}: {error}", path.display()); + } + }) + .ok(); +} + +pub fn best_effort_remove_dir_all(path: &Path) { + std::fs::remove_dir_all(path) + .inspect_err(|error| { + if error.kind() != ErrorKind::NotFound { + warn!("failed to remove {}: {error}", path.display()); + } + }) + .ok(); +} diff --git a/vtether-cli/src/inspect.rs b/vtether-cli/src/inspect.rs index 0ff3ba8..6462cd1 100644 --- a/vtether-cli/src/inspect.rs +++ b/vtether-cli/src/inspect.rs @@ -2,6 +2,7 @@ use std::net::Ipv4Addr; use anyhow::Context as _; use aya::maps::{HashMap, Map, MapData, PerCpuHashMap, PerCpuValues}; +use log::warn; use crate::{ gc::{CtEntry, Ipv4CtTuple, SnatEntry, ktime_get_ns}, @@ -39,7 +40,9 @@ pub fn inspect(pin_path: &std::path::Path, verbose: bool) -> anyhow::Result<()> ); let state_dir = state_dir_for(pin_path); - let interface = std::fs::read_to_string(state_dir.join("interface")) + let interface_path = state_dir.join("interface"); + let interface = std::fs::read_to_string(&interface_path) + .inspect_err(|error| warn!("failed to read {}: {error}", interface_path.display())) .unwrap_or_else(|_| "unknown".to_string()); println!("vtether: attached to {}", interface.trim()); @@ -104,7 +107,16 @@ pub fn inspect(pin_path: &std::path::Path, verbose: bool) -> anyhow::Result<()> }; if let Some(s) = route_stats .as_ref() - .and_then(|m| m.get(&stats_key, 0).ok()) + .and_then(|map| { + map.get(&stats_key, 0) + .inspect_err(|error| { + warn!( + "failed to read route stats for rev_nat_index {}: {error}", + svc.rev_nat_index + ) + }) + .ok() + }) .map(|v| aggregate_stats(&v)) { print!( @@ -129,7 +141,14 @@ pub fn inspect(pin_path: &std::path::Path, verbose: bool) -> anyhow::Result<()> let map = Map::LruHashMap(map_data); let ct4: HashMap<_, Ipv4CtTuple, CtEntry> = HashMap::try_from(map).context("failed to parse CT4 map")?; - Ok(ct4.iter().filter_map(Result::ok).collect()) + Ok(ct4 + .iter() + .filter_map(|entry| { + entry + .inspect_err(|error| warn!("skipping unreadable CT4 entry: {error}")) + .ok() + }) + .collect()) })() { Ok(entries) => { println!("\nActive connections: {} CT entries", entries.len()); @@ -155,7 +174,14 @@ pub fn inspect(pin_path: &std::path::Path, verbose: bool) -> anyhow::Result<()> let map = Map::LruHashMap(map_data); let snat4: HashMap<_, Ipv4CtTuple, SnatEntry> = HashMap::try_from(map).context("failed to parse SNAT4 map")?; - Ok(snat4.iter().filter_map(Result::ok).collect()) + Ok(snat4 + .iter() + .filter_map(|entry| { + entry + .inspect_err(|error| warn!("skipping unreadable SNAT4 entry: {error}")) + .ok() + }) + .collect()) })() { Ok(entries) => { println!("\nSNAT4 entries ({}):", entries.len()); diff --git a/vtether-cli/src/proxy.rs b/vtether-cli/src/proxy.rs index 134067c..fb1fc02 100644 --- a/vtether-cli/src/proxy.rs +++ b/vtether-cli/src/proxy.rs @@ -1,11 +1,12 @@ use std::net::{Ipv4Addr, SocketAddrV4}; use std::path::PathBuf; +use std::process::Command; use anyhow::Context as _; use aya::maps::{Array, HashMap}; use aya::programs::links::FdLink; use aya::programs::{Xdp, XdpFlags}; -use log::info; +use log::{info, warn}; use serde::Deserialize; use crate::{ @@ -13,7 +14,10 @@ use crate::{ GC_INTERVAL_DEFAULT_SECS, GC_INTERVAL_MAX_SECS, GC_INTERVAL_MIN_SECS, IPPROTO_TCP, adapt_gc_interval, reap_conntrack, }, - helper::{get_interface_ipv4, state_dir_for}, + helper::{ + best_effort_command, best_effort_remove_dir, best_effort_remove_dir_all, + best_effort_remove_file, get_interface_ipv4, state_dir_for, + }, setup::setup_sysctl, }; @@ -122,6 +126,28 @@ pub struct SnatConfig { } unsafe impl aya::Pod for SnatConfig {} +fn cleanup_partial_proxy_state( + interface: &str, + prog_pin: &std::path::Path, + pin_path: &std::path::Path, + state_dir: &std::path::Path, +) { + let mut detach_xdp = Command::new("ip"); + detach_xdp.args(["link", "set", "dev", interface, "xdp", "off"]); + best_effort_command( + detach_xdp, + &format!("failed to detach XDP from {interface}"), + ); + + best_effort_remove_file(prog_pin); + best_effort_remove_file(&pin_path.join("link")); + for &(_, pin_name) in MAP_PINS { + best_effort_remove_file(&pin_path.join(pin_name)); + } + best_effort_remove_dir(pin_path); + best_effort_remove_dir_all(state_dir); +} + #[allow(clippy::too_many_lines)] pub async fn proxy_up(config_path: PathBuf, pin_path: PathBuf) -> anyhow::Result<()> { let config_str = std::fs::read_to_string(&config_path) @@ -288,9 +314,9 @@ pub async fn proxy_up(config_path: PathBuf, pin_path: PathBuf) -> anyhow::Result } // Init eBPF logger - if let Err(e) = aya_log::EbpfLogger::init(&mut ebpf) { - log::warn!("failed to init eBPF logger: {e}"); - } + aya_log::EbpfLogger::init(&mut ebpf) + .inspect_err(|error| warn!("failed to init eBPF logger: {error}")) + .ok(); // Pin maps std::fs::create_dir_all(&pin_path) @@ -336,19 +362,12 @@ pub async fn proxy_up(config_path: PathBuf, pin_path: PathBuf) -> anyhow::Result Ok(()) }; - if let Err(e) = finish() { - let _ = std::process::Command::new("ip") - .args(["link", "set", "dev", &config.interface, "xdp", "off"]) - .status(); - let _ = std::fs::remove_file(&prog_pin); - let _ = std::fs::remove_file(pin_path.join("link")); - for &(_, pin_name) in MAP_PINS { - let _ = std::fs::remove_file(pin_path.join(pin_name)); - } - let _ = std::fs::remove_dir(&pin_path); - let _ = std::fs::remove_dir_all(&state_dir); - return Err(e.context("proxy up failed, cleaned up partial state")); - } + finish() + .inspect_err(|error| { + warn!("proxy up finalization failed: {error:#}"); + cleanup_partial_proxy_state(&config.interface, &prog_pin, &pin_path, &state_dir); + }) + .map_err(|error| error.context("proxy up failed, cleaned up partial state"))?; info!( "proxy up: xdp on {}, snat_ip: {}, conntrack: {}", @@ -372,23 +391,20 @@ pub async fn proxy_up(config_path: PathBuf, pin_path: PathBuf) -> anyhow::Result ); loop { tokio::time::sleep(std::time::Duration::from_secs(gc_interval_secs)).await; - match reap_conntrack(&reaper_pin_path) { - Ok(result) => { - if result.expired > 0 || result.orphans > 0 { - info!( - "gc cycle: total={} expired={} orphans={} next_interval={}s", - result.total, - result.expired, - result.orphans, - adapt_gc_interval(gc_interval_secs, result.total, result.expired), - ); - } - gc_interval_secs = - adapt_gc_interval(gc_interval_secs, result.total, result.expired); - } - Err(e) => { - log::warn!("conntrack gc error: {e:#}"); + if let Ok(result) = reap_conntrack(&reaper_pin_path) + .inspect_err(|error| warn!("conntrack gc error: {error:#}")) + { + if result.expired > 0 || result.orphans > 0 { + info!( + "gc cycle: total={} expired={} orphans={} next_interval={}s", + result.total, + result.expired, + result.orphans, + adapt_gc_interval(gc_interval_secs, result.total, result.expired), + ); } + gc_interval_secs = + adapt_gc_interval(gc_interval_secs, result.total, result.expired); } } }); @@ -432,17 +448,20 @@ pub fn proxy_destroy(pin_path: &std::path::Path) -> anyhow::Result<()> { link.unpin().context("failed to unpin link")?; } - let _ = std::process::Command::new("ip") - .args(["link", "set", "dev", interface.trim(), "xdp", "off"]) - .status(); + let mut detach_xdp = Command::new("ip"); + detach_xdp.args(["link", "set", "dev", interface.trim(), "xdp", "off"]); + best_effort_command( + detach_xdp, + &format!("failed to detach XDP from {}", interface.trim()), + ); - let _ = std::fs::remove_file(&prog_pin); - let _ = std::fs::remove_file(pin_path.join("link")); + best_effort_remove_file(&prog_pin); + best_effort_remove_file(&pin_path.join("link")); for &(_, pin_name) in MAP_PINS { - let _ = std::fs::remove_file(pin_path.join(pin_name)); + best_effort_remove_file(&pin_path.join(pin_name)); } - let _ = std::fs::remove_dir(pin_path); - let _ = std::fs::remove_dir_all(&state_dir); + best_effort_remove_dir(pin_path); + best_effort_remove_dir_all(&state_dir); println!( "vtether: proxy destroy (detached from {})", diff --git a/vtether-cli/src/setup.rs b/vtether-cli/src/setup.rs index ee26c19..43ee2ce 100644 --- a/vtether-cli/src/setup.rs +++ b/vtether-cli/src/setup.rs @@ -1,7 +1,10 @@ use std::path::PathBuf; +use std::process::Command; use anyhow::Context as _; +use crate::helper::best_effort_command; + pub const DEFAULT_CONFIG_PATH: &str = "/etc/vtether/config.yaml"; const SYSTEMD_UNIT_PATH: &str = "/etc/systemd/system/vtether.service"; @@ -65,9 +68,16 @@ fn get_default_interface() -> anyhow::Result { pub fn setup() -> anyhow::Result<()> { let vtether_bin = std::env::current_exe().context("failed to determine vtether binary path")?; - let vtether_bin = vtether_bin.canonicalize().unwrap_or(vtether_bin); + let vtether_bin = vtether_bin + .canonicalize() + .inspect_err(|error| log::warn!("failed to canonicalize current executable: {error}")) + .unwrap_or(vtether_bin); - let default_iface = get_default_interface().unwrap_or_else(|_| "eth0".to_string()); + let default_iface = get_default_interface() + .inspect_err(|error| { + log::warn!("failed to detect default interface, falling back to eth0: {error:#}") + }) + .unwrap_or_else(|_| "eth0".to_string()); let config_dir = PathBuf::from(DEFAULT_CONFIG_PATH) .parent() @@ -109,12 +119,13 @@ pub fn setup() -> anyhow::Result<()> { } pub fn remove() -> anyhow::Result<()> { - let _ = std::process::Command::new("systemctl") - .args(["stop", "vtether"]) - .status(); - let _ = std::process::Command::new("systemctl") - .args(["disable", "vtether"]) - .status(); + let mut stop = Command::new("systemctl"); + stop.args(["stop", "vtether"]); + best_effort_command(stop, "failed to stop vtether service"); + + let mut disable = Command::new("systemctl"); + disable.args(["disable", "vtether"]); + best_effort_command(disable, "failed to disable vtether service"); if PathBuf::from(SYSTEMD_UNIT_PATH).exists() { std::fs::remove_file(SYSTEMD_UNIT_PATH) @@ -122,9 +133,9 @@ pub fn remove() -> anyhow::Result<()> { println!(" removed {SYSTEMD_UNIT_PATH}"); } - let _ = std::process::Command::new("systemctl") - .args(["daemon-reload"]) - .status(); + let mut daemon_reload = Command::new("systemctl"); + daemon_reload.args(["daemon-reload"]); + best_effort_command(daemon_reload, "failed to reload systemd units"); println!("\nvtether removed."); Ok(())