Re: [dpdk-dev] [PATCH 6/7] examples/vhost: enhance getopt_long usage

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

On Tue, Nov 24, 2020 at 1:35 PM Ibtisam Tariq <ibtisam.tariq@emumba.com> wrote:
> diff --git a/examples/vhost/main.c b/examples/vhost/main.c
> index 8d8c3038b..ce8b64035 100644
> --- a/examples/vhost/main.c
> +++ b/examples/vhost/main.c
> @@ -466,6 +466,33 @@ us_vhost_usage(const char *prgname)
>                prgname);
>  }
>
> +enum {
> +#define OPT_VM2VM               "vm2vm"
> +       OPT_VM2VM_NUM = 256,
> +#define OPT_RX_RETRY            "rx-retry"
> +       OPT_RX_RETRY_NUM,
> +#define OPT_RX_RETRY_DELAY      "rx-retry-delay"
> +       OPT_RX_RETRY_DELAY_NUM,
> +#define OPT_RX_RETRY_NUMB        "rx-retry-num"
> +       OPT_RX_RETRY_NUMB_NUM,
> +#define OPT_MERGEABLE           "mergeable"
> +       OPT_MERGEABLE_NUM,
> +#define OPT_STATS               "stats"
> +       OPT_STATS_NUM,
> +#define OPT_SOCKET_FILE         "socket-file"
> +       OPT_SOCKET_FILE_NUM,
> +#define OPT_TX_CSUM             "tx-csum"
> +       OPT_TX_CSUM_NUM,
> +#define OPT_TSO                 "tso"
> +       OPT_TSO_NUM,
> +#define OPT_CLIENT              "client"
> +#define OPT_BUILTIN_NET_DRIVER  "builtin-net-driver"
> +#define OPT_DMA_TYPE                   "dma-type"
> +       OPT_DMA_TYPE_NUM,
> +#define OPT_DMAS                               "dmas"
> +       OPT_DMAS_NUM,

Nit: could you align those last two strings?


> +};
> +
>  /*
>   * Parse the arguments given in the command line of the application.
>   */
> @@ -477,19 +504,31 @@ us_vhost_parse_args(int argc, char **argv)
>         unsigned i;
>         const char *prgname = argv[0];
>         static struct option long_option[] = {
> -               {"vm2vm", required_argument, NULL, 0},
> -               {"rx-retry", required_argument, NULL, 0},
> -               {"rx-retry-delay", required_argument, NULL, 0},
> -               {"rx-retry-num", required_argument, NULL, 0},
> -               {"mergeable", required_argument, NULL, 0},
> -               {"stats", required_argument, NULL, 0},
> -               {"socket-file", required_argument, NULL, 0},
> -               {"tx-csum", required_argument, NULL, 0},
> -               {"tso", required_argument, NULL, 0},
> -               {"client", no_argument, &client_mode, 1},
> -               {"builtin-net-driver", no_argument, &builtin_net_driver, 1},
> -               {"dma-type", required_argument, NULL, 0},
> -               {"dmas", required_argument, NULL, 0},
> +               {OPT_VM2VM, required_argument,
> +                               NULL, OPT_VM2VM_NUM},
> +               {OPT_RX_RETRY, required_argument,
> +                               NULL, OPT_RX_RETRY_NUM},
> +               {OPT_RX_RETRY_DELAY, required_argument,
> +                               NULL, OPT_RX_RETRY_DELAY_NUM},
> +               {OPT_RX_RETRY_NUMB, required_argument,
> +                               NULL, OPT_RX_RETRY_NUMB_NUM},
> +               {OPT_MERGEABLE, required_argument,
> +                               NULL, OPT_MERGEABLE_NUM},
> +               {OPT_STATS, required_argument,
> +                               NULL, OPT_STATS_NUM},
> +               {OPT_SOCKET_FILE, required_argument,
> +                               NULL, OPT_SOCKET_FILE_NUM},
> +               {OPT_TX_CSUM, required_argument,
> +                               NULL, OPT_TX_CSUM_NUM},
> +               {OPT_TSO, required_argument,
> +                               NULL, OPT_TSO_NUM},
> +               {OPT_CLIENT, no_argument, &client_mode, 1},
> +               {OPT_BUILTIN_NET_DRIVER, no_argument,
> +                               &builtin_net_driver, 1},

We will have an issue here.

Quoting the manual:
"""
       flag   specifies  how  results are returned for a long option.
If flag is NULL, then getopt_long() returns val.  (For example, the
calling program may set val to the equivalent short option charac‐
              ter.)  Otherwise, getopt_long() returns 0, and flag
points to a variable which is set to val if the option is found, but
left unchanged if the option is not found.
"""

$ build/examples/dpdk-vhost --no-huge -m 1024 -l 0,1 -a 0:0.0 --
--builtin-net-driver --socket-file /tmp/sock0 -p 0x1
EAL: Detected 8 lcore(s)
EAL: Detected 1 NUMA nodes
EAL: Multi-process socket /run/user/114840/dpdk/rte/mp_socket
EAL: Selected IOVA mode 'VA'
EAL: Probing VFIO support...
VHOST_CONFIG: build/examples/dpdk-vhost [EAL options] -- -p PORTMASK
        --vm2vm [0|1|2]
        --rx_retry [0|1] --mergeable [0|1] --stats [0-N]
        --socket-file <path>
        --nb-devices ND
        -p PORTMASK: Set mask for ports to be used by application
        --vm2vm [0|1|2]: disable/software(default)/hardware vm2vm comms
        --rx-retry [0|1]: disable/enable(default) retries on rx.
Enable retry if destintation queue is full
        --rx-retry-delay [0-N]: timeout(in usecond) between retries on
RX. This makes effect only if retries on rx enabled
        --rx-retry-num [0-N]: the number of retries on rx. This makes
effect only if retries on rx enabled
        --mergeable [0|1]: disable(default)/enable RX mergeable buffers
        --stats [0-N]: 0: Disable stats, N: Time in seconds to print stats
        --socket-file: The path of the socket file.
        --tx-csum [0|1] disable/enable TX checksum offload.
        --tso [0|1] disable/enable TCP segment offload.
        --client register a vhost-user socket as client mode.
        --dma-type register dma type for your vhost async driver. For
example "ioat" for now.
        --dmas register dma channel for specific vhost device.
EAL: Error - exiting with code: 1
  Cause: Invalid argument

This is because getopt_long returned 0.

We have two solutions:
- we keep this &builtin_net_driver flag use, but we must accept a 0
return value from getopt_long.
- we convert this option to the same scheme as the other and add a
OPT_BUILTIN_NET_DRIVER_NUM like other options in this example,

Maxime, opinion?


-- 
David Marchand