Improve route cleanup in the route manager

1 files changed, 85 insertions, 19 deletions

diff --git a/talpid-core/src/routing/linux.rs b/talpid-core/src/routing/linux.rs
index e27b0ad849..328ce161f2 100644
--- a/talpid-core/src/routing/linux.rs
+++ b/talpid-core/src/routing/linux.rs
@@ -20,7 +20,8 @@ use netlink_packet_route::{
     route::{nlas::Nla as RouteNla, RouteHeader, RouteMessage},
     rtnl::{
         constants::{
-            RTN_UNICAST, RTPROT_STATIC, RT_SCOPE_UNIVERSE, RT_TABLE_COMPAT, RT_TABLE_MAIN,
+            RTN_UNSPEC, RTPROT_UNSPEC, RT_SCOPE_LINK, RT_SCOPE_UNIVERSE, RT_TABLE_COMPAT,
+            RT_TABLE_MAIN,
         },
         RouteFlags,
     },
@@ -178,8 +179,24 @@ impl RouteManagerImpl {
         Err(Error::NoFreeRoutingTableId)
     }
 
+    async fn purge_exclusions_routes(&mut self) -> Result<()> {
+        let split_routes = self.get_routes(Some(self.split_table_id)).await?;
+        for route in split_routes {
+            if let Err(error) = self.delete_route(&route).await {
+                log::warn!(
+                    "Failed to delete exclusions route: {}\n{}",
+                    route,
+                    error.display_chain()
+                );
+            }
+        }
+        Ok(())
+    }
+
     async fn initialize_exclusions_routes(&mut self) -> Result<()> {
-        let main_routes = self.get_routes().await?;
+        self.purge_exclusions_routes().await?;
+
+        let main_routes = self.get_routes(None).await?;
         for mut route in main_routes {
             route.table_id = self.split_table_id;
             self.add_route_direct(route).await?;
@@ -331,21 +348,30 @@ impl RouteManagerImpl {
         Ok(())
     }
 
-    async fn get_routes(&self) -> Result<HashSet<Route>> {
-        let mut routes = self.get_routes_inner(IpVersion::V4).await?;
-        routes.extend(self.get_routes_inner(IpVersion::V6).await?);
+    async fn get_routes(&self, table_id: Option<u32>) -> Result<HashSet<Route>> {
+        let mut routes = self.get_routes_inner(IpVersion::V4, table_id).await?;
+        routes.extend(self.get_routes_inner(IpVersion::V6, table_id).await?);
         Ok(routes)
     }
 
-    async fn get_routes_inner(&self, version: IpVersion) -> Result<HashSet<Route>> {
+    async fn get_routes_inner(
+        &self,
+        version: IpVersion,
+        table_id: Option<u32>,
+    ) -> Result<HashSet<Route>> {
         let mut routes = HashSet::new();
+        let table_id = table_id.unwrap_or(RT_TABLE_MAIN as u32);
         let mut route_request = self.handle.route().get(version).execute();
+
         while let Some(route) = route_request
             .try_next()
             .await
             .map_err(Error::NetlinkError)?
         {
-            if let Some(route) = self.parse_route_message(route)? {
+            if let Some(route) = self.parse_route_message_inner(route)? {
+                if route.table_id != table_id {
+                    continue;
+                }
                 routes.insert(route);
             }
         }
@@ -401,7 +427,13 @@ impl RouteManagerImpl {
         {
             let mut exclusions_route = route.clone();
             exclusions_route.table_id = self.split_table_id;
-            self.add_route_direct(exclusions_route).await?;
+            if let Err(error) = self.add_route_direct(exclusions_route.clone()).await {
+                log::warn!(
+                    "Failed to add exclusions route: {}\n{}",
+                    exclusions_route,
+                    error.display_chain(),
+                );
+            }
         }
 
         if route.prefix.prefix() == 0 {
@@ -418,9 +450,11 @@ impl RouteManagerImpl {
             let mut exclusions_route = route.clone();
             exclusions_route.table_id = self.split_table_id;
             if let Err(error) = self.delete_route(&exclusions_route).await {
-                log::warn!(
-                    "{}",
-                    error.display_chain_with_msg("Failed to remove exclusions route")
+                // This may be expected when routes are deleted by the kernel
+                log::debug!(
+                    "Failed to remove exclusions route: {}\n{}",
+                    exclusions_route,
+                    error.display_chain(),
                 );
             }
         }
@@ -584,7 +618,7 @@ impl RouteManagerImpl {
         match command {
             RouteManagerCommand::Shutdown(shutdown_signal) => {
                 log::trace!("Shutting down route manager");
-                self.cleanup_routes().await;
+                self.destructor().await;
                 log::trace!("Route manager done");
                 let _ = shutdown_signal.send(());
                 return Err(Error::Shutdown);
@@ -653,8 +687,11 @@ impl RouteManagerImpl {
         if msg.header.table != RT_TABLE_MAIN {
             return Ok(None);
         }
+        self.parse_route_message_inner(msg)
+    }
 
-
+    // Tries to coax a Route out of a RouteMessage
+    fn parse_route_message_inner(&self, msg: RouteMessage) -> Result<Option<Route>> {
         let mut prefix = None;
         let mut node_addr = None;
         let mut device = None;
@@ -759,6 +796,23 @@ impl RouteManagerImpl {
 
     async fn delete_route(&self, route: &Route) -> Result<()> {
         let compat_table = compat_table_id(route.table_id);
+        let scope = match route.prefix {
+            IpNetwork::V4(v4_prefix) => {
+                if v4_prefix.prefix() > 0 && v4_prefix.prefix() < 32 {
+                    RT_SCOPE_LINK
+                } else {
+                    RT_SCOPE_UNIVERSE
+                }
+            }
+            IpNetwork::V6(v6_prefix) => {
+                if v6_prefix.prefix() > 0 && v6_prefix.prefix() < 128 {
+                    RT_SCOPE_LINK
+                } else {
+                    RT_SCOPE_UNIVERSE
+                }
+            }
+        };
+
         let mut route_message = RouteMessage {
             header: RouteHeader {
                 address_family: if route.prefix.is_ipv4() {
@@ -770,9 +824,9 @@ impl RouteManagerImpl {
                 destination_prefix_length: route.prefix.prefix(),
                 tos: 0u8,
                 table: compat_table,
-                protocol: RTPROT_STATIC,
-                scope: RT_SCOPE_UNIVERSE,
-                kind: RTN_UNICAST,
+                protocol: RTPROT_UNSPEC,
+                scope,
+                kind: RTN_UNSPEC,
                 flags: RouteFlags::empty(),
             },
             nlas: vec![RouteNla::Destination(ip_to_bytes(route.prefix.ip()))],
@@ -796,6 +850,9 @@ impl RouteManagerImpl {
             route_message.nlas.push(gateway_nla);
         }
 
+        if let Some(metric) = route.metric {
+            route_message.nlas.push(RouteNla::Priority(metric));
+        }
 
         self.handle
             .route()
@@ -864,8 +921,7 @@ impl RouteManagerImpl {
 
         // TODO: Request support for route priority in RouteAddIpv{4,6}Request
         if let Some(metric) = route.metric {
-            use netlink_packet_route::nlas::route;
-            add_message.nlas.push(route::Nla::Priority(metric));
+            add_message.nlas.push(RouteNla::Priority(metric));
         }
 
         // Need to modify the request in place to set the correct flags to be able to replace any
@@ -890,11 +946,21 @@ impl RouteManagerImpl {
         self.added_routes.insert(route);
         Ok(())
     }
+
+    async fn destructor(&mut self) {
+        if let Err(error) = self.purge_exclusions_routes().await {
+            log::error!(
+                "{}",
+                error.display_chain_with_msg("Failed to flush exclusions routes")
+            );
+        }
+        self.cleanup_routes().await;
+    }
 }
 
 impl Drop for RouteManagerImpl {
     fn drop(&mut self) {
-        futures::executor::block_on(self.cleanup_routes())
+        futures::executor::block_on(self.destructor());
     }
 }