dbs-virtio-devices: add metrics for net device

crates/dbs-virtio-devices/src/balloon.rs

@@ -39,7 +39,9 @@
 };
 
 use crate::device::{VirtioDevice, VirtioDeviceConfig, VirtioDeviceInfo, VirtioQueueConfig};
-use crate::{ActivateResult, DbsGuestAddressSpace, Error, Result, TYPE_BALLOON};
+use crate::{
+    ActivateResult, ConfigError, ConfigResult, DbsGuestAddressSpace, Error, Result, TYPE_BALLOON,
+};
 
 const BALLOON_DRIVER_NAME: &str = "virtio-balloon";
 
@@ -425,7 +427,7 @@
     fn init(&mut self, ops: &mut EventOps) {
         trace!(
             target: BALLOON_DRIVER_NAME,
             "{}: BalloonEpollHandler::init()",
             BALLOON_DRIVER_NAME,
         );
         let events = Events::with_data(
@@ -474,7 +476,7 @@
 
         trace!(
             target: BALLOON_DRIVER_NAME,
             "{}: BalloonEpollHandler::process() idx {}",
             BALLOON_DRIVER_NAME,
             idx
         );
@@ -594,7 +596,7 @@
     fn set_acked_features(&mut self, page: u32, value: u32) {
         trace!(
             target: BALLOON_DRIVER_NAME,
             "{}: VirtioDevice::set_acked_features({}, 0x{:x})",
             BALLOON_DRIVER_NAME,
             page,
             value
@@ -602,10 +604,10 @@
         self.device_info.set_acked_features(page, value)
     }
 
-    fn read_config(&mut self, offset: u64, mut data: &mut [u8]) {
+    fn read_config(&mut self, offset: u64, mut data: &mut [u8]) -> ConfigResult {
         trace!(
             target: BALLOON_DRIVER_NAME,
             "{}: VirtioDevice::read_config(0x{:x}, {:?})",
             BALLOON_DRIVER_NAME,
             offset,
             data
@@ -618,29 +620,31 @@
                 "{}: config space read request out of range, offset {}",
                 BALLOON_DRIVER_NAME, offset
             );
-            return;
+            return Err(ConfigError::InvalidOffset(offset));
         }
         if let Some(end) = offset.checked_add(data.len() as u64) {
             // This write can't fail, offset and end are checked against config_len.
             data.write_all(&config_space[offset as usize..cmp::min(end, config_len) as usize])
                 .unwrap();
         }
+        Ok(())
     }
 
-    fn write_config(&mut self, offset: u64, data: &[u8]) {
+    fn write_config(&mut self, offset: u64, data: &[u8]) -> ConfigResult {
         let config = &mut self.config.lock().unwrap();
         let config_slice = config.as_mut_slice();
         let Ok(start) = usize::try_from(offset) else {
             error!("Failed to write config space");
-            return;
+            return Err(ConfigError::InvalidOffset(offset));
         };
         let Some(dst) = start.checked_add(data.len())
             .and_then(|end| config_slice.get_mut(start..end)) else
         {
             error!("Failed to write config space");
-            return;
+            return Err(ConfigError::InvalidOffsetPlusDataLen(offset + data.len() as u64));
         };
         dst.copy_from_slice(data);
+        Ok(())
     }
 
     fn activate(&mut self, mut config: VirtioDeviceConfig<AS, Q, R>) -> ActivateResult {
@@ -794,11 +798,13 @@
         let config: [u8; 8] = [0; 8];
         VirtioDevice::<Arc<GuestMemoryMmap<()>>, QueueSync, GuestRegionMmap>::write_config(
             &mut dev, 0, &config,
-        );
+        )
+        .unwrap();
         let mut data: [u8; 8] = [1; 8];
         VirtioDevice::<Arc<GuestMemoryMmap<()>>, QueueSync, GuestRegionMmap>::read_config(
             &mut dev, 0, &mut data,
-        );
+        )
+        .unwrap();
         assert_eq!(config, data);
     }
 

crates/dbs-virtio-devices/src/block/device.rs

@@ -25,7 +25,7 @@ use vm_memory::GuestMemoryRegion;
 use vmm_sys_util::eventfd::{EventFd, EFD_NONBLOCK};
 
 use crate::{
-    ActivateError, ActivateResult, DbsGuestAddressSpace, Error, Result, VirtioDevice,
+    ActivateError, ActivateResult, ConfigResult, DbsGuestAddressSpace, Error, Result, VirtioDevice,
     VirtioDeviceConfig, VirtioDeviceInfo, TYPE_BLOCK,
 };
 
@@ -220,11 +220,11 @@ where
         self.device_info.set_acked_features(page, value)
     }
 
-    fn read_config(&mut self, offset: u64, data: &mut [u8]) {
+    fn read_config(&mut self, offset: u64, data: &mut [u8]) -> ConfigResult {
         self.device_info.read_config(offset, data)
     }
 
-    fn write_config(&mut self, offset: u64, data: &[u8]) {
+    fn write_config(&mut self, offset: u64, data: &[u8]) -> ConfigResult {
         self.device_info.write_config(offset, data)
     }
 
@@ -898,11 +898,13 @@ mod tests {
             &mut dev,
             0,
             &mut config,
-        );
+        )
+        .unwrap();
         let config: [u8; 16] = [0; 16];
         VirtioDevice::<Arc<GuestMemoryMmap<()>>, QueueSync, GuestRegionMmap>::write_config(
             &mut dev, 0, &config,
-        );
+        )
+        .unwrap();
     }
 
     #[test]

