https://bugzilla.redhat.com/show_bug.cgi?id=1366989 Qemu grew yet another virtio-net tunable [1]. It basically allows users to set the size of RX virtio ring. But because virtio-net uses two separate ring buffers to pass data from/to guest they named it explicitly rx_queue_size. We should expose it in our XML too. 1: http://lists.nongnu.org/archive/html/qemu-devel/2016-08/msg02029.html Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- docs/formatdomain.html.in | 7 +++++- docs/schemas/domaincommon.rng | 5 ++++ src/conf/domain_conf.c | 16 ++++++++++++ src/conf/domain_conf.h | 1 + src/qemu/qemu_domain.c | 7 ++++++ .../qemuxml2argv-net-virtio-rxqueuesize.xml | 29 ++++++++++++++++++++++ 6 files changed, 64 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-virtio-rxqueuesize.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index bfbb0f2..6642dc8 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4604,7 +4604,7 @@ qemu-kvm -net nic,model=? /dev/null &lt;source network='default'/&gt; &lt;target dev='vnet1'/&gt; &lt;model type='virtio'/&gt; - <b>&lt;driver name='vhost' txmode='iothread' ioeventfd='on' event_idx='off' queues='5'&gt; + <b>&lt;driver name='vhost' txmode='iothread' ioeventfd='on' event_idx='off' queues='5' rxqueuesize='128'&gt; &lt;host csum='off' gso='off' tso4='off' tso6='off' ecn='off' ufo='off' mrg_rxbuf='off'/&gt; &lt;guest csum='off' tso4='off' tso6='off' ecn='off' ufo='off'/&gt; &lt;/driver&gt; @@ -4720,6 +4720,11 @@ qemu-kvm -net nic,model=? /dev/null <span class="since">virtio-net since 1.0.6 (QEMU and KVM only)</span> <span class="since">vhost-user since 1.2.17 (QEMU and KVM only)</span> </dd> + <dt><code>rxqueuesize</code></dt> + <dd> + The optional <code>rxqueuesize</code> attribute controls + the size of virtio ring for each queue as described above. + </dd> </dl> <p> Offloading options for the host and guest can be configured using diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 052f28c..4b89a86 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2529,6 +2529,11 @@ </attribute> </optional> <optional> + <attribute name='rxqueuesize'> + <ref name='positiveInteger'/> + </attribute> + </optional> + <optional> <attribute name="txmode"> <choice> <value>iothread</value> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 14d4f7d..08cde19 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8911,6 +8911,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, char *ioeventfd = NULL; char *event_idx = NULL; char *queues = NULL; + char *rxqueuesize = NULL; char *str = NULL; char *filter = NULL; char *internal = NULL; @@ -9083,6 +9084,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, ioeventfd = virXMLPropString(cur, "ioeventfd"); event_idx = virXMLPropString(cur, "event_idx"); queues = virXMLPropString(cur, "queues"); + rxqueuesize = virXMLPropString(cur, "rxqueuesize"); } else if (xmlStrEqual(cur->name, BAD_CAST "filterref")) { if (filter) { virReportError(VIR_ERR_XML_ERROR, "%s", @@ -9466,6 +9468,17 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, if (q > 1) def->driver.virtio.queues = q; } + if (rxqueuesize) { + unsigned int q; + if (virStrToLong_uip(rxqueuesize, NULL, 10, &q) < 0) { + virReportError(VIR_ERR_XML_DETAIL, + _("'rxqueuesize' attribute must be positive number: %s"), + rxqueuesize); + goto error; + } + if (q > 1) + def->driver.virtio.rxqueuesize = q; + } if ((str = virXPathString("string(./driver/host/@csum)", ctxt))) { if ((val = virTristateSwitchTypeFromString(str)) <= 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -9646,6 +9659,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, VIR_FREE(ioeventfd); VIR_FREE(event_idx); VIR_FREE(queues); + VIR_FREE(rxqueuesize); VIR_FREE(str); VIR_FREE(filter); VIR_FREE(type); @@ -20768,6 +20782,8 @@ virDomainVirtioNetDriverFormat(char **outstr, } if (def->driver.virtio.queues) virBufferAsprintf(&buf, "queues='%u' ", def->driver.virtio.queues); + if (def->driver.virtio.rxqueuesize) + virBufferAsprintf(&buf, "rxqueuesize='%u' ", def->driver.virtio.rxqueuesize); virBufferTrim(&buf, " ", -1); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 8b26724..9197d70 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -906,6 +906,7 @@ struct _virDomainNetDef { virTristateSwitch ioeventfd; virTristateSwitch event_idx; unsigned int queues; /* Multiqueue virtio-net */ + unsigned int rxqueuesize; /* rx_queue_size */ struct { virTristateSwitch csum; virTristateSwitch gso; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 6c0f261..ee24086 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2372,6 +2372,13 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev, "not supported by QEMU")); goto cleanup; } + + if (STREQ_NULLABLE(net->model, "virtio") && + net->driver.virtio.rxqueuesize & (net->driver.virtio.rxqueuesize -1)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("rxqueuesize has to be a power of two")); + goto cleanup; + } } ret = 0; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-rxqueuesize.xml b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-rxqueuesize.xml new file mode 100644 index 0000000..e23d3e3 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-rxqueuesize.xml @@ -0,0 +1,29 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + </disk> + <controller type='usb' index='0'/> + <interface type='user'> + <mac address='00:11:22:33:44:55'/> + <model type='virtio'/> + <driver rxqueuesize='128'/> + </interface> + <memballoon model='virtio'/> + </devices> +</domain> -- 2.8.4

