[v7,4/5] qmp: Added new command to retrieve eBPF blob.

Now, the binary objects may be retrieved by id.
It would require for future qmp commands that may require specific
eBPF blob.

Added command "request-ebpf". This command returns
eBPF program encoded base64. The program taken from the
skeleton and essentially is an ELF object that can be
loaded in the future with libbpf.

The reason to use the command to provide the eBPF object
instead of a separate artifact was to avoid issues related
to finding the eBPF itself. eBPF object is an ELF binary
that contains the eBPF program and eBPF map description(BTF).
Overall, eBPF object should contain the program and enough
metadata to create/load eBPF with libbpf. As the eBPF
maps/program should correspond to QEMU, the eBPF can't
be used from different QEMU build.

The first solution was a helper that comes with QEMU
and loads appropriate eBPF objects. And the issue is
to find a proper helper if the system has several
different QEMUs installed and/or built from the source,
which helpers may not be compatible.

Another issue is QEMU updating while there is a running
QEMU instance. With an updated helper, it may not be
possible to hotplug virtio-net device to the already
running QEMU. Overall, requesting the eBPF object from
QEMU itself solves possible failures with acceptable effort.

Links:
[PATCH 3/5] qmp: Added the helper stamp check.
https://lore.kernel.org/all/20230219162100.174318-4-andrew@daynix.com/

Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
---
 ebpf/ebpf.c           | 70 +++++++++++++++++++++++++++++++++++++++++++
 ebpf/ebpf.h           | 31 +++++++++++++++++++
 ebpf/ebpf_rss.c       |  6 ++++
 ebpf/meson.build      |  2 +-
 qapi/ebpf.json        | 66 ++++++++++++++++++++++++++++++++++++++++
 qapi/meson.build      |  1 +
 qapi/qapi-schema.json |  1 +
 7 files changed, 176 insertions(+), 1 deletion(-)
 create mode 100644 ebpf/ebpf.c
 create mode 100644 ebpf/ebpf.h
 create mode 100644 qapi/ebpf.json

On 2023/08/31 15:51, Andrew Melnychenko wrote:
> Now, the binary objects may be retrieved by id.
> It would require for future qmp commands that may require specific
> eBPF blob.
> 
> Added command "request-ebpf". This command returns
> eBPF program encoded base64. The program taken from the
> skeleton and essentially is an ELF object that can be
> loaded in the future with libbpf.
> 
> The reason to use the command to provide the eBPF object
> instead of a separate artifact was to avoid issues related
> to finding the eBPF itself. eBPF object is an ELF binary
> that contains the eBPF program and eBPF map description(BTF).
> Overall, eBPF object should contain the program and enough
> metadata to create/load eBPF with libbpf. As the eBPF
> maps/program should correspond to QEMU, the eBPF can't
> be used from different QEMU build.
> 
> The first solution was a helper that comes with QEMU
> and loads appropriate eBPF objects. And the issue is
> to find a proper helper if the system has several
> different QEMUs installed and/or built from the source,
> which helpers may not be compatible.
> 
> Another issue is QEMU updating while there is a running
> QEMU instance. With an updated helper, it may not be
> possible to hotplug virtio-net device to the already
> running QEMU. Overall, requesting the eBPF object from
> QEMU itself solves possible failures with acceptable effort.
> 
> Links:
> [PATCH 3/5] qmp: Added the helper stamp check.
> https://lore.kernel.org/all/20230219162100.174318-4-andrew@daynix.com/
> 
> Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
> ---
>   ebpf/ebpf.c           | 70 +++++++++++++++++++++++++++++++++++++++++++
>   ebpf/ebpf.h           | 31 +++++++++++++++++++
>   ebpf/ebpf_rss.c       |  6 ++++
>   ebpf/meson.build      |  2 +-
>   qapi/ebpf.json        | 66 ++++++++++++++++++++++++++++++++++++++++
>   qapi/meson.build      |  1 +
>   qapi/qapi-schema.json |  1 +
>   7 files changed, 176 insertions(+), 1 deletion(-)
>   create mode 100644 ebpf/ebpf.c
>   create mode 100644 ebpf/ebpf.h
>   create mode 100644 qapi/ebpf.json
> 
> diff --git a/ebpf/ebpf.c b/ebpf/ebpf.c
> new file mode 100644
> index 0000000000..ea97c0403e
> --- /dev/null
> +++ b/ebpf/ebpf.c
> @@ -0,0 +1,70 @@
> +/*
> + * QEMU eBPF binary declaration routine.
> + *
> + * Developed by Daynix Computing LTD (http://www.daynix.com)
> + *
> + * Authors:
> + *  Andrew Melnychenko <andrew@daynix.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or
> + * later.  See the COPYING file in the top-level directory.
> + */

