[PATCH 0/3] ebpf: Error fixes and cleanups

[PATCH 0/3] ebpf: Error fixes and cleanups

[Markus Armbruster]

Note: not cc'ed to Andrew Melnychenko <andrew@daynix.com> and Yuri
Benditovich <yuri.benditovich@daynix.com> (R: in MAINTAINERS), because
e-mail to these addresses bounces.

Markus Armbruster (3):
  ebpf: Fix stubs to set an error when they return failure
  ebpf: Clean up useless error check in ebpf_rss_set_all()
  ebpf: Make ebpf_rss_load() return value consistent with @errp

 ebpf/ebpf_rss-stub.c |  4 +++-
 ebpf/ebpf_rss.c      | 10 +++-------
 hw/net/virtio-net.c  |  4 +---
 3 files changed, 7 insertions(+), 11 deletions(-)

-- 
2.49.0

[PATCH 1/3] ebpf: Fix stubs to set an error when they return failure

[Markus Armbruster]

Stubs in ebpf_rss-stub.c return false for failure without setting an
Error.  This is wrong.  Callers may assume that the functions set an
error when they fail, and crash when they try to examine or report the
error.  Callers may also check the error instead of the return value,
and misinterpret the failure as success.

ebpf_rss_load() and ebpf_rss_load() are reachable via
virtio_net_load_ebpf().  Fix them to set an error.

ebpf_rss_set_all() is unreachable: it can only be called when the
context has an eBPF program loaded, which is impossible with eBPF
support compiled out.  Call abort() there to make that clear, and to
get rid of the latent bug.

Fixes: 00b69f1d867d (ebpf: add formal error reporting to all APIs)
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 ebpf/ebpf_rss-stub.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/ebpf/ebpf_rss-stub.c b/ebpf/ebpf_rss-stub.c
index d0e7f99fb9..11729f3d8f 100644
--- a/ebpf/ebpf_rss-stub.c
+++ b/ebpf/ebpf_rss-stub.c
@@ -25,6 +25,7 @@ bool ebpf_rss_is_loaded(struct EBPFRSSContext *ctx)
 
 bool ebpf_rss_load(struct EBPFRSSContext *ctx, Error **errp)
 {
+    error_setg(errp, "eBPF support is not compiled in");
     return false;
 }
 
@@ -32,6 +33,7 @@ bool ebpf_rss_load_fds(struct EBPFRSSContext *ctx, int program_fd,
                        int config_fd, int toeplitz_fd, int table_fd,
                        Error **errp)
 {
+    error_setg(errp, "eBPF support is not compiled in");
     return false;
 }
 
@@ -39,7 +41,7 @@ bool ebpf_rss_set_all(struct EBPFRSSContext *ctx, struct EBPFRSSConfig *config,
                       uint16_t *indirections_table, uint8_t *toeplitz_key,
                       Error **errp)
 {
-    return false;
+    abort();
 }
 
 void ebpf_rss_unload(struct EBPFRSSContext *ctx)
-- 
2.49.0

[PATCH 2/3] ebpf: Clean up useless error check in ebpf_rss_set_all()

[Markus Armbruster]

