Skip to content

refactor(virtio-net): avoid copy on rx #4742

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.

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

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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 resources/seccomp/aarch64-unknown-linux-musl.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@
{
"syscall": "read"
},
{
"syscall": "readv",
"comment": "Used by the VirtIO net device to read from tap"
},
{
"syscall": "write"
},
Expand Down
4 changes: 4 additions & 0 deletions resources/seccomp/x86_64-unknown-linux-musl.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@
{
"syscall": "read"
},
{
"syscall": "readv",
"comment": "Used by the VirtIO net device to read from tap"
},
{
"syscall": "write"
},
Expand Down
133 changes: 117 additions & 16 deletions src/vmm/src/devices/virtio/iovec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,16 @@
len: u32,
}

impl IoVecBuffer {
/// Create an empty `IoVecBuffer` with a given capacity.
pub fn with_capacity(cap: usize) -> IoVecBuffer {
IoVecBuffer {
vecs: IoVecVec::with_capacity(cap),
len: 0,
}
}
}

// SAFETY: `IoVecBuffer` doesn't allow for interior mutability and no shared ownership is possible
// as it doesn't implement clone
unsafe impl Send for IoVecBuffer {}
Expand All @@ -57,12 +67,11 @@
/// The descriptor chain cannot be referencing the same memory location as another chain
pub unsafe fn load_descriptor_chain(
&mut self,
head: DescriptorChain,
mut desc: DescriptorChain,
) -> Result<(), IoVecError> {
self.clear();

let mut next_descriptor = Some(head);
while let Some(desc) = next_descriptor {
loop {
if desc.is_write_only() {
return Err(IoVecError::WriteOnlyDescriptor);
}
Expand All @@ -85,7 +94,9 @@
.checked_add(desc.len)
.ok_or(IoVecError::OverflowedDescriptor)?;

next_descriptor = desc.next_descriptor();
if desc.load_next_descriptor().is_none() {
break;
}
}

Ok(())
Expand Down Expand Up @@ -217,21 +228,39 @@
/// It describes a write-only buffer passed to us by the guest that is scattered across multiple
/// memory regions. Additionally, this wrapper provides methods that allow reading arbitrary ranges
/// of data from that buffer.
#[derive(Debug)]
#[derive(Debug, Default)]
pub struct IoVecBufferMut {
// container of the memory regions included in this IO vector
vecs: IoVecVec,
// Total length of the IoVecBufferMut
len: u32,
}

impl IoVecBufferMut {
/// Create an empty `IoVecBufferMut` with a given capacity.
pub fn with_capacity(cap: usize) -> IoVecBufferMut {
IoVecBufferMut {
vecs: IoVecVec::with_capacity(cap),
len: 0,
}
}
}

// SAFETY: iovec pointers are safe to send across threads.
unsafe impl Send for IoVecBufferMut {}

impl IoVecBufferMut {
/// Create an `IoVecBufferMut` from a `DescriptorChain`
pub fn from_descriptor_chain(head: DescriptorChain) -> Result<Self, IoVecError> {
let mut vecs = IoVecVec::new();
let mut len = 0u32;
/// # Safety
/// The descriptor chain cannot be referencing the same memory location as another chain.
pub unsafe fn load_descriptor_chain(
&mut self,
mut desc: DescriptorChain,
max_size: Option<u32>,
) -> Result<(), IoVecError> {
self.clear();

for desc in head {
loop {
if !desc.is_write_only() {
return Err(IoVecError::ReadOnlyDescriptor);
}
Expand All @@ -247,23 +276,74 @@
slice.bitmap().mark_dirty(0, desc.len as usize);

let iov_base = slice.ptr_guard_mut().as_ptr().cast::<c_void>();
vecs.push(iovec {
self.vecs.push(iovec {
iov_base,
iov_len: desc.len as size_t,
});
len = len
self.len = self
.len
.checked_add(desc.len)
.ok_or(IoVecError::OverflowedDescriptor)?;
if matches!(max_size, Some(max) if self.len >= max) {
break;

Check warning on line 288 in src/vmm/src/devices/virtio/iovec.rs

View check run for this annotation

Codecov / codecov/patch

src/vmm/src/devices/virtio/iovec.rs#L288

Added line #L288 was not covered by tests
}
if desc.load_next_descriptor().is_none() {
break;
}
}

Ok(Self { vecs, len })
Ok(())
}

/// Create an `IoVecBufferMut` from a `DescriptorChain`
/// # Safety
/// The descriptor chain cannot be referencing the same memory location as another chain.
pub unsafe fn from_descriptor_chain(head: DescriptorChain) -> Result<Self, IoVecError> {
let mut new_buffer = Self::default();

Self::load_descriptor_chain(&mut new_buffer, head, None)?;

Ok(new_buffer)
}

/// Get the total length of the memory regions covered by this `IoVecBuffer`
pub(crate) fn len(&self) -> u32 {
self.len
}

/// Returns a pointer to the memory keeping the `iovec` structs
pub fn as_iovec_ptr(&self) -> *const iovec {
self.vecs.as_ptr()
}

/// Returns the length of the `iovec` array.
pub fn iovec_count(&self) -> usize {
self.vecs.len()
}

/// Clears the `iovec` array
pub fn clear(&mut self) {
self.vecs.clear();
self.len = 0u32;
}

/// Push an iovec into the `IoVecBufferMut`.
/// # Safety
/// The iovec must refer to a valid memory slice.
pub unsafe fn push(&mut self, iovec: iovec) -> Result<(), IoVecError> {
let iov_len = iovec
.iov_len
.try_into()
.map_err(|_| IoVecError::OverflowedDescriptor)?;

self.vecs.push(iovec);
self.len = self
.len
.checked_add(iov_len)
.ok_or(IoVecError::OverflowedDescriptor)?;
Ok(())
}

/// Writes a number of bytes into the `IoVecBufferMut` starting at a given offset.
///
/// This will try to fill `IoVecBufferMut` writing bytes from the `buf` starting from
Expand Down Expand Up @@ -407,6 +487,24 @@
}
}

impl<'a> From<Vec<&'a mut [u8]>> for IoVecBufferMut {
fn from(buffer: Vec<&'a mut [u8]>) -> Self {
let mut len = 0_u32;
let vecs = buffer
.into_iter()
.map(|slice| {
len += TryInto::<u32>::try_into(slice.len()).unwrap();
iovec {
iov_base: slice.as_ptr() as *mut c_void,
iov_len: slice.len(),
}
})
.collect();

Self { vecs, len }
}
}

fn default_mem() -> GuestMemoryMmap {
multi_region_mem(&[
(GuestAddress(0), 0x10000),
Expand Down Expand Up @@ -468,11 +566,13 @@

let (mut q, _) = read_only_chain(&mem);
let head = q.pop(&mem).unwrap();
IoVecBufferMut::from_descriptor_chain(head).unwrap_err();
// SAFETY: This descriptor chain is only loaded into one buffer.
unsafe { IoVecBufferMut::from_descriptor_chain(head).unwrap_err() };

let (mut q, _) = write_only_chain(&mem);
let head = q.pop(&mem).unwrap();
IoVecBufferMut::from_descriptor_chain(head).unwrap();
// SAFETY: This descriptor chain is only loaded into one buffer.
unsafe { IoVecBufferMut::from_descriptor_chain(head).unwrap() };
}

#[test]
Expand All @@ -493,7 +593,7 @@
let head = q.pop(&mem).unwrap();

// SAFETY: This descriptor chain is only loaded once in this test
let iovec = IoVecBufferMut::from_descriptor_chain(head).unwrap();
let iovec = unsafe { IoVecBufferMut::from_descriptor_chain(head).unwrap() };
assert_eq!(iovec.len(), 4 * 64);
}

Expand Down Expand Up @@ -558,7 +658,8 @@
// This is a descriptor chain with 4 elements 64 bytes long each.
let head = q.pop(&mem).unwrap();

let mut iovec = IoVecBufferMut::from_descriptor_chain(head).unwrap();
// SAFETY: This descriptor chain is only loaded into one buffer.
let mut iovec = unsafe { IoVecBufferMut::from_descriptor_chain(head).unwrap() };
let buf = vec![0u8, 1, 2, 3, 4];

// One test vector for each part of the chain
Expand Down
Loading
Loading