[RFC PATCH 08/10] qemu_command: Added ebpf RSS helper call for NIC creation.

[Michal Prívozník]

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 at daynix.com>
> ---
>  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