Use SPDX License Identifier. I had been asked the same thing but 
unfortunately it's not documented in docs/devel/style.rst.

> +
> +#include "qemu/osdep.h"
> +#include "qemu/queue.h"
> +#include "qapi/error.h"
> +#include "qapi/qapi-commands-ebpf.h"
> +#include "ebpf/ebpf.h"
> +
> +struct ElfBinaryDataEntry {
> +    int id;
> +    const void *data;
> +    size_t datalen;
> +
> +    QSLIST_ENTRY(ElfBinaryDataEntry) node;
> +};

Add typedef. See docs/devel/style.rst.

> +
> +static QSLIST_HEAD(, ElfBinaryDataEntry) ebpf_elf_obj_list =
> +                                            QSLIST_HEAD_INITIALIZER();

Why don't you use GHashTable?

> +
> +void ebpf_register_binary_data(int id, const void *data, size_t datalen)
> +{
> +    struct ElfBinaryDataEntry *dataentry = NULL;
> +
> +    dataentry = g_new0(struct ElfBinaryDataEntry, 1);
> +    dataentry->data = data;
> +    dataentry->datalen = datalen;
> +    dataentry->id = id;
> +
> +    QSLIST_INSERT_HEAD(&ebpf_elf_obj_list, dataentry, node);
> +}
> +
> +const void *ebpf_find_binary_by_id(int id, size_t *sz, Error **errp)
> +{
> +    struct ElfBinaryDataEntry *it = NULL;
> +    QSLIST_FOREACH(it, &ebpf_elf_obj_list, node) {
> +        if (id == it->id) {
> +            *sz = it->datalen;
> +            return it->data;
> +        }
> +    }
> +
> +    error_setg(errp, "can't find eBPF object with id: %d", id);
> +
> +    return NULL;
> +}
> +
> +EbpfObject *qmp_request_ebpf(EbpfProgramID id, Error **errp)
> +{
> +    EbpfObject *ret = NULL;
> +    size_t size = 0;
> +    const void *data = ebpf_find_binary_by_id(id, &size, errp);
> +    if (!data) {
> +        return NULL;
> +    }
> +
> +    ret = g_new0(EbpfObject, 1);
> +    ret->object = g_base64_encode(data, size);
> +
> +    return ret;
> +}
> diff --git a/ebpf/ebpf.h b/ebpf/ebpf.h
> new file mode 100644
> index 0000000000..b6266b28b8
> --- /dev/null
> +++ b/ebpf/ebpf.h
> @@ -0,0 +1,31 @@
> +/*
> + * QEMU eBPF binary declaration routine.
> + *
> + * Developed by Daynix Computing LTD (http://www.daynix.com)
> + *
> + * Authors:
> + *  Andrew Melnychenko <andrew@daynix.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or
> + * later.  See the COPYING file in the top-level directory.
> + */
> +
> +#ifndef EBPF_H
> +#define EBPF_H
> +
> +struct Error;

Why do you need this?

