On 7/11/25 11:44 AM, Daniel Borkmann wrote:
> While testing, it turned out that upon error in the queue creation loop,
> we never trigger the af_xdp_cleanup() handler. This is because we pass
> errp instead of a local err pointer into the various AF_XDP setup functions
> instead of a scheme like:
>
> bool fn(..., Error **errp)
> {
> Error *err = NULL;
>
> foo(arg, &err);
> if (err) {
> handle the error...
> error_propagate(errp, err);
> return false;
> }
> ...
> }
>
> The same is true for the attachment probing with bpf_xdp_query_id(). With a
> conversion into the above format, the af_xdp_cleanup() handler is called as
> expected. Note the error_propagate() handles a NULL err internally.
>
> Fixes: cb039ef3d9e3 ("net: add initial support for AF_XDP network backend")
> Signed-off-by: Daniel Borkmann <dan...@iogearbox.net>
> Cc: Ilya Maximets <i.maxim...@ovn.org>
> Cc: Jason Wang <jasow...@redhat.com>
> Cc: Anton Protopopov <as...@isovalent.com>
> ---
> net/af-xdp.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/net/af-xdp.c b/net/af-xdp.c
> index c5d3b6a953..1692efe9f2 100644
> --- a/net/af-xdp.c
> +++ b/net/af-xdp.c
> @@ -482,9 +482,8 @@ int net_init_af_xdp(const Netdev *netdev,
> pstrcpy(s->ifname, sizeof(s->ifname), opts->ifname);
> s->ifindex = ifindex;
>
> - if (af_xdp_umem_create(s, sock_fds ? sock_fds[i] : -1, errp)
> - || af_xdp_socket_create(s, opts, errp)) {
> - error_propagate(errp, err);
> + if (af_xdp_umem_create(s, sock_fds ? sock_fds[i] : -1, &err) ||
> + af_xdp_socket_create(s, opts, &err)) {
> goto err;
> }
> }
> @@ -492,7 +491,7 @@ int net_init_af_xdp(const Netdev *netdev,
> if (nc0) {
> s = DO_UPCAST(AFXDPState, nc, nc0);
> if (bpf_xdp_query_id(s->ifindex, s->xdp_flags, &prog_id) ||
> !prog_id) {
> - error_setg_errno(errp, errno,
> + error_setg_errno(&err, errno,
> "no XDP program loaded on '%s', ifindex: %d",
> s->ifname, s->ifindex);
> goto err;
> @@ -506,6 +505,7 @@ int net_init_af_xdp(const Netdev *netdev,
> err:
> if (nc0) {
> qemu_del_net_client(nc0);
> + error_propagate(errp, err);
> }
>
> return -1;
Reviewed-by: Ilya Maximets <i.maxim...@ovn.org>
