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

["Daniel P. Berrangé" <berrange@redhat.com>]

On Tue, May 16, 2023 at 10:47:52AM +0200, Markus Armbruster wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
> 
> > 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.
> 
> Yes, but why is this useful?
> 
> Explaining why we want this patch is even more important than explaining
> what it does.  If the commit message does badly at the latter, I can
> still read the actual patch.  If it does badly at the former, I'm lost.
> 
> >> 
> >> 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;
> >> +}
> 
> I recently moved a load of commands from monitor/ to the appropriate
> subsystem.  I'm reluctant to add back commands that aren't about
> controlling the monitor.  Why not ebpf/ebpf-qmp-cmd.c, so MAINTAINERS
> covers it properly?
> 
> >> diff --git a/qapi/misc.json b/qapi/misc.json
> >> index 6ddd16ea283..e96dac8482b 100644
> >> --- a/qapi/misc.json
> >> +++ b/qapi/misc.json
> 
> Why not qapi/ebpf.json, so MAINTAINERS covers it properly?
> 
> >> @@ -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.
> 
> What is RSS, and why should I care?
> 
> >> +#
> >> +# Since: 8.1
> >> +##
> >> +{ 'enum': 'EbpfProgramID',
> >> +  'data': [ { 'name': 'rss', 'if': 'CONFIG_EBPF' } ] }
> >> +
> >> +##
> >> +# @request-ebpf:
> >> +#
> >> +# Function returns eBPF object that can be loaded with libbpf.
> 
> Command, not function.
> 
> Please use imperative mood like "Return X" instead of descriptions like
> "Command returns X" or "Function returns X".
> 
> >> +# 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
> >> +#
> >> +##
> 
> So, this is basically a way to retrieve an eBPF program by some
> well-known name.
> 
> Ignorant question: how are these programs desposited?

The eBPF code blob is linked into QEMU at build time. THis API lets
libvirt fetch it from QEMU, in base64 format. When libvirt later
creates NICs, it can attach the eBPF code blob to the TAP device (which
requires elevated privilleges that QEMU lacks). NB, libvirt would fetch
the eBPF code from QEMU when probing capabilities, as once a VM is
running it is untrusted.

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