[v1] exposing busy polling support for vhost-net

[libvirt] [PATCH 3/3] qemu: add busy polling support

Posted by sferdjao@redhat.com 7 years, 11 months ago

From: Sahid Orentino Ferdjaoui <sahid.ferdjaoui@redhat.com>

This patch finalizes support of 'poll_us' attribute as an option of
the NIC driver-specific element, the commit also takes care of
hotplug.

<devices>
  <interface type='ethernet'>
    <mac address='52:54:00:23:bc:ba'/>
    <model type='virtio'/>
    <driver name="vhost" poll_us='50'/>
  </interface>
</devices>

That option is supported for all networks which are using a tap
device.

To be used the backend should be specificly set to use vhost.

Signed-off-by: Sahid Orentino Ferdjaoui <sahid.ferdjaoui@redhat.com>
---
 docs/formatdomain.html.in                          | 12 ++++++++++-
 docs/schemas/domaincommon.rng                      |  5 +++++
 src/qemu/qemu_command.c                            | 22 +++++++++++++++++++++
 src/qemu/qemu_hotplug.c                            | 12 +++++++++++
 .../qemuxml2argv-net-virtio-netdev-pollus-fail.xml | 23 ++++++++++++++++++++++
 ...xml2argv-net-virtio-netdev-pollus-qemu-fail.xml | 23 ++++++++++++++++++++++
 .../qemuxml2argv-net-virtio-netdev-pollus.args     | 23 ++++++++++++++++++++++
 .../qemuxml2argv-net-virtio-netdev-pollus.xml      | 23 ++++++++++++++++++++++
 tests/qemuxml2argvtest.c                           |  9 +++++++++
 9 files changed, 151 insertions(+), 1 deletion(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus-fail.xml
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus-qemu-fail.xml
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 52119f0..7bfeabc 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -5043,7 +5043,7 @@ qemu-kvm -net nic,model=? /dev/null
     &lt;model type='virtio'/&gt;
     <b>&lt;driver name='vhost' txmode='iothread' ioeventfd='on' event_idx='off' queues='5' rx_queue_size='256'&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;guest csum='off' tso4='off' tso6='off' ecn='off' ufo='off' poll_us='50'/&gt;
     &lt;/driver&gt;
     </b>
   &lt;/interface&gt;
@@ -5171,6 +5171,16 @@ qemu-kvm -net nic,model=? /dev/null
         <b>In general you should leave this option alone, unless you
         are very certain you know what you are doing.</b>
       </dd>
+      <dd>
+        The optional <code>poll_us</code> attribute configure the
+        maximum number of microseconds that could be spent on busy
+        polling. It improves the performance, but requires more
+        CPU. This option is only available with vhost backend driver.
+        <span class="since">Since 2.7.0 (QEMU and KVM only)</span><br/><br/>
+
+        <b>In general you should leave this option alone, unless you
+        are very certain you know what you are doing.</b>
+      </dd>
       <dt>virtio options</dt>
       <dd>
         For virtio interfaces,
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 4950ddc..f304385 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2687,6 +2687,11 @@
               <optional>
                 <ref name="event_idx"/>
               </optional>
+              <optional>
+                <attribute name='poll_us'>
+                  <ref name='positiveInteger'/>
+                </attribute>
+              </optional>
             </group>
           </choice>
           <ref name="virtioOptions"/>
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 8c12b2b..412496e 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3955,6 +3955,9 @@ qemuBuildHostNetStr(virDomainNetDefPtr net,
         }
         if (net->tune.sndbuf_specified)
             virBufferAsprintf(&buf, "sndbuf=%lu,", net->tune.sndbuf);
+        if (net->driver.virtio.pollus)
+            virBufferAsprintf(&buf, "poll-us=%u,",
+                              net->driver.virtio.pollus);
     }
 
     virObjectUnref(cfg);
@@ -8451,6 +8454,25 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
         return -1;
     }
 
