| Message ID | 20240930171753.2572922-11-sdf@fomichev.me (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | selftests: ncdevmem: Add ncdevmem to ksft | expand |
On Mon, Sep 30, 2024 at 10:18 AM Stanislav Fomichev <sdf@fomichev.me> wrote: > > This will be used as a 'probe' mode in the selftest to check whether > the device supports the devmem or not. It's not really 'probing'. Running ncdevmem with -s or -c does the data path tests. Running ncdevmem without does all the control path tests in run_devmem_tests(). It's not just probing driver for support, it's mean to catch bugs. Maybe rename to 'control path tests' or 'config tests' instead of probing. > Use hard-coded queue layout > (two last queues) and prevent user from passing custom -q and/or -t. > > Cc: Mina Almasry <almasrymina@google.com> > Signed-off-by: Stanislav Fomichev <sdf@fomichev.me> > --- > tools/testing/selftests/net/ncdevmem.c | 27 +++++++++++++++++++------- > 1 file changed, 20 insertions(+), 7 deletions(-) > > diff --git a/tools/testing/selftests/net/ncdevmem.c b/tools/testing/selftests/net/ncdevmem.c > index 900a661a61af..9b0a81b12eac 100644 > --- a/tools/testing/selftests/net/ncdevmem.c > +++ b/tools/testing/selftests/net/ncdevmem.c > @@ -688,17 +688,26 @@ int main(int argc, char *argv[]) > } > } > > - if (!server_ip) > - error(1, 0, "Missing -s argument\n"); > - > - if (!port) > - error(1, 0, "Missing -p argument\n"); > - > if (!ifname) > error(1, 0, "Missing -f argument\n"); > > ifindex = if_nametoindex(ifname); > > + if (!server_ip && !client_ip) { > + if (start_queue != -1) > + error(1, 0, "don't support custom start queue for probing\n"); > + if (num_queues != 1) > + error(1, 0, "don't support custom number of queues for probing\n"); > + Remove the start_queue + num_queues check here please. I would like to be able to run the control path tests binding dmabuf to all the queues or 1 of the queues or some of the queues.
On 10/03, Mina Almasry wrote: > On Mon, Sep 30, 2024 at 10:18 AM Stanislav Fomichev <sdf@fomichev.me> wrote: > > > > This will be used as a 'probe' mode in the selftest to check whether > > the device supports the devmem or not. > > It's not really 'probing'. Running ncdevmem with -s or -c does the > data path tests. Running ncdevmem without does all the control path > tests in run_devmem_tests(). It's not just probing driver for support, > it's mean to catch bugs. Maybe rename to 'control path tests' or > 'config tests' instead of probing. Re 'probing' vs 'control path tests': I need something I can call in the python selftest to tell me whether the nic supports devmem or not (to skip the tests on the unsupported nics), so I'm reusing this 'control path tests' mode for this. I do agree that there might be an issue where the nic supports devmem, but has some bug which causes 'control path tests' to fail which leads to skipping the data plane tests... We can try to separate these two in the future. (and I'll keep the word 'probing' for now since it's only in the commit message to describe the intent) > > Use hard-coded queue layout > > (two last queues) and prevent user from passing custom -q and/or -t. > > > > Cc: Mina Almasry <almasrymina@google.com> > > Signed-off-by: Stanislav Fomichev <sdf@fomichev.me> > > --- > > tools/testing/selftests/net/ncdevmem.c | 27 +++++++++++++++++++------- > > 1 file changed, 20 insertions(+), 7 deletions(-) > > > > diff --git a/tools/testing/selftests/net/ncdevmem.c b/tools/testing/selftests/net/ncdevmem.c > > index 900a661a61af..9b0a81b12eac 100644 > > --- a/tools/testing/selftests/net/ncdevmem.c > > +++ b/tools/testing/selftests/net/ncdevmem.c > > @@ -688,17 +688,26 @@ int main(int argc, char *argv[]) > > } > > } > > > > - if (!server_ip) > > - error(1, 0, "Missing -s argument\n"); > > - > > - if (!port) > > - error(1, 0, "Missing -p argument\n"); > > - > > if (!ifname) > > error(1, 0, "Missing -f argument\n"); > > > > ifindex = if_nametoindex(ifname); > > > > + if (!server_ip && !client_ip) { > > + if (start_queue != -1) > > + error(1, 0, "don't support custom start queue for probing\n"); > > + if (num_queues != 1) > > + error(1, 0, "don't support custom number of queues for probing\n"); > > + > > Remove the start_queue + num_queues check here please. I would like to > be able to run the control path tests binding dmabuf to all the queues > or 1 of the queues or some of the queues. Will remove, but let's continue discussing this in the other thread.
On Thu, Oct 3, 2024 at 10:18 AM Stanislav Fomichev <stfomichev@gmail.com> wrote: > > On 10/03, Mina Almasry wrote: > > On Mon, Sep 30, 2024 at 10:18 AM Stanislav Fomichev <sdf@fomichev.me> wrote: > > > > > > This will be used as a 'probe' mode in the selftest to check whether > > > the device supports the devmem or not. > > > > It's not really 'probing'. Running ncdevmem with -s or -c does the > > data path tests. Running ncdevmem without does all the control path > > tests in run_devmem_tests(). It's not just probing driver for support, > > it's mean to catch bugs. Maybe rename to 'control path tests' or > > 'config tests' instead of probing. > > Re 'probing' vs 'control path tests': I need something I can call > in the python selftest to tell me whether the nic supports devmem > or not (to skip the tests on the unsupported nics), so I'm reusing this > 'control path tests' mode for this. I do agree that there might be an > issue where the nic supports devmem, but has some bug which causes > 'control path tests' to fail which leads to skipping the data plane tests... > > We can try to separate these two in the future. (and I'll keep the word > 'probing' for now since it's only in the commit message to describe the > intent) Ah, got it. Thanks for the explanation. Complete probing should call the binding API and assert that the return is EOPNOTSUPP or something like that. Other errors in the control path tests should be considered failures and not test skips. But probing is necessary to run this test and this is a good way to do it for now. We can improve later if necessary. Thanks!
diff --git a/tools/testing/selftests/net/ncdevmem.c b/tools/testing/selftests/net/ncdevmem.c index 900a661a61af..9b0a81b12eac 100644 --- a/tools/testing/selftests/net/ncdevmem.c +++ b/tools/testing/selftests/net/ncdevmem.c @@ -688,17 +688,26 @@ int main(int argc, char *argv[]) } } - if (!server_ip) - error(1, 0, "Missing -s argument\n"); - - if (!port) - error(1, 0, "Missing -p argument\n"); - if (!ifname) error(1, 0, "Missing -f argument\n"); ifindex = if_nametoindex(ifname); + if (!server_ip && !client_ip) { + if (start_queue != -1) + error(1, 0, "don't support custom start queue for probing\n"); + if (num_queues != 1) + error(1, 0, "don't support custom number of queues for probing\n"); + + start_queue = rxq_num(ifindex) - 2; + if (start_queue < 0) + error(1, 0, "couldn't detect number of queues\n"); + num_queues = 2; /* make sure can bind to multiple queues */ + + run_devmem_tests(); + return 0; + } + if (start_queue < 0) { start_queue = rxq_num(ifindex) - 1; @@ -711,7 +720,11 @@ int main(int argc, char *argv[]) for (; optind < argc; optind++) fprintf(stderr, "extra arguments: %s\n", argv[optind]); - run_devmem_tests(); + if (!server_ip) + error(1, 0, "Missing -s argument\n"); + + if (!port) + error(1, 0, "Missing -p argument\n"); mem = provider->alloc(getpagesize() * NUM_PAGES); ret = is_server ? do_server(mem) : 1;
This will be used as a 'probe' mode in the selftest to check whether the device supports the devmem or not. Use hard-coded queue layout (two last queues) and prevent user from passing custom -q and/or -t. Cc: Mina Almasry <almasrymina@google.com> Signed-off-by: Stanislav Fomichev <sdf@fomichev.me> --- tools/testing/selftests/net/ncdevmem.c | 27 +++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-)