Re: [RFC PATCH 0/5] ebpf: Added ebpf helper for libvirtd.

[Andrew Melnichenko]

Hi,

I may miss something.

But RSS requires to update the map. This won't work if you don't grant
any permission to qemu.

Thanks

Partly - with "kernel.unprivileged_bpf_disabled=0" capabilities is not required to update maps.

With "kernel.unprivileged_bpf_disabled=1" - setting maps will fail(without CAP_BPF) and "in-qemu" RSS will be used.

On Tue, Jun 15, 2021 at 12:13 PM Jason Wang <jasowang@redhat.com> wrote:

在 2021/6/12 上午12:49, Andrew Melnichenko 写道:
> Hi,
>
>     So I think the series is for unprivileged_bpf disabled. If I'm not
>     wrong, I guess the policy is to grant CAP_BPF but do fine grain
>     checks
>     via LSM.
>
>
> The main idea is to run eBPF RSS with qemu without any permission.
> Libvirt should handle everything and pass proper eBPF file descriptors.
> For current eBPF RSS, CAP_SYS_ADMIN(bypass some limitations)
> also required, and in the future may be other permissions.


I may miss something.

But RSS requires to update the map. This won't work if you don't grant
any permission to qemu.

Thanks


>
>     I'm not sure this is the best. We have several examples that let
>     libvirt
>     to involve. Examples:
>
>     1) create TAP device (and the TUN_SETIFF)
>
>     2) open vhost devices
>
>
> Technically TAP/vhost not related to a particular qemu emulator. So common
> TAP creation should fit any modern qemu. eBPF fds(program and maps) should
> suit the interface for current qemu, g.e. some qemu builds may have
> different map
> structures or their count. It's necessary that the qemu got fds
> prepared by the helper
> that was built with the qemu.
>
>     I think we need an example on the detail steps for how libvirt is
>     expected to use this.
>
>
> The simplified workflow looks like this:
>
>  1. Libvirt got "emulator" from domain document.
>  2. Libvirt queries for qemu capabilities.
>  3. One of the capabilities is "qemu-ebpf-rss-helper" path(if present).
>  4. On NIC preparation Libvirt checks for virtio-net + rss configurations.
>  5. If required, the "qemu-ebpf-rss-helper" called and fds are
>     received through unix fd.
>  6. Those fds are for eBPF RSS, which passed to child process - qemu.
>  7. Qemu launched with virtio-net-pci property "rss" and "ebpf_rss_fds".
>
>
> On Fri, Jun 11, 2021 at 8:36 AM Jason Wang <jasowang@redhat.com
> <mailto:jasowang@redhat.com>> wrote:
>
>
>     在 2021/6/10 下午2:55, Yuri Benditovich 写道:
>     > On Thu, Jun 10, 2021 at 9:41 AM Jason Wang<jasowang@redhat.com
>     <mailto:jasowang@redhat.com>>  wrote:
>     >> 在 2021/6/9 下午6:04, Andrew Melnychenko 写道:
>     >>> Libvirt usually launches qemu with strict permissions.
>     >>> To enable eBPF RSS steering, qemu-ebpf-rss-helper was added.
>     >> A silly question:
>     >>
>     >> Kernel had the following permission checks in bpf syscall:
>     >>
>     >>          if (sysctl_unprivileged_bpf_disabled && !bpf_capable())
>     >>                   return -EPERM;
>     >> ...
>     >>
>     >>           err = security_bpf(cmd, &attr, size);
>     >>           if (err < 0)
>     >>                   return err;
>     >>
>     >> So if I understand the code correctly, bpf syscall can only be
>     done if:
>     >>
>     >> 1) unprivileged_bpf is enabled or
>     >> 2) has the capability  and pass the LSM checks
>     >>
>     >> So I think the series is for unprivileged_bpf disabled. If I'm not
>     >> wrong, I guess the policy is to grant CAP_BPF but do fine grain
>     checks
>     >> via LSM.
>     >>
>     >> If this is correct, need to describe it in the commit log.
>     >>
>     >>
>     >>> Added property "ebpf_rss_fds" for "virtio-net" that allows to
>     >>> initialize eBPF RSS context with passed program & maps fds.
>     >>>
>     >>> Added qemu-ebpf-rss-helper - simple helper that loads eBPF
>     >>> context and passes fds through unix socket.
>     >>> Libvirt should call the helper and pass fds to qemu through
>     >>> "ebpf_rss_fds" property.
>     >>>
>     >>> Added explicit target OS check for libbpf dependency in meson.
>     >>> eBPF RSS works only with Linux TAP, so there is no reason to
>     >>> build eBPF loader/helper for non-Linux.
>     >>>
>     >>> Overall, libvirt process should not be aware of the "interface"
>     >>> of eBPF RSS, it will not be aware of eBPF maps/program "type" and
>     >>> their quantity.
>     >> I'm not sure this is the best. We have several examples that
>     let libvirt
>     >> to involve. Examples:
>     >>
>     >> 1) create TAP device (and the TUN_SETIFF)
>     >>
>     >> 2) open vhost devices
>     >>
>     >>
>     >>>    That's why qemu and the helper should be from
>     >>> the same build and be "synchronized". Technically each qemu may
>     >>> have its own helper. That's why "query-helper-paths" qmp command
>     >>> was added. Qemu should return the path to the helper that suits
>     >>> and libvirt should use "that" helper for "that" emulator.
>     >>>
>     >>> qmp sample:
>     >>> C: { "execute": "query-helper-paths" }
>     >>> S: { "return": [
>     >>>        {
>     >>>          "name": "qemu-ebpf-rss-helper",
>     >>>          "path": "/usr/local/libexec/qemu-ebpf-rss-helper"
>     >>>        }
>     >>>       ]
>     >>>      }
>     >> I think we need an example on the detail steps for how libvirt is
>     >> expected to use this.
>     > The preliminary patches for libvirt are at
>     > https://github.com/daynix/libvirt/tree/RSSv1
>     <https://github.com/daynix/libvirt/tree/RSSv1>
>
>
>     Will have a look but it would be better if the assumption of the
>     management is detailed here to ease the reviewers.
>
>     Thanks
>
>
>     >
>