Skip to content

Conversation

@damanm24
Copy link
Contributor

This PR adds multiqueue support for the mana emulator and propagates the feature enablement to other components as well.

@damanm24 damanm24 requested a review from a team as a code owner September 25, 2025 18:06
@benhillis
Copy link
Member

Looks great, could you add a bit of the "why" in the PR description? What will this enable? Thanks!

@github-actions
Copy link

@github-actions
Copy link

@Copilot Copilot AI review requested due to automatic review settings October 15, 2025 17:50
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds multiqueue support for the MANA network emulator, enabling multiple transmit/receive queue pairs to improve network performance. The changes extend the MANA driver configuration to support multiple queues per virtual port and update related components to handle multiqueue operations.

Key changes include:

  • Addition of queue_pairs field to vport configurations across multiple components
  • Enhanced queue management in the BasicNic implementation to support multiple queue pairs per vport
  • Updates to Consomme endpoint to support multiqueue functionality
  • New integration test to validate multiqueue behavior on both Windows and Linux

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
vmm_tests/vmm_tests/tests/tests/x86_64/openhcl_linux_direct.rs Adds integration test for MANA multiqueue functionality
vm/devices/net/net_mana/src/lib.rs Updates test configurations to include queue_pairs field
vm/devices/net/net_consomme/src/lib.rs Implements multiqueue support in Consomme endpoint
vm/devices/net/mana_driver/src/tests.rs Updates test to include queue_pairs field
vm/devices/net/gdma_resources/src/lib.rs Adds queue_pairs field to VportDefinition struct
vm/devices/net/gdma/src/resolver.rs Updates resolver to pass through queue_pairs configuration
vm/devices/net/gdma/src/lib.rs Adds queue_pairs field to VportConfig struct
vm/devices/net/gdma/src/bnic.rs Major refactoring to support multiple queue pairs per vport
petri/src/vm/openvmm/modify.rs Sets default queue_pairs value in test configuration
openvmm/openvmm_entry/src/lib.rs Configures queue_pairs from command line max_queues parameter

Comment on lines 154 to 161
let _ = output
.lines()
.filter(|line| line.starts_with("Combined:"))
.map(|line| {
let count = line.split_whitespace().nth(1).unwrap().trim().to_string();
let n: u32 = count.parse().unwrap();
assert!(n == vp_count, "expected {} queues, got {}", vp_count, n);
});
Copy link

Copilot AI Oct 15, 2025

Choose a reason for hiding this comment

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

The iterator is created but never consumed. The map operation is lazy and won't execute unless the iterator is consumed. Use for_each instead of map or call collect() to consume the iterator.

Copilot uses AI. Check for mistakes.

Comment on lines +417 to +418
state.queues.free_wq(true, wq_id).unwrap_or(());
state.queues.free_cq(cq_id).unwrap_or(());
Copy link

Copilot AI Oct 15, 2025

Choose a reason for hiding this comment

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

Using unwrap_or(()) silently ignores errors during resource cleanup. Consider logging errors or using a more explicit error handling approach to aid debugging if resource deallocation fails.

Copilot uses AI. Check for mistakes.

@github-actions
Copy link

@github-actions
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants