dragonball: Changes for ZizhengBian's comments

- Dragonball's vhost-net feature not depends on virtio-net feature. - Remove `TapError` from dbs-virtio-devices's Error, and add `VirtioNet` and `VhostNet` two fields. - Downgrade visiblity of two fields of `VhostNetDeviceMgr` from `pub(crate)`. - File an issue to record a todo for network rate limiter. - Print internal errors with `{0:?}. Signed-off-by: Xuewei Niu <niuxuewei.nxw@antgroup.com>
2025-09-13 12:59:36 +00:00 · 2023-10-27 16:43:58 +08:00
parent 8ea87405ed
commit 58e9709c1f
12 changed files with 106 additions and 58 deletions
--- a/src/dragonball/Cargo.lock
+++ b/src/dragonball/Cargo.lock
@@ -396,6 +396,7 @@ dependencies = [
 "serde_json",
 "thiserror",
 "threadpool",
+ "vhost",
 "virtio-bindings",
 "virtio-queue",
 "vm-memory",
@@ -2071,6 +2072,18 @@ version = "0.9.4"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "49874b5167b65d7193b8aba1567f5c7d93d001cafc34600cee003eda787e483f"

+[[package]]
+name = "vhost"
+version = "0.6.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "a6769e8dbf5276b4376439fbf36bb880d203bf614bf7ef444198edc24b5a9f35"
+dependencies = [
+ "bitflags 1.3.2",
+ "libc",
+ "vm-memory",
+ "vmm-sys-util",
+]
+
 [[package]]
 name = "virtio-bindings"
 version = "0.1.0"
--- a/src/dragonball/Cargo.toml
+++ b/src/dragonball/Cargo.toml
@@ -63,4 +63,4 @@ virtio-net = ["dbs-virtio-devices/virtio-net", "virtio-queue"]
 virtio-fs = ["dbs-virtio-devices/virtio-fs-pro", "virtio-queue", "atomic-guest-memory"]
 virtio-mem = ["dbs-virtio-devices/virtio-mem", "virtio-queue", "atomic-guest-memory"]
 virtio-balloon = ["dbs-virtio-devices/virtio-balloon", "virtio-queue"]
-vhost-net = ["virtio-net", "dbs-virtio-devices/vhost-net"]
+vhost-net = ["dbs-virtio-devices/vhost-net"]
--- a/src/dragonball/src/api/v1/mod.rs
+++ b/src/dragonball/src/api/v1/mod.rs
@@ -19,5 +19,7 @@ mod machine_config;
 pub use self::machine_config::{VmConfigError, MAX_SUPPORTED_VCPUS};

 /// Wrapper for configuring the virtio networking
+#[cfg(any(feature = "virtio-net", feature = "vhost-net"))]
 mod virtio_net;
+#[cfg(any(feature = "virtio-net", feature = "vhost-net"))]
 pub use virtio_net::{Backend, NetworkInterfaceConfig, NetworkInterfaceUpdateConfig, VirtioConfig};
--- a/src/dragonball/src/api/v1/vmm_action.rs
+++ b/src/dragonball/src/api/v1/vmm_action.rs
@@ -106,7 +106,7 @@ pub enum VmmActionError {
    VirtioNet(#[source] VirtioNetDeviceError),

    #[cfg(feature = "vhost-net")]
-    #[error("vhost-net device error: {0}")]
+    #[error("vhost-net device error: {0:?}")]
    /// Vhost-net device relared errors.
    VhostNet(#[source] VhostNetDeviceError),

@@ -318,7 +318,7 @@ impl VmmService {
            VmmAction::RemoveBlockDevice(drive_id) => {
                self.remove_block_device(vmm, event_mgr, &drive_id)
            }
-            #[cfg(feature = "virtio-net")]
+            #[cfg(any(feature = "virtio-net", feature = "vhost-net"))]
            VmmAction::InsertNetworkDevice(config) => match config.backend {
                Backend::Virtio(_) => {
                    #[cfg(not(feature = "virtio-net"))]
--- a/src/dragonball/src/dbs_virtio_devices/src/lib.rs
+++ b/src/dragonball/src/dbs_virtio_devices/src/lib.rs
@@ -46,6 +46,7 @@ pub mod vhost;

 use std::io::Error as IOError;

+use net::NetError;
 use virtio_queue::Error as VqError;
 use vm_memory::{GuestAddress, GuestAddressSpace, GuestMemoryError};

@@ -209,14 +210,14 @@ pub enum Error {
    #[error("virtio-fs error: {0}")]
    VirtioFs(fs::Error),

-    #[cfg(feature = "vhost")]
-    #[error("vhost error: {0}")]
-    /// Error from vhost-net.
-    VhostNet(#[from] vhost_rs::Error),
-
    #[cfg(feature = "virtio-net")]
-    #[error("tap device operation error: {0}")]
-    TapDeviceError(#[source] TapError),
+    #[error("virtio-net error: {0:?}")]
+    VirtioNet(NetError),
+
+    #[cfg(feature = "vhost-net")]
+    #[error("vhost-net error: {0:?}")]
+    /// Error from vhost-net.
+    VhostNet(vhost::vhost_kern::net::Error),

    #[cfg(feature = "virtio-mem")]
    #[error("Virtio-mem error: {0}")]
@@ -234,7 +235,7 @@ pub enum TapError {
    #[error("missing {0} flags")]
    MissingFlags(String),

-    #[error("failed to set offload: {0}")]
+    #[error("failed to set offload: {0:?}")]
    SetOffload(#[source] dbs_utils::net::TapError),

    #[error("failed to set vnet_hdr_size: {0}")]
--- a/src/dragonball/src/dbs_virtio_devices/src/net.rs
+++ b/src/dragonball/src/dbs_virtio_devices/src/net.rs
@@ -31,8 +31,8 @@ use vmm_sys_util::eventfd::EventFd;

 use crate::device::{VirtioDeviceConfig, VirtioDeviceInfo};
 use crate::{
-    ActivateError, ActivateResult, ConfigResult, DbsGuestAddressSpace, Error, Result, VirtioDevice,
-    VirtioQueueConfig, TYPE_NET,
+    ActivateError, ActivateResult, ConfigResult, DbsGuestAddressSpace, Error, Result, TapError,
+    VirtioDevice, VirtioQueueConfig, TYPE_NET,
 };

 const NET_DRIVER_NAME: &str = "virtio-net";
@@ -57,6 +57,13 @@ const PATCH_RATE_LIMITER_EVENT: u32 = 5;
 // Number of DeviceEventT events supported by this implementation.
 pub const NET_EVENTS_COUNT: u32 = 6;

+/// Error for virtio-net devices to handle requests from guests.
+#[derive(Debug, thiserror::Error)]
+pub enum NetError {
+    #[error("tap device operation error: {0:?}")]
+    TapError(#[source] TapError),
+}
+
 /// Metrics specific to the net device.
 #[derive(Default, Serialize)]
 pub struct NetDeviceMetrics {
@@ -651,11 +658,11 @@ impl<AS: GuestAddressSpace> Net<AS> {
        tap.set_offload(
            net_gen::TUN_F_CSUM | net_gen::TUN_F_UFO | net_gen::TUN_F_TSO4 | net_gen::TUN_F_TSO6,
        )
-        .map_err(|err| Error::TapDeviceError(crate::TapError::SetOffload(err)))?;
+        .map_err(|err| Error::VirtioNet(NetError::TapError(TapError::SetOffload(err))))?;

        let vnet_hdr_size = vnet_hdr_len() as i32;
        tap.set_vnet_hdr_size(vnet_hdr_size)
-            .map_err(|err| Error::TapDeviceError(crate::TapError::SetVnetHdrSize(err)))?;
+            .map_err(|err| Error::VirtioNet(NetError::TapError(TapError::SetVnetHdrSize(err))))?;
        info!("net tap set finished");

        let mut avail_features = 1u64 << VIRTIO_NET_F_GUEST_CSUM
@@ -709,7 +716,7 @@ impl<AS: GuestAddressSpace> Net<AS> {
    ) -> Result<Self> {
        info!("open net tap {}", host_dev_name);
        let tap = Tap::open_named(host_dev_name.as_str(), false)
-            .map_err(|err| Error::TapDeviceError(crate::TapError::Open(err)))?;
+            .map_err(|err| Error::VirtioNet(NetError::TapError(TapError::Open(err))))?;
        info!("net tap opened");

        Self::new_with_tap(
--- 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,7 +31,7 @@ use virtio_bindings::bindings::virtio_ring::*;
 use virtio_queue::{Descriptor, DescriptorChain, QueueT};
 use vm_memory::{Address, Bytes, GuestMemory, GuestMemoryRegion, MemoryRegionAddress};

-use crate::net::{NetDeviceMetrics, vnet_hdr_len};
+use crate::net::{vnet_hdr_len, NetDeviceMetrics};
 #[cfg(test)]
 use crate::vhost::vhost_kern::test_utils::{
    MockVhostBackend as VhostBackend, MockVhostNet as VhostNet,
@@ -47,6 +47,15 @@ const CTRL_SLOT: u32 = 0;
 // Control queue size
 const CTRL_QUEUE_SIZE: u16 = 64;

+/// Error for vhost-net devices to handle requests from guests.
+#[derive(Debug, thiserror::Error)]
+pub enum Error {
+    #[error("tap device operation error: {0:?}")]
+    TapError(#[source] TapError),
+    #[error("vhost error: {0}")]
+    VhostError(#[source] vhost_rs::Error),
+}
+
 /// Vhost-net device implementation
 pub struct Net<AS, Q, R>
 where
@@ -97,12 +106,14 @@ fn validate_and_configure_tap(tap: &Tap, vq_pairs: usize) -> VirtioResult<()> {
        )
        .collect::<Vec<_>>();
    if !missing_flags.is_empty() {
-        return Err(VirtioError::TapDeviceError(TapError::MissingFlags(
-            missing_flags
-                .into_iter()
-                .map(|flag| *flag)
-                .collect::<Vec<&str>>()
-                .join(", "),
+        return Err(VirtioError::VhostNet(Error::TapError(
+            TapError::MissingFlags(
+                missing_flags
+                    .into_iter()
+                    .map(|flag| *flag)
+                    .collect::<Vec<&str>>()
+                    .join(", "),
+            ),
        )));
    }

@@ -110,11 +121,11 @@ fn validate_and_configure_tap(tap: &Tap, vq_pairs: usize) -> VirtioResult<()> {
    tap.set_offload(
        net_gen::TUN_F_CSUM | net_gen::TUN_F_UFO | net_gen::TUN_F_TSO4 | net_gen::TUN_F_TSO6,
    )
-    .map_err(|err| VirtioError::TapDeviceError(TapError::SetOffload(err)))?;
+    .map_err(|err| VirtioError::VhostNet(Error::TapError(TapError::SetOffload(err))))?;

    let vnet_hdr_size = vnet_hdr_len() as i32;
    tap.set_vnet_hdr_size(vnet_hdr_size)
-        .map_err(|err| VirtioError::TapDeviceError(TapError::SetVnetHdrSize(err)))?;
+        .map_err(|err| VirtioError::VhostNet(Error::TapError(TapError::SetVnetHdrSize(err))))?;

    Ok(())
 }
@@ -137,7 +148,7 @@ where

        let taps = tap
            .into_mq_taps(vq_pairs)
-            .map_err(|err| VirtioError::TapDeviceError(TapError::Open(err)))?;
+            .map_err(|err| VirtioError::VhostNet(Error::TapError(TapError::Open(err))))?;
        for tap in taps.iter() {
            validate_and_configure_tap(tap, vq_pairs)?;
        }
@@ -234,9 +245,9 @@ where
    ) -> VirtioResult<Self> {
        // Open a TAP interface
        let tap = Tap::open_named(&host_dev_name, vq_pairs > 1)
-            .map_err(|err| VirtioError::TapDeviceError(TapError::Open(err)))?;
+            .map_err(|err| VirtioError::VhostNet(Error::TapError(TapError::Open(err))))?;
        tap.enable()
-            .map_err(|err| VirtioError::TapDeviceError(TapError::Enable(err)))?;
+            .map_err(|err| VirtioError::VhostNet(Error::TapError(TapError::Enable(err))))?;

        Self::new_with_tap(tap, vq_pairs, guest_mac, queue_sizes, event_mgr)
    }
@@ -255,8 +266,10 @@ where

        if self.handles.is_empty() {
            for _ in 0..vq_pairs {
-                self.handles
-                    .push(VhostNet::<AS>::new(config.vm_as.clone()).map_err(VirtioError::VhostNet)?);
+                self.handles.push(
+                    VhostNet::<AS>::new(config.vm_as.clone())
+                        .map_err(|err| VirtioError::VhostNet(Error::VhostError(err)))?,
+                );
            }
        }

@@ -317,11 +330,17 @@ where
        trace!(target: "vhost-net", "{}: Net::init_vhost_dev(pair_index: {})", NET_DRIVER_NAME, pair_index);

        let handle = &mut self.handles[pair_index];
-        handle.set_owner().map_err(VirtioError::VhostNet)?;
+        handle
+            .set_owner()
+            .map_err(|err| VirtioError::VhostNet(Error::VhostError(err)))?;

-        let avail_features = handle.get_features().map_err(VirtioError::VhostNet)?;
+        let avail_features = handle
+            .get_features()
+            .map_err(|err| VirtioError::VhostNet(Error::VhostError(err)))?;
        let features = self.device_info.acked_features() & avail_features;
-        handle.set_features(features).map_err(VirtioError::VhostNet)?;
+        handle
+            .set_features(features)
+            .map_err(|err| VirtioError::VhostNet(Error::VhostError(err)))?;

        let mut regions = Vec::new();
        for region in mem.iter() {
@@ -335,12 +354,14 @@ where
                guest_phys_addr: guest_phys_addr.raw_value(),
                memory_size: region.len(),
                userspace_addr: userspace_addr as *const u8 as u64,
-                mmap_offset: 0u64,
-                mmap_handle: -1i32,
+                mmap_offset: 0,
+                mmap_handle: -1,
            });
        }

-        handle.set_mem_table(&regions).map_err(VirtioError::VhostNet)?;
+        handle
+            .set_mem_table(&regions)
+            .map_err(|err| VirtioError::VhostNet(Error::VhostError(err)))?;

        self.init_vhost_queues(pair_index, config)?;

@@ -376,7 +397,7 @@ where

            handle
                .set_vring_num(vq_index, queue_cfg.queue.size())
-                .map_err(VirtioError::VhostNet)?;
+                .map_err(|err| VirtioError::VhostNet(Error::VhostError(err)))?;

            if let Some(vring_base) = &self.kernel_vring_bases {
                let base = if vq_index == 0 {
@@ -386,11 +407,11 @@ where
                };
                handle
                    .set_vring_base(vq_index, base as u16)
-                    .map_err(VirtioError::VhostNet)?;
+                    .map_err(|err| VirtioError::VhostNet(Error::VhostError(err)))?;
            } else {
                handle
                    .set_vring_base(vq_index, 0)
-                    .map_err(VirtioError::VhostNet)?;
+                    .map_err(|err| VirtioError::VhostNet(Error::VhostError(err)))?;
            }

            let config_data = &VringConfigData {
@@ -405,19 +426,19 @@ where

            handle
                .set_vring_addr(vq_index, config_data)
-                .map_err(VirtioError::VhostNet)?;
+                .map_err(|err| VirtioError::VhostNet(Error::VhostError(err)))?;

            handle
                .set_vring_call(vq_index, intr_evts[queue_index])
-                .map_err(VirtioError::VhostNet)?;
+                .map_err(|err| VirtioError::VhostNet(Error::VhostError(err)))?;

            handle
                .set_vring_kick(vq_index, &queue_cfg.eventfd)
-                .map_err(VirtioError::VhostNet)?;
+                .map_err(|err| VirtioError::VhostNet(Error::VhostError(err)))?;

            handle
                .set_backend(vq_index, Some(&tap.tap_file))
-                .map_err(VirtioError::VhostNet)?;
+                .map_err(|err| VirtioError::VhostNet(Error::VhostError(err)))?;
        }

        Ok(())
--- a/src/dragonball/src/device_manager/mod.rs
+++ b/src/dragonball/src/device_manager/mod.rs
@@ -545,7 +545,7 @@ pub struct DeviceManager {
    pub(crate) balloon_manager: BalloonDeviceMgr,

    #[cfg(feature = "vhost-net")]
-    pub(crate) vhost_net_manager: VhostNetDeviceMgr,
+    vhost_net_manager: VhostNetDeviceMgr,
 }

 impl DeviceManager {
--- a/src/dragonball/src/device_manager/vhost_net_dev_mgr.rs
+++ b/src/dragonball/src/device_manager/vhost_net_dev_mgr.rs
@@ -3,18 +3,18 @@
 //
 // SPDX-License-Identifier: Apache-2.0

+use std::result::Result;
+use std::sync::Arc;
+
 use dbs_utils::net::MacAddr;
-use dbs_virtio_devices::{vhost::vhost_kern::net::Net, Error as VirtioError};
+use dbs_virtio_devices::vhost::vhost_kern::net::Net;
+use dbs_virtio_devices::Error as VirtioError;
 use serde::{Deserialize, Serialize};
-use std::{result::Result, sync::Arc};
 use virtio_queue::QueueSync;

-use crate::{
-    address_space_manager::{GuestAddressSpaceImpl, GuestRegionImpl},
-    config_manager::{ConfigItem, DeviceConfigInfos},
-};
-
 use super::{DeviceManager, DeviceMgrError, DeviceOpContext};
+use crate::address_space_manager::{GuestAddressSpaceImpl, GuestRegionImpl};
+use crate::config_manager::{ConfigItem, DeviceConfigInfos};

 /// Default number of virtio queues, one rx/tx pair.
 pub const NUM_QUEUES: usize = 2;
@@ -44,10 +44,10 @@ pub enum VhostNetDeviceError {
    #[error("invalid queue number {0} for vhost-net device")]
    InvalidQueueNum(usize),
    /// Failure from device manager.
-    #[error("failure in device manager operations: {0}")]
+    #[error("failure in device manager operations: {0:?}")]
    DeviceManager(#[source] DeviceMgrError),
    /// Failure from virtio subsystem.
-    #[error(transparent)]
+    #[error("virtio error: {0:?}")]
    Virtio(VirtioError),
    /// Split this at some point.
    /// Internal errors are due to resource exhaustion.
@@ -60,6 +60,7 @@ pub enum VhostNetDeviceError {
 }

 /// Configuration information for vhost net devices.
+/// TODO: https://github.com/kata-containers/kata-containers/issues/8382.
 #[derive(Clone, Debug, Deserialize, PartialEq, Serialize)]
 pub struct VhostNetDeviceConfigInfo {
    /// Id of the guest network interface.
@@ -138,8 +139,8 @@ impl ConfigItem for VhostNetDeviceConfigInfo {

 /// Device manager to manage all vhost net devices.
 pub struct VhostNetDeviceMgr {
-    pub(crate) info_list: DeviceConfigInfos<VhostNetDeviceConfigInfo>,
-    pub(crate) use_shared_irq: bool,
+    info_list: DeviceConfigInfos<VhostNetDeviceConfigInfo>,
+    use_shared_irq: bool,
 }

 impl VhostNetDeviceMgr {
--- a/src/dragonball/src/device_manager/virtio_net_dev_mgr.rs
+++ b/src/dragonball/src/device_manager/virtio_net_dev_mgr.rs
@@ -123,6 +123,7 @@ impl VirtioNetDeviceConfigUpdateInfo {
 }

 /// Configuration information for virtio net devices.
+/// TODO: https://github.com/kata-containers/kata-containers/issues/8382.
 #[derive(Clone, Debug, Deserialize, PartialEq, Eq, Serialize, Default)]
 pub struct VirtioNetDeviceConfigInfo {
    /// ID of the guest network interface.
--- a/src/dragonball/src/error.rs
+++ b/src/dragonball/src/error.rs
@@ -196,7 +196,7 @@ pub enum StartMicroVmError {

    /// Vhost-net device errors.
    #[cfg(feature = "vhost-net")]
-    #[error("vhost-net errors: {0}")]
+    #[error("vhost-net errors: {0:?}")]
    VhostNetDeviceError(#[source] device_manager::vhost_net_dev_mgr::VhostNetDeviceError),
 }

--- a/src/runtime-rs/crates/hypervisor/src/dragonball/mod.rs
+++ b/src/runtime-rs/crates/hypervisor/src/dragonball/mod.rs
@@ -208,9 +208,11 @@ impl From<&NetworkConfig> for DragonballNetworkConfig {
        let virtio_config = DragonballVirtioConfig {
            iface_id: value.virt_iface_name.clone(),
            host_dev_name: value.host_dev_name.clone(),
-            // TODO(justxuewei): rx_rate_limiter is not supported.
+            // TODO(justxuewei): rx_rate_limiter is not supported, see:
+            // https://github.com/kata-containers/kata-containers/issues/8327.
            rx_rate_limiter: None,
-            // TODO(justxuewei): tx_rate_limiter is not supported.
+            // TODO(justxuewei): tx_rate_limiter is not supported, see:
+            // https://github.com/kata-containers/kata-containers/issues/8327.
            tx_rate_limiter: None,
            allow_duplicate_mac: value.allow_duplicate_mac,
        };