| Message ID | 20240912171251.937743-2-sdf@fomichev.me (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | selftests: ncdevmem: Add ncdevmem to ksft | expand |
On Thu, Sep 12, 2024 at 10:12 AM Stanislav Fomichev <sdf@fomichev.me> wrote: > > And rename it to 'probing'. This is gonna be used in the selftests > to probe devmem functionality. > > Cc: Mina Almasry <almasrymina@google.com> > Signed-off-by: Stanislav Fomichev <sdf@fomichev.me> > --- > tools/testing/selftests/net/ncdevmem.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/tools/testing/selftests/net/ncdevmem.c b/tools/testing/selftests/net/ncdevmem.c > index 64d6805381c5..352dba211fb0 100644 > --- a/tools/testing/selftests/net/ncdevmem.c > +++ b/tools/testing/selftests/net/ncdevmem.c > @@ -523,8 +523,9 @@ void run_devmem_tests(void) > int main(int argc, char *argv[]) > { > int is_server = 0, opt; > + int probe = 0; > > - while ((opt = getopt(argc, argv, "ls:c:p:v:q:t:f:")) != -1) { > + while ((opt = getopt(argc, argv, "ls:c:p:v:q:t:f:P")) != -1) { > switch (opt) { > case 'l': > is_server = 1; > @@ -550,6 +551,9 @@ int main(int argc, char *argv[]) > case 'f': > ifname = optarg; > break; > + case 'P': > + probe = 1; > + break; > case '?': > printf("unknown option: %c\n", optopt); > break; > @@ -561,7 +565,10 @@ int main(int argc, char *argv[]) > for (; optind < argc; optind++) > printf("extra arguments: %s\n", argv[optind]); > > - run_devmem_tests(); > + if (probe) { > + run_devmem_tests(); > + return 0; > + } > Before this change: ./ncdevmem (runs run_devmem_tests() and exits) ./ncdevmem -l: runs devmem tests and listens And I plan to add, for the tx path: ./ncdevmem -c: runs devmem tests and does a devmem client. After this change, running ncdevmem with no flags just exits without doing anything; a bit weird IMO, but I'm not opposed if you see an upside. Is your intention with this change to not run the devmem tests on listen? Maybe something like: if (is_server) return do_server(); else if (is_client) /* to be added */ return do_client(); else run_devmem_tests(); return 0; ? But, I'm not totally opposed if you see an upside. Maybe use -p instead of -P for consistency. -- Thanks, Mina
On 09/12, Mina Almasry wrote: > On Thu, Sep 12, 2024 at 10:12 AM Stanislav Fomichev <sdf@fomichev.me> wrote: > > > > And rename it to 'probing'. This is gonna be used in the selftests > > to probe devmem functionality. > > > > Cc: Mina Almasry <almasrymina@google.com> > > Signed-off-by: Stanislav Fomichev <sdf@fomichev.me> > > --- > > tools/testing/selftests/net/ncdevmem.c | 11 +++++++++-- > > 1 file changed, 9 insertions(+), 2 deletions(-) > > > > diff --git a/tools/testing/selftests/net/ncdevmem.c b/tools/testing/selftests/net/ncdevmem.c > > index 64d6805381c5..352dba211fb0 100644 > > --- a/tools/testing/selftests/net/ncdevmem.c > > +++ b/tools/testing/selftests/net/ncdevmem.c > > @@ -523,8 +523,9 @@ void run_devmem_tests(void) > > int main(int argc, char *argv[]) > > { > > int is_server = 0, opt; > > + int probe = 0; > > > > - while ((opt = getopt(argc, argv, "ls:c:p:v:q:t:f:")) != -1) { > > + while ((opt = getopt(argc, argv, "ls:c:p:v:q:t:f:P")) != -1) { > > switch (opt) { > > case 'l': > > is_server = 1; > > @@ -550,6 +551,9 @@ int main(int argc, char *argv[]) > > case 'f': > > ifname = optarg; > > break; > > + case 'P': > > + probe = 1; > > + break; > > case '?': > > printf("unknown option: %c\n", optopt); > > break; > > @@ -561,7 +565,10 @@ int main(int argc, char *argv[]) > > for (; optind < argc; optind++) > > printf("extra arguments: %s\n", argv[optind]); > > > > - run_devmem_tests(); > > + if (probe) { > > + run_devmem_tests(); > > + return 0; > > + } > > > > Before this change: > ./ncdevmem (runs run_devmem_tests() and exits) > ./ncdevmem -l: runs devmem tests and listens > > And I plan to add, for the tx path: > > ./ncdevmem -c: runs devmem tests and does a devmem client. > > After this change, running ncdevmem with no flags just exits without > doing anything; a bit weird IMO, but I'm not opposed if you see an > upside. > > Is your intention with this change to not run the devmem tests on > listen? Maybe something like: > > if (is_server) > return do_server(); > else if (is_client) /* to be added */ > return do_client(); > else > run_devmem_tests(); > > return 0; > > ? SG! That's even better without the extra flag.
diff --git a/tools/testing/selftests/net/ncdevmem.c b/tools/testing/selftests/net/ncdevmem.c index 64d6805381c5..352dba211fb0 100644 --- a/tools/testing/selftests/net/ncdevmem.c +++ b/tools/testing/selftests/net/ncdevmem.c @@ -523,8 +523,9 @@ void run_devmem_tests(void) int main(int argc, char *argv[]) { int is_server = 0, opt; + int probe = 0; - while ((opt = getopt(argc, argv, "ls:c:p:v:q:t:f:")) != -1) { + while ((opt = getopt(argc, argv, "ls:c:p:v:q:t:f:P")) != -1) { switch (opt) { case 'l': is_server = 1; @@ -550,6 +551,9 @@ int main(int argc, char *argv[]) case 'f': ifname = optarg; break; + case 'P': + probe = 1; + break; case '?': printf("unknown option: %c\n", optopt); break; @@ -561,7 +565,10 @@ int main(int argc, char *argv[]) for (; optind < argc; optind++) printf("extra arguments: %s\n", argv[optind]); - run_devmem_tests(); + if (probe) { + run_devmem_tests(); + return 0; + } if (is_server) return do_server();
And rename it to 'probing'. This is gonna be used in the selftests to probe devmem functionality. Cc: Mina Almasry <almasrymina@google.com> Signed-off-by: Stanislav Fomichev <sdf@fomichev.me> --- tools/testing/selftests/net/ncdevmem.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)