Re: [PATCH 3/5] ebpf: add formal error reporting to all APIs

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 6/8/24 16:56, Daniel P. Berrangé wrote:
> The eBPF code is currently reporting error messages through trace
> events. Trace events are fine for debugging, but they are not to be
> considered the primary error reporting mechanism, as their output
> is inaccessible to callers.
> 
> This adds an "Error **errp" parameter to all methods which have
> important error scenarios to report to the caller.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   ebpf/ebpf_rss.c     | 59 ++++++++++++++++++++++++++++++++++++---------
>   ebpf/ebpf_rss.h     | 10 +++++---
>   hw/net/virtio-net.c |  7 +++---
>   3 files changed, 59 insertions(+), 17 deletions(-)
> 
> diff --git a/ebpf/ebpf_rss.c b/ebpf/ebpf_rss.c
> index aa7170d997..59854c8b51 100644
> --- a/ebpf/ebpf_rss.c
> +++ b/ebpf/ebpf_rss.c
> @@ -47,13 +47,14 @@ bool ebpf_rss_is_loaded(struct EBPFRSSContext *ctx)
>       return ctx != NULL && (ctx->obj != NULL || ctx->program_fd != -1);
>   }
>   
> -static bool ebpf_rss_mmap(struct EBPFRSSContext *ctx)
> +static bool ebpf_rss_mmap(struct EBPFRSSContext *ctx, Error **errp)
>   {
>       ctx->mmap_configuration = mmap(NULL, qemu_real_host_page_size(),
>                                      PROT_READ | PROT_WRITE, MAP_SHARED,
>                                      ctx->map_configuration, 0);
>       if (ctx->mmap_configuration == MAP_FAILED) {
>           trace_ebpf_error("eBPF RSS", "can not mmap eBPF configuration array");
> +        error_setg(errp, "Unable to map eBPF configuration array");
>           return false;
>       }
>       ctx->mmap_toeplitz_key = mmap(NULL, qemu_real_host_page_size(),
> @@ -61,6 +62,7 @@ static bool ebpf_rss_mmap(struct EBPFRSSContext *ctx)
>                                      ctx->map_toeplitz_key, 0);
>       if (ctx->mmap_toeplitz_key == MAP_FAILED) {
>           trace_ebpf_error("eBPF RSS", "can not mmap eBPF toeplitz key");
> +        error_setg(errp, "Unable to map eBPF toeplitz array");
>           goto toeplitz_fail;
>       }
>       ctx->mmap_indirections_table = mmap(NULL, qemu_real_host_page_size(),
> @@ -68,6 +70,7 @@ static bool ebpf_rss_mmap(struct EBPFRSSContext *ctx)
>                                      ctx->map_indirections_table, 0);
>       if (ctx->mmap_indirections_table == MAP_FAILED) {
>           trace_ebpf_error("eBPF RSS", "can not mmap eBPF indirection table");
> +        error_setg(errp, "Unable to map eBPF indirection array");

Aren't these trace_ebpf_error() calls redundant now?

>           goto indirection_fail;
>       }