Hi all,
On Thu, Mar 30, 2023 at 11:33 AM Daniel P. Berrangé <berra...@redhat.com> wrote:
>
> On Thu, Mar 30, 2023 at 03:15:20AM +0300, Andrew Melnychenko wrote:
> > Now, the binary objects may be retrieved by id/name.
> > It would require for future qmp commands that may require specific
> > eBPF blob.
> >
> > Signed-off-by: Andrew Melnychenko <and...@daynix.com>
> > ---
> > ebpf/ebpf.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
> > ebpf/ebpf.h | 25 +++++++++++++++++++++++++
> > ebpf/ebpf_rss.c | 4 ++++
> > ebpf/meson.build | 1 +
> > 4 files changed, 78 insertions(+)
> > create mode 100644 ebpf/ebpf.c
> > create mode 100644 ebpf/ebpf.h
> >
> > diff --git a/ebpf/ebpf.c b/ebpf/ebpf.c
> > new file mode 100644
> > index 0000000000..86320d72f5
> > --- /dev/null
> > +++ b/ebpf/ebpf.c
> > @@ -0,0 +1,48 @@
> > +/*
> > + * QEMU eBPF binary declaration routine.
> > + *
> > + * Developed by Daynix Computing LTD (http://www.daynix.com)
> > + *
> > + * Authors:
> > + * Andrew Melnychenko <and...@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.
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "qemu/queue.h"
> > +#include "ebpf/ebpf.h"
> > +
> > +struct ElfBinaryDataEntry {
> > + const char *id;
> > + const void * (*fn)(size_t *);
>
> It feels odd to be storing the function there, as that's just
> an artifact of how the EBPF rss program is acquired. IMHO
> this should just be
>
> const void *data;
> size_t datalen;
>
Yeah, have an idea to store data/size instead. the skeleton provides
them in function,
didn't want to make dirty hacks for the constructor. So I've passed
the function straight to
the structure.
> > +
> > + QSLIST_ENTRY(ElfBinaryDataEntry) node;
> > +};
> > +
> > +static QSLIST_HEAD(, ElfBinaryDataEntry) ebpf_elf_obj_list =
> > + QSLIST_HEAD_INITIALIZER();
> > +
> > +void ebpf_register_binary_data(const char *id, const void * (*fn)(size_t
> > *))
> > +{
> > + struct ElfBinaryDataEntry *data = NULL;
> > +
> > + data = g_malloc0(sizeof(*data));
>
> We prefer g_new0 over g_malloc and initialize when declaring eg
>
> struct ElfBinaryDataEntry *data = g_new0(struct ElfBinaryDataEntry, 1);
>
Ok, thank you.
> > + data->fn = fn;
> > + data->id = id;
> > +
> > + QSLIST_INSERT_HEAD(&ebpf_elf_obj_list, data, node);
> > +}
> > +
> > +const void *ebpf_find_binary_by_id(const char *id, size_t *sz)
> > +{
> > + struct ElfBinaryDataEntry *it = NULL;
> > + QSLIST_FOREACH(it, &ebpf_elf_obj_list, node) {
> > + if (strcmp(id, it->id) == 0) {
> > + return it->fn(sz);
> > + }
> > + }
> > +
> > + return NULL;
> > +}
> > diff --git a/ebpf/ebpf.h b/ebpf/ebpf.h
> > new file mode 100644
> > index 0000000000..fd705cb73e
> > --- /dev/null
> > +++ b/ebpf/ebpf.h
> > @@ -0,0 +1,25 @@
> > +/*
> > + * QEMU eBPF binary declaration routine.
> > + *
> > + * Developed by Daynix Computing LTD (http://www.daynix.com)
> > + *
> > + * Authors:
> > + * Andrew Melnychenko <and...@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
> > +
> > +void ebpf_register_binary_data(const char *id, const void * (*fn)(size_t
> > *));
>
> IMHO it would be better as
>
> void ebpf_register_binary_data(const char *id, const void *data, size_t
> datalen);
>
>
> > +const void *ebpf_find_binary_by_id(const char *id, size_t *sz);
> > +
> > +#define ebpf_binary_init(id, fn)
> > \
> > +static void __attribute__((constructor)) ebpf_binary_init_ ## fn(void)
> > \
> > +{
> > \
> > + ebpf_register_binary_data(id, fn);
> > \
>
> size_t datalen;
> const void *data = fn(&datalen);
> ebpf_register_binary_data(oid, data, datalen);
>
Yeah, it can work like that. For some reason, I want that register
function and the constructor
have the same call/mention.
>
> > +}
> > +
> > +#endif /* EBPF_H */
> > diff --git a/ebpf/ebpf_rss.c b/ebpf/ebpf_rss.c
> > index 08015fecb1..b4038725f2 100644
> > --- a/ebpf/ebpf_rss.c
> > +++ b/ebpf/ebpf_rss.c
> > @@ -21,6 +21,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)
> > @@ -237,3 +239,5 @@ void ebpf_rss_unload(struct EBPFRSSContext *ctx)
> > ctx->obj = NULL;
> > ctx->program_fd = -1;
> > }
> > +
> > +ebpf_binary_init("rss", rss_bpf__elf_bytes)
> > diff --git a/ebpf/meson.build b/ebpf/meson.build
> > index 2dd0fd8948..67c3f53aa9 100644
> > --- a/ebpf/meson.build
> > +++ b/ebpf/meson.build
> > @@ -1 +1,2 @@
> > +softmmu_ss.add(files('ebpf.c'))
> > softmmu_ss.add(when: libbpf, if_true: files('ebpf_rss.c'), if_false:
> > files('ebpf_rss-stub.c'))
> > --
> > 2.39.1
> >
>
> 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 :|
>