+    if (net->driver.virtio.pollus > 0) {
+        if (net->driver.virtio.name != VIR_DOMAIN_NET_BACKEND_TYPE_VHOST) {
+            virReportError(
+                VIR_ERR_CONFIG_UNSUPPORTED,
+                _("Busy polling is only supported with vhost backend driver"));
+            return -1;
+        }
+        /* Nothing besides TAP devices supports busy polling. */
+        if (!(actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
+              actualType == VIR_DOMAIN_NET_TYPE_BRIDGE ||
+              actualType == VIR_DOMAIN_NET_TYPE_DIRECT ||
+              actualType == VIR_DOMAIN_NET_TYPE_ETHERNET)) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("Busy polling is not supported for: %s"),
+                           virDomainNetTypeToString(actualType));
+            return -1;
+        }
+    }
+
     /* and only TAP devices support nwfilter rules */
     if (net->filter &&
         !(actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 0b8d3d8..33884b7 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -995,6 +995,18 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
         return -1;
     }
 
+    /* and nothing besides TAP devices supports busy polling. */
+    if (net->driver.virtio.pollus > 0 &&
+        !(actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
+          actualType == VIR_DOMAIN_NET_TYPE_BRIDGE ||
+          actualType == VIR_DOMAIN_NET_TYPE_DIRECT ||
+          actualType == VIR_DOMAIN_NET_TYPE_ETHERNET)) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                       _("Busy polling is not supported for: %s"),
+                       virDomainNetTypeToString(actualType));
+        return -1;
+    }
+
     /* and only TAP devices support nwfilter rules */
     if (net->filter &&
         !(actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus-fail.xml b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus-fail.xml
new file mode 100644
index 0000000..3b664b9
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus-fail.xml
@@ -0,0 +1,23 @@
+<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-system-i686</emulator>
+    <interface type='ethernet'>
+      <mac address='52:54:00:23:bc:ba'/>
+      <model type='virtio'/>
+      <driver poll_us='50'/>
+    </interface>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus-qemu-fail.xml b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus-qemu-fail.xml
new file mode 100644
index 0000000..18999c3
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus-qemu-fail.xml
@@ -0,0 +1,23 @@
+<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-system-i686</emulator>
+    <interface type='ethernet'>
+      <mac address='52:54:00:23:bc:ba'/>
+      <model type='virtio'/>
+      <driver name="qemu" poll_us='50'/>
+    </interface>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus.args b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus.args
new file mode 100644
index 0000000..8d44134
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus.args
@@ -0,0 +1,23 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-i686 \
+-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 \
+-netdev tap,fd=3,id=hostnet0,poll-us=50 \
+-device virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:23:bc:ba,bus=pci.0,\
+addr=0x3 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus.xml b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus.xml
new file mode 100644
index 0000000..46ef7d6
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus.xml
@@ -0,0 +1,23 @@
+<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-system-i686</emulator>
+    <interface type='ethernet'>
+      <mac address='52:54:00:23:bc:ba'/>
+      <model type='virtio'/>
+      <driver name="vhost" poll_us='50'/>
+    </interface>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 799aea9..54d1b07 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1133,6 +1133,15 @@ mymain(void)
             QEMU_CAPS_NODEFCONFIG);
     DO_TEST("net-virtio-netdev",
             QEMU_CAPS_NETDEV, QEMU_CAPS_NODEFCONFIG);
+    DO_TEST("net-virtio-netdev-pollus",
+            QEMU_CAPS_NETDEV,
+            QEMU_CAPS_VHOST_NET_POLL_US);
+    DO_TEST_FAILURE("net-virtio-netdev-pollus-fail",
+                    QEMU_CAPS_NETDEV,
+                    QEMU_CAPS_VHOST_NET_POLL_US);
+    DO_TEST_FAILURE("net-virtio-netdev-pollus-qemu-fail",
+                    QEMU_CAPS_NETDEV,
+                    QEMU_CAPS_VHOST_NET_POLL_US);
     DO_TEST("net-virtio-s390",
             QEMU_CAPS_VIRTIO_S390);
     DO_TEST("net-virtio-ccw",
-- 
2.9.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 3/3] qemu: add busy polling support

Posted by Laine Stump 7 years, 11 months ago

On 06/15/2017 10:17 AM, sferdjao@redhat.com wrote:
> From: Sahid Orentino Ferdjaoui <sahid.ferdjaoui@redhat.com>
> 
> This patch finalizes support of 'poll_us' attribute as an option of
> the NIC driver-specific element, the commit also takes care of
> hotplug.
> 
> <devices>
>   <interface type='ethernet'>
>     <mac address='52:54:00:23:bc:ba'/>
>     <model type='virtio'/>
>     <driver name="vhost" poll_us='50'/>
>   </interface>
> </devices>
> 
> That option is supported for all networks which are using a tap
> device.
> 
> To be used the backend should be specificly set to use vhost.

Actually, libvirt's qemu driver will use vhost-net automatically as long
as the qemu binary supports it (unless 1) the domain is using TCG
instead of KVM, or 2) the interface configuration specifically says
"<driver name='qemu" .../>).
> 
> Signed-off-by: Sahid Orentino Ferdjaoui <sahid.ferdjaoui@redhat.com>
> ---
>  docs/formatdomain.html.in                          | 12 ++++++++++-
>  docs/schemas/domaincommon.rng                      |  5 +++++
>  src/qemu/qemu_command.c                            | 22 +++++++++++++++++++++
>  src/qemu/qemu_hotplug.c                            | 12 +++++++++++
>  .../qemuxml2argv-net-virtio-netdev-pollus-fail.xml | 23 ++++++++++++++++++++++
>  ...xml2argv-net-virtio-netdev-pollus-qemu-fail.xml | 23 ++++++++++++++++++++++
>  .../qemuxml2argv-net-virtio-netdev-pollus.args     | 23 ++++++++++++++++++++++
>  .../qemuxml2argv-net-virtio-netdev-pollus.xml      | 23 ++++++++++++++++++++++
>  tests/qemuxml2argvtest.c                           |  9 +++++++++

