vhost-device-can: use the same socket for send/recv

vhost-device-can/src/backend.rs

@@ -13,7 +13,7 @@ use std::{
     thread,
 };
 
-use log::{error, info, warn};
+use log::{info, warn};
 use thiserror::Error as ThisError;
 use vhost_user_backend::VhostUserDaemon;
 use vm_memory::{GuestMemoryAtomic, GuestMemoryMmap};
@@ -31,8 +31,6 @@ pub(crate) enum Error {
     CouldNotFindCANDevs,
     #[error("Could not create can controller: {0}")]
     CouldNotCreateCanController(crate::can::Error),
-    #[error("Could not create can controller output socket: {0}")]
-    FailCreateCanControllerSocket(crate::can::Error),
     #[error("Could not create can backend: {0}")]
     CouldNotCreateBackend(crate::vhu_can::Error),
     #[error("Could not create daemon: {0}")]
@@ -82,11 +80,6 @@ pub(crate) fn start_backend_server(socket: PathBuf, can_devs: String) -> Result<
             VhostUserCanBackend::new(lockable_controller.clone())
                 .map_err(Error::CouldNotCreateBackend)?,
         ));
-        lockable_controller
-            .write()
-            .unwrap()
-            .open_can_socket()
-            .map_err(Error::FailCreateCanControllerSocket)?;
 
         let read_handle = CanController::start_read_thread(lockable_controller.clone());
 
