1. Patchew
  2. QEMU
  3. [PATCH 0/5] eBPF RSS through QMP support.
  4. View patch

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

Andrew Melnychenko posted 5 patches 2 years, 9 months ago
Maintainers: Jason Wang <jasowang@redhat.com>, Andrew Melnychenko <andrew@daynix.com>, Yuri Benditovich <yuri.benditovich@daynix.com>, "Michael S. Tsirkin" <mst@redhat.com>, Markus Armbruster <armbru@redhat.com>, Eric Blake <eblake@redhat.com>
There is a newer version of this series
[PATCH 2/5] virtio-net: Added property to load eBPF RSS with fds.
Posted by Andrew Melnychenko 2 years, 9 months ago 
eBPF RSS program and maps may now be passed during initialization.
Initially was implemented for libvirt to launch qemu without permissions,
and initialized eBPF program through the helper.

Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
---
 hw/net/virtio-net.c            | 96 +++++++++++++++++++++++++++++++---
 include/hw/virtio/virtio-net.h |  1 +
 2 files changed, 91 insertions(+), 6 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 53e1c326433..9b3a997d872 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -42,6 +42,7 @@
 #include "sysemu/sysemu.h"
 #include "trace.h"
 #include "monitor/qdev.h"
+#include "monitor/monitor.h"
 #include "hw/pci/pci_device.h"
 #include "net_rx_pkt.h"
 #include "hw/virtio/vhost.h"
@@ -1305,14 +1306,96 @@ 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,
+                                       Error **errp)
 {
-    if (!virtio_net_attach_ebpf_to_backend(n->nic, -1)) {
-        /* backend does't support steering ebpf */
-        return false;
+    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;
+        }
+    }
+
+    if (cur != NULL) {
+        /* the string contains more arguments */
+        error_setg(errp,
+                   "Too many eBPF file descriptors for RSS provided.");
+    } else if (i < nfds) {
+        error_setg(errp,
+                   "Not enough eBPF file descriptors for RSS were provided.");
+    }
+
+    return i;
+}
+
+static bool virtio_net_load_ebpf_fds(VirtIONet *n, Error **errp)
+{
+    char *fds_strs[EBPF_RSS_MAX_FDS];
+    int fds[EBPF_RSS_MAX_FDS];
+    int nfds;
+    int ret = true;
+    int i = 0;
+
+    ERRP_GUARD();
+
+    nfds = virtio_net_get_ebpf_rss_fds(n->ebpf_rss_fds,
+                                       fds_strs, EBPF_RSS_MAX_FDS, errp);
+    if (*errp) {
+        ret = false;
+        goto exit;
     }
 
-    return ebpf_rss_load(&n->ebpf_rss);
+    for (i = 0; i < nfds; i++) {
+        fds[i] = monitor_fd_param(monitor_cur(), fds_strs[i], errp);
+        if (*errp) {
+            ret = false;
+            goto exit;
+        }
+    }
+
+    ret = ebpf_rss_load_fds(&n->ebpf_rss, fds[0], fds[1], fds[2], fds[3]);
+
+exit:
+    if (!ret || *errp) {
+        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, Error **errp)
+{
+    bool ret = false;
+
+    if (virtio_net_attach_ebpf_to_backend(n->nic, -1)) {
+        if (!(n->ebpf_rss_fds
+                && virtio_net_load_ebpf_fds(n, errp))) {
+            ret = ebpf_rss_load(&n->ebpf_rss);
+        }
+    }
+
+    return ret;
 }
 
 static void virtio_net_unload_ebpf(VirtIONet *n)
@@ -3738,7 +3821,7 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
     net_rx_pkt_init(&n->rx_pkt);
 
     if (virtio_has_feature(n->host_features, VIRTIO_NET_F_RSS)) {
-        virtio_net_load_ebpf(n);
+        virtio_net_load_ebpf(n, errp);
     }
 }
 
