[PATCH 0/2] l2fwd/l3fwd: rework long options parsing

[PATCH 0/2] l2fwd/l3fwd: rework long options parsing

[Olivier Matz]

These 2 patches were part of this RFC, which will not be integrated:
http://dpdk.org/ml/archives/dev/2016-September/046974.html

It does not bring any functional change, it just reworks the way long
options are parsed in l2fwd and l3fwd to avoid uneeded strcmp() calls
and to ease the addition of a new long option in the future.

I send them in case maintainers think it is better this way, but I have
no real need.

Olivier Matz (2):
  l3fwd: rework long options parsing
  l2fwd: rework long options parsing

 examples/l2fwd/main.c |  30 +++++++--
 examples/l3fwd/main.c | 169 ++++++++++++++++++++++++++------------------------
 2 files changed, 111 insertions(+), 88 deletions(-)

-- 
2.8.1

[PATCH 1/2] l3fwd: rework long options parsing

[Olivier Matz]

Avoid the use of several strncpy() since getopt is able to
map a long option with an id, which can be matched in the
same switch/case than short options.

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 examples/l3fwd/main.c | 169 ++++++++++++++++++++++++++------------------------
 1 file changed, 87 insertions(+), 82 deletions(-)

diff --git a/examples/l3fwd/main.c b/examples/l3fwd/main.c
index 7223e77..f84ef50 100644
--- a/examples/l3fwd/main.c
+++ b/examples/l3fwd/main.c
@@ -474,6 +474,13 @@ parse_eth_dest(const char *optarg)
 #define MAX_JUMBO_PKT_LEN  9600
 #define MEMPOOL_CACHE_SIZE 256
 
+static const char short_options[] =
+	"p:"  /* portmask */
+	"P"   /* promiscuous */
+	"L"   /* enable long prefix match */
+	"E"   /* enable exact match */
+	;
+
 #define CMD_LINE_OPT_CONFIG "config"
 #define CMD_LINE_OPT_ETH_DEST "eth-dest"
 #define CMD_LINE_OPT_NO_NUMA "no-numa"
@@ -481,6 +488,31 @@ parse_eth_dest(const char *optarg)
 #define CMD_LINE_OPT_ENABLE_JUMBO "enable-jumbo"
 #define CMD_LINE_OPT_HASH_ENTRY_NUM "hash-entry-num"
 #define CMD_LINE_OPT_PARSE_PTYPE "parse-ptype"
+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_ETH_DEST_NUM,
+	CMD_LINE_OPT_NO_NUMA_NUM,
+	CMD_LINE_OPT_IPV6_NUM,
+	CMD_LINE_OPT_ENABLE_JUMBO_NUM,
+	CMD_LINE_OPT_HASH_ENTRY_NUM_NUM,
+	CMD_LINE_OPT_PARSE_PTYPE_NUM,
+};
+
+static const struct option lgopts[] = {
+	{CMD_LINE_OPT_CONFIG, 1, 0, CMD_LINE_OPT_CONFIG_NUM},
+	{CMD_LINE_OPT_ETH_DEST, 1, 0, CMD_LINE_OPT_ETH_DEST_NUM},
+	{CMD_LINE_OPT_NO_NUMA, 0, 0, CMD_LINE_OPT_NO_NUMA_NUM},
+	{CMD_LINE_OPT_IPV6, 0, 0, CMD_LINE_OPT_IPV6_NUM},
+	{CMD_LINE_OPT_ENABLE_JUMBO, 0, 0, CMD_LINE_OPT_ENABLE_JUMBO_NUM},
+	{CMD_LINE_OPT_HASH_ENTRY_NUM, 1, 0, CMD_LINE_OPT_HASH_ENTRY_NUM_NUM},
+	{CMD_LINE_OPT_PARSE_PTYPE, 0, 0, CMD_LINE_OPT_PARSE_PTYPE_NUM},
+	{NULL, 0, 0, 0}
+};
 
 /*
  * This expression is used to calculate the number of mbufs needed
@@ -504,16 +536,6 @@ parse_args(int argc, char **argv)
 	char **argvopt;
 	int option_index;
 	char *prgname = argv[0];
-	static struct option lgopts[] = {
-		{CMD_LINE_OPT_CONFIG, 1, 0, 0},
-		{CMD_LINE_OPT_ETH_DEST, 1, 0, 0},
-		{CMD_LINE_OPT_NO_NUMA, 0, 0, 0},
-		{CMD_LINE_OPT_IPV6, 0, 0, 0},
-		{CMD_LINE_OPT_ENABLE_JUMBO, 0, 0, 0},
-		{CMD_LINE_OPT_HASH_ENTRY_NUM, 1, 0, 0},
-		{CMD_LINE_OPT_PARSE_PTYPE, 0, 0, 0},
-		{NULL, 0, 0, 0}
-	};
 
 	argvopt = argv;
 
@@ -534,7 +556,7 @@ parse_args(int argc, char **argv)
 		"L3FWD: LPM and EM are mutually exclusive, select only one";
 	const char *str13 = "L3FWD: LPM or EM none selected, default LPM on";
 
-	while ((opt = getopt_long(argc, argvopt, "p:PLE",
+	while ((opt = getopt_long(argc, argvopt, short_options,
 				lgopts, &option_index)) != EOF) {
 
 		switch (opt) {
@@ -547,6 +569,7 @@ parse_args(int argc, char **argv)
 				return -1;
 			}
 			break;
+
 		case 'P':
 			printf("%s\n", str2);
 			promiscuous_on = 1;
@@ -563,89 +586,71 @@ parse_args(int argc, char **argv)
 			break;
 
 		/* long options */
