From 4806357f52d74ec0114114a38369fa8b80259440 Mon Sep 17 00:00:00 2001 From: Rob Bradford Date: Wed, 28 Apr 2021 13:16:10 +0100 Subject: [PATCH] virtio-devices: net: Cleanup the MQ handling in the control queue Cleanup the control queue handling in preparation for supporting alternative commands. Note that this change does not make the MQ handling spec compliant. According to the specification MQ should only be enabled once the number of queue pairs the guest would like to use has been specified. The only improvement towards the specication in this change is correct error handling if the guest specifies an inappropriate number of queues (out of range.) Signed-off-by: Rob Bradford --- virtio-devices/src/net.rs | 1 - virtio-devices/src/net_util.rs | 107 ++++++++++++-------------- virtio-devices/src/seccomp_filters.rs | 7 +- virtio-devices/src/vhost_user/net.rs | 1 - 4 files changed, 52 insertions(+), 64 deletions(-) diff --git a/virtio-devices/src/net.rs b/virtio-devices/src/net.rs index cbb2e88ce..d51475759 100644 --- a/virtio-devices/src/net.rs +++ b/virtio-devices/src/net.rs @@ -513,7 +513,6 @@ impl VirtioDevice for Net { kill_evt, pause_evt, ctrl_q: CtrlVirtio::new(cvq_queue, cvq_queue_evt), - epoll_fd: 0, }; let paused = self.common.paused.clone(); diff --git a/virtio-devices/src/net_util.rs b/virtio-devices/src/net_util.rs index 47716610d..8df11d455 100644 --- a/virtio-devices/src/net_util.rs +++ b/virtio-devices/src/net_util.rs @@ -2,13 +2,10 @@ // // SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause -use super::{ - DescriptorChain, EpollHelper, EpollHelperError, EpollHelperHandler, Queue, - EPOLL_HELPER_EVENT_LAST, -}; +use super::{EpollHelper, EpollHelperError, EpollHelperHandler, Queue, EPOLL_HELPER_EVENT_LAST}; use net_util::MacAddr; use std::os::raw::c_uint; -use std::os::unix::io::{AsRawFd, RawFd}; +use std::os::unix::io::AsRawFd; use std::sync::atomic::AtomicBool; use std::sync::{Arc, Barrier}; use virtio_bindings::bindings::virtio_net::*; @@ -40,22 +37,16 @@ unsafe impl ByteValued for VirtioNetConfig {} #[derive(Debug)] pub enum Error { - /// Read process MQ. - FailedProcessMq, /// Read queue failed. GuestMemory(GuestMemoryError), /// Invalid ctrl class InvalidCtlClass, /// Invalid ctrl command InvalidCtlCmd, - /// Invalid descriptor - InvalidDesc, - /// Invalid queue pairs number - InvalidQueuePairsNum, - /// No memory passed in. - NoMemory, - /// No ueue pairs number. - NoQueuePairsNum, + /// No queue pairs number. + NoQueuePairsDescriptor, + /// No status descriptor + NoStatusDescriptor, } pub struct CtrlVirtio { @@ -72,61 +63,60 @@ impl std::clone::Clone for CtrlVirtio { } } +#[repr(C, packed)] +#[derive(Debug, Clone, Copy, Default)] +pub struct ControlHeader { + pub class: u8, + pub cmd: u8, +} + +unsafe impl ByteValued for ControlHeader {} + impl CtrlVirtio { pub fn new(queue: Queue, queue_evt: EventFd) -> Self { CtrlVirtio { queue_evt, queue } } - fn process_mq(&self, mem: &GuestMemoryMmap, avail_desc: DescriptorChain) -> Result<()> { - let mq_desc = if avail_desc.has_next() { - avail_desc.next_descriptor().unwrap() - } else { - return Err(Error::NoQueuePairsNum); - }; - let queue_pairs = mem - .read_obj::(mq_desc.addr) - .map_err(Error::GuestMemory)?; - if (queue_pairs < VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN as u16) - || (queue_pairs > VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX as u16) - { - return Err(Error::InvalidQueuePairsNum); - } - let status_desc = if mq_desc.has_next() { - mq_desc.next_descriptor().unwrap() - } else { - return Err(Error::NoQueuePairsNum); - }; - mem.write_obj::(0, status_desc.addr) - .map_err(Error::GuestMemory)?; - - Ok(()) - } - pub fn process_cvq(&mut self, mem: &GuestMemoryMmap) -> Result<()> { let mut used_desc_heads = [(0, 0); QUEUE_SIZE]; let mut used_count = 0; - if let Some(avail_desc) = self.queue.iter(&mem).next() { - used_desc_heads[used_count] = (avail_desc.index, avail_desc.len); - used_count += 1; - let ctrl_hdr = mem - .read_obj::(avail_desc.addr) - .map_err(Error::GuestMemory)?; - let ctrl_hdr_v = ctrl_hdr.as_slice(); - let class = ctrl_hdr_v[0]; - let cmd = ctrl_hdr_v[1]; - match u32::from(class) { + let queue = &mut self.queue; + for avail_desc in queue.iter(&mem) { + let ctrl_hdr: ControlHeader = + mem.read_obj(avail_desc.addr).map_err(Error::GuestMemory)?; + match u32::from(ctrl_hdr.class) { VIRTIO_NET_CTRL_MQ => { - if u32::from(cmd) != VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET { - return Err(Error::InvalidCtlCmd); - } - if let Err(_e) = self.process_mq(&mem, avail_desc) { - return Err(Error::FailedProcessMq); - } + let mq_desc = avail_desc + .next_descriptor() + .ok_or(Error::NoQueuePairsDescriptor)?; + let queue_pairs = mem + .read_obj::(mq_desc.addr) + .map_err(Error::GuestMemory)?; + let status_desc = mq_desc.next_descriptor().ok_or(Error::NoStatusDescriptor)?; + + let ok = if u32::from(ctrl_hdr.cmd) != VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET { + warn!("Unsupported command: {}", ctrl_hdr.cmd); + false + } else if (queue_pairs < VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN as u16) + || (queue_pairs > VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX as u16) + { + warn!("Number of MQ pairs out of range: {}", queue_pairs); + false + } else { + info!("Number of MQ pairs requested: {}", queue_pairs); + true + }; + + mem.write_obj( + if ok { VIRTIO_NET_OK } else { VIRTIO_NET_ERR } as u8, + status_desc.addr, + ) + .map_err(Error::GuestMemory)?; } _ => return Err(Error::InvalidCtlClass), } - } else { - return Err(Error::InvalidDesc); + used_desc_heads[used_count] = (avail_desc.index, avail_desc.len); + used_count += 1; } for &(desc_index, len) in &used_desc_heads[..used_count] { self.queue.add_used(&mem, desc_index, len); @@ -142,7 +132,6 @@ pub struct NetCtrlEpollHandler { pub kill_evt: EventFd, pub pause_evt: EventFd, pub ctrl_q: CtrlVirtio, - pub epoll_fd: RawFd, } impl NetCtrlEpollHandler { diff --git a/virtio-devices/src/seccomp_filters.rs b/virtio-devices/src/seccomp_filters.rs index 37a068561..7194de75b 100644 --- a/virtio-devices/src/seccomp_filters.rs +++ b/virtio-devices/src/seccomp_filters.rs @@ -378,12 +378,13 @@ fn virtio_vhost_net_ctl_thread_rules() -> Vec { allow_syscall(libc::SYS_epoll_pwait), #[cfg(target_arch = "x86_64")] allow_syscall(libc::SYS_epoll_wait), + allow_syscall(libc::SYS_exit), allow_syscall(libc::SYS_futex), - allow_syscall(libc::SYS_read), - allow_syscall(libc::SYS_sigaltstack), allow_syscall(libc::SYS_munmap), allow_syscall(libc::SYS_madvise), - allow_syscall(libc::SYS_exit), + allow_syscall(libc::SYS_read), + allow_syscall(libc::SYS_sigaltstack), + allow_syscall(libc::SYS_write), ] } diff --git a/virtio-devices/src/vhost_user/net.rs b/virtio-devices/src/vhost_user/net.rs index ea52084e6..ce743c81e 100644 --- a/virtio-devices/src/vhost_user/net.rs +++ b/virtio-devices/src/vhost_user/net.rs @@ -248,7 +248,6 @@ impl VirtioDevice for Net { kill_evt, pause_evt, ctrl_q: CtrlVirtio::new(cvq_queue, cvq_queue_evt), - epoll_fd: 0, }; let paused = self.common.paused.clone();