Snapshot-ga: Generate a MAC address if unspecified

Signed-off-by: Vibha Acharya <vibharya@amazon.co.uk>

Author: vibharya

src/devices/src/virtio/net/device.rs

@@ -4,9 +4,6 @@
 // Portions Copyright 2017 The Chromium OS Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the THIRD-PARTY file.
-
-#[cfg(not(test))]
-use std::io;
 use std::io::{Read, Write};
 use std::net::Ipv4Addr;
 use std::sync::atomic::AtomicUsize;
@@ -21,6 +18,7 @@ use mmds::ns::MmdsNetworkStack;
 use rate_limiter::{BucketUpdate, RateLimiter, TokenType};
 use utils::eventfd::EventFd;
 use utils::net::mac::{MacAddr, MAC_ADDR_LEN};
+use utils::rand_bytes;
 use virtio_gen::virtio_net::{
     virtio_net_hdr_v1, VIRTIO_F_VERSION_1, VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM,
     VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_UFO, VIRTIO_NET_F_HOST_TSO4, VIRTIO_NET_F_HOST_UFO,
@@ -49,6 +47,9 @@ enum FrontendError {
     ReadOnlyDescriptor,
 }
 
+// KVM OUI MAC address is 52:54:00:xx:xx:xx
+const KVM_OUI: [u8; 3] = [0x52, 0x54, 0x00];
+
 pub(crate) fn vnet_hdr_len() -> usize {
     mem::size_of::<virtio_net_hdr_v1>()
 }
@@ -80,6 +81,14 @@ fn init_vnet_hdr(buf: &mut [u8]) {
     }
 }
 
