diff --git a/resources/seccomp/x86_64-unknown-linux-musl.json b/resources/seccomp/x86_64-unknown-linux-musl.json index c3462d2f86b..67929c90a62 100644 --- a/resources/seccomp/x86_64-unknown-linux-musl.json +++ b/resources/seccomp/x86_64-unknown-linux-musl.json @@ -799,6 +799,12 @@ "syscall": "sigaltstack", "comment": "sigaltstack is used by Rust stdlib to remove alternative signal stack during thread teardown." }, + { + "syscall": "fcntl" + }, + { + "syscall": "ftruncate" + }, { "syscall": "futex", "comment": "Used for synchronization (during thread teardown when joining multiple vcpu threads at once)", @@ -890,6 +896,56 @@ } ] }, + { + "syscall": "memfd_create" + }, + { + "syscall": "mmap", + "comment": "Used to recreate TX/RX queues in VirtIO net device", + "args": [ + { + "index": 3, + "type": "dword", + "op": "eq", + "val": 34, + "comment": "libc::MAP_ANONYMOUS|libc::MAP_PRIVATE" + }, + { + "index": 2, + "type": "dword", + "op": "eq", + "val": 0, + "comment": "libc::PROT_NONE" + } + ] + }, + { + "syscall": "mmap", + "comment": "Used to recreate TX/RX queues in VirtIO net device", + "args": [ + { + "index": 4, + "type": "dword", + "op": "eq", + "val": 5, + "comment": "file descriptor" + }, + { + "index": 3, + "type": "dword", + "op": "eq", + "val": 17, + "comment": "libc::MAP_SHARED|libc::MAP_FIXED" + }, + { + "index": 2, + "type": "dword", + "op": "eq", + "val": 3, + "comment": "libc::PROT_READ|libc::PROT_WRITE" + } + ] + }, { "syscall": "mmap", "comment": "Used for reading the timezone in LocalTime::now()", diff --git a/src/vmm/src/devices/virtio/device.rs b/src/vmm/src/devices/virtio/device.rs index ba1ca6b279e..405b3a8eca0 100644 --- a/src/vmm/src/devices/virtio/device.rs +++ b/src/vmm/src/devices/virtio/device.rs @@ -11,7 +11,7 @@ use std::sync::atomic::{AtomicU32, Ordering}; use vmm_sys_util::eventfd::EventFd; -use super::ActivateError; +use super::{ActivateError, ResetError}; use super::mmio::{VIRTIO_MMIO_INT_CONFIG, VIRTIO_MMIO_INT_VRING}; use super::queue::{Queue, QueueError}; use crate::devices::virtio::AsAny; @@ -175,10 +175,9 @@ pub trait VirtioDevice: AsAny + Send { /// Checks if the resources of this device are activated. fn is_activated(&self) -> bool; - /// Optionally deactivates this device and returns ownership of the guest memory map, interrupt - /// event, and queue events. - fn reset(&mut self) -> Option<(EventFd, Vec)> { - None + /// Optionally deactivates this device. + fn reset(&mut self) -> Result<(), ResetError> { + Err(ResetError::NotImplemented) } /// Mark pages used by queues as dirty. diff --git a/src/vmm/src/devices/virtio/mmio.rs b/src/vmm/src/devices/virtio/mmio.rs index 4114838bdd3..04673f6819c 100644 --- a/src/vmm/src/devices/virtio/mmio.rs +++ b/src/vmm/src/devices/virtio/mmio.rs @@ -204,8 +204,12 @@ impl MmioTransport { let mut device_status = self.device_status; let reset_result = self.locked_device().reset(); match reset_result { - Some((_interrupt_evt, mut _queue_evts)) => {} - None => { + Ok(_) => { + // The device MUST initialize device status to 0 upon reset. + device_status = INIT; + } + Err(e) => { + warn!("failed to reset virtio device: {:?}", e); device_status |= FAILED; } } @@ -471,7 +475,7 @@ pub(crate) mod tests { let m = single_region_mem(0x1000); let mut dummy = DummyDevice::new(); // Validate reset is no-op. - assert!(dummy.reset().is_none()); + assert!(dummy.reset().is_err()); let mut d = MmioTransport::new(m, Arc::new(Mutex::new(dummy)), false); // We just make sure here that the implementation of a mmio device behaves as we expect, diff --git a/src/vmm/src/devices/virtio/mod.rs b/src/vmm/src/devices/virtio/mod.rs index f298d28e9bd..a62aa408d82 100644 --- a/src/vmm/src/devices/virtio/mod.rs +++ b/src/vmm/src/devices/virtio/mod.rs @@ -76,6 +76,15 @@ pub enum ActivateError { QueueMemoryError(QueueError), } +// Errors triggered when resetting a VirtioDevice. +#[derive(Debug, thiserror::Error, displaydoc::Display)] +pub enum ResetError { + /// Error when creating RX buffers + RxBuffer, + /// Reset is not implemented for the device. + NotImplemented, +} + /// Trait that helps in upcasting an object to Any pub trait AsAny { /// Return the immutable any encapsulated object. diff --git a/src/vmm/src/devices/virtio/net/device.rs b/src/vmm/src/devices/virtio/net/device.rs index 2ce60707271..64ee7838c6c 100755 --- a/src/vmm/src/devices/virtio/net/device.rs +++ b/src/vmm/src/devices/virtio/net/device.rs @@ -32,7 +32,7 @@ use crate::devices::virtio::net::{ MAX_BUFFER_SIZE, NET_QUEUE_SIZES, NetError, NetQueue, RX_INDEX, TX_INDEX, generated, }; use crate::devices::virtio::queue::{DescriptorChain, InvalidAvailIdx, Queue}; -use crate::devices::virtio::{ActivateError, TYPE_NET}; +use crate::devices::virtio::{ActivateError, ResetError, TYPE_NET}; use crate::devices::{DeviceError, report_net_event_fail}; use crate::dumbo::pdu::arp::ETH_IPV4_FRAME_LEN; use crate::dumbo::pdu::ethernet::{EthernetFrame, PAYLOAD_OFFSET}; @@ -1030,6 +1030,26 @@ impl VirtioDevice for Net { fn is_activated(&self) -> bool { self.device_state.is_activated() } + + fn reset(&mut self) -> Result<(), ResetError> { + self.device_state = DeviceState::Inactive; + self.rx_frame_buf = [0u8; MAX_BUFFER_SIZE]; + self.acked_features = 0; + self.metrics.device_resets.inc(); + let mut queues = Vec::new(); + for size in NET_QUEUE_SIZES { + queues.push(Queue::new(size)); + } + self.tx_buffer = Default::default(); + self.rx_buffer = match RxBuffers::new() { + Ok(rx_buffer) => rx_buffer, + Err(err) => { + error!("Failed to reset RX buffers: {:?}", err); + return Err(ResetError::RxBuffer); + } + }; + Ok(()) + } } #[cfg(test)] @@ -2402,19 +2422,30 @@ pub mod tests { let mem = single_region_mem(2 * MAX_BUFFER_SIZE); let mut th = TestHelper::get_default(&mem); th.activate_net(); - let net = th.net.lock().unwrap(); + let mut net = th.net.lock().unwrap(); - // Test queues count (TX and RX). - let queues = net.queues(); - assert_eq!(queues.len(), NET_QUEUE_SIZES.len()); - assert_eq!(queues[RX_INDEX].size, th.rxq.size()); - assert_eq!(queues[TX_INDEX].size, th.txq.size()); + let validate = |net: &Net| { + // Test queues count (TX and RX). + let queues = net.queues(); + assert_eq!(queues.len(), NET_QUEUE_SIZES.len()); + assert_eq!(queues[RX_INDEX].size, th.rxq.size()); + assert_eq!(queues[TX_INDEX].size, th.txq.size()); + + // Test corresponding queues events. + assert_eq!(net.queue_events().len(), NET_QUEUE_SIZES.len()); + + // Test interrupts. + assert!(!&net.irq_trigger.has_pending_irq(IrqType::Vring)); + }; + + validate(&net); - // Test corresponding queues events. - assert_eq!(net.queue_events().len(), NET_QUEUE_SIZES.len()); + // Test reset. + assert!(net.device_state.is_activated()); + net.reset().unwrap(); + assert!(!net.device_state.is_activated()); - // Test interrupts. - assert!(!&net.irq_trigger.has_pending_irq(IrqType::Vring)); + validate(&net); } #[test] diff --git a/src/vmm/src/devices/virtio/net/metrics.rs b/src/vmm/src/devices/virtio/net/metrics.rs index 60e03f224de..efabf2d69a0 100644 --- a/src/vmm/src/devices/virtio/net/metrics.rs +++ b/src/vmm/src/devices/virtio/net/metrics.rs @@ -150,6 +150,8 @@ pub struct NetDeviceMetrics { /// Number of times when interacting with the space config of a network device failed. pub cfg_fails: SharedIncMetric, /// Number of times the mac address was updated through the config space. + /// Number of device resets. + pub device_resets: SharedIncMetric, pub mac_address_updates: SharedIncMetric, /// No available buffer for the net device rx queue. pub no_rx_avail_buffer: SharedIncMetric, @@ -222,6 +224,7 @@ impl NetDeviceMetrics { pub fn aggregate(&mut self, other: &Self) { self.activate_fails.add(other.activate_fails.fetch_diff()); self.cfg_fails.add(other.cfg_fails.fetch_diff()); + self.device_resets.add(other.device_resets.fetch_diff()); self.mac_address_updates .add(other.mac_address_updates.fetch_diff()); self.no_rx_avail_buffer diff --git a/tests/integration_tests/functional/test_net.py b/tests/integration_tests/functional/test_net.py index 7abf23406d5..318b0ffc42a 100644 --- a/tests/integration_tests/functional/test_net.py +++ b/tests/integration_tests/functional/test_net.py @@ -127,3 +127,38 @@ def test_tap_offload(uvm_any): with attempt: ret = vm.ssh.check_output(f"sync; cat {out_filename}") assert ret.stdout == message, f"{ret.stdout=} {ret.stderr=}" + +def test_reset(uvm_plain_any): + """ + Verify that we can reset a net device in the guest + """ + vm = uvm_plain_any + vm.spawn() + vm.basic_config() + + # eth0/virtio1 + vm.add_net_iface() + # eth1/virtio2 + vm.add_net_iface() + guest_ip_eth1 = vm.iface["eth1"]["iface"].guest_ip + + vm.start() + + # Check eth1 + vm.ssh.check_output("ping -I eth1 192.168.0.1 -c 1 -W 1") + + # Trigger reset of eth1 in the guest + vm.ssh.check_output("echo virtio2 > /sys/bus/virtio/devices/virtio2/driver/unbind") + + exitcode, _, _ = vm.ssh.run("ping -I eth1 192.168.0.1 -c 1 -W 1") + assert exitcode != 0, "Ping should fail after resetting the net device" + + # Bring eth1 back up + vm.ssh.check_output(f""" + echo virtio2 > /sys/bus/virtio/drivers/virtio_net/bind; + ip addr add {guest_ip_eth1}/30 dev eth1; + ip link set eth1 up; + ip route add default via 192.168.0.1; + """) + + vm.ssh.check_output("ping -I eth1 192.168.0.1 -c 1 -W 1")