[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC PATCH 4/5] qmp: Added qemu-ebpf-rss-path command.
From: |
Daniel P . Berrangé |
Subject: |
Re: [RFC PATCH 4/5] qmp: Added qemu-ebpf-rss-path command. |
Date: |
Fri, 11 Jun 2021 18:21:55 +0100 |
User-agent: |
Mutt/2.0.7 (2021-05-04) |
On Fri, Jun 11, 2021 at 09:15:52AM -0500, Eric Blake wrote:
> 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?
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.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
- [RFC PATCH 0/5] ebpf: Added ebpf helper for libvirtd., Andrew Melnychenko, 2021/06/09
- [RFC PATCH 1/5] ebpf: Added eBPF initialization by fds., Andrew Melnychenko, 2021/06/09
- [RFC PATCH 2/5] virtio-net: Added property to load eBPF RSS with fds., Andrew Melnychenko, 2021/06/09
- [RFC PATCH 3/5] ebpf_rss_helper: Added helper for eBPF RSS., Andrew Melnychenko, 2021/06/09
- [RFC PATCH 4/5] qmp: Added qemu-ebpf-rss-path command., Andrew Melnychenko, 2021/06/09
- [RFC PATCH 5/5] meson: libbpf dependency now exclusively for Linux., Andrew Melnychenko, 2021/06/09
- Re: [RFC PATCH 0/5] ebpf: Added ebpf helper for libvirtd., Jason Wang, 2021/06/10
- Re: [RFC PATCH 0/5] ebpf: Added ebpf helper for libvirtd., Yuri Benditovich, 2021/06/10
- Re: [RFC PATCH 0/5] ebpf: Added ebpf helper for libvirtd., Jason Wang, 2021/06/11
- Re: [RFC PATCH 0/5] ebpf: Added ebpf helper for libvirtd., Andrew Melnichenko, 2021/06/11
- Re: [RFC PATCH 0/5] ebpf: Added ebpf helper for libvirtd., Daniel P . Berrangé, 2021/06/11
- Re: [RFC PATCH 0/5] ebpf: Added ebpf helper for libvirtd., Jason Wang, 2021/06/15
- Re: [RFC PATCH 0/5] ebpf: Added ebpf helper for libvirtd., Andrew Melnichenko, 2021/06/15