vm-virtio: Simplify virtio-net configuration

This commit introduces a clear definition of the virtio-net
configuration structure, allowing both vhost-user-net and
virtio-net devices to rely on it.

This makes the code more readable for developers.

Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com>

vm-virtio/src/net.rs

@@ -7,8 +7,8 @@
 
 use super::net_util::{
     build_net_config_space, build_net_config_space_with_mq, open_tap, register_listener,
-    unregister_listener, CtrlVirtio, NetCtrlEpollHandler, RxVirtio, TxVirtio, KILL_EVENT,
+    unregister_listener, CtrlVirtio, NetCtrlEpollHandler, RxVirtio, TxVirtio, VirtioNetConfig,
-    NET_EVENTS_COUNT, PAUSE_EVENT, RX_QUEUE_EVENT, RX_TAP_EVENT, TX_QUEUE_EVENT,
+    KILL_EVENT, NET_EVENTS_COUNT, PAUSE_EVENT, RX_QUEUE_EVENT, RX_TAP_EVENT, TX_QUEUE_EVENT,
 };
 use super::Error as DeviceError;
 use super::{
@@ -32,7 +32,7 @@ use std::thread;
 use std::vec::Vec;
 use virtio_bindings::bindings::virtio_net::*;
 use vm_device::{Migratable, MigratableError, Pausable, Snapshotable};
-use vm_memory::GuestMemoryMmap;
+use vm_memory::{ByteValued, GuestMemoryMmap};
 use vmm_sys_util::eventfd::EventFd;
 
 #[derive(Debug)]
@@ -297,9 +297,7 @@ pub struct Net {
     taps: Option<Vec<Tap>>,
     avail_features: u64,
     acked_features: u64,
-    // The config space will only consist of the MAC address specified by the user,
+    config: VirtioNetConfig,
-    // or nothing, if no such address if provided.
-    config_space: Vec<u8>,
     queue_evts: Option<Vec<EventFd>>,
     interrupt_cb: Option<Arc<dyn VirtioInterrupt>>,
     epoll_threads: Option<Vec<thread::JoinHandle<result::Result<(), DeviceError>>>>,
@@ -332,12 +330,11 @@ impl Net {
         avail_features |= 1 << VIRTIO_NET_F_CTRL_VQ;
         let queue_num = num_queues + 1;
 
-        let mut config_space;
+        let mut config = VirtioNetConfig::default();
         if let Some(mac) = guest_mac {
-            config_space = build_net_config_space(mac, num_queues, &mut avail_features);
+            build_net_config_space(&mut config, mac, num_queues, &mut avail_features);
         } else {
-            config_space = Vec::new();
+            build_net_config_space_with_mq(&mut config, num_queues, &mut avail_features);
-            build_net_config_space_with_mq(num_queues, &mut config_space, &mut avail_features);
         }
 
         Ok(Net {
@@ -346,7 +343,7 @@ impl Net {
             taps: Some(taps),
             avail_features,
             acked_features: 0u64,
-            config_space,
+            config,
             queue_evts: None,
             interrupt_cb: None,
             epoll_threads: None,
@@ -425,26 +422,28 @@ impl VirtioDevice for Net {
     }
 
     fn read_config(&self, offset: u64, mut data: &mut [u8]) {
-        let config_len = self.config_space.len() as u64;
+        let config_slice = self.config.as_slice();
+        let config_len = config_slice.len() as u64;
         if offset >= config_len {
             error!("Failed to read config space");
             return;
         }
         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])
+            data.write_all(&config_slice[offset as usize..cmp::min(end, config_len) as usize])
                 .unwrap();
         }
     }
 
     fn write_config(&mut self, offset: u64, data: &[u8]) {
+        let config_slice = self.config.as_mut_slice();
         let data_len = data.len() as u64;
-        let config_len = self.config_space.len() as u64;
+        let config_len = config_slice.len() as u64;
         if offset + data_len > config_len {
             error!("Failed to write config space");
             return;
         }
-        let (_, right) = self.config_space.split_at_mut(offset as usize);
+        let (_, right) = config_slice.split_at_mut(offset as usize);
         right.copy_from_slice(&data[..]);
     }
 

vm-virtio/src/net_util.rs

@@ -5,7 +5,7 @@
 use super::Error as DeviceError;
 use super::{DescriptorChain, DeviceEventT, Queue};
 use arc_swap::ArcSwap;
-use net_util::{MacAddr, Tap, TapError, MAC_ADDR_LEN};
+use net_util::{MacAddr, Tap, TapError};
 use std::cmp;
 use std::io::{self, Write};
 use std::mem;
@@ -25,11 +25,6 @@ type Result<T> = std::result::Result<T, Error>;
 const MAX_BUFFER_SIZE: usize = 65562;
 const QUEUE_SIZE: usize = 256;
 
-const CONFIG_SPACE_MAC: usize = MAC_ADDR_LEN;
-const CONFIG_SPACE_STATUS: usize = 2;
-const CONFIG_SPACE_QUEUE_PAIRS: usize = 2;
-const CONFIG_SPACE_NET: usize = CONFIG_SPACE_MAC + CONFIG_SPACE_STATUS + CONFIG_SPACE_QUEUE_PAIRS;
-
 // The guest has made a buffer available to receive a frame into.
 pub const RX_QUEUE_EVENT: DeviceEventT = 0;
 // The transmit queue has a frame that is ready to send from the guest.
@@ -47,6 +42,20 @@ const CTRL_QUEUE_EVENT: DeviceEventT = 0;
 // Number of DeviceEventT events supported by this implementation.
 const CTRL_EVENT_COUNT: usize = 3;
 
+#[repr(C, packed)]
+#[derive(Copy, Clone, Debug, Default)]
+pub struct VirtioNetConfig {
+    pub mac: [u8; 6],
+    pub status: u16,
+    pub max_virtqueue_pairs: u16,
+    pub mtu: u16,
+    pub speed: u32,
+    pub duplex: u8,
+}
+
+// Safe because it only has data and has no implicit padding.
+unsafe impl ByteValued for VirtioNetConfig {}
+
 #[derive(Debug)]
 pub enum Error {
     /// Read process MQ.
@@ -414,33 +423,27 @@ impl RxVirtio {
 }
 
 pub fn build_net_config_space(
+    mut config: &mut VirtioNetConfig,
     mac: MacAddr,
     num_queues: usize,
     mut avail_features: &mut u64,
-) -> Vec<u8> {
+) {
-    let mut config_space = Vec::with_capacity(MAC_ADDR_LEN);
+    config.mac.copy_from_slice(mac.get_bytes());
-    unsafe { config_space.set_len(MAC_ADDR_LEN) }
-    config_space[..].copy_from_slice(mac.get_bytes());
     *avail_features |= 1 << VIRTIO_NET_F_MAC;
 
-    build_net_config_space_with_mq(num_queues, &mut config_space, &mut avail_features);
+    build_net_config_space_with_mq(&mut config, num_queues, &mut avail_features);
-
-    config_space
 }
 
 pub fn build_net_config_space_with_mq(
+    config: &mut VirtioNetConfig,
     num_queues: usize,
-    config_space: &mut Vec<u8>,
     avail_features: &mut u64,
 ) {
     let num_queue_pairs = (num_queues / 2) as u16;
     if (num_queue_pairs >= VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN as u16)
         && (num_queue_pairs <= VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX as u16)
     {
-        config_space.resize(CONFIG_SPACE_NET, 0);
+        config.max_virtqueue_pairs = num_queue_pairs;
-        let max_queue_pairs = num_queue_pairs.to_le_bytes();
-        config_space[CONFIG_SPACE_MAC + CONFIG_SPACE_STATUS..CONFIG_SPACE_NET]
-            .copy_from_slice(&max_queue_pairs);
         *avail_features |= 1 << VIRTIO_NET_F_MQ;
     }
 }

vm-virtio/src/vhost_user/net.rs

@@ -1,7 +1,9 @@
 // Copyright 2019 Intel Corporation. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
-use super::super::net_util::{build_net_config_space, CtrlVirtio, NetCtrlEpollHandler};
+use super::super::net_util::{
+    build_net_config_space, CtrlVirtio, NetCtrlEpollHandler, VirtioNetConfig,
+};
 use super::super::Error as CtrlError;
 use super::super::{ActivateError, ActivateResult, Queue, VirtioDevice, VirtioDeviceType};
 use super::handler::*;
@@ -26,7 +28,7 @@ use vhost_rs::VhostBackend;
 use virtio_bindings::bindings::virtio_net;
 use virtio_bindings::bindings::virtio_ring;
 use vm_device::{Migratable, MigratableError, Pausable, Snapshotable};
-use vm_memory::GuestMemoryMmap;
+use vm_memory::{ByteValued, GuestMemoryMmap};
 use vmm_sys_util::eventfd::EventFd;
 
 const DEFAULT_QUEUE_NUMBER: usize = 2;
@@ -41,7 +43,7 @@ pub struct Net {
     avail_features: u64,
     acked_features: u64,
     backend_features: u64,
-    config_space: Vec<u8>,
+    config: VirtioNetConfig,
     queue_sizes: Vec<u16>,
     queue_evts: Option<Vec<EventFd>>,
     interrupt_cb: Option<Arc<dyn VirtioInterrupt>>,
@@ -122,7 +124,13 @@ impl Net {
         avail_features |= 1 << virtio_net::VIRTIO_NET_F_CTRL_VQ;
         let queue_num = vu_cfg.num_queues + 1;
 
-        let config_space = build_net_config_space(mac_addr, vu_cfg.num_queues, &mut avail_features);
+        let mut config = VirtioNetConfig::default();
+        build_net_config_space(
+            &mut config,
+            mac_addr,
+            vu_cfg.num_queues,
+            &mut avail_features,
+        );
 
         // Send set_vring_base here, since it could tell backends, like OVS + DPDK,
         // how many virt queues to be handled, which backend required to know at early stage.
@@ -139,7 +147,7 @@ impl Net {
             avail_features,
             acked_features,
             backend_features,
-            config_space,
+            config,
             queue_sizes: vec![vu_cfg.queue_size; queue_num],
             queue_evts: None,
             interrupt_cb: None,
@@ -201,26 +209,28 @@ impl VirtioDevice for Net {
     }
 
     fn read_config(&self, offset: u64, mut data: &mut [u8]) {
-        let config_len = self.config_space.len() as u64;
+        let config_slice = self.config.as_slice();
+        let config_len = config_slice.len() as u64;
         if offset >= config_len {
             error!("Failed to read config space");
             return;
         }
         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])
+            data.write_all(&config_slice[offset as usize..cmp::min(end, config_len) as usize])
                 .unwrap();
         }
     }
 
     fn write_config(&mut self, offset: u64, data: &[u8]) {
+        let config_slice = self.config.as_mut_slice();
         let data_len = data.len() as u64;
-        let config_len = self.config_space.len() as u64;
+        let config_len = config_slice.len() as u64;
         if offset + data_len > config_len {
             error!("Failed to write config space");
             return;
         }
-        let (_, right) = self.config_space.split_at_mut(offset as usize);
+        let (_, right) = config_slice.split_at_mut(offset as usize);
         right.copy_from_slice(&data[..]);
     }