From 279cf5c74eeafc313c3783604d1d1966544da9bf Mon Sep 17 00:00:00 2001 From: Nikita Kalyazin Date: Wed, 2 Oct 2024 11:20:36 +0000 Subject: [PATCH] fix(net): set tap offload features on restore Tap offload features configuration was moved from the device creation time to the device activation time by the following commit: commit 1e5d3dba6a92358e3bee3ccb429ef383c479c588 Author: Nikita Zakirov Date: Fri Jan 19 15:48:21 2024 +0000 fix(net): Apply only supported TAP offloading features Since device activation code is only called on the boot path, the features were not automatically configured on the restore path. This change configures them on the restore path as well. The change does not include a unit test as we do not have a mockable interface for the tap device. The change does not include an integration test as we have not yet found a way to reproduce the issue using the existing test framework. Signed-off-by: Nikita Kalyazin --- CHANGELOG.md | 12 ++++++++++++ src/vmm/src/devices/virtio/net/device.rs | 2 +- src/vmm/src/devices/virtio/net/persist.rs | 9 ++++++++- 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 143412c50f5..e0ad7ca78fd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,18 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## \[Unreleased\] + +### Fixed + +- [#4824](https://github.com/firecracker-microvm/firecracker/pull/4824): Add + missing configuration of tap offload features when restoring from a snapshot. + Setting the features was previously + [moved](https://github.com/firecracker-microvm/firecracker/pull/4680/commits/49ed5ea4b48ccd98903da037368fa3108f58ac1f) + from net device creation to device activation time, but it was not reflected + in the restore path. This was leading to inability to connect to the restored + VM if the offload features were used. + ## \[1.9.0\] ### Added diff --git a/src/vmm/src/devices/virtio/net/device.rs b/src/vmm/src/devices/virtio/net/device.rs index e34676b2c31..d6543794e02 100755 --- a/src/vmm/src/devices/virtio/net/device.rs +++ b/src/vmm/src/devices/virtio/net/device.rs @@ -669,7 +669,7 @@ impl Net { /// Builds the offload features we will setup on the TAP device based on the features that the /// guest supports. - fn build_tap_offload_features(guest_supported_features: u64) -> u32 { + pub fn build_tap_offload_features(guest_supported_features: u64) -> u32 { let add_if_supported = |tap_features: &mut u32, supported_features: u64, tap_flag: u32, virtio_flag: u32| { if supported_features & (1 << virtio_flag) != 0 { diff --git a/src/vmm/src/devices/virtio/net/persist.rs b/src/vmm/src/devices/virtio/net/persist.rs index 271977a4792..eb482e29af8 100644 --- a/src/vmm/src/devices/virtio/net/persist.rs +++ b/src/vmm/src/devices/virtio/net/persist.rs @@ -11,7 +11,7 @@ use serde::{Deserialize, Serialize}; use utils::net::mac::MacAddr; use super::device::Net; -use super::NET_NUM_QUEUES; +use super::{TapError, NET_NUM_QUEUES}; use crate::devices::virtio::device::DeviceState; use crate::devices::virtio::persist::{PersistError as VirtioStateError, VirtioDeviceState}; use crate::devices::virtio::queue::FIRECRACKER_MAX_QUEUE_SIZE; @@ -65,6 +65,8 @@ pub enum NetPersistError { VirtioState(#[from] VirtioStateError), /// Indicator that no MMDS is associated with this device. NoMmdsDataStore, + /// Setting tap interface offload flags failed: {0} + TapSetOffload(TapError), } impl Persist<'_> for Net { @@ -129,6 +131,11 @@ impl Persist<'_> for Net { net.acked_features = state.virtio_state.acked_features; if state.virtio_state.activated { + let supported_flags: u32 = Net::build_tap_offload_features(net.acked_features); + net.tap + .set_offload(supported_flags) + .map_err(NetPersistError::TapSetOffload)?; + net.device_state = DeviceState::Activated(constructor_args.mem); }