From 48faf3abac2331cfb5d0faed85fc2ae103892435 Mon Sep 17 00:00:00 2001 From: Rob Bradford Date: Tue, 7 Jul 2020 15:02:18 +0100 Subject: [PATCH] net_util, virtio-devices, vhost_user_net: Relocate {Rx,Tx}Virtio By moving the code for opening the two RX and TX queues into a shared location we are starting to remove the requirement for the vhost-user-net backend to depend on the virtio-devices crate which in of itself depends on many other crates that are not necessary for the backend to function. Signed-off-by: Rob Bradford --- Cargo.lock | 3 + net_util/Cargo.toml | 3 + net_util/src/lib.rs | 6 ++ net_util/src/queue_pair.rs | 189 +++++++++++++++++++++++++++++++++ vhost_user_net/src/lib.rs | 3 +- virtio-devices/src/net.rs | 7 +- virtio-devices/src/net_util.rs | 187 +------------------------------- 7 files changed, 208 insertions(+), 190 deletions(-) create mode 100644 net_util/src/queue_pair.rs diff --git a/Cargo.lock b/Cargo.lock index 9532b18a7..a07d10b2b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -633,12 +633,15 @@ version = "0.1.0" dependencies = [ "lazy_static", "libc", + "log 0.4.8", "net_gen", "pnet", "rand 0.7.3", "serde", "serde_json", "virtio-bindings", + "vm-memory", + "vm-virtio", "vmm-sys-util", ] diff --git a/net_util/Cargo.toml b/net_util/Cargo.toml index 2e71fc52f..81257aebf 100644 --- a/net_util/Cargo.toml +++ b/net_util/Cargo.toml @@ -5,10 +5,13 @@ authors = ["The Chromium OS Authors"] [dependencies] libc = "0.2.72" +log = "0.4.8" net_gen = { path = "../net_gen" } rand = "0.7.3" serde = "1.0.114" virtio-bindings = "0.1.0" +vm-memory = { version = "0.2.1", features = ["backend-mmap", "backend-atomic"] } +vm-virtio = { path = "../vm-virtio" } vmm-sys-util = ">=0.3.1" [dev-dependencies] diff --git a/net_util/src/lib.rs b/net_util/src/lib.rs index 943c3d5e7..c8fd8f7fa 100644 --- a/net_util/src/lib.rs +++ b/net_util/src/lib.rs @@ -11,14 +11,19 @@ #[macro_use] extern crate lazy_static; extern crate libc; +#[macro_use] +extern crate log; extern crate net_gen; extern crate rand; extern crate serde; extern crate virtio_bindings; +extern crate vm_memory; +extern crate vm_virtio; extern crate vmm_sys_util; mod mac; mod open_tap; +mod queue_pair; mod tap; use std::io::Error as IoError; @@ -28,6 +33,7 @@ use std::os::unix::io::FromRawFd; pub use mac::{MacAddr, MAC_ADDR_LEN}; pub use open_tap::{open_tap, Error as OpenTapError}; +pub use queue_pair::{RxVirtio, TxVirtio}; pub use tap::{Error as TapError, Tap}; #[derive(Debug)] diff --git a/net_util/src/queue_pair.rs b/net_util/src/queue_pair.rs new file mode 100644 index 000000000..44fe157cb --- /dev/null +++ b/net_util/src/queue_pair.rs @@ -0,0 +1,189 @@ +// Copyright (c) 2020 Intel Corporation. All rights reserved. +// +// SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause + +use super::Tap; +use std::cmp; +use std::io::Write; +use std::num::Wrapping; +use vm_memory::{Bytes, GuestAddress, GuestMemoryMmap}; +use vm_virtio::{DescriptorChain, Queue}; +use virtio_bindings::bindings::virtio_net::virtio_net_hdr_v1 + +/// The maximum buffer size when segmentation offload is enabled. This +/// includes the 12-byte virtio net header. +/// http://docs.oasis-open.org/virtio/virtio/v1.0/virtio-v1.0.html#x1-1740003 +const MAX_BUFFER_SIZE: usize = 65562; + +#[derive(Clone)] +pub struct TxVirtio { + pub iovec: Vec<(GuestAddress, usize)>, + pub frame_buf: [u8; MAX_BUFFER_SIZE], + pub counter_bytes: Wrapping, + pub counter_frames: Wrapping, +} + +impl Default for TxVirtio { + fn default() -> Self { + Self::new() + } +} + +impl TxVirtio { + pub fn new() -> Self { + TxVirtio { + iovec: Vec::new(), + frame_buf: [0u8; MAX_BUFFER_SIZE], + counter_bytes: Wrapping(0), + counter_frames: Wrapping(0), + } + } + + pub fn process_desc_chain(&mut self, mem: &GuestMemoryMmap, tap: &mut Tap, queue: &mut Queue) { + while let Some(avail_desc) = queue.iter(&mem).next() { + let head_index = avail_desc.index; + let mut read_count = 0; + let mut next_desc = Some(avail_desc); + + self.iovec.clear(); + while let Some(desc) = next_desc { + if desc.is_write_only() { + break; + } + self.iovec.push((desc.addr, desc.len as usize)); + read_count += desc.len as usize; + next_desc = desc.next_descriptor(); + } + + read_count = 0; + // Copy buffer from across multiple descriptors. + // TODO(performance - Issue #420): change this to use `writev()` instead of `write()` + // and get rid of the intermediate buffer. + for (desc_addr, desc_len) in self.iovec.drain(..) { + let limit = cmp::min((read_count + desc_len) as usize, self.frame_buf.len()); + + let read_result = + mem.read_slice(&mut self.frame_buf[read_count..limit as usize], desc_addr); + match read_result { + Ok(_) => { + // Increment by number of bytes actually read + read_count += limit - read_count; + } + Err(e) => { + println!("Failed to read slice: {:?}", e); + break; + } + } + } + + let write_result = tap.write(&self.frame_buf[..read_count]); + match write_result { + Ok(_) => {} + Err(e) => { + println!("net: tx: error failed to write to tap: {}", e); + } + }; + + self.counter_bytes += Wrapping((read_count - vnet_hdr_len()) as u64); + self.counter_frames += Wrapping(1); + + queue.add_used(&mem, head_index, 0); + queue.update_avail_event(&mem); + } + } +} + +#[derive(Clone)] +pub struct RxVirtio { + pub deferred_frame: bool, + pub deferred_irqs: bool, + pub bytes_read: usize, + pub frame_buf: [u8; MAX_BUFFER_SIZE], + pub counter_bytes: Wrapping, + pub counter_frames: Wrapping, +} + +impl Default for RxVirtio { + fn default() -> Self { + Self::new() + } +} + +impl RxVirtio { + pub fn new() -> Self { + RxVirtio { + deferred_frame: false, + deferred_irqs: false, + bytes_read: 0, + frame_buf: [0u8; MAX_BUFFER_SIZE], + counter_bytes: Wrapping(0), + counter_frames: Wrapping(0), + } + } + + pub fn process_desc_chain( + &mut self, + mem: &GuestMemoryMmap, + mut next_desc: Option, + queue: &mut Queue, + ) -> bool { + let head_index = next_desc.as_ref().unwrap().index; + let mut write_count = 0; + + // Copy from frame into buffer, which may span multiple descriptors. + loop { + match next_desc { + Some(desc) => { + if !desc.is_write_only() { + break; + } + let limit = cmp::min(write_count + desc.len as usize, self.bytes_read); + let source_slice = &self.frame_buf[write_count..limit]; + let write_result = mem.write_slice(source_slice, desc.addr); + + match write_result { + Ok(_) => { + write_count = limit; + } + Err(e) => { + error!("Failed to write slice: {:?}", e); + break; + } + }; + + if write_count >= self.bytes_read { + break; + } + next_desc = desc.next_descriptor(); + } + None => { + warn!("Receiving buffer is too small to hold frame of current size"); + break; + } + } + } + + self.counter_bytes += Wrapping((write_count - vnet_hdr_len()) as u64); + self.counter_frames += Wrapping(1); + + queue.add_used(&mem, head_index, write_count as u32); + queue.update_avail_event(&mem); + + // Mark that we have at least one pending packet and we need to interrupt the guest. + self.deferred_irqs = true; + + // Update the frame_buf buffer. + if write_count < self.bytes_read { + self.frame_buf.copy_within(write_count..self.bytes_read, 0); + self.bytes_read -= write_count; + false + } else { + self.bytes_read = 0; + true + } + } +} + +fn vnet_hdr_len() -> usize { + std::mem::size_of::() +} diff --git a/vhost_user_net/src/lib.rs b/vhost_user_net/src/lib.rs index 01662305f..6353b0a7b 100644 --- a/vhost_user_net/src/lib.rs +++ b/vhost_user_net/src/lib.rs @@ -14,7 +14,7 @@ extern crate virtio_devices; use libc::{self, EFD_NONBLOCK}; use log::*; -use net_util::{open_tap, MacAddr, OpenTapError, Tap}; +use net_util::{open_tap, MacAddr, OpenTapError, RxVirtio, Tap, TxVirtio}; use option_parser::{OptionParser, OptionParserError}; use std::fmt; use std::io::{self}; @@ -28,7 +28,6 @@ use vhost_rs::vhost_user::{Error as VhostUserError, Listener}; use vhost_user_backend::{VhostUserBackend, VhostUserDaemon, Vring, VringWorker}; use virtio_bindings::bindings::virtio_net::*; use virtio_bindings::bindings::virtio_ring::VIRTIO_RING_F_EVENT_IDX; -use virtio_devices::net_util::{RxVirtio, TxVirtio}; use virtio_devices::{NetCounters, NetQueuePair}; use vm_memory::{GuestMemoryAtomic, GuestMemoryMmap}; use vmm_sys_util::eventfd::EventFd; diff --git a/virtio-devices/src/net.rs b/virtio-devices/src/net.rs index 1cd6dcffa..4900fa389 100644 --- a/virtio-devices/src/net.rs +++ b/virtio-devices/src/net.rs @@ -7,8 +7,8 @@ use super::net_util::{ build_net_config_space, build_net_config_space_with_mq, register_listener, unregister_listener, - CtrlVirtio, NetCtrlEpollHandler, RxVirtio, TxVirtio, VirtioNetConfig, KILL_EVENT, - NET_EVENTS_COUNT, PAUSE_EVENT, RX_QUEUE_EVENT, RX_TAP_EVENT, TX_QUEUE_EVENT, + CtrlVirtio, NetCtrlEpollHandler, VirtioNetConfig, KILL_EVENT, NET_EVENTS_COUNT, PAUSE_EVENT, + RX_QUEUE_EVENT, RX_TAP_EVENT, TX_QUEUE_EVENT, }; use super::Error as DeviceError; use super::{ @@ -18,8 +18,7 @@ use crate::VirtioInterrupt; use anyhow::anyhow; use libc::EAGAIN; use libc::EFD_NONBLOCK; -use net_util::{open_tap, OpenTapError}; -use net_util::{MacAddr, Tap}; +use net_util::{open_tap, MacAddr, OpenTapError, RxVirtio, Tap, TxVirtio}; use std::cmp; use std::collections::HashMap; use std::fs::File; diff --git a/virtio-devices/src/net_util.rs b/virtio-devices/src/net_util.rs index 167692726..72be75e84 100644 --- a/virtio-devices/src/net_util.rs +++ b/virtio-devices/src/net_util.rs @@ -4,30 +4,22 @@ use super::Error as DeviceError; use super::{DescriptorChain, DeviceEventT, Queue}; -use net_util::{MacAddr, Tap}; +use net_util::MacAddr; use serde::ser::{Serialize, SerializeStruct, Serializer}; -use std::cmp; use std::fs::File; -use std::io::{self, Write}; -use std::mem; -use std::num::Wrapping; +use std::io; use std::os::unix::io::{AsRawFd, FromRawFd, RawFd}; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Arc; use std::thread; use virtio_bindings::bindings::virtio_net::*; use vm_memory::{ - ByteValued, Bytes, GuestAddress, GuestAddressSpace, GuestMemoryAtomic, GuestMemoryError, - GuestMemoryMmap, + ByteValued, Bytes, GuestAddressSpace, GuestMemoryAtomic, GuestMemoryError, GuestMemoryMmap, }; use vmm_sys_util::eventfd::EventFd; type Result = std::result::Result; -/// The maximum buffer size when segmentation offload is enabled. This -/// includes the 12-byte virtio net header. -/// http://docs.oasis-open.org/virtio/virtio/v1.0/virtio-v1.0.html#x1-1740003 -const MAX_BUFFER_SIZE: usize = 65562; const QUEUE_SIZE: usize = 256; // The guest has made a buffer available to receive a frame into. @@ -315,175 +307,6 @@ impl NetCtrlEpollHandler { } } -#[derive(Clone)] -pub struct TxVirtio { - pub iovec: Vec<(GuestAddress, usize)>, - pub frame_buf: [u8; MAX_BUFFER_SIZE], - pub counter_bytes: Wrapping, - pub counter_frames: Wrapping, -} - -impl Default for TxVirtio { - fn default() -> Self { - Self::new() - } -} - -impl TxVirtio { - pub fn new() -> Self { - TxVirtio { - iovec: Vec::new(), - frame_buf: [0u8; MAX_BUFFER_SIZE], - counter_bytes: Wrapping(0), - counter_frames: Wrapping(0), - } - } - - pub fn process_desc_chain(&mut self, mem: &GuestMemoryMmap, tap: &mut Tap, queue: &mut Queue) { - while let Some(avail_desc) = queue.iter(&mem).next() { - let head_index = avail_desc.index; - let mut read_count = 0; - let mut next_desc = Some(avail_desc); - - self.iovec.clear(); - while let Some(desc) = next_desc { - if desc.is_write_only() { - break; - } - self.iovec.push((desc.addr, desc.len as usize)); - read_count += desc.len as usize; - next_desc = desc.next_descriptor(); - } - - read_count = 0; - // Copy buffer from across multiple descriptors. - // TODO(performance - Issue #420): change this to use `writev()` instead of `write()` - // and get rid of the intermediate buffer. - for (desc_addr, desc_len) in self.iovec.drain(..) { - let limit = cmp::min((read_count + desc_len) as usize, self.frame_buf.len()); - - let read_result = - mem.read_slice(&mut self.frame_buf[read_count..limit as usize], desc_addr); - match read_result { - Ok(_) => { - // Increment by number of bytes actually read - read_count += limit - read_count; - } - Err(e) => { - println!("Failed to read slice: {:?}", e); - break; - } - } - } - - let write_result = tap.write(&self.frame_buf[..read_count]); - match write_result { - Ok(_) => {} - Err(e) => { - println!("net: tx: error failed to write to tap: {}", e); - } - }; - - self.counter_bytes += Wrapping((read_count - vnet_hdr_len()) as u64); - self.counter_frames += Wrapping(1); - - queue.add_used(&mem, head_index, 0); - queue.update_avail_event(&mem); - } - } -} - -#[derive(Clone)] -pub struct RxVirtio { - pub deferred_frame: bool, - pub deferred_irqs: bool, - pub bytes_read: usize, - pub frame_buf: [u8; MAX_BUFFER_SIZE], - pub counter_bytes: Wrapping, - pub counter_frames: Wrapping, -} - -impl Default for RxVirtio { - fn default() -> Self { - Self::new() - } -} - -impl RxVirtio { - pub fn new() -> Self { - RxVirtio { - deferred_frame: false, - deferred_irqs: false, - bytes_read: 0, - frame_buf: [0u8; MAX_BUFFER_SIZE], - counter_bytes: Wrapping(0), - counter_frames: Wrapping(0), - } - } - - pub fn process_desc_chain( - &mut self, - mem: &GuestMemoryMmap, - mut next_desc: Option, - queue: &mut Queue, - ) -> bool { - let head_index = next_desc.as_ref().unwrap().index; - let mut write_count = 0; - - // Copy from frame into buffer, which may span multiple descriptors. - loop { - match next_desc { - Some(desc) => { - if !desc.is_write_only() { - break; - } - let limit = cmp::min(write_count + desc.len as usize, self.bytes_read); - let source_slice = &self.frame_buf[write_count..limit]; - let write_result = mem.write_slice(source_slice, desc.addr); - - match write_result { - Ok(_) => { - write_count = limit; - } - Err(e) => { - error!("Failed to write slice: {:?}", e); - break; - } - }; - - if write_count >= self.bytes_read { - break; - } - next_desc = desc.next_descriptor(); - } - None => { - warn!("Receiving buffer is too small to hold frame of current size"); - break; - } - } - } - - self.counter_bytes += Wrapping((write_count - vnet_hdr_len()) as u64); - self.counter_frames += Wrapping(1); - - queue.add_used(&mem, head_index, write_count as u32); - queue.update_avail_event(&mem); - - // Mark that we have at least one pending packet and we need to interrupt the guest. - self.deferred_irqs = true; - - // Update the frame_buf buffer. - if write_count < self.bytes_read { - self.frame_buf.copy_within(write_count..self.bytes_read, 0); - self.bytes_read -= write_count; - false - } else { - self.bytes_read = 0; - true - } - } -} - pub fn build_net_config_space( mut config: &mut VirtioNetConfig, mac: MacAddr, @@ -509,7 +332,3 @@ pub fn build_net_config_space_with_mq( *avail_features |= 1 << VIRTIO_NET_F_MQ; } } - -fn vnet_hdr_len() -> usize { - mem::size_of::() -}