Re: [PATCH 4/5] ebpf_rss_helper: Added helper for eBPF RSS.

[Yuri Benditovich <yuri.benditovich@daynix.com>]

On Wed, Sep 1, 2021 at 9:42 AM Jason Wang <jasowang@redhat.com> wrote:
>
>
> 在 2021/8/31 上午1:07, Yuri Benditovich 写道:
> > On Fri, Aug 20, 2021 at 6:41 AM Jason Wang <jasowang@redhat.com> wrote:
> >>
> >> 在 2021/7/13 下午11:37, Andrew Melnychenko 写道:
> >>> Helper program. Loads eBPF RSS program and maps and passes them through unix socket.
> >>> Libvirt may launch this helper and pass eBPF fds to qemu virtio-net.
> >>
> >> I wonder if this can be done as helper for TAP/bridge.
> >>
> >> E.g it's the qemu to launch those helper with set-uid.
> >>
> >> Then libvirt won't even need to care about that?
> >>
> > There are pros and cons for such a solution with set-uid.
> >  From my point of view one of the cons is that set-uid is efficient
> > only at install time so the coexistence of different qemu builds (and
> > different helpers for each one) is kind of problematic.
> > With the current solution this does not present any problem: the
> > developer can have several different builds, each one automatically
> > has its own helper and there is no conflict between these builds and
> > between these builds and installed qemu package. Changing the
> > 'emulator' in the libvirt profile automatically brings the proper
> > helper to work.
>
>
> I'm not sure I get you here. We can still have default/sample helper to
> make sure it works for different builds.
>
> If we can avoid the involvement of libvirt, that would be better.

Hi Jason,

Indeed I did not get the idea, can you please explain it in more
details (as detailed as possible to avoid future misunderstanding),
especially how exactly we can use the set-uid and what is the 'default' helper.
We also would prefer to do everything from qemu but we do not see how
we can do that.

Our main points (what should be addressed):
- qemu should be able to load ebpf and use the maps when it runs from
libvirt (without special caps) and standalone (with caps)
- it is possible that there are different qemu builds on the machine,
one of them might be installed, their ebpf's might be different and
the interface between qemu and ebpf (exact content of maps and number
of maps)
- qemu configures the RSS dynamically according to the commands
provided by the guest

Thanks in advance
Yuri