> +
> +void ebpf_register_binary_data(int id, const void *data,
> +                               size_t datalen);
> +const void *ebpf_find_binary_by_id(int id, size_t *sz,
> +                                   struct Error **errp);
> +
> +#define ebpf_binary_init(id, fn)                                           \
> +static void __attribute__((constructor)) ebpf_binary_init_ ## fn(void)     \
> +{                                                                          \
> +    size_t datalen = 0;                                                    \
> +    const void *data = fn(&datalen);                                       \
> +    ebpf_register_binary_data(id, data, datalen);                          \
> +}
> +
> +#endif /* EBPF_H */
> diff --git a/ebpf/ebpf_rss.c b/ebpf/ebpf_rss.c
> index 24bc6cc409..8679dc452d 100644
> --- a/ebpf/ebpf_rss.c
> +++ b/ebpf/ebpf_rss.c
> @@ -13,6 +13,8 @@
>   
>   #include "qemu/osdep.h"
>   #include "qemu/error-report.h"
> +#include "qapi/qapi-types-misc.h"
> +#include "qapi/qapi-commands-ebpf.h"
>   
>   #include <bpf/libbpf.h>
>   #include <bpf/bpf.h>
> @@ -21,6 +23,8 @@
>   
>   #include "ebpf/ebpf_rss.h"
>   #include "ebpf/rss.bpf.skeleton.h"
> +#include "ebpf/ebpf.h"
> +
>   #include "trace.h"
>   
>   void ebpf_rss_init(struct EBPFRSSContext *ctx)
> @@ -261,3 +265,5 @@ void ebpf_rss_unload(struct EBPFRSSContext *ctx)
>       ctx->map_toeplitz_key = -1;
>       ctx->map_indirections_table = -1;
>   }
> +
> +ebpf_binary_init(EBPF_PROGRAMID_RSS, rss_bpf__elf_bytes)
> diff --git a/ebpf/meson.build b/ebpf/meson.build
> index 2f627d6c7d..c5bf9295a2 100644
> --- a/ebpf/meson.build
> +++ b/ebpf/meson.build
> @@ -1 +1 @@
> -system_ss.add(when: libbpf, if_true: files('ebpf_rss.c'), if_false: files('ebpf_rss-stub.c'))
> +common_ss.add(when: libbpf, if_true: files('ebpf.c', 'ebpf_rss.c'), if_false: files('ebpf_rss-stub.c'))
> diff --git a/qapi/ebpf.json b/qapi/ebpf.json
> new file mode 100644
> index 0000000000..ba78407148
> --- /dev/null
> +++ b/qapi/ebpf.json
> @@ -0,0 +1,66 @@
> +# -*- Mode: Python -*-
> +# vim: filetype=python
> +#
> +# This work is licensed under the terms of the GNU GPL, version 2 or later.
> +# See the COPYING file in the top-level directory.
> +
> +##
> +# = eBPF Objects
> +#
> +# eBPF object is an ELF binary that contains the eBPF
> +# program and eBPF map description(BTF). Overall, eBPF
> +# object should contain the program and enough metadata
> +# to create/load eBPF with libbpf. As the eBPF maps/program
> +# should correspond to QEMU, the eBPF can't be used from
> +# different QEMU build.
> +#
> +# Currently, there is a possible eBPF for receive-side scaling (RSS).
> +#
> +##
> +
> +##
> +# @EbpfObject:
> +#
> +# An eBPF ELF object.
> +#
> +# @object: the eBPF object encoded in base64
> +#
> +# Since: 8.2

These need to be updated; we missed 8.2.

Uh, I missed this one, my apologies!  Let me have a quick look...

Commit message style nitpick:

    qmp: Add command to retrieve eBPF blob

or

    qmp: New command to retrieve eBPF blob

It's a title, not a sentence.

Andrew Melnychenko <andrew@daynix.com> writes:

