Merge branch 'fix-boot-time-iface-issue'

5 files changed, 57 insertions, 42 deletions

diff --git a/talpid-core/src/tunnel/openvpn/mod.rs b/talpid-core/src/tunnel/openvpn/mod.rs
index 9eaa25875f..db36e3d035 100644
--- a/talpid-core/src/tunnel/openvpn/mod.rs
+++ b/talpid-core/src/tunnel/openvpn/mod.rs
@@ -120,7 +120,7 @@ pub enum Error {
     /// cannot create a wintun interface
     #[cfg(windows)]
     #[error(display = "Failed to create Wintun adapter")]
-    WintunError(#[error(source)] io::Error),
+    WintunCreateAdapterError(#[error(source)] io::Error),
 
     /// cannot determine adapter name
     #[cfg(windows)]
@@ -265,7 +265,7 @@ pub struct OpenVpnMonitor<C: OpenVpnBuilder = OpenVpnCommand> {
     server_join_handle: Option<task::JoinHandle<std::result::Result<(), event_server::Error>>>,
 
     #[cfg(windows)]
-    wintun: Arc<Box<dyn WintunContext>>,
+    _wintun: Arc<Box<dyn WintunContext>>,
 }
 
 #[cfg(windows)]
@@ -384,8 +384,7 @@ impl OpenVpnMonitor<OpenVpnCommand> {
                 &*ADAPTER_POOL,
                 Some(ADAPTER_GUID.clone()),
             )
-            .map_err(Error::WintunError)?;
-
+            .map_err(Error::WintunCreateAdapterError)?;
             if reboot_required {
                 log::warn!("You may need to restart Windows to complete the install of Wintun");
             }
@@ -572,7 +571,7 @@ impl<C: OpenVpnBuilder + Send + 'static> OpenVpnMonitor<C> {
             server_join_handle: Some(server_join_handle),
 
             #[cfg(windows)]
-            wintun,
+            _wintun: wintun,
         })
     }
 
diff --git a/talpid-core/src/tunnel/openvpn/windows.rs b/talpid-core/src/tunnel/openvpn/windows.rs
index 6e49bb1f0d..db5cd4d933 100644
--- a/talpid-core/src/tunnel/openvpn/windows.rs
+++ b/talpid-core/src/tunnel/openvpn/windows.rs
@@ -198,7 +198,7 @@ impl WintunAdapter {
         let mut guid = mem::MaybeUninit::zeroed();
         let result = unsafe { ConvertInterfaceLuidToGuid(&self.luid(), guid.as_mut_ptr()) };
         if result != NO_ERROR {
-            return Err(io::Error::last_os_error());
+            return Err(io::Error::from_raw_os_error(result as i32));
         }
         Ok(unsafe { guid.assume_init() })
     }
diff --git a/talpid-core/src/tunnel/wireguard/wireguard_go.rs b/talpid-core/src/tunnel/wireguard/wireguard_go.rs
index f8fbd21e2e..e83a74ab47 100644
--- a/talpid-core/src/tunnel/wireguard/wireguard_go.rs
+++ b/talpid-core/src/tunnel/wireguard/wireguard_go.rs
@@ -75,34 +75,18 @@ impl WgGoTunnel {
             .map_err(TunnelError::LoggingError)?;
 
         #[cfg(not(target_os = "android"))]
+        let mtu = config.mtu as isize;
         let handle = unsafe {
             wgTurnOn(
-                config.mtu as isize,
+                #[cfg(not(target_os = "android"))]
+                mtu,
                 wg_config_str.as_ptr() as *const i8,
                 tunnel_fd,
                 Some(logging_callback),
                 logging_context.0 as *mut libc::c_void,
             )
         };
-
-        #[cfg(target_os = "android")]
-        let handle = unsafe {
-            wgTurnOn(
-                wg_config_str.as_ptr() as *const i8,
-                tunnel_fd,
-                Some(logging_callback),
-                logging_context.0 as *mut libc::c_void,
-            )
-        };
-
-        if handle < 0 {
-            // Error values returned from the wireguard-go library
-            return match handle {
-                -1 => Err(TunnelError::FatalStartWireguardError),
-                -2 => Err(TunnelError::RecoverableStartWireguardError),
-                _ => unreachable!("Unknown status code returned from wireguard-go"),
-            };
-        }
+        check_wg_status(handle)?;
 
         #[cfg(target_os = "android")]
         Self::bypass_tunnel_sockets(&mut tunnel_device, handle)
@@ -153,10 +137,7 @@ impl WgGoTunnel {
                 logging_context.0 as *mut libc::c_void,
             )
         };
