Give a system VM one NIC queue per CPU - #14136
bhouse-nexthop wants to merge 2 commits into
Conversation
A system VM gets a single NIC queue however many CPUs it has, so every packet interrupt lands on CPU0. Adding CPUs to a router does not move more packets, which is the only reason to add them. Queue count is already a VM detail, nic.multiqueue.number, and the agent already turns it into <driver queues='N'/>. A user VM sets it three ways: deployVirtualMachine nicmultiqueuenumber=N updateVirtualMachine details[0].nic.multiqueue.number=N the Settings tab, listDetailOptions offers the key A system VM goes through none of them, so there is no way to set it at all. This gives one queue per CPU when nothing is set. 1 CPU no queues attribute, unchanged 4 CPU queues='4' 512 CPU queues='256', a tap device goes no higher The default offering is a single CPU, so a default install is unchanged. Only a resized system VM differs, and it differs the way the resize asked for. A queue number set on the VM still wins. Signed-off-by: Brad House <bhouse@nexthop.ai>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 4.22 #14136 +/- ##
=========================================
Coverage 17.93% 17.93%
- Complexity 16143 16150 +7
=========================================
Files 5928 5928
Lines 535174 535188 +14
Branches 65494 65497 +3
=========================================
+ Hits 95981 96007 +26
+ Misses 428266 428254 -12
Partials 10927 10927
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
MAX_TAP_QUEUES was declared between the @Inject and networkOfferingDetailsDao, so the annotation bound to the constant and the DAO was never injected. getNicDetails() dereferences it for every NIC, so every VM deploy failed with an NPE behind a 530 "Internal Server Error", which is what the simulator CI run was reporting. Move the constant above the injected fields and give the DAO its annotation back. Signed-off-by: Brad House <bhouse@nexthop.ai>
216b4cb to
0d63a76
Compare
|
Instead of deriving the count unconditionally, could we make it configurable per system/router service offering? A detail on the offering (read into nic.multiqueue.number, still letting an explicit per-VM value win) would keep your CPU-derived default for anyone who sets -1, let an operator pin an explicit N, and leave it off when unset — which also covers the "no way to turn this off" note. |
The offering size is pretty small at 1 cpu, which likely most people are sticking with. I know cloudstack likes knobs for everything, and I found out the hard way on a lot of things since there is no documentation on 'recommended knobs to turn on'. I honestly don't see harm in this ... but if its a requirement I can add yet another knob to bit someone else in the future. |
|
Hello, @bhouse-nexthop and @poddm @bhouse-nexthop, thanks for the PR! I'm finishing the port of a new feature that adds support for granular multi-queue management for the different NIC types of VRs in guest networks and VPCs. Basically, it'll be possible to configure, for VPC offerings:
And, for network offerings:
Similarly to multi-queue support for end-user VMs, if these values are set to Therefore, with this approach, we can granularly define the desired number of queues based on the type of network traffic each NIC carries. |
ok, that sounds reasonable. Any idea on an ETA for your implementation? In the mean time we're actually running this in production ourselves. |
@bhouse-nexthop, yes, probably no later than the middle or end of next week. I'll ping you guys for review once I submit the PR! |
This branch has not been deployed
3 participants
Description
A system VM gets one NIC queue no matter how many CPUs it has, so every packet interrupt lands on
CPU0. Adding CPUs to a router therefore does not move more packets, which is the only reason to add
them.
The queue count is already a VM detail,
nic.multiqueue.number, and the agent already turns it into<driver queues='N'/>on the interface. A user VM can set it three ways:deployVirtualMachine nicmultiqueuenumber=NupdateVirtualMachine details[0].nic.multiqueue.number=NlistDetailOptionsoffers the key for KVMA system VM goes through none of them. It is not created by
deployVirtualMachine, it is not aUserVmsoupdateVirtualMachinedoes not apply, and it has no details editor. There is no wayto set it at all, which is why this is a fix rather than a new setting.
So a system VM now gets one queue per CPU when nothing is set:
queues='4'queues='256'The 256 is the tap device ceiling in the host kernel, which refuses the interface rather than
trimming to fit. Above the CPU count it would make no difference anyway - the guest driver uses
min(CPUs, queues)and pushes that at probe, so no guest side change is needed.A queue number already set on the VM still wins.
The default offering is a single CPU, so a default install emits the same domain XML as before.
Only a system VM someone has already resized differs, and it differs in the direction the resize
asked for.
Worth calling out
symmetric, but it is worth being explicit about on a release branch.
4 CPU router fronting 8 tiers goes from 8 vhost threads to 32. Idle queues are cheap, but the
thread count is not nothing on a dense host.
Types of changes
Feature/Enhancement Scale or Bug Severity
Bug Severity
How Has This Been Tested?
KVMGuruTest,Tests run: 35, Failures: 0. Six new cases:queues='4'The guest side was checked against the driver source the system VM runs, Linux 6.1
drivers/net/virtio_net.c:followed by
virtnet_set_queues(vi, vi->curr_queue_pairs)at probe, so the queues come up withoutethtool -L. The 256 comes fromMAX_TAP_QUEUESindrivers/net/tun.c.