+fn generate_mac_address() -> MacAddr {
+    let mut random_mac: [u8; MAC_ADDR_LEN] = [0; MAC_ADDR_LEN];
+    let (left, right) = random_mac.split_at_mut(3);
+    left.copy_from_slice(&KVM_OUI);
+    right.copy_from_slice(rand_bytes(3).as_slice());
+    MacAddr::from_bytes_unchecked(&random_mac)
+}
+
 #[derive(Clone, Copy)]
 pub struct ConfigSpace {
     pub guest_mac: [u8; MAC_ADDR_LEN],
@@ -120,7 +129,7 @@ pub struct Net {
     pub(crate) irq_trigger: IrqTrigger,
 
     pub(crate) config_space: ConfigSpace,
-    pub(crate) guest_mac: Option<MacAddr>,
+    pub(crate) guest_mac: MacAddr,
 
     pub(crate) device_state: DeviceState,
     pub(crate) activate_evt: EventFd,
@@ -162,12 +171,18 @@ impl Net {
             | 1 << VIRTIO_RING_F_EVENT_IDX;
 
         let mut config_space = ConfigSpace::default();
-        if let Some(mac) = guest_mac {
-            config_space.guest_mac.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 |= 1 << VIRTIO_NET_F_MAC;
-        }
+        // Enable feature to configure a MAC address
+        // If not enabled, the driver generates a random MAC address.
+        avail_features |= 1 << VIRTIO_NET_F_MAC;
+        let mac_addr = {
+            if let Some(some_mac_adrr) = guest_mac {
+                *some_mac_adrr
+            } else {
+                // When the MAC address is not specified explicitly, generate one and assign it.
+                generate_mac_address()
+            }
+        };
+        config_space.guest_mac.copy_from_slice(mac_addr.get_bytes());
 
         let mut queue_evts = Vec::new();
         let mut queues = Vec::new();
@@ -195,7 +210,7 @@ impl Net {
             activate_evt: EventFd::new(libc::EFD_NONBLOCK).map_err(Error::EventFd)?,
             config_space,
             mmds_ns: None,
-            guest_mac: guest_mac.copied(),
+            guest_mac: mac_addr,
 
             #[cfg(test)]
             mocks: Mocks::default(),
@@ -208,8 +223,8 @@ impl Net {
     }
 
     /// Provides the MAC of this net device.
-    pub fn guest_mac(&self) -> Option<&MacAddr> {
-        self.guest_mac.as_ref()
+    pub fn guest_mac(&self) -> &MacAddr {
+        &self.guest_mac
     }
 
     /// Provides the host IFACE name of this net device.
@@ -413,7 +428,7 @@ impl Net {
         rate_limiter: &mut RateLimiter,
         frame_buf: &[u8],
         tap: &mut Tap,
-        guest_mac: Option<MacAddr>,
+        guest_mac: MacAddr,
     ) -> Result<bool> {
         let checked_frame = |frame_buf| {
             frame_bytes_from_buf(frame_buf).map_err(|err| {
@@ -438,13 +453,11 @@ impl Net {
         // This frame goes to the TAP.
 
         // Check for guest MAC spoofing.
-        if let Some(mac) = guest_mac {
-            let _ = EthernetFrame::from_bytes(checked_frame(frame_buf)?).map(|eth_frame| {
-                if mac != eth_frame.src_mac() {
-                    METRICS.net.tx_spoofed_mac_count.inc();
-                }
-            });
-        }
+        let _ = EthernetFrame::from_bytes(checked_frame(frame_buf)?).map(|eth_frame| {
+            if guest_mac != eth_frame.src_mac() {
+                METRICS.net.tx_spoofed_mac_count.inc();
+            }
+        });
 
         match tap.write(frame_buf) {
             Ok(_) => {
@@ -661,7 +674,7 @@ impl Net {
     }
 
     #[cfg(not(test))]
-    fn read_tap(&mut self) -> io::Result<usize> {
+    fn read_tap(&mut self) -> std::io::Result<usize> {
         self.tap.read(&mut self.rx_frame_buf)
     }
 
@@ -830,9 +843,8 @@ impl VirtioDevice for Net {
         }
 
         config_space_bytes[offset as usize..(offset + data_len) as usize].copy_from_slice(data);
-        self.guest_mac = Some(MacAddr::from_bytes_unchecked(
-            &self.config_space.guest_mac[..MAC_ADDR_LEN],
-        ));
+        self.guest_mac =
+            MacAddr::from_bytes_unchecked(&self.config_space.guest_mac[..MAC_ADDR_LEN]);
         METRICS.net.mac_address_updates.inc();
     }
 
@@ -1003,7 +1015,7 @@ pub mod tests {
 
         // Check that the guest MAC was updated.
         let expected_guest_mac = MacAddr::from_bytes_unchecked(&new_config);
-        assert_eq!(expected_guest_mac, net.guest_mac.unwrap());
+        assert_eq!(expected_guest_mac, net.guest_mac);
         assert_eq!(METRICS.net.mac_address_updates.count(), 1);
 
         // Partial write (this is how the kernel sets a new mac address) - byte by byte.
@@ -1470,7 +1482,7 @@ pub mod tests {
                 &mut net.tx_rate_limiter,
                 &frame_buf[..frame_len],
                 &mut net.tap,
-                Some(src_mac),
+                src_mac,
             )
             .unwrap())
         );
@@ -1504,7 +1516,7 @@ pub mod tests {
                 &mut net.tx_rate_limiter,
                 &frame_buf[..frame_len],
                 &mut net.tap,
-                Some(guest_mac),
+                guest_mac,
             )
         );
 
@@ -1517,7 +1529,7 @@ pub mod tests {
                 &mut net.tx_rate_limiter,
                 &frame_buf[..frame_len],
                 &mut net.tap,
-                Some(not_guest_mac),
+                not_guest_mac,
             )
         );
     }

src/devices/src/virtio/net/persist.rs

@@ -18,7 +18,7 @@ use versionize::{VersionMap, Versionize, VersionizeResult};
 use versionize_derive::Versionize;
 use vm_memory::GuestMemoryMmap;
 
-use super::device::{ConfigSpace, Net};
+use super::device::Net;
 use super::{NUM_QUEUES, QUEUE_SIZE};
 use crate::virtio::persist::{Error as VirtioStateError, VirtioDeviceState};
 use crate::virtio::{DeviceState, TYPE_NET};
@@ -83,7 +83,10 @@ impl Persist<'_> for Net {
         let mut net = Net::new_with_tap(
             state.id.clone(),
             state.tap_if_name.clone(),
-            None,
+            Some(MacAddr::from_bytes_unchecked(
+                &state.config_space.guest_mac[..MAC_ADDR_LEN],
+            ))
+            .as_ref(),
             rx_rate_limiter,
             tx_rate_limiter,
         )?;
@@ -115,13 +118,6 @@ impl Persist<'_> for Net {
             Arc::new(AtomicUsize::new(state.virtio_state.interrupt_status));
         net.avail_features = state.virtio_state.avail_features;
         net.acked_features = state.virtio_state.acked_features;
-        net.config_space = ConfigSpace {
-            guest_mac: state.config_space.guest_mac,
-        };
-
-        net.guest_mac = Some(MacAddr::from_bytes_unchecked(
-            &state.config_space.guest_mac[..MAC_ADDR_LEN],
-        ));
 
         if state.virtio_state.activated {
             net.device_state = DeviceState::Activated(constructor_args.mem);

src/devices/src/virtio/net/test_utils.rs

@@ -298,7 +298,7 @@ pub fn default_guest_memory() -> GuestMemoryMmap {
 }
 
 pub fn set_mac(net: &mut Net, mac: MacAddr) {
-    net.guest_mac = Some(mac);
+    net.guest_mac = mac;
     net.config_space.guest_mac.copy_from_slice(mac.get_bytes());
 }
 

src/utils/src/lib.rs

@@ -28,3 +28,32 @@ pub fn get_page_size() -> Result<usize, errno::Error> {
         ps => Ok(ps as usize),
     }
 }
+
+// The below fucntions will get merged in rust-vmm function once we will upstream it there
+fn xor_pseudo_rng_u8_bytes(rand_fn: &dyn Fn() -> u32) -> Vec<u8> {
+    let mut r = vec![];
+
+    for n in &rand_fn().to_ne_bytes() {
+        r.push(*n);
+    }
+    r
+}
+
+fn rand_bytes_impl(rand_fn: &dyn Fn() -> u32, len: usize) -> Vec<u8> {
+    let mut buf: Vec<u8> = Vec::new();
+    let mut done = 0;
+    loop {
+        for n in xor_pseudo_rng_u8_bytes(rand_fn) {
+            done += 1;
+            buf.push(n);
+            if done >= len {
+                return buf;
+            }
+        }
+    }
+}
+
+/// Get a pseudo random vector of length `len` with bytes.
+pub fn rand_bytes(len: usize) -> Vec<u8> {
+    rand_bytes_impl(&rand::xor_psuedo_rng_u32, len)
+}

src/vmm/src/device_manager/persist.rs

@@ -526,6 +526,7 @@ impl<'a> Persist<'a> for MMIODeviceManager {
 #[cfg(test)]
 mod tests {
     use devices::virtio::block::CacheType;
+    use utils::net::mac::MacAddr;
     use utils::tempfile::TempFile;
 
     use super::*;
@@ -695,7 +696,7 @@ mod tests {
             let network_interface = NetworkInterfaceConfig {
                 iface_id: String::from("netif"),
                 host_dev_name: String::from("hostname"),
-                guest_mac: None,
+                guest_mac: Some(MacAddr::parse_str("00:00:00:00:00:00").unwrap()),
                 rx_rate_limiter: None,
                 tx_rate_limiter: None,
             };

src/vmm/src/resources.rs

@@ -1076,11 +1076,13 @@ mod tests {
                     "network-interfaces": [
                         {{
                             "iface_id": "netif1",
-                            "host_dev_name": "hostname9"
+                            "host_dev_name": "hostname9",
+                            "guest_mac": "06:00:c0:00:32:de"
                         }},
                         {{
                             "iface_id": "netif2",
-                            "host_dev_name": "hostname10"
+                            "host_dev_name": "hostname10",
+                            "guest_mac": "0a:00:20:00:a2:de"
                         }}
                     ],
                     "machine-config": {{
@@ -1151,11 +1153,13 @@ mod tests {
                     "network-interfaces": [
                         {{
                             "iface_id": "netif1",
-                            "host_dev_name": "hostname9"
+                            "host_dev_name": "hostname9",
+                            "guest_mac": "06:00:00:00:32:de"
                         }},
                         {{
                             "iface_id": "netif2",
-                            "host_dev_name": "hostname10"
+                            "host_dev_name": "hostname10",
+                            "guest_mac": "06:00:00:00:a2:de"
                         }}
                     ],
                     "machine-config": {{
@@ -1210,11 +1214,13 @@ mod tests {
                     "network-interfaces": [
                         {{
                             "iface_id": "netif1",
-                            "host_dev_name": "hostname9"
+                            "host_dev_name": "hostname9",
+                            "guest_mac": "06:00:00:00:32:de"
                         }},
                         {{
                             "iface_id": "netif2",
-                            "host_dev_name": "hostname10"
+                            "host_dev_name": "hostname10",
+                            "guest_mac": "06:00:00:00:a2:de"
                         }}
                     ],
                     "machine-config": {{

src/vmm/src/vmm_config/net.rs

@@ -38,7 +38,7 @@ impl From<&Net> for NetworkInterfaceConfig {
         NetworkInterfaceConfig {
             iface_id: net.id().clone(),
             host_dev_name: net.iface_name(),
-            guest_mac: net.guest_mac().copied(),
+            guest_mac: Some(*net.guest_mac()),
             rx_rate_limiter: rx_rl.into_option(),
             tx_rate_limiter: tx_rl.into_option(),
         }
@@ -143,7 +143,7 @@ impl NetBuilder {
             let net = net.lock().expect("Poisoned lock");
             // Check if another net dev has same MAC.
             netif_config.guest_mac.is_some()
-                && netif_config.guest_mac.as_ref() == net.guest_mac()
+                && netif_config.guest_mac.as_ref().unwrap() == net.guest_mac()
                 && &netif_config.iface_id != net.id()
         };
         // Validate there is no Mac conflict.
@@ -409,4 +409,35 @@ mod tests {
             net_id
         );
     }
+
+    #[test]
+    fn test_mac_address_generation() {
+        let mut net_builder = NetBuilder::new();
+        let net_id = "test_id";
+        let host_dev_name = "dev";
+        let guest_mac = "00:00:00:00:00:00";
+
+        let net = Net::new_with_tap(
+            net_id.to_string(),
+            host_dev_name.to_string(),
+            None,
+            RateLimiter::default(),
+            RateLimiter::default(),
+        )
+        .unwrap();
+
+        net_builder.add_device(Arc::new(Mutex::new(net)));
+        assert_eq!(net_builder.net_devices.len(), 1);
+        assert_ne!(
+            net_builder
+                .net_devices
+                .pop()
+                .unwrap()
+                .lock()
+                .unwrap()
+                .deref()
+                .guest_mac(),
+            MacAddr::parse_str(guest_mac).as_ref().unwrap()
+        );
+    }
 }