Soft Patch Panel
 help / color / mirror / Atom feed
* [spp] [PATCH 00/11] Refactor functions for handling commands
@ 2019-05-31  3:35 ogawa.yasufumi
  2019-05-31  3:35 ` [spp] [PATCH 01/11] shared/sec: rename functions of sppwk_cmd_runner ogawa.yasufumi
                   ` (10 more replies)
  0 siblings, 11 replies; 12+ messages in thread
From: ogawa.yasufumi @ 2019-05-31  3:35 UTC (permalink / raw)
  To: spp, ferruh.yigit, ogawa.yasufumi

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

Some of function in SPP workers such as parsing command or checking
params for are named or have comments inappropriately. This series of
update is to refactor this issues.

Yasufumi Ogawa (11):
  shared/sec: rename functions of sppwk_cmd_runner
  shared/sec: revise enum for cmd response
  shared/sec: refactor passing err in cmd_runner
  shared/sec: rename struct for command response
  shared/sec: refactor funcs for managing port info
  shared/sec: rename util functions in cmd_runner
  shared/sec: rename func for getting component ID
  shared/sec: refactor func for updating cls table
  shared/sec: rename func for executing command
  shared/sec: add helpers for logging cmd parser
  shared/sec: rename func for updating port

 src/mirror/spp_mirror.c                       |   4 +-
 .../secondary/spp_worker_th/cmd_parser.c      |  52 ++-
 .../secondary/spp_worker_th/cmd_parser.h      |   4 +
 .../secondary/spp_worker_th/cmd_runner.c      | 408 ++++++++----------
 .../secondary/spp_worker_th/cmd_runner.h      |  32 +-
 src/shared/secondary/spp_worker_th/spp_proc.c |  76 ++--
 src/shared/secondary/spp_worker_th/spp_proc.h |  53 +--
 src/vf/spp_vf.c                               |   4 +-
 8 files changed, 319 insertions(+), 314 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [spp] [PATCH 01/11] shared/sec: rename functions of sppwk_cmd_runner
  2019-05-31  3:35 [spp] [PATCH 00/11] Refactor functions for handling commands ogawa.yasufumi
@ 2019-05-31  3:35 ` ogawa.yasufumi
  2019-05-31  3:35 ` [spp] [PATCH 02/11] shared/sec: revise enum for cmd response ogawa.yasufumi
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: ogawa.yasufumi @ 2019-05-31  3:35 UTC (permalink / raw)
  To: spp, ferruh.yigit, ogawa.yasufumi

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

This update is to rename functions defined in header file
`sppwk_cmd_runner.h` to be more specific.

Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
 src/mirror/spp_mirror.c                       |  4 +--
 .../secondary/spp_worker_th/cmd_runner.c      | 10 +++---
 .../secondary/spp_worker_th/cmd_runner.h      | 32 +++++++++----------
 src/vf/spp_vf.c                               |  4 +--
 4 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/src/mirror/spp_mirror.c b/src/mirror/spp_mirror.c
index d46d3da..202475a 100644
--- a/src/mirror/spp_mirror.c
+++ b/src/mirror/spp_mirror.c
@@ -626,7 +626,7 @@ main(int argc, char *argv[])
 		spp_port_ability_init();
 
 		/* Setup connection for accepting commands from controller */
