[PATCH v2 1/4] domain_conf: Added configs for RSS and Hash report.

Andrew Melnychenko posted 4 patches 4 years, 1 month ago
[PATCH v2 1/4] domain_conf: Added configs for RSS and Hash report.
Posted by Andrew Melnychenko 4 years, 1 month ago
Added "rss" and "rss_hash_report" configuration that should be used with
qemu virtio RSS.
Both options are triswitches. Used as "driver" options and affects only NIC
with model type "virtio".
In other patches - options should turn on virtio-net RSS and hash properties.

Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
---
 docs/formatdomain.rst         | 18 ++++++++++++++++++
 docs/schemas/domaincommon.rng | 10 ++++++++++
 src/conf/domain_conf.c        | 31 ++++++++++++++++++++++++++++++-
 src/conf/domain_conf.h        |  2 ++
 4 files changed, 60 insertions(+), 1 deletion(-)

diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index d4f30bb8af..ce3e8a5dbf 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -5305,6 +5305,24 @@ following attributes are available for the ``"virtio"`` NIC driver:
    only for ``vhostuser`` type. :since:`Since 3.7.0 (QEMU and KVM only)`
    **In general you should leave this option alone, unless you are very certain
    you know what you are doing.**
+``rss``
+   The ``rss`` option enables in-qemu/ebpf RSS for virtio NIC. RSS works with
+   virtio and tap backends only. Virtio NIC will be launched with "rss"
+   property. For now "in-qemu" RSS is supported by libvirt.
+   QEMU may load eBPF RSS if it has CAP_SYS_ADMIN permissions, which is
+   not supported by default in libvirt.
+   **In general you should leave this option alone, unless you are very certain
+   you know what you are doing. Proper RSS configuration depends from vcpu,
+   tap, and vhost settings.**
+``rss_hash_report``
+   The ``rss_hash_report`` option enables in-qemu RSS hash report for virtio
+   NIC. Virtio NIC will be launched with a "hash" property. Network packets provided
+   to VM will contain a hash of the packet in the virt header. Usually enabled
+   alongside with ``rss``. Without ``rss`` option, the hash report doesn't affect
+   steering itself but provides vnet header with a calculated hash.
+   **In general you should leave this option alone, unless you are very certain
+   you know what you are doing. Proper RSS configuration depends from vcpu,
+   tap, and vhost settings.**
 virtio options
    For virtio interfaces, `Virtio-specific options <#elementsVirtio>`__ can also
    be set. ( :since:`Since 3.5.0` )
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 7fa5c2b8b5..9b5b94fc6c 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3595,6 +3595,16 @@
                 </optional>
               </element>
             </optional>
+            <optional>
+              <attribute name="rss">
+                <ref name="virOnOff"/>
+              </attribute>
+            </optional>
+            <optional>
+              <attribute name="rss_hash_report">
+                <ref name="virOnOff"/>
+              </attribute>
+            </optional>
           </interleave>
         </element>
       </optional>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 716c6d2240..762987e8a9 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -10271,6 +10271,8 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt,
     g_autofree char *vhost_path = NULL;
     g_autofree char *tap = NULL;
     g_autofree char *vhost = NULL;
+    g_autofree char *virtio_rss = NULL;
+    g_autofree char *virtio_rss_hash_report = NULL;
     const char *prefix = xmlopt ? xmlopt->config.netPrefix : NULL;
 
     if (!(def = virDomainNetDefNew(xmlopt)))
@@ -10412,6 +10414,8 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt,
     queues = virXMLPropString(driver_node, "queues");
     rx_queue_size = virXMLPropString(driver_node, "rx_queue_size");
     tx_queue_size = virXMLPropString(driver_node, "tx_queue_size");
+    virtio_rss = virXMLPropString(driver_node, "rss");
+    virtio_rss_hash_report = virXMLPropString(driver_node, "rss_hash_report");
 
     if ((filterref_node = virXPathNode("./filterref", ctxt))) {
         filter = virXMLPropString(filterref_node, "filter");
@@ -10822,7 +10826,24 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt,
             }
             def->driver.virtio.tx_queue_size = q;
         }