-		case 0:
-			if (!strncmp(lgopts[option_index].name,
-					CMD_LINE_OPT_CONFIG,
-					sizeof(CMD_LINE_OPT_CONFIG))) {
-
-				ret = parse_config(optarg);
-				if (ret) {
-					printf("%s\n", str5);
-					print_usage(prgname);
-					return -1;
-				}
-			}
-
-			if (!strncmp(lgopts[option_index].name,
-					CMD_LINE_OPT_ETH_DEST,
-					sizeof(CMD_LINE_OPT_ETH_DEST))) {
-					parse_eth_dest(optarg);
-			}
-
-			if (!strncmp(lgopts[option_index].name,
-					CMD_LINE_OPT_NO_NUMA,
-					sizeof(CMD_LINE_OPT_NO_NUMA))) {
-				printf("%s\n", str6);
-				numa_on = 0;
+		case CMD_LINE_OPT_CONFIG_NUM:
+			ret = parse_config(optarg);
+			if (ret) {
+				printf("%s\n", str5);
+				print_usage(prgname);
+				return -1;
 			}
+			break;
 
-			if (!strncmp(lgopts[option_index].name,
-				CMD_LINE_OPT_IPV6,
-				sizeof(CMD_LINE_OPT_IPV6))) {
-				printf("%sn", str7);
-				ipv6 = 1;
-			}
+		case CMD_LINE_OPT_ETH_DEST_NUM:
+			parse_eth_dest(optarg);
+			break;
 
-			if (!strncmp(lgopts[option_index].name,
-					CMD_LINE_OPT_ENABLE_JUMBO,
-					sizeof(CMD_LINE_OPT_ENABLE_JUMBO))) {
-				struct option lenopts = {
-					"max-pkt-len", required_argument, 0, 0
-				};
-
-				printf("%s\n", str8);
-				port_conf.rxmode.jumbo_frame = 1;
-
-				/*
-				 * if no max-pkt-len set, use the default
-				 * value 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("%s\n", str9);
-						print_usage(prgname);
-						return -1;
-					}
-					port_conf.rxmode.max_rx_pkt_len = ret;
-				}
-				printf("%s %u\n", str10,
-				(unsigned int)port_conf.rxmode.max_rx_pkt_len);
-			}
+		case CMD_LINE_OPT_NO_NUMA_NUM:
+			printf("%s\n", str6);
+			numa_on = 0;
+			break;
 
-			if (!strncmp(lgopts[option_index].name,
-				CMD_LINE_OPT_HASH_ENTRY_NUM,
-				sizeof(CMD_LINE_OPT_HASH_ENTRY_NUM))) {
+		case CMD_LINE_OPT_IPV6_NUM:
+			printf("%sn", str7);
+			ipv6 = 1;
+			break;
 
-				ret = parse_hash_entry_number(optarg);
-				if ((ret > 0) && (ret <= L3FWD_HASH_ENTRIES)) {
-					hash_entry_number = ret;
-				} else {
-					printf("%s\n", str11);
+		case CMD_LINE_OPT_ENABLE_JUMBO_NUM: {
+			struct option lenopts = {
+				"max-pkt-len", required_argument, 0, 0
+			};
+
+			printf("%s\n", str8);
+			port_conf.rxmode.jumbo_frame = 1;
+
+			/*
+			 * if no max-pkt-len set, use the default
+			 * value 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("%s\n", str9);
 					print_usage(prgname);
 					return -1;
 				}
+				port_conf.rxmode.max_rx_pkt_len = ret;
 			}
+			printf("%s %u\n", str10,
+				(unsigned int)port_conf.rxmode.max_rx_pkt_len);
+			break;
+		}
 
-			if (!strncmp(lgopts[option_index].name,
-				     CMD_LINE_OPT_PARSE_PTYPE,
-				     sizeof(CMD_LINE_OPT_PARSE_PTYPE))) {
-				printf("soft parse-ptype is enabled\n");
-				parse_ptype = 1;
+		case CMD_LINE_OPT_HASH_ENTRY_NUM_NUM:
+			ret = parse_hash_entry_number(optarg);
+			if ((ret > 0) && (ret <= L3FWD_HASH_ENTRIES)) {
+				hash_entry_number = ret;
+			} else {
+				printf("%s\n", str11);
+				print_usage(prgname);
+				return -1;
 			}
+			break;
 
+		case CMD_LINE_OPT_PARSE_PTYPE_NUM:
+			printf("soft parse-ptype is enabled\n");
+			parse_ptype = 1;
 			break;
 
 		default:
-- 
2.8.1

[PATCH 2/2] l2fwd: rework long options parsing

[Olivier Matz]

Do the same than in l3fwd to avoid strcmp() for long options.

For l2fwd, there is no long option that take advantage of this new
mechanism as --mac-updating and --no-mac-updating are directly setting a
flag without needing an entry in the switch/case.

So this patch just prepares the framework in case a new long option is
added in the future.

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 examples/l2fwd/main.c | 30 ++++++++++++++++++++++++------
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/examples/l2fwd/main.c b/examples/l2fwd/main.c
index b2f5851..97d6454 100644
--- a/examples/l2fwd/main.c
+++ b/examples/l2fwd/main.c
@@ -392,6 +392,29 @@ l2fwd_parse_timer_period(const char *q_arg)
 	return n;
 }
 
+static const char short_options[] =
+	"p:"  /* portmask */
+	"q:"  /* number of queues */
+	"T:"  /* timer period */
+	;
+
+#define CMD_LINE_OPT_MAC_UPDATING "mac-updating"
+#define CMD_LINE_OPT_NO_MAC_UPDATING "no-mac-updating"
+
+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,
+};
+
+static const struct option lgopts[] = {
+	{ CMD_LINE_OPT_MAC_UPDATING, no_argument, &mac_updating, 1},
+	{ CMD_LINE_OPT_NO_MAC_UPDATING, no_argument, &mac_updating, 0},
+	{NULL, 0, 0, 0}
+};
+
 /* Parse the argument given in the command line of the application */
 static int
 l2fwd_parse_args(int argc, char **argv)
@@ -400,15 +423,10 @@ l2fwd_parse_args(int argc, char **argv)
 	char **argvopt;
 	int option_index;
 	char *prgname = argv[0];
-	static struct option lgopts[] = {
-		{ "mac-updating", no_argument, &mac_updating, 1},
-		{ "no-mac-updating", no_argument, &mac_updating, 0},
-		{NULL, 0, 0, 0}
-	};
 
 	argvopt = argv;
 
-	while ((opt = getopt_long(argc, argvopt, "p:q:T:",
+	while ((opt = getopt_long(argc, argvopt, short_options,
 				  lgopts, &option_index)) != EOF) {
 
 		switch (opt) {
-- 
2.8.1

Re: [PATCH 0/2] l2fwd/l3fwd: rework long options parsing

[Olivier Matz]

Hi,

On 11/22/2016 02:52 PM, Olivier Matz wrote:
> These 2 patches were part of this RFC, which will not be integrated:
> http://dpdk.org/ml/archives/dev/2016-September/046974.html
> 
> It does not bring any functional change, it just reworks the way long
> options are parsed in l2fwd and l3fwd to avoid uneeded strcmp() calls
> and to ease the addition of a new long option in the future.
> 
> I send them in case maintainers think it is better this way, but I have
> no real need.
> 
> Olivier Matz (2):
>   l3fwd: rework long options parsing
>   l2fwd: rework long options parsing
> 
>  examples/l2fwd/main.c |  30 +++++++--
>  examples/l3fwd/main.c | 169 ++++++++++++++++++++++++++------------------------
>  2 files changed, 111 insertions(+), 88 deletions(-)
> 

Sorry, I missed some checkpatch issues. I'll fix them in v2.
I'm waiting a bit for other comments, in case of.


Olivier

Re: [PATCH 0/2] l2fwd/l3fwd: rework long options parsing

[Thomas Monjalon]

2016-11-22 14:59, Olivier Matz:
> Hi,
> 
> On 11/22/2016 02:52 PM, Olivier Matz wrote:
> > These 2 patches were part of this RFC, which will not be integrated:
> > http://dpdk.org/ml/archives/dev/2016-September/046974.html
> > 
> > It does not bring any functional change, it just reworks the way long
> > options are parsed in l2fwd and l3fwd to avoid uneeded strcmp() calls
> > and to ease the addition of a new long option in the future.
> > 
> > I send them in case maintainers think it is better this way, but I have
> > no real need.
> > 
> > Olivier Matz (2):
> >   l3fwd: rework long options parsing
> >   l2fwd: rework long options parsing
> > 
> >  examples/l2fwd/main.c |  30 +++++++--
> >  examples/l3fwd/main.c | 169 ++++++++++++++++++++++++++------------------------
> >  2 files changed, 111 insertions(+), 88 deletions(-)
> > 
> 
> Sorry, I missed some checkpatch issues. I'll fix them in v2.
> I'm waiting a bit for other comments, in case of.

There was no comment.
It seems this version is good enough.

Applied with a checkpatch fix, thanks