All of lore.kernel.org
 help / color / mirror / Atom feed
From: Akihiko Odaki <akihiko.odaki@daynix.com>
To: Jason Wang <jasowang@redhat.com>,
	 Dmitry Fleytman <dmitry.fleytman@gmail.com>,
	 Sriram Yagnaraman <sriram.yagnaraman@ericsson.com>,
	 "Michael S. Tsirkin" <mst@redhat.com>,
	Luigi Rizzo <rizzo@iet.unipi.it>,
	 Giuseppe Lettieri <g.lettieri@iet.unipi.it>,
	 Vincenzo Maffione <v.maffione@gmail.com>,
	 Andrew Melnychenko <andrew@daynix.com>,
	 Yuri Benditovich <yuri.benditovich@daynix.com>
Cc: qemu-devel@nongnu.org, Akihiko Odaki <akihiko.odaki@daynix.com>
Subject: [PATCH v10 07/18] virtio-net: Do not propagate ebpf-rss-fds errors
Date: Sun, 28 Apr 2024 16:00:50 +0900	[thread overview]
Message-ID: <20240428-rss-v10-7-73cbaa91aeb6@daynix.com> (raw)
In-Reply-To: <20240428-rss-v10-0-73cbaa91aeb6@daynix.com>

Propagating ebpf-rss-fds errors has several problems.

First, it makes device realization fail and disables the fallback to the
conventional eBPF loading.

Second, it leaks memory by making device realization fail without
freeing memory already allocated.

Third, the convention is to set an error when a function returns false,
but virtio_net_load_ebpf_fds() and virtio_net_load_ebpf() returns false
without setting an error, which is confusing.

Remove the propagation to fix these problems.

Fixes: 0524ea0510a3 ("ebpf: Added eBPF initialization by fds.")
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 hw/net/virtio-net.c | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index f6112c0ac97d..8ede38aadbbe 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1329,24 +1329,22 @@ static void virtio_net_detach_epbf_rss(VirtIONet *n)
     virtio_net_attach_ebpf_to_backend(n->nic, -1);
 }
 
-static bool virtio_net_load_ebpf_fds(VirtIONet *n, Error **errp)
+static bool virtio_net_load_ebpf_fds(VirtIONet *n)
 {
     int fds[EBPF_RSS_MAX_FDS] = { [0 ... EBPF_RSS_MAX_FDS - 1] = -1};
     int ret = true;
     int i = 0;
 
-    ERRP_GUARD();
-
     if (n->nr_ebpf_rss_fds != EBPF_RSS_MAX_FDS) {
-        error_setg(errp,
-                  "Expected %d file descriptors but got %d",
-                  EBPF_RSS_MAX_FDS, n->nr_ebpf_rss_fds);
+        warn_report("Expected %d file descriptors but got %d",
+                    EBPF_RSS_MAX_FDS, n->nr_ebpf_rss_fds);
        return false;
    }
 
     for (i = 0; i < n->nr_ebpf_rss_fds; i++) {
-        fds[i] = monitor_fd_param(monitor_cur(), n->ebpf_rss_fds[i], errp);
-        if (*errp) {
+        fds[i] = monitor_fd_param(monitor_cur(), n->ebpf_rss_fds[i],
+                                  &error_warn);
+        if (fds[i] < 0) {
             ret = false;
             goto exit;
         }
@@ -1355,7 +1353,7 @@ static bool virtio_net_load_ebpf_fds(VirtIONet *n, Error **errp)
     ret = ebpf_rss_load_fds(&n->ebpf_rss, fds[0], fds[1], fds[2], fds[3]);
 
 exit:
-    if (!ret || *errp) {
+    if (!ret) {
         for (i = 0; i < n->nr_ebpf_rss_fds && fds[i] != -1; i++) {
             close(fds[i]);
         }
@@ -1364,13 +1362,12 @@ exit:
     return ret;
 }
 
-static bool virtio_net_load_ebpf(VirtIONet *n, Error **errp)
+static bool virtio_net_load_ebpf(VirtIONet *n)
 {
     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))) {
+        if (!(n->ebpf_rss_fds && virtio_net_load_ebpf_fds(n))) {
             ret = ebpf_rss_load(&n->ebpf_rss);
         }
     }
@@ -3825,7 +3822,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, errp);
+        virtio_net_load_ebpf(n);
     }
 }
 

-- 
2.44.0



  parent reply	other threads:[~2024-04-28  7:03 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-28  7:00 [PATCH v10 00/18] virtio-net RSS/hash report fixes and improvements Akihiko Odaki
2024-04-28  7:00 ` [PATCH v10 01/18] tap: Remove tap_probe_vnet_hdr_len() Akihiko Odaki
2024-04-28  7:00 ` [PATCH v10 02/18] tap: Remove qemu_using_vnet_hdr() Akihiko Odaki
2024-04-28  7:00 ` [PATCH v10 03/18] net: Move virtio-net header length assertion Akihiko Odaki
2024-04-28  7:00 ` [PATCH v10 04/18] net: Remove receive_raw() Akihiko Odaki
2024-04-28  7:00 ` [PATCH v10 05/18] tap: Call tap_receive_iov() from tap_receive() Akihiko Odaki
2024-04-28  7:00 ` [PATCH v10 06/18] tap: Shrink zeroed virtio-net header Akihiko Odaki
2024-04-28  7:00 ` Akihiko Odaki [this message]
2024-04-28  7:00 ` [PATCH v10 08/18] virtio-net: Add only one queue pair when realizing Akihiko Odaki
2024-04-28  7:00 ` [PATCH v10 09/18] virtio-net: Copy header only when necessary Akihiko Odaki
2024-04-28  7:00 ` [PATCH v10 10/18] virtio-net: Shrink header byte swapping buffer Akihiko Odaki
2024-04-28  7:00 ` [PATCH v10 11/18] virtio-net: Disable RSS on reset Akihiko Odaki
2024-04-28  7:00 ` [PATCH v10 12/18] virtio-net: Unify the logic to update NIC state for RSS Akihiko Odaki
2024-04-28  7:00 ` [PATCH v10 13/18] virtio-net: Always set populate_hash Akihiko Odaki
2024-04-28  7:00 ` [PATCH v10 14/18] virtio-net: Do not write hashes to peer buffer Akihiko Odaki
2024-04-28  7:00 ` [PATCH v10 15/18] ebpf: Fix RSS error handling Akihiko Odaki
2024-04-28  7:00 ` [PATCH v10 16/18] ebpf: Return 0 when configuration fails Akihiko Odaki
2024-04-28  7:01 ` [PATCH v10 17/18] ebpf: Refactor tun_rss_steering_prog() Akihiko Odaki
2024-04-28  7:01 ` [PATCH v10 18/18] ebpf: Add a separate target for skeleton Akihiko Odaki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240428-rss-v10-7-73cbaa91aeb6@daynix.com \
    --to=akihiko.odaki@daynix.com \
    --cc=andrew@daynix.com \
    --cc=dmitry.fleytman@gmail.com \
    --cc=g.lettieri@iet.unipi.it \
    --cc=jasowang@redhat.com \
    --cc=mst@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=rizzo@iet.unipi.it \
    --cc=sriram.yagnaraman@ericsson.com \
    --cc=v.maffione@gmail.com \
    --cc=yuri.benditovich@daynix.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.