[dpdk-dev] [PATCH 2/7] examples/l3fwd-acl: enhance getopt_long usage
Ananyev, Konstantin
konstantin.ananyev at intel.com
Fri Nov 13 13:26:51 CET 2020
Hi,
>
> Instead of using getopt_long return value, strcmp was used to
> compare the input parameters with the struct option array. This
> patch get rid of all those strcmp by directly binding each longopt
> with an int enum. This is to improve readability and consistency in
> all examples.
>
> Bugzilla ID: 238
> Cc: konstantin.ananyev at intel.com
>
> Reported-by: David Marchand <david.marchand at redhat.com>
> Signed-off-by: Ibtisam Tariq <ibtisam.tariq at emumba.com>
> ---
> v2
> * Remove extra indentations.
> * Remove extra block brackets in switch statement.
> * Change enum names to start with OPT_ and remove KEYWORD from enum names.
>
> v1
> * enhance getopt_long usage
> ---
> examples/l3fwd-acl/main.c | 219 +++++++++++++++++++-------------------
> 1 file changed, 110 insertions(+), 109 deletions(-)
>
> diff --git a/examples/l3fwd-acl/main.c b/examples/l3fwd-acl/main.c
> index 961594f5f..5725963ad 100644
> --- a/examples/l3fwd-acl/main.c
> +++ b/examples/l3fwd-acl/main.c
> @@ -195,13 +195,24 @@ send_single_packet(struct rte_mbuf *m, uint16_t port);
> #define ACL_LEAD_CHAR ('@')
> #define ROUTE_LEAD_CHAR ('R')
> #define COMMENT_LEAD_CHAR ('#')
> -#define OPTION_CONFIG "config"
> -#define OPTION_NONUMA "no-numa"
> -#define OPTION_ENBJMO "enable-jumbo"
> -#define OPTION_RULE_IPV4 "rule_ipv4"
> -#define OPTION_RULE_IPV6 "rule_ipv6"
> -#define OPTION_ALG "alg"
> -#define OPTION_ETH_DEST "eth-dest"
> +
> +enum {
> +#define OPT_CONFIG "config"
> + OPT_CONFIG_NUM = 256,
It is probably better not to mix enum values with corresponding defines
for string literals. Looks really weird and hard to read (at least to me).
Apart from that - LGTM.
> +#define OPT_NONUMA "no-numa"
> + OPT_NONUMA_NUM,
> +#define OPT_ENBJMO "enable-jumbo"
> + OPT_ENBJMO_NUM,
> +#define OPT_RULE_IPV4 "rule_ipv4"
> + OPT_RULE_IPV4_NUM,
> +#define OPT_RULE_IPV6 "rule_ipv6"
> + OPT_RULE_IPV6_NUM,
> +#define OPT_ALG "alg"
> + OPT_ALG_NUM,
> +#define OPT_ETH_DEST "eth-dest"
> + OPT_ETH_DEST_NUM,
> +};
> +
> #define ACL_DENY_SIGNATURE 0xf0000000
> #define RTE_LOGTYPE_L3FWDACL RTE_LOGTYPE_USER3
> #define acl_log(format, ...) RTE_LOG(ERR, L3FWDACL, format, ##__VA_ARGS__)
> @@ -1177,9 +1188,9 @@ static void
> dump_acl_config(void)
> {
> printf("ACL option are:\n");
> - printf(OPTION_RULE_IPV4": %s\n", parm_config.rule_ipv4_name);
> - printf(OPTION_RULE_IPV6": %s\n", parm_config.rule_ipv6_name);
> - printf(OPTION_ALG": %s\n", str_acl_alg(parm_config.alg));
> + printf(OPT_RULE_IPV4": %s\n", parm_config.rule_ipv4_name);
> + printf(OPT_RULE_IPV6": %s\n", parm_config.rule_ipv6_name);
> + printf(OPT_ALG": %s\n", str_acl_alg(parm_config.alg));
> }
>
> static int
> @@ -1608,27 +1619,27 @@ print_usage(const char *prgname)
>
> usage_acl_alg(alg, sizeof(alg));
> printf("%s [EAL options] -- -p PORTMASK -P"
> - "--"OPTION_RULE_IPV4"=FILE"
> - "--"OPTION_RULE_IPV6"=FILE"
> - " [--"OPTION_CONFIG" (port,queue,lcore)[,(port,queue,lcore]]"
> - " [--"OPTION_ENBJMO" [--max-pkt-len PKTLEN]]\n"
> + "--"OPT_RULE_IPV4"=FILE"
> + "--"OPT_RULE_IPV6"=FILE"
> + " [--"OPT_CONFIG" (port,queue,lcore)[,(port,queue,lcore]]"
> + " [--"OPT_ENBJMO" [--max-pkt-len PKTLEN]]\n"
> " -p PORTMASK: hexadecimal bitmask of ports to configure\n"
> " -P : enable promiscuous mode\n"
> - " --"OPTION_CONFIG": (port,queue,lcore): "
> + " --"OPT_CONFIG": (port,queue,lcore): "
> "rx queues configuration\n"
> - " --"OPTION_NONUMA": optional, disable numa awareness\n"
> - " --"OPTION_ENBJMO": enable jumbo frame"
> + " --"OPT_NONUMA": optional, disable numa awareness\n"
> + " --"OPT_ENBJMO": enable jumbo frame"
> " which max packet len is PKTLEN in decimal (64-9600)\n"
> - " --"OPTION_RULE_IPV4"=FILE: specify the ipv4 rules entries "
> + " --"OPT_RULE_IPV4"=FILE: specify the ipv4 rules entries "
> "file. "
> "Each rule occupy one line. "
> "2 kinds of rules are supported. "
> "One is ACL entry at while line leads with character '%c', "
> "another is route entry at while line leads with "
> "character '%c'.\n"
> - " --"OPTION_RULE_IPV6"=FILE: specify the ipv6 rules "
> + " --"OPT_RULE_IPV6"=FILE: specify the ipv6 rules "
> "entries file.\n"
> - " --"OPTION_ALG": ACL classify method to use, one of: %s\n",
> + " --"OPT_ALG": ACL classify method to use, one of: %s\n",
> prgname, ACL_LEAD_CHAR, ROUTE_LEAD_CHAR, alg);
> }
>
> @@ -1747,14 +1758,14 @@ parse_args(int argc, char **argv)
> int option_index;
> char *prgname = argv[0];
> static struct option lgopts[] = {
> - {OPTION_CONFIG, 1, 0, 0},
> - {OPTION_NONUMA, 0, 0, 0},
> - {OPTION_ENBJMO, 0, 0, 0},
> - {OPTION_RULE_IPV4, 1, 0, 0},
> - {OPTION_RULE_IPV6, 1, 0, 0},
> - {OPTION_ALG, 1, 0, 0},
> - {OPTION_ETH_DEST, 1, 0, 0},
> - {NULL, 0, 0, 0}
> + {OPT_CONFIG, 1, NULL, OPT_CONFIG_NUM },
> + {OPT_NONUMA, 0, NULL, OPT_NONUMA_NUM },
> + {OPT_ENBJMO, 0, NULL, OPT_ENBJMO_NUM },
> + {OPT_RULE_IPV4, 1, NULL, OPT_RULE_IPV4_NUM },
> + {OPT_RULE_IPV6, 1, NULL, OPT_RULE_IPV6_NUM },
> + {OPT_ALG, 1, NULL, OPT_ALG_NUM },
> + {OPT_ETH_DEST, 1, NULL, OPT_ETH_DEST_NUM },
> + {NULL, 0, 0, 0 }
> };
>
> argvopt = argv;
> @@ -1772,104 +1783,94 @@ parse_args(int argc, char **argv)
> return -1;
> }
> break;
> +
> case 'P':
> printf("Promiscuous mode selected\n");
> promiscuous_on = 1;
> break;
>
> /* long options */
> - case 0:
> - if (!strncmp(lgopts[option_index].name,
> - OPTION_CONFIG,
> - sizeof(OPTION_CONFIG))) {
> - ret = parse_config(optarg);
> - if (ret) {
> - printf("invalid config\n");
> - print_usage(prgname);
> - return -1;
> - }
> - }
> -
> - if (!strncmp(lgopts[option_index].name,
> - OPTION_NONUMA,
> - sizeof(OPTION_NONUMA))) {
> - printf("numa is disabled\n");
> - numa_on = 0;
> - }
> -
> - if (!strncmp(lgopts[option_index].name,
> - OPTION_ENBJMO, sizeof(OPTION_ENBJMO))) {
> - struct option lenopts = {
> - "max-pkt-len",
> - required_argument,
> - 0,
> - 0
> - };
> -
> - printf("jumbo frame is enabled\n");
> - port_conf.rxmode.offloads |=
> - DEV_RX_OFFLOAD_JUMBO_FRAME;
> - port_conf.txmode.offloads |=
> - DEV_TX_OFFLOAD_MULTI_SEGS;
> -
> - /*
> - * if no max-pkt-len set, then use the
> - * default value RTE_ETHER_MAX_LEN
> - */
> - if (0 == getopt_long(argc, argvopt, "",
> - &lenopts, &option_index)) {
> - ret = parse_max_pkt_len(optarg);
> - if ((ret < 64) ||
> - (ret > MAX_JUMBO_PKT_LEN)) {
> - printf("invalid packet "
> - "length\n");
> - print_usage(prgname);
> - return -1;
> - }
> - port_conf.rxmode.max_rx_pkt_len = ret;
> - }
> - printf("set jumbo frame max packet length "
> - "to %u\n",
> - (unsigned int)
> - port_conf.rxmode.max_rx_pkt_len);
> + case OPT_CONFIG_NUM:
> + ret = parse_config(optarg);
> + if (ret) {
> + printf("invalid config\n");
> + print_usage(prgname);
> + return -1;
> }
> + break;
>
> - if (!strncmp(lgopts[option_index].name,
> - OPTION_RULE_IPV4,
> - sizeof(OPTION_RULE_IPV4)))
> - parm_config.rule_ipv4_name = optarg;
> -
> - if (!strncmp(lgopts[option_index].name,
> - OPTION_RULE_IPV6,
> - sizeof(OPTION_RULE_IPV6))) {
> - parm_config.rule_ipv6_name = optarg;
> - }
> + case OPT_NONUMA_NUM:
> + printf("numa is disabled\n");
> + numa_on = 0;
> + break;
>
> - if (!strncmp(lgopts[option_index].name,
> - OPTION_ALG, sizeof(OPTION_ALG))) {
> - parm_config.alg = parse_acl_alg(optarg);
> - if (parm_config.alg ==
> - RTE_ACL_CLASSIFY_DEFAULT) {
> - printf("unknown %s value:\"%s\"\n",
> - OPTION_ALG, optarg);
> + case OPT_ENBJMO_NUM:
> + {
> + struct option lenopts = {
> + "max-pkt-len",
> + required_argument,
> + 0,
> + 0
> + };
> +
> + printf("jumbo frame is enabled\n");
> + port_conf.rxmode.offloads |=
> + DEV_RX_OFFLOAD_JUMBO_FRAME;
> + port_conf.txmode.offloads |=
> + DEV_TX_OFFLOAD_MULTI_SEGS;
> +
> + /*
> + * if no max-pkt-len set, then use the
> + * default value RTE_ETHER_MAX_LEN
> + */
> + if (0 == getopt_long(argc, argvopt, "",
> + &lenopts, &option_index)) {
> + ret = parse_max_pkt_len(optarg);
> + if ((ret < 64) ||
> + (ret > MAX_JUMBO_PKT_LEN)) {
> + printf("invalid packet "
> + "length\n");
> print_usage(prgname);
> return -1;
> }
> + port_conf.rxmode.max_rx_pkt_len = ret;
> }
> + printf("set jumbo frame max packet length "
> + "to %u\n",
> + (unsigned int)
> + port_conf.rxmode.max_rx_pkt_len);
> + break;
> + }
> + case OPT_RULE_IPV4_NUM:
> + parm_config.rule_ipv4_name = optarg;
> + break;
>
> - if (!strncmp(lgopts[option_index].name, OPTION_ETH_DEST,
> - sizeof(OPTION_ETH_DEST))) {
> - const char *serr = parse_eth_dest(optarg);
> - if (serr != NULL) {
> - printf("invalid %s value:\"%s\": %s\n",
> - OPTION_ETH_DEST, optarg, serr);
> - print_usage(prgname);
> - return -1;
> - }
> - }
> + case OPT_RULE_IPV6_NUM:
> + parm_config.rule_ipv6_name = optarg;
> + break;
>
> + case OPT_ALG_NUM:
> + parm_config.alg = parse_acl_alg(optarg);
> + if (parm_config.alg ==
> + RTE_ACL_CLASSIFY_DEFAULT) {
> + printf("unknown %s value:\"%s\"\n",
> + OPT_ALG, optarg);
> + print_usage(prgname);
> + return -1;
> + }
> break;
>
> + case OPT_ETH_DEST_NUM:
> + {
> + const char *serr = parse_eth_dest(optarg);
> + if (serr != NULL) {
> + printf("invalid %s value:\"%s\": %s\n",
> + OPT_ETH_DEST, optarg, serr);
> + print_usage(prgname);
> + return -1;
> + }
> + break;
> + }
> default:
> print_usage(prgname);
> return -1;
> --
> 2.17.1
More information about the dev
mailing list