Merge branch 'macos-set-entry-route-mtu'

5 files changed, 55 insertions, 16 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index e4e1228f4b..27a140fd3e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -45,6 +45,8 @@ Line wrap the file at 100 chars.                                              Th
   address.
 - Fix app sometimes getting stuck in error state when the connection is unstable. This occurred
   when the default route was removed while connecting.
+- Improve multihop performance by preventing fragmentation in the tunnel. This is done by setting
+  an MTU on the default route.
 
 ### Changed
 - Remove `--location` flag from `mullvad status` CLI. Location and IP will now always
diff --git a/talpid-routing/src/lib.rs b/talpid-routing/src/lib.rs
index aa43490419..0801807598 100644
--- a/talpid-routing/src/lib.rs
+++ b/talpid-routing/src/lib.rs
@@ -37,7 +37,7 @@ pub struct Route {
     metric: Option<u32>,
     #[cfg(target_os = "linux")]
     table_id: u32,
-    #[cfg(target_os = "linux")]
+    #[cfg(any(target_os = "linux", target_os = "macos"))]
     mtu: Option<u32>,
 }
 
@@ -50,7 +50,7 @@ impl Route {
             metric: None,
             #[cfg(target_os = "linux")]
             table_id: u32::from(RT_TABLE_MAIN),
-            #[cfg(target_os = "linux")]
+            #[cfg(any(target_os = "linux", target_os = "macos"))]
             mtu: None,
         }
     }
@@ -95,7 +95,7 @@ pub struct RequiredRoute {
     #[cfg(target_os = "linux")]
     main_table: bool,
     /// Specifies route MTU
-    #[cfg(target_os = "linux")]
+    #[cfg(any(target_os = "linux", target_os = "macos"))]
     mtu: Option<u16>,
 }
 
@@ -107,7 +107,7 @@ impl RequiredRoute {
             prefix,
             #[cfg(target_os = "linux")]
             main_table: true,
-            #[cfg(target_os = "linux")]
+            #[cfg(any(target_os = "linux", target_os = "macos"))]
             mtu: None,
         }
     }
@@ -120,7 +120,7 @@ impl RequiredRoute {
     }
 
     /// Set route MTU to the given value.
