Re: [PATCH bpf-next 02/10] xsk: diversify return codes in xsk_rcv_check()

Jesper Dangaard Brouer
2022-04-05T14:25:09Z

On 05/04/2022 13.06, Maciej Fijalkowski wrote:
> Inspired by patch that made xdp_do_redirect() return values for XSKMAP
> more meaningful, return -ENXIO instead of -EINVAL for socket being
> unbound in xsk_rcv_check() as this is the usual value that is returned
> for such event. In turn, it is now possible to easily distinguish what
> went wrong, which is a bit harder when for both cases checked, -EINVAL
> was returned.

I like this as it makes it easier to troubleshoot.
Could you update the description to explain how to debug this easily.
E.g. via this bpftrace one liner:

bpftrace -e 'tracepoint:xdp:xdp_redirect* {@err[-args->err] = count();}'

> Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> ---

Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>

> net/xdp/xsk.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index f75e121073e7..040c73345b7c 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c
> @@ -217,7 +217,7 @@ static bool xsk_is_bound(struct xdp_sock *xs)
> static int xsk_rcv_check(struct xdp_sock *xs, struct xdp_buff *xdp)
> {
> if (!xsk_is_bound(xs))
> - return -EINVAL;
> + return -ENXIO;
>
> if (xs->dev != xdp->rxq->dev || xs->queue_id != xdp->rxq->queue_index)
> return -EINVAL;
>