Unit tests for PATCH /network-interfaces/

Added unit tests for the new code that enables support for post-boot
updating of the network interface rate limiters.

Signed-off-by: Dan Horobeanu <[email protected]>

Author: dhrgit

api_server/src/http_service.rs

@@ -1225,7 +1225,50 @@ mod tests {
         assert!(
             parse_netif_req(path, Method::Options, &body,)
                 == Err(Error::InvalidPathMethod(path, Method::Options))
-        )
+        );
+
+        // PATCH tests
+
+        // Fail when path ID != body ID.
+        assert!(NetworkInterfaceUpdateConfig {
+            iface_id: "1".to_string(),
+            rx_rate_limiter: None,
+            tx_rate_limiter: None,
+        }
+        .into_parsed_request(Some("2".to_string()), Method::Patch)
+        .is_err());
+
+        let json = r#"{
+            "iface_id": "1",
+            "rx_rate_limiter": {
+                "bandwidth": {
+                    "size": 1024,
+                    "refill_time": 100
+                }
+            }
+        }"#;
+        let body = Chunk::from(json);
+        let nuc = serde_json::from_slice::<NetworkInterfaceUpdateConfig>(json.as_bytes()).unwrap();
+        let nuc_pr = nuc
+            .into_parsed_request(Some("1".to_string()), Method::Patch)
+            .unwrap();
+        match parse_netif_req(&"/network-interfaces/1", Method::Patch, &body) {
+            Ok(pr) => assert!(nuc_pr.eq(&pr)),
+            _ => assert!(false),
+        };
+
+        let json = r#"{
+            "iface_id": "1",
+            "invalid_key": true
+        }"#;
+        let body = Chunk::from(json);
+        assert!(parse_netif_req(&"/network-interfaces/1", Method::Patch, &body).is_err());
+
+        let json = r#"{
+            "iface_id": "1"
+        }"#;
+        let body = Chunk::from(json);
+        assert!(parse_netif_req(&"/network-interfaces/2", Method::Patch, &body).is_err());
     }
 
     #[test]

vmm/src/lib.rs

@@ -136,8 +136,8 @@ pub enum Error {
     EpollFd(io::Error),
     /// Cannot read from an Event file descriptor.
     EventFd(std::io::Error),
-    /// Describes a logical problem.
-    GeneralFailure, // TODO: there are some cases in which this error should be replaced.
+    /// An event arrived for a device, but the dispatcher can't find the event (epoll) handler.
+    DeviceEventHandlerNotFound,
     /// Cannot open /dev/kvm. Either the host does not have KVM or Firecracker does not have
     /// permission to open the file descriptor.
     Kvm(io::Error),
@@ -161,11 +161,21 @@ impl std::fmt::Debug for Error {
         use self::Error::*;
 
         match self {
-            // This only implements Debug for the most common Firecracker error
-            // which is the one coming from KVM at the time.
-            // TODO: implement debug for the other errors as well.
+            ApiChannel => write!(f, "ApiChannel: error receiving data from the API server"),
+            CreateLegacyDevice(e) => write!(f, "Error creating legacy device: {:?}", e),
+            EpollFd(e) => write!(f, "Epoll fd error: {:?}", e),
+            EventFd(e) => write!(f, "Event fd error: {}", e.to_string()),
+            DeviceEventHandlerNotFound => write!(
+                f,
+                "Device event handler not found. This might point to a guest device driver issue."
+            ),
             Kvm(ref os_err) => write!(f, "Cannot open /dev/kvm. Error: {}", os_err.to_string()),
-            _ => write!(f, "{:?}", self),
+            KvmApiVersion(ver) => write!(f, "Bad KVM API version: {}", ver),
+            KvmCap(cap) => write!(f, "Missing KVM capability: {:?}", cap),
+            Poll(e) => write!(f, "Epoll wait failed: {}", e.to_string()),
+            Serial(e) => write!(f, "Error writing to the serial console: {:?}", e),
+            TimerFd(e) => write!(f, "Error creating timer fd: {}", e.to_string()),
+            Vm(e) => write!(f, "Error opening VM fd: {:?}", e),
         }
     }
 }
