Re: [RFC PATCH 2/5] virtio-net: Added property to load eBPF RSS with fds.
On Thu, Mar 30, 2023 at 03:15:19AM +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
> ---
> 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 53e1c32643..45d448a83d 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,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)
Needs to have an 'Error **errp' parameter
> +{
> +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;
> +}
> +}
...and here needs to error_setg(...) if the string being parsed has
more FDs present than 'nfds', and return error.
It should also report error if less than 'nfds' were present.
> +
> +return i;
> +}
> +
> +static bool virtio_net_load_ebpf_fds(VirtIONet *n)
> {
> -if (!virtio_net_attach_ebpf_to_backend(n->nic, -1)) {
> -/* backend does't support steering ebpf */
> +char *fds_strs[EBPF_RSS_MAX_FDS];
> +int fds[EBPF_RSS_MAX_FDS];
> +int nfds;
> +int ret = false;
> +Error *errp;
This method should be having 'Error **errp' passed into it
instead
> +int i = 0;
> +
> +if (n == NULL || !n->ebpf_rss_fds) {
> return false;
> }
The only caller has already de-referenced 'n' so n == NULL is
impossible. The caller has also chck ebpf_rss_fds != NULL.
IOW, this is redundant
> -return ebpf_rss_load(&n->ebpf_rss);
> +nfds = virtio_net_get_ebpf_rss_fds(n->ebpf_rss_fds,
> + fds_strs, EBPF_RSS_MAX_FDS);
Needs to pass in 'errp' and check for the error condition on return
if (nfds != EBPF_RSS_MAX_FDS)
return false;
> +for (i = 0; i < nfds; i++) {
> +fds[i] = monitor_fd_param(monitor_cur(), fds_strs[i], &errp);
if (fds[i] < 0)
close all FDs so far and return error...
> +}
> +
> +if (nfds == EBPF_RSS_MAX_FDS) {
> +ret = ebpf_rss_load_fds(&n->ebpf_rss, fds[0], fds[1], fds[2],
> fds[3]);
> +}
This condition is not required, since we should have check
it immediately after the virtio_net_get_ebpf_rss_fds() call
> +
> +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)
This needs to have "Error **errp' added to it and return an
error on failure. The caller already has an 'errp' available
to pass in.
> +{
> +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)
> @@ -3900,6 +3968,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 ef234ffe7e..e10ce88f91 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://ww
[RFC PATCH 2/5] virtio-net: Added property to load eBPF RSS with fds.
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
---
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 53e1c32643..45d448a83d 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,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)
+{
+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)
{
-if (!virtio_net_attach_ebpf_to_backend(n->nic, -1)) {
-/* backend does't support steering ebpf */
+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)
@@ -3900,6 +3968,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 ef234ffe7e..e10ce88f91 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
[RFC PATCH 2/5] virtio-net: Added property to load eBPF RSS with fds.
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
---
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;
};
void virtio_net_set_netclient_name(VirtIONet *n, const char *name,
--
2.31.1
