From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-10.6 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,HTML_MESSAGE,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 37422C48BDF for ; Tue, 15 Jun 2021 23:17:58 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 64394610CD for ; Tue, 15 Jun 2021 23:17:57 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 64394610CD Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=daynix.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Received: from localhost ([::1]:35546 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1ltIJg-0005Fd-0P for qemu-devel@archiver.kernel.org; Tue, 15 Jun 2021 19:17:56 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:33296) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ltIIT-000450-Ac for qemu-devel@nongnu.org; Tue, 15 Jun 2021 19:16:41 -0400 Received: from mail-ot1-x329.google.com ([2607:f8b0:4864:20::329]:41864) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1ltIIP-0005od-Vn for qemu-devel@nongnu.org; Tue, 15 Jun 2021 19:16:41 -0400 Received: by mail-ot1-x329.google.com with SMTP id p5-20020a9d45450000b029043ee61dce6bso599968oti.8 for ; Tue, 15 Jun 2021 16:16:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=daynix-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=fc+0qZBkRB7VcPWwE/haf1PRjUybDq3TeDkBbp05LMU=; b=shfGpzj6NHMT4ZWpvbREB9PA7p3jKBVunB1HqkUbI8FlWeVX7XX0HyuinZeMsmtRA7 bbMhKPdlO+w6tUwYtIx2U58tiTmD5iiZOKlJzjyWpWlG2eo6gLtrXcKKZz8+k70Uj0nv anjYusbzt4dGqL/qeyQTP0OxLjd1WPCoTQbtMWu66LBDtjsh+YFlTNIRQcM0b7nboOF6 P/4r0XguCNyynBvGgJ4EuzwYrZ8xQfIrkgh4KY3i1lj7ZKqS1ODm1pWoc+Qk7N0vaj1f ELF46U7Kg0ZKk6Un++hdJP6EfaZigfd63XhhSqAgA2IGHjueBpCjHGw14x+Y8r1nvUV+ 4YKg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=fc+0qZBkRB7VcPWwE/haf1PRjUybDq3TeDkBbp05LMU=; b=nbo3smRAJS2CO2WmlT04/9fLbwHYxjAmPOUel0hjEd0T2P9A4e4NMLBG8o/OZ1RGXO CWAkyLNcO5pVHEnwCsO/ETbeG7zWg/d0V0jpokLU8sFCAJwpxQ3xRohlKajGb89Adnwp QXJYXnGxzieeJm41z44IRwAuR6hHzbuBURxcRlKwc8b1bXIoxa3DCmxUnohM8h5SaJxy N3ZwqjOVAH9zP0m4bWwZ0Zo4YiYDMopSii4vgXyOK5/p8w+xKoDtTs+LPXWCuyc3pyWz 3a4nZBM075bLMLrT2mIJOKOVcY9mnyc3z+D2sO13IvXGXfKVbaWPcB5opSNQcTNmPU3l epqA== X-Gm-Message-State: AOAM530Sk5U1QFOYST1nnHK0zpWm06XEDx+IYPwMw+NcGw18p5RPmaYA ZhqlzhVWPtChogR5Ugty0ZqCiIG9Oahb0fgKx/3JMg== X-Google-Smtp-Source: ABdhPJxlxzHpVQPYSi4KoWGoYeCsHhnGpGkizMH4weVtBLMknpJqZZe0Z4u/RA4enR7Ur7gSjdqik/kq2/57KG3ODGY= X-Received: by 2002:a9d:7a4b:: with SMTP id z11mr1311439otm.67.1623798996551; Tue, 15 Jun 2021 16:16:36 -0700 (PDT) MIME-Version: 1.0 References: <20210609100457.142570-1-andrew@daynix.com> <20210609100457.142570-5-andrew@daynix.com> <87fsxnejnw.fsf@dusky.pond.sub.org> In-Reply-To: <87fsxnejnw.fsf@dusky.pond.sub.org> From: Andrew Melnichenko Date: Wed, 16 Jun 2021 02:16:25 +0300 Message-ID: Subject: Re: [RFC PATCH 4/5] qmp: Added qemu-ebpf-rss-path command. To: Markus Armbruster Content-Type: multipart/alternative; boundary="000000000000605c7905c4d62af7" Received-SPF: none client-ip=2607:f8b0:4864:20::329; envelope-from=andrew@daynix.com; helo=mail-ot1-x329.google.com X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_NONE=0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: =?UTF-8?Q?Daniel_P=2E_Berrang=C3=A9?= , "Michael S. Tsirkin" , Jason Wang , qemu-devel@nongnu.org, Yuri Benditovich , Yan Vugenfirer , Eric Blake Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" --000000000000605c7905c4d62af7 Content-Type: text/plain; charset="UTF-8" Hi all, > Seems like this function is duplicating what glib should already be > able to do. > Yea, but it's required a Linux specific header - without it, qemu builds but crashes. 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? > Yes, we can define something like CONFIG_QEMU_HELPERDIR ## "qemu-ebpf-rss-helper", for RSS helper. But I've tried to implement generic function for possible other helpers. Yeah I think avoiding /proc/self/exe is desirable, because I can > imagine scenarios where this can lead to picking the wrong helper. > Better to always use the compile time install directory. > The main scenario that find_helper() should solve - non installed qemu use helper from own build. That's why reading /proc/self/exe is implemented. 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. > Yes, for now, eBPF RSS is only for virtio-net + Linux TAP. Checking F_OK (existence) instea of X_OK is odd. > Libvirt launches qemu to get different properties. That qemu may not have permission to launch the helper. It uses /proc/self/exe to find the running executable's directory. This > is specific to Linux[*]. You get different behavior on Linux vs. other > systems. > The code guarded by CONFIG_LINUX. * If the host isn't Linux, it returns /usr/libexec/qemu-ebpf-rss-helper. > Not good. > No, "query-helper-paths" will return an empty list. * If Alice runs bld/x86_64-softmmu/qemu-system-x86_64, it also returns > /usr/libexec/qemu-ebpf-rss-helper. Not good. > No, /proc/self/exe dereferences "bld/x86_64-softmmu/qemu-system-x86_64" to "bld/qemu-system-x86_64" and we will get bld/qemu-ebpf-rss-helper. The name query-helper-paths is generic, the documented purpose "Query > specific eBPF RSS helper" is specific. > > qemu-ebpf-rss-helper isn't necessarily the only helper that needs to be > in sync with QEMU. > Yea, I'll update the document. If we want to ensure the right helper runs, we should use a build > identifier compiled into the programs, like we do for modules. > Thanks, I'll check. Overall, current idea was to avoid the use of the helper from CONFIG_QEMU_HELPERDIR if qemu is not installed(like in your examples). Helpers QEMU code runs itself should be run as > CONFIG_QEMU_HELPERDIR/HELPER, with a suitable user override. This is > how qemu-bridge-helper works. > > Helpers some other program runs are that other program's problem. > They'll probably work the same: built-in default that can be overridden > with configuration. > Well, for qemu it does not really matter how TAP fd was created. It can be the helper, Libvirt itself, or a script. In the end, "netdev" gets its fds and for qemu there is no difference. TAP fd is TAP fd. And Libvirt would use the same qemu-bridge-helper(from libvirt/qemu.conf) for every qemu "emulator". For eBPF we need to create specific maps(and/or thair quantities) that contain specific structures and for different qemu it may be different. On Sat, Jun 12, 2021 at 8:28 AM Markus Armbruster wrote: > Andrew Melnychenko writes: > > > 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 > > --- > > 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 "."; > > +} > > + > > +static char *find_helper(const char *name) > > +{ > > + char qemu_exec[PATH_MAX]; > > + 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); > > + if (access(helper, F_OK) == 0) { > > + return helper; > > + } > > + g_free(helper); > > + > > + return NULL; > > +} > > This returns the helper in the same directory as the running executable, > or as a fallback the helper in CONFIG_QEMU_HELPERDIR. > > Checking F_OK (existence) instea of X_OK is odd. > > It uses /proc/self/exe to find the running executable's directory. This > is specific to Linux[*]. You get different behavior on Linux vs. other > systems. > > CONFIG_QEMU_HELPERDIR is $prefix/libexec/. > > If $prefix is /usr, then qemu-system-FOO is normally installed in > /usr/bin/, and the helper in /usr/libexec/. We look for the helper in > the wrong place first, and the right one only when it isn't in the wrong > place. Feels overcomplicated and fragile. > > Consider the following scenario: > > * The system has a binary package's /usr/bin/qemu-system-x86_64 and > /usr/libexec/qemu-ebpf-rss-helper installed > > * Alice builds her own QEMU with prefix /usr (and no intention to > install), resulting in bld/qemu-system-x86_64, bld/qemu-ebpf-rss-path, > and a symlink bld/x86_64-softmmu/qemu-system-x86_64. > > Now: > > * If Alice runs bld/qemu-system-x86_64, and the host is Linux, > find_helper() returns bld/qemu-ebpf-rss-path. Good. > > * If the host isn't Linux, it returns /usr/libexec/qemu-ebpf-rss-helper. > Not good. > > * If Alice runs bld/x86_64-softmmu/qemu-system-x86_64, it also returns > /usr/libexec/qemu-ebpf-rss-helper. Not good. > > > + > > +HelperPathList *qmp_query_helper_paths(Error **errp) > > +{ > > + HelperPathList *ret = NULL; > > + const char *helpers_list[] = { > > +#ifdef CONFIG_EBPF > > + "qemu-ebpf-rss-helper", > > +#endif > > + NULL > > + }; > > + const char **helper_iter = helpers_list; > > + > > + for (; *helper_iter != NULL; ++helper_iter) { > > + char *path = find_helper(*helper_iter); > > + if (path) { > > + HelperPath *helper = g_new0(HelperPath, 1); > > + helper->name = g_strdup(*helper_iter); > > + helper->path = path; > > + > > + QAPI_LIST_PREPEND(ret, helper); > > + } > > + } > > + > > + return ret; > > +} > > +#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. > > +## > > +{ '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'] } > > The name query-helper-paths is generic, the documented purpose "Query > specific eBPF RSS helper" is specific. > > qemu-ebpf-rss-helper isn't necessarily the only helper that needs to be > in sync with QEMU. > > I doubt a query command is a good way to help with using the right one. > qemu-system-FOO doesn't really know where the right one is. Only the > person or program that put them where they are does. > > If we want to ensure the right helper runs, we should use a build > identifier compiled into the programs, like we do for modules. > > For modules, the program loading a module checks the module's build > identifier matches its own. > > For programs talking to each other, the peers together check their build > identifiers match. > > For programs where that isn't practical, the management application can > check. > > This should be a lot more reliable. > > Helpers QEMU code runs itself should be run as > CONFIG_QEMU_HELPERDIR/HELPER, with a suitable user override. This is > how qemu-bridge-helper works. > > Helpers some other program runs are that other program's problem. > They'll probably work the same: built-in default that can be overridden > with configuration. > > > [*] For detailed advice, see > > https://stackoverflow.com/questions/1023306/finding-current-executables-path-without-proc-self-exe > > --000000000000605c7905c4d62af7 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi all,
Seems like this function is duplicating what glib should al= ready be
able to do.
Yea,= but it's required a Linux specific header - without it, qemu builds bu= t crashes.