@@ -3900,6 +3983,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 ef234ffe7ef..e10ce88f918 100644
--- a/include/hw/virtio/virtio-net.h
+++ b/include/hw/virtio/virtio-net.h
@@ -219,6 +219,7 @@ struct VirtIONet {
     VirtioNetRssData rss_data;
     struct NetRxPkt *rx_pkt;
     struct EBPFRSSContext ebpf_rss;
+    char *ebpf_rss_fds;
 };
 
 size_t virtio_net_handle_ctrl_iov(VirtIODevice *vdev,
-- 
2.39.1
Re: [PATCH 2/5] virtio-net: Added property to load eBPF RSS with fds.
Posted by Daniel P. Berrangé 2 years, 9 months ago 
On Mon, May 01, 2023 at 10:20:58AM +0300, Andrew Melnychenko wrote:
> eBPF RSS program and maps may now be passed during initialization.
> Initially was implemented for libvirt to launch qemu without permissions,
> and initialized eBPF program through the helper.
> 
> Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
> ---
>  hw/net/virtio-net.c            | 96 +++++++++++++++++++++++++++++++---
>  include/hw/virtio/virtio-net.h |  1 +
>  2 files changed, 91 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 53e1c326433..9b3a997d872 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -42,6 +42,7 @@
>  #include "sysemu/sysemu.h"
>  #include "trace.h"
>  #include "monitor/qdev.h"
> +#include "monitor/monitor.h"
>  #include "hw/pci/pci_device.h"
>  #include "net_rx_pkt.h"
>  #include "hw/virtio/vhost.h"
> @@ -1305,14 +1306,96 @@ 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,
> +                                       Error **errp)
>  {
> -    if (!virtio_net_attach_ebpf_to_backend(n->nic, -1)) {
> -        /* backend does't support steering ebpf */
> -        return false;
> +    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;
> +        }
> +    }
> +
> +    if (cur != NULL) {
> +        /* the string contains more arguments */
> +        error_setg(errp,
> +                   "Too many eBPF file descriptors for RSS provided.");
> +    } else if (i < nfds) {
> +        error_setg(errp,
> +                   "Not enough eBPF file descriptors for RSS were provided.");
> +    }
> +
> +    return i;
> +}

This whole method could be replaced by a call to g_strsplit in
the caller....
 

