[8/8] examples/tep_termination: enhance getopt_long usage

Message ID 20201029125339.30916-8-ibtisam.tariq@emumba.com (mailing list archive)
State Rejected, archived
Delegated to: David Marchand
Headers
Series [1/8] examples/fips_validation: enhance getopt_long usage |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-testing fail Testing issues
ci/iol-intel-Performance success Performance Testing PASS
ci/Intel-compilation fail apply issues
ci/iol-mellanox-Performance success Performance Testing PASS

Commit Message

Ibtisam Tariq Oct. 29, 2020, 12:53 p.m. UTC
  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.

Bugzilla ID: 238
Fixes: a50245ede7 ("examples/tep_term: initialize VXLAN sample")
Fixes: 2bb43bd435 ("examples/tep_term: add TSO offload configuration")
Fixes: 39c6daca9b ("examples/tep_term: add UDP tunneling port configuration")
Fixes: 9b96dd2609 ("examples/tep_term: add inner checksum Tx offload configuration")
Fixes: e627e8843d ("examples/tep_term: add tunnel filter type configuration")
Fixes: c6a0fb5f54 ("examples/tep_term: add encap/decap configuration")
Cc: jijiang.liu@intel.com

Reported-by: David Marchand <david.marchand@redhat.com>
Signed-off-by: Ibtisam Tariq <ibtisam.tariq@emumba.com>
---
 examples/tep_termination/main.c | 341 ++++++++++++++++----------------
 1 file changed, 173 insertions(+), 168 deletions(-)
  

Comments

David Marchand Oct. 29, 2020, 1:16 p.m. UTC | #1
Hello Ibtisam,

On Thu, Oct 29, 2020 at 1:58 PM Ibtisam Tariq <ibtisam.tariq@emumba.com> wrote:
>
> 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.
>
> Bugzilla ID: 238
> Fixes: a50245ede7 ("examples/tep_term: initialize VXLAN sample")
> Fixes: 2bb43bd435 ("examples/tep_term: add TSO offload configuration")
> Fixes: 39c6daca9b ("examples/tep_term: add UDP tunneling port configuration")
> Fixes: 9b96dd2609 ("examples/tep_term: add inner checksum Tx offload configuration")
> Fixes: e627e8843d ("examples/tep_term: add tunnel filter type configuration")
> Fixes: c6a0fb5f54 ("examples/tep_term: add encap/decap configuration")
> Cc: jijiang.liu@intel.com
>
> Reported-by: David Marchand <david.marchand@redhat.com>
> Signed-off-by: Ibtisam Tariq <ibtisam.tariq@emumba.com>

This patch can be dropped.
Thomas just merged this example removal.
  
Ibtisam Tariq Nov. 2, 2020, 8:18 a.m. UTC | #2
Hi David,

Thank you for the update.

On Thu, Oct 29, 2020 at 6:16 PM David Marchand <david.marchand@redhat.com>
wrote:

