Re: [PATCH 2/5] virtio-net: Added property to load eBPF RSS with fds.

[Andrew Melnichenko]

Hi,

I wonder if it's better to use separated properties instead of implying
an order here?

Not really, technically RSS BPF interface may be changed (it's already changed after RFC).

And libvirt should use something unified, so it's better to use fd array.

If any changes occur - those changes will be applied only for qemu and the helper.

Also, now all maps are combined in one configuration map.

On Fri, Aug 20, 2021 at 6:36 AM Jason Wang <jasowang@redhat.com> wrote:

在 2021/7/13 下午11:37, Andrew Melnychenko 写道:
> eBPF RSS program and maps now may be passed during initialization.
> Initially was implemented for libvirt to launch qemu without permissions.
>
> Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
> ---
>   hw/net/virtio-net.c            | 77 ++++++++++++++++++++++++++++++++--
>   include/hw/virtio/virtio-net.h |  1 +
>   2 files changed, 74 insertions(+), 4 deletions(-)
>
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index bd7958b9f0..0602b1772e 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -41,6 +41,7 @@
>   #include "sysemu/sysemu.h"
>   #include "trace.h"
>   #include "monitor/qdev.h"
> +#include "monitor/monitor.h"
>   #include "hw/pci/pci.h"
>   #include "net_rx_pkt.h"
>   #include "hw/virtio/vhost.h"
> @@ -1223,14 +1224,81 @@ static void virtio_net_detach_epbf_rss(VirtIONet *n)
>       virtio_net_attach_ebpf_to_backend(n->nic, -1);
>   }
>   
> -static bool virtio_net_load_ebpf(VirtIONet *n)
> +static int virtio_net_get_ebpf_rss_fds(char *str, char *fds[], int nfds)
>   {
> -    if (!virtio_net_attach_ebpf_to_backend(n->nic, -1)) {
> -        /* backend does't support steering ebpf */
> +    char *ptr = str;
> +    char *cur = NULL;
> +    size_t len = strlen(str);
> +    int i = 0;
> +
> +    for (; i < nfds && ptr < str + len;) {
> +        cur = strchr(ptr, ':');
> +
> +        if (cur == NULL) {
> +            fds[i] = g_strdup(ptr);
> +        } else {
> +            fds[i] = g_strndup(ptr, cur - ptr);
> +        }
> +
> +        i++;
> +        if (cur == NULL) {
> +            break;
> +        } else {
> +            ptr = cur + 1;
> +        }
> +    }
> +
> +    return i;
> +}
> +
> +static bool virtio_net_load_ebpf_fds(VirtIONet *n)
> +{
> +    char *fds_strs[EBPF_RSS_MAX_FDS];
> +    int fds[EBPF_RSS_MAX_FDS];
> +    int nfds;
> +    int ret = false;
> +    Error *errp;
> +    int i = 0;
> +
> +    if (n == NULL || !n->ebpf_rss_fds) {
>           return false;
>       }
>   
> -    return ebpf_rss_load(&n->ebpf_rss);
> +    nfds = virtio_net_get_ebpf_rss_fds(n->ebpf_rss_fds,
> +                                       fds_strs, EBPF_RSS_MAX_FDS);
> +    for (i = 0; i < nfds; i++) {
> +        fds[i] = monitor_fd_param(monitor_cur(), fds_strs[i], &errp);
> +    }
> +
> +    if (nfds == EBPF_RSS_MAX_FDS) {
> +        ret = ebpf_rss_load_fds(&n->ebpf_rss, fds[0], fds[1], fds[2], fds[3]);
> +    }
> +
> +    if (!ret) {
> +        for (i = 0; i < nfds; i++) {
> +            close(fds[i]);
> +        }
> +    }
> +
> +    for (i = 0; i < nfds; i++) {
> +        g_free(fds_strs[i]);
> +    }
> +
> +    return ret;
> +}
> +
> +static bool virtio_net_load_ebpf(VirtIONet *n)
> +{
> +    bool ret = true;
> +
> +    if (virtio_net_attach_ebpf_to_backend(n->nic, -1)) {
> +        if (!(n->ebpf_rss_fds
> +                && virtio_net_load_ebpf_fds(n))) {
> +            ret = ebpf_rss_load(&n->ebpf_rss);
> +        }
> +    }
> +
> +    return ret;
>   }
>   
>   static void virtio_net_unload_ebpf(VirtIONet *n)
> @@ -3605,6 +3673,7 @@ static Property virtio_net_properties[] = {
>                       VIRTIO_NET_F_RSS, false),
>       DEFINE_PROP_BIT64("hash", VirtIONet, host_features,
>                       VIRTIO_NET_F_HASH_REPORT, false),
> +    DEFINE_PROP_STRING("ebpf_rss_fds", VirtIONet, ebpf_rss_fds),
>       DEFINE_PROP_BIT64("guest_rsc_ext", VirtIONet, host_features,
>                       VIRTIO_NET_F_RSC_EXT, false),
>       DEFINE_PROP_UINT32("rsc_interval", VirtIONet, rsc_timeout,
> diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h
> index 824a69c23f..993f2f3036 100644
> --- a/include/hw/virtio/virtio-net.h
> +++ b/include/hw/virtio/virtio-net.h
> @@ -213,6 +213,7 @@ struct VirtIONet {
>       VirtioNetRssData rss_data;
>       struct NetRxPkt *rx_pkt;
>       struct EBPFRSSContext ebpf_rss;
> +    char *ebpf_rss_fds;


I wonder if it's better to use separated properties instead of implying
an order here?

E.g "program_fd", "key_map_fd", "indirection_table_map_fd" etc.

Thanks


>   };
>   
>   void virtio_net_set_netclient_name(VirtIONet *n, const char *name,