[spp] [PATCH 0/3] Refactor spp_priamry

[spp] [PATCH 0/3] Refactor spp_priamry

[ogawa.yasufumi]

From: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>

Implementation of spp_primary is derived from DPDK's multi process
sample app, and some of variable names are remained old after the usage
is changed. For example, `client` or `num_client` was used for rings for
communicating with clients, but it is used for ring port used to
forward. This series of patches is to refactor spp_primary and its
Makefile.

Yasufumi Ogawa (3):
  spp_primary: refactor var names for ring ports
  spp_primary: change include path in Makefile
  spp_primary: add lcore_id_used variable

 src/primary/Makefile |  2 +-
 src/primary/args.c   | 12 +++++------
 src/primary/args.h   |  4 ++--
 src/primary/init.c   | 57 ++++++++++++++++++++++++++++++----------------------
 src/primary/init.h   | 21 ++++++++++++-------
 src/primary/main.c   | 11 +++++-----
 src/shared/common.c  |  8 ++++++--
 src/shared/common.h  |  2 ++
 8 files changed, 70 insertions(+), 47 deletions(-)

-- 
2.7.4

[spp] [PATCH 1/3] spp_primary: refactor var names for ring ports

[ogawa.yasufumi]

From: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>

Spp_priamry has ring ports and its stats as structure `client` and
`num_clients` because ring was used only for communication between
primary and secondary. However, it is used for packet forwarding
betweeen secondaries, not primary.

This update is to change the var names of `client` to `ring` to avoid
misunderstanding.

Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
 src/primary/args.c  | 10 +++++-----
 src/primary/args.h  |  2 +-
 src/primary/init.c  | 41 ++++++++++++++++++-----------------------
 src/primary/init.h  | 21 ++++++++++++++-------
 src/primary/main.c  | 11 ++++++-----
 src/shared/common.c |  8 ++++++--
 6 files changed, 50 insertions(+), 43 deletions(-)

diff --git a/src/primary/args.c b/src/primary/args.c
index f882ace..d0a28e8 100644
--- a/src/primary/args.c
+++ b/src/primary/args.c
@@ -12,8 +12,8 @@
 #include "init.h"
 #include "primary.h"
 
-/* global var for number of clients - extern in header */
-uint16_t num_clients;
+/* global var for number of rings - extern in header */
+uint16_t num_rings;
 char *server_ip;
 int server_port;
 
@@ -28,7 +28,7 @@ usage(void)
 	RTE_LOG(INFO, PRIMARY,
 	    "%s [EAL options] -- -p PORTMASK -n NUM_CLIENTS [-s NUM_SOCKETS]\n"
 	    " -p PORTMASK: hexadecimal bitmask of ports to use\n"
-	    " -n NUM_CLIENTS: number of client processes to use\n"
+	    " -n NUM_RINGS: number of ring ports used from secondaries\n"
 	    , progname);
 }
 
@@ -97,7 +97,7 @@ parse_app_args(uint16_t max_ports, int argc, char *argv[])
 			}
 			break;
 		case 'n':
-			if (parse_num_clients(&num_clients, optarg) != 0) {
+			if (parse_num_clients(&num_rings, optarg) != 0) {
 				usage();
 				return -1;
 			}
@@ -117,7 +117,7 @@ parse_app_args(uint16_t max_ports, int argc, char *argv[])
 		}
 	}
 