On 31.08.2016 14:48, John Ferlan wrote: No, I had to apply the patch locally and work over patched qemu. QEMU devels are currently in freeze for 2.7 release (not sure what their release date is planned on though). That's what I wanted to prevent. Copying name from qemu. But I've struggled with the naming too (as can be seen - look how far I got :D). I don't have a strong opinion on either of those, so whatever we feel like I'll stick with that. Well, as you point out later in the review too, qemu might change these limitation anytime and we would advice untrue. But if I'm careful with wording we might be safe. Nope. Moreover, this feature is going to be used by expert users who know exactly what they are doing. So I'd say we're okay with just trying to start qemu and see if it fails or not. IOW too much work on our side not worth it. Good catch. Sure, I can introduce such test. I don't think we need to. We should probably check whether it is a power of two (this looks like invariant requirement) = who would want a ring size of a non-power of two size? But the maximum/minimum size can change. Currently the limits are [256;1024]. Moreover, qemu fails with sensible message: error: internal error: process exited while connecting to monitor: 2016-08-31T14:23:16.583247Z qemu-system-x86_64: -device virtio-net-pci,mq=on,vectors=42,rx_queue_size=2048,netdev=hostnet1,id=net1,mac=52:54:00:6e:0e:72,bus=pci.2,addr=0x2: Invalid rx_queue_size (= 2048), must be a power of 2 between 256 and 1024. I'll post v2 which will is rebased onto current HEAD and address your findings. Thank you! Michal

Just like in the previous commit, teach qemu driver to detect whether qemu supports this configuration knob or not. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + 2 files changed, 3 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 8efb8fb..373002d 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -341,6 +341,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "intel-iommu", "smm", "virtio-pci-disable-legacy", + "virtio-net.rx_queue_size", ); @@ -1580,6 +1581,7 @@ static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = { static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioNet[] = { { "tx", QEMU_CAPS_VIRTIO_TX_ALG }, { "event_idx", QEMU_CAPS_VIRTIO_NET_EVENT_IDX }, + { "rx_queue_size", QEMU_CAPS_VIRTIO_NET_RX_QUEUE_SIZE }, }; static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioSCSI[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 776a0f3..c8b60bf 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -374,6 +374,7 @@ typedef enum { QEMU_CAPS_DEVICE_INTEL_IOMMU, /* -device intel-iommu */ QEMU_CAPS_MACHINE_SMM_OPT, /* -machine xxx,smm=on/off/auto */ QEMU_CAPS_VIRTIO_PCI_DISABLE_LEGACY, /* virtio-*pci.disable-legacy */ + QEMU_CAPS_VIRTIO_NET_RX_QUEUE_SIZE, /* virtio-net-*.rx_queue_size */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; -- 2.8.4

Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/qemu/qemu_command.c | 8 +++++++ .../qemuxml2argv-net-virtio-rxqueuesize.args | 25 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ 3 files changed, 35 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-virtio-rxqueuesize.args diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index a6dea6a..0c9dff4 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3553,6 +3553,14 @@ qemuBuildNicDevStr(virDomainDefPtr def, virBufferAsprintf(&buf, ",mq=on,vectors=%zu", 2 * vhostfdSize + 2); } } + if (usingVirtio && net->driver.virtio.rxqueuesize) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_NET_RX_QUEUE_SIZE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("virtio rx_queue_size option is not supported with this QEMU binary")); + goto error; + } + virBufferAsprintf(&buf, ",rx_queue_size=%u", net->driver.virtio.rxqueuesize); + } if (vlan == -1) virBufferAsprintf(&buf, ",netdev=host%s", net->info.alias); else diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-rxqueuesize.args b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-rxqueuesize.args new file mode 100644 index 0000000..6392889 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-rxqueuesize.args @@ -0,0 +1,25 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu \ +-name QEMUGuest1 \ +-S \ +-M pc \ +-m 214 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nographic \ +-nodefaults \ +-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \ +-no-acpi \ +-boot c \ +-usb \ +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ +-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ +-device virtio-net-pci,rx_queue_size=128,vlan=0,id=net0,mac=00:11:22:33:44:55,\ +bus=pci.0,addr=0x3 \ +-net user,vlan=0,name=hostnet0 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4 diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index e8b8cb4..fc94e30 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1043,6 +1043,8 @@ mymain(void) QEMU_CAPS_VIRTIO_S390); DO_TEST("net-virtio-ccw", QEMU_CAPS_VIRTIO_CCW, QEMU_CAPS_VIRTIO_S390); + DO_TEST("net-virtio-rxqueuesize", + QEMU_CAPS_VIRTIO_NET_RX_QUEUE_SIZE); DO_TEST("net-eth", NONE); DO_TEST("net-eth-ifname", NONE); DO_TEST("net-eth-names", NONE); -- 2.8.4