Skip to content

vhost-device-can: consume desc chain based on rx fifo queue #924

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 1 commit into
base: main
Choose a base branch
from
Open

vhost-device-can: consume desc chain based on rx fifo queue #924

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 4 additions & 0 deletions vhost-device-can/src/can.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,10 @@ impl CanController {
self.rx_fifo.size() == 0
}

pub fn rx_fifo_size(&self) -> usize {
self.rx_fifo.size()
}

pub fn pop(&mut self) -> Result<VirtioCanFrame> {
if self.rx_fifo.size() < 1 {
return Err(Error::QueueEmpty);
Expand Down
54 changes: 34 additions & 20 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 @@ -300,13 +300,15 @@ impl VhostUserCanBackend {
Ok(can_rx)
}

// return true if at least one descriptor was processed thus requiring a notification
// return false if no descriptors were processed
fn process_ctrl_requests(
&self,
requests: Vec<CanDescriptorChain>,
vring: &VringRwLock,
) -> Result<bool> {
if requests.is_empty() {
return Ok(true);
return Ok(false);
}

for desc_chain in requests {
Expand Down Expand Up @@ -371,13 +373,15 @@ impl VhostUserCanBackend {
Ok(true)
}

// return true if at least one descriptor was processed thus requiring a notification
// return false if no descriptors were processed
fn process_tx_requests(
&self,
requests: Vec<CanDescriptorChain>,
vring: &VringRwLock,
) -> Result<bool> {
if requests.is_empty() {
return Ok(true);
return Ok(false);
}

for desc_chain in requests {
Expand Down Expand Up @@ -475,18 +479,17 @@ impl VhostUserCanBackend {
Ok(true)
}

// return true if at least one descriptor was processed thus requiring a notification
// return false if no descriptors were processed
pub fn process_rx_requests(
&mut self,
requests: Vec<CanDescriptorChain>,
vring: &VringRwLock,
) -> Result<bool> {
if requests.is_empty() {
return Ok(true);
return Ok(false);
}

// This flag, if true, requests a new tx buffer from the front-end
let mut req_rx_buf = false;

for desc_chain in requests {
let atomic_mem = self.mem.as_ref().unwrap().memory();
let mut writer = desc_chain
Expand All @@ -495,14 +498,9 @@ impl VhostUserCanBackend {
.map_err(|_| Error::DescriptorWriteFailed)?;

let response = match self.controller.write().unwrap().pop() {
Ok(item) => {
// If one or more frames have been received then request
// a new rx buffer.
req_rx_buf = true;
item
}
Ok(item) => item,
Err(QueueEmpty) => {
return Ok(req_rx_buf);
return Ok(false);
}
Err(_) => {
return Err(Error::HandleEventUnknown);
Expand Down Expand Up @@ -571,15 +569,19 @@ impl VhostUserCanBackend {

/// Process the messages in the vring and dispatch replies
fn process_rx_queue(&mut self, vring: &VringRwLock) -> Result<()> {
let queue_size = self.controller.read().unwrap().rx_fifo_size();

// take only the number of descriptor chains needed to process
// the frames in the fifo queue
let requests: Vec<CanDescriptorChain> = vring
.get_mut()
.get_queue_mut()
.iter(self.mem.as_ref().unwrap().memory())
.map_err(|_| Error::DescriptorNotFound)?
.take(queue_size)
.collect();

if self.process_rx_requests(requests, vring)? {
// Send notification once all the requests are processed
vring
.signal_used_queue()
.map_err(|_| Error::NotificationFailed)?;
Expand Down Expand Up @@ -699,6 +701,17 @@ impl VhostUserBackendMut for VhostUserCanBackend {
// calling process_request_queue() until it stops finding
// new requests on the queue.
loop {
// For BACKEND_EFD, exit the loop when the RX FIFO is empty.
// Unlike RX_QUEUE (which processes guest-provided descriptors),
// BACKEND_EFD is triggered when the backend puts frames in the FIFO.
// Continuing the loop with an empty FIFO causes infinite looping
// since enable_notification() will keep returning true if there
// are available descriptors, but there's nothing to process.
if device_event == BACKEND_EFD
&& self.controller.read().unwrap().rx_fifo_size() == 0
{
break;
}
vring.disable_notification().unwrap();
match device_event {
CTRL_QUEUE => self.process_ctrl_queue(vring),
Expand Down Expand Up @@ -787,14 +800,14 @@ mod tests {
);
let vring = VringRwLock::new(mem.clone(), 0x1000).unwrap();

// Empty descriptor chain should be ignored
assert!(vu_can_backend
// Empty descriptor chain should not trigger a notification
assert!(!vu_can_backend
.process_rx_requests(Vec::<CanDescriptorChain>::new(), &vring)
.expect("Fail to examine empty rx vring"));
assert!(vu_can_backend
assert!(!vu_can_backend
.process_tx_requests(Vec::<CanDescriptorChain>::new(), &vring)
.expect("Fail to examine empty tx vring"));
assert!(vu_can_backend
assert!(!vu_can_backend
.process_ctrl_requests(Vec::<CanDescriptorChain>::new(), &vring)
.expect("Fail to examine empty ctrl vring"));
}
Expand Down Expand Up @@ -2058,8 +2071,9 @@ mod tests {

// Note: The following test are focusing only to simple CAN messages.

// Test 1: This should succeed because there is no element inserted in the
// CAN/FD frames' queue
// Test 1: This should return false because there is no element inserted in the
// CAN/FD frames' queue so none of the descriptors will be processed and thus
// no notification will be sent.
let desc_chain = build_desc_chain_mem(&mem, 1, vec![VRING_DESC_F_WRITE as u16], 0x200);
let mem1 = GuestMemoryAtomic::new(mem.clone());
let vring = VringRwLock::new(mem1.clone(), 0x1000).unwrap();
Expand Down