[dpdk-dev] [PATCH 2/3] distributor_app: code review comments implementation

[reshmapa <reshma.pattan@intel.com>]

From: Reshma Pattan <reshma.pattan@intel.com>

    * support provided for command-line option portmask.
    * removed -g option in make file
    * spacing adjustment in header files inclusion in main.c
    * removal of const +3 in delcarration of rte_mbuf inside
      output_buffer structure declaration

    Changes in V2:

Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
---
 examples/distributor_app/Makefile |   2 +-
 examples/distributor_app/main.c   | 137 ++++++++++++++++++++++++++++++++++++--
 2 files changed, 133 insertions(+), 6 deletions(-)

diff --git a/examples/distributor_app/Makefile b/examples/distributor_app/Makefile
index d46746e..394785d 100644
--- a/examples/distributor_app/Makefile
+++ b/examples/distributor_app/Makefile
@@ -52,6 +52,6 @@ ifeq ($(CONFIG_RTE_TOOLCHAIN_GCC),y)
 CFLAGS_main.o += -Wno-return-type
 endif
 
-EXTRA_CFLAGS += -O3 -g -Wfatal-errors
+EXTRA_CFLAGS += -O3 -Wfatal-errors
 
 include $(RTE_SDK)/mk/rte.extapp.mk
diff --git a/examples/distributor_app/main.c b/examples/distributor_app/main.c
index 45d5bc1..fab8199 100644
--- a/examples/distributor_app/main.c
+++ b/examples/distributor_app/main.c
@@ -35,6 +35,7 @@
 #include <inttypes.h>
 #include <unistd.h>
 #include <signal.h>
+#include <getopt.h>
 
 #include <rte_eal.h>
 #include <rte_ethdev.h>
@@ -42,6 +43,7 @@
 #include <rte_malloc.h>
 #include <rte_debug.h>
 #include <rte_distributor.h>
+
 #include "main.h"
 
 #define RX_RING_SIZE 256
@@ -66,6 +68,9 @@
 #define BURST_SIZE 32
 #define RTE_RING_SZ 1024
 
