From ed542761e6a64377c4ad6ceb8b3edcdcdbf206d7 Mon Sep 17 00:00:00 2001 From: Nikita Zakirov Date: Fri, 19 Jan 2024 15:48:21 +0000 Subject: [PATCH] fix(net): Apply only supported TAP offloading features virtio-net driver may not support all offloading features supported by a TAP device. This commit setup only those features, that got acknowledged by the driver. Signed-off-by: Nikita Zakirov --- src/vmm/src/devices/virtio/mod.rs | 4 + src/vmm/src/devices/virtio/net/device.rs | 96 ++++++++++++++++++++++-- src/vmm/src/devices/virtio/net/mod.rs | 2 - 3 files changed, 92 insertions(+), 10 deletions(-) diff --git a/src/vmm/src/devices/virtio/mod.rs b/src/vmm/src/devices/virtio/mod.rs index e79787223c4..6f7302cf188 100644 --- a/src/vmm/src/devices/virtio/mod.rs +++ b/src/vmm/src/devices/virtio/mod.rs @@ -10,6 +10,8 @@ use std::any::Any; use std::io::Error as IOError; +use crate::devices::virtio::net::TapError; + pub mod balloon; pub mod block_common; pub mod device; @@ -68,6 +70,8 @@ pub enum ActivateError { BadActivate, /// Vhost user: {0} VhostUser(vhost_user::VhostUserError), + /// Setting tap interface offload flags failed: {0} + TapSetOffload(TapError), } /// Trait that helps in upcasting an object to Any diff --git a/src/vmm/src/devices/virtio/net/device.rs b/src/vmm/src/devices/virtio/net/device.rs index 9d8cdfd7372..ce405f6555f 100755 --- a/src/vmm/src/devices/virtio/net/device.rs +++ b/src/vmm/src/devices/virtio/net/device.rs @@ -23,8 +23,10 @@ use vm_memory::GuestMemoryError; use crate::devices::virtio::device::{DeviceState, IrqTrigger, IrqType, VirtioDevice}; use crate::devices::virtio::gen::virtio_blk::VIRTIO_F_VERSION_1; use crate::devices::virtio::gen::virtio_net::{ - virtio_net_hdr_v1, VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM, VIRTIO_NET_F_GUEST_TSO4, - VIRTIO_NET_F_GUEST_UFO, VIRTIO_NET_F_HOST_TSO4, VIRTIO_NET_F_HOST_UFO, VIRTIO_NET_F_MAC, + virtio_net_hdr_v1, VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM, VIRTIO_NET_F_GUEST_ECN, + VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_TSO6, VIRTIO_NET_F_GUEST_UFO, + VIRTIO_NET_F_HOST_ECN, VIRTIO_NET_F_HOST_TSO4, VIRTIO_NET_F_HOST_TSO6, VIRTIO_NET_F_HOST_UFO, + VIRTIO_NET_F_MAC, }; use crate::devices::virtio::gen::virtio_ring::VIRTIO_RING_F_EVENT_IDX; use crate::devices::virtio::iovec::IoVecBuffer; @@ -160,7 +162,11 @@ impl Net { | 1 << VIRTIO_NET_F_HOST_TSO4 | 1 << VIRTIO_NET_F_HOST_UFO | 1 << VIRTIO_F_VERSION_1 - | 1 << VIRTIO_RING_F_EVENT_IDX; + | 1 << VIRTIO_RING_F_EVENT_IDX + | 1 << VIRTIO_NET_F_GUEST_TSO6 + | 1 << VIRTIO_NET_F_HOST_TSO6 + | 1 << VIRTIO_NET_F_GUEST_ECN + | 1 << VIRTIO_NET_F_HOST_ECN; let mut config_space = ConfigSpace::default(); if let Some(mac) = guest_mac { @@ -210,10 +216,6 @@ impl Net { ) -> Result { let tap = Tap::open_named(tap_if_name).map_err(NetError::TapOpen)?; - // Set offload flags to match the virtio features below. - tap.set_offload(gen::TUN_F_CSUM | gen::TUN_F_UFO | gen::TUN_F_TSO4 | gen::TUN_F_TSO6) - .map_err(NetError::TapSetOffload)?; - let vnet_hdr_size = i32::try_from(vnet_hdr_len()).unwrap(); tap.set_vnet_hdr_size(vnet_hdr_size) .map_err(NetError::TapSetVnetHdrSize)?; @@ -644,6 +646,44 @@ impl Net { } } + fn build_tap_supported_features(virtio_supported_features: u64) -> u32 { + let add_if_supported = + |tap_features: &mut u32, virtio_features: u64, tap_flag: u32, virtio_flag: u32| { + if virtio_features & (1 << virtio_flag) != 0 { + *tap_features |= tap_flag; + } + }; + + let mut tap_features: u32 = 0; + + add_if_supported( + &mut tap_features, + virtio_supported_features, + gen::TUN_F_CSUM, + VIRTIO_NET_F_CSUM, + ); + add_if_supported( + &mut tap_features, + virtio_supported_features, + gen::TUN_F_UFO, + VIRTIO_NET_F_GUEST_UFO, + ); + add_if_supported( + &mut tap_features, + virtio_supported_features, + gen::TUN_F_TSO4, + VIRTIO_NET_F_GUEST_TSO4, + ); + add_if_supported( + &mut tap_features, + virtio_supported_features, + gen::TUN_F_TSO6, + VIRTIO_NET_F_GUEST_TSO6, + ); + + tap_features + } + /// Updates the parameters for the rate limiters pub fn patch_rate_limiters( &mut self, @@ -859,6 +899,11 @@ impl VirtioDevice for Net { } } + let supported_flags: u32 = Net::build_tap_supported_features(self.acked_features); + self.tap + .set_offload(supported_flags) + .map_err(super::super::ActivateError::TapSetOffload)?; + if self.activate_evt.write(1).is_err() { error!("Net: Cannot write to activate_evt"); return Err(super::super::ActivateError::BadActivate); @@ -978,7 +1023,11 @@ pub mod tests { | 1 << VIRTIO_NET_F_HOST_TSO4 | 1 << VIRTIO_NET_F_HOST_UFO | 1 << VIRTIO_F_VERSION_1 - | 1 << VIRTIO_RING_F_EVENT_IDX; + | 1 << VIRTIO_RING_F_EVENT_IDX + | 1 << VIRTIO_NET_F_GUEST_TSO6 + | 1 << VIRTIO_NET_F_HOST_TSO6 + | 1 << VIRTIO_NET_F_GUEST_ECN + | 1 << VIRTIO_NET_F_HOST_ECN; assert_eq!( net.avail_features_by_page(0), @@ -996,6 +1045,37 @@ pub mod tests { assert_eq!(net.acked_features, features); } + #[test] + // We can't get offload features, that were set up to a TAP device by ioctl, + // hence that we have to validate, that we sort out unsupported features correctly + fn test_build_tap_supported_features_all() { + let supported_features = 1 << VIRTIO_NET_F_CSUM + | 1 << VIRTIO_NET_F_GUEST_UFO + | 1 << VIRTIO_NET_F_GUEST_TSO4 + | 1 << VIRTIO_NET_F_GUEST_TSO6; + + let expected_tap_features = + gen::TUN_F_CSUM | gen::TUN_F_UFO | gen::TUN_F_TSO4 | gen::TUN_F_TSO6; + + let supported_flags = Net::build_tap_supported_features(supported_features); + + assert_eq!(supported_flags, expected_tap_features); + } + + #[test] + fn test_build_tap_supported_features_one_by_one() { + let features = [ + (1 << VIRTIO_NET_F_CSUM, gen::TUN_F_CSUM), + (1 << VIRTIO_NET_F_GUEST_UFO, gen::TUN_F_UFO), + (1 << VIRTIO_NET_F_GUEST_TSO4, gen::TUN_F_TSO4), + (1 << VIRTIO_NET_F_GUEST_TSO6, gen::TUN_F_TSO6), + ]; + for (virtio_flag, tap_flag) in features { + let supported_flags = Net::build_tap_supported_features(virtio_flag); + assert_eq!(supported_flags, tap_flag); + } + } + #[test] fn test_virtio_device_read_config() { let mut net = default_net(); diff --git a/src/vmm/src/devices/virtio/net/mod.rs b/src/vmm/src/devices/virtio/net/mod.rs index 52d4cb92d7b..03bd13660c7 100644 --- a/src/vmm/src/devices/virtio/net/mod.rs +++ b/src/vmm/src/devices/virtio/net/mod.rs @@ -45,8 +45,6 @@ pub enum NetQueue { pub enum NetError { /// Open tap device failed: {0} TapOpen(TapError), - /// Setting tap interface offload flags failed: {0} - TapSetOffload(TapError), /// Setting vnet header size failed: {0} TapSetVnetHdrSize(TapError), /// EventFd error: {0}