Re: [dpdk-dev] [PATCH v3 1/3] app/flow-perf: support meter policy API

[Thomas Monjalon <thomas@monjalon.net>]

05/11/2021 11:24, Rongwei Liu:
> HI Tomas:
> 	Flow-perf application run-time options are growing quickly and some of them are a little complex.
> 	Under current architecture, I don't find a valid way to reduce the indention level unless maintainer agree to simply the logic.

Trust checkpatch, it is possible.
One basic solution is to have sub-functions.

Please do not top-post.

> 	For the fill_actions() function, I will rebase it once our github branch updated.
> 	It should be something like:
> 
> > > +             fill_actions(actions[i], all_actions[i], 0, 0, 0,
> > > +                          0, 0, 0, unique_data, rx_queues_count, dst_port);
> 
> BR
> Rongwei
> 
> > -----Original Message-----
> > From: Thomas Monjalon <thomas@monjalon.net>
> > Sent: Friday, November 5, 2021 5:58 PM
> > To: Wisam Monther <wisamm@nvidia.com>; Rongwei Liu
> > <rongweil@nvidia.com>
> > Cc: Matan Azrad <matan@nvidia.com>; Slava Ovsiienko
> > <viacheslavo@nvidia.com>; Ori Kam <orika@nvidia.com>; dev@dpdk.org;
> > Raslan Darawsheh <rasland@nvidia.com>; Haifei Luo <haifeil@nvidia.com>;
> > Jiawei(Jonny) Wang <jiaweiw@nvidia.com>
> > Subject: Re: [dpdk-dev] [PATCH v3 1/3] app/flow-perf: support meter policy
> > API
> > 
> > External email: Use caution opening links or attachments
> > 
> > 
> > 28/10/2021 05:25, Rongwei Liu:
> > > @@ -854,6 +862,46 @@ args_parse(int argc, char **argv)
> > >                                               RTE_MAX_LCORE);
> > >                               }
> > >                       }
> > > +                     if (strcmp(lgopts[opt_idx].name, "policy-mtr") == 0) {
> > > +                             j = 0;
> > > +                             k = 0;
> > > +                             arg = optarg;
> > > +                             policy_mtr = true;
> > > +                             token = strsep(&arg, ":\0");
> > > +                             while (token != NULL && j < RTE_COLORS) {
> > > +                                     actions_str[j++] = token;
> > > +                                     token = strsep(&arg, ":\0");
> > > +                             }
> > > +                             j = 0;
> > > +                             token = strtok(actions_str[0], ",\0");
> > > +                             while (token == NULL && j < RTE_COLORS - 1)
> > > +                                     token = strtok(actions_str[++j], ",\0");
> > > +                             while (j < RTE_COLORS && token != NULL) {
> > > +                                     for (i = 0;
> > > +                                          i < RTE_DIM(flow_options); i++) {
> > > +                                             if (!strcmp(token,
> > > +                                                 flow_options[i].str)) {
> > > +                                                     all_actions[j][k++] =
> > > +                                                     flow_options[i].mask;
> > > +                                                     break;
> > > +                                             }
> > > +                                     }
> > > +                                     /* Reached last item with no match */
> > > +                                     if (i >= RTE_DIM(flow_options)) {
> > > +                                             fprintf(stderr,
> > > +                                                     "Invalid actions "
> > > +                                                     "item: %s\n", token);
> > > +                                             usage(argv[0]);
> > > +                                             rte_exit(EXIT_SUCCESS, "Invalid actions item\n");
> > > +                                     }
> > > +                                     token = strtok(NULL, ",\0");
> > > +                                     while (!token && j < RTE_COLORS - 1) {
> > > +                                             token = strtok(actions_str[++j],
> > > +                                                             ",\0");
> > > +                                             k = 0;
> > > +                                     }
> > > +                             }
> > > +                     }
> > 
> > Could we avoid having so many indents?
> > Checkpatch suggest considering a refactoring.
> > 
> > [...]
> > > +             fill_actions(actions[i], all_actions[i], 0, 0, 0,
> > > +                          0, 0, 0, unique_data, rx_queues_count);
> > 
> > error: too few arguments to function ‘fill_actions’
> > 
> > That's a rebase issue because I merged first the patch
> > "app/flow-perf: add destination ports parameter"
> > 
> > 
> 
>