Could we use a compile-time determination of where we were (supp= osed)
to be installed, and therefore where our helper should be installed,
rather than the dynamic /proc/self/exe munging?
Yes, we can define something like CONF= IG_QEMU_HELPERDIR ## "qemu-ebpf-rss-helper", for RSS helper.
But I've tried to implement generic function for possible other h= elpers.

Yeah I think avoiding /proc/self/exe is desirable, because = I can
imagine scenarios where this can lead to picking the wrong helper.
Better to always use the compile time install directory.
The main scenario that find_helper() should solve - non installed= qemu use helper from own build.
That's why reading /proc/sel= f/exe is implemented.

So the intent is that we can make this list large= r if we write other
helper binaries.=C2=A0 But this code is in an overall #ifdef CONFIG_LINUX,<= br> which means it won't work on other platforms.
Ye= s, for now, eBPF RSS is only for virtio-net=C2=A0+ Linux TAP.
Checking F_OK= (existence) instea of X_OK is odd.
Libvirt launches= qemu to get different properties. That qemu may not have permission to lau= nch the helper.

It uses /proc/self/exe to find the running executable's= directory.=C2=A0 This
is specific to Linux[*].=C2=A0 You get different behavior on Linux vs. othe= r
systems.
The code guarded by CONFIG_LINUX.

