[bpf-next,v3,04/15] selftests/xsk: print correct error codes when exiting

Commit Message

Magnus Karlsson Jan. 11, 2023, 9:35 a.m. UTC

From: Magnus Karlsson <[e-mail]>

Print the correct error codes when exiting the test suite due to some
terminal error. Some of these had a switched sign and some of them
printed zero instead of errno.

Fixes: facb7cb2e909 ("selftests/bpf: Xsk selftests - SKB POLL, NOPOLL")
Signed-off-by: Magnus Karlsson <[e-mail]>
Acked-by: Maciej Fijalkowski <[e-mail]>
---
 tools/testing/selftests/bpf/xskxceiver.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

Comments

Alexei Starovoitov Jan. 12, 2023, 2:23 a.m. UTC | #1

On Wed, Jan 11, 2023 at 10:35:15AM +0100, Magnus Karlsson wrote:
> -						exit_with_error(-ret);
> +						exit_with_error(errno);
...
> @@ -1323,18 +1323,18 @@ static void thread_common_ops(struct test_spec *test, struct ifobject *ifobject)
>  	if (ifobject->xdp_flags & XDP_FLAGS_SKB_MODE) {
>  		if (opts.attach_mode != XDP_ATTACHED_SKB) {
>  			ksft_print_msg("ERROR: [%s] XDP prog not in SKB mode\n");
> -			exit_with_error(-EINVAL);
> +			exit_with_error(EINVAL);

My understanding is that you want exit_with_error() to always see a positive error, right?
Have you considered doing something like:
#define exit_with_error(error) ({\
  if (__builtin_constant_p(error) && error < 0) // build error;
  __exit_with_error(error, __FILE__, __func__, __LINE__);
})
would it help to catch some of these issues?

Magnus Karlsson Jan. 12, 2023, 9:29 a.m. UTC | #2

On Thu, Jan 12, 2023 at 3:23 AM Alexei Starovoitov
<[e-mail]> wrote:
>
> On Wed, Jan 11, 2023 at 10:35:15AM +0100, Magnus Karlsson wrote:
> > -                                             exit_with_error(-ret);
> > +                                             exit_with_error(errno);
> ...
> > @@ -1323,18 +1323,18 @@ static void thread_common_ops(struct test_spec *test, struct ifobject *ifobject)
> >       if (ifobject->xdp_flags & XDP_FLAGS_SKB_MODE) {
> >               if (opts.attach_mode != XDP_ATTACHED_SKB) {
> >                       ksft_print_msg("ERROR: [%s] XDP prog not in SKB mode\n");
> > -                     exit_with_error(-EINVAL);
> > +                     exit_with_error(EINVAL);
>
> My understanding is that you want exit_with_error() to always see a positive error, right?
> Have you considered doing something like:
> #define exit_with_error(error) ({\
>   if (__builtin_constant_p(error) && error < 0) // build error;
>   __exit_with_error(error, __FILE__, __func__, __LINE__);
> })
> would it help to catch some of these issues?

Yes it would. Will add this to the next series. Thanks!

Patch

diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
index a239e975ab66..72578cebfbf7 100644
--- a/tools/testing/selftests/bpf/xskxceiver.c
+++ b/tools/testing/selftests/bpf/xskxceiver.c
@@ -350,7 +350,7 @@  static bool ifobj_zc_avail(struct ifobject *ifobject)
 	umem = calloc(1, sizeof(struct xsk_umem_info));
 	if (!umem) {
 		munmap(bufs, umem_sz);
-		exit_with_error(-ENOMEM);
+		exit_with_error(ENOMEM);
 	}
 	umem->frame_size = XSK_UMEM__DEFAULT_FRAME_SIZE;
 	ret = xsk_configure_umem(umem, bufs, umem_sz);
@@ -936,7 +936,7 @@  static int receive_pkts(struct test_spec *test, struct pollfd *fds)
 		if (ifobj->use_poll) {
 			ret = poll(fds, 1, POLL_TMOUT);
 			if (ret < 0)
-				exit_with_error(-ret);
+				exit_with_error(errno);
 
 			if (!ret) {
 				if (!is_umem_valid(test->ifobj_tx))
@@ -963,7 +963,7 @@  static int receive_pkts(struct test_spec *test, struct pollfd *fds)
 				if (xsk_ring_prod__needs_wakeup(&umem->fq)) {
 					ret = poll(fds, 1, POLL_TMOUT);
 					if (ret < 0)
-						exit_with_error(-ret);
+						exit_with_error(errno);
 				}
 				ret = xsk_ring_prod__reserve(&umem->fq, rcvd, &idx_fq);
 			}
@@ -1015,7 +1015,7 @@  static int __send_pkts(struct ifobject *ifobject, u32 *pkt_nb, struct pollfd *fd
 			if (timeout) {
 				if (ret < 0) {
 					ksft_print_msg("ERROR: [%s] Poll error %d\n",
-						       __func__, ret);
+						       __func__, errno);
 					return TEST_FAILURE;
 				}
 				if (ret == 0)
@@ -1024,7 +1024,7 @@  static int __send_pkts(struct ifobject *ifobject, u32 *pkt_nb, struct pollfd *fd
 			}
 			if (ret <= 0) {
 				ksft_print_msg("ERROR: [%s] Poll error %d\n",
-					       __func__, ret);
+					       __func__, errno);
 				return TEST_FAILURE;
 			}
 		}
@@ -1323,18 +1323,18 @@  static void thread_common_ops(struct test_spec *test, struct ifobject *ifobject)
 	if (ifobject->xdp_flags & XDP_FLAGS_SKB_MODE) {
 		if (opts.attach_mode != XDP_ATTACHED_SKB) {
 			ksft_print_msg("ERROR: [%s] XDP prog not in SKB mode\n");
-			exit_with_error(-EINVAL);
+			exit_with_error(EINVAL);
 		}
 	} else if (ifobject->xdp_flags & XDP_FLAGS_DRV_MODE) {
 		if (opts.attach_mode != XDP_ATTACHED_DRV) {
 			ksft_print_msg("ERROR: [%s] XDP prog not in DRV mode\n");
-			exit_with_error(-EINVAL);
+			exit_with_error(EINVAL);
 		}
 	}
 
 	ret = xsk_socket__update_xskmap(ifobject->xsk->xsk, ifobject->xsk_map_fd);
 	if (ret)
-		exit_with_error(-ret);
+		exit_with_error(errno);
 }
 
 static void *worker_testapp_validate_tx(void *arg)
@@ -1541,7 +1541,7 @@  static void swap_xsk_resources(struct ifobject *ifobj_tx, struct ifobject *ifobj
 
 	ret = xsk_socket__update_xskmap(ifobj_rx->xsk->xsk, ifobj_rx->xsk_map_fd);
 	if (ret)
-		exit_with_error(-ret);
+		exit_with_error(errno);
 }
 
 static void testapp_bpf_res(struct test_spec *test)