-	if (ports->num_ports == 0 || num_clients == 0) {
+	if (ports->num_ports == 0 || num_rings == 0) {
 		usage();
 		return -1;
 	}
diff --git a/src/primary/args.h b/src/primary/args.h
index bbaf993..8561bd3 100644
--- a/src/primary/args.h
+++ b/src/primary/args.h
@@ -9,7 +9,7 @@
 #include <stdint.h>
 #include "common.h"
 
-extern uint16_t num_clients;
+extern uint16_t num_rings;
 extern char *server_ip;
 extern int server_port;
 
diff --git a/src/primary/init.c b/src/primary/init.c
index 2e33df9..8b1f396 100644
--- a/src/primary/init.c
+++ b/src/primary/init.c
@@ -14,14 +14,8 @@
 #include "init.h"
 #include "primary.h"
 
-#define CLIENT_QUEUE_RINGSIZE 128
-
-#define MBUFS_PER_CLIENT 1536
-#define MBUFS_PER_PORT 1536
-#define MBUF_CACHE_SIZE 512
-
-/* array of info/queues for clients */
-struct client *clients;
+/* array of info/queues for ring_ports */
+struct ring_port *ring_ports;
 
 /* The mbuf pool for packet rx */
 static struct rte_mempool *pktmbuf_pool;
@@ -36,7 +30,7 @@ struct port_info *ports;
 static int
 init_mbuf_pools(void)
 {
-	const unsigned int num_mbufs = (num_clients * MBUFS_PER_CLIENT)
+	const unsigned int num_mbufs = (num_rings * MBUFS_PER_CLIENT)
 		+ (ports->num_ports * MBUFS_PER_PORT);
 
 	/*
@@ -65,7 +59,7 @@ init_mbuf_pools(void)
 /**
  * Set up the DPDK rings which will be used to pass packets, via
  * pointers, between the multi-process server and client processes.
- * Each client needs one RX queue.
+ * Each ring_port needs one RX queue.
  */
 static int
 init_shm_rings(void)
@@ -75,27 +69,27 @@ init_shm_rings(void)
 	const char *q_name;
 	unsigned int i;
 
-	clients = rte_malloc("client details",
-		sizeof(*clients) * num_clients, 0);
-	if (clients == NULL)
+	ring_ports = rte_malloc("ring_port details",
+		sizeof(*ring_ports) * num_rings, 0);
+	if (ring_ports == NULL)
 		rte_exit(EXIT_FAILURE,
-			"Cannot allocate memory for client program details\n");
+			"Cannot allocate memory for ring_port details\n");
 
-	for (i = 0; i < num_clients; i++) {
-		/* Create an RX queue for each client */
+	for (i = 0; i < num_rings; i++) {
+		/* Create an RX queue for each ring_ports */
 		socket_id = rte_socket_id();
 		q_name = get_rx_queue_name(i);
 		if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
-			clients[i].rx_q = rte_ring_lookup(q_name);
+			ring_ports[i].rx_q = rte_ring_lookup(q_name);
 		} else {
-			clients[i].rx_q = rte_ring_create(q_name,
+			ring_ports[i].rx_q = rte_ring_create(q_name,
 				ringsize, socket_id,
 				/* single prod, single cons */
 				RING_F_SP_ENQ | RING_F_SC_DEQ);
 		}
-		if (clients[i].rx_q == NULL)
+		if (ring_ports[i].rx_q == NULL)
 			rte_exit(EXIT_FAILURE,
-				"Cannot create rx ring queue for client %u\n",
+				"Cannot create rx ring queue for ring_port %u\n",
 				i);
 	}
 
@@ -162,7 +156,7 @@ init(int argc, char *argv[])
 	}
 	check_all_ports_link_status(ports, ports->num_ports, (~0x0));
 
-	/* initialise the client queues/rings for inter-eu comms */
+	/* Initialise the ring_port. */
 	init_shm_rings();
 
 	return 0;
@@ -179,7 +173,7 @@ check_all_ports_link_status(struct port_info *ports, uint16_t port_num,
 	uint16_t portid;
 	struct rte_eth_link link;
 
-	RTE_LOG(INFO, PRIMARY, "\nChecking link status");
+	RTE_LOG(INFO, PRIMARY, "Checking link status ");
 	fflush(stdout);
 	for (count = 0; count <= MAX_CHECK_TIME; count++) {
 		all_ports_up = 1;
@@ -206,6 +200,7 @@ check_all_ports_link_status(struct port_info *ports, uint16_t port_num,
 			break;
 		}
 	}
+	printf("\n");
 
 	/* all ports up or timed out */
 	for (portid = 0; portid < port_num; portid++) {
@@ -253,7 +248,7 @@ init_port(uint16_t port_num, struct rte_mempool *pktmbuf_pool)
 	struct rte_eth_conf local_port_conf = port_conf;
 	struct rte_eth_txconf txq_conf;
 
-	RTE_LOG(INFO, PRIMARY, "Port %u init ... ", port_num);
+	RTE_LOG(INFO, PRIMARY, "Port %u init ...\n", port_num);
 	fflush(stdout);
 
 	rte_eth_dev_info_get(port_num, &dev_info);
diff --git a/src/primary/init.h b/src/primary/init.h
index 286bdb5..3ea69f0 100644
--- a/src/primary/init.h
+++ b/src/primary/init.h
@@ -8,20 +8,27 @@
 
 #include <stdint.h>
 
+#define CLIENT_QUEUE_RINGSIZE 128
+
+#define MBUFS_PER_CLIENT 1536
+#define MBUFS_PER_PORT 1536
+#define MBUF_CACHE_SIZE 512
+
 #define MBUF_OVERHEAD (sizeof(struct rte_mbuf) + RTE_PKTMBUF_HEADROOM)
 #define RX_MBUF_DATA_SIZE 2048
 #define MBUF_SIZE (RX_MBUF_DATA_SIZE + MBUF_OVERHEAD)
 
 /*
- * Define a client structure with all needed info, including
- * stats from the clients.
+ * Define a ring_port structure with all needed info, including
+ * stats from the ring_ports.
  */
-struct client {
+struct ring_port {
 	struct rte_ring *rx_q;
-	unsigned int client_id;
+	unsigned int ring_id;
 	/*
-	 * These stats hold how many packets the client will actually receive,
-	 * and how many packets were dropped because the client's queue was full
+	 * These stats hold how many packets the ring_port will actually
+	 * receive, and how many packets were dropped because the ring_port's
+	 * queue was full.
 	 * The port-info stats, in contrast, record how many packets were
 	 * received or transmitted on an actual NIC port.
 	 */
@@ -31,7 +38,7 @@ struct client {
 	} stats;
 };
 
-extern struct client *clients;
+extern struct ring_port *ring_ports;
 
 /* the shared port information: port numbers, rx and tx stats etc. */
 extern struct port_info *ports;
diff --git a/src/primary/main.c b/src/primary/main.c
index 81d1aec..a039b3f 100644
--- a/src/primary/main.c
+++ b/src/primary/main.c
@@ -20,6 +20,8 @@
 #define PRI_BUF_SIZE_PHY 512
 #define PRI_BUF_SIZE_RING 1512
 
+#define POLL_TIMEOUT_MS 100
+
 static sig_atomic_t on = 1;
 
 static enum cmd_type cmd = STOP;
@@ -89,7 +91,7 @@ do_stats_display(void)
 
 	printf("\nCLIENTS\n");
 	printf("-------\n");
-	for (i = 0; i < num_clients; i++) {
+	for (i = 0; i < num_rings; i++) {
 		printf("Client %2u - rx: %9"PRIu64", rx_drop: %9"PRIu64"\n"
 			"            tx: %9"PRIu64", tx_drop: %9"PRIu64"\n",
 			i, ports->client_stats[i].rx,
@@ -233,7 +235,7 @@ get_status_json(char *str)
 	}
 
 	char ring_port[buf_size];
-	for (i = 0; i < num_clients; i++) {
+	for (i = 0; i < num_rings; i++) {
 
 		RTE_LOG(DEBUG, PRIMARY, "Size of ring_ports str: %d\n",
 				(int)strlen(ring_ports));
@@ -254,14 +256,14 @@ get_status_json(char *str)
 		if (cur_buf_size > ringp_buf_size - 1) {
 			RTE_LOG(ERR, PRIMARY,
 				"Cannot send all of ring_port stats (%d/%d)\n",
-				i, num_clients);
+				i, num_rings);
 			sprintf(ring_ports + strlen(ring_ports) - 1, "%s", "");
 			break;
 		}
 
 		sprintf(ring_ports + strlen(ring_ports), "%s", ring_port);
 
-		if (i < num_clients - 1)
+		if (i < num_rings - 1)
 			sprintf(ring_ports, "%s,", ring_ports);
 	}
 
@@ -316,7 +318,6 @@ do_receive(int *connected, int *sock, char *str)
 
 	memset(str, '\0', MSG_SIZE);
 
-#define POLL_TIMEOUT_MS 100
 	ret = poll(&pfd, 1, POLL_TIMEOUT_MS);
 	if (ret <= 0) {
 		if (ret < 0) {
diff --git a/src/shared/common.c b/src/shared/common.c
index f7d522a..0943dd3 100644
--- a/src/shared/common.c
+++ b/src/shared/common.c
@@ -44,8 +44,12 @@ set_user_log_debug(int num_user_log)
 }
 
 /**
- * Take the number of clients parameter passed to the app
- * and convert to a number to store in the num_clients variable
+ * Take the number of clients passed with `-n` option and convert to
+ * to a number to store in the num_clients variable.
+ *
+ * TODO(yasufum): Revise the usage of this function for spp_primary because
+ * it does not use for the number of ring ports, but clients. The name of
+ * function is inadequte.
  */
 int
 parse_num_clients(uint16_t *num_clients, const char *clients)
-- 
2.7.4

[spp] [PATCH 2/3] spp_primary: change include path in Makefile

[ogawa.yasufumi]

From: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>

As a part of refactoring, change include path to include headers in
shared directory explicitly.

Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
 src/primary/Makefile | 2 +-
 src/primary/args.c   | 2 +-
 src/primary/args.h   | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/primary/Makefile b/src/primary/Makefile
index 59c9ba7..f348146 100644
--- a/src/primary/Makefile
+++ b/src/primary/Makefile
@@ -25,7 +25,7 @@ INC := $(wildcard *.h)
 
 CFLAGS += -DALLOW_EXPERIMENTAL_API
 CFLAGS += $(WERROR_FLAGS) -O3 -MMD
-CFLAGS += -I$(SRCDIR)/../shared
+CFLAGS += -I$(SRCDIR)/../
 
 ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),y)
 LDLIBS += -lrte_pmd_ring
diff --git a/src/primary/args.c b/src/primary/args.c
index d0a28e8..020e731 100644
--- a/src/primary/args.c
+++ b/src/primary/args.c
@@ -7,7 +7,7 @@
 
 #include <rte_memory.h>
 
-#include "common.h"
+#include "shared/common.h"
 #include "args.h"
 #include "init.h"
 #include "primary.h"
diff --git a/src/primary/args.h b/src/primary/args.h
index 8561bd3..29bca0d 100644
--- a/src/primary/args.h
+++ b/src/primary/args.h
@@ -7,7 +7,7 @@
 #define _PRIMARY_ARGS_H_
 
 #include <stdint.h>
-#include "common.h"
+#include "shared/common.h"
 
 extern uint16_t num_rings;
 extern char *server_ip;
-- 
2.7.4

[spp] [PATCH 3/3] spp_primary: add lcore_id_used variable

[ogawa.yasufumi]

From: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>

To enable to retrieve lcore IDs used by spp_primary, add
`lcore_id_used` variable. It is intended to be refered for inspecting
the rest of cores can be used.

Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
 src/primary/init.c  | 16 +++++++++++++++-
 src/shared/common.h |  2 ++
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/src/primary/init.c b/src/primary/init.c
index 8b1f396..85b54a3 100644
--- a/src/primary/init.c
+++ b/src/primary/init.c
@@ -9,8 +9,8 @@
 #include <rte_malloc.h>
 #include <rte_memzone.h>
 
+#include "shared/common.h"
 #include "args.h"
-#include "common.h"
 #include "init.h"
 #include "primary.h"
 
@@ -23,6 +23,8 @@ static struct rte_mempool *pktmbuf_pool;
 /* the port details */
 struct port_info *ports;
 
+uint8_t lcore_id_used[RTE_MAX_LCORE] = {};
+
 /**
  * Initialise the mbuf pool for packet reception for the NIC, and any other
  * buffer pools needed by the app - currently none.
@@ -104,8 +106,10 @@ int
 init(int argc, char *argv[])
 {
 	int retval;
+	int lcore_id;
 	const struct rte_memzone *mz;
 	uint16_t count, total_ports;
+	char log_msg[1024] = { '\0' };
 
 	/* init EAL, parsing EAL args */
 	retval = rte_eal_init(argc, argv);
@@ -159,6 +163,16 @@ init(int argc, char *argv[])
 	/* Initialise the ring_port. */
 	init_shm_rings();
 
+	RTE_LCORE_FOREACH(lcore_id) {
+		lcore_id_used[lcore_id] = 1;
+	}
+	sprintf(log_msg, "Used lcores: ");
+	for (int i = 0; i < RTE_MAX_LCORE; i++) {
+		if (lcore_id_used[i] == 1)
+			sprintf(log_msg + strlen(log_msg), "%d ", i);
+	}
+	RTE_LOG(DEBUG, PRIMARY, "%s\n", log_msg);
+
 	return 0;
 }
 
diff --git a/src/shared/common.h b/src/shared/common.h
index e244db9..d5c62bd 100644
--- a/src/shared/common.h
+++ b/src/shared/common.h
@@ -127,4 +127,6 @@ int set_user_log_debug(int num_user_log);
 int parse_num_clients(uint16_t *num_clients, const char *clients);
 int parse_server(char **server_ip, int *server_port, char *server_addr);
 
+extern uint8_t lcore_id_used[RTE_MAX_LCORE];
+
 #endif
-- 
2.7.4