All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Melnichenko <andrew@daynix.com>
To: Jason Wang <jasowang@redhat.com>
Cc: "Daniel P. Berrangé" <berrange@redhat.com>,
	"Michael S . Tsirkin" <mst@redhat.com>,
	qemu-devel@nongnu.org, "Markus Armbruster" <armbru@redhat.com>,
	"Yuri Benditovich" <yuri.benditovich@daynix.com>,
	"Yan Vugenfirer" <yan@daynix.com>,
	"Eric Blake" <eblake@redhat.com>
Subject: Re: [PATCH 4/5] ebpf_rss_helper: Added helper for eBPF RSS.
Date: Mon, 6 Sep 2021 18:50:30 +0300	[thread overview]
Message-ID: <CABcq3pGuqjBY_uqs3BthXS4oSViC=kP16sUbeqLFKJAoQWq6Xw@mail.gmail.com> (raw)
In-Reply-To: <38ea6b36-b968-02bf-b3a8-3d6393df31a5@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 9568 bytes --]

Hi,

> I think it's for back-compatibility.
>
> E.g current codes works without mmap(), and user will surprise that it
> wont' work after upgrading their qemu.
>
Well, the current code would require additional capabilities with
"kernel.unprivileged_bpf_disabled=1", which may be possible on RedHat
systems.
Technically we may have mmap test which will show that mmap for
BPF_MAP_TYPE_ARRAY works, but on the target system, we will know it only in
runtime.
If I'm not mistaken, mmap for BPF_MAP_TYPE_ARRAY was added before kernel
5.4 and our bpf program requires kernel 5.8+.
So, there are no reasons to add bpf() update map as a fallback for mmap().

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

[-- Attachment #2: Type: text/html, Size: 13705 bytes --]

  reply	other threads:[~2021-09-06 15:51 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-13 15:37 [PATCH 0/5] ebpf: Added ebpf helper for libvirtd Andrew Melnychenko
2021-07-13 15:37 ` [PATCH 1/5] ebpf: Added eBPF initialization by fds and map update Andrew Melnychenko
2021-08-20  3:34   ` Jason Wang
2021-08-25 18:13     ` Andrew Melnichenko
2021-07-13 15:37 ` [PATCH 2/5] virtio-net: Added property to load eBPF RSS with fds Andrew Melnychenko
2021-08-20  3:36   ` Jason Wang
2021-08-25 18:18     ` Andrew Melnichenko
2021-07-13 15:37 ` [PATCH 3/5] qmp: Added the helper stamp check Andrew Melnychenko
2021-07-13 15:37 ` [PATCH 4/5] ebpf_rss_helper: Added helper for eBPF RSS Andrew Melnychenko
2021-08-20  3:40   ` Jason Wang
2021-08-25 18:24     ` Andrew Melnichenko
2021-09-01  6:37       ` Jason Wang
2021-08-30 17:07     ` Yuri Benditovich
2021-09-01  6:42       ` Jason Wang
2021-09-06 15:50         ` Andrew Melnichenko [this message]
2021-09-07  3:22           ` Jason Wang
2021-09-07 10:40         ` Yuri Benditovich
2021-09-08  3:45           ` Jason Wang
2021-09-09  0:00             ` Yuri Benditovich
2021-09-09  1:16               ` Jason Wang
2021-09-09 23:43                 ` Yuri Benditovich
2021-09-10  1:37                   ` Jason Wang
2021-07-13 15:37 ` [PATCH 5/5] qmp: Added qemu-ebpf-rss-path command Andrew Melnychenko
2021-08-07 12:54   ` Markus Armbruster
2021-08-10 11:58     ` Andrew Melnichenko
2021-08-24  6:41       ` Markus Armbruster
2021-08-25 18:45         ` Andrew Melnichenko
2021-08-26  4:53           ` Markus Armbruster
2021-08-29 20:13         ` Yuri Benditovich
2021-08-30  6:10           ` Markus Armbruster
2021-08-30  7:51             ` Yuri Benditovich
2021-08-30  8:13               ` Markus Armbruster
2021-08-30 16:56                 ` Yuri Benditovich
2021-08-31 15:00                   ` Markus Armbruster
2021-08-31 19:37                     ` Andrew Melnichenko
2021-09-01  7:16                       ` Markus Armbruster
     [not found]                         ` <CABcq3pGzs=RqLCuu70KyWt7W6T=qEhihK6v=iHJyfuGqiN_Q+A@mail.gmail.com>
     [not found]                           ` <CAOEp5Oc_uUn2nJq+B+SK-iQSo5udyUTirWHS5=8N0JxerRaz7A@mail.gmail.com>
2021-09-09 10:35                             ` Andrew Melnichenko
2021-09-02 16:06                   ` Markus Armbruster
2021-07-22  8:37 ` [PATCH 0/5] ebpf: Added ebpf helper for libvirtd Andrew Melnichenko
2021-08-16 11:57   ` Yuri Benditovich
2021-08-17  5:49     ` Jason Wang
2021-08-20  3:43 ` Jason Wang
2023-02-19 16:20 [PATCH 0/5] eBPF RSS Helper support Andrew Melnychenko
2023-02-19 16:20 ` [PATCH 4/5] ebpf_rss_helper: Added helper for eBPF RSS Andrew Melnychenko
2023-02-20  9:54   ` Daniel P. Berrangé
2023-02-27  3:50     ` Andrew Melnichenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CABcq3pGuqjBY_uqs3BthXS4oSViC=kP16sUbeqLFKJAoQWq6Xw@mail.gmail.com' \
    --to=andrew@daynix.com \
    --cc=armbru@redhat.com \
    --cc=berrange@redhat.com \
    --cc=eblake@redhat.com \
    --cc=jasowang@redhat.com \
    --cc=mst@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=yan@daynix.com \
    --cc=yuri.benditovich@daynix.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.