Re: [PATCH v2 5/6] qmp: Added new command to retrieve eBPF blob.

[Daniel P . Berrangé]

Question for Markus at the bottom....

On Fri, May 12, 2023 at 03:29:01PM +0300, Andrew Melnychenko wrote:
> 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.
> 
> Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
> ---
>  monitor/qmp-cmds.c | 16 ++++++++++++++++
>  qapi/misc.json     | 38 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 54 insertions(+)
> 
> diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
> index b0f948d3376..259bc87ccb1 100644
> --- a/monitor/qmp-cmds.c
> +++ b/monitor/qmp-cmds.c
> @@ -32,6 +32,7 @@
>  #include "hw/mem/memory-device.h"
>  #include "hw/intc/intc.h"
>  #include "hw/rdma/rdma.h"
> +#include "ebpf/ebpf.h"
>  
>  NameInfo *qmp_query_name(Error **errp)
>  {
> @@ -209,3 +210,18 @@ static void __attribute__((__constructor__)) 
> monitor_init_qmp_commands(void)
>                           qmp_marshal_qmp_capabilities,
>                           QCO_ALLOW_PRECONFIG, 0);
>  }
> +
> +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/qapi/misc.json b/qapi/misc.json
> index 6ddd16ea283..e96dac8482b 100644
> --- a/qapi/misc.json
> +++ b/qapi/misc.json
> @@ -618,3 +618,41 @@
>  { 'event': 'VFU_CLIENT_HANGUP',
>    'data': { 'vfu-id': 'str', 'vfu-qom-path': 'str',
>              'dev-id': 'str', 'dev-qom-path': 'str' } }
> +
> +##
> +# @EbpfObject:
> +#
> +# Structure that holds eBPF ELF object encoded in base64.
> +#
> +# Since: 8.1
> +#
> +##
> +{ 'struct': 'EbpfObject',
> +  'data': {'object': 'str'} }
> +
> +##
> +# @EbpfProgramID:
> +#
> +# An enumeration of the eBPF programs. Currently, only RSS is presented.
> +#
> +# Since: 8.1
> +##
> +{ 'enum': 'EbpfProgramID',
> +  'data': [ { 'name': 'rss', 'if': 'CONFIG_EBPF' } ] }
> +
> +##
> +# @request-ebpf:
> +#
> +# Function returns eBPF object that can be loaded with libbpf.
> +# Management applications (g.e. libvirt) may load it and pass file
> +# descriptors to QEMU. Which allows running QEMU without BPF capabilities.
> +#
> +# Returns: RSS eBPF object encoded in base64.
> +#
> +# Since: 8.1
> +#
> +##
> +{ 'command': 'request-ebpf',
> +  'data': { 'id': 'EbpfProgramID' },
> +  'returns': 'EbpfObject' }
> +

Fnuctionally this is fine so

  Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


A question for Markus though - is is perhaps better to mark all the
command/enum/object as conditional on CONFIG_EBPF, rather than just
reporting an empty EbpfProgramID enum when EBPF is disabled at build
time ?


With 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 :|