From 4c023e341c86726814868a38d7d36bd851f6d52a Mon Sep 17 00:00:00 2001 From: Xuewei Niu Date: Thu, 28 Dec 2023 11:08:02 +0800 Subject: [PATCH] dragonball: Fix compilation issue without all net features Combinations of network features were tested: - None - virtio-net - vhost-net - vhost-user-net - virtio-net,vhost-net - vhost-net,vhost-user-net - virtio-net,vhost-user-net - virtio-net,vhost-net,vhost-user-net Fixes: #8742 Signed-off-by: Xuewei Niu --- src/dragonball/Cargo.lock | 1 + src/dragonball/src/api/v1/mod.rs | 6 +- .../src/dbs_virtio_devices/src/lib.rs | 12 ++ .../src/dbs_virtio_devices/src/net.rs | 181 +---------------- .../src/dbs_virtio_devices/src/net_common.rs | 190 ++++++++++++++++++ .../src/vhost/vhost_kern/net.rs | 7 +- .../src/vhost/vhost_user/net.rs | 6 +- 7 files changed, 217 insertions(+), 186 deletions(-) create mode 100644 src/dragonball/src/dbs_virtio_devices/src/net_common.rs diff --git a/src/dragonball/Cargo.lock b/src/dragonball/Cargo.lock index 7915c83cd4..14deba5e3c 100644 --- a/src/dragonball/Cargo.lock +++ b/src/dragonball/Cargo.lock @@ -398,6 +398,7 @@ dependencies = [ "serde_json", "thiserror", "threadpool", + "timerfd", "vhost", "virtio-bindings", "virtio-queue", diff --git a/src/dragonball/src/api/v1/mod.rs b/src/dragonball/src/api/v1/mod.rs index 99ffd689f9..043f1d30a3 100644 --- a/src/dragonball/src/api/v1/mod.rs +++ b/src/dragonball/src/api/v1/mod.rs @@ -25,6 +25,8 @@ pub use self::machine_config::{VmConfigError, MAX_SUPPORTED_VCPUS}; feature = "vhost-user-net" ))] mod virtio_net; +#[cfg(feature = "vhost-user-net")] +pub use virtio_net::VhostUserConfig; #[cfg(any(feature = "virtio-net", feature = "vhost-net"))] pub use virtio_net::VirtioConfig; #[cfg(any( @@ -32,6 +34,4 @@ pub use virtio_net::VirtioConfig; feature = "vhost-net", feature = "vhost-user-net" ))] -pub use virtio_net::{ - Backend, NetworkInterfaceConfig, NetworkInterfaceUpdateConfig, VhostUserConfig, -}; +pub use virtio_net::{Backend, NetworkInterfaceConfig, NetworkInterfaceUpdateConfig}; diff --git a/src/dragonball/src/dbs_virtio_devices/src/lib.rs b/src/dragonball/src/dbs_virtio_devices/src/lib.rs index 4fb48de87b..ebb9b58d37 100644 --- a/src/dragonball/src/dbs_virtio_devices/src/lib.rs +++ b/src/dragonball/src/dbs_virtio_devices/src/lib.rs @@ -28,6 +28,18 @@ pub mod vsock; #[cfg(feature = "virtio-net")] pub mod net; +#[cfg(any( + feature = "virtio-net", + feature = "vhost-net", + feature = "vhost-user-net" +))] +mod net_common; +#[cfg(any( + feature = "virtio-net", + feature = "vhost-net", + feature = "vhost-user-net" +))] +pub use net_common::*; #[cfg(feature = "virtio-blk")] pub mod block; diff --git a/src/dragonball/src/dbs_virtio_devices/src/net.rs b/src/dragonball/src/dbs_virtio_devices/src/net.rs index 8e09569cdd..45af943a2a 100644 --- a/src/dragonball/src/dbs_virtio_devices/src/net.rs +++ b/src/dragonball/src/dbs_virtio_devices/src/net.rs @@ -18,7 +18,7 @@ use dbs_utils::epoll_manager::{ EpollManager, EventOps, EventSet, Events, MutEventSubscriber, SubscriberId, }; use dbs_utils::metric::IncMetric; -use dbs_utils::net::{net_gen, MacAddr, Tap, MAC_ADDR_LEN}; +use dbs_utils::net::{net_gen, MacAddr, Tap}; use dbs_utils::rate_limiter::{BucketUpdate, RateLimiter, TokenType}; use libc; use log::{debug, error, info, trace, warn}; @@ -29,8 +29,9 @@ use vmm_sys_util::eventfd::EventFd; use crate::device::{VirtioDeviceConfig, VirtioDeviceInfo}; use crate::{ - vnet_hdr_len, ActivateError, ActivateResult, ConfigResult, DbsGuestAddressSpace, Error, - NetDeviceMetrics, Result, TapError, VirtioDevice, VirtioQueueConfig, TYPE_NET, + setup_config_space, vnet_hdr_len, ActivateError, ActivateResult, ConfigResult, + DbsGuestAddressSpace, Error, NetDeviceMetrics, Result, TapError, VirtioDevice, + VirtioQueueConfig, DEFAULT_MTU, TYPE_NET, }; const NET_DRIVER_NAME: &str = "virtio-net"; @@ -55,73 +56,6 @@ const PATCH_RATE_LIMITER_EVENT: u32 = 5; // Number of DeviceEventT events supported by this implementation. pub const NET_EVENTS_COUNT: u32 = 6; -// Config space of network config: -// https://docs.oasis-open.org/virtio/virtio/v1.1/csprd01/virtio-v1.1-csprd01.html#x1-2000004 -// MAC -const CONFIG_SPACE_MAC: usize = 0; -// Status -const CONFIG_SPACE_STATUS: usize = CONFIG_SPACE_MAC + MAC_ADDR_LEN; -const CONFIG_SPACE_STATUS_SIZE: usize = 2; -// Max virtqueue pairs -const CONFIG_SPACE_MAX_VIRTQUEUE_PAIRS: usize = CONFIG_SPACE_STATUS + CONFIG_SPACE_STATUS_SIZE; -const CONFIG_SPACE_MAX_VIRTQUEUE_PAIRS_SIZE: usize = 2; -// MTU -const CONFIG_SPACE_MTU: usize = - CONFIG_SPACE_MAX_VIRTQUEUE_PAIRS + CONFIG_SPACE_MAX_VIRTQUEUE_PAIRS_SIZE; -const CONFIG_SPACE_MTU_SIZE: usize = 2; -// Size of config space -const CONFIG_SPACE_SIZE: usize = MAC_ADDR_LEN - + CONFIG_SPACE_STATUS_SIZE - + CONFIG_SPACE_MAX_VIRTQUEUE_PAIRS_SIZE - + CONFIG_SPACE_MTU_SIZE; - -// Default MTU for network device -pub const DEFAULT_MTU: u16 = 1500; - -/// Setup config space for network device. -pub fn setup_config_space( - device_name: &str, - guest_mac: &Option<&MacAddr>, - avail_features: &mut u64, - vq_pairs: u16, - mtu: u16, -) -> Result> { - let mut config_space = vec![0u8; CONFIG_SPACE_SIZE]; - if let Some(mac) = guest_mac.as_ref() { - config_space[CONFIG_SPACE_MAC..CONFIG_SPACE_MAC + MAC_ADDR_LEN] - .copy_from_slice(mac.get_bytes()); - // When this feature isn't available, the driver generates a random MAC address. - // Otherwise, it should attempt to read the device MAC address from the config space. - *avail_features |= 1u64 << VIRTIO_NET_F_MAC; - } - - // Mark link as up: status only exists if VIRTIO_NET_F_STATUS is set. - if *avail_features & (1 << VIRTIO_NET_F_STATUS) != 0 { - config_space[CONFIG_SPACE_STATUS..CONFIG_SPACE_STATUS + CONFIG_SPACE_STATUS_SIZE] - .copy_from_slice(&(VIRTIO_NET_S_LINK_UP as u16).to_le_bytes()); - } - - // Set max virtqueue pairs, which only exists if VIRTIO_NET_F_MQ is set. - if *avail_features & (1 << VIRTIO_NET_F_MQ) != 0 { - if vq_pairs <= 1 { - return Err(Error::InvalidInput); - } - config_space[CONFIG_SPACE_MAX_VIRTQUEUE_PAIRS - ..CONFIG_SPACE_MAX_VIRTQUEUE_PAIRS + CONFIG_SPACE_MAX_VIRTQUEUE_PAIRS_SIZE] - .copy_from_slice(&vq_pairs.to_le_bytes()); - } - - config_space[CONFIG_SPACE_MTU..CONFIG_SPACE_MTU + CONFIG_SPACE_MTU_SIZE] - .copy_from_slice(&mtu.to_le_bytes()); - - debug!( - "{}: config space is set to {:X?}, guest_mac: {:?}, avail_feature: 0x{:X}, vq_pairs: {}, mtu: {}", - device_name, config_space, guest_mac, avail_features, vq_pairs, mtu - ); - - Ok(config_space) -} - /// Error for virtio-net devices to handle requests from guests. #[derive(Debug, thiserror::Error)] pub enum NetError { @@ -923,7 +857,7 @@ mod tests { use super::*; use crate::tests::{create_address_space, VirtQueue, VIRTQ_DESC_F_NEXT, VIRTQ_DESC_F_WRITE}; - use crate::ConfigError; + use crate::{ConfigError, CONFIG_SPACE_SIZE}; static NEXT_IP: AtomicUsize = AtomicUsize::new(1); @@ -1471,109 +1405,4 @@ mod tests { assert!(!handler.rx.rate_limiter.is_blocked()); } } - - #[test] - fn test_set_config_space() { - let mac = MacAddr::parse_str("bf:b7:72:50:82:00").unwrap(); - let mut afeatures: u64; - let mut vq_pairs: u16; - // Avail features: VIRTIO_NET_F_STATUS + VIRTIO_NET_F_MQ - { - afeatures = 0; - vq_pairs = 2; - afeatures |= 1 << VIRTIO_NET_F_STATUS | 1 << VIRTIO_NET_F_MQ; - - let cs = setup_config_space( - "virtio-net", - &Some(&mac), - &mut afeatures, - vq_pairs, - DEFAULT_MTU, - ) - .unwrap(); - - // Mac - assert_eq!( - mac.get_bytes(), - &cs[CONFIG_SPACE_MAC..CONFIG_SPACE_MAC + MAC_ADDR_LEN] - ); - // Status - assert_eq!( - VIRTIO_NET_S_LINK_UP as u16, - u16::from_le_bytes( - cs[CONFIG_SPACE_STATUS..CONFIG_SPACE_STATUS + CONFIG_SPACE_STATUS_SIZE] - .try_into() - .unwrap() - ) - ); - // Max virtqueue pairs - assert_eq!( - vq_pairs, - u16::from_le_bytes( - cs[CONFIG_SPACE_MAX_VIRTQUEUE_PAIRS - ..CONFIG_SPACE_MAX_VIRTQUEUE_PAIRS + CONFIG_SPACE_MAX_VIRTQUEUE_PAIRS_SIZE] - .try_into() - .unwrap() - ) - ); - // MTU - assert_eq!( - DEFAULT_MTU, - u16::from_le_bytes( - cs[CONFIG_SPACE_MTU..CONFIG_SPACE_MTU + CONFIG_SPACE_MTU_SIZE] - .try_into() - .unwrap() - ) - ); - } - // No avail features - { - afeatures = 0; - vq_pairs = 1; - - let cs = setup_config_space( - "virtio-net", - &Some(&mac), - &mut afeatures, - vq_pairs, - DEFAULT_MTU, - ) - .unwrap(); - - // Status - assert_eq!( - 0, - u16::from_le_bytes( - cs[CONFIG_SPACE_STATUS..CONFIG_SPACE_STATUS + CONFIG_SPACE_STATUS_SIZE] - .try_into() - .unwrap() - ) - ); - // Max virtqueue pairs - assert_eq!( - 0, - u16::from_le_bytes( - cs[CONFIG_SPACE_MAX_VIRTQUEUE_PAIRS - ..CONFIG_SPACE_MAX_VIRTQUEUE_PAIRS + CONFIG_SPACE_MAX_VIRTQUEUE_PAIRS_SIZE] - .try_into() - .unwrap() - ) - ); - } - // Avail features: VIRTIO_NET_F_MQ and invalid value of vq_pairs - { - afeatures = 0; - vq_pairs = 1; - afeatures |= 1 << VIRTIO_NET_F_MQ; - - let cs = setup_config_space( - "virtio-net", - &Some(&mac), - &mut afeatures, - vq_pairs, - DEFAULT_MTU, - ); - assert!(cs.is_err()); - } - } } diff --git a/src/dragonball/src/dbs_virtio_devices/src/net_common.rs b/src/dragonball/src/dbs_virtio_devices/src/net_common.rs new file mode 100644 index 0000000000..83ab43f65d --- /dev/null +++ b/src/dragonball/src/dbs_virtio_devices/src/net_common.rs @@ -0,0 +1,190 @@ +// Copyright (C) 2019-2023 Ant Group. All rights reserved. +// +// SPDX-License-Identifier: Apache-2.0 + +use dbs_utils::net::{MacAddr, MAC_ADDR_LEN}; +use log::debug; +use virtio_bindings::bindings::virtio_net::{ + VIRTIO_NET_F_MAC, VIRTIO_NET_F_MQ, VIRTIO_NET_F_STATUS, VIRTIO_NET_S_LINK_UP, +}; + +use crate::{Error, Result}; + +// Config space of network config: +// https://docs.oasis-open.org/virtio/virtio/v1.1/csprd01/virtio-v1.1-csprd01.html#x1-2000004 +// MAC +pub const CONFIG_SPACE_MAC: usize = 0; +// Status +pub const CONFIG_SPACE_STATUS: usize = CONFIG_SPACE_MAC + MAC_ADDR_LEN; +pub const CONFIG_SPACE_STATUS_SIZE: usize = 2; +// Max virtqueue pairs +pub const CONFIG_SPACE_MAX_VIRTQUEUE_PAIRS: usize = CONFIG_SPACE_STATUS + CONFIG_SPACE_STATUS_SIZE; +pub const CONFIG_SPACE_MAX_VIRTQUEUE_PAIRS_SIZE: usize = 2; +// MTU +pub const CONFIG_SPACE_MTU: usize = + CONFIG_SPACE_MAX_VIRTQUEUE_PAIRS + CONFIG_SPACE_MAX_VIRTQUEUE_PAIRS_SIZE; +pub const CONFIG_SPACE_MTU_SIZE: usize = 2; +// Size of config space +pub const CONFIG_SPACE_SIZE: usize = MAC_ADDR_LEN + + CONFIG_SPACE_STATUS_SIZE + + CONFIG_SPACE_MAX_VIRTQUEUE_PAIRS_SIZE + + CONFIG_SPACE_MTU_SIZE; + +// Default MTU for network device +pub const DEFAULT_MTU: u16 = 1500; + +/// Setup config space for network device. +pub fn setup_config_space( + device_name: &str, + guest_mac: &Option<&MacAddr>, + avail_features: &mut u64, + vq_pairs: u16, + mtu: u16, +) -> Result> { + let mut config_space = vec![0u8; CONFIG_SPACE_SIZE]; + if let Some(mac) = guest_mac.as_ref() { + config_space[CONFIG_SPACE_MAC..CONFIG_SPACE_MAC + MAC_ADDR_LEN] + .copy_from_slice(mac.get_bytes()); + // When this feature isn't available, the driver generates a random MAC address. + // Otherwise, it should attempt to read the device MAC address from the config space. + *avail_features |= 1u64 << VIRTIO_NET_F_MAC; + } + + // Mark link as up: status only exists if VIRTIO_NET_F_STATUS is set. + if *avail_features & (1 << VIRTIO_NET_F_STATUS) != 0 { + config_space[CONFIG_SPACE_STATUS..CONFIG_SPACE_STATUS + CONFIG_SPACE_STATUS_SIZE] + .copy_from_slice(&(VIRTIO_NET_S_LINK_UP as u16).to_le_bytes()); + } + + // Set max virtqueue pairs, which only exists if VIRTIO_NET_F_MQ is set. + if *avail_features & (1 << VIRTIO_NET_F_MQ) != 0 { + if vq_pairs <= 1 { + return Err(Error::InvalidInput); + } + config_space[CONFIG_SPACE_MAX_VIRTQUEUE_PAIRS + ..CONFIG_SPACE_MAX_VIRTQUEUE_PAIRS + CONFIG_SPACE_MAX_VIRTQUEUE_PAIRS_SIZE] + .copy_from_slice(&vq_pairs.to_le_bytes()); + } + + config_space[CONFIG_SPACE_MTU..CONFIG_SPACE_MTU + CONFIG_SPACE_MTU_SIZE] + .copy_from_slice(&mtu.to_le_bytes()); + + debug!( + "{}: config space is set to {:X?}, guest_mac: {:?}, avail_feature: 0x{:X}, vq_pairs: {}, mtu: {}", + device_name, config_space, guest_mac, avail_features, vq_pairs, mtu + ); + + Ok(config_space) +} + +#[cfg(test)] +mod tests { + use std::convert::TryInto; + + use super::*; + + #[test] + fn test_set_config_space() { + let mac = MacAddr::parse_str("bf:b7:72:50:82:00").unwrap(); + let mut afeatures: u64; + let mut vq_pairs: u16; + // Avail features: VIRTIO_NET_F_STATUS + VIRTIO_NET_F_MQ + { + afeatures = 0; + vq_pairs = 2; + afeatures |= 1 << VIRTIO_NET_F_STATUS | 1 << VIRTIO_NET_F_MQ; + + let cs = setup_config_space( + "virtio-net", + &Some(&mac), + &mut afeatures, + vq_pairs, + DEFAULT_MTU, + ) + .unwrap(); + + // Mac + assert_eq!( + mac.get_bytes(), + &cs[CONFIG_SPACE_MAC..CONFIG_SPACE_MAC + MAC_ADDR_LEN] + ); + // Status + assert_eq!( + VIRTIO_NET_S_LINK_UP as u16, + u16::from_le_bytes( + cs[CONFIG_SPACE_STATUS..CONFIG_SPACE_STATUS + CONFIG_SPACE_STATUS_SIZE] + .try_into() + .unwrap() + ) + ); + // Max virtqueue pairs + assert_eq!( + vq_pairs, + u16::from_le_bytes( + cs[CONFIG_SPACE_MAX_VIRTQUEUE_PAIRS + ..CONFIG_SPACE_MAX_VIRTQUEUE_PAIRS + CONFIG_SPACE_MAX_VIRTQUEUE_PAIRS_SIZE] + .try_into() + .unwrap() + ) + ); + // MTU + assert_eq!( + DEFAULT_MTU, + u16::from_le_bytes( + cs[CONFIG_SPACE_MTU..CONFIG_SPACE_MTU + CONFIG_SPACE_MTU_SIZE] + .try_into() + .unwrap() + ) + ); + } + // No avail features + { + afeatures = 0; + vq_pairs = 1; + + let cs = setup_config_space( + "virtio-net", + &Some(&mac), + &mut afeatures, + vq_pairs, + DEFAULT_MTU, + ) + .unwrap(); + + // Status + assert_eq!( + 0, + u16::from_le_bytes( + cs[CONFIG_SPACE_STATUS..CONFIG_SPACE_STATUS + CONFIG_SPACE_STATUS_SIZE] + .try_into() + .unwrap() + ) + ); + // Max virtqueue pairs + assert_eq!( + 0, + u16::from_le_bytes( + cs[CONFIG_SPACE_MAX_VIRTQUEUE_PAIRS + ..CONFIG_SPACE_MAX_VIRTQUEUE_PAIRS + CONFIG_SPACE_MAX_VIRTQUEUE_PAIRS_SIZE] + .try_into() + .unwrap() + ) + ); + } + // Avail features: VIRTIO_NET_F_MQ and invalid value of vq_pairs + { + afeatures = 0; + vq_pairs = 1; + afeatures |= 1 << VIRTIO_NET_F_MQ; + + let cs = setup_config_space( + "virtio-net", + &Some(&mac), + &mut afeatures, + vq_pairs, + DEFAULT_MTU, + ); + assert!(cs.is_err()); + } + } +} diff --git a/src/dragonball/src/dbs_virtio_devices/src/vhost/vhost_kern/net.rs b/src/dragonball/src/dbs_virtio_devices/src/vhost/vhost_kern/net.rs index fe832e7f41..6290d2b6d7 100644 --- a/src/dragonball/src/dbs_virtio_devices/src/vhost/vhost_kern/net.rs +++ b/src/dragonball/src/dbs_virtio_devices/src/vhost/vhost_kern/net.rs @@ -31,16 +31,15 @@ use virtio_bindings::bindings::virtio_ring::*; use virtio_queue::{DescriptorChain, QueueT}; use vm_memory::{Address, GuestMemory, GuestMemoryRegion, MemoryRegionAddress}; -use crate::net::{setup_config_space, DEFAULT_MTU}; use crate::vhost::net::{virtio_handle_ctrl_mq, virtio_handle_ctrl_status, FromNetCtrl}; #[cfg(test)] use crate::vhost::vhost_kern::test_utils::{ MockVhostBackend as VhostBackend, MockVhostNet as VhostNet, }; use crate::{ - vnet_hdr_len, ActivateError, ConfigResult, DbsGuestAddressSpace, Error as VirtioError, - NetDeviceMetrics, Result as VirtioResult, TapError, VirtioDevice, VirtioDeviceConfig, - VirtioDeviceInfo, TYPE_NET, + setup_config_space, vnet_hdr_len, ActivateError, ConfigResult, DbsGuestAddressSpace, + Error as VirtioError, NetDeviceMetrics, Result as VirtioResult, TapError, VirtioDevice, + VirtioDeviceConfig, VirtioDeviceInfo, DEFAULT_MTU, TYPE_NET, }; const NET_DRIVER_NAME: &str = "vhost-net"; diff --git a/src/dragonball/src/dbs_virtio_devices/src/vhost/vhost_user/net.rs b/src/dragonball/src/dbs_virtio_devices/src/vhost/vhost_user/net.rs index d1c907c4f5..4342c13088 100644 --- a/src/dragonball/src/dbs_virtio_devices/src/vhost/vhost_user/net.rs +++ b/src/dragonball/src/dbs_virtio_devices/src/vhost/vhost_user/net.rs @@ -25,12 +25,12 @@ use vm_memory::GuestMemoryRegion; use vmm_sys_util::epoll::EventSet; use super::connection::{Endpoint, Listener}; -use crate::net::{setup_config_space, DEFAULT_MTU}; use crate::vhost::net::{virtio_handle_ctrl_mq, virtio_handle_ctrl_status, FromNetCtrl}; use crate::vhost::vhost_user::connection::EndpointParam; use crate::{ - ActivateResult, ConfigResult, DbsGuestAddressSpace, Error as VirtioError, - Result as VirtioResult, VirtioDevice, VirtioDeviceConfig, VirtioDeviceInfo, TYPE_NET, + setup_config_space, ActivateResult, ConfigResult, DbsGuestAddressSpace, Error as VirtioError, + Result as VirtioResult, VirtioDevice, VirtioDeviceConfig, VirtioDeviceInfo, DEFAULT_MTU, + TYPE_NET, }; const NET_DRIVER_NAME: &str = "vhost-user-net";