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. Also "rss" may used to affect "ebpf_rss_fds" property of virtio-net in qemu. Signed-off-by: Andrew Melnychenko <andrew(a)daynix.com&gt; --- src/conf/domain_conf.c | 31 ++++++++++++++++++++++++++++++- src/conf/domain_conf.h | 2 ++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 06c1fcf5e5..7289acc9c6 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10244,6 +10244,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))) @@ -10385,6 +10387,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"); @@ -10796,7 +10800,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) @@ -24568,6 +24589,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 ca21082624..60cb70b204 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1046,6 +1046,8 @@ struct _virDomainNetDef { virTristateSwitch ecn; virTristateSwitch ufo; } guest; + virTristateSwitch rss; + virTristateSwitch rss_hash_report; } virtio; } driver; struct { -- 2.31.1

On 7/28/21 10:17 AM, Andrew Melnychenko wrote: This needs to be updated because meanwhile we introduced comments to these enum items. Michal

If domain config contains "rss" and/or "rss_hash_report" options for driver. Also if the qemu has device capabilities for RSS. Libvirt will create an NIC device command line with enabled "rss"/"hash". Signed-off-by: Andrew Melnychenko <andrew(a)daynix.com&gt; --- src/qemu/qemu_command.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 156af4caee..67a396a513 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3495,6 +3495,16 @@ qemuBuildNicDevStr(virDomainDef *def, if (net->driver.virtio.tx_queue_size) virBufferAsprintf(&buf, ",tx_queue_size=%u", net->driver.virtio.tx_queue_size); + if (net->driver.virtio.rss == VIR_TRISTATE_SWITCH_ON) { + virBufferAsprintf(&buf, ",rss=%s", + virTristateSwitchTypeToString(net->driver.virtio.rss)); + } + + if (net->driver.virtio.rss_hash_report == VIR_TRISTATE_SWITCH_ON) { + virBufferAsprintf(&buf, ",hash=%s", + virTristateSwitchTypeToString(net->driver.virtio.rss_hash_report)); + } + if (net->mtu) virBufferAsprintf(&buf, ",host_mtu=%u", net->mtu); -- 2.31.1

On 7/28/21 10:17 AM, Andrew Melnychenko wrote: This needs to be ordered. The correct order is: # util/virsocket.h virSocketRecvFD; virSocketRecvMultipleFDs; virSocketSendFD; Please put a space before and after '/'. Like this: ret = fdSize / sizeof(int); This isn't needed really, because .. .. VIR_FORCE_CLOSE() doesn't change errno. But I wonder if this function is needed. I mean, we currently have virSocketRecvFD() and this new function looks very much like it. Would it be possible to turn virSocketRecvFD() into virSocketRecvMultipleFDs() and fix all (current) callers of virSocketRecvFD()? Alternatively, we can have virSocketRecvMultipleFDs() as you propose and then virSocketRecvFD() be just a thin wrapper over virSocketRecvMultipleFDs(), e.g. like this: virSocketRecvFD() { int fds[1]; virSocketRecvMultipleFDs(sock, fds, 1, fdflags); return fds[0]; } Or even better, have virSocketRecvFD() return FD via argument and its retval be 0/-1 (success/fail). My aim is to avoid having nearly the same code twice. Michal

Added check for qemu virtio-net "ebpf_rss_fds" property. This property allows to pass eBPF program/map file descriptors for RSS program. In other patches, libvirt may launch qemu-ebpf-rss-helper and pass fds to virtio-net qemu. Signed-off-by: Andrew Melnychenko <andrew(a)daynix.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 7cd6dab259..f16115226d 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -638,6 +638,7 @@ VIR_ENUM_IMPL(virQEMUCaps, "query-display-options", "s390-pv-guest", "virtio-net.rss", + "virtio-net.ebpf_rss_fds", ); @@ -1423,6 +1424,7 @@ static struct virQEMUCapsDevicePropsFlags virQEMUCapsDevicePropsVirtioNet[] = { { "packed", QEMU_CAPS_VIRTIO_PACKED_QUEUES, NULL }, { "acpi-index", QEMU_CAPS_ACPI_INDEX, NULL }, { "rss", QEMU_CAPS_VIRTIO_RSS, NULL }, + { "ebpf_rss_fds", QEMU_CAPS_VIRTIO_EBPF_RSS_FDS, NULL }, }; static struct virQEMUCapsDevicePropsFlags virQEMUCapsDevicePropsPCIeRootPort[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 6b2446fe5f..c938b63e91 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -618,6 +618,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ QEMU_CAPS_QUERY_DISPLAY_OPTIONS, /* 'query-display-options' qmp command present */ QEMU_CAPS_S390_PV_GUEST, /* -object s390-pv-guest,... */ QEMU_CAPS_VIRTIO_RSS, /* virtio-net rss feature */ + QEMU_CAPS_VIRTIO_EBPF_RSS_FDS, /* virtio-net ebpf_rss_fds feature */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; -- 2.31.1

On 8/20/21 2:57 PM, Michal Prívozník wrote: Ooops, hit send too early. What I wanted to say is that it's perfectly okay to send patches before QEMU part is merged. However, libvirt part won't be merged before QEMU. We did this mistake too many times before and learned our lesson. Michal

Added qmp call to receive helper path. Also added monitor functions. For virQEMUCaps added field for helper path. Libvirt queries "query-helper-paths" from qmp and saves in qemuCaps. The qemu and the helper should be "sync" and the helper should return proper eBPF fd and maps with "interface" for current qemu. So, qemu returns where to find the helper through qmp, if the qemu supports "ebpf_rss_fds". Signed-off-by: Andrew Melnychenko <andrew(a)daynix.com&gt; --- src/qemu/qemu_capabilities.c | 44 +++++++++++++++++++++++++++++++ src/qemu/qemu_capabilities.h | 3 +++ src/qemu/qemu_monitor.c | 9 +++++++ src/qemu/qemu_monitor.h | 3 +++ src/qemu/qemu_monitor_json.c | 50 ++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 3 +++ 6 files changed, 112 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index f16115226d..f62088d32f 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -722,6 +722,9 @@ struct _virQEMUCaps { /* Capabilities which may differ depending on the accelerator. */ virQEMUCapsAccel kvm; virQEMUCapsAccel tcg; + + /* Helpers returned by qemu */ + char *helperEbpfRSS; }; struct virQEMUCapsSearchData { @@ -1994,6 +1997,8 @@ virQEMUCaps *virQEMUCapsNewCopy(virQEMUCaps *qemuCaps) qemuCaps->sevCapabilities) < 0) goto error; + ret->helperEbpfRSS = g_strdup(qemuCaps->helperEbpfRSS); + return ret; error: @@ -2037,6 +2042,8 @@ void virQEMUCapsDispose(void *obj) virQEMUCapsAccelClear(&qemuCaps->kvm); virQEMUCapsAccelClear(&qemuCaps->tcg); + + g_free(qemuCaps->helperEbpfRSS); } void @@ -4418,6 +4425,8 @@ virQEMUCapsLoadCache(virArch hostArch, if (virXPathBoolean("boolean(./kvmSupportsSecureGuest)", ctxt) > 0) qemuCaps->kvmSupportsSecureGuest = true; + qemuCaps->helperEbpfRSS = virXPathString("string(./EbpfHelperPath)", ctxt); + if (skipInvalidation) qemuCaps->invalidation = false; @@ -4671,6 +4680,10 @@ virQEMUCapsFormatCache(virQEMUCaps *qemuCaps) if (qemuCaps->kvmSupportsSecureGuest) virBufferAddLit(&buf, "<kvmSupportsSecureGuest/>\n"); + if (qemuCaps->helperEbpfRSS) { + virBufferAsprintf(&buf, "<EbpfHelperPath>%s</EbpfHelperPath>\n", qemuCaps->helperEbpfRSS); + } + virBufferAdjustIndent(&buf, -2); virBufferAddLit(&buf, "</qemuCaps>\n"); @@ -5265,6 +5278,30 @@ virQEMUCapsGetVirtType(virQEMUCaps *qemuCaps) return type; } +static int +virQEMUCapsProbeQMPHelperPath(virQEMUCaps *qemuCaps, + qemuMonitor *mon) +{ + g_autoptr(GHashTable) helperList = NULL; + const char *entry = NULL; + + helperList = qemuMonitorGetHelperPath(mon); + + if (!helperList) { + return -1; + } + + /* TODO: parse all helpers? */ + entry = virHashLookup(helperList, "qemu-ebpf-rss-helper"); + if (!entry) { + return -1; + } + + qemuCaps->helperEbpfRSS = g_strdup(entry); + + return 0; +} + int virQEMUCapsInitQMPMonitor(virQEMUCaps *qemuCaps, qemuMonitor *mon) @@ -5345,6 +5382,8 @@ virQEMUCapsInitQMPMonitor(virQEMUCaps *qemuCaps, if (virQEMUCapsProbeQMPHostCPU(qemuCaps, accel, mon, type) < 0) return -1; + virQEMUCapsProbeQMPHelperPath(qemuCaps, mon); + return 0; } @@ -6459,3 +6498,8 @@ virQEMUCapsStripMachineAliases(virQEMUCaps *qemuCaps) virQEMUCapsStripMachineAliasesForVirtType(qemuCaps, VIR_DOMAIN_VIRT_KVM); virQEMUCapsStripMachineAliasesForVirtType(qemuCaps, VIR_DOMAIN_VIRT_QEMU); } + +const char *virQEMUCapsGetEBPFHelperPath(virQEMUCaps *qemuCaps) +{ + return qemuCaps->helperEbpfRSS; +} diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index c938b63e91..b25bdd4ec3 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -816,3 +816,6 @@ virQEMUCapsGetKVMSupportsSecureGuest(virQEMUCaps *qemuCaps) G_GNUC_NO_INLINE; virArch virQEMUCapsArchFromString(const char *arch); const char *virQEMUCapsArchToString(virArch arch); + +const char * +virQEMUCapsGetEBPFHelperPath(virQEMUCaps *qemuCaps); diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 6e2d8010c5..585fb2d6b4 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -4617,3 +4617,12 @@ qemuMonitorQueryDirtyRate(qemuMonitor *mon, return qemuMonitorJSONQueryDirtyRate(mon, info); } + + +GHashTable * +qemuMonitorGetHelperPath(qemuMonitor *mon) +{ + QEMU_CHECK_MONITOR_NULL(mon); + + return qemuMonitorJSONGetHelperPath(mon); +} diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 1491c1297c..0ee87d2c46 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1491,3 +1491,6 @@ struct _qemuMonitorDirtyRateInfo { int qemuMonitorQueryDirtyRate(qemuMonitor *mon, qemuMonitorDirtyRateInfo *info); + +GHashTable * +qemuMonitorGetHelperPath(qemuMonitor *mon); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index ca2c3bb6cf..0579eee36a 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -9387,3 +9387,53 @@ qemuMonitorJSONQueryDirtyRate(qemuMonitor *mon, return qemuMonitorJSONExtractDirtyRateInfo(data, info); } + +static int +qemuMonitorJSONGetHelperPathWorker(size_t pos G_GNUC_UNUSED, + virJSONValue *item, + void *opaque) +{ + const char *name = virJSONValueObjectGetString(item, "name"); + const char *path = virJSONValueObjectGetString(item, "path"); + GHashTable *pathsList = opaque; + + if (!name) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("reply data was missing 'name'")); + return -1; + } + + if (virHashAddEntry(pathsList, name, g_strdup(path)) < 0) + return -1; + + return 0; +} + +GHashTable * +qemuMonitorJSONGetHelperPath(qemuMonitor *mon) +{ + g_autoptr(GHashTable) pathsList = virHashNew(g_free); + g_autoptr(virJSONValue) cmd = NULL; + g_autoptr(virJSONValue) reply = NULL; + + if (!(cmd = qemuMonitorJSONMakeCommand("query-helper-paths", NULL))) + return NULL; + + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + return NULL; + + /* return empty hash */ + if (qemuMonitorJSONHasError(reply, "CommandNotFound")) + return g_steal_pointer(&pathsList); + + if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_ARRAY) < 0) + return NULL; + + if (virJSONValueArrayForeachSteal(virJSONValueObjectGetArray(reply, "return"), + qemuMonitorJSONGetHelperPathWorker, + pathsList) < 0) + return NULL; + + return g_steal_pointer(&pathsList); +} + diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 01a3ba25f1..80e47cf68f 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -706,3 +706,6 @@ qemuMonitorJSONStartDirtyRateCalc(qemuMonitor *mon, int qemuMonitorJSONQueryDirtyRate(qemuMonitor *mon, qemuMonitorDirtyRateInfo *info); + +GHashTable * +qemuMonitorJSONGetHelperPath(qemuMonitor *mon); -- 2.31.1

Hi, Well, qemu-ebpf-rss-helper is kinda unique for each qemu emulator, that's why it's stored with qemucaps. On Fri, Aug 20, 2021 at 3:57 PM Michal Prívozník <mprivozn(a)redhat.com&gt; wrote:

On 7/28/21 10:17 AM, Andrew Melnychenko wrote: If you'd use: g_autoptr(virCommand) cmd = NULL; then you can ditch that explicit virCommandFree() call below. This can't really happen. Also, all virCommand* APIs are prepared for this situation so that we can write simpler code: cmd = virCommandNew(); virCommandSomething(cmd); virCommandSomethingElse(cmd); if (virCommandRunAsync(cmd) < 0) /* This is the point where we learn about any previous error */ This doesn't look right. Even in case of failure virCommandRun* should have closed all CLOSE_PARENT FDs. So this is effectively a double close. This check seems needless. After successful return from virSocketRecvMultipleFDs() the @fds array must be filled with only valid FDs. Michal

On 7/28/21 10:17 AM, Andrew Melnychenko wrote: I think it's sufficient to check just for ebpfRSSnfds > 0 because it can't happen that these two variable disagree. This constant look magic. I'm this this ret < 0 check is duplicated. Also usually we write it like this: if (ret < 0) { for () ; /* other cleanup code goes here */ } Michal

Signed-off-by: Andrew Melnychenko <andrew(a)daynix.com&gt; --- docs/formatdomain.rst | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 61ccd8895a..22ad19439f 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -5233,6 +5233,22 @@ 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. For ebpf RSS the "ebpf helper" is used. Helper + binary path received from qemu by qmp. The helper is't used if qemu doesn't + provide "ebpf_rss_fds" property or helper path. Without helper, virtio NIC + will be launched only with "rss" property. Qemu may load eBPF by itself if it + has CAP_SYS_ADMIN permissions. In other cases, "in-qemu" RSS is used. + **In general you should leave this option alone, unless you are very certain + you know what you are doing.** +``rss_hash_report`` + The ``rss_hash_report`` option enables in-qemu RSS hash report for virtio + NIC. Ebpf RSS doesn't support hash report yet. 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.** virtio options For virtio interfaces, `Virtio-specific options <#elementsVirtio>`__ can also be set. ( :since:`Since 3.5.0` ) -- 2.31.1