diff --git a/vhost-device-can/src/backend.rs b/vhost-device-can/src/backend.rs index 42ce20cd9..01816bda3 100644 --- a/vhost-device-can/src/backend.rs +++ b/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)) ); } } diff --git a/vhost-device-can/src/can.rs b/vhost-device-can/src/can.rs index 47a1fbc76..f3b6d5260 100644 --- a/vhost-device-can/src/can.rs +++ b/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 { + 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, + pub can_socket: CanSocketKind, pub rx_event_fd: EventFd, rx_fifo: Queue, 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 { + 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(frame: F, is_fd: bool) -> VirtioCanFrame { VirtioCanFrame { @@ -159,25 +201,6 @@ impl CanController { } pub fn read_can_socket(controller: Arc>) -> 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); } } diff --git a/vhost-device-can/src/vhu_can.rs b/vhost-device-can/src/vhu_can.rs index 2505700fe..b4079c734 100644 --- a/vhost-device-can/src/vhu_can.rs +++ b/vhost-device-can/src/vhu_can.rs @@ -13,7 +13,7 @@ use std::{ sync::{Arc, RwLock}, }; -use log::{error, trace, warn}; +use log::{trace, warn}; use thiserror::Error as ThisError; use vhost::vhost_user::message::{VhostUserProtocolFeatures, VhostUserVirtioFeatures}; use vhost_user_backend::{VhostUserBackendMut, VringEpollHandler, VringRwLock, VringT}; @@ -279,7 +279,7 @@ impl VhostUserCanBackend { // bigger than 8 bytes is being received we consider it as // CANFD message. let can_name = self.controller.read().unwrap().can_name.clone(); - if self.check_features(VIRTIO_CAN_F_CAN_FD) && res_len > 8 && can_name == "vcan0" { + if self.check_features(VIRTIO_CAN_F_CAN_FD) && res_len > 8 && can_name == "can0" { res_flags |= CAN_FRMF_TYPE_FD; warn!("\n\n\nCANFD VCAN0\n\n"); } @@ -778,7 +778,7 @@ mod tests { #[test] fn test_virtio_can_empty_requests() { let controller = - CanController::new("can0".to_string()).expect("Could not build controller"); + CanController::new_mock("can0".to_string()).expect("Could not build controller"); let controller = Arc::new(RwLock::new(controller)); let mut vu_can_backend = VhostUserCanBackend::new(controller.clone()).expect("Could not build vhucan device"); @@ -802,7 +802,7 @@ mod tests { #[test] fn test_virtio_can_empty_handle_request() { let controller = - CanController::new("can0".to_string()).expect("Could not build controller"); + CanController::new_mock("can0".to_string()).expect("Could not build controller"); let controller = Arc::new(RwLock::new(controller)); let mut vu_can_backend = VhostUserCanBackend::new(controller.clone()).expect("Could not build vhucan device"); @@ -874,7 +874,7 @@ mod tests { fn test_virtio_can_ctrl_request() { // Initialize can device and vhost-device-can backend let controller = - CanController::new("can0".to_string()).expect("Could not build controller"); + CanController::new_mock("can0".to_string()).expect("Could not build controller"); let controller = Arc::new(RwLock::new(controller)); let mut vu_can_backend = VhostUserCanBackend::new(controller.clone()).expect("Could not build vhucan device"); @@ -1101,7 +1101,7 @@ mod tests { #[test] fn test_virtio_can_check_tx_unknown_frame() { let controller = - CanController::new("can0".to_string()).expect("Could not build controller"); + CanController::new_mock("can0".to_string()).expect("Could not build controller"); let controller = Arc::new(RwLock::new(controller)); let vu_can_backend = VhostUserCanBackend::new(controller.clone()).expect("Could not build vhucan device"); @@ -1124,7 +1124,7 @@ mod tests { #[test] fn test_virtio_can_check_tx_can_frame() { let controller = - CanController::new("can0".to_string()).expect("Could not build controller"); + CanController::new_mock("can0".to_string()).expect("Could not build controller"); let controller = Arc::new(RwLock::new(controller)); let mut vu_can_backend = VhostUserCanBackend::new(controller.clone()).expect("Could not build vhucan device"); @@ -1172,7 +1172,7 @@ mod tests { #[test] fn test_virtio_can_check_tx_canfd_frame() { let controller = - CanController::new("can0".to_string()).expect("Could not build controller"); + CanController::new_mock("can0".to_string()).expect("Could not build controller"); let controller = Arc::new(RwLock::new(controller)); let mut vu_can_backend = VhostUserCanBackend::new(controller).expect("Could not build vhucan device"); @@ -1279,7 +1279,7 @@ mod tests { #[test] fn test_virtio_can_check_tx_rtr_frame() { let controller = - CanController::new("can0".to_string()).expect("Could not build controller"); + CanController::new_mock("can0".to_string()).expect("Could not build controller"); let controller = Arc::new(RwLock::new(controller)); let mut vu_can_backend = VhostUserCanBackend::new(controller.clone()).expect("Could not build vhucan device"); @@ -1341,7 +1341,7 @@ mod tests { #[test] fn test_virtio_can_check_tx_eff_frame() { let controller = - CanController::new("can0".to_string()).expect("Could not build controller"); + CanController::new_mock("can0".to_string()).expect("Could not build controller"); let controller = Arc::new(RwLock::new(controller)); let mut vu_can_backend = VhostUserCanBackend::new(controller.clone()).expect("Could not build vhucan device"); @@ -1402,7 +1402,7 @@ mod tests { #[test] fn test_virtio_can_tx_general_tests() { let controller = - CanController::new("can0".to_string()).expect("Could not build controller"); + CanController::new_mock("can0".to_string()).expect("Could not build controller"); let controller = Arc::new(RwLock::new(controller)); let mut vu_can_backend = VhostUserCanBackend::new(controller.clone()).expect("Could not build vhucan device"); @@ -1499,7 +1499,7 @@ mod tests { #[test] fn test_virtio_can_tx_device_stopped_test() { let controller = - CanController::new("can0".to_string()).expect("Could not build controller"); + CanController::new_mock("can0".to_string()).expect("Could not build controller"); let controller = Arc::new(RwLock::new(controller)); let mut vu_can_backend = VhostUserCanBackend::new(controller.clone()).expect("Could not build vhucan device"); @@ -1552,9 +1552,9 @@ mod tests { } #[test] - fn test_virtio_can_tx_device_started_test_send_fail() { + fn test_virtio_can_tx_device_started_test_send_success() { let controller = - CanController::new("can0".to_string()).expect("Could not build controller"); + CanController::new_mock("can0".to_string()).expect("Could not build controller"); let controller = Arc::new(RwLock::new(controller)); let mut vu_can_backend = VhostUserCanBackend::new(controller.clone()).expect("Could not build vhucan device"); @@ -1606,13 +1606,13 @@ mod tests { .map_err(|_| Error::DescriptorReadFailed) .unwrap(); - assert_eq!(VIRTIO_CAN_RESULT_NOT_OK, can_frame_res); + assert_eq!(VIRTIO_CAN_RESULT_OK, can_frame_res); } #[test] fn test_virtio_can_tx_device_started_check_frame_fail() { let controller = - CanController::new("can0".to_string()).expect("Could not build controller"); + CanController::new_mock("can0".to_string()).expect("Could not build controller"); let controller = Arc::new(RwLock::new(controller)); let mut vu_can_backend = VhostUserCanBackend::new(controller.clone()).expect("Could not build vhucan device"); @@ -1667,7 +1667,7 @@ mod tests { #[test] fn test_virtio_can_check_rx_err_frame() { let controller = - CanController::new("can0".to_string()).expect("Could not build controller"); + CanController::new_mock("can0".to_string()).expect("Could not build controller"); let controller = Arc::new(RwLock::new(controller)); let vu_can_backend = VhostUserCanBackend::new(controller.clone()).expect("Could not build vhucan device"); @@ -1710,7 +1710,7 @@ mod tests { #[test] fn test_virtio_can_check_rx_features_not_negotiated() { let controller = - CanController::new("can0".to_string()).expect("Could not build controller"); + CanController::new_mock("can0".to_string()).expect("Could not build controller"); let controller = Arc::new(RwLock::new(controller)); let vu_can_backend = VhostUserCanBackend::new(controller.clone()).expect("Could not build vhucan device"); @@ -1747,7 +1747,7 @@ mod tests { #[test] fn test_virtio_can_check_rx_eff_frame() { let controller = - CanController::new("can0".to_string()).expect("Could not build controller"); + CanController::new_mock("can0".to_string()).expect("Could not build controller"); let controller = Arc::new(RwLock::new(controller)); let mut vu_can_backend = VhostUserCanBackend::new(controller.clone()).expect("Could not build vhucan device"); @@ -1815,7 +1815,7 @@ mod tests { #[test] fn test_virtio_can_check_rx_rtr_frame() { let controller = - CanController::new("can0".to_string()).expect("Could not build controller"); + CanController::new_mock("can0".to_string()).expect("Could not build controller"); let controller = Arc::new(RwLock::new(controller)); let mut vu_can_backend = VhostUserCanBackend::new(controller.clone()).expect("Could not build vhucan device"); @@ -1879,7 +1879,7 @@ mod tests { #[test] fn test_virtio_can_check_rx_can_frame() { let controller = - CanController::new("can0".to_string()).expect("Could not build controller"); + CanController::new_mock("can0".to_string()).expect("Could not build controller"); let controller = Arc::new(RwLock::new(controller)); let mut vu_can_backend = VhostUserCanBackend::new(controller.clone()).expect("Could not build vhucan device"); @@ -1926,7 +1926,7 @@ mod tests { #[test] fn test_virtio_can_check_rx_canfd_frame() { let controller = - CanController::new("can0".to_string()).expect("Could not build controller"); + CanController::new_mock("can0".to_string()).expect("Could not build controller"); let controller = Arc::new(RwLock::new(controller)); let mut vu_can_backend = VhostUserCanBackend::new(controller.clone()).expect("Could not build vhucan device"); @@ -2009,9 +2009,9 @@ mod tests { } #[test] - fn test_virtio_can_check_rx_canfd_vcan0() { + fn test_virtio_can_check_rx_canfd_can0() { let controller = - CanController::new("vcan0".to_string()).expect("Could not build controller"); + CanController::new_mock("can0".to_string()).expect("Could not build controller"); let controller = Arc::new(RwLock::new(controller)); let mut vu_can_backend = VhostUserCanBackend::new(controller.clone()).expect("Could not build vhucan device"); @@ -2020,7 +2020,7 @@ mod tests { vu_can_backend.acked_features((1 << VIRTIO_CAN_F_CAN_FD) | (1 << VIRTIO_CAN_F_CAN_CLASSIC)); // If VIRTIO_CAN_F_CAN_FD and VIRTIO_CAN_F_CAN_CLASSIC are negotiated - // and interface is "vcan0" check if return message has + // and interface is "can0" check if return message has // VIRTIO_CAN_FLAGS_FD in flags and has been treated as CANFD frame. let frame = VirtioCanFrame { msg_type: 0.into(), @@ -2050,7 +2050,7 @@ mod tests { #[test] fn test_virtio_can_rx_request() { let controller = - CanController::new("can0".to_string()).expect("Could not build controller"); + CanController::new_mock("can0".to_string()).expect("Could not build controller"); let controller = Arc::new(RwLock::new(controller)); let mut vu_can_backend = VhostUserCanBackend::new(controller.clone()).expect("Could not build vhucan device");