> Now, the binary objects may be retrieved by id.
> It would require for future qmp commands that may require specific
> eBPF blob.
>
> Added command "request-ebpf". This command returns
> eBPF program encoded base64. The program taken from the
> skeleton and essentially is an ELF object that can be
> loaded in the future with libbpf.
>
> The reason to use the command to provide the eBPF object
> instead of a separate artifact was to avoid issues related
> to finding the eBPF itself. eBPF object is an ELF binary
> that contains the eBPF program and eBPF map description(BTF).
> Overall, eBPF object should contain the program and enough
> metadata to create/load eBPF with libbpf. As the eBPF
> maps/program should correspond to QEMU, the eBPF can't
> be used from different QEMU build.
>
> The first solution was a helper that comes with QEMU
> and loads appropriate eBPF objects. And the issue is
> to find a proper helper if the system has several
> different QEMUs installed and/or built from the source,
> which helpers may not be compatible.
>
> Another issue is QEMU updating while there is a running
> QEMU instance. With an updated helper, it may not be
> possible to hotplug virtio-net device to the already
> running QEMU. Overall, requesting the eBPF object from
> QEMU itself solves possible failures with acceptable effort.
>
> Links:
> [PATCH 3/5] qmp: Added the helper stamp check.
> https://lore.kernel.org/all/20230219162100.174318-4-andrew@daynix.com/
>
> Signed-off-by: Andrew Melnychenko <andrew@daynix.com>

[...]

> diff --git a/qapi/ebpf.json b/qapi/ebpf.json
> new file mode 100644
> index 0000000000..ba78407148
> --- /dev/null
> +++ b/qapi/ebpf.json
> @@ -0,0 +1,66 @@
> +# -*- Mode: Python -*-
> +# vim: filetype=python
> +#
> +# This work is licensed under the terms of the GNU GPL, version 2 or later.
> +# See the COPYING file in the top-level directory.
> +
> +##
> +# = eBPF Objects
> +#
> +# eBPF object is an ELF binary that contains the eBPF
> +# program and eBPF map description(BTF). Overall, eBPF
> +# object should contain the program and enough metadata
> +# to create/load eBPF with libbpf. As the eBPF maps/program
> +# should correspond to QEMU, the eBPF can't be used from
> +# different QEMU build.

An introduction, nice!

Two spaces between sentences for consistency, please.

Wrap comment lines at column 70, like this:

   # eBPF object is an ELF binary that contains the eBPF program and eBPF
   # map description(BTF).  Overall, eBPF object should contain the
   # program and enough metadata to create/load eBPF with libbpf.  As the
   # eBPF maps/program should correspond to QEMU, the eBPF can't be used
   # from different QEMU build.

> +#
> +# Currently, there is a possible eBPF for receive-side scaling (RSS).
> +#
> +##
> +
> +##
> +# @EbpfObject:
> +#
> +# An eBPF ELF object.
> +#
> +# @object: the eBPF object encoded in base64
> +#
> +# Since: 8.2

Won't make 8.2, so bump to 9.0.  More of the same below, not noting it
again.

