Skip to content

Fix tests in vhost-can-device #957

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
MatiasVara wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Fix tests in vhost-can-device #957

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1416ecf
Fix test_can_controller_operation test
MatiasVara Mar 3, 2026
e17c119
Fix test_can_controller_start_read_thread()
MatiasVara Apr 23, 2026
bd317e3
Add mock interface for tests
MatiasVara Apr 23, 2026
8a2257e
remove checking interface's name in check_rx_frame
MatiasVara Apr 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 57 additions & 19 deletions vhost-device-can/src/can.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,28 @@ pub(crate) enum Error {
NoOutputInterface,
}

#[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(()),
}
}
}

#[derive(Debug)]
pub(crate) struct CanController {
pub config: VirtioCanConfig,
pub can_name: String,
pub can_socket: Option<CanFdSocket>,
pub can_socket: Option<CanSocketKind>,
pub rx_event_fd: EventFd,
rx_fifo: Queue<VirtioCanFrame>,
pub status: bool,
Expand All @@ -80,6 +97,30 @@ impl CanController {
})
}

#[cfg(test)]
pub(crate) fn new_mock() -> 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: "vcan0".to_string(),
can_socket: Some(CanSocketKind::Mock),
rx_event_fd: rx_efd,
rx_fifo,
status: true,
ctrl_state: CAN_CS_STOPPED,
})
}

#[cfg(test)]
pub(crate) fn new_test() -> Result<CanController> {
if let Ok(iface) = std::env::var("TEST_CAN_INTERFACE") {
return CanController::new(iface);
}
CanController::new_mock()
}

