[dpdk-dev] examples/ipsec-secgw: remove redundant string compare
Checks
Commit Message
Removing redundant strncmp in parsing long arguments. The getopt library
provides means to identify long options using the "val" field of
structure option. The existing code gets 0 as "val" for all long
arguments and then uses strncmp to figure out which long option was
being referred to. Fixing this.
In addition, the macros and enums used for long arguments have been
renamed and repositioned adhering to the general convention followed in
various other apps, like l3fwd.
Signed-off-by: Anoob Joseph <anoob.joseph@caviumnetworks.com>
---
examples/ipsec-secgw/ipsec-secgw.c | 104 +++++++++++++++++++------------------
1 file changed, 54 insertions(+), 50 deletions(-)
Comments
Hi Akhil, Radu,
Did you get time to review the patch?
Thanks,
Anoob
On 23/03/18 11:20, Anoob Joseph wrote:
> Removing redundant strncmp in parsing long arguments. The getopt library
> provides means to identify long options using the "val" field of
> structure option. The existing code gets 0 as "val" for all long
> arguments and then uses strncmp to figure out which long option was
> being referred to. Fixing this.
>
> In addition, the macros and enums used for long arguments have been
> renamed and repositioned adhering to the general convention followed in
> various other apps, like l3fwd.
>
> Signed-off-by: Anoob Joseph <anoob.joseph@caviumnetworks.com>
> ---
> examples/ipsec-secgw/ipsec-secgw.c | 104 +++++++++++++++++++------------------
> 1 file changed, 54 insertions(+), 50 deletions(-)
>
> diff --git a/examples/ipsec-secgw/ipsec-secgw.c b/examples/ipsec-secgw/ipsec-secgw.c
> index 5726fd3..b833686 100644
> --- a/examples/ipsec-secgw/ipsec-secgw.c
> +++ b/examples/ipsec-secgw/ipsec-secgw.c
> @@ -59,10 +59,6 @@
> #define CDEV_MP_CACHE_SZ 64
> #define MAX_QUEUE_PAIRS 1
>
> -#define OPTION_CONFIG "config"
> -#define OPTION_SINGLE_SA "single-sa"
> -#define OPTION_CRYPTODEV_MASK "cryptodev_mask"
> -
> #define BURST_TX_DRAIN_US 100 /* TX drain every ~100us */
>
> #define NB_SOCKETS 4
> @@ -125,6 +121,29 @@ struct ethaddr_info ethaddr_tbl[RTE_MAX_ETHPORTS] = {
> { 0, ETHADDR(0x00, 0x16, 0x3e, 0x49, 0x9e, 0xdd) }
> };
>
> +#define CMD_LINE_OPT_CONFIG "config"
> +#define CMD_LINE_OPT_SINGLE_SA "single-sa"
> +#define CMD_LINE_OPT_CRYPTODEV_MASK "cryptodev_mask"
> +
> +enum {
> + /* long options mapped to a short option */
> +
> + /* first long only option value must be >= 256, so that we won't
> + * conflict with short options
> + */
> + CMD_LINE_OPT_MIN_NUM = 256,
> + CMD_LINE_OPT_CONFIG_NUM,
> + CMD_LINE_OPT_SINGLE_SA_NUM,
> + CMD_LINE_OPT_CRYPTODEV_MASK_NUM,
> +};
> +
> +static const struct option lgopts[] = {
> + {CMD_LINE_OPT_CONFIG, 1, 0, CMD_LINE_OPT_CONFIG_NUM},
> + {CMD_LINE_OPT_SINGLE_SA, 1, 0, CMD_LINE_OPT_SINGLE_SA_NUM},
> + {CMD_LINE_OPT_CRYPTODEV_MASK, 1, 0, CMD_LINE_OPT_CRYPTODEV_MASK_NUM},
> + {NULL, 0, 0, 0}
> +};
> +
> /* mask of enabled ports */
> static uint32_t enabled_port_mask;
> static uint64_t enabled_cryptodev_mask = UINT64_MAX;
> @@ -928,13 +947,13 @@ static void
> print_usage(const char *prgname)
> {
> printf("%s [EAL options] -- -p PORTMASK -P -u PORTMASK"
> - " --"OPTION_CONFIG" (port,queue,lcore)[,(port,queue,lcore]"
> + " --"CMD_LINE_OPT_CONFIG" (port,queue,lcore)[,(port,queue,lcore]"
> " --single-sa SAIDX -f CONFIG_FILE\n"
> " -p PORTMASK: hexadecimal bitmask of ports to configure\n"
> " -P : enable promiscuous mode\n"
> " -u PORTMASK: hexadecimal bitmask of unprotected ports\n"
> " -j FRAMESIZE: jumbo frame maximum size\n"
> - " --"OPTION_CONFIG": (port,queue,lcore): "
> + " --"CMD_LINE_OPT_CONFIG": (port,queue,lcore): "
> "rx queues configuration\n"
> " --single-sa SAIDX: use single SA index for outbound, "
> "bypassing the SP\n"
> @@ -1030,42 +1049,6 @@ parse_config(const char *q_arg)
> return 0;
> }
>
> -#define __STRNCMP(name, opt) (!strncmp(name, opt, sizeof(opt)))
> -static int32_t
> -parse_args_long_options(struct option *lgopts, int32_t option_index)
> -{
> - int32_t ret = -1;
> - const char *optname = lgopts[option_index].name;
> -
> - if (__STRNCMP(optname, OPTION_CONFIG)) {
> - ret = parse_config(optarg);
> - if (ret)
> - printf("invalid config\n");
> - }
> -
> - if (__STRNCMP(optname, OPTION_SINGLE_SA)) {
> - ret = parse_decimal(optarg);
> - if (ret != -1) {
> - single_sa = 1;
> - single_sa_idx = ret;
> - printf("Configured with single SA index %u\n",
> - single_sa_idx);
> - ret = 0;
> - }
> - }
> -
> - if (__STRNCMP(optname, OPTION_CRYPTODEV_MASK)) {
> - ret = parse_portmask(optarg);
> - if (ret != -1) {
> - enabled_cryptodev_mask = ret;
> - ret = 0;
> - }
> - }
> -
> - return ret;
> -}
> -#undef __STRNCMP
> -
> static int32_t
> parse_args(int32_t argc, char **argv)
> {
> @@ -1073,12 +1056,6 @@ parse_args(int32_t argc, char **argv)
> char **argvopt;
> int32_t option_index;
> char *prgname = argv[0];
> - static struct option lgopts[] = {
> - {OPTION_CONFIG, 1, 0, 0},
> - {OPTION_SINGLE_SA, 1, 0, 0},
> - {OPTION_CRYPTODEV_MASK, 1, 0, 0},
> - {NULL, 0, 0, 0}
> - };
> int32_t f_present = 0;
>
> argvopt = argv;
> @@ -1139,12 +1116,39 @@ parse_args(int32_t argc, char **argv)
> }
> printf("Enabled jumbo frames size %u\n", frame_size);
> break;
> - case 0:
> - if (parse_args_long_options(lgopts, option_index)) {
> + case CMD_LINE_OPT_CONFIG_NUM:
> + ret = parse_config(optarg);
> + if (ret) {
> + printf("Invalid config\n");
> print_usage(prgname);
> return -1;
> }
> break;
> + case CMD_LINE_OPT_SINGLE_SA_NUM:
> + ret = parse_decimal(optarg);
> + if (ret == -1) {
> + printf("Invalid argument[sa_idx]\n");
> + print_usage(prgname);
> + return -1;
> + }
> +
> + /* else */
> + single_sa = 1;
> + single_sa_idx = ret;
> + printf("Configured with single SA index %u\n",
> + single_sa_idx);
> + break;
> + case CMD_LINE_OPT_CRYPTODEV_MASK_NUM:
> + ret = parse_portmask(optarg);
> + if (ret == -1) {
> + printf("Invalid argument[portmask]\n");
> + print_usage(prgname);
> + return -1;
> + }
> +
> + /* else */
> + enabled_cryptodev_mask = ret;
> + break;
> default:
> print_usage(prgname);
> return -1;
On 3/28/2018 3:28 PM, Anoob Joseph wrote:
> Hi Akhil, Radu,
>
> Did you get time to review the patch?
>
> Thanks,
> Anoob
>
> On 23/03/18 11:20, Anoob Joseph wrote:
>> Removing redundant strncmp in parsing long arguments. The getopt library
>> provides means to identify long options using the "val" field of
>> structure option. The existing code gets 0 as "val" for all long
>> arguments and then uses strncmp to figure out which long option was
>> being referred to. Fixing this.
>>
>> In addition, the macros and enums used for long arguments have been
>> renamed and repositioned adhering to the general convention followed in
>> various other apps, like l3fwd.
>>
>> Signed-off-by: Anoob Joseph <anoob.joseph@caviumnetworks.com>
>> ---
Acked-by: Radu Nicolau <radu.nicolau@intel.com>
<mailto:radu.nicolau@intel.com>
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Radu Nicolau
> Sent: Thursday, March 29, 2018 11:06 AM
> To: Anoob Joseph <Anoob.Joseph@caviumnetworks.com>; Akhil Goyal
> <akhil.goyal@nxp.com>
> Cc: Doherty, Declan <declan.doherty@intel.com>; Jerin Jacob
> <jerin.jacob@caviumnetworks.com>; Narayana Prasad
> <narayanaprasad.athreya@caviumnetworks.com>; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] examples/ipsec-secgw: remove redundant
> string compare
>
>
> On 3/28/2018 3:28 PM, Anoob Joseph wrote:
> > Hi Akhil, Radu,
> >
> > Did you get time to review the patch?
> >
> > Thanks,
> > Anoob
> >
> > On 23/03/18 11:20, Anoob Joseph wrote:
> >> Removing redundant strncmp in parsing long arguments. The getopt
> >> library provides means to identify long options using the "val" field
> >> of structure option. The existing code gets 0 as "val" for all long
> >> arguments and then uses strncmp to figure out which long option was
> >> being referred to. Fixing this.
> >>
> >> In addition, the macros and enums used for long arguments have been
> >> renamed and repositioned adhering to the general convention followed
> >> in various other apps, like l3fwd.
> >>
> >> Signed-off-by: Anoob Joseph <anoob.joseph@caviumnetworks.com>
> >> ---
> Acked-by: Radu Nicolau <radu.nicolau@intel.com>
> <mailto:radu.nicolau@intel.com>
Applied to dpdk-next-crypto.
Thanks,
Pablo
@@ -59,10 +59,6 @@
#define CDEV_MP_CACHE_SZ 64
#define MAX_QUEUE_PAIRS 1
-#define OPTION_CONFIG "config"
-#define OPTION_SINGLE_SA "single-sa"
-#define OPTION_CRYPTODEV_MASK "cryptodev_mask"
-
#define BURST_TX_DRAIN_US 100 /* TX drain every ~100us */
#define NB_SOCKETS 4
@@ -125,6 +121,29 @@ struct ethaddr_info ethaddr_tbl[RTE_MAX_ETHPORTS] = {
{ 0, ETHADDR(0x00, 0x16, 0x3e, 0x49, 0x9e, 0xdd) }
};
+#define CMD_LINE_OPT_CONFIG "config"
+#define CMD_LINE_OPT_SINGLE_SA "single-sa"
+#define CMD_LINE_OPT_CRYPTODEV_MASK "cryptodev_mask"
+
+enum {
+ /* long options mapped to a short option */
+
+ /* first long only option value must be >= 256, so that we won't
+ * conflict with short options
+ */
+ CMD_LINE_OPT_MIN_NUM = 256,
+ CMD_LINE_OPT_CONFIG_NUM,
+ CMD_LINE_OPT_SINGLE_SA_NUM,
+ CMD_LINE_OPT_CRYPTODEV_MASK_NUM,
+};
+
+static const struct option lgopts[] = {
+ {CMD_LINE_OPT_CONFIG, 1, 0, CMD_LINE_OPT_CONFIG_NUM},
+ {CMD_LINE_OPT_SINGLE_SA, 1, 0, CMD_LINE_OPT_SINGLE_SA_NUM},
+ {CMD_LINE_OPT_CRYPTODEV_MASK, 1, 0, CMD_LINE_OPT_CRYPTODEV_MASK_NUM},
+ {NULL, 0, 0, 0}
+};
+
/* mask of enabled ports */
static uint32_t enabled_port_mask;
static uint64_t enabled_cryptodev_mask = UINT64_MAX;
@@ -928,13 +947,13 @@ static void
print_usage(const char *prgname)
{
printf("%s [EAL options] -- -p PORTMASK -P -u PORTMASK"
- " --"OPTION_CONFIG" (port,queue,lcore)[,(port,queue,lcore]"
+ " --"CMD_LINE_OPT_CONFIG" (port,queue,lcore)[,(port,queue,lcore]"
" --single-sa SAIDX -f CONFIG_FILE\n"
" -p PORTMASK: hexadecimal bitmask of ports to configure\n"
" -P : enable promiscuous mode\n"
" -u PORTMASK: hexadecimal bitmask of unprotected ports\n"
" -j FRAMESIZE: jumbo frame maximum size\n"
- " --"OPTION_CONFIG": (port,queue,lcore): "
+ " --"CMD_LINE_OPT_CONFIG": (port,queue,lcore): "
"rx queues configuration\n"
" --single-sa SAIDX: use single SA index for outbound, "
"bypassing the SP\n"
@@ -1030,42 +1049,6 @@ parse_config(const char *q_arg)
return 0;
}
-#define __STRNCMP(name, opt) (!strncmp(name, opt, sizeof(opt)))
-static int32_t
-parse_args_long_options(struct option *lgopts, int32_t option_index)
-{
- int32_t ret = -1;
- const char *optname = lgopts[option_index].name;
-
- if (__STRNCMP(optname, OPTION_CONFIG)) {
- ret = parse_config(optarg);
- if (ret)
- printf("invalid config\n");
- }
-
- if (__STRNCMP(optname, OPTION_SINGLE_SA)) {
- ret = parse_decimal(optarg);
- if (ret != -1) {
- single_sa = 1;
- single_sa_idx = ret;
- printf("Configured with single SA index %u\n",
- single_sa_idx);
- ret = 0;
- }
- }
-
- if (__STRNCMP(optname, OPTION_CRYPTODEV_MASK)) {
- ret = parse_portmask(optarg);
- if (ret != -1) {
- enabled_cryptodev_mask = ret;
- ret = 0;
- }
- }
-
- return ret;
-}
-#undef __STRNCMP
-
static int32_t
parse_args(int32_t argc, char **argv)
{
@@ -1073,12 +1056,6 @@ parse_args(int32_t argc, char **argv)
char **argvopt;
int32_t option_index;
char *prgname = argv[0];
- static struct option lgopts[] = {
- {OPTION_CONFIG, 1, 0, 0},
- {OPTION_SINGLE_SA, 1, 0, 0},
- {OPTION_CRYPTODEV_MASK, 1, 0, 0},
- {NULL, 0, 0, 0}
- };
int32_t f_present = 0;
argvopt = argv;
@@ -1139,12 +1116,39 @@ parse_args(int32_t argc, char **argv)
}
printf("Enabled jumbo frames size %u\n", frame_size);
break;
- case 0:
- if (parse_args_long_options(lgopts, option_index)) {
+ case CMD_LINE_OPT_CONFIG_NUM:
+ ret = parse_config(optarg);
+ if (ret) {
+ printf("Invalid config\n");
print_usage(prgname);
return -1;
}
break;
+ case CMD_LINE_OPT_SINGLE_SA_NUM:
+ ret = parse_decimal(optarg);
+ if (ret == -1) {
+ printf("Invalid argument[sa_idx]\n");
+ print_usage(prgname);
+ return -1;
+ }
+
+ /* else */
+ single_sa = 1;
+ single_sa_idx = ret;
+ printf("Configured with single SA index %u\n",
+ single_sa_idx);
+ break;
+ case CMD_LINE_OPT_CRYPTODEV_MASK_NUM:
+ ret = parse_portmask(optarg);
+ if (ret == -1) {
+ printf("Invalid argument[portmask]\n");
+ print_usage(prgname);
+ return -1;
+ }
+
+ /* else */
+ enabled_cryptodev_mask = ret;
+ break;
default:
print_usage(prgname);
return -1;