crates/dbs-virtio-devices/src/device.rs

@@ -31,7 +31,7 @@
 };
 use vmm_sys_util::eventfd::{EventFd, EFD_NONBLOCK};
 
-use crate::{ActivateError, ActivateResult, Error, Result};
+use crate::{ActivateError, ActivateResult, ConfigError, ConfigResult, Error, Result};
 
 /// Virtio queue configuration information.
 ///
@@ -362,10 +362,10 @@
     fn set_acked_features(&mut self, page: u32, value: u32);
 
     /// Reads this device configuration space at `offset`.
-    fn read_config(&mut self, offset: u64, data: &mut [u8]);
+    fn read_config(&mut self, offset: u64, data: &mut [u8]) -> ConfigResult;
 
     /// Writes to this device configuration space at `offset`.
-    fn write_config(&mut self, offset: u64, data: &[u8]);
+    fn write_config(&mut self, offset: u64, data: &[u8]) -> ConfigResult;
 
     /// Activates this device for real usage.
     fn activate(&mut self, config: VirtioDeviceConfig<AS, Q, R>) -> ActivateResult;
@@ -485,40 +485,54 @@
     /// Reads device specific configuration data of virtio backend device.
     ///
     /// The `offset` is based of 0x100 from the MMIO configuration address space.
