[RFC PATCH 07/10] qemu_interface: Added ebpf helper call.

[Michal Prívozník]

On 7/28/21 10:17 AM, Andrew Melnychenko wrote:
> New function to call "qemu-ebpf-rss-helper".
> The helper passes few fds through unix socket.
> Technically libvirt should not be aware how many and what those fds.
> The helper should return fds that should be passed to the qemu as is and in same order.
> 
> Signed-off-by: Andrew Melnychenko <andrew at daynix.com>
> ---
>  src/qemu/qemu_interface.c | 54 +++++++++++++++++++++++++++++++++++++++
>  src/qemu/qemu_interface.h |  2 ++
>  2 files changed, 56 insertions(+)
> 
> diff --git a/src/qemu/qemu_interface.c b/src/qemu/qemu_interface.c
> index ac0168c80d..76435af94e 100644
> --- a/src/qemu/qemu_interface.c
> +++ b/src/qemu/qemu_interface.c
> @@ -777,3 +777,57 @@ qemuInterfaceOpenVhostNet(virDomainDef *def,
>  
>      return -1;
>  }
> +
> +
> +int qemuEbpfRssHelper(const char *helper, int *fds, int nfds)
> +{
> +    int ret = 0;
> +    int err = 0;
> +    int unix_fds[2] = { -1, -1 };
> +    virCommand *cmd = NULL;

If you'd use:

g_autoptr(virCommand) cmd = NULL;

then you can ditch that explicit virCommandFree() call below.


> +
> +    if (!helper || !fds || !nfds) {
> +        return -1;
> +    }
> +
> +    ret = socketpair(AF_UNIX, SOCK_STREAM, 0, unix_fds);
> +    if (ret) {
> +        virReportSystemError(errno, "%s", _("failed to create socket"));
> +        return -1;
> +    }
> +
> +    cmd = virCommandNew(helper);
> +    if (cmd == NULL) {

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 */

> +        VIR_FORCE_CLOSE(unix_fds[1]);
> +        ret = -1;
> +        goto cleanup;
> +    }
> +    virCommandAddArgFormat(cmd, "--fd=%d", unix_fds[1]);
> +    virCommandPassFD(cmd, unix_fds[1], VIR_COMMAND_PASS_FD_CLOSE_PARENT);
> +    virCommandDoAsyncIO(cmd);
> +
> +    if (virCommandRunAsync(cmd, NULL) < 0) {
> +        VIR_FORCE_CLOSE(unix_fds[1]);

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.

> +        ret = -1;
> +        goto cleanup;
> +    }
> +
> +    memset(fds, 0, sizeof(*fds) * nfds);
> +
> +    ret = virSocketRecvMultipleFDs(unix_fds[0], fds, nfds, 0);
> +
> +    if (virCommandWait(cmd, &err) < 0) {
> +        int i = 0;
> +        for (; i < ret; ++i) {
> +            if (fds[i]) {

This check seems needless. After successful return from
virSocketRecvMultipleFDs() the @fds array must be filled with only valid
FDs.

> +                VIR_FORCE_CLOSE(fds[i]);
> +            }
> +        }
> +        ret = -1;
> +    }
> +
> +cleanup:
> +    VIR_FORCE_CLOSE(unix_fds[0]);
> +    virCommandFree(cmd);
> +    return ret;
> +}
> diff --git a/src/qemu/qemu_interface.h b/src/qemu/qemu_interface.h
> index 438d548065..63d7590035 100644
> --- a/src/qemu/qemu_interface.h
> +++ b/src/qemu/qemu_interface.h
> @@ -61,3 +61,5 @@ int qemuInterfacePrepareSlirp(virQEMUDriver *driver,
>                                qemuSlirp **slirp);
>  
>  int qemuInterfaceVDPAConnect(virDomainNetDef *net) G_GNUC_NO_INLINE;
> +
> +int qemuEbpfRssHelper(const char *helper, int *fds, int nfds);
> 

Michal