I agree. I can make the changes.

On 30 July 2015 at 12:58, Stuart Haslam <stuart.has...@linaro.org> wrote:

> On Thu, Jul 30, 2015 at 10:02:46AM +0200, Balakrishna.Garapati wrote:
> > Signed-off-by: Balakrishna.Garapati <balakrishna.garap...@linaro.org>
> > ---
> >  This patch also includes changes to enable the option to support number
> of workers.
> >  example/generator/odp_generator.c | 35
> +++++++++++++++++++++++++++++------
> >  1 file changed, 29 insertions(+), 6 deletions(-)
> >
> > diff --git a/example/generator/odp_generator.c
> b/example/generator/odp_generator.c
> > index d6ec758..08cd577 100644
> > --- a/example/generator/odp_generator.c
> > +++ b/example/generator/odp_generator.c
> > @@ -42,6 +42,7 @@
> >   */
> >  typedef struct {
> >       int cpu_count;          /**< system CPU count */
> > +     const char *mask;       /**< core mask */
> >       int if_count;           /**< Number of interfaces to be used */
> >       char **if_names;        /**< Array of pointers to interface names
> */
> >       char *if_str;           /**< Storage for interface names */
> > @@ -633,18 +634,27 @@ int main(int argc, char *argv[])
> >       if (args->appl.cpu_count)
> >               num_workers = args->appl.cpu_count;
> >
> > -     /* ping mode need two worker */
> > -     if (args->appl.mode == APPL_MODE_PING)
> > -             num_workers = 2;
> > +     if (args->appl.mask) {
> > +             (void)odp_cpumask_from_str(&cpumask, args->appl.mask);
> > +             num_workers = odp_cpumask_count(&cpumask);
> > +     } else {
> > +             num_workers = odp_cpumask_def_worker(&cpumask,
> num_workers);
> > +     }
> >
> > -     /* Get default worker cpumask */
> > -     num_workers = odp_cpumask_def_worker(&cpumask, num_workers);
> >       (void)odp_cpumask_to_str(&cpumask, cpumaskstr, sizeof(cpumaskstr));
> >
> >       printf("num worker threads: %i\n", num_workers);
> >       printf("first CPU:          %i\n", odp_cpumask_first(&cpumask));
> >       printf("cpu mask:           %s\n", cpumaskstr);
> >
> > +     /* ping mode need two workers */
> > +     if (args->appl.mode == APPL_MODE_PING) {
> > +             if (num_workers < 2) {
> > +                     EXAMPLE_ERR("Need at least two worker threads\n");
> > +                     exit(EXIT_FAILURE);
> > +             }
> > +     }
> > +
> >       /* Create packet pool */
> >       memset(&params, 0, sizeof(params));
> >       params.pkt.seg_len = SHM_PKT_POOL_BUF_SIZE;
> > @@ -812,6 +822,7 @@ static void parse_args(int argc, char *argv[],
> appl_args_t *appl_args)
> >       static struct option longopts[] = {
> >               {"interface", required_argument, NULL, 'I'},
> >               {"workers", required_argument, NULL, 'w'},
> > +             {"coremask", required_argument, NULL, 'k'},
>
> It would be better to use -c for coremask, on the assumption that we'll
> need to add the same to other examples and it's much more intuitive. I
> see that will mean needing to change the srcip short name, but it's
> fairly arbitrary as it is (a, b, c, d) and everyone seems to use the
> long name anyway.
>
> >               {"srcmac", required_argument, NULL, 'a'},
> >               {"dstmac", required_argument, NULL, 'b'},
> >               {"srcip", required_argument, NULL, 'c'},
> > @@ -831,7 +842,7 @@ static void parse_args(int argc, char *argv[],
> appl_args_t *appl_args)
> >       appl_args->timeout = -1;
> >
> >       while (1) {
> > -             opt = getopt_long(argc, argv, "+I:a:b:c:d:s:i:m:n:t:w:h",
> > +             opt = getopt_long(argc, argv, "+I:a:b:c:d:s:i:m:n:t:w:k:h",
> >                                       longopts, &long_index);
> >               if (opt == -1)
> >                       break;  /* No more options */
> > @@ -840,6 +851,15 @@ static void parse_args(int argc, char *argv[],
> appl_args_t *appl_args)
> >               case 'w':
> >                       appl_args->cpu_count = atoi(optarg);
> >                       break;
> > +             case 'k':
> > +                     len = strlen(optarg);
> > +                     if (len == 0) {
>
> Can this actually happen? won't getopt fail if you don't provide an
> argument?
>
> > +                             usage(argv[0]);
> > +                             exit(EXIT_FAILURE);
> > +                     }
> > +
> > +                     appl_args->mask = optarg;
> > +                     break;
> >               /* parse packet-io interface names */
> >               case 'I':
> >                       len = strlen(optarg);
> > @@ -1029,6 +1049,9 @@ static void usage(char *progname)
> >              "  -t, --timeout only for ping mode, wait ICMP reply
> timeout seconds\n"
> >              "  -i, --interval wait interval ms between sending each
> packet\n"
> >              "                 default is 1000ms. 0 for flood mode\n"
> > +            "  -w, --workers specify number of workers need to be
> assigned to application\n"
> > +            "                 default is to assign all\n"
> > +            "  -k, --coremask to set on cores\n"
> >              "\n"
> >              "Optional OPTIONS\n"
> >              "  -h, --help       Display help and exit.\n"
> > --
> > 1.9.1
> >
>
_______________________________________________
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Reply via email to