Message ID | 20230501072101.22890-3-andrew@daynix.com |
---|---|
State | New |
Headers | show |
Series | eBPF RSS through QMP support. | expand |
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
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 :| >
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,
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(-)