@@ -165,13 +158,13 @@ mod tests {
     use assert_matches::assert_matches;
 
     use super::*;
-    use crate::{backend::Error::FailCreateCanControllerSocket, can::Error::SocketOpen, CanArgs};
+    use crate::{backend::Error::CouldNotCreateCanController, can::Error::SocketOpen, CanArgs};
 
     #[test]
     fn test_can_valid_configuration() {
         let valid_args = CanArgs {
             socket_path: "/tmp/vhost.sock".to_string().into(),
-            can_devices: "can0".to_string(),
+            can_devices: "canx".to_string(),
             socket_count: 1,
         };
 
@@ -243,23 +236,23 @@ mod tests {
         let config = VuCanConfig {
             socket_path: PathBuf::from("/tmp/vhost.sock"),
             socket_count: 1,
-            can_devices: vec!["can0".to_string()],
+            can_devices: vec!["canx".to_string()],
         };
 
         assert_matches!(
             start_backend(config),
-            Err(FailCreateCanControllerSocket(SocketOpen))
+            Err(CouldNotCreateCanController(SocketOpen))
         );
     }
 
     #[test]
     fn test_can_valid_configuration_start_backend_server_fail() {
         let socket_path = PathBuf::from("/tmp/vhost.sock");
-        let can_devs = "can0".to_string();
+        let can_devs = "canx".to_string();
 
         assert_matches!(
             start_backend_server(socket_path, can_devs),
-            Err(FailCreateCanControllerSocket(SocketOpen))
+            Err(CouldNotCreateCanController(SocketOpen))
         );
     }
 }

vhost-device-can/src/can.rs

@@ -10,7 +10,7 @@ use std::{
     thread::{spawn, JoinHandle},
 };
 
-use log::{error, info, trace, warn};
+use log::{info, trace, warn};
 use thiserror::Error as ThisError;
 use vmm_sys_util::eventfd::{EventFd, EFD_NONBLOCK};
 extern crate queues;
@@ -45,15 +45,40 @@ pub(crate) enum Error {
     EventFdFailed,
     #[error("Push can element operation failed")]
     PushFailed,
-    #[error("No output interface available")]
-    NoOutputInterface,
+    #[error("Can socket configuration failed")]
+    SocketConfig,
+}
+
+#[derive(Debug)]
+pub(crate) enum CanSocketKind {
+    Fd(CanFdSocket),
+    #[cfg(test)]
+    Mock,
+}
+
+impl CanSocketKind {
+    pub fn write_frame(&self, frame: &CanAnyFrame) -> std::io::Result<()> {
+        match self {
+            CanSocketKind::Fd(socket) => socket.write_frame(frame),
+            #[cfg(test)]
+            CanSocketKind::Mock => Ok(()),
+        }
+    }
+
+    pub fn read_frame(&self) -> std::io::Result<CanAnyFrame> {
+        match self {
+            CanSocketKind::Fd(socket) => socket.read_frame(),
+            #[cfg(test)]
+            CanSocketKind::Mock => Err(std::io::ErrorKind::WouldBlock.into()),
+        }
+    }
 }
 
 #[derive(Debug)]
 pub(crate) struct CanController {
     pub config: VirtioCanConfig,
     pub can_name: String,
-    pub can_socket: Option<CanFdSocket>,
+    pub can_socket: CanSocketKind,
     pub rx_event_fd: EventFd,
     rx_fifo: Queue<VirtioCanFrame>,
     pub status: bool,
@@ -69,10 +94,38 @@ impl CanController {
         let rx_fifo = Queue::new();
         let rx_efd = EventFd::new(EFD_NONBLOCK).map_err(|_| Error::EventFdFailed)?;
 
+        let socket: CanFdSocket = match CanFdSocket::open(can_name.as_str()) {
+            Ok(socket) => socket,
+            Err(_) => {
+                warn!("Error opening CAN socket");
+                return Err(Error::SocketOpen);
+            }
+        };
+
+        socket
+            .set_nonblocking(true)
+            .map_err(|_| Error::SocketConfig)?;
+
         Ok(CanController {
             config: VirtioCanConfig { status: 0x0.into() },
             can_name,
-            can_socket: None,
+            can_socket: CanSocketKind::Fd(socket),
+            rx_event_fd: rx_efd,
+            rx_fifo,
+            status: true,
+            ctrl_state: CAN_CS_STOPPED,
+        })
+    }
+
+    #[cfg(test)]
+    pub(crate) fn new_mock(can_name: String) -> Result<CanController> {
+        let rx_fifo = Queue::new();
+        let rx_efd = EventFd::new(EFD_NONBLOCK).map_err(|_| Error::EventFdFailed)?;
+
+        Ok(CanController {
+            config: VirtioCanConfig { status: 0x0.into() },
+            can_name,
+            can_socket: CanSocketKind::Mock,
             rx_event_fd: rx_efd,
             rx_fifo,
             status: true,
@@ -127,17 +180,6 @@ impl CanController {
         }
     }
 
-    pub fn open_can_socket(&mut self) -> Result<()> {
-        self.can_socket = match CanFdSocket::open(&self.can_name) {
-            Ok(socket) => Some(socket),
-            Err(_) => {
-                warn!("Error opening CAN socket");
-                return Err(Error::SocketOpen);
-            }
-        };
-        Ok(())
-    }
-
     // Helper function to process frame
     fn process_frame<F: Frame>(frame: F, is_fd: bool) -> VirtioCanFrame {
         VirtioCanFrame {
@@ -159,25 +201,6 @@ impl CanController {
     }
 
     pub fn read_can_socket(controller: Arc<RwLock<CanController>>) -> Result<()> {
-        let can_name = &controller.read().unwrap().can_name.clone();
-        log::debug!("Start reading from {can_name} socket!");
-        let socket = match CanFdSocket::open(can_name) {
-            Ok(socket) => socket,
-            Err(_) => {
-                warn!("Error opening CAN socket");
-                return Err(Error::SocketOpen);
-            }
-        };
-
-        // Set non-blocking otherwise the device will not restart immediately
-        // when the VM closes, and a new canfd message needs to be received for
-        // restart to happen.
-        // This caused by the fact that the thread is stacked in read function
-        // and does not go to the next loop to check the status condition.
-        socket
-            .set_nonblocking(true)
-            .expect("Cannot set nonblocking");
-
         // Receive CAN messages
         loop {
             // If the status variable is false then break and exit.
@@ -186,7 +209,12 @@ impl CanController {
                 return Ok(());
             }
 
-            if let Ok(frame) = socket.read_frame() {
+            let frame_result = {
+                let ctrl = controller.read().unwrap();
+                ctrl.can_socket.read_frame()
+            };
+
+            if let Ok(frame) = frame_result {
                 // If ctrl_state is stopped, consume the received CAN/FD frame
                 // and loop till the ctrl_state changes to started or the thread
                 // to exit.
@@ -272,17 +300,12 @@ impl CanController {
         };
 
         // Send the CAN/FD frame
-        let socket = self.can_socket.as_ref().ok_or("No available device");
-
-        match socket {
-            Ok(socket) => match socket.write_frame(&frame) {
-                Ok(_) => Ok(()),
-                Err(_) => {
-                    warn!("Error write CAN socket");
-                    Err(Error::SocketWrite)
-                }
-            },
-            Err(_) => Err(Error::NoOutputInterface),
+        match self.can_socket.write_frame(&frame) {
+            Ok(_) => Ok(()),
+            Err(_) => {
+                warn!("Error write CAN socket");
+                Err(Error::SocketWrite)
+            }
         }
     }
 }
@@ -292,20 +315,19 @@ mod tests {
     use std::sync::{Arc, RwLock};
 
     use super::*;
-    use crate::vhu_can::VhostUserCanBackend;
 
     #[test]
     fn test_can_controller_creation() {
-        let can_name = "can".to_string();
+        let can_name = "can0".to_string();
 
-        let controller = CanController::new(can_name.clone()).unwrap();
+        let controller = CanController::new_mock(can_name.clone()).unwrap();
         assert_eq!(controller.can_name, can_name);
     }
 
     #[test]
     fn test_can_controller_push_and_pop() {
-        let can_name = "can".to_string();
-        let mut controller = CanController::new(can_name.clone()).unwrap();
+        let can_name = "can0".to_string();
+        let mut controller = CanController::new_mock(can_name.clone()).unwrap();
 
         let frame = VirtioCanFrame {
             msg_type: VIRTIO_CAN_RX.into(),
@@ -326,8 +348,8 @@ mod tests {
 
     #[test]
     fn test_can_controller_config() {
-        let can_name = "can".to_string();
-        let mut controller = CanController::new(can_name.clone()).unwrap();
+        let can_name = "can0".to_string();
+        let mut controller = CanController::new_mock(can_name.clone()).unwrap();
 
         // Test config
         let config = controller.config();
@@ -336,62 +358,40 @@ mod tests {
 
     #[test]
     fn test_can_controller_operation() {
-        let can_name = "can".to_string();
-        let mut controller = CanController::new(can_name.clone()).unwrap();
+        let can_name = "can0".to_string();
+        let controller = CanController::new_mock(can_name.clone()).unwrap();
 
         let frame = VirtioCanFrame {
             msg_type: VIRTIO_CAN_RX.into(),
             can_id: 123.into(),
             length: 64.into(),
             reserved: 0.into(),
-            flags: 0.into(),
+            flags: CAN_FRMF_TYPE_FD.into(),
             sdu: [0; 64],
         };
 
-        match controller.open_can_socket() {
-            Ok(_) => {
-                // Test operation
-                let operation_result = controller.can_out(frame);
-                assert!(operation_result.is_ok());
-            }
-            Err(_) => warn!("There is no CAN interface with {can_name} name"),
-        }
+        // Test operation
+        let operation_result = controller.can_out(frame);
+        assert!(operation_result.is_ok());
     }
 
     #[test]
     fn test_can_controller_start_read_thread() {
-        let can_name = "can".to_string();
-        let controller = CanController::new(can_name.clone()).unwrap();
+        let can_name = "can0".to_string();
+        let controller = CanController::new_mock(can_name.clone()).unwrap();
         let arc_controller = Arc::new(RwLock::new(controller));
 
         // Test start_read_thread
         let thread_handle = CanController::start_read_thread(arc_controller.clone());
+
+        // Signal the thread to exit and wait for it
+        arc_controller.write().unwrap().exit_read_thread();
         assert!(thread_handle.join().is_ok());
     }
 
     #[test]
     fn test_can_open_socket_fail() {
-        let controller =
-            CanController::new("can0".to_string()).expect("Could not build controller");
-        let controller = Arc::new(RwLock::new(controller));
-        VhostUserCanBackend::new(controller.clone()).expect("Could not build vhucan device");
-
-        assert_eq!(
-            controller.write().unwrap().open_can_socket(),
-            Err(Error::SocketOpen)
-        );
-    }
-
-    #[test]
-    fn test_can_read_socket_fail() {
-        let controller =
-            CanController::new("can0".to_string()).expect("Could not build controller");
-        let controller = Arc::new(RwLock::new(controller));
-        VhostUserCanBackend::new(controller.clone()).expect("Could not build vhucan device");
-
-        assert_eq!(
-            CanController::read_can_socket(controller),
-            Err(Error::SocketOpen)
-        );
+        let err = CanController::new("canx".to_string()).unwrap_err();
+        assert_eq!(err, Error::SocketOpen);
     }
 }