>
> Thanks
>
>
> >
> >>> Also, libbpf dependency now exclusively for Linux.
> >>> Libbpf is used for eBPF RSS steering, which is supported only by Linux TAP.
> >>> There is no reason yet to build eBPF loader and helper for non Linux systems,
> >>> even if libbpf is present.
> >>>
> >>> Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
> >>> ---
> >>>    ebpf/qemu-ebpf-rss-helper.c | 130 ++++++++++++++++++++++++++++++++++++
> >>>    meson.build                 |  37 ++++++----
> >>>    2 files changed, 154 insertions(+), 13 deletions(-)
> >>>    create mode 100644 ebpf/qemu-ebpf-rss-helper.c
> >>>
> >>> diff --git a/ebpf/qemu-ebpf-rss-helper.c b/ebpf/qemu-ebpf-rss-helper.c
> >>> new file mode 100644
> >>> index 0000000000..fe68758f57
> >>> --- /dev/null
> >>> +++ b/ebpf/qemu-ebpf-rss-helper.c
> >>> @@ -0,0 +1,130 @@
> >>> +/*
> >>> + * eBPF RSS Helper
> >>> + *
> >>> + * 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.  See
> >>> + * the COPYING file in the top-level directory.
> >>> + *
> >>> + * Description: This is helper program for libvirtd.
> >>> + *              It loads eBPF RSS program and passes fds through unix socket.
> >>> + *              Built by meson, target - 'qemu-ebpf-rss-helper'.
> >>> + */
> >>> +
> >>> +#include <stdio.h>
> >>> +#include <stdint.h>
> >>> +#include <stdlib.h>
> >>> +#include <stdbool.h>
> >>> +#include <getopt.h>
> >>> +#include <memory.h>
> >>> +#include <errno.h>
> >>> +#include <sys/socket.h>
> >>> +
> >>> +#include "ebpf_rss.h"
> >>> +
> >>> +#include "qemu-helper-stamp.h"
> >>> +
> >>> +void QEMU_HELPER_STAMP(void) {}
> >>> +
> >>> +static int send_fds(int socket, int *fds, int n)
> >>> +{
> >>> +    struct msghdr msg = {};
> >>> +    struct cmsghdr *cmsg = NULL;
> >>> +    char buf[CMSG_SPACE(n * sizeof(int))];
> >>> +    char dummy_buffer = 0;
> >>> +    struct iovec io = { .iov_base = &dummy_buffer,
> >>> +                        .iov_len = sizeof(dummy_buffer) };
> >>> +
> >>> +    memset(buf, 0, sizeof(buf));
> >>> +
> >>> +    msg.msg_iov = &io;
> >>> +    msg.msg_iovlen = 1;
> >>> +    msg.msg_control = buf;
> >>> +    msg.msg_controllen = sizeof(buf);
> >>> +
> >>> +    cmsg = CMSG_FIRSTHDR(&msg);
> >>> +    cmsg->cmsg_level = SOL_SOCKET;
> >>> +    cmsg->cmsg_type = SCM_RIGHTS;
> >>> +    cmsg->cmsg_len = CMSG_LEN(n * sizeof(int));
> >>> +
> >>> +    memcpy(CMSG_DATA(cmsg), fds, n * sizeof(int));
> >>> +
> >>> +    return sendmsg(socket, &msg, 0);
> >>> +}
> >>> +
> >>> +static void print_help_and_exit(const char *prog, int exitcode)
> >>> +{
> >>> +    fprintf(stderr, "%s - load eBPF RSS program for qemu and pass eBPF fds"
> >>> +            " through unix socket.\n", prog);
> >>> +    fprintf(stderr, "\t--fd <num>, -f <num> - unix socket file descriptor"
> >>> +            " used to pass eBPF fds.\n");
> >>> +    fprintf(stderr, "\t--help, -h - this help.\n");
> >>> +    exit(exitcode);
> >>> +}
> >>> +
> >>> +int main(int argc, char **argv)
> >>> +{
> >>> +    char *fd_string = NULL;
> >>> +    int unix_fd = 0;
> >>> +    struct EBPFRSSContext ctx = {};
> >>> +    int fds[EBPF_RSS_MAX_FDS] = {};
> >>> +    int ret = -1;
> >>> +
> >>> +    for (;;) {
> >>> +        int c;
> >>> +        static struct option long_options[] = {
> >>> +                {"help",  no_argument, 0, 'h'},
> >>> +                {"fd",  required_argument, 0, 'f'},
> >>> +                {0, 0, 0, 0}
> >>> +        };
> >>> +        c = getopt_long(argc, argv, "hf:",
> >>> +                long_options, NULL);
> >>> +
> >>> +        if (c == -1) {
> >>> +            break;
> >>> +        }
> >>> +
> >>> +        switch (c) {
> >>> +        case 'f':
> >>> +            fd_string = optarg;
> >>> +            break;
> >>> +        case 'h':
> >>> +        default:
> >>> +            print_help_and_exit(argv[0],
> >>> +                    c == 'h' ? EXIT_SUCCESS : EXIT_FAILURE);
> >>> +        }
> >>> +    }
> >>> +
> >>> +    if (!fd_string) {
> >>> +        fprintf(stderr, "Unix file descriptor not present.\n");
> >>> +        print_help_and_exit(argv[0], EXIT_FAILURE);
> >>> +    }
> >>> +
> >>> +    unix_fd = atoi(fd_string);
> >>> +
> >>> +    if (!unix_fd) {
> >>> +        fprintf(stderr, "Unix file descriptor is invalid.\n");
> >>> +        return EXIT_FAILURE;
> >>> +    }
> >>> +
> >>> +    ebpf_rss_init(&ctx);
> >>> +    if (!ebpf_rss_load(&ctx)) {
> >>> +        fprintf(stderr, "Can't load ebpf.\n");
> >>> +        return EXIT_FAILURE;
> >>> +    }
> >>> +    fds[0] = ctx.program_fd;
> >>> +    fds[1] = ctx.map_configuration;
> >>> +
> >>> +    ret = send_fds(unix_fd, fds, EBPF_RSS_MAX_FDS);
> >>> +    if (ret < 0) {
> >>> +        fprintf(stderr, "Issue while sending fds: %s.\n", strerror(errno));
> >>> +    }
> >>> +
> >>> +    ebpf_rss_unload(&ctx);
> >>> +
> >>> +    return ret < 0 ? EXIT_FAILURE : EXIT_SUCCESS;
> >>> +}
> >>> +
> >>> diff --git a/meson.build b/meson.build
> >>> index 257e51d91b..913aa1fee5 100644
> >>> --- a/meson.build
> >>> +++ b/meson.build
> >>> @@ -1033,19 +1033,22 @@ if not get_option('fuse_lseek').disabled()
> >>>    endif
> >>>
> >>>    # libbpf
> >>> -libbpf = dependency('libbpf', required: get_option('bpf'), method: 'pkg-config')
> >>> -if libbpf.found() and not cc.links('''
> >>> -   #include <bpf/libbpf.h>
> >>> -   int main(void)
> >>> -   {
> >>> -     bpf_object__destroy_skeleton(NULL);
> >>> -     return 0;
> >>> -   }''', dependencies: libbpf)
> >>> -  libbpf = not_found
> >>> -  if get_option('bpf').enabled()
> >>> -    error('libbpf skeleton test failed')
> >>> -  else
> >>> -    warning('libbpf skeleton test failed, disabling')
> >>> +libbpf = not_found
> >>> +if targetos == 'linux'
> >>> +  libbpf = dependency('libbpf', required: get_option('bpf'), method: 'pkg-config')
> >>> +  if libbpf.found() and not cc.links('''
> >>> +    #include <bpf/libbpf.h>
> >>> +    int main(void)
> >>> +    {
> >>> +      bpf_object__destroy_skeleton(NULL);
> >>
> >> Do we need to test whether the bpf can do mmap() here?
> >>
> >> Thanks
> >>
> >>
> >>> +      return 0;
> >>> +    }''', dependencies: libbpf)
> >>> +    libbpf = not_found
> >>> +    if get_option('bpf').enabled()
> >>> +      error('libbpf skeleton test failed')
> >>> +    else
> >>> +      warning('libbpf skeleton test failed, disabling')
> >>> +    endif
> >>>      endif
> >>>    endif
> >>>
> >>> @@ -2423,6 +2426,14 @@ if have_tools
> >>>                   dependencies: [authz, crypto, io, qom, qemuutil,
> >>>                                  libcap_ng, mpathpersist],
> >>>                   install: true)
> >>> +
> >>> +    if libbpf.found()
> >>> +        executable('qemu-ebpf-rss-helper', files(
> >>> +                   'ebpf/qemu-ebpf-rss-helper.c', 'ebpf/ebpf_rss.c'),
> >>> +                   dependencies: [qemuutil, libbpf, glib],
> >>> +                   install: true,
> >>> +                   install_dir: get_option('libexecdir'))
> >>> +    endif
> >>>      endif
> >>>
> >>>      if 'CONFIG_IVSHMEM' in config_host
>