+/* mask of enabled ports */
+static uint32_t enabled_port_mask = 0;
+
 static volatile struct app_stats {
 	struct {
 		uint64_t rx_pkts;
@@ -127,7 +132,7 @@ static const struct rte_eth_txconf tx_conf_default = {
 
 struct output_buffer {
 	unsigned count;
-	struct rte_mbuf *mbufs[BURST_SIZE + 3];
+	struct rte_mbuf *mbufs[BURST_SIZE];
 };
 
 /*
@@ -210,16 +215,27 @@ lcore_rx(struct lcore_params *p)
 	const int socket_id = rte_socket_id();
 	uint8_t port;
 
-	for (port = 0; port < nb_ports; port++)
+	for (port = 0; port < nb_ports; port++){
+		/* skip ports that are not enabled */
+		if ((enabled_port_mask & (1 << port)) == 0){
+			continue;
+		}
 		if (rte_eth_dev_socket_id(port) > 0 &&
 				rte_eth_dev_socket_id(port) != socket_id)
 			printf("WARNING, port %u is on remote NUMA node to "
 					"RX thread.\n\tPerformance will not "
 					"be optimal.\n", port);
+	}
 
 	printf("\nCore %u doing packet RX.\n", rte_lcore_id());
 	port = 0;
 	for (;;) {
+		/* skip ports that are not enabled */ 
+		if ((enabled_port_mask & (1 << port)) == 0){
+			if (++port == nb_ports)
+				port = 0;
+			continue;
+		}
 		struct rte_mbuf *bufs[BURST_SIZE*2];
 		const uint16_t nb_rx = rte_eth_rx_burst(port, 0, bufs,
 				BURST_SIZE);
@@ -265,6 +281,11 @@ flush_all_ports(struct output_buffer *tx_buffers, uint8_t nb_ports)
 {
 	uint8_t outp;
 	for (outp = 0; outp < nb_ports; outp++) {
+		/* skip ports that are not enabled */
+		if ((enabled_port_mask & (1 << outp)) == 0){
+			continue; 
+		}
+
 		if (tx_buffers[outp].count == 0)
 			continue;
 
@@ -280,16 +301,25 @@ lcore_tx(struct rte_ring *in_r)
 	const int socket_id = rte_socket_id();
 	uint8_t port;
 
-	for (port = 0; port < nb_ports; port++)
+	for (port = 0; port < nb_ports; port++){
+		/* skip ports that are not enabled */ 
+		if ((enabled_port_mask & (1 << port)) == 0){
+			continue;
+		}
 		if (rte_eth_dev_socket_id(port) > 0 &&
 				rte_eth_dev_socket_id(port) != socket_id)
 			printf("WARNING, port %u is on remote NUMA node to "
 					"TX thread.\n\tPerformance will not "
 					"be optimal.\n", port);
+	}
 
 	printf("\nCore %u doing packet TX.\n", rte_lcore_id());
 	for (;;) {
 		for (port = 0; port < nb_ports; port++) {
+			/* skip ports that are not enabled */ 
+			if ((enabled_port_mask & (1 << port)) == 0){
+				continue;
+			}
 			struct rte_mbuf *bufs[BURST_SIZE];
 			const uint16_t nb_rx = rte_ring_dequeue_burst(in_r,
 					(void *)bufs, BURST_SIZE);
@@ -313,6 +343,10 @@ lcore_tx(struct rte_ring *in_r)
 				/* workers should update in_port to hold the
 				 * output port value */
 				outp = bufs[i]->pkt.in_port;
+				/* skip ports that are not enabled */
+				if ((enabled_port_mask & (1 << outp)) == 0){
+					continue;
+				}
 				outbuf = &tx_buffers[outp];
 				outbuf->mbufs[outbuf->count++] = bufs[i];
 				if (outbuf->count == BURST_SIZE)
@@ -370,6 +404,77 @@ int_handler(int sig_num)
 	exit(0);
 }
 
+/* display usage */
+static void
+print_usage(const char *prgname)
+{
+        printf("%s [EAL options] -- -p PORTMASK\n"
+               "  -p PORTMASK: hexadecimal bitmask of ports to configure\n",
+               prgname);
+}
+
+static int
+parse_portmask(const char *portmask)
+{
+        char *end = NULL;
+        unsigned long pm;
+
+        /* parse hexadecimal string */
+        pm = strtoul(portmask, &end, 16);
+        if ((portmask[0] == '\0') || (end == NULL) || (*end != '\0'))
+                return -1;
+
+        if (pm == 0)
+                return -1;
+
+        return pm;
+}
+
+/* Parse the argument given in the command line of the application */
+static int 
+parse_args(int argc, char **argv)
+{
+        int opt;
+        char **argvopt;
+        int option_index;
+        char *prgname = argv[0];
+        static struct option lgopts[] = { 
+                {NULL, 0, 0, 0}
+        };  
+
+        argvopt = argv;
+
+        while ((opt = getopt_long(argc, argvopt, "p:",
+                                  lgopts, &option_index)) != EOF) {
+
+                switch (opt) {
+                /* portmask */
+                case 'p':
+                        enabled_port_mask = parse_portmask(optarg);
+                        if (enabled_port_mask == 0) {
+                                printf("invalid portmask\n");
+                                print_usage(prgname);
+                                return -1; 
+                        }   
+                        break;
+		
+		default:
+			print_usage(prgname);
+			return -1;
+		}
+        }
+	
+	if (optind <= 1) {
+		print_usage(prgname);
+		return -1;
+	}
+
+	argv[optind-1] = prgname;
+
+        optind = 0; /* reset getopt lib */
+        return 0;
+}
+
 /* Main function, does initialization and calls the per-lcore functions */
 int
 MAIN(int argc, char *argv[])
@@ -380,6 +485,7 @@ MAIN(int argc, char *argv[])
 	unsigned lcore_id, worker_id = 0;
 	unsigned nb_ports;
 	uint8_t portid;
+	uint8_t nb_ports_available;
 
 	/* catch ctrl-c so we can print on exit */
 	signal(SIGINT, int_handler);
@@ -391,6 +497,11 @@ MAIN(int argc, char *argv[])
 	argc -= ret;
 	argv += ret;
 
+        /* parse application arguments (after the EAL ones) */
+        ret = parse_args(argc, argv);
+        if (ret < 0) 
+                rte_exit(EXIT_FAILURE, "Invalid distributor parameters\n");
+
 	if (rte_lcore_count() < 3)
 		rte_exit(EXIT_FAILURE, "Error, This application needs at "
 				"least 3 logical cores to run:\n"
@@ -417,11 +528,28 @@ MAIN(int argc, char *argv[])
 	if (mbuf_pool == NULL)
 		rte_exit(EXIT_FAILURE, "Cannot create mbuf pool\n");
 
+	nb_ports_available = nb_ports;
+
 	/* initialize all ports */
-	for (portid = 0; portid < nb_ports; portid++)
+	for (portid = 0; portid < nb_ports; portid++){
+		/* skip ports that are not enabled */
+		if ((enabled_port_mask & (1 << portid)) == 0){
+			printf("\nSkipping disabled port %d\n", portid);
+			nb_ports_available--;
+			continue;
+		}
+		/* init port */
+                printf("Initializing port %u... done\n", (unsigned) portid);
+
 		if (port_init(portid, mbuf_pool) != 0)
 			rte_exit(EXIT_FAILURE, "Cannot initialize port %"PRIu8"\n",
 					portid);
+	}
+
+        if (!nb_ports_available) {
+                rte_exit(EXIT_FAILURE,
+                        "All available ports are disabled. Please set portmask.\n");
+        }   
 
 	d = rte_distributor_create("PKT_DIST", rte_socket_id(),
 			rte_lcore_count() - 2);
@@ -456,4 +584,3 @@ MAIN(int argc, char *argv[])
 	lcore_rx(&p);
 	return 0;
 }
-
-- 
1.8.3.1