The successful tests should all be done in qemuxml2xmltest.c as well
>  9 files changed, 151 insertions(+), 1 deletion(-)
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus-fail.xml
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus-qemu-fail.xml
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus.args
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus.xml
> 
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 52119f0..7bfeabc 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -5043,7 +5043,7 @@ qemu-kvm -net nic,model=? /dev/null
>      &lt;model type='virtio'/&gt;
>      <b>&lt;driver name='vhost' txmode='iothread' ioeventfd='on' event_idx='off' queues='5' rx_queue_size='256'&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;guest csum='off' tso4='off' tso6='off' ecn='off' ufo='off' poll_us='50'/&gt;
>      &lt;/driver&gt;
>      </b>
>    &lt;/interface&gt;
> @@ -5171,6 +5171,16 @@ qemu-kvm -net nic,model=? /dev/null
>          <b>In general you should leave this option alone, unless you
>          are very certain you know what you are doing.</b>
>        </dd>
> +      <dd>
> +        The optional <code>poll_us</code> attribute configure the
> +        maximum number of microseconds that could be spent on busy
> +        polling. It improves the performance, but requires more
> +        CPU. This option is only available with vhost backend driver.
> +        <span class="since">Since 2.7.0 (QEMU and KVM only)</span><br/><br/>
> +
> +        <b>In general you should leave this option alone, unless you
> +        are very certain you know what you are doing.</b>
> +      </dd>
>        <dt>virtio options</dt>
>        <dd>
>          For virtio interfaces,
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 4950ddc..f304385 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -2687,6 +2687,11 @@
>                <optional>
>                  <ref name="event_idx"/>
>                </optional>
> +              <optional>
> +                <attribute name='poll_us'>
> +                  <ref name='positiveInteger'/>
> +                </attribute>
> +              </optional>
>              </group>
>            </choice>
>            <ref name="virtioOptions"/>

As mentioned in the previous patch, the RNG and docs changes should be
in the previous patch. (In this patch you're going to want to add an
entry to docs/news.xml)

> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 8c12b2b..412496e 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -3955,6 +3955,9 @@ qemuBuildHostNetStr(virDomainNetDefPtr net,
>          }
>          if (net->tune.sndbuf_specified)
>              virBufferAsprintf(&buf, "sndbuf=%lu,", net->tune.sndbuf);
> +        if (net->driver.virtio.pollus)
> +            virBufferAsprintf(&buf, "poll-us=%u,",
> +                              net->driver.virtio.pollus);
>      }
>  
>      virObjectUnref(cfg);
> @@ -8451,6 +8454,25 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
>          return -1;
>      }
>  
> +    if (net->driver.virtio.pollus > 0) {
> +        if (net->driver.virtio.name != VIR_DOMAIN_NET_BACKEND_TYPE_VHOST) {

I haven't checked - does this get set automatically when the vhost-net
FD's are successfully opened?

> +            virReportError(
> +                VIR_ERR_CONFIG_UNSUPPORTED,
> +                _("Busy polling is only supported with vhost backend driver"));

I wonder if anyone would be confused by the lack of the exact option
name "poll_us" in these error messages.

> +            return -1;
> +        }
> +        /* Nothing besides TAP devices supports busy polling. */
> +        if (!(actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
> +              actualType == VIR_DOMAIN_NET_TYPE_BRIDGE ||
> +              actualType == VIR_DOMAIN_NET_TYPE_DIRECT ||
> +              actualType == VIR_DOMAIN_NET_TYPE_ETHERNET)) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                           _("Busy polling is not supported for: %s"),
> +                           virDomainNetTypeToString(actualType));
> +            return -1;
> +        }
> +    }
> +
>      /* and only TAP devices support nwfilter rules */
>      if (net->filter &&
>          !(actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 0b8d3d8..33884b7 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -995,6 +995,18 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
>          return -1;
>      }
>  
> +    /* and nothing besides TAP devices supports busy polling. */
> +    if (net->driver.virtio.pollus > 0 &&
> +        !(actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
> +          actualType == VIR_DOMAIN_NET_TYPE_BRIDGE ||
> +          actualType == VIR_DOMAIN_NET_TYPE_DIRECT ||
> +          actualType == VIR_DOMAIN_NET_TYPE_ETHERNET)) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                       _("Busy polling is not supported for: %s"),
> +                       virDomainNetTypeToString(actualType));
> +        return -1;
> +    }