> Hello Ibtisam,
>
> On Thu, Oct 29, 2020 at 1:58 PM Ibtisam Tariq <ibtisam.tariq@emumba.com>
> wrote:
> >
> > 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.
> >
> > Bugzilla ID: 238
> > Fixes: a50245ede7 ("examples/tep_term: initialize VXLAN sample")
> > Fixes: 2bb43bd435 ("examples/tep_term: add TSO offload configuration")
> > Fixes: 39c6daca9b ("examples/tep_term: add UDP tunneling port
> configuration")
> > Fixes: 9b96dd2609 ("examples/tep_term: add inner checksum Tx offload
> configuration")
> > Fixes: e627e8843d ("examples/tep_term: add tunnel filter type
> configuration")
> > Fixes: c6a0fb5f54 ("examples/tep_term: add encap/decap configuration")
> > Cc: jijiang.liu@intel.com
> >
> > Reported-by: David Marchand <david.marchand@redhat.com>
> > Signed-off-by: Ibtisam Tariq <ibtisam.tariq@emumba.com>
>
> This patch can be dropped.
> Thomas just merged this example removal.
>
>
> --
> David Marchand
>
>
  

Patch

diff --git a/examples/tep_termination/main.c b/examples/tep_termination/main.c
index 15bf8bbf7..089d8cc7a 100644
--- a/examples/tep_termination/main.c
+++ b/examples/tep_termination/main.c
@@ -75,18 +75,32 @@ 
 /* Used to compare MAC addresses. */
 #define MAC_ADDR_CMP 0xFFFFFFFFFFFFULL
 
+enum {
 #define CMD_LINE_OPT_NB_DEVICES "nb-devices"
+	CMD_LINE_OPT_NB_DEVICES_NUM = 256,
 #define CMD_LINE_OPT_UDP_PORT "udp-port"
+	CMD_LINE_OPT_UDP_PORT_NUM,
 #define CMD_LINE_OPT_TX_CHECKSUM "tx-checksum"
+	CMD_LINE_OPT_TX_CHECKSUM_NUM,
 #define CMD_LINE_OPT_TSO_SEGSZ "tso-segsz"
+	CMD_LINE_OPT_TSO_SEGSZ_NUM,
 #define CMD_LINE_OPT_FILTER_TYPE "filter-type"
+	CMD_LINE_OPT_FILTER_TYPE_NUM,
 #define CMD_LINE_OPT_ENCAP "encap"
+	CMD_LINE_OPT_ENCAP_NUM,
 #define CMD_LINE_OPT_DECAP "decap"
+	CMD_LINE_OPT_DECAP_NUM,
 #define CMD_LINE_OPT_RX_RETRY "rx-retry"
+	CMD_LINE_OPT_RX_RETRY_NUM,
 #define CMD_LINE_OPT_RX_RETRY_DELAY "rx-retry-delay"
-#define CMD_LINE_OPT_RX_RETRY_NUM "rx-retry-num"
+	CMD_LINE_OPT_RX_RETRY_DELAY_NUM,
+#define CMD_LINE_OPT_RX_RETRY_NUMB "rx-retry-num"
+	CMD_LINE_OPT_RX_RETRY_NUMB_NUM,
 #define CMD_LINE_OPT_STATS "stats"
+	CMD_LINE_OPT_STATS_NUM,
 #define CMD_LINE_OPT_DEV_BASENAME "dev-basename"
+	CMD_LINE_OPT_DEV_BASENAME_NUM,
+};
 
 /* mask of enabled ports */
 static uint32_t enabled_port_mask;
@@ -268,18 +282,30 @@  tep_termination_parse_args(int argc, char **argv)
 	unsigned i;
 	const char *prgname = argv[0];
 	static struct option long_option[] = {
-		{CMD_LINE_OPT_NB_DEVICES, required_argument, NULL, 0},
-		{CMD_LINE_OPT_UDP_PORT, required_argument, NULL, 0},
-		{CMD_LINE_OPT_TX_CHECKSUM, required_argument, NULL, 0},
-		{CMD_LINE_OPT_TSO_SEGSZ, required_argument, NULL, 0},
-		{CMD_LINE_OPT_DECAP, required_argument, NULL, 0},
-		{CMD_LINE_OPT_ENCAP, required_argument, NULL, 0},
-		{CMD_LINE_OPT_FILTER_TYPE, required_argument, NULL, 0},
-		{CMD_LINE_OPT_RX_RETRY, required_argument, NULL, 0},
-		{CMD_LINE_OPT_RX_RETRY_DELAY, required_argument, NULL, 0},
-		{CMD_LINE_OPT_RX_RETRY_NUM, required_argument, NULL, 0},
-		{CMD_LINE_OPT_STATS, required_argument, NULL, 0},
-		{CMD_LINE_OPT_DEV_BASENAME, required_argument, NULL, 0},
+		{CMD_LINE_OPT_NB_DEVICES, required_argument,
+				NULL, CMD_LINE_OPT_NB_DEVICES_NUM},
+		{CMD_LINE_OPT_UDP_PORT, required_argument,
+				NULL, CMD_LINE_OPT_UDP_PORT_NUM},
+		{CMD_LINE_OPT_TX_CHECKSUM, required_argument,
+				NULL, CMD_LINE_OPT_TX_CHECKSUM_NUM},
+		{CMD_LINE_OPT_TSO_SEGSZ, required_argument,
+				NULL, CMD_LINE_OPT_TSO_SEGSZ_NUM},
+		{CMD_LINE_OPT_DECAP, required_argument,
+				NULL, CMD_LINE_OPT_DECAP_NUM},
+		{CMD_LINE_OPT_ENCAP, required_argument,
+				NULL, CMD_LINE_OPT_ENCAP_NUM},
+		{CMD_LINE_OPT_FILTER_TYPE, required_argument,
+				NULL, CMD_LINE_OPT_FILTER_TYPE_NUM},
+		{CMD_LINE_OPT_RX_RETRY, required_argument,
+				NULL, CMD_LINE_OPT_RX_RETRY_NUM},
+		{CMD_LINE_OPT_RX_RETRY_DELAY, required_argument,
+				NULL, CMD_LINE_OPT_RX_RETRY_DELAY_NUM},
+		{CMD_LINE_OPT_RX_RETRY_NUMB, required_argument,
+				NULL, CMD_LINE_OPT_RX_RETRY_NUMB_NUM},
+		{CMD_LINE_OPT_STATS, required_argument,
+				NULL, CMD_LINE_OPT_STATS_NUM},
+		{CMD_LINE_OPT_DEV_BASENAME, required_argument,
+				NULL, CMD_LINE_OPT_DEV_BASENAME_NUM},
 		{NULL, 0, 0, 0},
 	};
 
@@ -297,174 +323,153 @@  tep_termination_parse_args(int argc, char **argv)
 				return -1;
 			}
 			break;
-		case 0:
-			if (!strncmp(long_option[option_index].name,
-				CMD_LINE_OPT_NB_DEVICES,
-				sizeof(CMD_LINE_OPT_NB_DEVICES))) {
-				ret = parse_num_opt(optarg, MAX_DEVICES);
-				if (ret == -1) {
-					RTE_LOG(INFO, VHOST_CONFIG,
-					"Invalid argument for nb-devices [0-%d]\n",
-					MAX_DEVICES);
-					tep_termination_usage(prgname);
-					return -1;
-				} else
-					nb_devices = ret;
-			}
 
-			/* Enable/disable retries on RX. */
-			if (!strncmp(long_option[option_index].name,
-				CMD_LINE_OPT_RX_RETRY,
-				sizeof(CMD_LINE_OPT_RX_RETRY))) {
-				ret = parse_num_opt(optarg, 1);
-				if (ret == -1) {
-					RTE_LOG(INFO, VHOST_CONFIG,
-						"Invalid argument for rx-retry [0|1]\n");
-					tep_termination_usage(prgname);
-					return -1;
-				} else
-					enable_retry = ret;
+		case CMD_LINE_OPT_NB_DEVICES_NUM:
+		{
+			ret = parse_num_opt(optarg, MAX_DEVICES);
+			if (ret == -1) {
+				RTE_LOG(INFO, VHOST_CONFIG,
+				"Invalid argument for nb-devices [0-%d]\n",
+				MAX_DEVICES);
+				tep_termination_usage(prgname);
+				return -1;
 			}
-
-			if (!strncmp(long_option[option_index].name,
-				CMD_LINE_OPT_TSO_SEGSZ,
-				sizeof(CMD_LINE_OPT_TSO_SEGSZ))) {
-				ret = parse_num_opt(optarg, INT16_MAX);
-				if (ret == -1) {
-					RTE_LOG(INFO, VHOST_CONFIG,
-						"Invalid argument for TCP segment size [0-N]\n");
-					tep_termination_usage(prgname);
-					return -1;
-				} else
-					tso_segsz = ret;
+			nb_devices = ret;
+			break;
+		}
+		case CMD_LINE_OPT_RX_RETRY_NUM:
+		{
+			ret = parse_num_opt(optarg, 1);
+			if (ret == -1) {
+				RTE_LOG(INFO, VHOST_CONFIG,
+					"Invalid argument for rx-retry [0|1]\n");
+				tep_termination_usage(prgname);
+				return -1;
 			}
-
-			if (!strncmp(long_option[option_index].name,
-					CMD_LINE_OPT_UDP_PORT,
-					sizeof(CMD_LINE_OPT_UDP_PORT))) {
-				ret = parse_num_opt(optarg, INT16_MAX);
-				if (ret == -1) {
-					RTE_LOG(INFO, VHOST_CONFIG,
-						"Invalid argument for UDP port [0-N]\n");
-					tep_termination_usage(prgname);
-					return -1;
-				} else
-					udp_port = ret;
+			enable_retry = ret;
+			break;
+		}
+		case CMD_LINE_OPT_TSO_SEGSZ_NUM:
+		{
+			ret = parse_num_opt(optarg, INT16_MAX);
+			if (ret == -1) {
+				RTE_LOG(INFO, VHOST_CONFIG,
+					"Invalid argument for TCP segment size [0-N]\n");
+				tep_termination_usage(prgname);
+				return -1;
 			}
-
-			/* Specify the retries delay time (in useconds) on RX.*/
-			if (!strncmp(long_option[option_index].name,
-				CMD_LINE_OPT_RX_RETRY_DELAY,
-				sizeof(CMD_LINE_OPT_RX_RETRY_DELAY))) {
-				ret = parse_num_opt(optarg, INT32_MAX);
-				if (ret == -1) {
-					RTE_LOG(INFO, VHOST_CONFIG,
-						"Invalid argument for rx-retry-delay [0-N]\n");
-					tep_termination_usage(prgname);
-					return -1;
-				} else
-					burst_rx_delay_time = ret;
+			tso_segsz = ret;
+			break;
+		}
+		case CMD_LINE_OPT_UDP_PORT_NUM:
+		{
+			ret = parse_num_opt(optarg, INT16_MAX);
+			if (ret == -1) {
+				RTE_LOG(INFO, VHOST_CONFIG,
+					"Invalid argument for UDP port [0-N]\n");
+				tep_termination_usage(prgname);
+				return -1;
 			}
-
-			/* Specify the retries number on RX. */
-			if (!strncmp(long_option[option_index].name,
-				CMD_LINE_OPT_RX_RETRY_NUM,
-				sizeof(CMD_LINE_OPT_RX_RETRY_NUM))) {
-				ret = parse_num_opt(optarg, INT32_MAX);
-				if (ret == -1) {
-					RTE_LOG(INFO, VHOST_CONFIG,
-						"Invalid argument for rx-retry-num [0-N]\n");
-					tep_termination_usage(prgname);
-					return -1;
-				} else
-					burst_rx_retry_num = ret;
+			udp_port = ret;
+			break;
+		}
+		case CMD_LINE_OPT_RX_RETRY_DELAY_NUM:
+		{
+			ret = parse_num_opt(optarg, INT32_MAX);
+			if (ret == -1) {
+				RTE_LOG(INFO, VHOST_CONFIG,
+					"Invalid argument for rx-retry-delay [0-N]\n");
+				tep_termination_usage(prgname);
+				return -1;
 			}
-
-			if (!strncmp(long_option[option_index].name,
-				CMD_LINE_OPT_TX_CHECKSUM,
-				sizeof(CMD_LINE_OPT_TX_CHECKSUM))) {
-				ret = parse_num_opt(optarg, 1);
-				if (ret == -1) {
-					RTE_LOG(INFO, VHOST_CONFIG,
-						"Invalid argument for tx-checksum [0|1]\n");
-					tep_termination_usage(prgname);
-					return -1;
-				} else
-					tx_checksum = ret;
+			burst_rx_delay_time = ret;
+			break;
+		}
+		case CMD_LINE_OPT_RX_RETRY_NUMB_NUM:
+		{
+			ret = parse_num_opt(optarg, INT32_MAX);
+			if (ret == -1) {
+				RTE_LOG(INFO, VHOST_CONFIG,
+					"Invalid argument for rx-retry-num [0-N]\n");
+				tep_termination_usage(prgname);
+				return -1;
 			}
-
-			if (!strncmp(long_option[option_index].name,
-					CMD_LINE_OPT_FILTER_TYPE,
-					sizeof(CMD_LINE_OPT_FILTER_TYPE))) {
-				ret = parse_num_opt(optarg, 3);
-				if ((ret == -1) || (ret == 0)) {
-					RTE_LOG(INFO, VHOST_CONFIG,
-						"Invalid argument for filter type [1-3]\n");
-					tep_termination_usage(prgname);
-					return -1;
-				} else
-					filter_idx = ret - 1;
+			burst_rx_retry_num = ret;
+			break;
+		}
+		case CMD_LINE_OPT_TX_CHECKSUM_NUM:
+		{
+			ret = parse_num_opt(optarg, 1);
+			if (ret == -1) {
+				RTE_LOG(INFO, VHOST_CONFIG,
+					"Invalid argument for tx-checksum [0|1]\n");
+				tep_termination_usage(prgname);
+				return -1;
 			}
-
-			/* Enable/disable encapsulation on RX. */
-			if (!strncmp(long_option[option_index].name,
-				CMD_LINE_OPT_DECAP,
-				sizeof(CMD_LINE_OPT_DECAP))) {
-				ret = parse_num_opt(optarg, 1);
-				if (ret == -1) {
-					RTE_LOG(INFO, VHOST_CONFIG,
-						"Invalid argument for decap [0|1]\n");
-					tep_termination_usage(prgname);
-					return -1;
-				} else
-					rx_decap = ret;
+			tx_checksum = ret;
+			break;
+		}
+		case CMD_LINE_OPT_FILTER_TYPE_NUM:
+		{
+			ret = parse_num_opt(optarg, 3);
+			if ((ret == -1) || (ret == 0)) {
+				RTE_LOG(INFO, VHOST_CONFIG,
+					"Invalid argument for filter type [1-3]\n");
+				tep_termination_usage(prgname);
+				return -1;
 			}
-
-			/* Enable/disable encapsulation on TX. */
-			if (!strncmp(long_option[option_index].name,
-				CMD_LINE_OPT_ENCAP,
-				sizeof(CMD_LINE_OPT_ENCAP))) {
-				ret = parse_num_opt(optarg, 1);
-				if (ret == -1) {
-					RTE_LOG(INFO, VHOST_CONFIG,
-						"Invalid argument for encap [0|1]\n");
-					tep_termination_usage(prgname);
-					return -1;
-				} else
-					tx_encap = ret;
+			filter_idx = ret - 1;
+			break;
+		}
+		case CMD_LINE_OPT_DECAP_NUM:
+		{
+			ret = parse_num_opt(optarg, 1);
+			if (ret == -1) {
+				RTE_LOG(INFO, VHOST_CONFIG,
+					"Invalid argument for decap [0|1]\n");
+				tep_termination_usage(prgname);
+				return -1;
 			}
-
-			/* Enable/disable stats. */
-			if (!strncmp(long_option[option_index].name,
-				CMD_LINE_OPT_STATS,
-				sizeof(CMD_LINE_OPT_STATS))) {
-				ret = parse_num_opt(optarg, INT32_MAX);
-				if (ret == -1) {
-					RTE_LOG(INFO, VHOST_CONFIG,
-							"Invalid argument for stats [0..N]\n");
-					tep_termination_usage(prgname);
-					return -1;
-				} else
-					enable_stats = ret;
+			rx_decap = ret;
+			break;
+		}
+		case CMD_LINE_OPT_ENCAP_NUM:
+		{
+			ret = parse_num_opt(optarg, 1);
+			if (ret == -1) {
+				RTE_LOG(INFO, VHOST_CONFIG,
+					"Invalid argument for encap [0|1]\n");
+				tep_termination_usage(prgname);
+				return -1;
 			}
-
-			/* Set character device basename. */
-			if (!strncmp(long_option[option_index].name,
-				CMD_LINE_OPT_DEV_BASENAME,
-				sizeof(CMD_LINE_OPT_DEV_BASENAME))) {
-				if (us_vhost_parse_basename(optarg) == -1) {
-					RTE_LOG(INFO, VHOST_CONFIG,
-						"Invalid argument for character "
-						"device basename (Max %d characters)\n",
-						MAX_BASENAME_SZ);
-					tep_termination_usage(prgname);
-					return -1;
-				}
+			tx_encap = ret;
+			break;
+		}
+		case CMD_LINE_OPT_STATS_NUM:
+		{
+			ret = parse_num_opt(optarg, INT32_MAX);
+			if (ret == -1) {
+				RTE_LOG(INFO, VHOST_CONFIG,
+						"Invalid argument for stats [0..N]\n");
+				tep_termination_usage(prgname);
+				return -1;
 			}
-
+			enable_stats = ret;
 			break;
-
-			/* Invalid option - print options. */
+		}
+		case CMD_LINE_OPT_DEV_BASENAME_NUM:
+		{
+			if (us_vhost_parse_basename(optarg) == -1) {
+				RTE_LOG(INFO, VHOST_CONFIG,
+					"Invalid argument for character "
+					"device basename (Max %d characters)\n",
+					MAX_BASENAME_SZ);
+				tep_termination_usage(prgname);
+				return -1;
+			}
+			break;
+		}
+		/* Invalid option - print options. */
 		default:
 			tep_termination_usage(prgname);
 			return -1;