pub fn print_can_frame(canframe: VirtioCanFrame) {
trace!("canframe.msg_type 0x{:x}", canframe.msg_type.to_native());
trace!("canframe.can_id 0x{:x}", canframe.can_id.to_native());
Expand Down Expand Up @@ -129,7 +170,7 @@ impl CanController {

pub fn open_can_socket(&mut self) -> Result<()> {
self.can_socket = match CanFdSocket::open(&self.can_name) {
Ok(socket) => Some(socket),
Ok(socket) => Some(CanSocketKind::Fd(socket)),
Err(_) => {
warn!("Error opening CAN socket");
return Err(Error::SocketOpen);
Expand Down Expand Up @@ -293,19 +334,17 @@ mod tests {

use super::*;
use crate::vhu_can::VhostUserCanBackend;
use crate::virtio_can::VIRTIO_CAN_TX;

#[test]
fn test_can_controller_creation() {
let can_name = "can".to_string();

let controller = CanController::new(can_name.clone()).unwrap();
assert_eq!(controller.can_name, can_name);
let controller = CanController::new_test().unwrap();
assert!(!controller.can_name.is_empty());
}

#[test]
fn test_can_controller_push_and_pop() {
let can_name = "can".to_string();
let mut controller = CanController::new(can_name.clone()).unwrap();
let mut controller = CanController::new_test().unwrap();

let frame = VirtioCanFrame {
msg_type: VIRTIO_CAN_RX.into(),
Expand All @@ -326,8 +365,7 @@ mod tests {

#[test]
fn test_can_controller_config() {
let can_name = "can".to_string();
let mut controller = CanController::new(can_name.clone()).unwrap();
let mut controller = CanController::new_test().unwrap();

// Test config
let config = controller.config();
Expand All @@ -336,15 +374,14 @@ mod tests {

#[test]
fn test_can_controller_operation() {
let can_name = "can".to_string();
let mut controller = CanController::new(can_name.clone()).unwrap();
let mut controller = CanController::new_test().unwrap();

let frame = VirtioCanFrame {
msg_type: VIRTIO_CAN_RX.into(),
msg_type: VIRTIO_CAN_TX.into(),
can_id: 123.into(),
length: 64.into(),
reserved: 0.into(),
flags: 0.into(),
flags: CAN_FRMF_TYPE_FD.into(),

@epilys epilys Apr 22, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be VIRTIO_CAN_FLAGS_FD?

@MatiasVara MatiasVara Apr 22, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case, the frame is sent directly to the host backend through can_out(), which expect CAN_FRMF_TYPE_FD. It seems that when we get something from the guest in the tx queue, the flag could be VIRTIO_CAN_FLAGS_FD but then it is processed by check_tx_frame() that convert it to CAN_FRMF_TYPE_FD. This test would sit after check_tx_frame(). I think, to use VIRTIO_CAN_FLAGS_FD, we would need to add check_tx_frame() to the test.

@epilys epilys Apr 22, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, makes sense. This is not ideal modelling but this isn't this patch's fault. Is it easy to add check_tx_frame() here so that we are consistent?

@MatiasVara MatiasVara Apr 22, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will try it and let you know.

@MatiasVara MatiasVara Apr 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check_tx_frame() is private in vhu_can.rs. I think if I want to use it in can.rs I would need to make it public. The other option is to move the test to vhu_can.rs and use check_tx_frame() with VIRTIO_CAN_FLAGS_FD.

sdu: [0; 64],
};

Expand All @@ -354,25 +391,26 @@ mod tests {
let operation_result = controller.can_out(frame);
assert!(operation_result.is_ok());
}
Err(_) => warn!("There is no CAN interface with {can_name} name"),
Err(_) => warn!("There is no CAN interface"),
}
}

#[test]
fn test_can_controller_start_read_thread() {
let can_name = "can".to_string();
let controller = CanController::new(can_name.clone()).unwrap();
let controller = CanController::new_test().unwrap();
let arc_controller = Arc::new(RwLock::new(controller));

// Test start_read_thread
let thread_handle = CanController::start_read_thread(arc_controller.clone());

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");
CanController::new("nonexistent_can".to_string()).expect("Could not build controller");
let controller = Arc::new(RwLock::new(controller));
VhostUserCanBackend::new(controller.clone()).expect("Could not build vhucan device");

Expand All @@ -385,7 +423,7 @@ mod tests {
#[test]
fn test_can_read_socket_fail() {
let controller =
CanController::new("can0".to_string()).expect("Could not build controller");
CanController::new("nonexistent_can".to_string()).expect("Could not build controller");
let controller = Arc::new(RwLock::new(controller));
VhostUserCanBackend::new(controller.clone()).expect("Could not build vhucan device");

Expand Down
73 changes: 25 additions & 48 deletions vhost-device-can/src/vhu_can.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,16 +272,12 @@ impl VhostUserCanBackend {
can_rx.flags = (can_rx.flags.to_native() | VIRTIO_CAN_FLAGS_RTR).into();
}

// Treat Vcan interface as CANFD if MTU is set to 64 bytes.
//
// Vcan can not be configured as CANFD interface, but it is
// possible to configure its MTU to 64 bytes. So if a messages
// 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" {
// Classic CAN frames cannot exceed 8 bytes. If CAN-FD has been
// negotiated and the frame length is > 8, treat it as a CAN-FD
// frame. This also covers virtual interfaces (vcan) which do not
// distinguish between CAN and CAN-FD at the socket level.
if self.check_features(VIRTIO_CAN_F_CAN_FD) && res_len > 8 {
res_flags |= CAN_FRMF_TYPE_FD;
warn!("\n\n\nCANFD VCAN0\n\n");
}

// Check if CAN/FD length is out-of-range (based on negotiated features)
Expand Down Expand Up @@ -777,8 +773,7 @@ mod tests {

#[test]
fn test_virtio_can_empty_requests() {
let controller =
CanController::new("can0".to_string()).expect("Could not build controller");
let controller = CanController::new_test().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");
Expand All @@ -801,8 +796,7 @@ mod tests {

#[test]
fn test_virtio_can_empty_handle_request() {
let controller =
CanController::new("can0".to_string()).expect("Could not build controller");
let controller = CanController::new_test().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");
Expand Down Expand Up @@ -873,8 +867,7 @@ mod tests {
#[test]
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");
let controller = CanController::new_test().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");
Expand Down Expand Up @@ -1100,8 +1093,7 @@ mod tests {

#[test]
fn test_virtio_can_check_tx_unknown_frame() {
let controller =
CanController::new("can0".to_string()).expect("Could not build controller");
let controller = CanController::new_test().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");
Expand All @@ -1123,8 +1115,7 @@ mod tests {

#[test]
fn test_virtio_can_check_tx_can_frame() {
let controller =
CanController::new("can0".to_string()).expect("Could not build controller");
let controller = CanController::new_test().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");
Expand Down Expand Up @@ -1171,8 +1162,7 @@ mod tests {

#[test]
fn test_virtio_can_check_tx_canfd_frame() {
let controller =
CanController::new("can0".to_string()).expect("Could not build controller");
let controller = CanController::new_test().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");
Expand Down Expand Up @@ -1278,8 +1268,7 @@ mod tests {

#[test]
fn test_virtio_can_check_tx_rtr_frame() {
let controller =
CanController::new("can0".to_string()).expect("Could not build controller");
let controller = CanController::new_test().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");
Expand Down Expand Up @@ -1340,8 +1329,7 @@ mod tests {

#[test]
fn test_virtio_can_check_tx_eff_frame() {
let controller =
CanController::new("can0".to_string()).expect("Could not build controller");
let controller = CanController::new_test().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");
Expand Down Expand Up @@ -1401,8 +1389,7 @@ mod tests {

#[test]
fn test_virtio_can_tx_general_tests() {
let controller =
CanController::new("can0".to_string()).expect("Could not build controller");
let controller = CanController::new_test().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");
Expand Down Expand Up @@ -1498,8 +1485,7 @@ mod tests {

#[test]
fn test_virtio_can_tx_device_stopped_test() {
let controller =
CanController::new("can0".to_string()).expect("Could not build controller");
let controller = CanController::new_test().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");
Expand Down Expand Up @@ -1554,7 +1540,7 @@ mod tests {
#[test]
fn test_virtio_can_tx_device_started_test_send_fail() {
let controller =
CanController::new("can0".to_string()).expect("Could not build controller");
CanController::new("nonexistent_can".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");
Expand Down Expand Up @@ -1611,8 +1597,7 @@ mod tests {

#[test]
fn test_virtio_can_tx_device_started_check_frame_fail() {
let controller =
CanController::new("can0".to_string()).expect("Could not build controller");
let controller = CanController::new_test().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");
Expand Down Expand Up @@ -1666,8 +1651,7 @@ mod tests {

#[test]
fn test_virtio_can_check_rx_err_frame() {
let controller =
CanController::new("can0".to_string()).expect("Could not build controller");
let controller = CanController::new_test().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");
Expand Down Expand Up @@ -1709,8 +1693,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");
let controller = CanController::new_test().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");
Expand Down Expand Up @@ -1746,8 +1729,7 @@ mod tests {

#[test]
fn test_virtio_can_check_rx_eff_frame() {
let controller =
CanController::new("can0".to_string()).expect("Could not build controller");
let controller = CanController::new_test().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");
Expand Down Expand Up @@ -1814,8 +1796,7 @@ mod tests {

#[test]
fn test_virtio_can_check_rx_rtr_frame() {
let controller =
CanController::new("can0".to_string()).expect("Could not build controller");
let controller = CanController::new_test().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");
Expand Down Expand Up @@ -1878,8 +1859,7 @@ mod tests {

#[test]
fn test_virtio_can_check_rx_can_frame() {
let controller =
CanController::new("can0".to_string()).expect("Could not build controller");
let controller = CanController::new_test().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");
Expand Down Expand Up @@ -1925,8 +1905,7 @@ mod tests {

#[test]
fn test_virtio_can_check_rx_canfd_frame() {
let controller =
CanController::new("can0".to_string()).expect("Could not build controller");
let controller = CanController::new_test().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");
Expand Down Expand Up @@ -2010,8 +1989,7 @@ mod tests {

#[test]
fn test_virtio_can_check_rx_canfd_vcan0() {
let controller =
CanController::new("vcan0".to_string()).expect("Could not build controller");
let controller = CanController::new_test().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");
Expand Down Expand Up @@ -2049,8 +2027,7 @@ mod tests {

#[test]
fn test_virtio_can_rx_request() {
let controller =
CanController::new("can0".to_string()).expect("Could not build controller");
let controller = CanController::new_test().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");
Expand Down