Re: [PATCH net-next 01/13] selftests: ncdevmem: Add a flag for the selftest

[Stanislav Fomichev <stfomichev@gmail.com>]

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.