-
-        if handle < 0 {
-            return Err(TunnelError::FatalStartWireguardError);
-        }
+        check_wg_status(handle)?;
 
         let actual_iface_name = {
             let actual_iface_name_c = unsafe { CStr::from_ptr(alias_ptr) };
@@ -343,6 +324,18 @@ impl Tunnel for WgGoTunnel {
     }
 }
 
+fn check_wg_status(wg_code: i32) -> Result<()> {
+    match wg_code {
+        ERROR_GENERAL_FAILURE => Err(TunnelError::FatalStartWireguardError),
+        ERROR_INTERMITTENT_FAILURE => Err(TunnelError::RecoverableStartWireguardError),
+        0.. => Ok(()),
+        _ => {
+            log::error!("Unknown status code returned from wireguard-go");
+            Err(TunnelError::FatalStartWireguardError)
+        }
+    }
+}
+
 #[cfg(unix)]
 pub type Fd = std::os::unix::io::RawFd;
 
@@ -352,6 +345,9 @@ pub type LoggingCallback = unsafe extern "system" fn(
     context: *mut libc::c_void,
 );
 
+const ERROR_GENERAL_FAILURE: i32 = -1;
+const ERROR_INTERMITTENT_FAILURE: i32 = -2;
+
 extern "C" {
     /// Creates a new wireguard tunnel, uses the specific interface name, MTU and file descriptors
     /// for the tunnel device and logging.
diff --git a/talpid-core/src/tunnel_state_machine/connecting_state.rs b/talpid-core/src/tunnel_state_machine/connecting_state.rs
index 1566e28517..066163fd73 100644
--- a/talpid-core/src/tunnel_state_machine/connecting_state.rs
+++ b/talpid-core/src/tunnel_state_machine/connecting_state.rs
@@ -41,6 +41,8 @@ pub(crate) type TunnelCloseEvent = Fuse<oneshot::Receiver<Option<ErrorStateCause
 #[cfg(target_os = "android")]
 const MAX_ATTEMPTS_WITH_SAME_TUN: u32 = 5;
 const MIN_TUNNEL_ALIVE_TIME: Duration = Duration::from_millis(1000);
+#[cfg(target_os = "windows")]
+const MAX_ADAPTER_FAIL_RETRIES: u32 = 4;
 
 /// The tunnel has been started, but it is not established/functional.
 pub struct ConnectingState {
@@ -118,7 +120,8 @@ impl ConnectingState {
             route_manager,
         )?;
         let close_handle = Some(monitor.close_handle());
-        let tunnel_close_event = Self::spawn_tunnel_monitor_wait_thread(Some(monitor));
+        let tunnel_close_event =
+            Self::spawn_tunnel_monitor_wait_thread(Some(monitor), retry_attempt);
 
         Ok(ConnectingState {
             tunnel_events: event_rx.fuse(),
@@ -130,14 +133,17 @@ impl ConnectingState {
         })
     }
 
-    fn spawn_tunnel_monitor_wait_thread(tunnel_monitor: Option<TunnelMonitor>) -> TunnelCloseEvent {
+    fn spawn_tunnel_monitor_wait_thread(
+        tunnel_monitor: Option<TunnelMonitor>,
+        retry_attempt: u32,
+    ) -> TunnelCloseEvent {
         let (tunnel_close_event_tx, tunnel_close_event_rx) = oneshot::channel();
 
         thread::spawn(move || {
             let start = Instant::now();
 
             let block_reason = if let Some(monitor) = tunnel_monitor {
-                let reason = Self::wait_for_tunnel_monitor(monitor);
+                let reason = Self::wait_for_tunnel_monitor(monitor, retry_attempt);
                 debug!("Tunnel monitor exited with block reason: {:?}", reason);
                 reason
             } else {
@@ -160,7 +166,10 @@ impl ConnectingState {
         tunnel_close_event_rx.fuse()
     }
 
-    fn wait_for_tunnel_monitor(tunnel_monitor: TunnelMonitor) -> Option<ErrorStateCause> {
+    fn wait_for_tunnel_monitor(
+        tunnel_monitor: TunnelMonitor,
+        retry_attempt: u32,
+    ) -> Option<ErrorStateCause> {
         match tunnel_monitor.wait() {
             Ok(_) => None,
             Err(error) => match error {
@@ -171,7 +180,7 @@ impl ConnectingState {
                     None
                 }
                 error @ tunnel::Error::WireguardTunnelMonitoringError(..)
-                    if !should_retry(&error) =>
+                    if !should_retry(&error, retry_attempt) =>
                 {
                     error!(
                         "{}",
@@ -403,10 +412,11 @@ impl ConnectingState {
     }
 }
 
-fn should_retry(error: &tunnel::Error) -> bool {
-    use tunnel::wireguard::Error;
-    #[cfg(not(windows))]
-    use tunnel::wireguard::TunnelError;
+#[cfg_attr(not(target_os = "windows"), allow(unused_variables))]
+fn should_retry(error: &tunnel::Error, retry_attempt: u32) -> bool {
+    #[cfg(windows)]
+    use tunnel::openvpn;
+    use tunnel::wireguard::{Error, TunnelError};
 
     match error {
         tunnel::Error::WireguardTunnelMonitoringError(Error::Udp2TcpError(_)) => true,
@@ -426,6 +436,16 @@ fn should_retry(error: &tunnel::Error) -> bool {
             is_recoverable_routing_error(error)
         }
 
+        #[cfg(windows)]
+        tunnel::Error::WireguardTunnelMonitoringError(Error::TunnelError(
+            TunnelError::RecoverableStartWireguardError,
+        )) if retry_attempt < MAX_ADAPTER_FAIL_RETRIES => true,
+
+        #[cfg(windows)]
+        tunnel::Error::OpenVpnTunnelMonitoringError(openvpn::Error::WintunCreateAdapterError(
+            _,
+        )) if retry_attempt < MAX_ADAPTER_FAIL_RETRIES => true,
+
         _ => false,
     }
 }
@@ -510,7 +530,7 @@ impl TunnelState for ConnectingState {
                             )
                         }
                         Err(error) => {
-                            if should_retry(&error) {
+                            if should_retry(&error, retry_attempt) {
                                 log::warn!(
                                     "{}",
                                     error.display_chain_with_msg(
@@ -521,7 +541,7 @@ impl TunnelState for ConnectingState {
                                     shared_values,
                                     (
                                         None,
-                                        Self::spawn_tunnel_monitor_wait_thread(None),
+                                        Self::spawn_tunnel_monitor_wait_thread(None, retry_attempt),
                                         AfterDisconnect::Reconnect(retry_attempt + 1),
                                     ),
                                 )
diff --git a/wireguard/libwg/libwg_windows.go b/wireguard/libwg/libwg_windows.go
index 2cf04ed140..2f7dcc7ecc 100644
--- a/wireguard/libwg/libwg_windows.go
+++ b/wireguard/libwg/libwg_windows.go
@@ -72,7 +72,7 @@ func wgTurnOn(cIfaceName *C.char, mtu int, waitOnIpv6 bool, cSettings *C.char, c
 	if err != nil {
 		logger.Errorf("Failed to create tunnel\n")
 		logger.Errorf("%s\n", err)
-		return ERROR_GENERAL_FAILURE
+		return ERROR_INTERMITTENT_FAILURE
 	}
 
 	nativeTun := wintun.(*tun.NativeTun)