simplify netlink operations

src/main.rs

@@ -54,6 +54,18 @@ struct Config {
     failback_delay: u64,
 }
 
+fn apply_env_overrides(mut config: Config) -> Config {
+    config.primary_interface =
+        std::env::var("PRIMARY_INTERFACE").unwrap_or(config.primary_interface);
+    config.secondary_interface =
+        std::env::var("SECONDARY_INTERFACE").unwrap_or(config.secondary_interface);
+    config.primary_gateway = std::env::var("PRIMARY_GATEWAY").unwrap_or(config.primary_gateway);
+    config.secondary_gateway =
+        std::env::var("SECONDARY_GATEWAY").unwrap_or(config.secondary_gateway);
+    config.ping_target = std::env::var("PING_TARGET").unwrap_or(config.ping_target);
+    config
+}
+
 #[tokio::main]
 async fn main() -> Result<()> {
     let env = Env::default().filter_or("RUST_LOG", "info");
@@ -64,22 +76,7 @@ async fn main() -> Result<()> {
     let config = Config::parse();
 
     // Override with environment variables if present
-    let primary_interface =
-        std::env::var("PRIMARY_INTERFACE").unwrap_or(config.primary_interface.clone());
-    let secondary_interface =
-        std::env::var("SECONDARY_INTERFACE").unwrap_or(config.secondary_interface.clone());
-    let primary_gateway =
-        std::env::var("PRIMARY_GATEWAY").unwrap_or(config.primary_gateway.clone());
-    let secondary_gateway =
-        std::env::var("SECONDARY_GATEWAY").unwrap_or(config.secondary_gateway.clone());
-    let ping_target = std::env::var("PING_TARGET").unwrap_or(config.ping_target.clone());
-
-    let mut config_with_env = config;
-    config_with_env.primary_interface = primary_interface;
-    config_with_env.secondary_interface = secondary_interface;
-    config_with_env.primary_gateway = primary_gateway;
-    config_with_env.secondary_gateway = secondary_gateway;
-    config_with_env.ping_target = ping_target;
+    let config_with_env = apply_env_overrides(config);
 
     debug!("Configuration: {:?}", config_with_env);
 
@@ -127,6 +124,45 @@ async fn main() -> Result<()> {
 
 use state_machine::StateMachine;
 
+async fn handle_ping_result(
+    result: pinger::PingResult,
+    interface_name: &str,
+    state_machine: &Arc<tokio::sync::Mutex<StateMachine>>,
+    last_failover: &Arc<tokio::sync::Mutex<Option<chrono::DateTime<Utc>>>>,
+    route_manager: &mut routing::RouteManager,
+    primary_gateway: &Ipv4Addr,
+    secondary_gateway: &Ipv4Addr,
+    config: &Config,
+) -> Result<()> {
+    debug!("{} ping result: {}", interface_name, result);
+    let mut sm = state_machine.lock().await;
+
+    // Add result to appropriate history based on interface
+    if interface_name == "primary" {
+        sm.add_primary_result(result);
+    } else {
+        sm.add_secondary_result(result);
+    }
+
+    if let Some((old_state, new_state)) = sm.update_state() {
+        let mut last_failover_lock = last_failover.lock().await;
+        if new_state == state_machine::State::Fallback
+            && old_state != state_machine::State::Fallback
+        {
+            *last_failover_lock = Some(Utc::now());
+        }
+        state_machine::handle_state_change(
+            new_state,
+            old_state,
+            route_manager,
+            primary_gateway,
+            secondary_gateway,
+            config,
+        )?;
+    }
+    Ok(())
+}
+
 async fn main_service(
     config: Config,
     primary_gateway: Ipv4Addr,
@@ -204,32 +240,30 @@ async fn main_service(
         tokio::select! {
             // Handle primary ping results
             Some(result) = primary_rx.recv() => {
-                debug!("Primary ping result: {}", result);
-                let mut sm = state_machine.lock().await;
-                sm.add_primary_result(result);
-
-                if let Some((old_state, new_state)) = sm.update_state() {
-                    let mut last_failover_lock = last_failover.lock().await;
-                    if new_state == state_machine::State::Fallback && old_state != state_machine::State::Fallback {
-                        *last_failover_lock = Some(Utc::now());
-                    }
-                    state_machine::handle_state_change(new_state, old_state, &mut route_manager, &primary_gateway, &secondary_gateway, &config)?;
-                }
+                handle_ping_result(
+                    result,
+                    "primary",
+                    &state_machine,
+                    &last_failover,
+                    &mut route_manager,
+                    &primary_gateway,
+                    &secondary_gateway,
+                    &config,
+                ).await?;
             }
 
             // Handle secondary ping results
             Some(result) = secondary_rx.recv() => {
-                debug!("Secondary ping result: {}", result);
-                let mut sm = state_machine.lock().await;
-                sm.add_secondary_result(result);
-
-                if let Some((old_state, new_state)) = sm.update_state() {
-                    let mut last_failover_lock = last_failover.lock().await;
-                    if new_state == state_machine::State::Fallback && old_state != state_machine::State::Fallback {
-                        *last_failover_lock = Some(Utc::now());
-                    }
-                    state_machine::handle_state_change(new_state, old_state, &mut route_manager, &primary_gateway, &secondary_gateway, &config)?;
-                }
+                handle_ping_result(
+                    result,
+                    "secondary",
+                    &state_machine,
+                    &last_failover,
+                    &mut route_manager,
+                    &primary_gateway,
+                    &secondary_gateway,
+                    &config,
+                ).await?;
             }
 
             // Handle shutdown signal

src/routing.rs

@@ -2,11 +2,20 @@ use anyhow::Result;
 use libc::if_nametoindex;
 use log::{debug, info};
 use netlink_packet_route::route::RouteAddress;
+use netlink_packet_route::{
+    AddressFamily, RouteNetlinkMessage,
+    route::{RouteAttribute, RouteHeader, RouteMessage, RouteProtocol, RouteScope, RouteType},
+};
 use std::ffi::CString;
 use std::net::Ipv4Addr;
 
 const MAIN_TABLE_ID: u8 = 254;
 
+// Route metrics - higher priority = lower number
+const FAILOVER_METRIC: u32 = 5;
+const PRIMARY_METRIC: u32 = 10;
+const SECONDARY_METRIC: u32 = 20;
+
 #[derive(Debug, Clone)]
 pub struct RouteInfo {
     pub gateway: Ipv4Addr,
@@ -60,8 +69,6 @@ impl RouteManager {
     }
 
     pub fn set_primary_route(&mut self, gateway: Ipv4Addr, interface: String) -> Result<()> {
-        let primary_metric = 10;
-
         // Remove existing routes for this interface if any
         if let Some(pos) = self.routes.iter().position(|r| r.interface == interface) {
             let existing_route = self.routes[pos].clone();
@@ -73,7 +80,7 @@ impl RouteManager {
         }
 
         // Add as primary route
-        self.add_route(gateway, interface, primary_metric)?;
+        self.add_route(gateway, interface, PRIMARY_METRIC)?;
         Ok(())
     }
 
@@ -88,20 +95,17 @@ impl RouteManager {
         self.set_primary_route(primary_gateway, primary_interface)?;
 
         // Set secondary route with metric 20 (lower priority)
-        let secondary_metric = 20;
-        self.add_route(secondary_gateway, secondary_interface, secondary_metric)?;
+        self.add_route(secondary_gateway, secondary_interface, SECONDARY_METRIC)?;
         Ok(())
     }
 
     pub fn add_failover_route(&mut self, gateway: Ipv4Addr, interface: String) -> Result<()> {
-        let failover_metric = 5; // Higher priority than both primary (10) and secondary (20)
-        self.add_route(gateway, interface, failover_metric)?;
+        self.add_route(gateway, interface, FAILOVER_METRIC)?;
         Ok(())
     }
 
     pub fn remove_failover_route(&mut self, gateway: Ipv4Addr, interface: String) -> Result<()> {
-        let failover_metric = 5;
-        self.remove_route(gateway, &interface, failover_metric)?;
+        self.remove_route(gateway, &interface, FAILOVER_METRIC)?;
         Ok(())
     }
 
@@ -123,36 +127,16 @@ impl RouteManager {
         use netlink_packet_core::{
             NLM_F_ACK, NLM_F_CREATE, NLM_F_REQUEST, NetlinkHeader, NetlinkMessage, NetlinkPayload,
         };
-        use netlink_packet_route::{
-            AddressFamily, RouteNetlinkMessage,
-            route::RouteProtocol,
-            route::RouteScope,
-            route::{RouteAttribute, RouteHeader, RouteMessage, RouteType},
-        };
         use netlink_sys::{Socket, SocketAddr, protocols::NETLINK_ROUTE};
 
         let mut socket = Socket::new(NETLINK_ROUTE)?;
         let _port_number = socket.bind_auto()?.port_number();
         socket.connect(&SocketAddr::new(0, 0))?;
-        let route_msg_hdr = RouteHeader {
-            address_family: AddressFamily::Inet,
-            table: MAIN_TABLE_ID,
-            destination_prefix_length: 0, // Default route
-            protocol: RouteProtocol::Boot,
-            scope: RouteScope::Universe,
-            kind: RouteType::Unicast,
-            ..Default::default()
-        };
 
-        let mut route_msg = RouteMessage::default();
-        route_msg.header = route_msg_hdr;
-        route_msg.attributes = vec![
-            RouteAttribute::Gateway(RouteAddress::Inet(route_info.gateway)),
-            RouteAttribute::Oif(index),
-            RouteAttribute::Priority(route_info.metric),
-        ];
+        let route_msg = create_route_message(route_info.gateway, index, route_info.metric);
+
         let mut nl_hdr = NetlinkHeader::default();
-        nl_hdr.flags = NLM_F_REQUEST | NLM_F_CREATE | NLM_F_ACK; // Remove NLM_F_EXCL to allow updates
+        nl_hdr.flags = NLM_F_REQUEST | NLM_F_CREATE | NLM_F_ACK;
 
         let mut msg = NetlinkMessage::new(
             nl_hdr,
@@ -161,11 +145,8 @@ impl RouteManager {
 
         msg.finalize();
         let mut buf = vec![0; 1024 * 8];
-
         msg.serialize(&mut buf[..msg.buffer_len()]);
 
-        // Debug: Log the netlink message being sent
-        debug!("Netlink message being sent: {:?}", &buf[..msg.buffer_len()]);
         debug!(
             "Route addition attempt: gateway={}, interface={}, metric={}, interface_index={}",
             route_info.gateway, route_info.interface, route_info.metric, index
@@ -198,33 +179,18 @@ impl RouteManager {
                                 route_info.metric
                             );
                         } else {
-                            let error_str = match error_code {
-                                -1 => "EPERM - Operation not permitted (need root privileges)",
-                                -2 => "ENOENT - No such file or directory",
-                                -13 => "EACCES - Permission denied",
-                                -22 => "EINVAL - Invalid argument",
-                                _ => "Unknown error",
-                            };
-                            return Err(anyhow::anyhow!(
-                                "Failed to add route: {} (code: {}): {:?}",
-                                error_str,
-                                error_code,
-                                error_msg
-                            ));
+                            return handle_netlink_error(error_code);
                         }
                     }
                     debug!("Route added successfully");
                 }
+                Ok(())
             }
-            Err(e) => {
-                return Err(anyhow::anyhow!(
-                    "Failed to deserialize netlink message: {}",
-                    e
-                ));
-            }
+            Err(e) => Err(anyhow::anyhow!(
+                "Failed to deserialize netlink message: {}",
+                e
+            )),
         }
-
-        Ok(())
     }
 
     fn delete_default_route_internal(
@@ -242,35 +208,13 @@ impl RouteManager {
         use netlink_packet_core::{
             NLM_F_ACK, NLM_F_REQUEST, NetlinkHeader, NetlinkMessage, NetlinkPayload,
         };
-        use netlink_packet_route::{
-            AddressFamily, RouteNetlinkMessage,
-            route::RouteProtocol,
-            route::RouteScope,
-            route::{RouteAttribute, RouteHeader, RouteMessage, RouteType},
-        };
         use netlink_sys::{Socket, SocketAddr, protocols::NETLINK_ROUTE};
 
         let mut socket = Socket::new(NETLINK_ROUTE)?;
         let _port_number = socket.bind_auto()?.port_number();
         socket.connect(&SocketAddr::new(0, 0))?;
 
-        let route_msg_hdr = RouteHeader {
-            address_family: AddressFamily::Inet,
-            table: MAIN_TABLE_ID,
-            destination_prefix_length: 0, // Default route
-            protocol: RouteProtocol::Boot,
-            scope: RouteScope::Universe,
-            kind: RouteType::Unicast,
-            ..Default::default()
-        };
-
-        let mut route_msg = RouteMessage::default();
-        route_msg.header = route_msg_hdr;
-        route_msg.attributes = vec![
-            RouteAttribute::Gateway(RouteAddress::Inet(gateway)),
-            RouteAttribute::Oif(index),
-            RouteAttribute::Priority(metric),
-        ];
+        let route_msg = create_route_message(gateway, index, metric);
 
         let mut nl_hdr = NetlinkHeader::default();
         nl_hdr.flags = NLM_F_REQUEST | NLM_F_ACK;
@@ -282,14 +226,8 @@ impl RouteManager {
 
         msg.finalize();
         let mut buf = vec![0; 1024 * 8];
-
         msg.serialize(&mut buf[..msg.buffer_len()]);
 
-        // Debug: Log the netlink message being sent
-        debug!(
-            "Netlink delete message being sent: {:?}",
-            &buf[..msg.buffer_len()]
-        );
         debug!(
             "Route deletion attempt: gateway={}, interface={}, metric={}, interface_index={}",
             gateway, interface, metric, index
@@ -315,16 +253,13 @@ impl RouteManager {
                     }
                     debug!("Route deleted successfully");
                 }
+                Ok(())
             }
-            Err(e) => {
-                return Err(anyhow::anyhow!(
-                    "Failed to deserialize netlink message: {}",
-                    e
-                ));
-            }
+            Err(e) => Err(anyhow::anyhow!(
+                "Failed to deserialize netlink message: {}",
+                e
+            )),
         }
-
-        Ok(())
     }
 }
 
@@ -338,3 +273,44 @@ fn get_interface_index(iface_name: &str) -> Result<u32> {
         Ok(index)
     }
 }
+
+fn create_route_header() -> RouteHeader {
+    RouteHeader {
+        address_family: AddressFamily::Inet,
+        table: MAIN_TABLE_ID,
+        destination_prefix_length: 0, // Default route
+        protocol: RouteProtocol::Boot,
+        scope: RouteScope::Universe,
+        kind: RouteType::Unicast,
+        ..Default::default()
+    }
+}
+
+fn handle_netlink_error(error_code: i32) -> Result<()> {
+    if error_code == -17 {
+        // EEXIST - Route already exists, treat as success
+        return Ok(());
+    }
+
+    let error_str = match error_code {
+        -1 => "EPERM - Operation not permitted (need root privileges)",
+        -2 => "ENOENT - No such file or directory",
+        -13 => "EACCES - Permission denied",
+        -22 => "EINVAL - Invalid argument",
+        _ => "Unknown error",
+    };
+    Err(anyhow::anyhow!("Netlink operation failed: {}", error_str))
+}
+
+fn create_route_message(gateway: Ipv4Addr, interface_index: u32, metric: u32) -> RouteMessage {
+    let route_msg_hdr = create_route_header();
+
+    let mut route_msg = RouteMessage::default();
+    route_msg.header = route_msg_hdr;
+    route_msg.attributes = vec![
+        RouteAttribute::Gateway(RouteAddress::Inet(gateway)),
+        RouteAttribute::Oif(interface_index),
+        RouteAttribute::Priority(metric),
+    ];
+    route_msg
+}