ebpf_rss_set_all() is only called when the context has an eBPF program
loaded.  Replace the dead error check with an assertion.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 ebpf/ebpf_rss.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/ebpf/ebpf_rss.c b/ebpf/ebpf_rss.c
index e793786c17..b64e9da3e3 100644
--- a/ebpf/ebpf_rss.c
+++ b/ebpf/ebpf_rss.c
@@ -247,10 +247,8 @@ bool ebpf_rss_set_all(struct EBPFRSSContext *ctx, struct EBPFRSSConfig *config,
                       uint16_t *indirections_table, uint8_t *toeplitz_key,
                       Error **errp)
 {
-    if (!ebpf_rss_is_loaded(ctx)) {
-        error_setg(errp, "eBPF program is not loaded");
-        return false;
-    }
+    g_assert(ebpf_rss_is_loaded(ctx));
+
     if (config == NULL) {
         error_setg(errp, "eBPF config table is NULL");
         return false;
-- 
2.49.0

[PATCH 3/3] ebpf: Make ebpf_rss_load() return value consistent with @errp

[Markus Armbruster]

ebpf_rss_load() returns false for failure without setting an Error
when its @ctx argument already has an eBPF program loaded.  This is
wrong.  Fortunately, it is only called @ctx has a program.  Replace
the incorrect error check by an assertion.

The return value is now obviously reliable.  Change the caller to use
it, because it's more concise.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 ebpf/ebpf_rss.c     | 4 +---
 hw/net/virtio-net.c | 4 +---
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/ebpf/ebpf_rss.c b/ebpf/ebpf_rss.c
index b64e9da3e3..926392b3c5 100644
--- a/ebpf/ebpf_rss.c
+++ b/ebpf/ebpf_rss.c
@@ -106,9 +106,7 @@ bool ebpf_rss_load(struct EBPFRSSContext *ctx, Error **errp)
 {
     struct rss_bpf *rss_bpf_ctx;
 
-    if (ebpf_rss_is_loaded(ctx)) {
-        return false;
-    }
+    g_assert(!ebpf_rss_is_loaded(ctx));
 
     rss_bpf_ctx = rss_bpf__open();
     if (rss_bpf_ctx == NULL) {
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 3b85560f6f..f5d93eb400 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1363,9 +1363,7 @@ static bool virtio_net_load_ebpf(VirtIONet *n, Error **errp)
         return virtio_net_load_ebpf_fds(n, errp);
     }
 
-    ebpf_rss_load(&n->ebpf_rss, &err);
-    /* Beware, ebpf_rss_load() can return false with @err unset */
-    if (err) {
+    if (!ebpf_rss_load(&n->ebpf_rss, &err)) {
         warn_report_err(err);
     }
     return true;
-- 
2.49.0

Re: [PATCH 1/3] ebpf: Fix stubs to set an error when they return failure

[Daniel P. Berrangé]

On Tue, Nov 18, 2025 at 04:47:16PM +0100, Markus Armbruster wrote:
> Stubs in ebpf_rss-stub.c return false for failure without setting an
> Error.  This is wrong.  Callers may assume that the functions set an
> error when they fail, and crash when they try to examine or report the
> error.  Callers may also check the error instead of the return value,
> and misinterpret the failure as success.
> 
> ebpf_rss_load() and ebpf_rss_load() are reachable via
> virtio_net_load_ebpf().  Fix them to set an error.
> 
> ebpf_rss_set_all() is unreachable: it can only be called when the
> context has an eBPF program loaded, which is impossible with eBPF
> support compiled out.  Call abort() there to make that clear, and to
> get rid of the latent bug.
> 
> Fixes: 00b69f1d867d (ebpf: add formal error reporting to all APIs)
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  ebpf/ebpf_rss-stub.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

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/3] ebpf: Clean up useless error check in ebpf_rss_set_all()

[Daniel P. Berrangé]

On Tue, Nov 18, 2025 at 04:47:17PM +0100, Markus Armbruster wrote:
> ebpf_rss_set_all() is only called when the context has an eBPF program
> loaded.  Replace the dead error check with an assertion.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  ebpf/ebpf_rss.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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 3/3] ebpf: Make ebpf_rss_load() return value consistent with @errp

[Daniel P. Berrangé]

On Tue, Nov 18, 2025 at 04:47:18PM +0100, Markus Armbruster wrote:
> ebpf_rss_load() returns false for failure without setting an Error
> when its @ctx argument already has an eBPF program loaded.  This is
> wrong.  Fortunately, it is only called @ctx has a program.  Replace
> the incorrect error check by an assertion.
> 
> The return value is now obviously reliable.  Change the caller to use
> it, because it's more concise.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  ebpf/ebpf_rss.c     | 4 +---
>  hw/net/virtio-net.c | 4 +---
>  2 files changed, 2 insertions(+), 6 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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 0/3] ebpf: Error fixes and cleanups

[Philippe Mathieu-Daudé]

On 18/11/25 16:47, Markus Armbruster wrote:

> Markus Armbruster (3):
>    ebpf: Fix stubs to set an error when they return failure
>    ebpf: Clean up useless error check in ebpf_rss_set_all()
>    ebpf: Make ebpf_rss_load() return value consistent with @errp

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

and queued, thanks!