Add feature config to critical uapi error handler

Added two new fields to FeatureWireguard to control
the critical error handler. Default values are present
that should be safe enough but can be overriden if
needed.

These changes do not break the API.

Additionally integration tests are added and some
renaming.

Signed-off-by: Lukas Pukenis <lukas.pukenis@nordsec.com>

Author: LukasPukenis

.unreleased/LLT-6859

@@ -0,0 +1 @@
+FeatureConfig allows configuring critical UAPI error handler parameters

.unreleased/restore_interface_gone_check_in_uapi_error_handler

@@ -22,7 +22,7 @@ macro_rules! report_event {
 }
 
 /// Error levels. Used for app to decide what to do with `telio` device when error happens.
-#[derive(Clone, Debug, Default, Serialize)]
+#[derive(Clone, Debug, Default, Serialize, Eq, PartialEq)]
 #[serde(rename_all = "lowercase")]
 pub enum ErrorLevel {
     /// The error level is critical (highest priority)
@@ -37,7 +37,7 @@ pub enum ErrorLevel {
 }
 
 /// Error code. Common error code representation (for statistics).
-#[derive(Clone, Debug, Default, Serialize)]
+#[derive(Clone, Debug, Default, Serialize, Eq, PartialEq)]
 #[serde(rename_all = "lowercase")]
 pub enum ErrorCode {
     /// There is no error in the execution

crates/telio-model/src/event.rs

@@ -93,7 +93,7 @@ pub struct FeatureBatching {
 }
 
 /// Configurable features for Wireguard peers
-#[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)]
+#[derive(Clone, Debug, SmartDefault, PartialEq, Eq, Serialize, Deserialize)]
 #[cfg_attr(test, derive(proptest_derive::Arbitrary))]
 pub struct FeatureWireguard {
     /// Configurable persistent keepalive periods for wireguard peers
@@ -114,6 +114,12 @@ pub struct FeatureWireguard {
     /// Configurable socket buffer size for NepTUN
     #[serde(default)]
     pub max_inter_thread_batched_pkts: Option<u32>,
+    /// Configure time needed to emit critical error when adapter is gone
+    #[default(15)]
+    pub adapter_gone_error_threshold_secs: u32,
+    /// /// Configure the expected time between uapi calls to consider critical error
+    #[default(30)]
+    pub adapter_gone_max_uapi_inerval_secs: u32,
 }
 
 impl FeatureWireguard {
@@ -701,7 +707,9 @@ mod tests {
                 "enable_dynamic_wg_nt_control": true,
                 "skt_buffer_size": 123456,
                 "inter_thread_channel_size": 123456,
-                "max_inter_thread_batched_pkts": 123456
+                "max_inter_thread_batched_pkts": 123456,
+                "adapter_gone_error_threshold_secs": 123,
+                "adapter_gone_max_uapi_inerval_secs": 456
             },
             "nurse": {
                 "fingerprint": "test_fingerprint",
@@ -805,6 +813,8 @@ mod tests {
                         skt_buffer_size: Some(123456),
                         inter_thread_channel_size: Some(123456),
                         max_inter_thread_batched_pkts: Some(123456),
+                        adapter_gone_error_threshold_secs: 123,
+                        adapter_gone_max_uapi_inerval_secs: 456
                     },
                     nurse: Some(FeatureNurse {
                         heartbeat_interval: 5,

crates/telio-model/src/features.rs

@@ -125,12 +125,6 @@ pub struct Io {
 pub const KEEPALIVE_PACKET_SIZE: u64 = 32;
 /// Default wireguard keepalive duration
 pub const WG_KEEPALIVE: Duration = Duration::from_secs(10);
-/// Used to ensure uapi errors happen for at least this long before considering critical error event.
-/// The failures are usually transient and observed during power transitions when using WireguardNT.
-const REQUIRED_CRITICAL_ERROR_GAP: Duration = Duration::from_secs(15);
-/// Used to detect long gaps between UAPI commands that suggest the device might have been suspended.
-/// Usually libtelio should poll every second.
-const MAX_UAPI_CMD_GAP: Duration = Duration::from_secs(30);
 
 pub use telio_utils::{BytesAndTimestamps, Instant};
 
@@ -139,12 +133,12 @@ pub use telio_utils::{BytesAndTimestamps, Instant};
 #[derive(Debug)]
 struct UAPIErrorTracker {
     // Minimum required time between first error and later ones to consider returning critical verdict.
-    required_critical_error_gap: Duration,
+    adapter_gone_error_threshold: Duration,
 
     // Maximum time allowed between uapi commands.
-    max_uapi_cmd_gap: Duration,
+    adapter_gone_max_uapi_inerval: Duration,
 
-    // Timestamp of the first error from which the required_critical_error_gap is measured.
+    // Timestamp of the first error from which the adapter_gone_error_threshold is measured.
     first_error_at: Option<Instant>,
 
     // Timestamp of the most recent UAPI command.
@@ -160,10 +154,13 @@ enum UAPIErrorTrackerVerdict {
 }
 
 impl UAPIErrorTracker {
-    fn new(required_critical_error_gap: Duration, max_uapi_cmd_gap: Duration) -> Self {
+    fn new(
+        adapter_gone_error_threshold: Duration,
+        adapter_gone_max_uapi_inerval: Duration,
+    ) -> Self {
         UAPIErrorTracker {
-            required_critical_error_gap,
-            max_uapi_cmd_gap,
+            adapter_gone_error_threshold,
+            adapter_gone_max_uapi_inerval,
             first_error_at: None,
             last_uapi_cmd_at: None,
         }
@@ -177,16 +174,16 @@ impl UAPIErrorTracker {
             return UAPIErrorTrackerVerdict::OK;
         }
 
-        let uapi_within_allowed_window =
-            prev_uapi_cmd_time.is_none_or(|t| inst.duration_since(t) < self.max_uapi_cmd_gap);
+        let uapi_within_allowed_window = prev_uapi_cmd_time
+            .is_none_or(|t| inst.duration_since(t) < self.adapter_gone_max_uapi_inerval);
 
         if !uapi_within_allowed_window {
             self.first_error_at = Some(inst);
         }
 
         match self.first_error_at {
             Some(t) => {
-                if inst.duration_since(t) >= self.required_critical_error_gap
+                if inst.duration_since(t) >= self.adapter_gone_error_threshold
                     && resp.interface.is_none()
                 {
                     self.first_error_at = None;
@@ -298,7 +295,9 @@ impl DynamicWg {
     ///         },
     ///         None,
     ///         Duration::from_millis(1000),
-    ///         Duration::from_millis(50)
+    ///         Duration::from_millis(50),
+    ///         Duration::from_secs(15),
+    ///         Duration::from_secs(30),
     ///     );
     /// }
     /// ```
@@ -308,6 +307,8 @@ impl DynamicWg {
         link_detection: Option<LinkDetection>,
         polling_period: Duration,
         polling_period_after_update: Duration,
+        adapter_gone_error_duration: Duration,
+        adapter_gone_error_max_uapi_interval: Duration,
     ) -> Result<Self, Error>
     where
         Self: Sized,
@@ -321,6 +322,8 @@ impl DynamicWg {
             cfg,
             polling_period,
             polling_period_after_update,
+            adapter_gone_error_duration,
+            adapter_gone_error_max_uapi_interval,
         ));
         #[cfg(windows)]
         return Ok(Self::start_with(
@@ -329,16 +332,21 @@ impl DynamicWg {
             link_detection,
             polling_period,
             polling_period_after_update,
+            adapter_gone_error_duration,
+            adapter_gone_error_max_uapi_interval,
         ));
     }
 
+    #[allow(clippy::too_many_arguments)]
     fn start_with(
         io: Io,
         adapter: Box<dyn Adapter>,
         link_detection: Option<LinkDetection>,
         #[cfg(unix)] cfg: Config,
         polling_period: Duration,
         polling_period_after_update: Duration,
+        adapter_gone_error_duration: Duration,
+        adapter_gone_error_max_uapi_interval: Duration,
     ) -> Self {
         let interval = interval(polling_period);
         Self {
@@ -351,8 +359,8 @@ impl DynamicWg {
                 event: io.events,
                 analytics_tx: io.analytics_tx,
                 uapi_error_tracker: UAPIErrorTracker::new(
-                    REQUIRED_CRITICAL_ERROR_GAP,
-                    MAX_UAPI_CMD_GAP,
+                    adapter_gone_error_duration,
+                    adapter_gone_error_max_uapi_interval,
                 ),
                 link_detection,
                 libtelio_event: io.libtelio_wide_event_publisher,
@@ -1095,6 +1103,8 @@ pub mod tests {
 
     const DEFAULT_POLLING_PERIOD_MS: u64 = 1000;
     const DEFAULT_POLLING_PERIOD_AFTER_UPDATE_MS: u64 = 50;
+    const ADAPTER_GONE_ERROR_THRESHOLD_S: u64 = 15;
+    const ADAPTER_GONE_MAX_UAPI_INERVAL_S: u64 = 30;
 
     fn random_interface() -> Interface {
         let mut rng = rand::thread_rng();
@@ -1253,6 +1263,8 @@ pub mod tests {
             cfg,
             Duration::from_millis(DEFAULT_POLLING_PERIOD_MS),
             Duration::from_millis(DEFAULT_POLLING_PERIOD_AFTER_UPDATE_MS),
+            Duration::from_secs(ADAPTER_GONE_ERROR_THRESHOLD_S),
+            Duration::from_secs(ADAPTER_GONE_MAX_UAPI_INERVAL_S),
         );
         time::advance(Duration::from_millis(0)).await;
         adapter.lock().await.checkpoint();
@@ -1276,6 +1288,138 @@ pub mod tests {
         wg.stop().await;
     }
 
+    #[tokio::test(start_paused = true)]
+    async fn wg_test_uapi_critical_error() {
+        struct ErrorReturningAdapter {}
+
+        #[async_trait]
+        impl Adapter for ErrorReturningAdapter {
+            async fn send_uapi_cmd(&self, _cmd: &Cmd) -> Result<Response, AdapterError> {
+                return Ok(Response {
+                    errno: 2,
+                    interface: None,
+                });
+            }
+
+            async fn stop(&self) {}
+            fn get_adapter_luid(&self) -> u64 {
+                0
+            }
+            async fn set_tun(&self, _tun: Tun) -> Result<(), Error> {
+                Ok(())
+            }
+            fn clone_box(&self) -> Option<Box<dyn Adapter>> {
+                None
+            }
+        }
+
+        let events_ch = Chan::default();
+        let (event_tx, mut event_rx) = tokio::sync::broadcast::channel(256);
+
+        let analytics_ch = Some(McChan::default().tx);
+        let wg = DynamicWg::start_with(
+            Io {
+                events: events_ch.tx,
+                analytics_tx: analytics_ch.clone(),
+                libtelio_wide_event_publisher: Some(event_tx),
+            },
+            Box::new(ErrorReturningAdapter {}),
+            None,
+            #[cfg(all(unix, test))]
+            Config::new().unwrap(),
+            #[cfg(all(unix, not(test)))]
+            cfg,
+            Duration::from_millis(DEFAULT_POLLING_PERIOD_MS),
+            Duration::from_millis(DEFAULT_POLLING_PERIOD_AFTER_UPDATE_MS),
+            Duration::from_secs(ADAPTER_GONE_ERROR_THRESHOLD_S),
+            Duration::from_secs(ADAPTER_GONE_MAX_UAPI_INERVAL_S),
+        );
+
+        // Now lets advance time and eventually expect critical error since adapter is gone
+        for _ in 0..ADAPTER_GONE_ERROR_THRESHOLD_S {
+            tokio::time::advance(Duration::from_secs(1)).await;
+            tokio::task::yield_now().await;
+        }
+
+        assert_eq!(event_rx.len(), 0);
+
+        tokio::time::advance(Duration::from_secs(1)).await;
+        tokio::task::yield_now().await;
+
+        assert!(matches!(
+            *event_rx.try_recv().unwrap(),
+            telio_model::event::Event::Error { body }
+                if body.code == ErrorCode::Unknown
+                && body.level == ErrorLevel::Critical
+                && body.msg == "Interface gone"
+        ));
+
+        wg.stop().await;
+    }
+
+    #[tokio::test(start_paused = true)]
+    async fn wg_test_uapi_no_critical_error() {
+        struct ErrorReturningAdapter {}
+
+        #[async_trait]
+        impl Adapter for ErrorReturningAdapter {
+            async fn send_uapi_cmd(&self, _cmd: &Cmd) -> Result<Response, AdapterError> {
+                return Ok(Response {
+                    errno: 2,
+                    interface: Some(Interface::default()),
+                });
+            }
+
+            async fn stop(&self) {}
+            fn get_adapter_luid(&self) -> u64 {
+                0
+            }
+            async fn set_tun(&self, _tun: Tun) -> Result<(), Error> {
+                Ok(())
+            }
+            fn clone_box(&self) -> Option<Box<dyn Adapter>> {
+                None
+            }
+        }
+
+        let events_ch = Chan::default();
+        let (event_tx, event_rx) = tokio::sync::broadcast::channel(256);
+
+        let analytics_ch = Some(McChan::default().tx);
+        let wg = DynamicWg::start_with(
+            Io {
+                events: events_ch.tx,
+                analytics_tx: analytics_ch.clone(),
+                libtelio_wide_event_publisher: Some(event_tx),
+            },
+            Box::new(ErrorReturningAdapter {}),
+            None,
+            #[cfg(all(unix, test))]
+            Config::new().unwrap(),
+            #[cfg(all(unix, not(test)))]
+            cfg,
+            Duration::from_millis(DEFAULT_POLLING_PERIOD_MS),
+            Duration::from_millis(DEFAULT_POLLING_PERIOD_AFTER_UPDATE_MS),
+            Duration::from_secs(ADAPTER_GONE_ERROR_THRESHOLD_S),
+            Duration::from_secs(ADAPTER_GONE_MAX_UAPI_INERVAL_S),
+        );
+
+        // Now lets advance time and since adapter is present, no critical event should be there
+        for _ in 0..ADAPTER_GONE_ERROR_THRESHOLD_S {
+            tokio::time::advance(Duration::from_secs(1)).await;
+            tokio::task::yield_now().await;
+        }
+
+        assert_eq!(event_rx.len(), 0);
+
+        tokio::time::advance(Duration::from_secs(1)).await;
+        tokio::task::yield_now().await;
+
+        assert_eq!(event_rx.len(), 0);
+
+        wg.stop().await;
+    }
+
     /// See RFC LLT-0083 for more details on how and why polling speed up was added.
     #[tokio::test(start_paused = true)]
     async fn faster_polling() {
@@ -2079,7 +2223,12 @@ pub mod tests {
             interface: Some(Interface::default()),
         };
 
-        let mut tracker = UAPIErrorTracker::new(REQUIRED_CRITICAL_ERROR_GAP, MAX_UAPI_CMD_GAP);
+        use super::*;
+
+        let mut tracker = UAPIErrorTracker::new(
+            Duration::from_secs(ADAPTER_GONE_ERROR_THRESHOLD_S),
+            Duration::from_secs(ADAPTER_GONE_MAX_UAPI_INERVAL_S),
+        );
         let mut now = Instant::now();
 
         // Initially, no error: should Ignore
@@ -2092,12 +2241,12 @@ pub mod tests {
         assert_eq!(verdict, UAPIErrorTrackerVerdict::OK);
 
         // After threshold duration, should Ignore (gap too big between uapi calls)
-        now += MAX_UAPI_CMD_GAP;
+        now += Duration::from_secs(ADAPTER_GONE_MAX_UAPI_INERVAL_S);
         let verdict = tracker.update(&err_resp, now);
         assert_eq!(verdict, UAPIErrorTrackerVerdict::OK);
 
         // Trigger errors every second, until just before the threshold, should Ignore
-        for _ in 1..REQUIRED_CRITICAL_ERROR_GAP.as_secs() {
+        for _ in 1..ADAPTER_GONE_ERROR_THRESHOLD_S {
             now += Duration::from_secs(1);
             let verdict = tracker.update(&err_resp, now);
             assert_eq!(verdict, UAPIErrorTrackerVerdict::OK);
@@ -2113,7 +2262,7 @@ pub mod tests {
         assert_eq!(verdict, UAPIErrorTrackerVerdict::OK);
 
         // Trigger errors every second, until just before the threshold, should Ignore
-        for _ in 1..REQUIRED_CRITICAL_ERROR_GAP.as_secs() {
+        for _ in 1..ADAPTER_GONE_ERROR_THRESHOLD_S {
             now += Duration::from_secs(1);
             let verdict = tracker.update(&err_resp, now);
             assert_eq!(verdict, UAPIErrorTrackerVerdict::OK);
@@ -2131,7 +2280,7 @@ pub mod tests {
 
         // Trigger errors repeatedly with big time gaps in between, should be Ignore
         for _ in 0..50 {
-            now += MAX_UAPI_CMD_GAP;
+            now += Duration::from_secs(ADAPTER_GONE_MAX_UAPI_INERVAL_S);
             let verdict = tracker.update(&err_resp, now);
             assert_eq!(verdict, UAPIErrorTrackerVerdict::OK);
         }