-    #[cfg(target_os = "linux")]
+    #[cfg(any(target_os = "linux", target_os = "macos"))]
     pub fn mtu(mut self, mtu: u16) -> Self {
         self.mtu = Some(mtu);
         self
diff --git a/talpid-routing/src/unix/macos/data.rs b/talpid-routing/src/unix/macos/data.rs
index 1952578860..b404511d2c 100644
--- a/talpid-routing/src/unix/macos/data.rs
+++ b/talpid-routing/src/unix/macos/data.rs
@@ -13,6 +13,7 @@ use std::{
 #[derive(Debug, Clone, PartialEq)]
 pub struct RouteMessage {
     sockaddrs: BTreeMap<AddressFlag, RouteSocketAddress>,
+    mtu: u32,
     route_flags: RouteFlag,
     interface_index: u16,
     errno: i32,
@@ -41,6 +42,7 @@ impl RouteMessage {
 
         Self {
             sockaddrs,
+            mtu: 0,
             route_flags,
             interface_index: 0,
             errno: 0,
@@ -136,8 +138,15 @@ impl RouteMessage {
             .collect::<Result<BTreeMap<_, _>>>()?;
         let interface_index = header.rtm_index;
 
+        let mtu = if header.rtm_inits & RTV_MTU != 0 {
+            header.rtm_rmx.rmx_mtu
+        } else {
+            0
+        };
+
         Ok(Self {
             route_flags,
+            mtu,
             sockaddrs,
             interface_index,
             errno: header.rtm_errno,
@@ -167,6 +176,11 @@ impl RouteMessage {
         self
     }
 
+    pub fn set_mtu(mut self, mtu: u32) -> Self {
+        self.mtu = mtu;
+        self
+    }
+
     pub fn set_interface_addr(mut self, link: &InterfaceAddress) -> Self {
         self.insert_sockaddr(RouteSocketAddress::Gateway(link.address));
         self.route_flags |= RouteFlag::RTF_GATEWAY;
@@ -298,7 +312,7 @@ impl RouteMessage {
             .try_into()
             .expect("route message buffer size cannot fit in 32 bits");
 
-        let header = super::data::rt_msghdr {
+        let mut header = super::data::rt_msghdr {
             rtm_msglen,
             rtm_version: libc::RTM_VERSION.try_into().unwrap(),
             rtm_type: message_type.bits(),
@@ -313,6 +327,11 @@ impl RouteMessage {
             rtm_rmx: Default::default(),
         };
 
+        if self.mtu != 0 {
+            header.rtm_inits |= RTV_MTU;
+            header.rtm_rmx.rmx_mtu = self.mtu;
+        }
+
         (header, payload_bytes)
     }
 
@@ -1150,3 +1169,6 @@ fn test_failing_rtmsg() {
     ];
     let _ = RouteSocketMessage::parse_message(&bytes).unwrap();
 }
+
+// Set MTU flag. See route.h
+const RTV_MTU: u32 = 0x1;
diff --git a/talpid-routing/src/unix/macos/mod.rs b/talpid-routing/src/unix/macos/mod.rs
index 9c01b70820..f45d67c8d0 100644
--- a/talpid-routing/src/unix/macos/mod.rs
+++ b/talpid-routing/src/unix/macos/mod.rs
@@ -201,6 +201,7 @@ impl RouteManagerImpl {
                                     },
                                     prefix: interface::Family::V4.default_network(),
                                     metric: None,
+                                    mtu: None,
                                 }
                             });
                             let v6_route = self.v6_default_route.as_ref().map(|route| {
@@ -211,6 +212,7 @@ impl RouteManagerImpl {
                                     },
                                     prefix: interface::Family::V6.default_network(),
                                     metric: None,
+                                    mtu: None,
                                 }
                             });
 
@@ -263,7 +265,11 @@ impl RouteManagerImpl {
                     self.non_tunnel_routes.insert(route.prefix);
                 }
 
-                NetNode::RealNode(node) => routes_to_apply.push(Route::new(node, route.prefix)),
+                NetNode::RealNode(node) => {
+                    let mut applied_route = Route::new(node, route.prefix);
+                    applied_route.mtu = route.mtu.map(u32::from);
+                    routes_to_apply.push(applied_route);
+                }
             }
         }
 
@@ -273,7 +279,7 @@ impl RouteManagerImpl {
 
         // Add routes not using the default interface
         for route in routes_to_apply {
-            let message = if let Some(ref device) = route.node.device {
+            let mut message = if let Some(ref device) = route.node.device {
                 // If we specify route by interface name, use the link address of the given
                 // interface
                 match interface_link_addrs.get(device) {
@@ -289,6 +295,10 @@ impl RouteManagerImpl {
                 continue;
             };
 
+            if let Some(mtu) = route.mtu {
+                message = message.set_mtu(mtu);
+            }
+
             // Default routes are a special case: We must apply it after replacing the current
             // default route with an ifscope route.
             if route.prefix.prefix() == 0 {
diff --git a/talpid-wireguard/src/lib.rs b/talpid-wireguard/src/lib.rs
index a42a01ee54..f1ddc827e5 100644
--- a/talpid-wireguard/src/lib.rs
+++ b/talpid-wireguard/src/lib.rs
@@ -812,7 +812,7 @@ impl WireguardMonitor {
 
         let (node_v4, node_v6) = Self::get_tunnel_nodes(iface_name, config);
 
-        #[cfg(target_os = "linux")]
+        #[cfg(any(target_os = "linux", target_os = "macos"))]
         let gateway_routes =
             gateway_routes.map(|route| Self::apply_route_mtu_for_multihop(route, config));
 
@@ -847,31 +847,36 @@ impl WireguardMonitor {
                     RequiredRoute::new(allowed_ip, node_v6.clone())
                 }
             });
-        #[cfg(not(target_os = "linux"))]
+        #[cfg(not(any(target_os = "linux", target_os = "macos")))]
         return iter;
 
         #[cfg(target_os = "linux")]
-        iter.map(|route| route.use_main_table(false))
-            .map(|route| Self::apply_route_mtu_for_multihop(route, config))
+        return iter
+            .map(|route| route.use_main_table(false))
+            .map(|route| Self::apply_route_mtu_for_multihop(route, config));
+
+        #[cfg(target_os = "macos")]
+        iter.map(|route| Self::apply_route_mtu_for_multihop(route, config))
     }
 
-    #[cfg(target_os = "linux")]
+    #[cfg(any(target_os = "linux", target_os = "macos"))]
     fn apply_route_mtu_for_multihop(route: RequiredRoute, config: &Config) -> RequiredRoute {
         if !config.is_multihop() {
             route
         } else {
-            // Set route MTU by subtracting the WireGuard overhead from the tunnel MTU.
-            // NOTE: Somewhat incorrect since it doesn't account for packet padding/alignment?
+            // Set route MTU by subtracting the WireGuard overhead from the tunnel MTU. Plus
+            // some margin to make room for padding bytes.
             // TODO: Move consts to shared location
             const IPV4_HEADER_SIZE: u16 = 20;
             const IPV6_HEADER_SIZE: u16 = 40;
             const WIREGUARD_HEADER_SIZE: u16 = 40;
+            const PADDING_BYTES_MARGIN: u16 = 15;
 
             let ip_overhead = match route.prefix.is_ipv4() {
                 true => IPV4_HEADER_SIZE,
                 false => IPV6_HEADER_SIZE,
             };
-            let mtu = config.mtu - ip_overhead - WIREGUARD_HEADER_SIZE;
+            let mtu = config.mtu - ip_overhead - WIREGUARD_HEADER_SIZE - PADDING_BYTES_MARGIN;
 
             route.mtu(mtu)
         }