From b0cec2776268c4bf47b5a54fc31631b355c48616 Mon Sep 17 00:00:00 2001 From: Nikita Kalyazin Date: Mon, 30 Sep 2024 13:05:28 +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 | 7 +++++++ src/vmm/src/devices/virtio/net/device.rs | 2 +- src/vmm/src/devices/virtio/net/persist.rs | 9 ++++++++- 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2afff333952..1bb1e1ae429 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,13 @@ and this project adheres to - [#4790](https://github.com/firecracker-microvm/firecracker/pull/4790): v1.9.0 was missing most of the debugging information in the debuginfo file, due to a change in the Cargo defaults. This has been corrected. +- [#4826](https://github.com/firecracker-microvm/firecracker/pull/4826): 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\] diff --git a/src/vmm/src/devices/virtio/net/device.rs b/src/vmm/src/devices/virtio/net/device.rs index feb7488cc05..f8c29f95175 100755 --- a/src/vmm/src/devices/virtio/net/device.rs +++ b/src/vmm/src/devices/virtio/net/device.rs @@ -665,7 +665,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 54ff724ce51..4f0ae35d966 100644 --- a/src/vmm/src/devices/virtio/net/persist.rs +++ b/src/vmm/src/devices/virtio/net/persist.rs @@ -10,7 +10,7 @@ use std::sync::{Arc, Mutex}; use serde::{Deserialize, Serialize}; 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); }