| Message ID | 20220405110631.404427-3-maciej.fijalkowski@intel.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | xsk: stop softirq processing on full XSK Rx queue | expand |
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; >
On Tue, Apr 05, 2022 at 03:00:25PM +0200, Jesper Dangaard Brouer wrote: > > > 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();}' Nice one! I'll include this in the commit message of v2. > > > > 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; > > >
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;
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. Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> --- net/xdp/xsk.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)