On 7/28/21 10:17 AM, Andrew Melnychenko wrote:
> The helper called before NIC string creation. EBPF fds passed to the child process.
> The helper called if qemu supports "ebpf_rss_fds" and returns the helper path.
> If libvirt can't retrieve fds from the helper, qemu will be launched
> without "ebpf_rss_fds" virtio-net property.
>
> Signed-off-by: Andrew Melnychenko <andrew@xxxxxxxxxx>
> ---
> src/qemu/qemu_command.c | 36 +++++++++++++++++++++++++++++++++++-
> src/qemu/qemu_command.h | 2 ++
> 2 files changed, 37 insertions(+), 1 deletion(-)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 67a396a513..2fffcee609 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -3377,11 +3377,14 @@ qemuBuildNicDevStr(virDomainDef *def,
> virDomainNetDef *net,
> unsigned int bootindex,
> size_t vhostfdSize,
> + char **ebpfRSSfds,
> + size_t ebpfRSSnfds,
> virQEMUCaps *qemuCaps)
> {
> g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
> bool usingVirtio = false;
> char macaddr[VIR_MAC_STRING_BUFLEN];
> + size_t i = 0;
>
> if (virDomainNetIsVirtioModel(net)) {
> if (qemuBuildVirtioDevStr(&buf, "virtio-net", qemuCaps,
> @@ -3498,6 +3501,15 @@ qemuBuildNicDevStr(virDomainDef *def,
> if (net->driver.virtio.rss == VIR_TRISTATE_SWITCH_ON) {
> virBufferAsprintf(&buf, ",rss=%s",
> virTristateSwitchTypeToString(net->driver.virtio.rss));
> +
> + if (ebpfRSSfds != NULL && ebpfRSSnfds)
> + {
I think it's sufficient to check just for ebpfRSSnfds > 0 because it
can't happen that these two variable disagree.
> + virBufferAsprintf(&buf, ",ebpf_rss_fds=");
> + for (i = 0; i < (ebpfRSSnfds - 1); ++i) {
> + virBufferAsprintf(&buf, "%s:",ebpfRSSfds[i]);
> + }
> + virBufferAsprintf(&buf, "%s",ebpfRSSfds[ebpfRSSnfds - 1]);
> + }
> }
>
> if (net->driver.virtio.rss_hash_report == VIR_TRISTATE_SWITCH_ON) {
> @@ -8492,6 +8504,9 @@ qemuBuildInterfaceCommandLine(virQEMUDriver *driver,
> qemuSlirp *slirp;
> size_t i;
> g_autoptr(virJSONValue) hostnetprops = NULL;
> + char **ebpfRSSfdsName = NULL;
> + int ebpfRSSfds[16] = {};
This constant look magic.
> + int ebpfRSSnfds = 0;
>
>
> if (!bootindex)
> @@ -8744,8 +8759,20 @@ qemuBuildInterfaceCommandLine(virQEMUDriver *driver,
> if (qemuCommandAddExtDevice(cmd, &net->info) < 0)
> goto cleanup;
>
> + if (net->driver.virtio.rss == VIR_TRISTATE_SWITCH_ON
> + && virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_EBPF_RSS_FDS)) {
> + ebpfRSSnfds = qemuEbpfRssHelper(virQEMUCapsGetEBPFHelperPath(qemuCaps), ebpfRSSfds, 16);
> + if (ebpfRSSnfds > 0) {
> + ebpfRSSfdsName = g_new0(char *, ebpfRSSnfds);
> + for (i = 0; i < ebpfRSSnfds; ++i) {
> + ebpfRSSfdsName[i] = g_strdup_printf("%d", ebpfRSSfds[i]);
> + virCommandPassFD(cmd, ebpfRSSfds[i], VIR_COMMAND_PASS_FD_CLOSE_PARENT);
> + }
> + }
> + }
> +
> if (!(nic = qemuBuildNicDevStr(def, net, bootindex,
> - net->driver.virtio.queues, qemuCaps)))
> + net->driver.virtio.queues, ebpfRSSfdsName, ebpfRSSnfds, qemuCaps)))
> goto cleanup;
> virCommandAddArgList(cmd, "-device", nic, NULL);
> } else if (!requireNicdev) {
> @@ -8785,6 +8812,13 @@ qemuBuildInterfaceCommandLine(virQEMUDriver *driver,
> }
> VIR_FREE(tapfdName);
> VIR_FREE(vhostfd);
> + for (i = 0; ret < 0 && ebpfRSSfds[i] && i < 16; i++) {
> + if (ret < 0)
> + VIR_FORCE_CLOSE(ebpfRSSfds[i]);
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
