On Wed, Jun 29, 2022 at 4:39 PM Maciej Fijalkowski
<maciej.fijalkowski@xxxxxxxxx> wrote:
>
> Currently, xsk_socket__delete frees BPF resources regardless of ctx
> refcount. Xdpxceiver has a test to verify whether underlying BPF
> resources would not be wiped out after closing XSK socket that was bound
> to interface with other active sockets. From library's xsk part
> perspective it also means that the internal xsk context is shared and
> its refcount is bumped accordingly.
>
> After a switch to loading XDP prog based on previously opened XSK
> socket, mentioned xdpxceiver test fails with:
> not ok 16 [xdpxceiver.c:swap_xsk_resources:1334]: ERROR: 9/"Bad file descriptor
>
> which means that in swap_xsk_resources(), xsk_socket__delete() released
> xskmap which in turn caused a failure of xsk_socket__update_xskmap().
>
> To fix this, when deleting socket, decrement ctx refcount before
> releasing BPF resources and do so only when refcount dropped to 0 which
> means there are no more active sockets for this ctx so BPF resources can
> be freed safely.
Please fix this in libxdp too as the bug is present there also.
Acked-by: Magnus Karlsson <magnus.karlsson@xxxxxxxxx>
> Fixes: 2f6324a3937f ("libbpf: Support shared umems between queues and devices")
> Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@xxxxxxxxx>
> ---
> tools/testing/selftests/bpf/xsk.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/xsk.c b/tools/testing/selftests/bpf/xsk.c
> index db911127720e..95ce5cd572bb 100644
> --- a/tools/testing/selftests/bpf/xsk.c
> +++ b/tools/testing/selftests/bpf/xsk.c
> @@ -1156,8 +1156,6 @@ int xsk_socket__create_shared(struct xsk_socket **xsk_ptr,
> goto out_mmap_tx;
> }
>
> - ctx->prog_fd = -1;
> -
> if (!(xsk->config.libbpf_flags & XSK_LIBBPF_FLAGS__INHIBIT_PROG_LOAD)) {
> err = __xsk_setup_xdp_prog(xsk, NULL);
> if (err)
> @@ -1238,7 +1236,10 @@ void xsk_socket__delete(struct xsk_socket *xsk)
>
> ctx = xsk->ctx;
> umem = ctx->umem;
> - if (ctx->prog_fd != -1) {
> +
> + xsk_put_ctx(ctx, true);
> +
> + if (!ctx->refcount) {
> xsk_delete_bpf_maps(xsk);
> close(ctx->prog_fd);
> if (ctx->has_bpf_link)
> @@ -1257,7 +1258,6 @@ void xsk_socket__delete(struct xsk_socket *xsk)
> }
> }
>
> - xsk_put_ctx(ctx, true);
>
> umem->refcount--;
> /* Do not close an fd that also has an associated umem connected
> --
> 2.27.0
>
