Re: [dpdk-dev] [PATCH v4 2/2] app/pdump: enhance to support multi-core capture

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

On Tue, Apr 2, 2019 at 6:33 AM Vipin Varghese <vipin.varghese@intel.com>
wrote:

> Add option --multi, to enhance the pdump application to allow capture
> on unique cores for each --pdump option. If option --multi is ignored
> the default capture occurs on a single core for all --pdump options.
>
> Signed-off-by: Vipin Varghese <vipin.varghese@intel.com>
> ---
>  app/pdump/main.c           | 76 ++++++++++++++++++++++++++++++++------
>  doc/guides/tools/pdump.rst |  8 +++-
>  2 files changed, 72 insertions(+), 12 deletions(-)
>
> diff --git a/app/pdump/main.c b/app/pdump/main.c
> index c1db2eb8d..997c8942f 100644
> --- a/app/pdump/main.c
> +++ b/app/pdump/main.c
> @@ -28,6 +28,7 @@
>  #include <rte_pdump.h>
>
>  #define CMD_LINE_OPT_PDUMP "pdump"
> +#define CMD_LINE_OPT_MULTI "multi"
>  #define PDUMP_PORT_ARG "port"
>  #define PDUMP_PCI_ARG "device_id"
>  #define PDUMP_QUEUE_ARG "queue"
> @@ -139,12 +140,14 @@ struct parse_val {
>  static int num_tuples;
>  static struct rte_eth_conf port_conf_default;
>  static volatile uint8_t quit_signal;
> +static uint8_t multiple_core_capture;
>
>  /**< display usage */
>  static void
>  pdump_usage(const char *prgname)
>  {
> -       printf("usage: %s [EAL options] -- --pdump "
> +       printf("usage: %s [EAL options] -- [--multi] "
> +                       "--pdump "
>                         "'(port=<port id> | device_id=<pci id or vdev
> name>),"
>                         "(queue=<queue_id>),"
>                         "(rx-dev=<iface or pcap file> |"
>

Rather than hardcode the usage, reuse the macro you introduced:
CMD_LINE_OPT_MULTI.


@@ -376,6 +379,7 @@ launch_args_parse(int argc, char **argv, char *prgname)
>         int option_index;
>         static struct option long_option[] = {
>                 {"pdump", 1, 0, 0},
> +               {"multi", 0, 0, 0},
>                 {NULL, 0, 0, 0}
>         };
>
>
Idem reuse the macro.

Besides look at lib/librte_eal/common/eal_options.h and
lib/librte_eal/common/eal_common_options.c.
Define an enum for long only options and map "multi" to an integer that has
no printable character associated.

This way, you can avoid (see below)...

@@ -395,6 +399,10 @@ launch_args_parse(int argc, char **argv, char *prgname)
>                                         pdump_usage(prgname);
>                                         return -1;
>                                 }
> +                       } else if (!strncmp(long_option[option_index].name,
> +                                       CMD_LINE_OPT_MULTI,
> +                                       sizeof(CMD_LINE_OPT_MULTI))) {
> +                               multiple_core_capture = 1;
>                         }
>                         break;
>                 default:
>

... this strncmp.
getopt_long already matched the input option for you.


-- 
David Marchand