* If the = host isn't Linux, it returns /usr/libexec/qemu-ebpf-rss-helper.
=C2=A0 Not good.
No,=C2=A0 = "query-helper-paths" will return an empty list.
=
* If Alice runs bld/x86_64-s= oftmmu/qemu-system-x86_64, it also returns
=C2=A0 /usr/libexec/qemu-ebpf-rss-helper.= =C2=A0 Not good.
No, /proc/self/exe dereferences "bld/x86_64-softmmu/qemu-system-x86_64" to "= ;bld/qemu-system-x86_64"
a= nd we will get bld/qemu-ebpf-rss-helper.
=C2=A0The nam= e query-helper-paths is generic, the documented purpose "Query
specific eBPF RSS helper" is specific.

qemu-ebpf-rss-helper isn't necessarily the only helper that needs to be=
in sync with QEMU.
Yea, I'll update the document= .

If we want to ensure the right helper runs, we should use a build
identifier compiled into the programs, like we do for modules.
Thanks, I'll check. Overall, current idea was t= o avoid the use of the helper
from CONFIG_QEMU_HELPERDIR if qemu is not installed(like in your example= s).

Helpers QEMU code runs itself should be run as=
CONFIG_QEMU_HELPERDIR/HELPER, with a suitable user override.=C2=A0 This is<= br> how qemu-bridge-helper works.