> +
> +static bool virtio_net_load_ebpf_fds(VirtIONet *n, Error **errp)
> +{
> +    char *fds_strs[EBPF_RSS_MAX_FDS];

   g_autoptr(GStrv) fds_strs = g_strsplit(n->ebpf_rss_fds, ",", 0);

> +    int fds[EBPF_RSS_MAX_FDS];

Left as uninitialized stack memory.

> +    int nfds;
> +    int ret = true;
> +    int i = 0;
> +
> +    ERRP_GUARD();
> +
> +    nfds = virtio_net_get_ebpf_rss_fds(n->ebpf_rss_fds,
> +                                       fds_strs, EBPF_RSS_MAX_FDS, errp);
> +    if (*errp) {
> +        ret = false;
> +        goto exit;
>      }

Replace with 

   if (g_strv_length(fds_strs) != EBPF_RSS_MAX_FDS) {
       error_setg(errp,
                  "Expected %d file descriptors but got %d",
		  EBPF_RSS_MAX_FDS, g_strv_length(fds_strs));
       return false;
   }

>  
> -    return ebpf_rss_load(&n->ebpf_rss);
> +    for (i = 0; i < nfds; i++) {
> +        fds[i] = monitor_fd_param(monitor_cur(), fds_strs[i], errp);
> +        if (*errp) {
> +            ret = false;
> +            goto exit;

This can break out of the loop before all elements in 'fds' are
initialized.

> +        }
> +    }
> +
> +    ret = ebpf_rss_load_fds(&n->ebpf_rss, fds[0], fds[1], fds[2], fds[3]);
> +
> +exit:
> +    if (!ret || *errp) {
> +        for (i = 0; i < nfds; i++) {
> +            close(fds[i]);
> +        }
> +    }

This now calls close() on uninitialized memory, killing
arbitary FDs QEMU has open elsewhere.

> +
> +    for (i = 0; i < nfds; i++) {
> +        g_free(fds_strs[i]);
> +    }

Not required if we use g_autoptr(GStrv)

> +
> +    return ret;
> +}
> +
> +static bool virtio_net_load_ebpf(VirtIONet *n, Error **errp)
> +{
> +    bool ret = false;
> +
> +    if (virtio_net_attach_ebpf_to_backend(n->nic, -1)) {
> +        if (!(n->ebpf_rss_fds
> +                && virtio_net_load_ebpf_fds(n, errp))) {
> +            ret = ebpf_rss_load(&n->ebpf_rss);
> +        }
> +    }
> +
> +    return ret;
>  }
>  
>  static void virtio_net_unload_ebpf(VirtIONet *n)
> @@ -3738,7 +3821,7 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
>      net_rx_pkt_init(&n->rx_pkt);
>  
>      if (virtio_has_feature(n->host_features, VIRTIO_NET_F_RSS)) {
> -        virtio_net_load_ebpf(n);
> +        virtio_net_load_ebpf(n, errp);
>      }
>  }
>  
> @@ -3900,6 +3983,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 ef234ffe7ef..e10ce88f918 100644
> --- a/include/hw/virtio/virtio-net.h
> +++ b/include/hw/virtio/virtio-net.h
> @@ -219,6 +219,7 @@ struct VirtIONet {
>      VirtioNetRssData rss_data;
>      struct NetRxPkt *rx_pkt;
>      struct EBPFRSSContext ebpf_rss;
> +    char *ebpf_rss_fds;
>  };
>  
>  size_t virtio_net_handle_ctrl_iov(VirtIODevice *vdev,
> -- 
> 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 :|
Re: [PATCH 2/5] virtio-net: Added property to load eBPF RSS with fds.
Posted by Andrew Melnichenko 2 years, 9 months ago 
Hi all,

Thank you for your comments. I'll update it in the next patch version.

On Wed, May 3, 2023 at 2:03 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Mon, May 01, 2023 at 10:20:58AM +0300, Andrew Melnychenko wrote:
> > eBPF RSS program and maps may now be passed during initialization.
> > Initially was implemented for libvirt to launch qemu without permissions,
> > and initialized eBPF program through the helper.
> >
> > Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
> > ---
> >  hw/net/virtio-net.c            | 96 +++++++++++++++++++++++++++++++---
> >  include/hw/virtio/virtio-net.h |  1 +
> >  2 files changed, 91 insertions(+), 6 deletions(-)
> >
> > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> > index 53e1c326433..9b3a997d872 100644
> > --- a/hw/net/virtio-net.c
> > +++ b/hw/net/virtio-net.c
> > @@ -42,6 +42,7 @@
> >  #include "sysemu/sysemu.h"
> >  #include "trace.h"
> >  #include "monitor/qdev.h"
> > +#include "monitor/monitor.h"
> >  #include "hw/pci/pci_device.h"
> >  #include "net_rx_pkt.h"
> >  #include "hw/virtio/vhost.h"
> > @@ -1305,14 +1306,96 @@ 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,
> > +                                       Error **errp)
> >  {
> > -    if (!virtio_net_attach_ebpf_to_backend(n->nic, -1)) {
> > -        /* backend does't support steering ebpf */
> > -        return false;
> > +    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;
> > +        }
> > +    }
> > +
> > +    if (cur != NULL) {
> > +        /* the string contains more arguments */
> > +        error_setg(errp,
> > +                   "Too many eBPF file descriptors for RSS provided.");
> > +    } else if (i < nfds) {
> > +        error_setg(errp,
> > +                   "Not enough eBPF file descriptors for RSS were provided.");
> > +    }
> > +
> > +    return i;
> > +}
>
> This whole method could be replaced by a call to g_strsplit in
> the caller....
>
>
> > +
> > +static bool virtio_net_load_ebpf_fds(VirtIONet *n, Error **errp)
> > +{
> > +    char *fds_strs[EBPF_RSS_MAX_FDS];
>
>    g_autoptr(GStrv) fds_strs = g_strsplit(n->ebpf_rss_fds, ",", 0);
>
> > +    int fds[EBPF_RSS_MAX_FDS];
>
> Left as uninitialized stack memory.
>
> > +    int nfds;
> > +    int ret = true;
> > +    int i = 0;
> > +
> > +    ERRP_GUARD();
> > +
> > +    nfds = virtio_net_get_ebpf_rss_fds(n->ebpf_rss_fds,
> > +                                       fds_strs, EBPF_RSS_MAX_FDS, errp);
> > +    if (*errp) {
> > +        ret = false;
> > +        goto exit;
> >      }
>
> Replace with
>
>    if (g_strv_length(fds_strs) != EBPF_RSS_MAX_FDS) {
>        error_setg(errp,
>                   "Expected %d file descriptors but got %d",
>                   EBPF_RSS_MAX_FDS, g_strv_length(fds_strs));
>        return false;
>    }
>
> >
> > -    return ebpf_rss_load(&n->ebpf_rss);
> > +    for (i = 0; i < nfds; i++) {
> > +        fds[i] = monitor_fd_param(monitor_cur(), fds_strs[i], errp);
> > +        if (*errp) {
> > +            ret = false;
> > +            goto exit;
>
> This can break out of the loop before all elements in 'fds' are
> initialized.
>
> > +        }
> > +    }
> > +
> > +    ret = ebpf_rss_load_fds(&n->ebpf_rss, fds[0], fds[1], fds[2], fds[3]);
> > +
> > +exit:
> > +    if (!ret || *errp) {
> > +        for (i = 0; i < nfds; i++) {
> > +            close(fds[i]);
> > +        }
> > +    }
>
> This now calls close() on uninitialized memory, killing
> arbitary FDs QEMU has open elsewhere.
>
> > +
> > +    for (i = 0; i < nfds; i++) {
> > +        g_free(fds_strs[i]);
> > +    }
>
> Not required if we use g_autoptr(GStrv)
>
> > +
> > +    return ret;
> > +}
> > +
> > +static bool virtio_net_load_ebpf(VirtIONet *n, Error **errp)
> > +{
> > +    bool ret = false;
> > +
> > +    if (virtio_net_attach_ebpf_to_backend(n->nic, -1)) {
> > +        if (!(n->ebpf_rss_fds
> > +                && virtio_net_load_ebpf_fds(n, errp))) {
> > +            ret = ebpf_rss_load(&n->ebpf_rss);
> > +        }
> > +    }
> > +
> > +    return ret;
> >  }
> >
> >  static void virtio_net_unload_ebpf(VirtIONet *n)
> > @@ -3738,7 +3821,7 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
> >      net_rx_pkt_init(&n->rx_pkt);
> >
> >      if (virtio_has_feature(n->host_features, VIRTIO_NET_F_RSS)) {
> > -        virtio_net_load_ebpf(n);
> > +        virtio_net_load_ebpf(n, errp);
> >      }
> >  }
> >
> > @@ -3900,6 +3983,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 ef234ffe7ef..e10ce88f918 100644
> > --- a/include/hw/virtio/virtio-net.h
> > +++ b/include/hw/virtio/virtio-net.h
> > @@ -219,6 +219,7 @@ struct VirtIONet {
> >      VirtioNetRssData rss_data;
> >      struct NetRxPkt *rx_pkt;
> >      struct EBPFRSSContext ebpf_rss;
> > +    char *ebpf_rss_fds;
> >  };
> >
> >  size_t virtio_net_handle_ctrl_iov(VirtIODevice *vdev,
> > --
> > 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 :|
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.