Re: [PATCH 3/4] app/testpmd: check queue count for related options

[David Marchand <david.marchand@redhat.com>]

On Tue, Mar 12, 2024 at 5:59 PM Ferruh Yigit <ferruh.yigit@amd.com> wrote:
>
> On 3/8/2024 2:48 PM, David Marchand wrote:
> > Checking the number of rxq/txq in the middle of option parsing is
> > confusing. Move the check where nb_rxq / nb_txq are modified.
> >
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > ---
> >  app/test-pmd/parameters.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
> > index 8c21744009..271f0c995a 100644
> > --- a/app/test-pmd/parameters.c
> > +++ b/app/test-pmd/parameters.c
> > @@ -1063,6 +1063,8 @@ launch_args_parse(int argc, char** argv)
> >                                       rte_exit(EXIT_FAILURE, "rxq %d invalid - must be"
> >                                                 " >= 0 && <= %u\n", n,
> >                                                 get_allowed_max_nb_rxq(&pid));
> > +                             if (!nb_rxq && !nb_txq)
> > +                                     rte_exit(EXIT_FAILURE, "Either rx or tx queues should be non-zero\n");
> >                       }
> >                       if (!strcmp(lgopts[opt_idx].name, "txq")) {
> >                               n = atoi(optarg);
> > @@ -1072,6 +1074,8 @@ launch_args_parse(int argc, char** argv)
> >                                       rte_exit(EXIT_FAILURE, "txq %d invalid - must be"
> >                                                 " >= 0 && <= %u\n", n,
> >                                                 get_allowed_max_nb_txq(&pid));
> > +                             if (!nb_rxq && !nb_txq)
> > +                                     rte_exit(EXIT_FAILURE, "Either rx or tx queues should be non-zero\n");
> >                       }
> >                       if (!strcmp(lgopts[opt_idx].name, "hairpinq")) {
> >                               n = atoi(optarg);
> > @@ -1098,10 +1102,6 @@ launch_args_parse(int argc, char** argv)
> >                                                 n + nb_rxq,
> >                                                 get_allowed_max_nb_rxq(&pid));
> >                       }
> > -                     if (!nb_rxq && !nb_txq) {
> > -                             rte_exit(EXIT_FAILURE, "Either rx or tx queues should "
> > -                                             "be non-zero\n");
> > -                     }
> >                       if (!strcmp(lgopts[opt_idx].name, "hairpin-mode")) {
> >                               char *end = NULL;
> >                               unsigned int n;
>
> There is already a runtime check for queues [1], perhaps we can remove
> it altogether from arg parse.

Good catch.

This other check comes after parsing args, so I suspect it is just dead code.
I guess I'll change it into a rte_exit(EXIT_FAILURE..).
Is this what you propose?


>
> Also I think the order of the 'hairpinq' and queue number parameter
> processing depends on order user provided, so this may not be very
> reliable anyway.

Indeed.


-- 
David Marchand