@@ -250,6 +260,7 @@ impl Display for VmmActionError {
 
 /// This enum represents the public interface of the VMM. Each action contains various
 /// bits of information (ids, paths, etc.), together with an OutcomeSender, which is always present.
+#[derive(Debug)]
 pub enum VmmAction {
     /// Configure the boot source of the microVM using as input the `ConfigureBootSource`. This
     /// action can only be called before the microVM has booted. The response is sent using the
@@ -543,7 +554,7 @@ impl EpollContext {
                 let received = maybe
                     .receiver
                     .try_recv()
-                    .map_err(|_| Error::GeneralFailure)?;
+                    .map_err(|_| Error::DeviceEventHandlerNotFound)?;
                 Ok(maybe.handler.get_or_insert(received).as_mut())
             }
         }
@@ -1946,6 +1957,10 @@ impl PartialEq for VmmAction {
                 &VmmAction::InsertNetworkDevice(ref net_dev, _),
                 &VmmAction::InsertNetworkDevice(ref other_net_dev, _),
             ) => net_dev == other_net_dev,
+            (
+                &VmmAction::UpdateNetworkInterface(ref net_dev, _),
+                &VmmAction::UpdateNetworkInterface(ref other_net_dev, _),
+            ) => net_dev == other_net_dev,
             (
                 &VmmAction::RescanBlockDevice(ref req, _),
                 &VmmAction::RescanBlockDevice(ref other_req, _),
@@ -2019,6 +2034,7 @@ mod tests {
     use devices::virtio::ActivateResult;
     use net_util::MacAddr;
     use vmm_config::machine_config::CpuFeaturesTemplate;
+    use vmm_config::{RateLimiterConfig, TokenBucketConfig};
 
     impl Vmm {
         fn get_kernel_cmdline_str(&self) -> &str {
@@ -2312,6 +2328,98 @@ mod tests {
         assert!(vmm.insert_net_device(network_interface).is_err());
     }
 
+    #[test]
+    fn test_update_net_device() {
+        let mut vmm = create_vmm_object(InstanceState::Uninitialized);
+
+        let tbc_1mtps = TokenBucketConfig {
+            size: 1024 * 1024,
+            one_time_burst: None,
+            refill_time: 1000,
+        };
+        let tbc_2mtps = TokenBucketConfig {
+            size: 2 * 1024 * 1024,
+            one_time_burst: None,
+            refill_time: 1000,
+        };
+
+        vmm.insert_net_device(NetworkInterfaceConfig {
+            iface_id: String::from("1"),
+            host_dev_name: String::from("hostname5"),
+            guest_mac: None,
+            rx_rate_limiter: Some(RateLimiterConfig {
+                bandwidth: Some(tbc_1mtps),
+                ops: None,
+            }),
+            tx_rate_limiter: None,
+            allow_mmds_requests: false,
+            tap: None,
+        })
+        .unwrap();
+
+        vmm.update_net_device(NetworkInterfaceUpdateConfig {
+            iface_id: "1".to_string(),
+            rx_rate_limiter: Some(RateLimiterConfig {
+                bandwidth: None,
+                ops: Some(tbc_2mtps),
+            }),
+            tx_rate_limiter: Some(RateLimiterConfig {
+                bandwidth: None,
+                ops: Some(tbc_2mtps),
+            }),
+        })
+        .unwrap();
+
+        {
+            let nic_1: &mut NetworkInterfaceConfig =
+                vmm.network_interface_configs.iter_mut().next().unwrap();
+            // The RX bandwidth should be unaffected.
+            assert_eq!(nic_1.rx_rate_limiter.unwrap().bandwidth.unwrap(), tbc_1mtps);
+            // The RX ops should be set to 2mtps.
+            assert_eq!(nic_1.rx_rate_limiter.unwrap().ops.unwrap(), tbc_2mtps);
+            // The TX bandwith should be unlimited (unaffected).
+            assert_eq!(nic_1.tx_rate_limiter.unwrap().bandwidth, None);
+            // The TX ops should be set to 2mtps.
+            assert_eq!(nic_1.tx_rate_limiter.unwrap().ops.unwrap(), tbc_2mtps);
+        }
+
+        vmm.init_guest_memory().unwrap();
+        vmm.default_kernel_config();
+        let guest_mem = vmm.guest_memory.clone().unwrap();
+        let mut device_manager =
+            MMIODeviceManager::new(guest_mem.clone(), arch::get_reserved_mem_addr() as u64);
+        vmm.attach_net_devices(&mut device_manager).unwrap();
+        vmm.set_instance_state(InstanceState::Running);
+
+        // The update should fail before device activation.
+        assert!(vmm
+            .update_net_device(NetworkInterfaceUpdateConfig {
+                iface_id: "1".to_string(),
+                rx_rate_limiter: None,
+                tx_rate_limiter: None,
+            })
+            .is_err());
+
+        // Fake device activation by explicitly setting a dummy epoll handler.
+        vmm.epoll_context.device_handlers[0].handler = Some(Box::new(DummyEpollHandler {
+            evt: None,
+            flags: None,
+            payload: None,
+        }));
+        vmm.update_net_device(NetworkInterfaceUpdateConfig {
+            iface_id: "1".to_string(),
+            rx_rate_limiter: Some(RateLimiterConfig {
+                bandwidth: Some(tbc_2mtps),
+                ops: None,
+            }),
+            tx_rate_limiter: Some(RateLimiterConfig {
+                bandwidth: Some(tbc_1mtps),
+                ops: None,
+            }),
+        })
+        .unwrap();
+    }
+
     #[test]
     fn test_machine_configuration() {
         let mut vmm = create_vmm_object(InstanceState::Uninitialized);

vmm/src/vmm_config/mod.rs

@@ -86,3 +86,63 @@ impl RateLimiterConfig {
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_rate_limiter_configs() {
+        const SIZE: u64 = 1024 * 1024;
+        const ONE_TIME_BURST: u64 = 1024;
+        const REFILL_TIME: u64 = 1000;
+
+        let b = TokenBucketConfig {
+            size: SIZE,
+            one_time_burst: Some(ONE_TIME_BURST),
+            refill_time: REFILL_TIME,
+        }
+        .into_token_bucket();
+        assert_eq!(b.capacity(), SIZE);
+        assert_eq!(b.one_time_burst(), ONE_TIME_BURST);
+        assert_eq!(b.refill_time_ms(), REFILL_TIME);
+
+        let mut rlconf = RateLimiterConfig {
+            bandwidth: Some(TokenBucketConfig {
+                size: SIZE,
+                one_time_burst: Some(ONE_TIME_BURST),
+                refill_time: REFILL_TIME,
+            }),
+            ops: Some(TokenBucketConfig {
+                size: SIZE * 2,
+                one_time_burst: None,
+                refill_time: REFILL_TIME * 2,
+            }),
+        };
+        let rl = rlconf.into_rate_limiter().unwrap();
+        assert_eq!(rl.bandwidth().unwrap().capacity(), SIZE);
+        assert_eq!(rl.bandwidth().unwrap().one_time_burst(), ONE_TIME_BURST);
+        assert_eq!(rl.bandwidth().unwrap().refill_time_ms(), REFILL_TIME);
+        assert_eq!(rl.ops().unwrap().capacity(), SIZE * 2);
+        assert_eq!(rl.ops().unwrap().one_time_burst(), 0);
+        assert_eq!(rl.ops().unwrap().refill_time_ms(), REFILL_TIME * 2);
+
+        rlconf.update(&RateLimiterConfig {
+            bandwidth: Some(TokenBucketConfig {
+                size: SIZE * 2,
+                one_time_burst: Some(ONE_TIME_BURST * 2),
+                refill_time: REFILL_TIME * 2,
+            }),
+            ops: None,
+        });
+        assert_eq!(rlconf.bandwidth.unwrap().size, SIZE * 2);
+        assert_eq!(
+            rlconf.bandwidth.unwrap().one_time_burst,
+            Some(ONE_TIME_BURST * 2)
+        );
+        assert_eq!(rlconf.bandwidth.unwrap().refill_time, REFILL_TIME * 2);
+        assert_eq!(rlconf.ops.unwrap().size, SIZE * 2);
+        assert_eq!(rlconf.ops.unwrap().one_time_burst, None);
+        assert_eq!(rlconf.ops.unwrap().refill_time, REFILL_TIME * 2);
+    }
+}

vmm/src/vmm_config/net.rs

@@ -65,7 +65,7 @@ impl NetworkInterfaceConfig {
 
 /// The data fed into a network iface update request. Currently, only the RX and TX rate limiters
 /// can be updated.
-#[derive(Debug, Deserialize)]
+#[derive(Debug, Deserialize, PartialEq)]
 #[serde(deny_unknown_fields)]
 pub struct NetworkInterfaceUpdateConfig {
     /// The net iface ID, as provided by the user at iface creation time.
@@ -290,6 +290,7 @@ mod tests {
 
     use super::*;
     use net_util::MacAddr;
+    use VmmActionError::NetworkConfig;
 
     fn create_netif(id: &str, name: &str, mac: &str) -> NetworkInterfaceConfig {
         NetworkInterfaceConfig {
@@ -426,4 +427,56 @@ mod tests {
             expected_error
         );
     }
+
+    #[test]
+    fn test_error_display() {
+        let _ = format!(
+            "{}{:?}",
+            NetworkInterfaceError::GuestMacAddressInUse("00:00:00:00:00:00".to_string()),
+            NetworkInterfaceError::GuestMacAddressInUse("00:00:00:00:00:00".to_string())
+        );
+        let _ = format!(
+            "{}{:?}",
+            NetworkInterfaceError::EpollHandlerNotFound(
+                VmmInternalError::DeviceEventHandlerNotFound
+            ),
+            NetworkInterfaceError::EpollHandlerNotFound(
+                VmmInternalError::DeviceEventHandlerNotFound
+            )
+        );
+        let _ = format!(
+            "{}{:?}",
+            NetworkInterfaceError::HostDeviceNameInUse("hostdev".to_string()),
+            NetworkInterfaceError::HostDeviceNameInUse("hostdev".to_string())
+        );
+        let _ = format!(
+            "{}{:?}",
+            NetworkInterfaceError::DeviceIdNotFound,
+            NetworkInterfaceError::DeviceIdNotFound
+        );
+        let _ = format!(
+            "{}{:?}",
+            NetworkInterfaceError::OpenTap(TapError::InvalidIfname),
+            NetworkInterfaceError::OpenTap(TapError::InvalidIfname)
+        );
+        let _ = format!(
+            "{}{:?}",
+            NetworkInterfaceError::RateLimiterError(io::Error::last_os_error()),
+            NetworkInterfaceError::RateLimiterError(io::Error::last_os_error())
+        );
+        let _ = format!(
+            "{}{:?}",
+            NetworkInterfaceError::RateLimiterUpdateFailed(devices::Error::IoError(
+                io::Error::last_os_error()
+            )),
+            NetworkInterfaceError::RateLimiterUpdateFailed(devices::Error::IoError(
+                io::Error::last_os_error()
+            ))
+        );
+        let _ = format!(
+            "{}{:?}",
+            NetworkInterfaceError::UpdateNotAllowedPostBoot,
+            NetworkInterfaceError::UpdateNotAllowedPostBoot
+        );
+    }
 }