Hi all,
On Fri, May 17, 2024 at 5:00 PM Michal Prívozník <mprivozn@xxxxxxxxxx> wrote:
>
> On 5/12/24 21:45, Andrew Melnychenko wrote:
> > Also, added dependencies for libbpf with bpf option.
> >
> > Signed-off-by: Andrew Melnychenko <andrew@xxxxxxxxxx>
> > ---
> > meson.build | 7 ++++
> > meson_options.txt | 1 +
> > src/qemu/meson.build | 1 +
> > src/qemu/qemu_interface.c | 83 +++++++++++++++++++++++++++++++++++++++
> > src/qemu/qemu_interface.h | 4 ++
> > 5 files changed, 96 insertions(+)
>
>
> libvirt.spec.in should be changed too to either selectively disable this
> feature or enable it and drag in the requirement (preferred).
Yes, I'll "enable it" in the next version.
>
> >
> > diff --git a/meson.build b/meson.build
> > index e8b0094b91..e12a703a4d 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -998,6 +998,12 @@ else
> > libkvm_dep = dependency('', required: false)
> > endif
> >
> > +libbpf_version = '1.1.0'
> > +libbpf_dep = dependency('libbpf', version: '>=' + libbpf_version, required: get_option('libbpf'))
> > +if libbpf_dep.found()
> > + conf.set('WITH_BPF', 1)
> > +endif
> > +
> > libiscsi_version = '1.18.0'
> > libiscsi_dep = dependency('libiscsi', version: '>=' + libiscsi_version, required: get_option('libiscsi'))
> >
> > @@ -2283,6 +2289,7 @@ libs_summary = {
> > 'dlopen': dlopen_dep.found(),
> > 'fuse': fuse_dep.found(),
> > 'glusterfs': glusterfs_dep.found(),
> > + 'libbpf': libbpf_dep.found(),
> > 'libiscsi': libiscsi_dep.found(),
> > 'libkvm': libkvm_dep.found(),
> > 'libnbd': libnbd_dep.found(),
> > diff --git a/meson_options.txt b/meson_options.txt
> > index 9d729b3e1f..9b7bd9d1f8 100644
> > --- a/meson_options.txt
> > +++ b/meson_options.txt
> > @@ -48,6 +48,7 @@ option('udev', type: 'feature', value: 'auto', description: 'udev support')
> > option('wireshark_dissector', type: 'feature', value: 'auto', description: 'wireshark support')
> > option('wireshark_plugindir', type: 'string', value: '', description: 'wireshark plugins directory for use when installing wireshark plugin')
> > option('yajl', type: 'feature', value: 'auto', description: 'yajl support')
> > +option('libbpf', type: 'feature', value: 'auto', description: 'qemu libbpf support')
> >
> >
> > # build driver options
> > diff --git a/src/qemu/meson.build b/src/qemu/meson.build
> > index 907893d431..de7ae87d5b 100644
> > --- a/src/qemu/meson.build
> > +++ b/src/qemu/meson.build
> > @@ -105,6 +105,7 @@ if conf.has('WITH_QEMU')
> > selinux_dep,
> > src_dep,
> > xdr_dep,
> > + libbpf_dep,
>
> We tend to keep this kind of lists sorted alphabetically.
>
> > ],
> > include_directories: [
> > conf_inc_dir,
> > diff --git a/src/qemu/qemu_interface.c b/src/qemu/qemu_interface.c
> > index c2007c7043..ec8cf18d86 100644
> > --- a/src/qemu/qemu_interface.c
> > +++ b/src/qemu/qemu_interface.c
> > @@ -38,6 +38,10 @@
> > #include <sys/stat.h>
> > #include <fcntl.h>
> >
> > +#ifdef WITH_BPF
> > +#include <bpf/libbpf.h>
>
> s/#include/# include/
> if you'd install 'cppi' then a syntax-check rule of ours would have
> warned you about this.
>
> > +#endif
> > +
> > #define VIR_FROM_THIS VIR_FROM_QEMU
> >
> > VIR_LOG_INIT("qemu.qemu_interface");
> > @@ -432,3 +436,82 @@ qemuInterfaceOpenVhostNet(virDomainObj *vm,
> > virDomainAuditNetDevice(vm->def, net, vhostnet_path, vhostfdSize);
> > return 0;
> > }
> > +
> > +#ifdef WITH_BPF
> > +
> > +int
> > +qemuInterfaceLoadEbpf(const char *ebpfObject, void **retLibbpfObj, int *fds, size_t nfds)
> > +{
> > + int err = 0;
> > + size_t i = 0;
> > + struct bpf_program *prog;
> > + struct bpf_map *map;
> > + struct bpf_object *obj;
> > + size_t ebpfSize = 0;
> > + g_autofree void *ebpfRawData = NULL;
> > +
> > + ebpfRawData = g_base64_decode(ebpfObject, &ebpfSize);
> > + if (ebpfRawData == NULL) {
> > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("can't decode the eBPF from base64"));
> > + return -1;
> > + }
> > +
> > + obj = bpf_object__open_mem(ebpfRawData, ebpfSize, NULL);
> > + err = libbpf_get_error(obj);
> > + if (err) {
> > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("can't open eBPF object"));
> > + return -1;
> > + }
>
> IIUC, libbpf_get_error() is deprecated and upon failure bpf_object_*()
> APIs return NULL and set errno. Thus this (and the rest) could look
> something like this:
>
> obj = bpf_object__open_mem(...);
> if (!obj) {
> virReportSystemError(errno, "%s", _("can't open eBPF object"));
> return -1;
> }
>
Ok. originally I've tied to omit errno and use VIR_ERR* macros.
> > +
> > +
> > + err = bpf_object__load(obj);
> > + if (err) {
> > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("can't load eBPF object"));
> > + return -1;
> > + }
> > +
> > + bpf_object__for_each_program(prog, obj) {
> > + fds[i] = bpf_program__fd(prog);
> > + ++i;
> > + if (i > nfds) {
> > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("to much file descriptors in eBPF"));
> > + return -1;
> > + }
> > + }
> > +
> > + bpf_object__for_each_map(map, obj) {
> > + fds[i] = bpf_map__fd(map);
> > + ++i;
> > + if (i > nfds) {
> > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("to much file descriptors in eBPF"));
> > + return -1;
> > + }
> > + }
> > +
> > + *retLibbpfObj = obj;
> > +
> > + return i - 1;
> > +}
> > +
> > +
> > +void
> > +qemuInterfaceCloseEbpf(void *libbpfObj)
> > +{
> > + if (libbpfObj)
> > + bpf_object__close(libbpfObj);
> > +}
> > +#else
> > +
> > +int
> > +qemuInterfaceLoadEbpf(const char *ebpfObject G_GNUC_UNUSED, void **retLibbpfObj G_GNUC_UNUSED,
> > + int *fds G_GNUC_UNUSED, size_t nfds G_GNUC_UNUSED)
> > +{
> > + return -1;
>
> Maybe this can return -2 so that callers can distinguish this version
> and the version above failing?
>
Ok.
> > +}
>
> Michal
>