-		int ret_command_init = spp_command_proc_init(
+		int ret_command_init = sppwk_cmd_runner_conn(
 				g_startup_param.server_ip,
 				g_startup_param.server_port);
 		if (unlikely(ret_command_init != SPP_RET_OK))
@@ -675,7 +675,7 @@ main(int argc, char *argv[])
 		{
 #endif
 			/* Receive command */
-			ret_do = spp_command_proc_do();
+			ret_do = sppwk_cmd_run();
 			if (unlikely(ret_do != SPP_RET_OK))
 				break;
 			/*
diff --git a/src/shared/secondary/spp_worker_th/cmd_runner.c b/src/shared/secondary/spp_worker_th/cmd_runner.c
index 153ea01..74b41dd 100644
--- a/src/shared/secondary/spp_worker_th/cmd_runner.c
+++ b/src/shared/secondary/spp_worker_th/cmd_runner.c
@@ -1710,16 +1710,16 @@ process_request(int *sock, const char *request_str, size_t request_str_len)
 	return SPP_RET_OK;
 }
 
-/* initialize command processor. */
+/* Setup connection for accepting commands from spp-ctl. */
 int
-spp_command_proc_init(const char *controller_ip, int controller_port)
+sppwk_cmd_runner_conn(const char *ctl_ipaddr, int ctl_port)
 {
-	return spp_command_conn_init(controller_ip, controller_port);
+	return spp_command_conn_init(ctl_ipaddr, ctl_port);
 }
 
-/* process command from controller. */
+/* Run command from spp-ctl. */
 int
-spp_command_proc_do(void)
+sppwk_cmd_run(void)
 {
 	int ret = SPP_RET_NG;
 	int msg_ret = -1;
diff --git a/src/shared/secondary/spp_worker_th/cmd_runner.h b/src/shared/secondary/spp_worker_th/cmd_runner.h
index bccc0c5..4b768b7 100644
--- a/src/shared/secondary/spp_worker_th/cmd_runner.h
+++ b/src/shared/secondary/spp_worker_th/cmd_runner.h
@@ -7,36 +7,36 @@
 
 /**
  * @file
- * SPP Command processing
+ * Run command for SPP worker thread.
  *
- * Receive and process the command message, then send back the
- * result JSON formatted data.
+ * Receive command message from SPP controller and run. The result is returned
+ * to SPP controller as a JSON formatted message.
  */
 
 #include "spp_proc.h"
 
 /**
- * initialize command processor.
+ * Setup connection for accepting commands from spp-ctl.
  *
- * @param controller_ip
- *  The controller's ip address.
- * @param controller_port
- *  The controller's port number.
+ * @param ctl_ipaddr
+ * IP address of spp-ctl.
+ * @param ctl_port
+ * Port number of spp-ctl.
  *
- * @retval SPP_RET_OK succeeded.
- * @retval SPP_RET_NG failed.
+ * @retval SPP_RET_OK if succeeded.
+ * @retval SPP_RET_NG if failed.
  */
 int
-spp_command_proc_init(const char *controller_ip, int controller_port);
+sppwk_cmd_runner_conn(const char *ctl_ipaddr, int ctl_port);
 
 /**
- * process command from controller.
+ * Run command from spp-ctl.
  *
- * @retval SPP_RET_OK succeeded.
- * @retval SPP_RET_NG process termination is required.
- *            (occurred connection failure, or received exit command)
+ * @retval SPP_RET_OK if succeeded.
+ * TODO(yasufum) change exclude case of exit cmd because it is not NG.
+ * @retval SPP_RET_NG if connection failure or received exit command.
  */
 int
-spp_command_proc_do(void);
+sppwk_cmd_run(void);
 
 #endif  /* _SPPWK_CMD_RUNNER_H_ */
diff --git a/src/vf/spp_vf.c b/src/vf/spp_vf.c
index cbaa0c1..4134647 100644
--- a/src/vf/spp_vf.c
+++ b/src/vf/spp_vf.c
@@ -299,7 +299,7 @@ main(int argc, char *argv[])
 		spp_port_ability_init();
 
 		/* Setup connection for accepting commands from controller */
-		int ret_command_init = spp_command_proc_init(
+		int ret_command_init = sppwk_cmd_runner_conn(
 				g_startup_param.server_ip,
 				g_startup_param.server_port);
 		if (unlikely(ret_command_init != SPP_RET_OK))
@@ -342,7 +342,7 @@ main(int argc, char *argv[])
 		{
 #endif
 			/* Receive command */
-			ret_do = spp_command_proc_do();
+			ret_do = sppwk_cmd_run();
 			if (unlikely(ret_do != SPP_RET_OK))
 				break;
 
-- 
2.17.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [spp] [PATCH 02/11] shared/sec: revise enum for cmd response
  2019-05-31  3:35 [spp] [PATCH 00/11] Refactor functions for handling commands ogawa.yasufumi
  2019-05-31  3:35 ` [spp] [PATCH 01/11] shared/sec: rename functions of sppwk_cmd_runner ogawa.yasufumi
@ 2019-05-31  3:35 ` ogawa.yasufumi
  2019-05-31  3:35 ` [spp] [PATCH 03/11] shared/sec: refactor passing err in cmd_runner ogawa.yasufumi
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: ogawa.yasufumi @ 2019-05-31  3:35 UTC (permalink / raw)
  To: spp, ferruh.yigit, ogawa.yasufumi

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

This update is to revise the name of enum `command_result_code` and its
members.

Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
 .../secondary/spp_worker_th/cmd_runner.c      | 52 +++++++++----------
 1 file changed, 24 insertions(+), 28 deletions(-)

diff --git a/src/shared/secondary/spp_worker_th/cmd_runner.c b/src/shared/secondary/spp_worker_th/cmd_runner.c
index 74b41dd..1908c8c 100644
--- a/src/shared/secondary/spp_worker_th/cmd_runner.c
+++ b/src/shared/secondary/spp_worker_th/cmd_runner.c
@@ -21,37 +21,33 @@
 #define RTE_LOGTYPE_APP RTE_LOGTYPE_USER2
 
 /* request message initial size */
-#define CMD_RES_ERR_MSG_SIZE  128
-#define CMD_TAG_APPEND_SIZE   16
+#define CMD_ERR_MSG_LEN 128
+#define CMD_TAG_APPEND_SIZE 16
 #define CMD_REQ_BUF_INIT_SIZE 2048
 #define CMD_RES_BUF_INIT_SIZE 2048
 
 #define COMMAND_RESP_LIST_EMPTY { "", NULL }
 
+/* TODO(yasufum) revise later `JSON_*`. */
 #define JSON_COMMA                ", "
+/* TODO(yasufum) confirm why using "" for the alternative of comma? */
 #define JSON_APPEND_COMMA(flg)    ((flg)?JSON_COMMA:"")
 #define JSON_APPEND_VALUE(format) "%s\"%s\": "format
 #define JSON_APPEND_ARRAY         "%s\"%s\": [ %s ]"
 #define JSON_APPEND_BLOCK         "%s\"%s\": { %s }"
 #define JSON_APPEND_BLOCK_NONAME  "%s%s{ %s }"
 
-/* command execution result code */
-enum command_result_code {
-	CRES_SUCCESS = 0,
-	CRES_FAILURE,
-	CRES_INVALID,
+enum cmd_res_codes {
+	CMD_SUCCESS = 0,
+	CMD_FAILED,
+	CMD_INVALID,
 };
 
 /* command execution result information */
 struct command_result {
-	/* Response code */
-	int code;
-
-	/* Response message */
-	char result[SPPWK_NAME_BUFSZ];
-
-	/* Detailed response message */
-	char error_message[CMD_RES_ERR_MSG_SIZE];
+	int code;  /* Response code. */
+	char result[SPPWK_NAME_BUFSZ];  /* Response msg in short. */
+	char error_message[CMD_ERR_MSG_LEN];  /* Detailed response msg. */
 };
 
 /* command response list control structure */
@@ -846,18 +842,18 @@ set_command_results(struct command_result *result,
 {
 	result->code = code;
 	switch (code) {
-	case CRES_SUCCESS:
+	case CMD_SUCCESS:
 		strcpy(result->result, "success");
-		memset(result->error_message, 0x00, CMD_RES_ERR_MSG_SIZE);
+		memset(result->error_message, 0x00, CMD_ERR_MSG_LEN);
 		break;
-	case CRES_FAILURE:
+	case CMD_FAILED:
 		strcpy(result->result, "error");
 		strcpy(result->error_message, error_messege);
 		break;
-	case CRES_INVALID: /* FALLTHROUGH */
+	case CMD_INVALID:
 	default:
 		strcpy(result->result, "invalid");
-		memset(result->error_message, 0x00, CMD_RES_ERR_MSG_SIZE);
+		memset(result->error_message, 0x00, CMD_ERR_MSG_LEN);
 		break;
 	}
 }
@@ -870,19 +866,19 @@ prepare_parse_err_msg(struct command_result *results,
 {
 	int i;
 	const char *tmp_buff;
-	char error_messege[CMD_RES_ERR_MSG_SIZE];
+	char error_messege[CMD_ERR_MSG_LEN];
 
 	for (i = 0; i < request->num_command; i++) {
 		if (wk_err_msg->code == 0)
-			set_command_results(&results[i], CRES_SUCCESS, "");
+			set_command_results(&results[i], CMD_SUCCESS, "");
 		else
-			set_command_results(&results[i], CRES_INVALID, "");
+			set_command_results(&results[i], CMD_INVALID, "");
 	}
 
 	if (wk_err_msg->code != 0) {
 		tmp_buff = get_parse_err_msg(wk_err_msg, error_messege);
 		set_command_results(&results[request->num_valid_command],
-				CRES_FAILURE, tmp_buff);
+				CMD_FAILED, tmp_buff);
 	}
 }
 
@@ -1678,24 +1674,24 @@ process_request(int *sock, const char *request_str, size_t request_str_len)
 	for (i = 0; i < request.num_command ; ++i) {
 		ret = execute_command(request.commands + i);
 		if (unlikely(ret != SPP_RET_OK)) {
-			set_command_results(&command_results[i], CRES_FAILURE,
+			set_command_results(&command_results[i], CMD_FAILED,
 					"error occur");
 
 			/* not execute remaining commands */
 			for (++i; i < request.num_command ; ++i)
 				set_command_results(&command_results[i],
-					CRES_INVALID, "");
+					CMD_INVALID, "");
 
 			break;
 		}
 
-		set_command_results(&command_results[i], CRES_SUCCESS, "");
+		set_command_results(&command_results[i], CMD_SUCCESS, "");
 	}
 
 	if (request.is_requested_exit) {
 		/* Terminated by process exit command.                       */
 		/* Other route is normal end because it responds to command. */
-		set_command_results(&command_results[0], CRES_SUCCESS, "");
+		set_command_results(&command_results[0], CMD_SUCCESS, "");
 		send_command_result_response(sock, &request, command_results);
 		RTE_LOG(INFO, SPP_COMMAND_PROC,
 				"Terminate process for exit.\n");
-- 
2.17.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [spp] [PATCH 03/11] shared/sec: refactor passing err in cmd_runner
  2019-05-31  3:35 [spp] [PATCH 00/11] Refactor functions for handling commands ogawa.yasufumi
  2019-05-31  3:35 ` [spp] [PATCH 01/11] shared/sec: rename functions of sppwk_cmd_runner ogawa.yasufumi
  2019-05-31  3:35 ` [spp] [PATCH 02/11] shared/sec: revise enum for cmd response ogawa.yasufumi
@ 2019-05-31  3:35 ` ogawa.yasufumi
  2019-05-31  3:35 ` [spp] [PATCH 04/11] shared/sec: rename struct for command response ogawa.yasufumi
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: ogawa.yasufumi @ 2019-05-31  3:35 UTC (permalink / raw)
  To: spp, ferruh.yigit, ogawa.yasufumi

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

The names of struct and function for passing error info in cmd_runner
are redundant. This update is to revise the names and its comments for
refactoring.

Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
 .../secondary/spp_worker_th/cmd_runner.c      | 59 ++++++++++---------
 1 file changed, 32 insertions(+), 27 deletions(-)

diff --git a/src/shared/secondary/spp_worker_th/cmd_runner.c b/src/shared/secondary/spp_worker_th/cmd_runner.c
index 1908c8c..4874adc 100644
--- a/src/shared/secondary/spp_worker_th/cmd_runner.c
+++ b/src/shared/secondary/spp_worker_th/cmd_runner.c
@@ -44,10 +44,10 @@ enum cmd_res_codes {
 };
 
 /* command execution result information */
-struct command_result {
+struct cmd_result {
 	int code;  /* Response code. */
 	char result[SPPWK_NAME_BUFSZ];  /* Response msg in short. */
-	char error_message[CMD_ERR_MSG_LEN];  /* Detailed response msg. */
+	char err_msg[CMD_ERR_MSG_LEN];  /* Used only if cmd is failed. */
 };
 
 /* command response list control structure */
@@ -835,32 +835,37 @@ get_parse_err_msg(
 	return message;
 }
 
-/* set the command result */
+/* Setup cmd_result with given code and message. */
 static inline void
-set_command_results(struct command_result *result,
+set_cmd_result(struct cmd_result *cmd_res,
 		int code, const char *error_messege)
 {
-	result->code = code;
+	cmd_res->code = code;
+	/**
+	 * TODO(yasufum) confirm these string "success", "error" or "invalid"
+	 * should be fixed or not because this no meaning short message is
+	 * obvious from code and nouse actually.
+	 */
 	switch (code) {
 	case CMD_SUCCESS:
-		strcpy(result->result, "success");
-		memset(result->error_message, 0x00, CMD_ERR_MSG_LEN);
+		strcpy(cmd_res->result, "success");
+		memset(cmd_res->err_msg, 0x00, CMD_ERR_MSG_LEN);
 		break;
 	case CMD_FAILED:
-		strcpy(result->result, "error");
-		strcpy(result->error_message, error_messege);
+		strcpy(cmd_res->result, "error");
+		strcpy(cmd_res->err_msg, error_messege);
 		break;
 	case CMD_INVALID:
 	default:
-		strcpy(result->result, "invalid");
-		memset(result->error_message, 0x00, CMD_ERR_MSG_LEN);
+		strcpy(cmd_res->result, "invalid");
+		memset(cmd_res->err_msg, 0x00, CMD_ERR_MSG_LEN);
 		break;
 	}
 }
 
 /* Setup error message of parsing for requested command. */
 static void
-prepare_parse_err_msg(struct command_result *results,
+prepare_parse_err_msg(struct cmd_result *results,
 		const struct sppwk_cmd_req *request,
 		const struct sppwk_parse_err_msg *wk_err_msg)
 {
@@ -870,14 +875,14 @@ prepare_parse_err_msg(struct command_result *results,
 
 	for (i = 0; i < request->num_command; i++) {
 		if (wk_err_msg->code == 0)
-			set_command_results(&results[i], CMD_SUCCESS, "");
+			set_cmd_result(&results[i], CMD_SUCCESS, "");
 		else
-			set_command_results(&results[i], CMD_INVALID, "");
+			set_cmd_result(&results[i], CMD_INVALID, "");
 	}
 
 	if (wk_err_msg->code != 0) {
 		tmp_buff = get_parse_err_msg(wk_err_msg, error_messege);
-		set_command_results(&results[request->num_valid_command],
+		set_cmd_result(&results[request->num_valid_command],
 				CMD_FAILED, tmp_buff);
 	}
 }
@@ -886,7 +891,7 @@ prepare_parse_err_msg(struct command_result *results,
 static int
 append_result_value(const char *name, char **output, void *tmp)
 {
-	const struct command_result *result = tmp;
+	const struct cmd_result *result = tmp;
 	return append_json_str_value(name, output, result->result);
 }
 
@@ -895,10 +900,10 @@ static int
 append_error_details_value(const char *name, char **output, void *tmp)
 {
 	int ret = SPP_RET_NG;
-	const struct command_result *result = tmp;
+	const struct cmd_result *result = tmp;
 	char *tmp_buff;
 	/* string is empty, except for errors */
-	if (result->error_message[0] == '\0')
+	if (result->err_msg[0] == '\0')
 		return SPP_RET_OK;
 
 	tmp_buff = spp_strbuf_allocate(CMD_RES_BUF_INIT_SIZE);
@@ -911,7 +916,7 @@ append_error_details_value(const char *name, char **output, void *tmp)
 	}
 
 	ret = append_json_str_value("message", &tmp_buff,
-			result->error_message);
+			result->err_msg);
 	if (unlikely(ret < 0)) {
 		spp_strbuf_free(tmp_buff);
 		return SPP_RET_NG;
@@ -1417,7 +1422,7 @@ struct command_response_list response_info_list[] = {
 /* append a list of command results for JSON format. */
 static int
 append_command_results_value(const char *name, char **output,
-		int num, struct command_result *results)
+		int num, struct cmd_result *results)
 {
 	int ret = SPP_RET_NG;
 	int i;
@@ -1496,7 +1501,7 @@ append_info_value(const char *name, char **output)
 static void
 send_decode_error_response(int *sock,
 		const struct sppwk_cmd_req *request,
-		struct command_result *command_results)
+		struct cmd_result *command_results)
 {
 	int ret = SPP_RET_NG;
 	char *msg, *tmp_buff;
@@ -1555,7 +1560,7 @@ send_decode_error_response(int *sock,
 static void
 send_command_result_response(int *sock,
 		const struct sppwk_cmd_req *request,
-		struct command_result *command_results)
+		struct cmd_result *command_results)
 {
 	int ret = SPP_RET_NG;
 	char *msg, *tmp_buff;
@@ -1643,7 +1648,7 @@ process_request(int *sock, const char *request_str, size_t request_str_len)
 
 	struct sppwk_cmd_req request;
 	struct sppwk_parse_err_msg wk_err_msg;
-	struct command_result command_results[SPPWK_MAX_CMDS];
+	struct cmd_result command_results[SPPWK_MAX_CMDS];
 
 	memset(&request, 0, sizeof(struct sppwk_cmd_req));
 	memset(&wk_err_msg, 0, sizeof(struct sppwk_parse_err_msg));
@@ -1674,24 +1679,24 @@ process_request(int *sock, const char *request_str, size_t request_str_len)
 	for (i = 0; i < request.num_command ; ++i) {
 		ret = execute_command(request.commands + i);
 		if (unlikely(ret != SPP_RET_OK)) {
-			set_command_results(&command_results[i], CMD_FAILED,
+			set_cmd_result(&command_results[i], CMD_FAILED,
 					"error occur");
 
 			/* not execute remaining commands */
 			for (++i; i < request.num_command ; ++i)
-				set_command_results(&command_results[i],
+				set_cmd_result(&command_results[i],
 					CMD_INVALID, "");
 
 			break;
 		}
 
-		set_command_results(&command_results[i], CMD_SUCCESS, "");
+		set_cmd_result(&command_results[i], CMD_SUCCESS, "");
 	}
 
 	if (request.is_requested_exit) {
 		/* Terminated by process exit command.                       */
 		/* Other route is normal end because it responds to command. */
-		set_command_results(&command_results[0], CMD_SUCCESS, "");
+		set_cmd_result(&command_results[0], CMD_SUCCESS, "");
 		send_command_result_response(sock, &request, command_results);
 		RTE_LOG(INFO, SPP_COMMAND_PROC,
 				"Terminate process for exit.\n");
-- 
2.17.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [spp] [PATCH 04/11] shared/sec: rename struct for command response
  2019-05-31  3:35 [spp] [PATCH 00/11] Refactor functions for handling commands ogawa.yasufumi
                   ` (2 preceding siblings ...)
  2019-05-31  3:35 ` [spp] [PATCH 03/11] shared/sec: refactor passing err in cmd_runner ogawa.yasufumi
@ 2019-05-31  3:35 ` ogawa.yasufumi
  2019-05-31  3:35 ` [spp] [PATCH 05/11] shared/sec: refactor funcs for managing port info ogawa.yasufumi
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: ogawa.yasufumi @ 2019-05-31  3:35 UTC (permalink / raw)
  To: spp, ferruh.yigit, ogawa.yasufumi

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

This update is to rename scruct `command_response_list` to
`cmd_response` because it is not a list actually and redundant. Some of
comments are also revised to improve maintainability.

Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
 .../secondary/spp_worker_th/cmd_runner.c      | 52 +++++++++----------
 1 file changed, 24 insertions(+), 28 deletions(-)

diff --git a/src/shared/secondary/spp_worker_th/cmd_runner.c b/src/shared/secondary/spp_worker_th/cmd_runner.c
index 4874adc..a7005c3 100644
--- a/src/shared/secondary/spp_worker_th/cmd_runner.c
+++ b/src/shared/secondary/spp_worker_th/cmd_runner.c
@@ -26,8 +26,6 @@
 #define CMD_REQ_BUF_INIT_SIZE 2048
 #define CMD_RES_BUF_INIT_SIZE 2048
 
-#define COMMAND_RESP_LIST_EMPTY { "", NULL }
-
 /* TODO(yasufum) revise later `JSON_*`. */
 #define JSON_COMMA                ", "
 /* TODO(yasufum) confirm why using "" for the alternative of comma? */
@@ -43,19 +41,19 @@ enum cmd_res_codes {
 	CMD_INVALID,
 };
 
-/* command execution result information */
 struct cmd_result {
 	int code;  /* Response code. */
 	char result[SPPWK_NAME_BUFSZ];  /* Response msg in short. */
 	char err_msg[CMD_ERR_MSG_LEN];  /* Used only if cmd is failed. */
 };
 
-/* command response list control structure */
-struct command_response_list {
-	/* Tag name */
+/**
+ * Contains command response nad operator func for. It is used as an array of
+ * this struct.
+ */
+/* TODO(yasufum) add comment describes the purpose of this struct is used. */
+struct cmd_response {
 	char tag_name[SPPWK_NAME_BUFSZ];
-
-	/* Pointer to handling function */
 	int (*func)(const char *name, char **output, void *tmp);
 };
 
@@ -1331,10 +1329,11 @@ append_classifier_table_value(const char *name, char **output,
 	return ret;
 }
 #endif /* SPP_VF_MODULE */
+
 /* append string of command response list for JSON format */
 static int
 append_response_list_value(char **output,
-		struct command_response_list *list,
+		struct cmd_response *responses,
 		void *tmp)
 {
 	int ret = SPP_RET_NG;
@@ -1348,14 +1347,14 @@ append_response_list_value(char **output,
 		return SPP_RET_NG;
 	}
 
-	for (i = 0; list[i].tag_name[0] != '\0'; i++) {
+	for (i = 0; responses[i].tag_name[0] != '\0'; i++) {
 		tmp_buff[0] = '\0';
-		ret = list[i].func(list[i].tag_name, &tmp_buff, tmp);
+		ret = responses[i].func(responses[i].tag_name, &tmp_buff, tmp);
 		if (unlikely(ret < SPP_RET_OK)) {
 			spp_strbuf_free(tmp_buff);
 			RTE_LOG(ERR, SPP_COMMAND_PROC,
 					"Failed to get reply string. "
-					"(tag = %s)\n", list[i].tag_name);
+					"(tag = %s)\n", responses[i].tag_name);
 			return SPP_RET_NG;
 		}
 
@@ -1369,7 +1368,7 @@ append_response_list_value(char **output,
 				RTE_LOG(ERR, SPP_COMMAND_PROC,
 						"Failed to add commas. "
 						"(tag = %s)\n",
-						list[i].tag_name);
+						responses[i].tag_name);
 				return SPP_RET_NG;
 			}
 		}
@@ -1381,7 +1380,7 @@ append_response_list_value(char **output,
 			RTE_LOG(ERR, SPP_COMMAND_PROC,
 					"Failed to add reply string. "
 					"(tag = %s)\n",
-					list[i].tag_name);
+					responses[i].tag_name);
 			return SPP_RET_NG;
 		}
 	}
@@ -1390,14 +1389,11 @@ append_response_list_value(char **output,
 	return SPP_RET_OK;
 }
 
-/* termination constant of command response list */
-#define COMMAND_RESP_TAG_LIST_EMPTY { "", NULL }
-
 /* command response result string list */
-struct command_response_list response_result_list[] = {
-	{ "result",        append_result_value },
+struct cmd_response response_result_list[] = {
+	{ "result", append_result_value },
 	{ "error_details", append_error_details_value },
-	COMMAND_RESP_TAG_LIST_EMPTY
+	{ "", NULL }
 };
 
 /**
@@ -1406,17 +1402,17 @@ struct command_response_list response_result_list[] = {
  * response.
  */
 /* command response status information string list */
-struct command_response_list response_info_list[] = {
-	{ "client-id",        append_client_id_value },
-	{ "phy",              append_interface_value },
-	{ "vhost",            append_interface_value },
-	{ "ring",             append_interface_value },
-	{ "master-lcore",     append_master_lcore_value },
-	{ "core",             append_core_value },
+struct cmd_response response_info_list[] = {
+	{ "client-id", append_client_id_value },
+	{ "phy", append_interface_value },
+	{ "vhost", append_interface_value },
+	{ "ring", append_interface_value },
+	{ "master-lcore", append_master_lcore_value },
+	{ "core", append_core_value },
 #ifdef SPP_VF_MODULE
 	{ "classifier_table", append_classifier_table_value },
 #endif /* SPP_VF_MODULE */
-	COMMAND_RESP_TAG_LIST_EMPTY
+	{ "", NULL }
 };
 
 /* append a list of command results for JSON format. */
-- 
2.17.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [spp] [PATCH 05/11] shared/sec: refactor funcs for managing port info
  2019-05-31  3:35 [spp] [PATCH 00/11] Refactor functions for handling commands ogawa.yasufumi
                   ` (3 preceding siblings ...)
  2019-05-31  3:35 ` [spp] [PATCH 04/11] shared/sec: rename struct for command response ogawa.yasufumi
@ 2019-05-31  3:35 ` ogawa.yasufumi
  2019-05-31  3:36 ` [spp] [PATCH 06/11] shared/sec: rename util functions in cmd_runner ogawa.yasufumi
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: ogawa.yasufumi @ 2019-05-31  3:35 UTC (permalink / raw)
  To: spp, ferruh.yigit, ogawa.yasufumi

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

This patch is to refactor functions for getting and deleting port info.

* Function `check_port_element()` is used for getting index of given port
  info array, and the name does not describe this feature. To fix this
  issue, rename to `get_idx_port_info()`.

* `get_del_port_element()` does not get anything, but just delete given
  port from the array. It is renamed as `delete_port_info()`.

Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
 .../secondary/spp_worker_th/cmd_runner.c      | 30 +++++-----
 src/shared/secondary/spp_worker_th/spp_proc.c | 55 +++++++++----------
 src/shared/secondary/spp_worker_th/spp_proc.h | 37 +++++--------
 3 files changed, 53 insertions(+), 69 deletions(-)

diff --git a/src/shared/secondary/spp_worker_th/cmd_runner.c b/src/shared/secondary/spp_worker_th/cmd_runner.c
index a7005c3..2a82edd 100644
--- a/src/shared/secondary/spp_worker_th/cmd_runner.c
+++ b/src/shared/secondary/spp_worker_th/cmd_runner.c
@@ -352,10 +352,6 @@ check_port_count(int component_type, enum spp_port_rxtx rxtx, int num_rx,
 }
 
 /* Port add or del to execute it */
-/**
- * TODO(Ogasawara) The name `action` should be revised to be more
- * appropriate one.
- */
 static int
 spp_update_port(enum sppwk_action wk_action,
 		const struct sppwk_port_idx *port,
@@ -364,13 +360,13 @@ spp_update_port(enum sppwk_action wk_action,
 		const struct spp_port_ability *ability)
 {
 	int ret = SPP_RET_NG;
-	int ret_check = -1;
+	int port_idx;
 	int ret_del = -1;
 	int component_id = 0;
 	int cnt = 0;
 	struct spp_component_info *comp_info = NULL;
 	struct sppwk_port_info *port_info = NULL;
-	int *num = NULL;
+	int *nof_ports = NULL;
 	struct sppwk_port_info **ports = NULL;
 	struct spp_component_info *comp_info_base = NULL;
 	int *change_component = NULL;
@@ -386,10 +382,10 @@ spp_update_port(enum sppwk_action wk_action,
 	comp_info = (comp_info_base + component_id);
 	port_info = get_sppwk_port(port->iface_type, port->iface_no);
 	if (rxtx == SPP_PORT_RXTX_RX) {
-		num = &comp_info->num_rx_port;
+		nof_ports = &comp_info->num_rx_port;
 		ports = comp_info->rx_ports;
 	} else {
-		num = &comp_info->num_tx_port;
+		nof_ports = &comp_info->num_tx_port;
 		ports = comp_info->tx_ports;
 	}
 
@@ -401,9 +397,9 @@ spp_update_port(enum sppwk_action wk_action,
 				comp_info->num_tx_port) != SPP_RET_OK)
 			return SPP_RET_NG;
 
-		ret_check = check_port_element(port_info, *num, ports);
-		/* Check whether a port has been already registered. */
-		if (ret_check >= SPP_RET_OK) {
+		/* Check if the port_info is included in array `ports`. */
+		port_idx = get_idx_port_info(port_info, *nof_ports, ports);
+		if (port_idx >= SPP_RET_OK) {
 			/* registered */
 			if (ability->ops == SPPWK_PORT_ABL_OPS_ADD_VLANTAG) {
 				while ((cnt < SPP_PORT_ABILITY_MAX) &&
@@ -424,7 +420,7 @@ spp_update_port(enum sppwk_action wk_action,
 			return SPP_RET_OK;
 		}
 
-		if (*num >= RTE_MAX_ETHPORTS) {
+		if (*nof_ports >= RTE_MAX_ETHPORTS) {
 			RTE_LOG(ERR, APP, "Cannot assign port over the "
 				"maximum number.\n");
 			return SPP_RET_NG;
@@ -446,8 +442,8 @@ spp_update_port(enum sppwk_action wk_action,
 		}
 
 		port_info->iface_type = port->iface_type;
-		ports[*num] = port_info;
-		(*num)++;
+		ports[*nof_ports] = port_info;
+		(*nof_ports)++;
 
 		ret = SPP_RET_OK;
 		break;
@@ -463,14 +459,14 @@ spp_update_port(enum sppwk_action wk_action,
 					sizeof(struct spp_port_ability));
 		}
 
-		ret_del = get_del_port_element(port_info, *num, ports);
+		ret_del = delete_port_info(port_info, *nof_ports, ports);
 		if (ret_del == 0)
-			(*num)--; /* If deleted, decrement number. */
+			(*nof_ports)--; /* If deleted, decrement number. */
 
 		ret = SPP_RET_OK;
 		break;
 
-	default:
+	default:  /* This case cannot be happend without invlid wk_action. */
 		return SPP_RET_NG;
 	}
 
diff --git a/src/shared/secondary/spp_worker_th/spp_proc.c b/src/shared/secondary/spp_worker_th/spp_proc.c
index 2cd98c3..971ec9c 100644
--- a/src/shared/secondary/spp_worker_th/spp_proc.c
+++ b/src/shared/secondary/spp_worker_th/spp_proc.c
@@ -771,45 +771,44 @@ del_component_info(int component_id, int component_num, int *componet_array)
 	return 0;
 }
 
-/* get port element which matches the condition */
+/**
+ * Get index of given entry in given port info array. It returns the index,
+ * or NG code if the entry is not found.
+ */
 int
-check_port_element(
-		struct sppwk_port_info *info,
-		int num,
-		struct sppwk_port_info *array[])
+get_idx_port_info(struct sppwk_port_info *p_info, int nof_ports,
+		struct sppwk_port_info *p_info_ary[])
 {
 	int cnt = 0;
-	int match = SPP_RET_NG;
-	for (cnt = 0; cnt < num; cnt++) {
-		if (info == array[cnt])
-			match = cnt;
+	int ret = SPP_RET_NG;
+	for (cnt = 0; cnt < nof_ports; cnt++) {
+		if (p_info == p_info_ary[cnt])
+			ret = cnt;
 	}
-	return match;
+	return ret;
 }
 
-/* search matched port_info from array and delete it */
+/* Delete given port info from the port info array. */
 int
-get_del_port_element(
-		struct sppwk_port_info *info,
-		int num,
-		struct sppwk_port_info *array[])
+delete_port_info(struct sppwk_port_info *p_info, int nof_ports,
+		struct sppwk_port_info *p_info_ary[])
 {
-	int cnt = 0;
-	int match = SPP_RET_NG;
-	int max = num;
+	int target_idx;  /* The index of deleted port */
+	int cnt;
 
-	match = check_port_element(info, num, array);
-	if (match < 0)
+	/* Find index of target port to be deleted. */
+	target_idx = get_idx_port_info(p_info, nof_ports, p_info_ary);
+	if (target_idx < 0)
 		return SPP_RET_NG;
 
-	/* Last element is excluded from movement. */
-	max--;
-
-	for (cnt = match; cnt < max; cnt++)
-		array[cnt] = array[cnt+1];
-
-	/* Last element is cleared. */
-	array[cnt] = NULL;
+	/**
+	 * Overwrite the deleted port by the next one, and shift all of
+	 * remained ports.
+	 */
+	nof_ports--;
+	for (cnt = target_idx; cnt < nof_ports; cnt++)
+		p_info_ary[cnt] = p_info_ary[cnt+1];
+	p_info_ary[cnt] = NULL;  /* Remove old last port. */
 	return SPP_RET_OK;
 }
 
diff --git a/src/shared/secondary/spp_worker_th/spp_proc.h b/src/shared/secondary/spp_worker_th/spp_proc.h
index 30bd8be..b490471 100644
--- a/src/shared/secondary/spp_worker_th/spp_proc.h
+++ b/src/shared/secondary/spp_worker_th/spp_proc.h
@@ -571,40 +571,29 @@ int
 del_component_info(int component_id, int component_num, int *componet_array);
 
 /**
- * get port element which matches the condition.
+ * Get index of given entry in given port info array. It returns the index,
+ * or NG code if the entry is not found.
  *
- * @param info
- *  sppwk_port_info address
- * @param num
- *  port count
- * @param array[]
- *  sppwk_port_info array address
- *
- * @retval 0~ match index.
- * @retval -1 failed.
+ * @param[in] p_info Target port_info for getting index.
+ * @param[in] nof_ports Num of ports for iterating given array.
+ * @param[in] p_info_ary The array of port_info.
+ * @return Index of given array, or NG code if not found.
  */
-int check_port_element(
-		struct sppwk_port_info *info,
-		int num,
-		struct sppwk_port_info *array[]);
+int get_idx_port_info(struct sppwk_port_info *p_info, int nof_ports,
+		struct sppwk_port_info *p_info_ary[]);
 
 /**
  *  search matched port_info from array and delete it.
  *
- * @param info
- *  sppwk_port_info address
- * @param num
- *  port count
- * @param array[]
- *  sppwk_port_info array address
+ * @param[in] p_info Target port to be deleted.
+ * @param[in] nof_ports Number of ports of given p_info_ary.
+ * @param[in] array[] Array of p_info.
  *
  * @retval 0  succeeded.
  * @retval -1 failed.
  */
-int get_del_port_element(
-		struct sppwk_port_info *info,
-		int num,
-		struct sppwk_port_info *array[]);
+int delete_port_info(struct sppwk_port_info *p_info, int nof_ports,
+		struct sppwk_port_info *p_info_ary[]);
 
 /**
  * Flush initial setting of each interface.
-- 
2.17.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [spp] [PATCH 06/11] shared/sec: rename util functions in cmd_runner
  2019-05-31  3:35 [spp] [PATCH 00/11] Refactor functions for handling commands ogawa.yasufumi
                   ` (4 preceding siblings ...)
  2019-05-31  3:35 ` [spp] [PATCH 05/11] shared/sec: refactor funcs for managing port info ogawa.yasufumi
@ 2019-05-31  3:36 ` ogawa.yasufumi
  2019-05-31  3:36 ` [spp] [PATCH 07/11] shared/sec: rename func for getting component ID ogawa.yasufumi
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: ogawa.yasufumi @ 2019-05-31  3:36 UTC (permalink / raw)
  To: spp, ferruh.yigit, ogawa.yasufumi

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

This update is to rename utility functions.

* `spp_get_client_id()` for getting client ID from global variable is
  renamed to `sppwk_get_client_id()`.

* `spp_get_process_type()` for getting process type from global variable
  is renamed to `sppwk_get_process_type()`.

* `spp_check_flush_port()` is simply renamed to `is_port_flushed()`.

Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
 .../secondary/spp_worker_th/cmd_runner.c      | 34 ++++++++-----------
 1 file changed, 15 insertions(+), 19 deletions(-)

diff --git a/src/shared/secondary/spp_worker_th/cmd_runner.c b/src/shared/secondary/spp_worker_th/cmd_runner.c
index 2a82edd..3d07a28 100644
--- a/src/shared/secondary/spp_worker_th/cmd_runner.c
+++ b/src/shared/secondary/spp_worker_th/cmd_runner.c
@@ -92,31 +92,27 @@ const char *CLS_TYPE_A_LIST[] = {
 	"",  /* termination */
 };
 
-/* get client id */
+/* Get client ID from global command params. */
 static int
-spp_get_client_id(void)
+sppwk_get_client_id(void)
 {
-	struct startup_param *startup_param;
-
-	sppwk_get_mng_data(&startup_param,
-			NULL, NULL, NULL, NULL, NULL, NULL);
-	return startup_param->client_id;
+	struct startup_param *params;
+	sppwk_get_mng_data(&params, NULL, NULL, NULL, NULL, NULL, NULL);
+	return params->client_id;
 }
 
-/* get process type */
+/* Get proc type from global command params. */
 static int
-spp_get_process_type(void)
+sppwk_get_proc_type(void)
 {
-	struct startup_param *startup_param;
-
-	sppwk_get_mng_data(&startup_param,
-			NULL, NULL, NULL, NULL, NULL, NULL);
-	return startup_param->secondary_type;
+	struct startup_param *params;
+	sppwk_get_mng_data(&params, NULL, NULL, NULL, NULL, NULL, NULL);
+	return params->secondary_type;
 }
 
-/* Check if port has been flushed. */
+/* Check if port is already flushed. */
 static int
-spp_check_flush_port(enum port_type iface_type, int iface_no)
+is_port_flushed(enum port_type iface_type, int iface_no)
 {
 	struct sppwk_port_info *port = get_sppwk_port(iface_type, iface_no);
 	return port->ethdev_port_id >= 0;
@@ -926,7 +922,7 @@ static int
 append_client_id_value(const char *name, char **output,
 		void *tmp __attribute__ ((unused)))
 {
-	return append_json_int_value(name, output, spp_get_client_id());
+	return append_json_int_value(name, output, sppwk_get_client_id());
 }
 
 /* append a list of interface numbers */
@@ -937,7 +933,7 @@ append_interface_array(char **output, const enum port_type type)
 	char tmp_str[CMD_TAG_APPEND_SIZE];
 
 	for (i = 0; i < RTE_MAX_ETHPORTS; i++) {
-		if (!spp_check_flush_port(type, i))
+		if (!is_port_flushed(type, i))
 			continue;
 
 		sprintf(tmp_str, "%s%d", JSON_APPEND_COMMA(port_cnt), i);
@@ -962,7 +958,7 @@ append_process_type_value(const char *name, char **output,
 		void *tmp __attribute__ ((unused)))
 {
 	return append_json_str_value(name, output,
-			SPPWK_PROC_TYPE_LIST[spp_get_process_type()]);
+			SPPWK_PROC_TYPE_LIST[sppwk_get_proc_type()]);
 }
 
 /* append a list of interface numbers for JSON format */
-- 
2.17.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [spp] [PATCH 07/11] shared/sec: rename func for getting component ID
  2019-05-31  3:35 [spp] [PATCH 00/11] Refactor functions for handling commands ogawa.yasufumi
                   ` (5 preceding siblings ...)
  2019-05-31  3:36 ` [spp] [PATCH 06/11] shared/sec: rename util functions in cmd_runner ogawa.yasufumi
@ 2019-05-31  3:36 ` ogawa.yasufumi
  2019-05-31  3:36 ` [spp] [PATCH 08/11] shared/sec: refactor func for updating cls table ogawa.yasufumi
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: ogawa.yasufumi @ 2019-05-31  3:36 UTC (permalink / raw)
  To: spp, ferruh.yigit, ogawa.yasufumi

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

The term `component ID` is lcore ID used by a SPP worker thread
actually. Fuctions for managing component ID should be renamed for
lcore ID because it is confusing to use several terms for the same
resource. This update is to rename the functions and variables of
component ID to lcore ID to be explicit in meaning.

Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
 .../secondary/spp_worker_th/cmd_parser.c      |  4 +-
 .../secondary/spp_worker_th/cmd_runner.c      | 42 ++++++++++---------
 src/shared/secondary/spp_worker_th/spp_proc.c | 21 ++++------
 src/shared/secondary/spp_worker_th/spp_proc.h | 16 ++++---
 4 files changed, 40 insertions(+), 43 deletions(-)

diff --git a/src/shared/secondary/spp_worker_th/cmd_parser.c b/src/shared/secondary/spp_worker_th/cmd_parser.c
index f78664b..084b3e2 100644
--- a/src/shared/secondary/spp_worker_th/cmd_parser.c
+++ b/src/shared/secondary/spp_worker_th/cmd_parser.c
@@ -335,7 +335,7 @@ parse_comp_name(void *output, const char *arg_val,
 	/* Parsing the name is required only for action `start`. */
 	if (component->wk_action == SPPWK_ACT_START) {
 		/* Check if lcore is already used. */
-		ret = spp_get_component_id(arg_val);  /* Get lcore ID. */
+		ret = sppwk_get_lcore_id(arg_val);  /* Get lcore ID. */
 		if (unlikely(ret >= 0)) {
 			RTE_LOG(ERR, SPP_COMMAND_PROC,
 					"Comp name '%s' is already used.\n",
@@ -492,7 +492,7 @@ parse_comp_name_portcmd(void *output, const char *arg_val,
 	int ret = SPP_RET_OK;
 
 	/* Check if lcore is already used. */
-	ret = spp_get_component_id(arg_val);  /* Get lcore ID. */
+	ret = sppwk_get_lcore_id(arg_val);  /* Get lcore ID. */
 	if (unlikely(ret < SPP_RET_OK)) {
 		RTE_LOG(ERR, SPP_COMMAND_PROC,
 				"Unknown component name. val=%s\n", arg_val);
diff --git a/src/shared/secondary/spp_worker_th/cmd_runner.c b/src/shared/secondary/spp_worker_th/cmd_runner.c
index 3d07a28..420e19c 100644
--- a/src/shared/secondary/spp_worker_th/cmd_runner.c
+++ b/src/shared/secondary/spp_worker_th/cmd_runner.c
@@ -92,6 +92,7 @@ const char *CLS_TYPE_A_LIST[] = {
 	"",  /* termination */
 };
 
+/* TODO(yasufum) move to another file for util funcs. */
 /* Get client ID from global command params. */
 static int
 sppwk_get_client_id(void)
@@ -101,6 +102,7 @@ sppwk_get_client_id(void)
 	return params->client_id;
 }
 
+/* TODO(yasufum) move to another file for util funcs. */
 /* Get proc type from global command params. */
 static int
 sppwk_get_proc_type(void)
@@ -215,7 +217,7 @@ spp_update_component(
 {
 	int ret = SPP_RET_NG;
 	int ret_del = -1;
-	int component_id = 0;
+	int comp_lcore_id = 0;
 	unsigned int tmp_lcore_id = 0;
 	struct spp_component_info *comp_info = NULL;
 	struct core_info *core = NULL;
@@ -237,15 +239,15 @@ spp_update_component(
 			return SPP_RET_NG;
 		}
 
-		component_id = spp_get_component_id(name);
-		if (component_id >= 0) {
+		comp_lcore_id = sppwk_get_lcore_id(name);
+		if (comp_lcore_id >= 0) {
 			RTE_LOG(ERR, APP, "Component name '%s' is already "
 				"used.\n", name);
 			return SPP_RET_NG;
 		}
 
-		component_id = get_free_component();
-		if (component_id < 0) {
+		comp_lcore_id = get_free_lcore_id();
+		if (comp_lcore_id < 0) {
 			RTE_LOG(ERR, APP, "Cannot assign component over the "
 				"maximum number.\n");
 			return SPP_RET_NG;
@@ -253,26 +255,26 @@ spp_update_component(
 
 		core = &info->core[info->upd_index];
 
-		comp_info = (comp_info_base + component_id);
+		comp_info = (comp_info_base + comp_lcore_id);
 		memset(comp_info, 0x00, sizeof(struct spp_component_info));
 		strcpy(comp_info->name, name);
 		comp_info->type		= type;
 		comp_info->lcore_id	= lcore_id;
-		comp_info->component_id	= component_id;
+		comp_info->component_id	= comp_lcore_id;
 
-		core->id[core->num] = component_id;
+		core->id[core->num] = comp_lcore_id;
 		core->num++;
 		ret = SPP_RET_OK;
 		tmp_lcore_id = lcore_id;
-		*(change_component + component_id) = 1;
+		*(change_component + comp_lcore_id) = 1;
 		break;
 
 	case SPPWK_ACT_STOP:
-		component_id = spp_get_component_id(name);
-		if (component_id < 0)
+		comp_lcore_id = sppwk_get_lcore_id(name);
+		if (comp_lcore_id < 0)
 			return SPP_RET_OK;
 
-		comp_info = (comp_info_base + component_id);
+		comp_info = (comp_info_base + comp_lcore_id);
 		tmp_lcore_id = comp_info->lcore_id;
 		memset(comp_info, 0x00, sizeof(struct spp_component_info));
 
@@ -282,17 +284,17 @@ spp_update_component(
 #ifdef SPP_VF_MODULE
 		/* initialize classifier information */
 		if (comp_info->type == SPP_COMPONENT_CLASSIFIER_MAC)
-			init_classifier_info(component_id);
+			init_classifier_info(comp_lcore_id);
 #endif /* SPP_VF_MODULE */
 
-		ret_del = del_component_info(component_id,
+		ret_del = del_component_info(comp_lcore_id,
 				core->num, core->id);
 		if (ret_del >= 0)
 			/* If deleted, decrement number. */
 			core->num--;
 
 		ret = SPP_RET_OK;
-		*(change_component + component_id) = 0;
+		*(change_component + comp_lcore_id) = 0;
 		break;
 
 	default:
@@ -358,7 +360,7 @@ spp_update_port(enum sppwk_action wk_action,
 	int ret = SPP_RET_NG;
 	int port_idx;
 	int ret_del = -1;
-	int component_id = 0;
+	int comp_lcore_id = 0;
 	int cnt = 0;
 	struct spp_component_info *comp_info = NULL;
 	struct sppwk_port_info *port_info = NULL;
@@ -367,15 +369,15 @@ spp_update_port(enum sppwk_action wk_action,
 	struct spp_component_info *comp_info_base = NULL;
 	int *change_component = NULL;
 
-	component_id = spp_get_component_id(name);
-	if (component_id < 0) {
+	comp_lcore_id = sppwk_get_lcore_id(name);
+	if (comp_lcore_id < 0) {
 		RTE_LOG(ERR, APP, "Unknown component by port command. "
 				"(component = %s)\n", name);
 		return SPP_RET_NG;
 	}
 	sppwk_get_mng_data(NULL, NULL,
 			&comp_info_base, NULL, NULL, &change_component, NULL);
-	comp_info = (comp_info_base + component_id);
+	comp_info = (comp_info_base + comp_lcore_id);
 	port_info = get_sppwk_port(port->iface_type, port->iface_no);
 	if (rxtx == SPP_PORT_RXTX_RX) {
 		nof_ports = &comp_info->num_rx_port;
@@ -466,7 +468,7 @@ spp_update_port(enum sppwk_action wk_action,
 		return SPP_RET_NG;
 	}
 
-	*(change_component + component_id) = 1;
+	*(change_component + comp_lcore_id) = 1;
 	return ret;
 }
 
diff --git a/src/shared/secondary/spp_worker_th/spp_proc.c b/src/shared/secondary/spp_worker_th/spp_proc.c
index 971ec9c..9c62bab 100644
--- a/src/shared/secondary/spp_worker_th/spp_proc.c
+++ b/src/shared/secondary/spp_worker_th/spp_proc.c
@@ -710,35 +710,32 @@ set_component_change_port(struct sppwk_port_info *port, enum spp_port_rxtx rxtx)
 	}
 }
 
-/* Get unused component id */
+/* Get ID of unused lcore. */
 int
-get_free_component(void)
+get_free_lcore_id(void)
 {
-	struct spp_component_info *component_info =
-					g_mng_data.p_component_info;
+	struct spp_component_info *comp_info = g_mng_data.p_component_info;
 
 	int cnt = 0;
 	for (cnt = 0; cnt < RTE_MAX_LCORE; cnt++) {
-		if ((component_info + cnt)->type == SPP_COMPONENT_UNUSE)
+		if ((comp_info + cnt)->type == SPP_COMPONENT_UNUSE)
 			return cnt;
 	}
 	return SPP_RET_NG;
 }
 
-/* Get lcore id for as component name. */
-/* TODO(yasufum) change the name because it's not comp ID. */
+/* Get lcore ID as user-defined component name. */
 int
-spp_get_component_id(const char *name)
+sppwk_get_lcore_id(const char *comp_name)
 {
-	struct spp_component_info *component_info =
-		g_mng_data.p_component_info;
+	struct spp_component_info *comp_info = g_mng_data.p_component_info;
 
 	int cnt = 0;
-	if (name[0] == '\0')
+	if (comp_name[0] == '\0')
 		return SPP_RET_NG;
 
 	for (cnt = 0; cnt < RTE_MAX_LCORE; cnt++) {
-		if (strcmp(name, (component_info + cnt)->name) == 0)
+		if (strcmp(comp_name, (comp_info + cnt)->name) == 0)
 			return cnt;
 	}
 	return SPP_RET_NG;
diff --git a/src/shared/secondary/spp_worker_th/spp_proc.h b/src/shared/secondary/spp_worker_th/spp_proc.h
index b490471..bdd628e 100644
--- a/src/shared/secondary/spp_worker_th/spp_proc.h
+++ b/src/shared/secondary/spp_worker_th/spp_proc.h
@@ -536,23 +536,21 @@ set_component_change_port(
 		struct sppwk_port_info *port, enum spp_port_rxtx rxtx);
 
 /**
- * Get unused component id
+ * Get ID of unused lcore.
  *
  * @retval 0~127 Component ID.
  * @retval -1    failed.
  */
-int get_free_component(void);
+int get_free_lcore_id(void);
 
 /**
- * Get component id for specified component name
+ * Get component ID from given name.
  *
- * @param name
- *  Component name.
- *
- * @retval 0~127      Component ID.
- * @retval SPP_RET_NG failed.
+ * @param[in] name Component name.
+ * @retval 0~127 Component ID.
+ * @retval SPP_RET_NG if failed.
  */
-int spp_get_component_id(const char *name);
+int sppwk_get_lcore_id(const char *comp_name);
 
 /**
  *  Delete component information.
-- 
2.17.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [spp] [PATCH 08/11] shared/sec: refactor func for updating cls table
  2019-05-31  3:35 [spp] [PATCH 00/11] Refactor functions for handling commands ogawa.yasufumi
                   ` (6 preceding siblings ...)
  2019-05-31  3:36 ` [spp] [PATCH 07/11] shared/sec: rename func for getting component ID ogawa.yasufumi
@ 2019-05-31  3:36 ` ogawa.yasufumi
  2019-05-31  3:36 ` [spp] [PATCH 09/11] shared/sec: rename func for executing command ogawa.yasufumi
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: ogawa.yasufumi @ 2019-05-31  3:36 UTC (permalink / raw)
  To: spp, ferruh.yigit, ogawa.yasufumi

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

This update is to rename file local `spp_update_classifier_table()`
to `update_cls_table()` to be simple, and revise var names, comments
and log messages for refactoring.

Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
 .../secondary/spp_worker_th/cmd_runner.c      | 71 +++++++++----------
 1 file changed, 34 insertions(+), 37 deletions(-)

diff --git a/src/shared/secondary/spp_worker_th/cmd_runner.c b/src/shared/secondary/spp_worker_th/cmd_runner.c
index 420e19c..f7476c4 100644
--- a/src/shared/secondary/spp_worker_th/cmd_runner.c
+++ b/src/shared/secondary/spp_worker_th/cmd_runner.c
@@ -122,85 +122,82 @@ is_port_flushed(enum port_type iface_type, int iface_no)
 
 /* Update classifier table with given action, add or del. */
 static int
-spp_update_classifier_table(
-		enum sppwk_action wk_action,
+update_cls_table(enum sppwk_action wk_action,
 		enum spp_classifier_type type __attribute__ ((unused)),
-		int vid,
-		const char *mac_addr_str,
+		int vid, const char *mac_str,
 		const struct sppwk_port_idx *port)
 {
-	struct sppwk_port_info *port_info = NULL;
-	int64_t ret_mac = 0;
-	uint64_t mac_addr = 0;
+	/**
+	 * Use two types of mac addr in int64_t and uint64_t because first
+	 * one is checked if converted value from string  is negative for error.
+	 * If it is invalid, convert it to uint64_t.
+	 */
+	int64_t mac_int64;
+	uint64_t mac_uint64;
+	struct sppwk_port_info *port_info;
 
-	RTE_LOG(DEBUG, APP, "update_classifier_table "
-			"( type = mac, mac addr = %s, port = %d:%d )\n",
-			mac_addr_str, port->iface_type, port->iface_no);
+	RTE_LOG(DEBUG, APP, "Called update_cls_table with "
+			"type `mac`, mac_addr `%s`, and port `%d:%d`.\n",
+			mac_str, port->iface_type, port->iface_no);
 
-	ret_mac = sppwk_convert_mac_str_to_int64(mac_addr_str);
-	if (unlikely(ret_mac == -1)) {
-		RTE_LOG(ERR, APP, "MAC address format error. ( mac = %s )\n",
-			mac_addr_str);
+	mac_int64 = sppwk_convert_mac_str_to_int64(mac_str);
+	if (unlikely(mac_int64 == -1)) {
+		RTE_LOG(ERR, APP, "Invalid MAC address `%s`.\n", mac_str);
 		return SPP_RET_NG;
 	}
-	mac_addr = (uint64_t)ret_mac;
+	mac_uint64 = (uint64_t)mac_int64;
 
 	port_info = get_sppwk_port(port->iface_type, port->iface_no);
 	if (unlikely(port_info == NULL)) {
-		RTE_LOG(ERR, APP, "No port. ( port = %d:%d )\n",
+		RTE_LOG(ERR, APP, "Failed to get port %d:%d.\n",
 				port->iface_type, port->iface_no);
 		return SPP_RET_NG;
 	}
 	if (unlikely(port_info->iface_type == UNDEF)) {
-		RTE_LOG(ERR, APP, "Port not added. ( port = %d:%d )\n",
+		RTE_LOG(ERR, APP, "Port %d:%d doesn't exist.\n",
 				port->iface_type, port->iface_no);
 		return SPP_RET_NG;
 	}
 
 	if (wk_action == SPPWK_ACT_DEL) {
-		/* Delete */
 		if ((port_info->cls_attrs.vlantag.vid != 0) &&
-				unlikely(port_info->cls_attrs.vlantag.vid !=
-				vid)) {
-			RTE_LOG(ERR, APP, "VLAN ID is different. "
-					"( vid = %d )\n", vid);
+				port_info->cls_attrs.vlantag.vid != vid) {
+			RTE_LOG(ERR, APP, "Unexpected VLAN ID `%d`.\n", vid);
 			return SPP_RET_NG;
 		}
 		if ((port_info->cls_attrs.mac_addr != 0) &&
-			unlikely(port_info->cls_attrs.mac_addr !=
-					mac_addr)) {
-			RTE_LOG(ERR, APP, "MAC address is different. "
-					"( mac = %s )\n", mac_addr_str);
+				port_info->cls_attrs.mac_addr != mac_uint64) {
+			RTE_LOG(ERR, APP, "Unexpected MAC %s.\n", mac_str);
 			return SPP_RET_NG;
 		}
 
+		/* Initialize deleted attributes again. */
 		port_info->cls_attrs.vlantag.vid = ETH_VLAN_ID_MAX;
-		port_info->cls_attrs.mac_addr    = 0;
+		port_info->cls_attrs.mac_addr = 0;
 		memset(port_info->cls_attrs.mac_addr_str, 0x00,
 							SPP_MIN_STR_LEN);
-
 	} else if (wk_action == SPPWK_ACT_ADD) {
-		/* Setting */
 		if (unlikely(port_info->cls_attrs.vlantag.vid !=
 				ETH_VLAN_ID_MAX)) {
-			RTE_LOG(ERR, APP, "Port in used. "
-					"( port = %d:%d, vlan = %d != %d )\n",
+			/* TODO(yasufum) why two vids are required in msg ? */
+			RTE_LOG(ERR, APP, "Used port %d:%d, vid %d != %d.\n",
 					port->iface_type, port->iface_no,
 					port_info->cls_attrs.vlantag.vid, vid);
 			return SPP_RET_NG;
 		}
 		if (unlikely(port_info->cls_attrs.mac_addr != 0)) {
-			RTE_LOG(ERR, APP, "Port in used. "
-					"( port = %d:%d, mac = %s != %s )\n",
+			/* TODO(yasufum) why two macs are required in msg ? */
+			RTE_LOG(ERR, APP, "Used port %d:%d, mac %s != %s.\n",
 					port->iface_type, port->iface_no,
 					port_info->cls_attrs.mac_addr_str,
-					mac_addr_str);
+					mac_str);
 			return SPP_RET_NG;
 		}
 
+		/* Update attrs with validated params. */
 		port_info->cls_attrs.vlantag.vid = vid;
-		port_info->cls_attrs.mac_addr    = mac_addr;
-		strcpy(port_info->cls_attrs.mac_addr_str, mac_addr_str);
+		port_info->cls_attrs.mac_addr = mac_uint64;
+		strcpy(port_info->cls_attrs.mac_addr_str, mac_str);
 	}
 
 	set_component_change_port(port_info, SPP_PORT_RXTX_TX);
@@ -733,7 +730,7 @@ execute_command(const struct spp_command *command)
 	case SPPWK_CMDTYPE_CLS_VLAN:
 		RTE_LOG(INFO, SPP_COMMAND_PROC,
 				"Execute classifier_table command.\n");
-		ret = spp_update_classifier_table(
+		ret = update_cls_table(
 				command->spec.cls_table.wk_action,
 				command->spec.cls_table.type,
 				command->spec.cls_table.vid,
-- 
2.17.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [spp] [PATCH 09/11] shared/sec: rename func for executing command
  2019-05-31  3:35 [spp] [PATCH 00/11] Refactor functions for handling commands ogawa.yasufumi
                   ` (7 preceding siblings ...)
  2019-05-31  3:36 ` [spp] [PATCH 08/11] shared/sec: refactor func for updating cls table ogawa.yasufumi
@ 2019-05-31  3:36 ` ogawa.yasufumi
  2019-05-31  3:36 ` [spp] [PATCH 10/11] shared/sec: add helpers for logging cmd parser ogawa.yasufumi
  2019-05-31  3:36 ` [spp] [PATCH 11/11] shared/sec: rename func for updating port ogawa.yasufumi
  10 siblings, 0 replies; 12+ messages in thread
From: ogawa.yasufumi @ 2019-05-31  3:36 UTC (permalink / raw)
  To: spp, ferruh.yigit, ogawa.yasufumi

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

This update is to rename the name of function from
`execute_command()` to `exec_cmd()`, and to refactor log messages.

Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
 .../secondary/spp_worker_th/cmd_runner.c      | 70 +++++++++----------
 1 file changed, 32 insertions(+), 38 deletions(-)

diff --git a/src/shared/secondary/spp_worker_th/cmd_runner.c b/src/shared/secondary/spp_worker_th/cmd_runner.c
index f7476c4..0000203 100644
--- a/src/shared/secondary/spp_worker_th/cmd_runner.c
+++ b/src/shared/secondary/spp_worker_th/cmd_runner.c
@@ -206,11 +206,8 @@ update_cls_table(enum sppwk_action wk_action,
 
 /* Assign worker thread or remove on specified lcore. */
 static int
-spp_update_component(
-		enum sppwk_action wk_action,
-		const char *name,
-		unsigned int lcore_id,
-		enum spp_component_type type)
+update_comp(enum sppwk_action wk_action, const char *name,
+		unsigned int lcore_id, enum spp_component_type type)
 {
 	int ret = SPP_RET_NG;
 	int ret_del = -1;
@@ -719,67 +716,64 @@ append_json_block_brackets(const char *name, char **output, const char *str)
 	return SPP_RET_OK;
 }
 
-/* execute one command */
+/* Execute one command. */
 static int
-execute_command(const struct spp_command *command)
+exec_cmd(const struct spp_command *cmd)
 {
-	int ret = SPP_RET_OK;
+	int ret;
 
-	switch (command->type) {
+	switch (cmd->type) {
 	case SPPWK_CMDTYPE_CLS_MAC:
 	case SPPWK_CMDTYPE_CLS_VLAN:
 		RTE_LOG(INFO, SPP_COMMAND_PROC,
-				"Execute classifier_table command.\n");
-		ret = update_cls_table(
-				command->spec.cls_table.wk_action,
-				command->spec.cls_table.type,
-				command->spec.cls_table.vid,
-				command->spec.cls_table.mac,
-				&command->spec.cls_table.port);
+				"Exec classifier_table cmd.\n");
+		ret = update_cls_table(cmd->spec.cls_table.wk_action,
+				cmd->spec.cls_table.type,
+				cmd->spec.cls_table.vid,
+				cmd->spec.cls_table.mac,
+				&cmd->spec.cls_table.port);
 		if (ret == 0) {
-			RTE_LOG(INFO, SPP_COMMAND_PROC,
-					"Execute flush.\n");
+			RTE_LOG(INFO, SPP_COMMAND_PROC, "Exec flush.\n");
 			ret = spp_flush();
 		}
 		break;
 
 	case SPPWK_CMDTYPE_WORKER:
 		RTE_LOG(INFO, SPP_COMMAND_PROC,
-				"Execute component command.\n");
-		ret = spp_update_component(
-				command->spec.comp.wk_action,
-				command->spec.comp.name,
-				command->spec.comp.core,
-				command->spec.comp.type);
+				"Exec component cmd.\n");
+		ret = update_comp(
+				cmd->spec.comp.wk_action,
+				cmd->spec.comp.name,
+				cmd->spec.comp.core,
+				cmd->spec.comp.type);
 		if (ret == 0) {
-			RTE_LOG(INFO, SPP_COMMAND_PROC,
-					"Execute flush.\n");
+			RTE_LOG(INFO, SPP_COMMAND_PROC, "Exec flush.\n");
 			ret = spp_flush();
 		}
 		break;
 
 	case SPPWK_CMDTYPE_PORT:
 		RTE_LOG(INFO, SPP_COMMAND_PROC,
-				"Execute port command. (act = %d)\n",
-				command->spec.port.wk_action);
+				"Exec port command, act=%d.\n",
+				cmd->spec.port.wk_action);
 		ret = spp_update_port(
-				command->spec.port.wk_action,
-				&command->spec.port.port,
-				command->spec.port.rxtx,
-				command->spec.port.name,
-				&command->spec.port.ability);
+				cmd->spec.port.wk_action,
+				&cmd->spec.port.port,
+				cmd->spec.port.rxtx,
+				cmd->spec.port.name,
+				&cmd->spec.port.ability);
 		if (ret == 0) {
-			RTE_LOG(INFO, SPP_COMMAND_PROC,
-					"Execute flush.\n");
+			RTE_LOG(INFO, SPP_COMMAND_PROC, "Exec flush.\n");
 			ret = spp_flush();
 		}
 		break;
 
 	default:
 		RTE_LOG(INFO, SPP_COMMAND_PROC,
-				"Execute other command. type=%d\n",
-				command->type);
+				"Exec other command, type=%d.\n",
+				cmd->type);
 		/* nothing to do here */
+		ret = SPP_RET_OK;
 		break;
 	}
 
@@ -1664,7 +1658,7 @@ process_request(int *sock, const char *request_str, size_t request_str_len)
 
 	/* execute commands */
 	for (i = 0; i < request.num_command ; ++i) {
-		ret = execute_command(request.commands + i);
+		ret = exec_cmd(request.commands + i);
 		if (unlikely(ret != SPP_RET_OK)) {
 			set_cmd_result(&command_results[i], CMD_FAILED,
 					"error occur");
-- 
2.17.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [spp] [PATCH 10/11] shared/sec: add helpers for logging cmd parser
  2019-05-31  3:35 [spp] [PATCH 00/11] Refactor functions for handling commands ogawa.yasufumi
                   ` (8 preceding siblings ...)
  2019-05-31  3:36 ` [spp] [PATCH 09/11] shared/sec: rename func for executing command ogawa.yasufumi
@ 2019-05-31  3:36 ` ogawa.yasufumi
  2019-05-31  3:36 ` [spp] [PATCH 11/11] shared/sec: rename func for updating port ogawa.yasufumi
  10 siblings, 0 replies; 12+ messages in thread
From: ogawa.yasufumi @ 2019-05-31  3:36 UTC (permalink / raw)
  To: spp, ferruh.yigit, ogawa.yasufumi

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

This update is to add helper functions for refactoring logging messages.
Some of INFO log messages show internal codes which users cannot
understand. These helper functions replace the internal code to
meaningful word such as `status`, `exit` or so.

Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
 .../secondary/spp_worker_th/cmd_parser.c      | 48 ++++++++++++++++++-
 .../secondary/spp_worker_th/cmd_parser.h      |  4 ++
 .../secondary/spp_worker_th/cmd_runner.c      | 17 +++----
 3 files changed, 56 insertions(+), 13 deletions(-)

diff --git a/src/shared/secondary/spp_worker_th/cmd_parser.c b/src/shared/secondary/spp_worker_th/cmd_parser.c
index 084b3e2..af8cc3e 100644
--- a/src/shared/secondary/spp_worker_th/cmd_parser.c
+++ b/src/shared/secondary/spp_worker_th/cmd_parser.c
@@ -15,8 +15,9 @@
 #define RTE_LOGTYPE_APP RTE_LOGTYPE_USER2
 
 /**
- * List of command action. The order of items should be same as the order of
- * enum `sppwk_action` in cmd_parser.h.
+ * List of command action for getting the index of enum enum `sppwk_action`.
+ * The order of items should be same as the order of enum `sppwk_action` in
+ * cmd_parser.h.
  */
 const char *CMD_ACT_LIST[] = {
 	"none",
@@ -27,6 +28,49 @@ const char *CMD_ACT_LIST[] = {
 	"",  /* termination */
 };
 
+/* Get string of action. It is mainly used for logging. */
+const char*
+sppwk_action_str(enum sppwk_action wk_action)
+{
+	switch (wk_action) {
+	case SPPWK_ACT_NONE:
+		return "none";
+	case SPPWK_ACT_START:
+		return "start";
+	case SPPWK_ACT_STOP:
+		return "stop";
+	case SPPWK_ACT_ADD:
+		return "add";
+	case SPPWK_ACT_DEL:
+		return "del";
+	default:
+		return "unknown";
+	}
+}
+
+/* Get string of cmd type. It is mainly used for logging. */
+const char*
+sppwk_cmd_type_str(enum sppwk_cmd_type ctype)
+{
+	switch (ctype) {
+	case SPPWK_CMDTYPE_CLS_MAC:
+	case SPPWK_CMDTYPE_CLS_VLAN:
+		return "classifier_mac";
+	case SPPWK_CMDTYPE_CLIENT_ID:
+		return "_get_client_id";
+	case SPPWK_CMDTYPE_STATUS:
+		return "status";
+	case SPPWK_CMDTYPE_EXIT:
+		return "exit";
+	case SPPWK_CMDTYPE_WORKER:
+		return "component";
+	case SPPWK_CMDTYPE_PORT:
+		return "port";
+	default:
+		return "unknown";
+	}
+}
+
 /**
  * List of classifier type. The order of items should be same as the order of
  * enum `spp_classifier_type` defined in spp_proc.h.
diff --git a/src/shared/secondary/spp_worker_th/cmd_parser.h b/src/shared/secondary/spp_worker_th/cmd_parser.h
index 286fde0..b90f52a 100644
--- a/src/shared/secondary/spp_worker_th/cmd_parser.h
+++ b/src/shared/secondary/spp_worker_th/cmd_parser.h
@@ -51,6 +51,8 @@ enum sppwk_action {
 	SPPWK_ACT_DEL,   /**< delete */
 };
 
+const char *sppwk_action_str(enum sppwk_action wk_action);
+
 /**
  * SPP command type.
  *
@@ -72,6 +74,8 @@ enum sppwk_cmd_type {
 	SPPWK_CMDTYPE_PORT,  /**< port */
 };
 
+const char *sppwk_cmd_type_str(enum sppwk_cmd_type ctype);
+
 /* `classifier_table` command specific parameters. */
 struct sppwk_cls_cmd_attrs {
 	enum sppwk_action wk_action;  /**< add or del */
diff --git a/src/shared/secondary/spp_worker_th/cmd_runner.c b/src/shared/secondary/spp_worker_th/cmd_runner.c
index 0000203..5584a48 100644
--- a/src/shared/secondary/spp_worker_th/cmd_runner.c
+++ b/src/shared/secondary/spp_worker_th/cmd_runner.c
@@ -722,11 +722,12 @@ exec_cmd(const struct spp_command *cmd)
 {
 	int ret;
 
+	RTE_LOG(INFO, SPP_COMMAND_PROC, "Exec `%s` cmd.\n",
+			sppwk_cmd_type_str(cmd->type));
+
 	switch (cmd->type) {
 	case SPPWK_CMDTYPE_CLS_MAC:
 	case SPPWK_CMDTYPE_CLS_VLAN:
-		RTE_LOG(INFO, SPP_COMMAND_PROC,
-				"Exec classifier_table cmd.\n");
 		ret = update_cls_table(cmd->spec.cls_table.wk_action,
 				cmd->spec.cls_table.type,
 				cmd->spec.cls_table.vid,
@@ -739,8 +740,6 @@ exec_cmd(const struct spp_command *cmd)
 		break;
 
 	case SPPWK_CMDTYPE_WORKER:
-		RTE_LOG(INFO, SPP_COMMAND_PROC,
-				"Exec component cmd.\n");
 		ret = update_comp(
 				cmd->spec.comp.wk_action,
 				cmd->spec.comp.name,
@@ -753,9 +752,8 @@ exec_cmd(const struct spp_command *cmd)
 		break;
 
 	case SPPWK_CMDTYPE_PORT:
-		RTE_LOG(INFO, SPP_COMMAND_PROC,
-				"Exec port command, act=%d.\n",
-				cmd->spec.port.wk_action);
+		RTE_LOG(INFO, SPP_COMMAND_PROC, "with action `%s`.\n",
+				sppwk_action_str(cmd->spec.port.wk_action));
 		ret = spp_update_port(
 				cmd->spec.port.wk_action,
 				&cmd->spec.port.port,
@@ -769,10 +767,7 @@ exec_cmd(const struct spp_command *cmd)
 		break;
 
 	default:
-		RTE_LOG(INFO, SPP_COMMAND_PROC,
-				"Exec other command, type=%d.\n",
-				cmd->type);
-		/* nothing to do here */
+		/* Do nothing. */
 		ret = SPP_RET_OK;
 		break;
 	}
-- 
2.17.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [spp] [PATCH 11/11] shared/sec: rename func for updating port
  2019-05-31  3:35 [spp] [PATCH 00/11] Refactor functions for handling commands ogawa.yasufumi
                   ` (9 preceding siblings ...)
  2019-05-31  3:36 ` [spp] [PATCH 10/11] shared/sec: add helpers for logging cmd parser ogawa.yasufumi
@ 2019-05-31  3:36 ` ogawa.yasufumi
  10 siblings, 0 replies; 12+ messages in thread
From: ogawa.yasufumi @ 2019-05-31  3:36 UTC (permalink / raw)
  To: spp, ferruh.yigit, ogawa.yasufumi

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

This update is to rename from `spp_update_port()` to `update_port()`.

Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
 src/shared/secondary/spp_worker_th/cmd_runner.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/src/shared/secondary/spp_worker_th/cmd_runner.c b/src/shared/secondary/spp_worker_th/cmd_runner.c
index 5584a48..e917007 100644
--- a/src/shared/secondary/spp_worker_th/cmd_runner.c
+++ b/src/shared/secondary/spp_worker_th/cmd_runner.c
@@ -345,7 +345,7 @@ check_port_count(int component_type, enum spp_port_rxtx rxtx, int num_rx,
 
 /* Port add or del to execute it */
 static int
-spp_update_port(enum sppwk_action wk_action,
+update_port(enum sppwk_action wk_action,
 		const struct sppwk_port_idx *port,
 		enum spp_port_rxtx rxtx,
 		const char *name,
@@ -754,12 +754,9 @@ exec_cmd(const struct spp_command *cmd)
 	case SPPWK_CMDTYPE_PORT:
 		RTE_LOG(INFO, SPP_COMMAND_PROC, "with action `%s`.\n",
 				sppwk_action_str(cmd->spec.port.wk_action));
-		ret = spp_update_port(
-				cmd->spec.port.wk_action,
-				&cmd->spec.port.port,
-				cmd->spec.port.rxtx,
-				cmd->spec.port.name,
-				&cmd->spec.port.ability);
+		ret = update_port(cmd->spec.port.wk_action,
+				&cmd->spec.port.port, cmd->spec.port.rxtx,
+				cmd->spec.port.name, &cmd->spec.port.ability);
 		if (ret == 0) {
 			RTE_LOG(INFO, SPP_COMMAND_PROC, "Exec flush.\n");
 			ret = spp_flush();
-- 
2.17.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2019-05-31  3:38 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-31  3:35 [spp] [PATCH 00/11] Refactor functions for handling commands ogawa.yasufumi
2019-05-31  3:35 ` [spp] [PATCH 01/11] shared/sec: rename functions of sppwk_cmd_runner ogawa.yasufumi
2019-05-31  3:35 ` [spp] [PATCH 02/11] shared/sec: revise enum for cmd response ogawa.yasufumi
2019-05-31  3:35 ` [spp] [PATCH 03/11] shared/sec: refactor passing err in cmd_runner ogawa.yasufumi
2019-05-31  3:35 ` [spp] [PATCH 04/11] shared/sec: rename struct for command response ogawa.yasufumi
2019-05-31  3:35 ` [spp] [PATCH 05/11] shared/sec: refactor funcs for managing port info ogawa.yasufumi
2019-05-31  3:36 ` [spp] [PATCH 06/11] shared/sec: rename util functions in cmd_runner ogawa.yasufumi
2019-05-31  3:36 ` [spp] [PATCH 07/11] shared/sec: rename func for getting component ID ogawa.yasufumi
2019-05-31  3:36 ` [spp] [PATCH 08/11] shared/sec: refactor func for updating cls table ogawa.yasufumi
2019-05-31  3:36 ` [spp] [PATCH 09/11] shared/sec: rename func for executing command ogawa.yasufumi
2019-05-31  3:36 ` [spp] [PATCH 10/11] shared/sec: add helpers for logging cmd parser ogawa.yasufumi
2019-05-31  3:36 ` [spp] [PATCH 11/11] shared/sec: rename func for updating port ogawa.yasufumi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).