Re: [RFC PATCH 4/5] qmp: Added qemu-ebpf-rss-path command.

[Eric Blake]

On Wed, Jun 09, 2021 at 01:04:56PM +0300, Andrew Melnychenko wrote:
> New qmp command to query ebpf helper.
> It's crucial that qemu and helper are in sync and in touch.
> Technically helper should pass eBPF fds that qemu may accept.
> And different qemu's builds may have different eBPF programs and helpers.
> Qemu returns helper that should "fit" to virtio-net.
> 
> Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
> ---
>  monitor/qmp-cmds.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++
>  qapi/misc.json     | 29 +++++++++++++++++
>  2 files changed, 107 insertions(+)
> 
> diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
> index f7d64a6457..5dd2a58ea2 100644
> --- a/monitor/qmp-cmds.c
> +++ b/monitor/qmp-cmds.c
> @@ -351,3 +351,81 @@ void qmp_display_reload(DisplayReloadOptions *arg, Error 
> **errp)
>          abort();
>      }
>  }
> +
> +#ifdef CONFIG_LINUX
> +
> +static const char *get_dirname(char *path)
> +{
> +    char *sep;
> +
> +    sep = strrchr(path, '/');
> +    if (sep == path) {
> +        return "/";
> +    } else if (sep) {
> +        *sep = 0;
> +        return path;
> +    }
> +    return ".";
> +}

Seems like this function is duplicating what glib should already be
able to do.

> +
> +static char *find_helper(const char *name)
> +{
> +    char qemu_exec[PATH_MAX];

Stack-allocating a PATH_MAX array for readlink() is poor practice.
Better is to use g_file_read_link().

> +    const char *qemu_dir = NULL;
> +    char *helper = NULL;
> +
> +    if (name == NULL) {
> +        return NULL;
> +    }
> +
> +    if (readlink("/proc/self/exe", qemu_exec, PATH_MAX) > 0) {
> +        qemu_dir = get_dirname(qemu_exec);
> +
> +        helper = g_strdup_printf("%s/%s", qemu_dir, name);
> +        if (access(helper, F_OK) == 0) {
> +            return helper;
> +        }
> +        g_free(helper);
> +    }
> +
> +    helper = g_strdup_printf("%s/%s", CONFIG_QEMU_HELPERDIR, name);

Could we use a compile-time determination of where we were (supposed)
to be installed, and therefore where our helper should be installed,
rather than the dynamic /proc/self/exe munging?

> +    if (access(helper, F_OK) == 0) {
> +        return helper;
> +    }
> +    g_free(helper);
> +
> +    return NULL;
> +}
> +
> +HelperPathList *qmp_query_helper_paths(Error **errp)
> +{
> +    HelperPathList *ret = NULL;
> +    const char *helpers_list[] = {
> +#ifdef CONFIG_EBPF
> +        "qemu-ebpf-rss-helper",
> +#endif

So the intent is that we can make this list larger if we write other
helper binaries.  But this code is in an overall #ifdef CONFIG_LINUX,
which means it won't work on other platforms.

> +#else
> +
> +HelperPathList *qmp_query_helper_paths(Error **errp)
> +{
> +    return NULL;
> +}
> +
> +#endif
> diff --git a/qapi/misc.json b/qapi/misc.json
> index 156f98203e..023bd2120d 100644
> --- a/qapi/misc.json
> +++ b/qapi/misc.json
> @@ -519,3 +519,32 @@
>   'data': { '*option': 'str' },
>   'returns': ['CommandLineOptionInfo'],
>   'allow-preconfig': true }
> +
> +##
> +# @HelperPath:
> +#
> +# Name of the helper and binary location.
> +##

Missing a 'Since: 6.1' line.

> +{ 'struct': 'HelperPath',
> +  'data': {'name': 'str', 'path': 'str'} }
> +
> +##
> +# @query-helper-paths:
> +#
> +# Query specific eBPF RSS helper for current qemu binary.
> +#
> +# Returns: list of object that contains name and path for helper.
> +#
> +# Example:
> +#
> +# -> { "execute": "query-helper-paths" }
> +# <- { "return": [
> +#        {
> +#          "name": "qemu-ebpf-rss-helper",
> +#          "path": "/usr/local/libexec/qemu-ebpf-rss-helper"
> +#        }
> +#      ]
> +#    }
> +#
> +##
> +{ 'command': 'query-helper-paths', 'returns': ['HelperPath'] }

Missing an 'if':... to designate that this command is only useful on
Linux.  Or you can make it work on other platforms with my suggestions
above about better utilizing glib functionality at runtime or even
just relying on compile-time use of the configured install locations.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org