> +##
> +{ 'struct': 'EbpfObject',
> +  'data': {'object': 'str'},
> +  'if': 'CONFIG_EBPF' }
> +
> +##
> +# @EbpfProgramID:
> +#
> +# The eBPF programs that can be gotten with request-ebpf.
> +#
> +# @rss: Receive side scaling, technology that allows steering traffic
> +#     between queues by calculation hash.  Users may set up
> +#     indirection table and hash/packet types configurations.  Used
> +#     with virtio-net.
> +#
> +# Since: 8.2
> +##
> +{ 'enum': 'EbpfProgramID',
> +  'if': 'CONFIG_EBPF',
> +  'data': [ { 'name': 'rss' } ] }
> +
> +##
> +# @request-ebpf:
> +#
> +# Retrieve an eBPF object that can be loaded with libbpf.  Management
> +# applications (g.e. libvirt) may load it and pass file descriptors to
> +# QEMU, so they can run running QEMU without BPF capabilities.
> +#
> +# @id: The ID of the program to return.
> +#
> +# Returns: eBPF object encoded in base64.
> +#
> +# Since: 8.2
> +##
> +{ 'command': 'request-ebpf',
> +  'data': { 'id': 'EbpfProgramID' },
> +  'returns': 'EbpfObject',
> +  'if': 'CONFIG_EBPF' }
> diff --git a/qapi/meson.build b/qapi/meson.build
> index 60a668b343..90047dae1c 100644
> --- a/qapi/meson.build
> +++ b/qapi/meson.build
> @@ -33,6 +33,7 @@ qapi_all_modules = [
>    'crypto',
>    'cxl',
>    'dump',
> +  'ebpf',
>    'error',
>    'introspect',
>    'job',
> diff --git a/qapi/qapi-schema.json b/qapi/qapi-schema.json
> index 6594afba31..2c82a49bae 100644
> --- a/qapi/qapi-schema.json
> +++ b/qapi/qapi-schema.json
> @@ -53,6 +53,7 @@
>  { 'include': 'char.json' }
>  { 'include': 'dump.json' }
>  { 'include': 'net.json' }
> +{ 'include': 'ebpf.json' }
>  { 'include': 'rdma.json' }
>  { 'include': 'rocker.json' }
>  { 'include': 'tpm.json' }

With the (minor) doc comment fixes:
Acked-by: Markus Armbruster <armbru@redhat.com>

On Mon, Dec 18, 2023 at 2:54 PM Markus Armbruster <armbru@redhat.com> wrote:

> Uh, I missed this one, my apologies!  Let me have a quick look...
>
> Commit message style nitpick:
>
>     qmp: Add command to retrieve eBPF blob
>
> or
>
>     qmp: New command to retrieve eBPF blob
>
> It's a title, not a sentence.
>
> Andrew Melnychenko <andrew@daynix.com> writes:
>
> > Now, the binary objects may be retrieved by id.
> > It would require for future qmp commands that may require specific
> > eBPF blob.
> >
> > Added command "request-ebpf". This command returns
> > eBPF program encoded base64. The program taken from the
> > skeleton and essentially is an ELF object that can be
> > loaded in the future with libbpf.
> >
> > The reason to use the command to provide the eBPF object
> > instead of a separate artifact was to avoid issues related
> > to finding the eBPF itself. eBPF object is an ELF binary
> > that contains the eBPF program and eBPF map description(BTF).
> > Overall, eBPF object should contain the program and enough
> > metadata to create/load eBPF with libbpf. As the eBPF
> > maps/program should correspond to QEMU, the eBPF can't
> > be used from different QEMU build.
> >
> > The first solution was a helper that comes with QEMU
> > and loads appropriate eBPF objects. And the issue is
> > to find a proper helper if the system has several
> > different QEMUs installed and/or built from the source,
> > which helpers may not be compatible.
> >
> > Another issue is QEMU updating while there is a running
> > QEMU instance. With an updated helper, it may not be
> > possible to hotplug virtio-net device to the already
> > running QEMU. Overall, requesting the eBPF object from
> > QEMU itself solves possible failures with acceptable effort.
> >
> > Links:
> > [PATCH 3/5] qmp: Added the helper stamp check.
> > https://lore.kernel.org/all/20230219162100.174318-4-andrew@daynix.com/
> >
> > Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
>
> [...]
>
> > diff --git a/qapi/ebpf.json b/qapi/ebpf.json
> > new file mode 100644
> > index 0000000000..ba78407148
> > --- /dev/null
> > +++ b/qapi/ebpf.json
> > @@ -0,0 +1,66 @@
> > +# -*- Mode: Python -*-
> > +# vim: filetype=python
> > +#
> > +# This work is licensed under the terms of the GNU GPL, version 2 or
> later.
> > +# See the COPYING file in the top-level directory.
> > +
> > +##
> > +# = eBPF Objects
> > +#
> > +# eBPF object is an ELF binary that contains the eBPF
> > +# program and eBPF map description(BTF). Overall, eBPF
> > +# object should contain the program and enough metadata
> > +# to create/load eBPF with libbpf. As the eBPF maps/program
> > +# should correspond to QEMU, the eBPF can't be used from
> > +# different QEMU build.
>
> An introduction, nice!
>
> Two spaces between sentences for consistency, please.
>
> Wrap comment lines at column 70, like this:
>
>    # eBPF object is an ELF binary that contains the eBPF program and eBPF
>    # map description(BTF).  Overall, eBPF object should contain the
>    # program and enough metadata to create/load eBPF with libbpf.  As the
>    # eBPF maps/program should correspond to QEMU, the eBPF can't be used
>    # from different QEMU build.
>
> > +#
> > +# Currently, there is a possible eBPF for receive-side scaling (RSS).
> > +#
> > +##
> > +
> > +##
> > +# @EbpfObject:
> > +#
> > +# An eBPF ELF object.
> > +#
> > +# @object: the eBPF object encoded in base64
> > +#
> > +# Since: 8.2
>
> Won't make 8.2, so bump to 9.0.  More of the same below, not noting it
> again.
>

Will there be 8.3?


>
> > +##
> > +{ 'struct': 'EbpfObject',
> > +  'data': {'object': 'str'},
> > +  'if': 'CONFIG_EBPF' }
> > +
> > +##
> > +# @EbpfProgramID:
> > +#
> > +# The eBPF programs that can be gotten with request-ebpf.
> > +#
> > +# @rss: Receive side scaling, technology that allows steering traffic
> > +#     between queues by calculation hash.  Users may set up
> > +#     indirection table and hash/packet types configurations.  Used
> > +#     with virtio-net.
> > +#
> > +# Since: 8.2
> > +##
> > +{ 'enum': 'EbpfProgramID',
> > +  'if': 'CONFIG_EBPF',
> > +  'data': [ { 'name': 'rss' } ] }
> > +
> > +##
> > +# @request-ebpf:
> > +#
> > +# Retrieve an eBPF object that can be loaded with libbpf.  Management
> > +# applications (g.e. libvirt) may load it and pass file descriptors to
> > +# QEMU, so they can run running QEMU without BPF capabilities.
> > +#
> > +# @id: The ID of the program to return.
> > +#
> > +# Returns: eBPF object encoded in base64.
> > +#
> > +# Since: 8.2
> > +##
> > +{ 'command': 'request-ebpf',
> > +  'data': { 'id': 'EbpfProgramID' },
> > +  'returns': 'EbpfObject',
> > +  'if': 'CONFIG_EBPF' }
> > diff --git a/qapi/meson.build b/qapi/meson.build
> > index 60a668b343..90047dae1c 100644
> > --- a/qapi/meson.build
> > +++ b/qapi/meson.build
> > @@ -33,6 +33,7 @@ qapi_all_modules = [
> >    'crypto',
> >    'cxl',
> >    'dump',
> > +  'ebpf',
> >    'error',
> >    'introspect',
> >    'job',
> > diff --git a/qapi/qapi-schema.json b/qapi/qapi-schema.json
> > index 6594afba31..2c82a49bae 100644
> > --- a/qapi/qapi-schema.json
> > +++ b/qapi/qapi-schema.json
> > @@ -53,6 +53,7 @@
> >  { 'include': 'char.json' }
> >  { 'include': 'dump.json' }
> >  { 'include': 'net.json' }
> > +{ 'include': 'ebpf.json' }
> >  { 'include': 'rdma.json' }
> >  { 'include': 'rocker.json' }
> >  { 'include': 'tpm.json' }
>
> With the (minor) doc comment fixes:
> Acked-by: Markus Armbruster <armbru@redhat.com>
>
>

On Mon, Dec 18, 2023 at 03:24:57PM +0200, Yuri Benditovich wrote:
> On Mon, Dec 18, 2023 at 2:54 PM Markus Armbruster <armbru@redhat.com> wrote:
> > > +#
> > > +# Currently, there is a possible eBPF for receive-side scaling (RSS).
> > > +#
> > > +##
> > > +
> > > +##
> > > +# @EbpfObject:
> > > +#
> > > +# An eBPF ELF object.
> > > +#
> > > +# @object: the eBPF object encoded in base64
> > > +#
> > > +# Since: 8.2
> >
> > Won't make 8.2, so bump to 9.0.  More of the same below, not noting it
> > again.
> >
> 
> Will there be 8.3?

No, there is never a N.3.0

QEMU does three releases per year, and the major version
increments for the first release of the year. So the
pattern is always

  N.0.0, N.1.0, N.2.0

where "N" changes each year.  The last digit is reserved
for stable branch releases.


With regards,
Daniel