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@xxxxxxxxxx>
> ---
> 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