Huh. I just noticed there's a lot of duplicated code between
qemuBuildInterfaceCommandLine() and qemuDomainAttachNetDevice()... (not
your problem, but it seems like this is just *screaming* to be
vombined/consolidated to eliminate behavioral differences / half-fixed
bugs etc.


> +
>      /* and only TAP devices support nwfilter rules */
>      if (net->filter &&
>          !(actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus-fail.xml b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus-fail.xml
> new file mode 100644
> index 0000000..3b664b9
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus-fail.xml
> @@ -0,0 +1,23 @@
> +<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-system-i686</emulator>
> +    <interface type='ethernet'>
> +      <mac address='52:54:00:23:bc:ba'/>
> +      <model type='virtio'/>
> +      <driver poll_us='50'/>

So this is supposed to fail because the config doesn't explicitly say
"driver name='vhost'". But as I said in the review of patch 2, vhost-net
is used by default. So this would normally be a correct config.


> +    </interface>
> +  </devices>
> +</domain>
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus-qemu-fail.xml b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus-qemu-fail.xml
> new file mode 100644
> index 0000000..18999c3
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus-qemu-fail.xml
> @@ -0,0 +1,23 @@
> +<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-system-i686</emulator>
> +    <interface type='ethernet'>
> +      <mac address='52:54:00:23:bc:ba'/>
> +      <model type='virtio'/>
> +      <driver name="qemu" poll_us='50'/>

This one is okay - it *should* fail.

> +    </interface>
> +  </devices>
> +</domain>
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus.args b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus.args
> new file mode 100644
> index 0000000..8d44134
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus.args
> @@ -0,0 +1,23 @@
> +LC_ALL=C \
> +PATH=/bin \
> +HOME=/home/test \
> +USER=test \
> +LOGNAME=test \
> +QEMU_AUDIO_DRV=none \
> +/usr/bin/qemu-system-i686 \
> +-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 \
> +-netdev tap,fd=3,id=hostnet0,poll-us=50 \
> +-device virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:23:bc:ba,bus=pci.0,\
> +addr=0x3 \
> +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus.xml b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus.xml
> new file mode 100644
> index 0000000..46ef7d6
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus.xml
> @@ -0,0 +1,23 @@
> +<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-system-i686</emulator>
> +    <interface type='ethernet'>
> +      <mac address='52:54:00:23:bc:ba'/>
> +      <model type='virtio'/>
> +      <driver name="vhost" poll_us='50'/>
> +    </interface>
> +  </devices>
> +</domain>
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index 799aea9..54d1b07 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -1133,6 +1133,15 @@ mymain(void)
>              QEMU_CAPS_NODEFCONFIG);
>      DO_TEST("net-virtio-netdev",
>              QEMU_CAPS_NETDEV, QEMU_CAPS_NODEFCONFIG);
> +    DO_TEST("net-virtio-netdev-pollus",
> +            QEMU_CAPS_NETDEV,
> +            QEMU_CAPS_VHOST_NET_POLL_US);
> +    DO_TEST_FAILURE("net-virtio-netdev-pollus-fail",
> +                    QEMU_CAPS_NETDEV,
> +                    QEMU_CAPS_VHOST_NET_POLL_US);
> +    DO_TEST_FAILURE("net-virtio-netdev-pollus-qemu-fail",
> +                    QEMU_CAPS_NETDEV,
> +                    QEMU_CAPS_VHOST_NET_POLL_US);
>      DO_TEST("net-virtio-s390",
>              QEMU_CAPS_VIRTIO_S390);
>      DO_TEST("net-virtio-ccw",
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 3/3] qemu: add busy polling support

Posted by Sahid Orentino Ferdjaoui 7 years, 10 months ago

On Sat, Jun 17, 2017 at 12:30:29PM -0400, Laine Stump wrote:
> On 06/15/2017 10:17 AM, sferdjao@redhat.com wrote:
> > From: Sahid Orentino Ferdjaoui <sahid.ferdjaoui@redhat.com>
> > 
> > This patch finalizes support of 'poll_us' attribute as an option of
> > the NIC driver-specific element, the commit also takes care of
> > hotplug.
> > 
> > <devices>
> >   <interface type='ethernet'>
> >     <mac address='52:54:00:23:bc:ba'/>
> >     <model type='virtio'/>
> >     <driver name="vhost" poll_us='50'/>
> >   </interface>
> > </devices>
> > 
> > That option is supported for all networks which are using a tap
> > device.
> > 
> > To be used the backend should be specificly set to use vhost.
> 
> Actually, libvirt's qemu driver will use vhost-net automatically as long
> as the qemu binary supports it (unless 1) the domain is using TCG
> instead of KVM, or 2) the interface configuration specifically says
> "<driver name='qemu" .../>).
> > 
> > Signed-off-by: Sahid Orentino Ferdjaoui <sahid.ferdjaoui@redhat.com>
> > ---
> >  docs/formatdomain.html.in                          | 12 ++++++++++-
> >  docs/schemas/domaincommon.rng                      |  5 +++++
> >  src/qemu/qemu_command.c                            | 22 +++++++++++++++++++++
> >  src/qemu/qemu_hotplug.c                            | 12 +++++++++++
> >  .../qemuxml2argv-net-virtio-netdev-pollus-fail.xml | 23 ++++++++++++++++++++++
> >  ...xml2argv-net-virtio-netdev-pollus-qemu-fail.xml | 23 ++++++++++++++++++++++
> >  .../qemuxml2argv-net-virtio-netdev-pollus.args     | 23 ++++++++++++++++++++++
> >  .../qemuxml2argv-net-virtio-netdev-pollus.xml      | 23 ++++++++++++++++++++++
> >  tests/qemuxml2argvtest.c                           |  9 +++++++++
> 
> The successful tests should all be done in qemuxml2xmltest.c as well
> >  9 files changed, 151 insertions(+), 1 deletion(-)
> >  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus-fail.xml
> >  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus-qemu-fail.xml
> >  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus.args
> >  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus.xml
> > 
> > diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> > index 52119f0..7bfeabc 100644
> > --- a/docs/formatdomain.html.in
> > +++ b/docs/formatdomain.html.in
> > @@ -5043,7 +5043,7 @@ qemu-kvm -net nic,model=? /dev/null
> >      &lt;model type='virtio'/&gt;
> >      <b>&lt;driver name='vhost' txmode='iothread' ioeventfd='on' event_idx='off' queues='5' rx_queue_size='256'&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;guest csum='off' tso4='off' tso6='off' ecn='off' ufo='off' poll_us='50'/&gt;
> >      &lt;/driver&gt;
> >      </b>
> >    &lt;/interface&gt;
> > @@ -5171,6 +5171,16 @@ qemu-kvm -net nic,model=? /dev/null
> >          <b>In general you should leave this option alone, unless you
> >          are very certain you know what you are doing.</b>
> >        </dd>
> > +      <dd>
> > +        The optional <code>poll_us</code> attribute configure the
> > +        maximum number of microseconds that could be spent on busy
> > +        polling. It improves the performance, but requires more
> > +        CPU. This option is only available with vhost backend driver.
> > +        <span class="since">Since 2.7.0 (QEMU and KVM only)</span><br/><br/>
> > +
> > +        <b>In general you should leave this option alone, unless you
> > +        are very certain you know what you are doing.</b>
> > +      </dd>
> >        <dt>virtio options</dt>
> >        <dd>
> >          For virtio interfaces,
> > diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> > index 4950ddc..f304385 100644
> > --- a/docs/schemas/domaincommon.rng
> > +++ b/docs/schemas/domaincommon.rng
> > @@ -2687,6 +2687,11 @@
> >                <optional>
> >                  <ref name="event_idx"/>
> >                </optional>
> > +              <optional>
> > +                <attribute name='poll_us'>
> > +                  <ref name='positiveInteger'/>
> > +                </attribute>
> > +              </optional>
> >              </group>
> >            </choice>
> >            <ref name="virtioOptions"/>
> 
> As mentioned in the previous patch, the RNG and docs changes should be
> in the previous patch. (In this patch you're going to want to add an
> entry to docs/news.xml)
> 
> > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> > index 8c12b2b..412496e 100644
> > --- a/src/qemu/qemu_command.c
> > +++ b/src/qemu/qemu_command.c
> > @@ -3955,6 +3955,9 @@ qemuBuildHostNetStr(virDomainNetDefPtr net,
> >          }
> >          if (net->tune.sndbuf_specified)
> >              virBufferAsprintf(&buf, "sndbuf=%lu,", net->tune.sndbuf);
> > +        if (net->driver.virtio.pollus)
> > +            virBufferAsprintf(&buf, "poll-us=%u,",
> > +                              net->driver.virtio.pollus);
> >      }
> >  
> >      virObjectUnref(cfg);
> > @@ -8451,6 +8454,25 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
> >          return -1;
> >      }
> >  
> > +    if (net->driver.virtio.pollus > 0) {
> > +        if (net->driver.virtio.name != VIR_DOMAIN_NET_BACKEND_TYPE_VHOST) {
> 
> I haven't checked - does this get set automatically when the vhost-net
> FD's are successfully opened?
> 
> > +            virReportError(
> > +                VIR_ERR_CONFIG_UNSUPPORTED,
> > +                _("Busy polling is only supported with vhost backend driver"));
> 
> I wonder if anyone would be confused by the lack of the exact option
> name "poll_us" in these error messages.
> 
> > +            return -1;
> > +        }
> > +        /* Nothing besides TAP devices supports busy polling. */
> > +        if (!(actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
> > +              actualType == VIR_DOMAIN_NET_TYPE_BRIDGE ||
> > +              actualType == VIR_DOMAIN_NET_TYPE_DIRECT ||
> > +              actualType == VIR_DOMAIN_NET_TYPE_ETHERNET)) {
> > +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> > +                           _("Busy polling is not supported for: %s"),
> > +                           virDomainNetTypeToString(actualType));
> > +            return -1;
> > +        }
> > +    }
> > +
> >      /* and only TAP devices support nwfilter rules */
> >      if (net->filter &&
> >          !(actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
> > diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> > index 0b8d3d8..33884b7 100644
> > --- a/src/qemu/qemu_hotplug.c
> > +++ b/src/qemu/qemu_hotplug.c
> > @@ -995,6 +995,18 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
> >          return -1;
> >      }
> >  
> > +    /* and nothing besides TAP devices supports busy polling. */
> > +    if (net->driver.virtio.pollus > 0 &&
> > +        !(actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
> > +          actualType == VIR_DOMAIN_NET_TYPE_BRIDGE ||
> > +          actualType == VIR_DOMAIN_NET_TYPE_DIRECT ||
> > +          actualType == VIR_DOMAIN_NET_TYPE_ETHERNET)) {
> > +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> > +                       _("Busy polling is not supported for: %s"),
> > +                       virDomainNetTypeToString(actualType));
> > +        return -1;
> > +    }
> 
> Huh. I just noticed there's a lot of duplicated code between
> qemuBuildInterfaceCommandLine() and qemuDomainAttachNetDevice()... (not
> your problem, but it seems like this is just *screaming* to be
> vombined/consolidated to eliminate behavioral differences / half-fixed
> bugs etc.
> 
> 
> > +
> >      /* and only TAP devices support nwfilter rules */
> >      if (net->filter &&
> >          !(actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
> > diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus-fail.xml b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus-fail.xml
> > new file mode 100644
> > index 0000000..3b664b9
> > --- /dev/null
> > +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus-fail.xml
> > @@ -0,0 +1,23 @@
> > +<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-system-i686</emulator>
> > +    <interface type='ethernet'>
> > +      <mac address='52:54:00:23:bc:ba'/>
> > +      <model type='virtio'/>
> > +      <driver poll_us='50'/>
> 
> So this is supposed to fail because the config doesn't explicitly say
> "driver name='vhost'". But as I said in the review of patch 2, vhost-net
> is used by default. So this would normally be a correct config.

If the driver name is not explicitly set it seems that libvirt is
going to use vhost but can fallback to qemu. Since that option is not
available for qemu I prefered to make this explicit.

> 
> > +    </interface>
> > +  </devices>
> > +</domain>
> > diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus-qemu-fail.xml b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus-qemu-fail.xml
> > new file mode 100644
> > index 0000000..18999c3
> > --- /dev/null
> > +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus-qemu-fail.xml
> > @@ -0,0 +1,23 @@
> > +<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-system-i686</emulator>
> > +    <interface type='ethernet'>
> > +      <mac address='52:54:00:23:bc:ba'/>
> > +      <model type='virtio'/>
> > +      <driver name="qemu" poll_us='50'/>
> 
> This one is okay - it *should* fail.
> 
> > +    </interface>
> > +  </devices>
> > +</domain>
> > diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus.args b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus.args
> > new file mode 100644
> > index 0000000..8d44134
> > --- /dev/null
> > +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus.args
> > @@ -0,0 +1,23 @@
> > +LC_ALL=C \
> > +PATH=/bin \
> > +HOME=/home/test \
> > +USER=test \
> > +LOGNAME=test \
> > +QEMU_AUDIO_DRV=none \
> > +/usr/bin/qemu-system-i686 \
> > +-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 \
> > +-netdev tap,fd=3,id=hostnet0,poll-us=50 \
> > +-device virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:23:bc:ba,bus=pci.0,\
> > +addr=0x3 \
> > +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4
> > diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus.xml b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus.xml
> > new file mode 100644
> > index 0000000..46ef7d6
> > --- /dev/null
> > +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus.xml
> > @@ -0,0 +1,23 @@
> > +<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-system-i686</emulator>
> > +    <interface type='ethernet'>
> > +      <mac address='52:54:00:23:bc:ba'/>
> > +      <model type='virtio'/>
> > +      <driver name="vhost" poll_us='50'/>
> > +    </interface>
> > +  </devices>
> > +</domain>
> > diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> > index 799aea9..54d1b07 100644
> > --- a/tests/qemuxml2argvtest.c
> > +++ b/tests/qemuxml2argvtest.c
> > @@ -1133,6 +1133,15 @@ mymain(void)
> >              QEMU_CAPS_NODEFCONFIG);
> >      DO_TEST("net-virtio-netdev",
> >              QEMU_CAPS_NETDEV, QEMU_CAPS_NODEFCONFIG);
> > +    DO_TEST("net-virtio-netdev-pollus",
> > +            QEMU_CAPS_NETDEV,
> > +            QEMU_CAPS_VHOST_NET_POLL_US);
> > +    DO_TEST_FAILURE("net-virtio-netdev-pollus-fail",
> > +                    QEMU_CAPS_NETDEV,
> > +                    QEMU_CAPS_VHOST_NET_POLL_US);
> > +    DO_TEST_FAILURE("net-virtio-netdev-pollus-qemu-fail",
> > +                    QEMU_CAPS_NETDEV,
> > +                    QEMU_CAPS_VHOST_NET_POLL_US);
> >      DO_TEST("net-virtio-s390",
> >              QEMU_CAPS_VIRTIO_S390);
> >      DO_TEST("net-virtio-ccw",
> > 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 1/3] qemu: add capability for vhost-net busy polling
[libvirt] [PATCH 2/3] conf: introduce 'poll_us' attribute for domain interface
[libvirt] [PATCH 3/3] qemu: add busy polling support