-    pub fn read_config(&self, offset: u64, mut data: &mut [u8]) {
+    pub fn read_config(&self, offset: u64, mut data: &mut [u8]) -> ConfigResult {
         let config_len = self.config_space.len() as u64;
         if offset >= config_len {
             error!(
                 "{}: config space read request out of range, offset {}",
                 self.driver_name, offset
             );
-            return;
+            return Err(ConfigError::InvalidOffset(offset));
         }
         if let Some(end) = offset.checked_add(data.len() as u64) {
             // This write can't fail, offset and end are checked against config_len.
             data.write_all(&self.config_space[offset as usize..cmp::min(end, config_len) as usize])
                 .unwrap();
         }
+        Ok(())
     }
 
     /// Writes device specific configuration data of virtio backend device.
     ///
     /// The `offset` is based of 0x100 from the MMIO configuration address space.
-    pub fn write_config(&mut self, offset: u64, data: &[u8]) {
+    pub fn write_config(&mut self, offset: u64, data: &[u8]) -> ConfigResult {
         let data_len = data.len() as u64;
         let config_len = self.config_space.len() as u64;
-        if offset >= config_len
-            || offset.checked_add(data_len).is_none()
-            || offset + data_len > config_len
-        {
+        if offset >= config_len {
             error!(
                 "{}: config space write request out of range, offset {}",
                 self.driver_name, offset
             );
-            return;
+            return Err(ConfigError::InvalidOffset(offset));
+        }
+        if offset.checked_add(data_len).is_none() {
+            error!(
+                "{}: config space write request out of range, offset {}, data length {}",
+                self.driver_name, offset, data_len
+            );
+            return Err(ConfigError::PlusOverflow(offset, data_len));
         }
+        if offset + data_len > config_len {
+            error!(
+                "{}: config space write request out of range, offset {}, data length {}",
+                self.driver_name, offset, data_len
+            );
+            return Err(ConfigError::InvalidOffsetPlusDataLen(offset + data_len));
+        }
+
         let dst = &mut self.config_space[offset as usize..(offset + data_len) as usize];
         dst.copy_from_slice(data);
+        Ok(())
     }
 
     /// Validate size of queues and queue eventfds.
@@ -553,9 +567,6 @@
 
 #[cfg(test)]
 pub(crate) mod tests {
-    use super::*;
-    use crate::{VIRTIO_INTR_CONFIG, VIRTIO_INTR_VRING};
-
     use dbs_interrupt::{
         InterruptManager, InterruptSourceType, InterruptStatusRegister32, LegacyNotifier,
     };
@@ -564,6 +575,9 @@
     use virtio_queue::QueueSync;
     use vm_memory::{GuestMemoryAtomic, GuestMemoryMmap, GuestMemoryRegion, MmapRegion};
 
+    use super::*;
+    use crate::{VIRTIO_INTR_CONFIG, VIRTIO_INTR_VRING};
+
     pub fn create_virtio_device_config() -> VirtioDeviceConfig<Arc<GuestMemoryMmap>> {
         let (vmfd, irq_manager) = crate::tests::create_vm_and_irq_manager();
         let group = irq_manager
@@ -713,10 +727,10 @@
             self.device_info.set_acked_features(page, value)
         }
 
-        fn read_config(&mut self, offset: u64, data: &mut [u8]) {
+        fn read_config(&mut self, offset: u64, data: &mut [u8]) -> ConfigResult {
             self.device_info.read_config(offset, data)
         }
-        fn write_config(&mut self, offset: u64, data: &[u8]) {
+        fn write_config(&mut self, offset: u64, data: &[u8]) -> ConfigResult {
             self.device_info.write_config(offset, data)
         }
         fn activate(
@@ -789,24 +803,33 @@
 
         // test config space invalid read
         let mut data = vec![0u8; 16];
-        device.read_config(16, data.as_mut_slice());
+        assert_eq!(
+            device.read_config(16, data.as_mut_slice()).unwrap_err(),
+            ConfigError::InvalidOffset(16)
+        );
         assert_eq!(data, vec![0; 16]);
         // test read config
-        device.read_config(4, &mut data[..14]);
+        device.read_config(4, &mut data[..14]).unwrap();
         assert_eq!(data, vec![1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0, 0, 0]);
-        device.read_config(0, data.as_mut_slice());
+        device.read_config(0, data.as_mut_slice()).unwrap();
         assert_eq!(data, vec![1; 16]);
 
         // test config space invalid write
         let write_data = vec![0xffu8; 16];
         let mut read_data = vec![0x0; 16];
-        device.write_config(4, &write_data[..13]);
-        device.write_config(16, &write_data[..4]);
-        device.read_config(0, read_data.as_mut_slice());
+        assert_eq!(
+            device.write_config(4, &write_data[..13]).unwrap_err(),
+            ConfigError::InvalidOffsetPlusDataLen(17)
+        );
+        assert_eq!(
+            device.write_config(16, &write_data[..4]).unwrap_err(),
+            ConfigError::InvalidOffset(16)
+        );
+        device.read_config(0, read_data.as_mut_slice()).unwrap();
         assert_eq!(read_data, vec![0x1; 16]);
 
         // test config space write
-        device.write_config(4, &write_data[6..10]);
+        device.write_config(4, &write_data[6..10]).unwrap();
         assert_eq!(
             device.device_info.config_space,
             vec![1, 1, 1, 1, 0xff, 0xff, 0xff, 0xff, 1, 1, 1, 1, 1, 1, 1, 1]

crates/dbs-virtio-devices/src/fs/device.rs

@@ -37,7 +37,7 @@ use vm_memory::{
 use vmm_sys_util::eventfd::EventFd;
 
 use crate::{
-    ActivateError, ActivateResult, Error, Result, VirtioDevice, VirtioDeviceConfig,
+    ActivateError, ActivateResult, ConfigResult, Error, Result, VirtioDevice, VirtioDeviceConfig,
     VirtioDeviceInfo, VirtioRegionHandler, VirtioSharedMemory, VirtioSharedMemoryList,
     TYPE_VIRTIO_FS,
 };
@@ -785,7 +785,7 @@ where
         self.device_info.set_acked_features(page, value)
     }
 
-    fn read_config(&mut self, offset: u64, data: &mut [u8]) {
+    fn read_config(&mut self, offset: u64, data: &mut [u8]) -> ConfigResult {
         trace!(
             target: VIRTIO_FS_NAME,
             "{}: VirtioDevice::read_config(0x{:x}, {:?})",
@@ -796,7 +796,7 @@ where
         self.device_info.read_config(offset, data)
     }
 
-    fn write_config(&mut self, offset: u64, data: &[u8]) {
+    fn write_config(&mut self, offset: u64, data: &[u8]) -> ConfigResult {
         trace!(
             target: VIRTIO_FS_NAME,
             "{}: VirtioDevice::write_config(0x{:x}, {:?})",
@@ -1173,11 +1173,13 @@ pub mod tests {
             &mut fs,
             0,
             &mut config,
-        );
+        )
+        .unwrap();
         let config: [u8; 16] = [0; 16];
         VirtioDevice::<Arc<GuestMemoryMmap<()>>, QueueSync, GuestRegionMmap>::write_config(
             &mut fs, 0, &config,
-        );
+        )
+        .unwrap();
     }
 
     #[test]

crates/dbs-virtio-devices/src/lib.rs

@@ -120,8 +120,21 @@ pub enum ActivateError {
     IOError(#[from] IOError),
 }
 
+/// Error code for VirtioDevice::read_config()/write_config().
+#[derive(Debug, thiserror::Error, Eq, PartialEq)]
+pub enum ConfigError {
+    #[error("Invalid offset: {0}.")]
+    InvalidOffset(u64),
+    #[error("Offset({0}) plus data length ({0}) overflow.")]
+    PlusOverflow(u64, u64),
+    #[error("Invalid offset plus data length: {0}.")]
+    InvalidOffsetPlusDataLen(u64),
+}
+
 /// Specialized std::result::Result for VirtioDevice::activate().
 pub type ActivateResult = std::result::Result<(), ActivateError>;
+/// Specialized std::result::Result for VirtioDevice::read_config()/write_config().
+pub type ConfigResult = std::result::Result<(), ConfigError>;
 
 /// Error for virtio devices to handle requests from guests.
 #[derive(Debug, thiserror::Error)]

crates/dbs-virtio-devices/src/mem.rs

@@ -38,8 +38,8 @@ use vm_memory::{
 
 use crate::device::{VirtioDevice, VirtioDeviceConfig, VirtioDeviceInfo};
 use crate::{
-    ActivateError, ActivateResult, DbsGuestAddressSpace, Error, Result, VirtioSharedMemoryList,
-    TYPE_MEM,
+    ActivateError, ActivateResult, ConfigResult, DbsGuestAddressSpace, Error, Result,
+    VirtioSharedMemoryList, TYPE_MEM,
 };
 
 /// Use 4 MiB alignment because current kernel use it as the subblock_size.
@@ -1072,7 +1072,7 @@ where
         self.device_info.set_acked_features(page, value)
     }
 
-    fn read_config(&mut self, offset: u64, mut data: &mut [u8]) {
+    fn read_config(&mut self, offset: u64, mut data: &mut [u8]) -> ConfigResult {
         trace!(
             target: MEM_DRIVER_NAME,
             "{}: {}: VirtioDevice::read_config(0x{:x}, {:?})",
@@ -1100,13 +1100,15 @@ where
             // This write can't fail, offset and end are checked against config_len.
             let _ = data.write(&config_space[offset as usize..end]).unwrap();
         }
+        Ok(())
     }
 
-    fn write_config(&mut self, _offset: u64, _data: &[u8]) {
+    fn write_config(&mut self, _offset: u64, _data: &[u8]) -> ConfigResult {
         debug!(
             target: MEM_DRIVER_NAME,
             "{}: {}: device configuration is read-only", MEM_DRIVER_NAME, self.id
         );
+        Ok(())
     }
 
     fn activate(&mut self, config: VirtioDeviceConfig<AS, Q, R>) -> ActivateResult {
@@ -1740,15 +1742,18 @@ pub(crate) mod tests {
         let mut data: [u8; 8] = [1; 8];
         VirtioDevice::<Arc<GuestMemoryMmap<()>>, QueueSync, GuestRegionMmap>::read_config(
             &mut dev, 0, &mut data,
-        );
+        )
+        .unwrap();
         let config: [u8; 8] = [0; 8];
         VirtioDevice::<Arc<GuestMemoryMmap<()>>, QueueSync, GuestRegionMmap>::write_config(
             &mut dev, 0, &config,
-        );
+        )
+        .unwrap();
         let mut data2: [u8; 8] = [1; 8];
         VirtioDevice::<Arc<GuestMemoryMmap<()>>, QueueSync, GuestRegionMmap>::read_config(
             &mut dev, 0, &mut data2,
-        );
+        )
+        .unwrap();
         assert_eq!(data, data2);
     }