-
+        if (virtio_rss) {
+            if ((val = virTristateSwitchTypeFromString(virtio_rss)) <= 0) {
+                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                        _("'rss' attribute must be 'on'/'off'/'default': %s"),
+                        virtio_rss);
+                goto error;
+            }
+            def->driver.virtio.rss = val;
+        }
+        if (virtio_rss_hash_report) {
+            if ((val = virTristateSwitchTypeFromString(virtio_rss_hash_report)) <= 0) {
+                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                        _("'rss_hash_report' attribute must be 'on'/'off'/'default': %s"),
+                        virtio_rss_hash_report);
+                goto error;
+            }
+            def->driver.virtio.rss_hash_report = val;
+        }
         if ((tmpNode = virXPathNode("./driver/host", ctxt))) {
             if (virXMLPropTristateSwitch(tmpNode, "csum", VIR_XML_PROP_NONE,
                                          &def->driver.virtio.host.csum) < 0)
@@ -24751,6 +24772,14 @@ virDomainVirtioNetDriverFormat(virBuffer *buf,
     if (def->driver.virtio.tx_queue_size)
         virBufferAsprintf(buf, " tx_queue_size='%u'",
                           def->driver.virtio.tx_queue_size);
+    if (def->driver.virtio.rss != VIR_TRISTATE_SWITCH_ABSENT) {
+        virBufferAsprintf(buf, " rss='%s'",
+                          virTristateSwitchTypeToString(def->driver.virtio.rss));
+    }
+    if (def->driver.virtio.rss_hash_report != VIR_TRISTATE_SWITCH_ABSENT) {
+        virBufferAsprintf(buf, " rss_hash_report='%s'",
+                          virTristateSwitchTypeToString(def->driver.virtio.rss_hash_report));
+    }
 
     virDomainVirtioOptionsFormat(buf, def->virtio);
 }
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 144ba4dd12..64ebff012e 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1055,6 +1055,8 @@ struct _virDomainNetDef {
                 virTristateSwitch ecn;
                 virTristateSwitch ufo;
             } guest;
+            virTristateSwitch rss;
+            virTristateSwitch rss_hash_report;
         } virtio;
     } driver;
     struct {
-- 
2.34.1

Re: [PATCH v2 1/4] domain_conf: Added configs for RSS and Hash report.
Posted by Michal Prívozník 3 years, 10 months ago
On 1/9/22 22:07, Andrew Melnychenko wrote:
> Added "rss" and "rss_hash_report" configuration that should be used with
> qemu virtio RSS.
> Both options are triswitches. Used as "driver" options and affects only NIC
> with model type "virtio".
> In other patches - options should turn on virtio-net RSS and hash properties.
> 
> Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
> ---
>  docs/formatdomain.rst         | 18 ++++++++++++++++++
>  docs/schemas/domaincommon.rng | 10 ++++++++++
>  src/conf/domain_conf.c        | 31 ++++++++++++++++++++++++++++++-
>  src/conf/domain_conf.h        |  2 ++
>  4 files changed, 60 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
> index d4f30bb8af..ce3e8a5dbf 100644
> --- a/docs/formatdomain.rst
> +++ b/docs/formatdomain.rst
> @@ -5305,6 +5305,24 @@ following attributes are available for the ``"virtio"`` NIC driver:
>     only for ``vhostuser`` type. :since:`Since 3.7.0 (QEMU and KVM only)`
>     **In general you should leave this option alone, unless you are very certain
>     you know what you are doing.**
> +``rss``
> +   The ``rss`` option enables in-qemu/ebpf RSS for virtio NIC. RSS works with
> +   virtio and tap backends only. Virtio NIC will be launched with "rss"
> +   property. For now "in-qemu" RSS is supported by libvirt.
> +   QEMU may load eBPF RSS if it has CAP_SYS_ADMIN permissions, which is
> +   not supported by default in libvirt.
> +   **In general you should leave this option alone, unless you are very certain
> +   you know what you are doing. Proper RSS configuration depends from vcpu,
> +   tap, and vhost settings.**
> +``rss_hash_report``
> +   The ``rss_hash_report`` option enables in-qemu RSS hash report for virtio
> +   NIC. Virtio NIC will be launched with a "hash" property. Network packets provided
> +   to VM will contain a hash of the packet in the virt header. Usually enabled
> +   alongside with ``rss``. Without ``rss`` option, the hash report doesn't affect
> +   steering itself but provides vnet header with a calculated hash.
> +   **In general you should leave this option alone, unless you are very certain
> +   you know what you are doing. Proper RSS configuration depends from vcpu,
> +   tap, and vhost settings.**
>  virtio options
>     For virtio interfaces, `Virtio-specific options <#elementsVirtio>`__ can also
>     be set. ( :since:`Since 3.5.0` )
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 7fa5c2b8b5..9b5b94fc6c 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -3595,6 +3595,16 @@
>                  </optional>
>                </element>
>              </optional>
> +            <optional>
> +              <attribute name="rss">
> +                <ref name="virOnOff"/>
> +              </attribute>
> +            </optional>
> +            <optional>
> +              <attribute name="rss_hash_report">
> +                <ref name="virOnOff"/>
> +              </attribute>
> +            </optional>
>            </interleave>
>          </element>
>        </optional>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 716c6d2240..762987e8a9 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -10271,6 +10271,8 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt,
>      g_autofree char *vhost_path = NULL;
>      g_autofree char *tap = NULL;
>      g_autofree char *vhost = NULL;
> +    g_autofree char *virtio_rss = NULL;
> +    g_autofree char *virtio_rss_hash_report = NULL;
>      const char *prefix = xmlopt ? xmlopt->config.netPrefix : NULL;
>  
>      if (!(def = virDomainNetDefNew(xmlopt)))
> @@ -10412,6 +10414,8 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt,
>      queues = virXMLPropString(driver_node, "queues");
>      rx_queue_size = virXMLPropString(driver_node, "rx_queue_size");
>      tx_queue_size = virXMLPropString(driver_node, "tx_queue_size");
> +    virtio_rss = virXMLPropString(driver_node, "rss");
> +    virtio_rss_hash_report = virXMLPropString(driver_node, "rss_hash_report");
>  
>      if ((filterref_node = virXPathNode("./filterref", ctxt))) {
>          filter = virXMLPropString(filterref_node, "filter");
> @@ -10822,7 +10826,24 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt,
>              }
>              def->driver.virtio.tx_queue_size = q;
>          }
> -
> +        if (virtio_rss) {
> +            if ((val = virTristateSwitchTypeFromString(virtio_rss)) <= 0) {
> +                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                        _("'rss' attribute must be 'on'/'off'/'default': %s"),
> +                        virtio_rss);
> +                goto error;
> +            }
> +            def->driver.virtio.rss = val;
> +        }
> +        if (virtio_rss_hash_report) {
> +            if ((val = virTristateSwitchTypeFromString(virtio_rss_hash_report)) <= 0) {
> +                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                        _("'rss_hash_report' attribute must be 'on'/'off'/'default': %s"),
> +                        virtio_rss_hash_report);
> +                goto error;
> +            }
> +            def->driver.virtio.rss_hash_report = val;
> +        }
>          if ((tmpNode = virXPathNode("./driver/host", ctxt))) {
>              if (virXMLPropTristateSwitch(tmpNode, "csum", VIR_XML_PROP_NONE,
>                                           &def->driver.virtio.host.csum) < 0)


Upstream has moved to virXMLPropTristateSwitch() meanwhile.

Usually, when I'm introducing new XML knob I include a xml2xml test case
within the same patch to demonstrate that parser and formatter are
working. It's okay to introduce such test case in a separate patch too,
but looks like you're not doing that either.

Let me fix that just before merge.

Michal