netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: Joe Damato <jdamato@fastly.com>
Cc: davem@davemloft.net, netdev@vger.kernel.org, edumazet@google.com,
	pabeni@redhat.com, andrew+netdev@lunn.ch, horms@kernel.org,
	shuah@kernel.org, hawk@kernel.org, petrm@nvidia.com,
	willemdebruijn.kernel@gmail.com
Subject: Re: [PATCH net-next 2/4] selftests: drv-net: add a way to wait for a local process
Date: Tue, 18 Feb 2025 15:05:12 -0800	[thread overview]
Message-ID: <20250218150512.282c94eb@kernel.org> (raw)
In-Reply-To: <Z7UBJ_CIrvsSdmnt@LQ3V64L9R2>

On Tue, 18 Feb 2025 16:52:39 -0500 Joe Damato wrote:
> Removing this check causes a stack trace on my XDP-disabled kernel,
> whereas with the existing code it caused a skip.
> 
> Maybe that's OK, though?
> 
> The issue is that xdp_helper.c fails and exits with return -1 before
> the call to ksft_ready() which results in the following:
> 
> # Exception| Traceback (most recent call last):
> # Exception|   File "/home/jdamato/code/net-next/tools/testing/selftests/net/lib/py/ksft.py", line 223, in ksft_run
> # Exception|     case(*args)
> # Exception|   File "/home/jdamato/code/net-next/./tools/testing/selftests/drivers/net/queues.py", line 27, in check_xsk
> # Exception|     with bkg(f'{cfg.rpath("xdp_helper")} {cfg.ifindex} {xdp_queue_id}',
> # Exception|          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> # Exception|   File "/home/jdamato/code/net-next/tools/testing/selftests/net/lib/py/utils.py", line 108, in __init__
> # Exception|     super().__init__(comm, background=True,
> # Exception|   File "/home/jdamato/code/net-next/tools/testing/selftests/net/lib/py/utils.py", line 63, in __init__
> # Exception|     raise Exception("Did not receive ready message")
> # Exception| Exception: Did not receive ready message
> not ok 4 queues.check_xsk
> # Totals: pass:3 fail:1 xfail:0 xpass:0 skip:0 error:0
> 
> I had originally modified the test so that if XDP is disabled in the
> kernel it would skip, but I think you mentioned in a previous thread
> that this was a "non-goal", IIRC ?
> 
> No strong opinion on my side as to what the behavior should be when
> XDP is disabled, but wanted to mention this so that the behavior
> change was known.

I thought of doing this:

diff --git a/tools/testing/selftests/drivers/net/xdp_helper.c b/tools/testing/selftests/drivers/net/xdp_helper.c
index 8f77da4f798f..8c34e8915fc4 100644
--- a/tools/testing/selftests/drivers/net/xdp_helper.c
+++ b/tools/testing/selftests/drivers/net/xdp_helper.c
@@ -53,7 +53,7 @@ int main(int argc, char **argv)
        int queue;
        char byte;
 
-       if (argc != 3) {
+       if (argc > 1 && argc != 3) {
                fprintf(stderr, "Usage: %s ifindex queue_id", argv[0]);
                return 1;
        }
@@ -69,6 +69,13 @@ int main(int argc, char **argv)
                return 1;
        }
 
+
+       if (argc == 1) {
+               printf("AF_XDP support detected\n");
+               close(sock_fd);
+               return 0;
+       }
+
        ifindex = atoi(argv[1]);
        queue = atoi(argv[2]);
 

Then we can run the helper with no arguments, just to check if af_xdp
is supported. If that returns 0 we go on, otherwise we print your nice
error.

LMK if that sounds good, assuming a respin is needed I can add that :)

> Separately: I retested this on a machine with XDP enabled, both with
> and without NETIF set and the test seems to hang because the helper
> is blocked on:
> 
> read(STDIN_FILENO, &byte, 1);
> 
> according to strace:
> 
> strace: Process 14198 attached
> 21:50:02 read(0,
> 
> So, I think this patch needs to be tweaked to write a byte to the
> helper so it exits (I assume before the defer was killing it?) or
> the helper needs to be modified in way?

What Python version do you have? 

For me the xdp process doesn't wait at all. Running this under vng 
and Python 3.13 the read returns 0 immediately.

Even if it doesn't we run bkg() with default params, so exit_wait=False
init will set:

	self.terminate = not exit_wait

and then __exit__ will do:

	return self.process(terminate=self.terminate, fail=self.check_fail)

which does:

	if self.terminate:
		self.proc.terminate()

so the helper should get a SIGINT, no?

We shall find out if NIPA agrees with my local system at 4p.

  reply	other threads:[~2025-02-18 23:05 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-18 19:50 [PATCH net-next 0/4] selftests: drv-net: improve the queue test for XSK Jakub Kicinski
2025-02-18 19:50 ` [PATCH net-next 1/4] selftests: drv-net: use cfg.rpath() in netlink xsk attr test Jakub Kicinski
2025-02-18 21:24   ` Joe Damato
2025-02-18 19:50 ` [PATCH net-next 2/4] selftests: drv-net: add a way to wait for a local process Jakub Kicinski
2025-02-18 21:10   ` Stanislav Fomichev
2025-02-18 21:21     ` Jakub Kicinski
2025-02-18 21:29       ` Stanislav Fomichev
2025-02-18 21:52   ` Joe Damato
2025-02-18 23:05     ` Jakub Kicinski [this message]
2025-02-19  1:37       ` Jakub Kicinski
2025-02-19 18:40         ` Joe Damato
2025-02-19 18:39       ` Joe Damato
2025-02-19 22:48         ` Jakub Kicinski
2025-02-20 17:45           ` Joe Damato
2025-02-18 19:50 ` [PATCH net-next 3/4] selftests: drv-net: improve the use of ksft helpers in XSK queue test Jakub Kicinski
2025-02-18 21:25   ` Joe Damato
2025-02-18 19:50 ` [PATCH net-next 4/4] selftests: drv-net: rename queues check_xdp to check_xsk Jakub Kicinski
2025-02-18 21:25   ` Joe Damato
2025-02-18 21:29 ` [PATCH net-next 0/4] selftests: drv-net: improve the queue test for XSK Stanislav Fomichev
2025-02-18 21:55 ` Joe Damato

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250218150512.282c94eb@kernel.org \
    --to=kuba@kernel.org \
    --cc=andrew+netdev@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=hawk@kernel.org \
    --cc=horms@kernel.org \
    --cc=jdamato@fastly.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=petrm@nvidia.com \
    --cc=shuah@kernel.org \
    --cc=willemdebruijn.kernel@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).