Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 56 additions & 0 deletions resources/seccomp/x86_64-unknown-linux-musl.json
Original file line number Diff line number Diff line change
Expand Up @@ -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)",
Expand Down Expand Up @@ -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()",
Expand Down
9 changes: 4 additions & 5 deletions src/vmm/src/devices/virtio/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<EventFd>)> {
None
/// Optionally deactivates this device.
fn reset(&mut self) -> Result<(), ResetError> {
Err(ResetError::NotImplemented)
Comment on lines +179 to +180
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's what I had in mind honestly. I assume the initial idea of giving back the EventFds was to "ensure" through the type system that they can't be used, but that is a very dubious choice, IMHO.

}

/// Mark pages used by queues as dirty.
Expand Down
10 changes: 7 additions & 3 deletions src/vmm/src/devices/virtio/mmio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down Expand Up @@ -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,
Expand Down
9 changes: 9 additions & 0 deletions src/vmm/src/devices/virtio/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
53 changes: 42 additions & 11 deletions src/vmm/src/devices/virtio/net/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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)]
Expand Down Expand Up @@ -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]
Expand Down
3 changes: 3 additions & 0 deletions src/vmm/src/devices/virtio/net/metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
35 changes: 35 additions & 0 deletions tests/integration_tests/functional/test_net.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")