Helpers some other program runs are t= hat other program's problem.
They'll probably work the same: built-in default that can be overridden=
with configuration.
Well, for qemu i= t does not really matter how TAP fd was created. It can be the helper, Libv= irt itself, or a script.
In the end, "netdev" gets its= fds and for qemu there is no difference. TAP fd is TAP fd.
And L= ibvirt would use the same qemu-bridge-helper(from libvirt/qemu.conf) for ev= ery qemu "emulator".
For eBPF we need to create spe= cific maps(and/or thair quantities) that contain specific structures and fo= r different
qemu it may be different.
=C2=A0
<= /div>


On Sat, Jun 12, 2021 at 8:28 AM Markus Armbruster= <armbru@redhat.c= om> wrote:
andrew@daynix.com> writes:

> 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 h= elpers.
> Qemu returns helper that should "fit" to virtio-net.
>
> Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
> ---
>=C2=A0 monitor/qmp-cmds.c | 78 ++++++++++++++++++++++++++++++++++++++++= ++++++
>=C2=A0 qapi/misc.json=C2=A0 =C2=A0 =C2=A0| 29 +++++++++++++++++
>=C2=A0 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)
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 abort();
>=C2=A0 =C2=A0 =C2=A0 }
>=C2=A0 }
> +
> +#ifdef CONFIG_LINUX
> +
> +static const char *get_dirname(char *path)
> +{
> +=C2=A0 =C2=A0 char *sep;
> +
> +=C2=A0 =C2=A0 sep =3D strrchr(path, '/');
> +=C2=A0 =C2=A0 if (sep =3D=3D path) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return "/";
> +=C2=A0 =C2=A0 } else if (sep) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 *sep =3D 0;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return path;
> +=C2=A0 =C2=A0 }
> +=C2=A0 =C2=A0 return ".";
> +}
> +
> +static char *find_helper(const char *name)
> +{
> +=C2=A0 =C2=A0 char qemu_exec[PATH_MAX];
> +=C2=A0 =C2=A0 const char *qemu_dir =3D NULL;
> +=C2=A0 =C2=A0 char *helper =3D NULL;
> +
> +=C2=A0 =C2=A0 if (name =3D=3D NULL) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return NULL;
> +=C2=A0 =C2=A0 }
> +
> +=C2=A0 =C2=A0 if (readlink("/proc/self/exe", qemu_exec, PAT= H_MAX) > 0) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 qemu_dir =3D get_dirname(qemu_exec);
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 helper =3D g_strdup_printf("%s/%s&qu= ot;, qemu_dir, name);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (access(helper, F_OK) =3D=3D 0) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return helper;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 }
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 g_free(helper);
> +=C2=A0 =C2=A0 }
> +
> +=C2=A0 =C2=A0 helper =3D g_strdup_printf("%s/%s", CONFIG_QE= MU_HELPERDIR, name);
> +=C2=A0 =C2=A0 if (access(helper, F_OK) =3D=3D 0) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return helper;
> +=C2=A0 =C2=A0 }
> +=C2=A0 =C2=A0 g_free(helper);
> +
> +=C2=A0 =C2=A0 return NULL;
> +}

This returns the helper in the same directory as the running executable, or as a fallback the helper in CONFIG_QEMU_HELPERDIR.

Checking F_OK (existence) instea of X_OK is odd.

It uses /proc/self/exe to find the running executable's directory.=C2= =A0 This
is specific to Linux[*].=C2=A0 You get different behavior on Linux vs. othe= r
systems.

CONFIG_QEMU_HELPERDIR is $prefix/libexec/.

