From 5466ea5cafc94206a6a813cf104b818faefc8947 Mon Sep 17 00:00:00 2001 From: Mark Vainomaa Date: Sun, 31 May 2026 03:43:04 +0300 Subject: [PATCH 01/10] net_util: tap: add set_queue() helper around TUNSETQUEUE Wraps the IFF_ATTACH_QUEUE / IFF_DETACH_QUEUE TUNSETQUEUE ioctl. Detaching a tap fd from a multi-queue tun removes it from tun_select_queue()'s steering set, so inbound packets stop being delivered to that fd. Needed to honour VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET and to disable queue pairs the guest never activated; both are no-ops on the kernel side until this ioctl is invoked. Signed-off-by: Mark Vainomaa --- net_util/src/tap.rs | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/net_util/src/tap.rs b/net_util/src/tap.rs index 6ec4f0ca77..82a5d74d3d 100644 --- a/net_util/src/tap.rs +++ b/net_util/src/tap.rs @@ -454,6 +454,32 @@ impl Tap { } } + /// Attach or detach this tap from the underlying multi-queue tun. + /// + /// Only meaningful for taps opened with `IFF_MULTI_QUEUE` (i.e. + /// `num_queue_pairs > 1`). When detached, the kernel's `tun_select_queue()` + /// stops steering inbound packets to this fd, so the corresponding RX + /// virtqueue receives nothing. + /// + /// Used to honour `VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET` from the guest and to + /// disable queue pairs the guest never activated -- without this, kernel + /// queue-selection keeps sharing RX across all bound tap fds even when + /// only some of them have an active worker on the userspace side. + pub fn set_queue(&self, attach: bool) -> Result<()> { + let ifru_flags = if attach { + libc::IFF_ATTACH_QUEUE + } else { + libc::IFF_DETACH_QUEUE + } as c_short; + let ifreq = libc::ifreq { + ifr_name: [0; libc::IFNAMSIZ], + ifr_ifru: __c_anonymous_ifr_ifru { ifru_flags }, + }; + // SAFETY: IOCTL with correct arguments -- ifreq is a kernel-defined + // struct and we pass a valid tap fd. + unsafe { Self::ioctl_with_ref(&self.tap_file, libc::TUNSETQUEUE as c_ulong, &ifreq) } + } + /// Enable the tap interface. pub fn enable(&self) -> Result<()> { let sock = create_unix_socket().map_err(Error::NetUtil)?; From ca49fa5eb8940ecd7bd1d43d30c4a136ebb0ad01 Mon Sep 17 00:00:00 2001 From: Mark Vainomaa Date: Sun, 31 May 2026 03:56:08 +0300 Subject: [PATCH 02/10] seccomp: allow TUNSETQUEUE on Vmm and VirtioNetCtl threads Signed-off-by: Mark Vainomaa --- virtio-devices/src/seccomp_filters.rs | 3 ++- vmm/src/seccomp_filters.rs | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/virtio-devices/src/seccomp_filters.rs b/virtio-devices/src/seccomp_filters.rs index 2cb11fff5c..97267b5b03 100644 --- a/virtio-devices/src/seccomp_filters.rs +++ b/virtio-devices/src/seccomp_filters.rs @@ -5,7 +5,7 @@ // SPDX-License-Identifier: Apache-2.0 use block::{BLKDISCARD, BLKZEROOUT}; -use libc::{FIONBIO, TIOCGWINSZ, TUNSETOFFLOAD}; +use libc::{FIONBIO, TIOCGWINSZ, TUNSETOFFLOAD, TUNSETQUEUE}; use seccompiler::SeccompCmpOp::Eq; use seccompiler::{ BpfProgram, Error, SeccompAction, SeccompCmpArgLen as ArgLen, SeccompCondition as Cond, @@ -175,6 +175,7 @@ fn virtio_net_thread_rules() -> Vec<(i64, Vec)> { fn create_virtio_net_ctl_ioctl_seccomp_rule() -> Vec { or![ and![Cond::new(1, ArgLen::Dword, Eq, TUNSETOFFLOAD as _).unwrap()], + and![Cond::new(1, ArgLen::Dword, Eq, TUNSETQUEUE as _).unwrap()], #[cfg(feature = "sev_snp")] mshv_sev_snp_ioctl_seccomp_rule(), ] diff --git a/vmm/src/seccomp_filters.rs b/vmm/src/seccomp_filters.rs index 8ee1d17082..bcc1056f89 100644 --- a/vmm/src/seccomp_filters.rs +++ b/vmm/src/seccomp_filters.rs @@ -9,7 +9,7 @@ use libc::{ BLKIOMIN, BLKIOOPT, BLKPBSZGET, BLKSSZGET, FIOCLEX, FIONBIO, SIOCGIFFLAGS, SIOCGIFHWADDR, SIOCGIFINDEX, SIOCGIFMTU, SIOCSIFADDR, SIOCSIFFLAGS, SIOCSIFHWADDR, SIOCSIFMTU, SIOCSIFNETMASK, TCGETS, TCGETS2, TCSETS, TCSETS2, TIOCGPGRP, TIOCGPTPEER, TIOCGWINSZ, TIOCSCTTY, TIOCSPGRP, - TIOCSPTLCK, TUNGETFEATURES, TUNGETIFF, TUNSETIFF, TUNSETOFFLOAD, TUNSETVNETHDRSZ, + TIOCSPTLCK, TUNGETFEATURES, TUNGETIFF, TUNSETIFF, TUNSETOFFLOAD, TUNSETQUEUE, TUNSETVNETHDRSZ, }; use seccompiler::SeccompCmpOp::Eq; use seccompiler::{ @@ -348,6 +348,7 @@ fn create_vmm_ioctl_seccomp_rule_common( and![Cond::new(1, ArgLen::Dword, Eq, TUNGETIFF as _)?], and![Cond::new(1, ArgLen::Dword, Eq, TUNSETIFF as _)?], and![Cond::new(1, ArgLen::Dword, Eq, TUNSETOFFLOAD as _)?], + and![Cond::new(1, ArgLen::Dword, Eq, TUNSETQUEUE as _)?], and![Cond::new(1, ArgLen::Dword, Eq, TUNSETVNETHDRSZ as _)?], and![Cond::new(1, ArgLen::Dword, Eq, VFIO_GET_API_VERSION)?], and![Cond::new(1, ArgLen::Dword, Eq, VFIO_CHECK_EXTENSION)?], From 18826f54f0e7de9de022e53329358abecb12ab09 Mon Sep 17 00:00:00 2001 From: Mark Vainomaa Date: Sun, 31 May 2026 18:59:51 +0300 Subject: [PATCH 03/10] net_util: ctrl_queue: honor VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously this command was acknowledged with VIRTIO_NET_OK but had no effect: the kernel's multi-queue tun kept steering RX across every bound tap fd, leaving packets queued where no userspace worker was reading. Per virtio 1.0+ §5.1.6.5.5 the device must act on the requested pair count, not just acknowledge it. Wire the handler through Tap::set_queue() so the kernel attachment matches the requested pair count. State lives on Net as a shared Arc so the tracker survives reset/re-activate cycles and never re-issues TUNSETQUEUE on a queue already in the target state (which the kernel would reject with EINVAL). The tracker is updated incrementally inside the apply loop so a partial failure leaves it in sync with actual kernel state. The handler also requires VIRTIO_NET_F_MQ to have been negotiated and rejects requests above the device's tap count. vhost-user owns no local taps; the backend handles queue selection via its own protocol, so we pass no tracker and the command is acknowledged without local effect. Forwarding to the vhost-user backend is left as a follow-up. Fixes: https://github.com/cloud-hypervisor/cloud-hypervisor/issues/8306 Signed-off-by: Mark Vainomaa --- net_util/src/ctrl_queue.rs | 148 +++++++++++++++++++++++++-- net_util/src/lib.rs | 2 +- virtio-devices/src/net.rs | 19 +++- virtio-devices/src/vhost_user/net.rs | 11 +- 4 files changed, 169 insertions(+), 11 deletions(-) diff --git a/net_util/src/ctrl_queue.rs b/net_util/src/ctrl_queue.rs index 8b34a33a7a..bf50c43822 100644 --- a/net_util/src/ctrl_queue.rs +++ b/net_util/src/ctrl_queue.rs @@ -2,6 +2,9 @@ // // SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause +use std::sync::Arc; +use std::sync::atomic::{AtomicU16, Ordering}; + use log::{debug, error, info, warn}; use thiserror::Error; use virtio_bindings::virtio_net::{ @@ -18,7 +21,7 @@ use vm_memory::{ByteValued, Bytes, GuestMemoryError}; use vm_virtio::{AccessPlatform, Translatable}; use super::virtio_features_to_tap_offload; -use crate::{GuestMemoryMmap, Tap}; +use crate::{GuestMemoryMmap, Tap, TapError}; #[derive(Error, Debug)] pub enum Error { @@ -79,11 +82,91 @@ fn is_tolerated_ctrl_command(ctrl_hdr: ControlHeader) -> bool { pub struct CtrlQueue { pub taps: Vec, + /// Number of RX/TX queue pairs currently attached to the underlying + /// multi-queue tun. Shared with the owning `Net` device so the tracker + /// survives reset/re-activate cycles and matches actual kernel state. + /// + /// `None` for backends that do not own local taps (e.g. vhost-user), + /// where `VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET` is acknowledged with no + /// local effect -- the backend is expected to manage queue activation + /// out-of-band. FIXME: forward the command to vhost-user backends. + active_queue_pairs: Option>, + /// Maximum queue pairs the device exposes. Bounds the requested count + /// in addition to the spec's `VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN/MAX`. + max_queue_pairs: u16, + /// Whether `VIRTIO_NET_F_MQ` was acknowledged by the driver. Captured + /// at activation time, after feature negotiation has settled. + mq_negotiated: bool, +} + +/// Returns the ordered list of `(queue_index, attach)` ops needed to drive +/// `active` to `desired`, clamped to `max`. Detaches walk from the top down +/// so a partial failure leaves a contiguous prefix of attached queues. +fn plan_queue_pair_delta(active: u16, desired: u16, max: u16) -> Vec<(usize, bool)> { + if max <= 1 { + return Vec::new(); + } + let desired = desired.min(max); + if desired == active { + return Vec::new(); + } + if desired > active { + (active..desired).map(|i| (i as usize, true)).collect() + } else { + (desired..active) + .rev() + .map(|i| (i as usize, false)) + .collect() + } +} + +/// Drive the kernel-side multi-queue attachment for `taps` to `desired` +/// pair count, updating `tracker` incrementally so a partial failure +/// leaves it in sync with kernel state. +/// +/// Used both by the control-queue handler in response to +/// `VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET` and by device activation to align +/// kernel attachment with the queue pairs the guest is about to use. +pub fn align_kernel_queue_pairs( + taps: &[Tap], + tracker: &AtomicU16, + desired: u16, +) -> std::result::Result<(), TapError> { + let max = taps.len() as u16; + let active = tracker.load(Ordering::Acquire); + for (idx, attach) in plan_queue_pair_delta(active, desired, max) { + taps[idx].set_queue(attach)?; + let new_active = if attach { idx as u16 + 1 } else { idx as u16 }; + tracker.store(new_active, Ordering::Release); + } + Ok(()) } impl CtrlQueue { - pub fn new(taps: Vec) -> Self { - CtrlQueue { taps } + pub fn new( + taps: Vec, + active_queue_pairs: Option>, + max_queue_pairs: u16, + mq_negotiated: bool, + ) -> Self { + CtrlQueue { + taps, + active_queue_pairs, + max_queue_pairs, + mq_negotiated, + } + } + + /// Drive the kernel-side multi-queue attachment to `desired` pair count. + /// + /// No-op when this `CtrlQueue` does not own local taps (vhost-user) or + /// when the tap is single-queue (`TUNSETQUEUE` would `EINVAL`). + fn apply_active_queue_pairs(&mut self, desired: u16) -> std::result::Result<(), TapError> { + let Some(tracker) = self.active_queue_pairs.as_ref() else { + debug!("MQ_VQ_PAIRS_SET={desired} acknowledged without local effect"); + return Ok(()); + }; + align_kernel_queue_pairs(&self.taps, tracker, desired) } pub fn process( @@ -122,14 +205,30 @@ impl CtrlQueue { if u32::from(ctrl_hdr.cmd) != VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET { warn!("Unsupported command: {}", ctrl_hdr.cmd); false + } else if !self.mq_negotiated { + warn!("MQ command received without VIRTIO_NET_F_MQ negotiated"); + false } else if (queue_pairs < VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN as u16) || (queue_pairs > VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX as u16) + || (queue_pairs > self.max_queue_pairs) { - warn!("Number of MQ pairs out of range: {queue_pairs}"); + warn!( + "Number of MQ pairs out of range: {queue_pairs} \ + (device max {})", + self.max_queue_pairs + ); false } else { - info!("Number of MQ pairs requested: {queue_pairs}"); - true + match self.apply_active_queue_pairs(queue_pairs) { + Ok(()) => { + info!("Number of MQ pairs set: {queue_pairs}"); + true + } + Err(e) => { + error!("Failed to apply MQ pairs={queue_pairs}: {e}"); + false + } + } } } VIRTIO_NET_CTRL_GUEST_OFFLOADS => { @@ -191,3 +290,40 @@ impl CtrlQueue { Ok(()) } } + +#[cfg(test)] +mod unit_tests { + use super::*; + + #[test] + fn delta_single_queue_device_is_noop() { + assert!(plan_queue_pair_delta(1, 1, 1).is_empty()); + assert!(plan_queue_pair_delta(0, 4, 1).is_empty()); + } + + #[test] + fn delta_same_count_is_noop() { + assert!(plan_queue_pair_delta(3, 3, 8).is_empty()); + } + + #[test] + fn delta_grow_attaches_upper_indices() { + assert_eq!( + plan_queue_pair_delta(1, 4, 8), + vec![(1, true), (2, true), (3, true)], + ); + } + + #[test] + fn delta_shrink_detaches_from_top_down() { + assert_eq!( + plan_queue_pair_delta(4, 1, 8), + vec![(3, false), (2, false), (1, false)], + ); + } + + #[test] + fn delta_clamps_desired_to_device_max() { + assert_eq!(plan_queue_pair_delta(2, 99, 4), vec![(2, true), (3, true)],); + } +} diff --git a/net_util/src/lib.rs b/net_util/src/lib.rs index 7152c1676f..a1741ff315 100644 --- a/net_util/src/lib.rs +++ b/net_util/src/lib.rs @@ -29,7 +29,7 @@ use vm_memory::bitmap::AtomicBitmap; type GuestMemoryMmap = vm_memory::GuestMemoryMmap; -pub use ctrl_queue::{CtrlQueue, Error as CtrlQueueError}; +pub use ctrl_queue::{CtrlQueue, Error as CtrlQueueError, align_kernel_queue_pairs}; pub use mac::{MAC_ADDR_LEN, MacAddr}; pub use open_tap::{Error as OpenTapError, open_tap}; pub use queue_pair::{NetCounters, NetQueuePair, NetQueuePairError, RxVirtio, TxVirtio}; diff --git a/virtio-devices/src/net.rs b/virtio-devices/src/net.rs index 6edc408948..74b2d4ce8d 100644 --- a/virtio-devices/src/net.rs +++ b/virtio-devices/src/net.rs @@ -10,7 +10,7 @@ use std::net::IpAddr; use std::num::Wrapping; use std::ops::Deref; use std::os::unix::io::{AsRawFd, RawFd}; -use std::sync::atomic::{AtomicBool, AtomicU8, Ordering}; +use std::sync::atomic::{AtomicBool, AtomicU8, AtomicU16, Ordering}; use std::sync::{Arc, Barrier}; use std::{result, thread}; @@ -406,6 +406,14 @@ pub struct Net { rate_limiter_config: Option, exit_evt: EventFd, device_status: Arc, + /// Number of queue pairs currently attached to the underlying + /// multi-queue tun. `open_tap`/`from_tap_fd` start with every tap + /// kernel-attached via `IFF_MULTI_QUEUE`, so the initial value matches + /// `taps.len()`. Shared with the spawned `CtrlQueue` worker so that + /// `VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET` survives reset/re-activate cycles + /// without re-issuing `TUNSETQUEUE` on queues already in the requested + /// state (which the kernel would reject with `EINVAL`). + kernel_active_pairs: Arc, } #[derive(Serialize, Deserialize)] @@ -506,6 +514,7 @@ impl Net { ) }; + let kernel_active_pairs = Arc::new(AtomicU16::new(taps.len() as u16)); Ok(Net { common: VirtioCommon { device_type: VirtioDeviceType::Net as u32, @@ -526,6 +535,7 @@ impl Net { rate_limiter_config, exit_evt, device_status: Arc::new(AtomicU8::new(0)), + kernel_active_pairs, }) } @@ -715,7 +725,12 @@ impl VirtioDevice for Net { mem: mem.clone(), kill_evt, pause_evt, - ctrl_q: CtrlQueue::new(self.taps.clone()), + ctrl_q: CtrlQueue::new( + self.taps.clone(), + Some(self.kernel_active_pairs.clone()), + self.taps.len() as u16, + self.common.feature_acked(VIRTIO_NET_F_MQ.into()), + ), queue: ctrl_queue, queue_evt: ctrl_queue_evt, access_platform: self.common.access_platform(), diff --git a/virtio-devices/src/vhost_user/net.rs b/virtio-devices/src/vhost_user/net.rs index 620f876a7c..de9308a052 100644 --- a/virtio-devices/src/vhost_user/net.rs +++ b/virtio-devices/src/vhost_user/net.rs @@ -14,7 +14,7 @@ use virtio_bindings::virtio_net::{ VIRTIO_NET_F_CSUM, VIRTIO_NET_F_CTRL_VQ, 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, VIRTIO_NET_F_MRG_RXBUF, VIRTIO_NET_F_MTU, + VIRTIO_NET_F_MAC, VIRTIO_NET_F_MQ, VIRTIO_NET_F_MRG_RXBUF, VIRTIO_NET_F_MTU, }; use virtio_bindings::virtio_ring::VIRTIO_RING_F_EVENT_IDX; use virtio_queue::QueueT; @@ -300,11 +300,18 @@ impl VirtioDevice for Net { let (kill_evt, pause_evt) = self.vu_common.virtio_common.dup_eventfds(); + let max_queue_pairs = (self.vu_common.vu_num_queues / 2) as u16; + let mq_negotiated = self + .vu_common + .virtio_common + .feature_acked(VIRTIO_NET_F_MQ.into()); let mut ctrl_handler = NetCtrlEpollHandler { mem: mem.clone(), kill_evt, pause_evt, - ctrl_q: CtrlQueue::new(Vec::new()), + // vhost-user owns no local taps; the backend handles queue + // selection via its own protocol, so we pass no tracker. + ctrl_q: CtrlQueue::new(Vec::new(), None, max_queue_pairs, mq_negotiated), queue: ctrl_queue, queue_evt: ctrl_queue_evt, access_platform: None, From 53cf2c3900ddc6200f80728f7c2b7ed731db7406 Mon Sep 17 00:00:00 2001 From: Mark Vainomaa Date: Sun, 31 May 2026 19:03:55 +0300 Subject: [PATCH 04/10] virtio-devices: net: start with one active tap queue MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per virtio 1.0+ §5.1.6.5.5 ("Device operation in multiqueue mode"), multiqueue is disabled by default: after reset/initialization the device must only queue packets on receiveq1, and the driver enables additional pairs by sending VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET. We were instead activating every configured queue pair at device activation. Drive the kernel-side multi-queue attachment down to one pair on each device activation; the guest grows it back via the control queue after negotiating VIRTIO_NET_F_MQ. Without this, the kernel's tun_select_queue() steers RX across every bound tap fd at all times, even when the guest never activated those pairs -- so the share hashed to queues with no userspace worker is silently dropped at the tap. The same trap affects single-queue drivers that never negotiate F_MQ, plus the post-reset path where a previous driver may have grown the count above what the new driver will use. Reuses align_kernel_queue_pairs() so activate and the control queue share the delta-and-tracker logic; this also keeps the in-memory tracker in sync with kernel state across activate/reset/re-activate cycles in a single VM lifetime. Fixes: https://github.com/cloud-hypervisor/cloud-hypervisor/issues/8307 Signed-off-by: Mark Vainomaa --- virtio-devices/src/net.rs | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/virtio-devices/src/net.rs b/virtio-devices/src/net.rs index 74b2d4ce8d..91baa1d608 100644 --- a/virtio-devices/src/net.rs +++ b/virtio-devices/src/net.rs @@ -21,7 +21,8 @@ use log::{debug, error, info, warn}; use net_util::virtio_features_to_tap_offload; use net_util::{ CtrlQueue, MacAddr, NetCounters, NetQueuePair, OpenTapError, RxVirtio, Tap, TapError, TxVirtio, - VirtioNetConfig, build_net_config_space, build_net_config_space_with_mq, open_tap, + VirtioNetConfig, align_kernel_queue_pairs, build_net_config_space, + build_net_config_space_with_mq, open_tap, }; use seccompiler::SeccompAction; use serde::{Deserialize, Serialize}; @@ -409,10 +410,11 @@ pub struct Net { /// Number of queue pairs currently attached to the underlying /// multi-queue tun. `open_tap`/`from_tap_fd` start with every tap /// kernel-attached via `IFF_MULTI_QUEUE`, so the initial value matches - /// `taps.len()`. Shared with the spawned `CtrlQueue` worker so that - /// `VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET` survives reset/re-activate cycles - /// without re-issuing `TUNSETQUEUE` on queues already in the requested - /// state (which the kernel would reject with `EINVAL`). + /// `taps.len()`. `activate()` then drives it down to one, and the + /// spawned `CtrlQueue` worker grows it back in response to + /// `VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET`. Sharing the tracker keeps both + /// ends from re-issuing `TUNSETQUEUE` on queues already in the + /// requested state, which the kernel would reject with `EINVAL`. kernel_active_pairs: Arc, } @@ -714,6 +716,18 @@ impl VirtioDevice for Net { let qp_threads = (num_queues - ctrl_threads) / 2; self.common.paused_sync = Some(Arc::new(Barrier::new(1 + qp_threads + ctrl_threads))); + // Start each activation with one attached tap queue pair, matching + // QEMU's virtio-net model. Multi-queue guests negotiate + // VIRTIO_NET_F_MQ and grow the count via VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET; + // this also covers single-queue drivers that never negotiate F_MQ, + // where leaving all taps attached would otherwise let the kernel + // steer RX to queues with no userspace reader (silently dropping + // hashed traffic at the tap). + align_kernel_queue_pairs(&self.taps, &self.kernel_active_pairs, 1).map_err(|e| { + error!("Failed to align tap queues to single active pair: {e}"); + ActivateError::BadActivate + })?; + if has_ctrl_queue { let ctrl_queue_index = num_queues - 1; let (_, mut ctrl_queue, ctrl_queue_evt) = queues.remove(ctrl_queue_index); From 5c88abc7947a025e5f44a1ad206c4df4dc3d8d0a Mon Sep 17 00:00:00 2001 From: Mark Vainomaa Date: Sun, 31 May 2026 20:47:50 +0300 Subject: [PATCH 05/10] net_util, virtio-devices: abstract MQ backend; wire vhost-user MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Abstract "apply VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET to the underlying transport" behind an MqBackend trait so the same CtrlQueue handler covers both kernel taps (TUNSETQUEUE) and vhost-user backends (VHOST_USER_SET_VRING_ENABLE). Without this, vhost-user devices acknowledged the command with no local effect -- a spec violation per virtio 1.0+ §5.1.6.5.5 -- and the backend never saw the guest's requested pair count. net_util grows TapMqBackend wrapping the existing tap path, plus a small MqBackendError enum so backends outside this crate (vhost-user) can plug in without dragging dependencies into net_util. virtio-devices/vhost_user adds VhostUserMqBackend, which loops over the device's data vrings and toggles each via the now-public VhostUserHandle::set_vring_enable. The control vring (when present) sits past the data range and is not touched. A CtrlQueue constructed with no backend now rejects MQ_VQ_PAIRS_SET with VIRTIO_NET_ERR instead of silently acking; both real call sites provide a backend, so this only affects degenerate/test configs. Signed-off-by: Mark Vainomaa --- net_util/src/ctrl_queue.rs | 89 +++++++++++++------ net_util/src/lib.rs | 2 +- virtio-devices/src/net.rs | 34 ++++--- virtio-devices/src/vhost_user/net.rs | 48 +++++++++- .../src/vhost_user/vu_common_ctrl.rs | 13 ++- 5 files changed, 139 insertions(+), 47 deletions(-) diff --git a/net_util/src/ctrl_queue.rs b/net_util/src/ctrl_queue.rs index bf50c43822..b20479c440 100644 --- a/net_util/src/ctrl_queue.rs +++ b/net_util/src/ctrl_queue.rs @@ -80,17 +80,58 @@ fn is_tolerated_ctrl_command(ctrl_hdr: ControlHeader) -> bool { } } +/// Error returned by an [`MqBackend`]. +/// +/// Variants exist so net_util-internal backends (taps) can surface their +/// native error type; backends defined outside this crate (e.g. vhost-user +/// in `virtio-devices`) erase their error into a string so net_util stays +/// free of cross-crate dependencies. +#[derive(Error, Debug)] +pub enum MqBackendError { + #[error("tap error: {0}")] + Tap(#[from] TapError), + #[error("{0}")] + Other(String), +} + +/// Strategy for honoring `VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET`. +/// +/// Implementations translate "active queue-pair count" to whatever the +/// underlying backend needs: `TUNSETQUEUE` for kernel taps, +/// `VHOST_USER_SET_VRING_ENABLE` for vhost-user, etc. The caller (the +/// `CtrlQueue` MQ handler, or device activation) bounds `pairs` against +/// the device's advertised max before invoking. +pub trait MqBackend: Send { + fn set_active_pairs(&mut self, pairs: u16) -> std::result::Result<(), MqBackendError>; +} + +/// `MqBackend` for the local kernel-tap path. Holds a shared tracker so +/// state survives `CtrlQueue` reconstruction across reset/re-activate +/// cycles and never re-issues `TUNSETQUEUE` on a queue already in the +/// requested state (which the kernel would reject with `EINVAL`). +pub struct TapMqBackend { + taps: Vec, + tracker: Arc, +} + +impl TapMqBackend { + pub fn new(taps: Vec, tracker: Arc) -> Self { + Self { taps, tracker } + } +} + +impl MqBackend for TapMqBackend { + fn set_active_pairs(&mut self, pairs: u16) -> std::result::Result<(), MqBackendError> { + Ok(align_kernel_queue_pairs(&self.taps, &self.tracker, pairs)?) + } +} + pub struct CtrlQueue { pub taps: Vec, - /// Number of RX/TX queue pairs currently attached to the underlying - /// multi-queue tun. Shared with the owning `Net` device so the tracker - /// survives reset/re-activate cycles and matches actual kernel state. - /// - /// `None` for backends that do not own local taps (e.g. vhost-user), - /// where `VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET` is acknowledged with no - /// local effect -- the backend is expected to manage queue activation - /// out-of-band. FIXME: forward the command to vhost-user backends. - active_queue_pairs: Option>, + /// Strategy for applying `VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET`. `None` + /// rejects the command -- only happens for net configurations that + /// have no working MQ backend at all, which should be rare. + mq_backend: Option>, /// Maximum queue pairs the device exposes. Bounds the requested count /// in addition to the spec's `VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN/MAX`. max_queue_pairs: u16, @@ -123,11 +164,7 @@ fn plan_queue_pair_delta(active: u16, desired: u16, max: u16) -> Vec<(usize, boo /// Drive the kernel-side multi-queue attachment for `taps` to `desired` /// pair count, updating `tracker` incrementally so a partial failure /// leaves it in sync with kernel state. -/// -/// Used both by the control-queue handler in response to -/// `VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET` and by device activation to align -/// kernel attachment with the queue pairs the guest is about to use. -pub fn align_kernel_queue_pairs( +fn align_kernel_queue_pairs( taps: &[Tap], tracker: &AtomicU16, desired: u16, @@ -145,28 +182,30 @@ pub fn align_kernel_queue_pairs( impl CtrlQueue { pub fn new( taps: Vec, - active_queue_pairs: Option>, + mq_backend: Option>, max_queue_pairs: u16, mq_negotiated: bool, ) -> Self { CtrlQueue { taps, - active_queue_pairs, + mq_backend, max_queue_pairs, mq_negotiated, } } - /// Drive the kernel-side multi-queue attachment to `desired` pair count. - /// - /// No-op when this `CtrlQueue` does not own local taps (vhost-user) or - /// when the tap is single-queue (`TUNSETQUEUE` would `EINVAL`). - fn apply_active_queue_pairs(&mut self, desired: u16) -> std::result::Result<(), TapError> { - let Some(tracker) = self.active_queue_pairs.as_ref() else { - debug!("MQ_VQ_PAIRS_SET={desired} acknowledged without local effect"); - return Ok(()); + /// Drive the backend to `desired` queue-pair count, or report no + /// backend was wired (which becomes `VIRTIO_NET_ERR` to the guest). + fn apply_active_queue_pairs( + &mut self, + desired: u16, + ) -> std::result::Result<(), MqBackendError> { + let Some(backend) = self.mq_backend.as_mut() else { + return Err(MqBackendError::Other( + "no MQ backend configured for this device".into(), + )); }; - align_kernel_queue_pairs(&self.taps, tracker, desired) + backend.set_active_pairs(desired) } pub fn process( diff --git a/net_util/src/lib.rs b/net_util/src/lib.rs index a1741ff315..efe6452088 100644 --- a/net_util/src/lib.rs +++ b/net_util/src/lib.rs @@ -29,7 +29,7 @@ use vm_memory::bitmap::AtomicBitmap; type GuestMemoryMmap = vm_memory::GuestMemoryMmap; -pub use ctrl_queue::{CtrlQueue, Error as CtrlQueueError, align_kernel_queue_pairs}; +pub use ctrl_queue::{CtrlQueue, Error as CtrlQueueError, MqBackend, MqBackendError, TapMqBackend}; pub use mac::{MAC_ADDR_LEN, MacAddr}; pub use open_tap::{Error as OpenTapError, open_tap}; pub use queue_pair::{NetCounters, NetQueuePair, NetQueuePairError, RxVirtio, TxVirtio}; diff --git a/virtio-devices/src/net.rs b/virtio-devices/src/net.rs index 91baa1d608..a994482eef 100644 --- a/virtio-devices/src/net.rs +++ b/virtio-devices/src/net.rs @@ -20,8 +20,8 @@ use log::{debug, error, info, warn}; #[cfg(not(fuzzing))] use net_util::virtio_features_to_tap_offload; use net_util::{ - CtrlQueue, MacAddr, NetCounters, NetQueuePair, OpenTapError, RxVirtio, Tap, TapError, TxVirtio, - VirtioNetConfig, align_kernel_queue_pairs, build_net_config_space, + CtrlQueue, MacAddr, MqBackend, NetCounters, NetQueuePair, OpenTapError, RxVirtio, Tap, + TapError, TapMqBackend, TxVirtio, VirtioNetConfig, build_net_config_space, build_net_config_space_with_mq, open_tap, }; use seccompiler::SeccompAction; @@ -716,17 +716,20 @@ impl VirtioDevice for Net { let qp_threads = (num_queues - ctrl_threads) / 2; self.common.paused_sync = Some(Arc::new(Barrier::new(1 + qp_threads + ctrl_threads))); - // Start each activation with one attached tap queue pair, matching - // QEMU's virtio-net model. Multi-queue guests negotiate - // VIRTIO_NET_F_MQ and grow the count via VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET; - // this also covers single-queue drivers that never negotiate F_MQ, - // where leaving all taps attached would otherwise let the kernel - // steer RX to queues with no userspace reader (silently dropping - // hashed traffic at the tap). - align_kernel_queue_pairs(&self.taps, &self.kernel_active_pairs, 1).map_err(|e| { - error!("Failed to align tap queues to single active pair: {e}"); - ActivateError::BadActivate - })?; + // Start each activation with one attached tap queue pair (per + // virtio 1.0+ §5.1.6.5.5: multiqueue is disabled by default). + // Multi-queue guests negotiate VIRTIO_NET_F_MQ and grow the count + // via VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET; this also covers + // single-queue drivers that never negotiate F_MQ, where leaving + // all taps attached would otherwise let the kernel steer RX to + // queues with no userspace reader (silently dropping hashed + // traffic at the tap). + TapMqBackend::new(self.taps.clone(), self.kernel_active_pairs.clone()) + .set_active_pairs(1) + .map_err(|e| { + error!("Failed to align tap queues to single active pair: {e}"); + ActivateError::BadActivate + })?; if has_ctrl_queue { let ctrl_queue_index = num_queues - 1; @@ -741,7 +744,10 @@ impl VirtioDevice for Net { pause_evt, ctrl_q: CtrlQueue::new( self.taps.clone(), - Some(self.kernel_active_pairs.clone()), + Some(Box::new(TapMqBackend::new( + self.taps.clone(), + self.kernel_active_pairs.clone(), + ))), self.taps.len() as u16, self.common.feature_acked(VIRTIO_NET_F_MQ.into()), ), diff --git a/virtio-devices/src/vhost_user/net.rs b/virtio-devices/src/vhost_user/net.rs index de9308a052..a8188e5436 100644 --- a/virtio-devices/src/vhost_user/net.rs +++ b/virtio-devices/src/vhost_user/net.rs @@ -6,7 +6,9 @@ use std::sync::{Arc, Barrier, Mutex}; use std::{result, thread}; use log::{error, info}; -use net_util::{CtrlQueue, MacAddr, VirtioNetConfig, build_net_config_space}; +use net_util::{ + CtrlQueue, MacAddr, MqBackend, MqBackendError, VirtioNetConfig, build_net_config_space, +}; use seccompiler::SeccompAction; use vhost::vhost_user::message::{VhostUserProtocolFeatures, VhostUserVirtioFeatures}; use vhost::vhost_user::{FrontendReqHandler, VhostUserFrontend, VhostUserFrontendReqHandler}; @@ -39,6 +41,40 @@ pub type State = VhostUserState; struct BackendReqHandler {} impl VhostUserFrontendReqHandler for BackendReqHandler {} +/// `MqBackend` for vhost-user net. Translates an active queue-pair count +/// into per-data-vring `VHOST_USER_SET_VRING_ENABLE` calls on the backend. +/// The control queue (when present) sits beyond the data vrings and is +/// not touched here. +struct VhostUserMqBackend { + vu: Arc>, + /// Total data vrings the device exposes (= `max_queue_pairs * 2`). + data_vring_count: usize, +} + +impl VhostUserMqBackend { + fn new(vu: Arc>, data_vring_count: usize) -> Self { + Self { + vu, + data_vring_count, + } + } +} + +impl MqBackend for VhostUserMqBackend { + fn set_active_pairs(&mut self, pairs: u16) -> std::result::Result<(), MqBackendError> { + let active_data = (pairs as usize) * 2; + let mut vu = self + .vu + .lock() + .map_err(|e| MqBackendError::Other(format!("vhost-user handle poisoned: {e}")))?; + for q in 0..self.data_vring_count { + vu.set_vring_enable(q, q < active_data) + .map_err(|e| MqBackendError::Other(format!("set_vring_enable({q}): {e}")))?; + } + Ok(()) + } +} + pub struct Net { vu_common: VhostUserCommon, id: String, @@ -305,13 +341,17 @@ impl VirtioDevice for Net { .vu_common .virtio_common .feature_acked(VIRTIO_NET_F_MQ.into()); + let mq_backend: Option> = self.vu_common.vu.as_ref().map(|vu| { + Box::new(VhostUserMqBackend::new( + vu.clone(), + self.vu_common.vu_num_queues, + )) as Box + }); let mut ctrl_handler = NetCtrlEpollHandler { mem: mem.clone(), kill_evt, pause_evt, - // vhost-user owns no local taps; the backend handles queue - // selection via its own protocol, so we pass no tracker. - ctrl_q: CtrlQueue::new(Vec::new(), None, max_queue_pairs, mq_negotiated), + ctrl_q: CtrlQueue::new(Vec::new(), mq_backend, max_queue_pairs, mq_negotiated), queue: ctrl_queue, queue_evt: ctrl_queue_evt, access_platform: None, diff --git a/virtio-devices/src/vhost_user/vu_common_ctrl.rs b/virtio-devices/src/vhost_user/vu_common_ctrl.rs index 23a37c3350..a9e1bfbd8d 100644 --- a/virtio-devices/src/vhost_user/vu_common_ctrl.rs +++ b/virtio-devices/src/vhost_user/vu_common_ctrl.rs @@ -301,14 +301,21 @@ impl VhostUserHandle { fn enable_vhost_user_vrings(&mut self, queue_indexes: Vec, enable: bool) -> Result<()> { for queue_index in queue_indexes { - self.vu - .set_vring_enable(queue_index, enable) - .map_err(Error::VhostUserSetVringEnable)?; + self.set_vring_enable(queue_index, enable)?; } Ok(()) } + /// Toggle a single vring on the backend. Exposed for handlers that + /// need to flip queue activation at runtime (e.g. honoring + /// `VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET`). + pub fn set_vring_enable(&mut self, queue_index: usize, enable: bool) -> Result<()> { + self.vu + .set_vring_enable(queue_index, enable) + .map_err(Error::VhostUserSetVringEnable) + } + pub fn reset_vhost_user(&mut self) -> Result<()> { for queue_index in self.queue_indexes.drain(..) { self.vu From 7954485bcfa1315b384bf4a9b017095469b327d5 Mon Sep 17 00:00:00 2001 From: Mark Vainomaa Date: Sun, 31 May 2026 20:49:09 +0300 Subject: [PATCH 06/10] virtio-devices: vhost_user: net: disable extra vrings on activate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per virtio 1.0+ §5.1.6.5.5 ("Device operation in multiqueue mode"), multiqueue is disabled by default: after reset/initialization the device must only queue packets on receiveq1/transmitq1, and the driver enables additional pairs by sending VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET. vu_common.activate() enables every configured data vring on the backend unconditionally, leaving the backend in a multiqueue-active state the spec forbids. Mirror the local-tap behavior added in the preceding net.rs change: right after vu_common.activate(), drive the backend down to one active pair via VhostUserMqBackend, which issues VHOST_USER_SET_VRING_ENABLE per data vring. The control vring (when present) sits past the data range and is left enabled. The guest grows the active count back through the now-wired ctrl-queue handler after negotiating VIRTIO_NET_F_MQ. Signed-off-by: Mark Vainomaa --- virtio-devices/src/vhost_user/net.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/virtio-devices/src/vhost_user/net.rs b/virtio-devices/src/vhost_user/net.rs index a8188e5436..6365381a6e 100644 --- a/virtio-devices/src/vhost_user/net.rs +++ b/virtio-devices/src/vhost_user/net.rs @@ -401,6 +401,20 @@ impl VirtioDevice for Net { pause_evt, )?; + // vu_common.activate enables every configured data vring. Per + // virtio 1.0+ §5.1.6.5.5 multiqueue is disabled by default, so + // disable everything past the first pair; multi-queue guests + // grow it back via VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET, single-queue + // and pre-MQ drivers see the spec-required initial state. + if let Some(vu) = self.vu_common.vu.as_ref() { + VhostUserMqBackend::new(vu.clone(), self.vu_common.vu_num_queues) + .set_active_pairs(1) + .map_err(|e| { + error!("Failed to disable extra vhost-user data vrings: {e}"); + crate::ActivateError::BadActivate + })?; + } + let paused = self.vu_common.virtio_common.paused.clone(); let paused_sync = self.vu_common.virtio_common.paused_sync.clone(); From e8b8ef77de87bed6ba14689ae220762ced850411 Mon Sep 17 00:00:00 2001 From: Mark Vainomaa Date: Sun, 31 May 2026 20:59:58 +0300 Subject: [PATCH 07/10] seccomp: allow sendmsg/recvmsg on VirtioVhostNetCtl thread The vhost-user net control-queue thread now forwards VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET to the backend by issuing VHOST_USER_SET_VRING_ENABLE on the existing vhost-user UNIX socket (and reading the ack back when REPLY_ACK is negotiated). The thread's seccomp filter previously had no per-thread rules, so the first such forward would trip the filter and tear the device down. Allow sendmsg/recvmsg on this thread, matching the rules already granted to the analogous vhost-user data thread and to the Vmm thread (which handles activate-time vring toggling). Signed-off-by: Mark Vainomaa --- virtio-devices/src/seccomp_filters.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/virtio-devices/src/seccomp_filters.rs b/virtio-devices/src/seccomp_filters.rs index 97267b5b03..60d150157d 100644 --- a/virtio-devices/src/seccomp_filters.rs +++ b/virtio-devices/src/seccomp_filters.rs @@ -240,7 +240,10 @@ fn virtio_generic_vhost_user_thread_rules() -> Vec<(i64, Vec)> { } fn virtio_vhost_net_ctl_thread_rules() -> Vec<(i64, Vec)> { - vec![] + // VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET handling sends + // VHOST_USER_SET_VRING_ENABLE to the backend over the vhost-user + // socket and (when REPLY_ACK is negotiated) reads the ack back. + vec![(libc::SYS_recvmsg, vec![]), (libc::SYS_sendmsg, vec![])] } fn virtio_vhost_net_thread_rules() -> Vec<(i64, Vec)> { From 6445db13b06ff957339c63e002aa700f8762c771 Mon Sep 17 00:00:00 2001 From: Mark Vainomaa Date: Sun, 31 May 2026 21:01:33 +0300 Subject: [PATCH 08/10] net_util: ctrl_queue: unit-test MqBackend routing Cover the three behaviors that the four-rev stack relies on but had no unit coverage: - CtrlQueue.apply_active_queue_pairs forwards the requested count to the configured MqBackend - backend failures surface as MqBackendError back to the caller (which the MQ handler translates to VIRTIO_NET_ERR) - constructing a CtrlQueue with no backend rejects the command rather than silently acking, so misconfigured callers fail loudly A tiny in-module MockBackend records the last requested pair count and optionally returns an error. Signed-off-by: Mark Vainomaa --- net_util/src/ctrl_queue.rs | 48 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/net_util/src/ctrl_queue.rs b/net_util/src/ctrl_queue.rs index b20479c440..45ac619325 100644 --- a/net_util/src/ctrl_queue.rs +++ b/net_util/src/ctrl_queue.rs @@ -365,4 +365,52 @@ mod unit_tests { fn delta_clamps_desired_to_device_max() { assert_eq!(plan_queue_pair_delta(2, 99, 4), vec![(2, true), (3, true)],); } + + use std::sync::Mutex; + + #[derive(Default)] + struct MockBackendInner { + last_pairs: Option, + fail: bool, + } + + #[derive(Default, Clone)] + struct MockBackend(Arc>); + + impl MqBackend for MockBackend { + fn set_active_pairs(&mut self, pairs: u16) -> std::result::Result<(), MqBackendError> { + let mut inner = self.0.lock().unwrap(); + inner.last_pairs = Some(pairs); + if inner.fail { + Err(MqBackendError::Other("mock fail".into())) + } else { + Ok(()) + } + } + } + + #[test] + fn ctrl_queue_routes_apply_to_backend() { + let mock = MockBackend::default(); + let inner = mock.0.clone(); + let mut cq = CtrlQueue::new(Vec::new(), Some(Box::new(mock)), 4, true); + cq.apply_active_queue_pairs(3).unwrap(); + assert_eq!(inner.lock().unwrap().last_pairs, Some(3)); + } + + #[test] + fn ctrl_queue_propagates_backend_failure() { + let mock = MockBackend::default(); + mock.0.lock().unwrap().fail = true; + let mut cq = CtrlQueue::new(Vec::new(), Some(Box::new(mock)), 4, true); + let err = cq.apply_active_queue_pairs(3).unwrap_err(); + assert!(matches!(err, MqBackendError::Other(_))); + } + + #[test] + fn ctrl_queue_with_no_backend_rejects_apply() { + let mut cq = CtrlQueue::new(Vec::new(), None, 4, true); + let err = cq.apply_active_queue_pairs(3).unwrap_err(); + assert!(matches!(err, MqBackendError::Other(_))); + } } From 35f45764eab98327fb4db998f5e4923bdb3110a0 Mon Sep 17 00:00:00 2001 From: Mark Vainomaa Date: Sun, 31 May 2026 21:05:48 +0300 Subject: [PATCH 09/10] virtio-devices: net: preserve active queue-pair count across snapshot The activate-time alignment landed earlier always drove the active pair count to one, but that's only spec-correct after a virtio reset. Snapshot restore is not a reset: the guest resumes with the same driver state it had pre-snapshot and expects its previously negotiated pair count to still apply. Forcing the count back to one would silently break multi-queue guests after migration/restore. Save the live count in NetState (gated behind serde(default) so old snapshots still deserialize) and stash it as restore_active_pairs on the Net struct when reconstructing from state. The first activate after restore drives the kernel to that count instead of the cold- boot default of one; .take() ensures subsequent activate cycles in the restored VM (driven by guest resets) fall back to the default, since by then the guest goes through the normal F_MQ negotiation again. Vhost-user snapshot/restore would need a similar fix at the backend boundary, but that requires backend-side cooperation that is out of scope here. Signed-off-by: Mark Vainomaa --- virtio-devices/src/net.rs | 148 +++++++++++++++++++++----------------- 1 file changed, 84 insertions(+), 64 deletions(-) diff --git a/virtio-devices/src/net.rs b/virtio-devices/src/net.rs index a994482eef..6bbeda9480 100644 --- a/virtio-devices/src/net.rs +++ b/virtio-devices/src/net.rs @@ -410,12 +410,18 @@ pub struct Net { /// Number of queue pairs currently attached to the underlying /// multi-queue tun. `open_tap`/`from_tap_fd` start with every tap /// kernel-attached via `IFF_MULTI_QUEUE`, so the initial value matches - /// `taps.len()`. `activate()` then drives it down to one, and the - /// spawned `CtrlQueue` worker grows it back in response to + /// `taps.len()`. `activate()` then drives it down to the target pair + /// count (`1` for cold boot, the snapshotted count for restore), and + /// the spawned `CtrlQueue` worker grows it back in response to /// `VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET`. Sharing the tracker keeps both /// ends from re-issuing `TUNSETQUEUE` on queues already in the /// requested state, which the kernel would reject with `EINVAL`. kernel_active_pairs: Arc, + /// Pair count to align to on the *next* activation, populated from + /// `NetState::active_pairs` when restoring from a snapshot and + /// consumed by the first `activate()` so subsequent reset cycles + /// fall back to the spec default of one. + restore_active_pairs: Option, } #[derive(Serialize, Deserialize)] @@ -424,6 +430,11 @@ pub struct NetState { pub acked_features: u64, pub config: VirtioNetConfig, pub queue_size: Vec, + /// Active RX/TX queue-pair count at snapshot time. `None` on old + /// snapshots that pre-date this field; restore then falls back to + /// the spec default of one active pair. + #[serde(default)] + pub active_pairs: Option, } impl Net { @@ -455,66 +466,73 @@ impl Net { } }; - let (avail_features, acked_features, config, queue_sizes, paused) = if let Some(state) = - state - { - info!("Restoring virtio-net {id}"); - ( - state.avail_features, - state.acked_features, - state.config, - state.queue_size, - true, - ) - } else { - let mut avail_features = (1 << VIRTIO_RING_F_EVENT_IDX) | (1 << VIRTIO_F_VERSION_1); - - if mtu.is_some() { - avail_features |= 1 << VIRTIO_NET_F_MTU; - } + let (avail_features, acked_features, config, queue_sizes, paused, restore_active_pairs) = + if let Some(state) = state { + info!("Restoring virtio-net {id}"); + ( + state.avail_features, + state.acked_features, + state.config, + state.queue_size, + true, + state.active_pairs, + ) + } else { + let mut avail_features = (1 << VIRTIO_RING_F_EVENT_IDX) | (1 << VIRTIO_F_VERSION_1); - if access_platform_enabled { - avail_features |= 1u64 << VIRTIO_F_ACCESS_PLATFORM; - } + if mtu.is_some() { + avail_features |= 1 << VIRTIO_NET_F_MTU; + } - // Configure TSO/UFO features when hardware checksum offload is enabled. - if offload_csum { - avail_features |= (1 << VIRTIO_NET_F_CSUM) - | (1 << VIRTIO_NET_F_GUEST_CSUM) - | (1 << VIRTIO_NET_F_CTRL_GUEST_OFFLOADS); - - if offload_tso { - avail_features |= (1 << VIRTIO_NET_F_HOST_ECN) - | (1 << VIRTIO_NET_F_HOST_TSO4) - | (1 << VIRTIO_NET_F_HOST_TSO6) - | (1 << VIRTIO_NET_F_GUEST_ECN) - | (1 << VIRTIO_NET_F_GUEST_TSO4) - | (1 << VIRTIO_NET_F_GUEST_TSO6); + if access_platform_enabled { + avail_features |= 1u64 << VIRTIO_F_ACCESS_PLATFORM; } - if offload_ufo { - avail_features |= (1 << VIRTIO_NET_F_HOST_UFO) | (1 << VIRTIO_NET_F_GUEST_UFO); + // Configure TSO/UFO features when hardware checksum offload is enabled. + if offload_csum { + avail_features |= (1 << VIRTIO_NET_F_CSUM) + | (1 << VIRTIO_NET_F_GUEST_CSUM) + | (1 << VIRTIO_NET_F_CTRL_GUEST_OFFLOADS); + + if offload_tso { + avail_features |= (1 << VIRTIO_NET_F_HOST_ECN) + | (1 << VIRTIO_NET_F_HOST_TSO4) + | (1 << VIRTIO_NET_F_HOST_TSO6) + | (1 << VIRTIO_NET_F_GUEST_ECN) + | (1 << VIRTIO_NET_F_GUEST_TSO4) + | (1 << VIRTIO_NET_F_GUEST_TSO6); + } + + if offload_ufo { + avail_features |= + (1 << VIRTIO_NET_F_HOST_UFO) | (1 << VIRTIO_NET_F_GUEST_UFO); + } } - } - avail_features |= 1 << VIRTIO_NET_F_CTRL_VQ; - let queue_num = num_queues + 1; + avail_features |= 1 << VIRTIO_NET_F_CTRL_VQ; + let queue_num = num_queues + 1; - let mut config = VirtioNetConfig::default(); - if let Some(mac) = guest_mac { - build_net_config_space(&mut config, mac, num_queues, mtu, &mut avail_features); - } else { - build_net_config_space_with_mq(&mut config, num_queues, mtu, &mut avail_features); - } + let mut config = VirtioNetConfig::default(); + if let Some(mac) = guest_mac { + build_net_config_space(&mut config, mac, num_queues, mtu, &mut avail_features); + } else { + build_net_config_space_with_mq( + &mut config, + num_queues, + mtu, + &mut avail_features, + ); + } - ( - avail_features, - 0, - config, - vec![queue_size; queue_num], - false, - ) - }; + ( + avail_features, + 0, + config, + vec![queue_size; queue_num], + false, + None, + ) + }; let kernel_active_pairs = Arc::new(AtomicU16::new(taps.len() as u16)); Ok(Net { @@ -538,6 +556,7 @@ impl Net { exit_evt, device_status: Arc::new(AtomicU8::new(0)), kernel_active_pairs, + restore_active_pairs, }) } @@ -650,6 +669,7 @@ impl Net { acked_features: self.common.acked_features, config: self.config, queue_size: self.common.queue_sizes.clone(), + active_pairs: Some(self.kernel_active_pairs.load(Ordering::Acquire)), } } @@ -716,18 +736,18 @@ impl VirtioDevice for Net { let qp_threads = (num_queues - ctrl_threads) / 2; self.common.paused_sync = Some(Arc::new(Barrier::new(1 + qp_threads + ctrl_threads))); - // Start each activation with one attached tap queue pair (per - // virtio 1.0+ §5.1.6.5.5: multiqueue is disabled by default). - // Multi-queue guests negotiate VIRTIO_NET_F_MQ and grow the count - // via VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET; this also covers - // single-queue drivers that never negotiate F_MQ, where leaving - // all taps attached would otherwise let the kernel steer RX to - // queues with no userspace reader (silently dropping hashed - // traffic at the tap). + // Default to one attached tap queue pair per virtio 1.0+ + // §5.1.6.5.5 (multiqueue disabled until the guest enables it). + // On the first activation after a snapshot restore, jump + // straight to the pre-snapshot pair count instead -- the guest + // hasn't been through a reset/MQ-negotiation cycle and still + // expects its previous active set. Subsequent reset cycles in + // the restored VM fall back to the default of one. + let target_pairs = self.restore_active_pairs.take().unwrap_or(1); TapMqBackend::new(self.taps.clone(), self.kernel_active_pairs.clone()) - .set_active_pairs(1) + .set_active_pairs(target_pairs) .map_err(|e| { - error!("Failed to align tap queues to single active pair: {e}"); + error!("Failed to align tap queues to {target_pairs} active pair(s): {e}"); ActivateError::BadActivate })?; From 5c7e1a45a8fe6aca032ab27d756bbb17a39db306 Mon Sep 17 00:00:00 2001 From: Mark Vainomaa Date: Sun, 31 May 2026 21:18:59 +0300 Subject: [PATCH 10/10] virtio-devices: net: keep multi-queue across legacy snapshot restore The preceding snapshot/restore change defaulted unwrap_or(1) when NetState::active_pairs was absent, which conflates two distinct states: "cold boot" (use spec default of 1) and "restoring from a snapshot that pre-dates the field" (the previous CH version left every tap attached, so falling back to 1 silently drops multi-queue pairs across the upgrade). Replace the field with an ActivationTarget enum so the three cases are explicit: - ColdBoot -> 1 active pair (spec) - LegacyRestore -> taps.len() (mimic pre-Rev-G CH behavior) - RestoreWith(n) -> n (new snapshots with recorded count) The target is consumed on the first activate and reset to ColdBoot, so subsequent reset/re-activate cycles inside the restored VM go through the spec-correct single-pair path regardless of where the restore started. Signed-off-by: Mark Vainomaa --- virtio-devices/src/net.rs | 58 +++++++++++++++++++++++++++++---------- 1 file changed, 43 insertions(+), 15 deletions(-) diff --git a/virtio-devices/src/net.rs b/virtio-devices/src/net.rs index 6bbeda9480..dd8cb0a800 100644 --- a/virtio-devices/src/net.rs +++ b/virtio-devices/src/net.rs @@ -417,11 +417,27 @@ pub struct Net { /// ends from re-issuing `TUNSETQUEUE` on queues already in the /// requested state, which the kernel would reject with `EINVAL`. kernel_active_pairs: Arc, - /// Pair count to align to on the *next* activation, populated from - /// `NetState::active_pairs` when restoring from a snapshot and - /// consumed by the first `activate()` so subsequent reset cycles - /// fall back to the spec default of one. - restore_active_pairs: Option, + /// What the next `activate()` should align the kernel-side tap + /// queue count to. Defaults to `ColdBoot` for fresh devices and + /// gets reset to `ColdBoot` after each activation, so only the + /// first activate post-restore deviates from the spec default. + next_activation_target: ActivationTarget, +} + +/// Target queue-pair count for the next `Net::activate()` call. +#[derive(Debug, Clone, Copy)] +enum ActivationTarget { + /// Spec default: one active pair (cold boot, plus any non-first + /// activate after restore). + ColdBoot, + /// First activate after restoring from a snapshot that pre-dates + /// the `NetState::active_pairs` field. The pre-snapshot CH version + /// left every tap attached, so mimic that to avoid silently losing + /// multi-queue across the upgrade. + LegacyRestore, + /// First activate after restoring from a snapshot that recorded + /// the active pair count. + RestoreWith(u16), } #[derive(Serialize, Deserialize)] @@ -466,16 +482,20 @@ impl Net { } }; - let (avail_features, acked_features, config, queue_sizes, paused, restore_active_pairs) = + let (avail_features, acked_features, config, queue_sizes, paused, next_activation_target) = if let Some(state) = state { info!("Restoring virtio-net {id}"); + let target = match state.active_pairs { + Some(n) => ActivationTarget::RestoreWith(n), + None => ActivationTarget::LegacyRestore, + }; ( state.avail_features, state.acked_features, state.config, state.queue_size, true, - state.active_pairs, + target, ) } else { let mut avail_features = (1 << VIRTIO_RING_F_EVENT_IDX) | (1 << VIRTIO_F_VERSION_1); @@ -530,7 +550,7 @@ impl Net { config, vec![queue_size; queue_num], false, - None, + ActivationTarget::ColdBoot, ) }; @@ -556,7 +576,7 @@ impl Net { exit_evt, device_status: Arc::new(AtomicU8::new(0)), kernel_active_pairs, - restore_active_pairs, + next_activation_target, }) } @@ -738,12 +758,20 @@ impl VirtioDevice for Net { // Default to one attached tap queue pair per virtio 1.0+ // §5.1.6.5.5 (multiqueue disabled until the guest enables it). - // On the first activation after a snapshot restore, jump - // straight to the pre-snapshot pair count instead -- the guest - // hasn't been through a reset/MQ-negotiation cycle and still - // expects its previous active set. Subsequent reset cycles in - // the restored VM fall back to the default of one. - let target_pairs = self.restore_active_pairs.take().unwrap_or(1); + // Restores deviate: a snapshot that recorded its active pair + // count is replayed at that count, and a pre-Rev-G snapshot + // (no recorded count) falls back to leaving every tap attached + // -- matching the broken-but-multi-queue-preserving behavior + // of the CH version that wrote the snapshot, so the upgrade + // doesn't silently lose pairs. The target is reset to + // `ColdBoot` so reset/re-activate cycles inside the restored + // VM go through the spec-correct one-pair path. + let target_pairs = match self.next_activation_target { + ActivationTarget::ColdBoot => 1, + ActivationTarget::LegacyRestore => self.taps.len() as u16, + ActivationTarget::RestoreWith(n) => n, + }; + self.next_activation_target = ActivationTarget::ColdBoot; TapMqBackend::new(self.taps.clone(), self.kernel_active_pairs.clone()) .set_active_pairs(target_pairs) .map_err(|e| {