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é <berra...@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 <and...@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 :| >