If $prefix is /usr, then qemu-system-FOO is normally installed in
/usr/bin/, and the helper in /usr/libexec/.=C2=A0 We look for the helper in=
the wrong place first, and the right one only when it isn't in the wron= g
place.=C2=A0 Feels overcomplicated and fragile.

Consider the following scenario:

* The system has a binary package's /usr/bin/qemu-system-x86_64 and
=C2=A0 /usr/libexec/qemu-ebpf-rss-helper installed

* Alice builds her own QEMU with prefix /usr (and no intention to
=C2=A0 install), resulting in bld/qemu-system-x86_64, bld/qemu-ebpf-rss-pat= h,
=C2=A0 and a symlink bld/x86_64-softmmu/qemu-system-x86_64.

Now:

* If Alice runs bld/qemu-system-x86_64, and the host is Linux,
=C2=A0 find_helper() returns bld/qemu-ebpf-rss-path.=C2=A0 Good.

* If the host isn't Linux, it returns /usr/libexec/qemu-ebpf-rss-helper= .
=C2=A0 Not good.

* If Alice runs bld/x86_64-softmmu/qemu-system-x86_64, it also returns
=C2=A0 /usr/libexec/qemu-ebpf-rss-helper.=C2=A0 Not good.

> +
> +HelperPathList *qmp_query_helper_paths(Error **errp)
> +{
> +=C2=A0 =C2=A0 HelperPathList *ret =3D NULL;
> +=C2=A0 =C2=A0 const char *helpers_list[] =3D {
> +#ifdef CONFIG_EBPF
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 "qemu-ebpf-rss-helper",
> +#endif
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 NULL
> +=C2=A0 =C2=A0 };
> +=C2=A0 =C2=A0 const char **helper_iter =3D helpers_list;
> +
> +=C2=A0 =C2=A0 for (; *helper_iter !=3D NULL; ++helper_iter) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 char *path =3D find_helper(*helper_iter);=
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (path) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 HelperPath *helper =3D g_ne= w0(HelperPath, 1);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 helper->name =3D g_strdu= p(*helper_iter);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 helper->path =3D path; > +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 QAPI_LIST_PREPEND(ret, help= er);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 }
> +=C2=A0 =C2=A0 }
> +
> +=C2=A0 =C2=A0 return ret;
> +}
> +#else
> +
> +HelperPathList *qmp_query_helper_paths(Error **errp)
> +{
> +=C2=A0 =C2=A0 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 @@
>=C2=A0 =C2=A0'data': { '*option': 'str' },
>=C2=A0 =C2=A0'returns': ['CommandLineOptionInfo'],
>=C2=A0 =C2=A0'allow-preconfig': true }
> +
> +##
> +# @HelperPath:
> +#
> +# Name of the helper and binary location.
> +##
> +{ 'struct': 'HelperPath',
> +=C2=A0 '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": [
> +#=C2=A0 =C2=A0 =C2=A0 =C2=A0 {
> +#=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 "name": "qemu-ebpf= -rss-helper",
> +#=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 "path": "/usr/loca= l/libexec/qemu-ebpf-rss-helper"
> +#=C2=A0 =C2=A0 =C2=A0 =C2=A0 }
> +#=C2=A0 =C2=A0 =C2=A0 ]
> +#=C2=A0 =C2=A0 }
> +#
> +##
> +{ 'command': 'query-helper-paths', 'returns':= ['HelperPath'] }

The name query-helper-paths is generic, the documented purpose "Query<= br> specific eBPF RSS helper" is specific.

qemu-ebpf-rss-helper isn't necessarily the only helper that needs to be=
in sync with QEMU.

I doubt a query command is a good way to help with using the right one.
qemu-system-FOO doesn't really know where the right one is.=C2=A0 Only = the
person or program that put them where they are does.

If we want to ensure the right helper runs, we should use a build
identifier compiled into the programs, like we do for modules.

For modules, the program loading a module checks the module's build
identifier matches its own.

For programs talking to each other, the peers together check their build identifiers match.

For programs where that isn't practical, the management application can=
check.

This should be a lot more reliable.

Helpers QEMU code runs itself should be run as
CONFIG_QEMU_HELPERDIR/HELPER, with a suitable user override.=C2=A0 This is<= br> how qemu-bridge-helper works.

Helpers some other program runs are that other program's problem.
They'll probably work the same: built-in default that can be overridden=
with configuration.


[*] For detailed advice, see
ht= tps://stackoverflow.com/questions/1023306/finding-current-executables-path-= without-proc-self-exe

--000000000000605c7905c4d62af7--