Skip to content
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

Virtq refactor #1174

Merged
merged 3 commits into from
May 15, 2024
+225 −563
Merged

Virtq refactor #1174

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f185bb0
virtq: remove Transfer
cagatay-y May 3, 2024
6ada7a6
virtq: remove TransferState
cagatay-y May 3, 2024
94bc283
virtq: use async_channel for moving buffers to the callers
cagatay-y May 3, 2024
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
46 changes: 43 additions & 3 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ async-lock = { version = "3.3.0", default-features = false }
simple-shell = { version = "0.0.1", optional = true }
volatile = { version = "0.5.4", features = ["unstable"] }
anstyle = { version = "1", default-features = false }
async-channel = { version = "2.2", default-features = false }

[dependencies.smoltcp]
version = "0.11"
Expand Down
15 changes: 2 additions & 13 deletions src/drivers/net/virtio_mmio.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,8 @@
//!
//! The module contains ...

use alloc::collections::VecDeque;
use alloc::rc::Rc;
use alloc::vec::Vec;
use core::cell::RefCell;
use core::ptr;
use core::ptr::read_volatile;
use core::str::FromStr;
Expand Down Expand Up @@ -138,17 +136,8 @@ impl VirtioNetDriver {
isr_stat,
notif_cfg,
ctrl_vq: CtrlQueue::new(None),
recv_vqs: RxQueues::new(
Vec::<Rc<dyn Virtq>>::new(),
Rc::new(RefCell::new(VecDeque::new())),
false,
),
send_vqs: TxQueues::new(
Vec::<Rc<dyn Virtq>>::new(),
Rc::new(RefCell::new(VecDeque::new())),
Vec::new(),
false,
),
recv_vqs: RxQueues::new(Vec::<Rc<dyn Virtq>>::new(), false),
send_vqs: TxQueues::new(Vec::<Rc<dyn Virtq>>::new(), Vec::new(), false),
num_vqs: 0,
irq,
mtu,
Expand Down
92 changes: 39 additions & 53 deletions src/drivers/net/virtio_net.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,9 @@
//!
//! The module contains ...

use alloc::collections::VecDeque;
use alloc::boxed::Box;
use alloc::rc::Rc;
use alloc::vec::Vec;
use core::cell::RefCell;
use core::cmp::Ordering;
use core::mem;

Expand All @@ -31,9 +30,7 @@ use crate::drivers::virtio::transport::mmio::{ComCfg, IsrStatus, NotifCfg};
use crate::drivers::virtio::transport::pci::{ComCfg, IsrStatus, NotifCfg};
use crate::drivers::virtio::virtqueue::packed::PackedVq;
use crate::drivers::virtio::virtqueue::split::SplitVq;
use crate::drivers::virtio::virtqueue::{
BuffSpec, BufferToken, Bytes, Transfer, Virtq, VqIndex, VqSize,
};
use crate::drivers::virtio::virtqueue::{BuffSpec, BufferToken, Bytes, Virtq, VqIndex, VqSize};
use crate::executor::device::{RxToken, TxToken};

/// A wrapper struct for the raw configuration structure.
Expand Down Expand Up @@ -155,19 +152,18 @@ enum MqCmd {

pub struct RxQueues {
vqs: Vec<Rc<dyn Virtq>>,
poll_queue: Rc<RefCell<VecDeque<Transfer>>>,
poll_sender: async_channel::Sender<Box<BufferToken>>,
poll_receiver: async_channel::Receiver<Box<BufferToken>>,
is_multi: bool,
}

impl RxQueues {
pub fn new(
vqs: Vec<Rc<dyn Virtq>>,
poll_queue: Rc<RefCell<VecDeque<Transfer>>>,
is_multi: bool,
) -> Self {
pub fn new(vqs: Vec<Rc<dyn Virtq>>, is_multi: bool) -> Self {
let (poll_sender, poll_receiver) = async_channel::unbounded();
Self {
vqs,
poll_queue,
poll_sender,
poll_receiver,
is_multi,
}
}
Expand All @@ -176,14 +172,8 @@ impl RxQueues {
/// This currently include nothing. But in the future it might include among others::
/// * Calculating missing checksums
/// * Merging receive buffers, by simply checking the poll_queue (if VIRTIO_NET_F_MRG_BUF)
fn post_processing(transfer: Transfer) -> Result<Transfer, VirtioNetError> {
if transfer.poll() {
// Here we could implement all features.
Ok(transfer)
} else {
warn!("Unfinished transfer in post processing. Returning buffer to queue. This will need explicit cleanup.");
Err(VirtioNetError::ProcessOngoing)
}
fn post_processing(buffer_tkn: Box<BufferToken>) -> Result<Box<BufferToken>, VirtioNetError> {
Ok(buffer_tkn)
}

/// Adds a given queue to the underlying vector and populates the queue with RecvBuffers.
Expand Down Expand Up @@ -219,7 +209,7 @@ impl RxQueues {
// Transfers will be awaited at the queue
buff_tkn
.provide()
.dispatch_await(Rc::clone(&self.poll_queue), false);
.dispatch_await(self.poll_sender.clone(), false);
}

// Safe virtqueue
Expand All @@ -230,15 +220,17 @@ impl RxQueues {
}
}

fn get_next(&mut self) -> Option<Transfer> {
let transfer = self.poll_queue.borrow_mut().pop_front();
fn get_next(&mut self) -> Option<Box<BufferToken>> {
let transfer = self.poll_receiver.try_recv();

transfer.or_else(|| {
// Check if any not yet provided transfers are in the queue.
self.poll();
transfer
.or_else(|_| {
// Check if any not yet provided transfers are in the queue.
self.poll();

self.poll_queue.borrow_mut().pop_front()
})
self.poll_receiver.try_recv()
})
.ok()
}

fn poll(&self) {
Expand Down Expand Up @@ -276,23 +268,21 @@ impl RxQueues {
/// to the respective queue structures.
pub struct TxQueues {
vqs: Vec<Rc<dyn Virtq>>,
poll_queue: Rc<RefCell<VecDeque<Transfer>>>,
poll_sender: async_channel::Sender<Box<BufferToken>>,
poll_receiver: async_channel::Receiver<Box<BufferToken>>,
ready_queue: Vec<BufferToken>,
/// Indicates, whether the Driver/Device are using multiple
/// queues for communication.
is_multi: bool,
}

impl TxQueues {
pub fn new(
vqs: Vec<Rc<dyn Virtq>>,
poll_queue: Rc<RefCell<VecDeque<Transfer>>>,
ready_queue: Vec<BufferToken>,
is_multi: bool,
) -> Self {
pub fn new(vqs: Vec<Rc<dyn Virtq>>, ready_queue: Vec<BufferToken>, is_multi: bool) -> Self {
let (poll_sender, poll_receiver) = async_channel::unbounded();
Self {
vqs,
poll_queue,
poll_sender,
poll_receiver,
ready_queue,
is_multi,
}
Expand Down Expand Up @@ -415,12 +405,12 @@ impl TxQueues {
}
}

if self.poll_queue.borrow().is_empty() {
if self.poll_receiver.is_empty() {
self.poll();
}

while let Some(transfer) = self.poll_queue.borrow_mut().pop_back() {
let mut tkn = transfer.reuse().unwrap();
while let Ok(buffer_token) = self.poll_receiver.try_recv() {
let mut tkn = buffer_token.reset();
let (send_len, _) = tkn.len();

match send_len.cmp(&len) {
Expand Down Expand Up @@ -491,7 +481,7 @@ impl NetworkDriver for VirtioNetDriver {
#[allow(dead_code)]
fn has_packet(&self) -> bool {
self.recv_vqs.poll();
!self.recv_vqs.poll_queue.borrow().is_empty()
!self.recv_vqs.poll_receiver.is_empty()
}

/// Provides smoltcp a slice to copy the IP packet and transfer the packet
Expand Down Expand Up @@ -554,7 +544,7 @@ impl NetworkDriver for VirtioNetDriver {

buff_tkn
.provide()
.dispatch_await(Rc::clone(&self.send_vqs.poll_queue), false);
.dispatch_await(self.send_vqs.poll_sender.clone(), false);

result
} else {
Expand Down Expand Up @@ -586,10 +576,9 @@ impl NetworkDriver for VirtioNetDriver {
// drop packets with invalid packet size
if packet.len() < HEADER_SIZE {
transfer
.reuse()
.unwrap()
.reset()
.provide()
.dispatch_await(Rc::clone(&self.recv_vqs.poll_queue), false);
.dispatch_await(self.recv_vqs.poll_sender.clone(), false);

return None;
}
Expand All @@ -604,10 +593,9 @@ impl NetworkDriver for VirtioNetDriver {

vec_data.extend_from_slice(&packet[mem::size_of::<VirtioNetHdr>()..]);
transfer
.reuse()
.unwrap()
.reset()
.provide()
.dispatch_await(Rc::clone(&self.recv_vqs.poll_queue), false);
.dispatch_await(self.recv_vqs.poll_sender.clone(), false);

num_buffers
};
Expand All @@ -627,22 +615,20 @@ impl NetworkDriver for VirtioNetDriver {
let packet = recv_data.pop().unwrap();
vec_data.extend_from_slice(packet);
transfer
.reuse()
.unwrap()
.reset()
.provide()
.dispatch_await(Rc::clone(&self.recv_vqs.poll_queue), false);
.dispatch_await(self.recv_vqs.poll_sender.clone(), false);
}

Some((RxToken::new(vec_data), TxToken::new()))
} else {
error!("Empty transfer, or with wrong buffer layout. Reusing and returning error to user-space network driver...");
transfer
.reuse()
.unwrap()
.reset()
.write_seq(None::<&VirtioNetHdr>, Some(&VirtioNetHdr::default()))
.unwrap()
.provide()
.dispatch_await(Rc::clone(&self.recv_vqs.poll_queue), false);
.dispatch_await(self.recv_vqs.poll_sender.clone(), false);

None
}
Expand Down
12 changes: 2 additions & 10 deletions src/drivers/net/virtio_pci.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,7 @@
//!
//! The module contains ...

use alloc::collections::VecDeque;
use alloc::rc::Rc;
use alloc::vec::Vec;
use core::cell::RefCell;
use core::str::FromStr;

use smoltcp::phy::ChecksumCapabilities;
Expand Down Expand Up @@ -146,13 +143,8 @@ impl VirtioNetDriver {
notif_cfg,

ctrl_vq: CtrlQueue::new(None),
recv_vqs: RxQueues::new(Vec::new(), Rc::new(RefCell::new(VecDeque::new())), false),
send_vqs: TxQueues::new(
Vec::new(),
Rc::new(RefCell::new(VecDeque::new())),
Vec::new(),
false,
),
recv_vqs: RxQueues::new(Vec::new(), false),
send_vqs: TxQueues::new(Vec::new(), Vec::new(), false),
num_vqs: 0,
irq: device.get_irq().unwrap(),
mtu,
Expand Down
Loading