Soft Patch Panel
 help / color / mirror / Atom feed
* [spp] [PATCH 00/10] Refactor cmd parser in spp_pcap
@ 2019-05-31  8:52 ogawa.yasufumi
  2019-05-31  8:52 ` [spp] [PATCH 01/10] spp_pcap: rename enum spp_command_type ogawa.yasufumi
                   ` (9 more replies)
  0 siblings, 10 replies; 11+ messages in thread
From: ogawa.yasufumi @ 2019-05-31  8:52 UTC (permalink / raw)
  To: spp, ferruh.yigit, ogawa.yasufumi

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

This series of patches is to revise a set of functions, structs and vars
to be appropriately because Some of them are too redundant or vague in
meaning.

Yasufumi Ogawa (10):
  spp_pcap: rename enum spp_command_type
  spp_pcap: rename struct spp_command
  spp_pcap: revise command response codes
  spp_pcap: revise parser functions
  spp_pcap: refactor func for splitting cmd tokens
  spp_pcap: revise log msgs in parser func
  spp_pcap: remove unused string list
  spp_pcap: refactor comments of spp_pcap
  spp_pcap: rename spp_get_core_status
  spp_pcap: revise name of vars for parser error

 src/pcap/cmd_parser.c                         | 158 ++++++++++--------
 src/pcap/cmd_parser.h                         |  32 ++--
 src/pcap/cmd_runner.c                         | 142 +++++++---------
 src/pcap/cmd_utils.c                          |   4 +-
 src/pcap/cmd_utils.h                          |  28 ++--
 src/pcap/spp_pcap.c                           |  95 +++++------
 src/pcap/spp_pcap.h                           |  12 +-
 .../secondary/spp_worker_th/cmd_parser.c      |   4 +-
 .../secondary/spp_worker_th/cmd_parser.h      |   3 +-
 .../secondary/spp_worker_th/cmd_runner.c      |   8 +-
 10 files changed, 221 insertions(+), 265 deletions(-)

-- 
2.17.1


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

* [spp] [PATCH 01/10] spp_pcap: rename enum spp_command_type
  2019-05-31  8:52 [spp] [PATCH 00/10] Refactor cmd parser in spp_pcap ogawa.yasufumi
@ 2019-05-31  8:52 ` ogawa.yasufumi
  2019-05-31  8:52 ` [spp] [PATCH 02/10] spp_pcap: rename struct spp_command ogawa.yasufumi
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: ogawa.yasufumi @ 2019-05-31  8:52 UTC (permalink / raw)
  To: spp, ferruh.yigit, ogawa.yasufumi

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

This update is to rename enum `spp_command_type` to `pcap_cmd_type` and
its members.

Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
 src/pcap/cmd_parser.c                         | 24 ++++++-------
 src/pcap/cmd_parser.h                         | 25 +++++--------
 src/pcap/cmd_runner.c                         | 35 ++++++-------------
 .../secondary/spp_worker_th/cmd_parser.h      |  3 +-
 4 files changed, 33 insertions(+), 54 deletions(-)

diff --git a/src/pcap/cmd_parser.c b/src/pcap/cmd_parser.c
index 4cf8059..946300e 100644
--- a/src/pcap/cmd_parser.c
+++ b/src/pcap/cmd_parser.c
@@ -66,18 +66,18 @@ struct parse_command_list {
 			char *argv[], struct sppwk_parse_err_msg *error,
 			int maxargc);
 				/* Pointer to command handling function */
-	enum spp_command_type type;
+	enum pcap_cmd_type type;
 				/* Command type */
 };
 
 /* command list */
 static struct parse_command_list command_list_pcap[] = {
-	{ "_get_client_id", 1, 1, NULL, CMD_CLIENT_ID },
-	{ "status",	    1, 1, NULL, CMD_STATUS    },
-	{ "exit",           1, 1, NULL, CMD_EXIT      },
-	{ "start",          1, 1, NULL, CMD_START     },
-	{ "stop",           1, 1, NULL, CMD_STOP      },
-	{ "",               0, 0, NULL, 0 }  /* termination */
+	{ "_get_client_id", 1, 1, NULL, PCAP_CMDTYPE_CLIENT_ID },
+	{ "status", 1, 1, NULL, PCAP_CMDTYPE_STATUS },
+	{ "exit",  1, 1, NULL, PCAP_CMDTYPE_EXIT },
+	{ "start", 1, 1, NULL, PCAP_CMDTYPE_START },
+	{ "stop",  1, 1, NULL, PCAP_CMDTYPE_STOP },
+	{ "", 0, 0, NULL, 0 }  /* termination */
 };
 
 /* Parse command line parameters */
@@ -164,19 +164,19 @@ spp_command_parse_request(
 	/* check getter command */
 	for (i = 0; i < request->num_valid_command; ++i) {
 		switch (request->commands[i].type) {
-		case CMD_CLIENT_ID:
+		case PCAP_CMDTYPE_CLIENT_ID:
 			request->is_requested_client_id = 1;
 			break;
-		case CMD_STATUS:
+		case PCAP_CMDTYPE_STATUS:
 			request->is_requested_status = 1;
 			break;
-		case CMD_EXIT:
+		case PCAP_CMDTYPE_EXIT:
 			request->is_requested_exit = 1;
 			break;
-		case CMD_START:
+		case PCAP_CMDTYPE_START:
 			request->is_requested_start = 1;
 			break;
-		case CMD_STOP:
+		case PCAP_CMDTYPE_STOP:
 			request->is_requested_stop = 1;
 			break;
 		default:
diff --git a/src/pcap/cmd_parser.h b/src/pcap/cmd_parser.h
index 88ad862..fd110cb 100644
--- a/src/pcap/cmd_parser.h
+++ b/src/pcap/cmd_parser.h
@@ -42,27 +42,18 @@ enum sppwk_parse_error_code {
  * @attention This enumerated type must have the same order of command_list
  *            defined in command_dec_pcap.c
  */
-enum spp_command_type {
-	/** get_client_id command */
-	CMD_CLIENT_ID,
-
-	/** status command */
-	CMD_STATUS,
-
-	/** exit command */
-	CMD_EXIT,
-
-	/** start command */
-	CMD_START,
-
-	/** stop command */
-	CMD_STOP,
-
+/* TODO(yasufum) consider to remove restriction above. */
+enum pcap_cmd_type {
+	PCAP_CMDTYPE_CLIENT_ID,  /**< get_client_id */
+	PCAP_CMDTYPE_STATUS,  /**< status */
+	PCAP_CMDTYPE_EXIT,  /**< exit */
+	PCAP_CMDTYPE_START,  /**< worker thread */
+	PCAP_CMDTYPE_STOP,  /**< port */
 };
 
 /** command parameters */
 struct spp_command {
-	enum spp_command_type type; /**< Command type */
+	enum pcap_cmd_type type; /**< Command type */
 };
 
 /** request parameters */
diff --git a/src/pcap/cmd_runner.c b/src/pcap/cmd_runner.c
index a931956..c83006f 100644
--- a/src/pcap/cmd_runner.c
+++ b/src/pcap/cmd_runner.c
@@ -233,6 +233,7 @@ append_json_block_brackets(const char *name, char **output, const char *str)
 	return SPPWK_RET_OK;
 }
 
+/* TODO(yasufum) confirm why this function does nothing is needed. */
 /* execute one command */
 static int
 execute_command(const struct spp_command *command)
@@ -240,29 +241,20 @@ execute_command(const struct spp_command *command)
 	int ret = SPPWK_RET_OK;
 
 	switch (command->type) {
-	case CMD_CLIENT_ID:
-		RTE_LOG(INFO, PCAP_RUNNER,
-				"Execute get_client_id command.\n");
+	case PCAP_CMDTYPE_CLIENT_ID:
+		RTE_LOG(INFO, PCAP_RUNNER, "Exec get_client_id cmd.\n");
 		break;
-
-	case CMD_STATUS:
-		RTE_LOG(INFO, PCAP_RUNNER,
-				"Execute status command.\n");
+	case PCAP_CMDTYPE_STATUS:
+		RTE_LOG(INFO, PCAP_RUNNER, "Exec status cmd.\n");
 		break;
-
-	case CMD_EXIT:
-		RTE_LOG(INFO, PCAP_RUNNER,
-				"Execute exit command.\n");
+	case PCAP_CMDTYPE_EXIT:
+		RTE_LOG(INFO, PCAP_RUNNER, "Exec exit cmd.\n");
 		break;
-
-	case CMD_START:
-		RTE_LOG(INFO, PCAP_RUNNER,
-				"Execute start command.\n");
+	case PCAP_CMDTYPE_START:
+		RTE_LOG(INFO, PCAP_RUNNER, "Exec start cmd.\n");
 		break;
-
-	case CMD_STOP:
-		RTE_LOG(INFO, PCAP_RUNNER,
-				"Execute stop command.\n");
+	case PCAP_CMDTYPE_STOP:
+		RTE_LOG(INFO, PCAP_RUNNER, "Exec stop cmd.\n");
 		break;
 	}
 
@@ -279,26 +271,21 @@ parse_error_message(
 	case SPPWK_PARSE_WRONG_FORMAT:
 		sprintf(message, "Wrong message format");
 		break;
-
 	case SPPWK_PARSE_UNKNOWN_CMD:
 		/* TODO(yasufum) Fix compile err if space exists before "(" */
 		sprintf(message, "Unknown command(%s)", wk_err_msg->details);
 		break;
-
 	case SPPWK_PARSE_NO_PARAM:
 		sprintf(message, "No or insufficient number of params (%s)",
 				wk_err_msg->msg);
 		break;
-
 	case SPPWK_PARSE_INVALID_TYPE:
 		sprintf(message, "Invalid value type (%s)",
 				wk_err_msg->msg);
 		break;
-
 	case SPPWK_PARSE_INVALID_VALUE:
 		sprintf(message, "Invalid value (%s)", wk_err_msg->msg);
 		break;
-
 	default:
 		sprintf(message, "error occur");
 		break;
diff --git a/src/shared/secondary/spp_worker_th/cmd_parser.h b/src/shared/secondary/spp_worker_th/cmd_parser.h
index 5a7df84..1018444 100644
--- a/src/shared/secondary/spp_worker_th/cmd_parser.h
+++ b/src/shared/secondary/spp_worker_th/cmd_parser.h
@@ -59,7 +59,8 @@ const char *sppwk_action_str(enum sppwk_action wk_action);
  * @attention This enumerated type must have the same order of command_list
  *            defined in cmd_parser.c
  */
-/*
+/* TODO(yasufum) consider to remove restriction above. */
+/**
  * TODO(yasufum) consider to divide because each of target of scope is
  * different and not so understandable for usage. For example, worker is
  * including classifier or it status.
-- 
2.17.1


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

* [spp] [PATCH 02/10] spp_pcap: rename struct spp_command
  2019-05-31  8:52 [spp] [PATCH 00/10] Refactor cmd parser in spp_pcap ogawa.yasufumi
  2019-05-31  8:52 ` [spp] [PATCH 01/10] spp_pcap: rename enum spp_command_type ogawa.yasufumi
@ 2019-05-31  8:52 ` ogawa.yasufumi
  2019-05-31  8:52 ` [spp] [PATCH 03/10] spp_pcap: revise command response codes ogawa.yasufumi
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: ogawa.yasufumi @ 2019-05-31  8:52 UTC (permalink / raw)
  To: spp, ferruh.yigit, ogawa.yasufumi

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

Rename struct `spp_command` to `pcap_cmd_attr` because it is not a
command, but a set of attributes actually.

Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
 src/pcap/cmd_parser.c | 7 ++++---
 src/pcap/cmd_parser.h | 7 +++----
 src/pcap/cmd_runner.c | 4 ++--
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/pcap/cmd_parser.c b/src/pcap/cmd_parser.c
index 946300e..1624347 100644
--- a/src/pcap/cmd_parser.c
+++ b/src/pcap/cmd_parser.c
@@ -13,7 +13,8 @@
 
 #define RTE_LOGTYPE_PCAP_PARSER RTE_LOGTYPE_USER2
 
-/* set parse error */
+/* Format error message object and return error code for an error case. */
+/* TODO(yasufum) merge it to the same definition in shared/.../cmd_parser.c */
 static inline int
 set_parse_error(struct sppwk_parse_err_msg *wk_err_msg,
 		const int err_code, const char *err_msg)
@@ -118,7 +119,7 @@ parse_command_in_list(struct spp_command_request *request,
 			continue;
 		}
 
-		request->commands[0].type = command_list_pcap[i].type;
+		request->cmd_attrs[0].type = command_list_pcap[i].type;
 		if (list->func != NULL)
 			return (*list->func)(request, argc, argv, wk_err_msg,
 							list->param_max);
@@ -163,7 +164,7 @@ spp_command_parse_request(
 
 	/* check getter command */
 	for (i = 0; i < request->num_valid_command; ++i) {
-		switch (request->commands[i].type) {
+		switch (request->cmd_attrs[i].type) {
 		case PCAP_CMDTYPE_CLIENT_ID:
 			request->is_requested_client_id = 1;
 			break;
diff --git a/src/pcap/cmd_parser.h b/src/pcap/cmd_parser.h
index fd110cb..706144d 100644
--- a/src/pcap/cmd_parser.h
+++ b/src/pcap/cmd_parser.h
@@ -51,16 +51,15 @@ enum pcap_cmd_type {
 	PCAP_CMDTYPE_STOP,  /**< port */
 };
 
-/** command parameters */
-struct spp_command {
-	enum pcap_cmd_type type; /**< Command type */
+struct pcap_cmd_attr {
+	enum pcap_cmd_type type;
 };
 
 /** request parameters */
 struct spp_command_request {
 	int num_command;                /**< Number of accepted commands */
 	int num_valid_command;          /**< Number of executed commands */
-	struct spp_command commands[SPPWK_MAX_CMDS];
+	struct pcap_cmd_attr cmd_attrs[SPPWK_MAX_CMDS];
 					/**<Information of executed commands */
 
 	int is_requested_client_id;     /**< Id for get_client_id command */
diff --git a/src/pcap/cmd_runner.c b/src/pcap/cmd_runner.c
index c83006f..9e4f4cf 100644
--- a/src/pcap/cmd_runner.c
+++ b/src/pcap/cmd_runner.c
@@ -236,7 +236,7 @@ append_json_block_brackets(const char *name, char **output, const char *str)
 /* TODO(yasufum) confirm why this function does nothing is needed. */
 /* execute one command */
 static int
-execute_command(const struct spp_command *command)
+execute_command(const struct pcap_cmd_attr *command)
 {
 	int ret = SPPWK_RET_OK;
 
@@ -892,7 +892,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 = execute_command(request.cmd_attrs + i);
 		if (unlikely(ret != SPPWK_RET_OK)) {
 			set_command_results(&command_results[i], CMD_FAILURE,
 					"error occur");
-- 
2.17.1


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

* [spp] [PATCH 03/10] spp_pcap: revise command response codes
  2019-05-31  8:52 [spp] [PATCH 00/10] Refactor cmd parser in spp_pcap ogawa.yasufumi
  2019-05-31  8:52 ` [spp] [PATCH 01/10] spp_pcap: rename enum spp_command_type ogawa.yasufumi
  2019-05-31  8:52 ` [spp] [PATCH 02/10] spp_pcap: rename struct spp_command ogawa.yasufumi
@ 2019-05-31  8:52 ` ogawa.yasufumi
  2019-05-31  8:52 ` [spp] [PATCH 04/10] spp_pcap: revise parser functions ogawa.yasufumi
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: ogawa.yasufumi @ 2019-05-31  8:52 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 of response codes,
structs and variables for refactoring.

Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
 src/pcap/cmd_runner.c                         | 58 +++++++++----------
 .../secondary/spp_worker_th/cmd_runner.c      |  2 +-
 2 files changed, 27 insertions(+), 33 deletions(-)

diff --git a/src/pcap/cmd_runner.c b/src/pcap/cmd_runner.c
index 9e4f4cf..9d63bde 100644
--- a/src/pcap/cmd_runner.c
+++ b/src/pcap/cmd_runner.c
@@ -29,31 +29,25 @@
 #define JSON_APPEND_BLOCK         "%s\"%s\": { %s }"
 #define JSON_APPEND_BLOCK_NONAME  "%s%s{ %s }"
 
-/* command execution result type */
-enum command_result_type {
+/* TODO(yasufum) merge it to the same definition in shared/.../cmd_runner.c */
+enum cmd_res_code {
 	CMD_SUCCESS = 0,
 	CMD_FAILURE,
 	CMD_INVALID,
 };
 
 /* command execution result information */
-struct command_result {
-	/* Response code */
-	int code;
-
-	/* Response message */
-	char msg[SPPWK_NAME_BUFSZ];
-
-	/* Detailed response message */
-	char error_message[CMD_RES_ERR_MSG_SIZE];
+/* TODO(yasufum) merge it to the same definition in shared/.../cmd_runner.c */
+struct cmd_result {
+	int code;  /* Response code */
+	char msg[SPPWK_NAME_BUFSZ];  /* Response msg in short. */
+	char err_msg[CMD_RES_ERR_MSG_SIZE];  /* Used only if cmd is failed. */
 };
 
 /* command response list control structure */
-struct command_response_list {
-	/* JSON Tag name */
-	char tag_name[SPPWK_NAME_BUFSZ];
-
-	/* Pointer to handling function */
+/* TODO(yasufum) merge it to the same definition in shared/.../cmd_runner.c */
+struct cmd_response {
+	char tag_name[SPPWK_NAME_BUFSZ];  /* JSON Tag name */
 	int (*func)(const char *name, char **output, void *tmp);
 };
 
@@ -296,30 +290,30 @@ parse_error_message(
 
 /* set the command result */
 static inline void
-set_command_results(struct command_result *result,
+set_command_results(struct cmd_result *result,
 		int code, const char *error_messege)
 {
 	result->code = code;
 	switch (code) {
 	case CMD_SUCCESS:
 		strcpy(result->msg, "success");
-		memset(result->error_message, 0x00, CMD_RES_ERR_MSG_SIZE);
+		memset(result->err_msg, 0x00, CMD_RES_ERR_MSG_SIZE);
 		break;
 	case CMD_FAILURE:
 		strcpy(result->msg, "error");
-		strcpy(result->error_message, error_messege);
+		strcpy(result->err_msg, error_messege);
 		break;
 	case CMD_INVALID: /* FALLTHROUGH */
 	default:
 		strcpy(result->msg, "invalid");
-		memset(result->error_message, 0x00, CMD_RES_ERR_MSG_SIZE);
+		memset(result->err_msg, 0x00, CMD_RES_ERR_MSG_SIZE);
 		break;
 	}
 }
 
 /* set parse error to command result */
 static void
-set_parse_error_to_results(struct command_result *results,
+set_parse_error_to_results(struct cmd_result *results,
 		const struct spp_command_request *request,
 		const struct sppwk_parse_err_msg *wk_err_msg)
 {
@@ -345,7 +339,7 @@ set_parse_error_to_results(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->msg);
 }
 
@@ -354,10 +348,10 @@ static int
 append_error_details_value(const char *name, char **output, void *tmp)
 {
 	int ret = SPPWK_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 SPPWK_RET_OK;
 
 	tmp_buff = spp_strbuf_allocate(CMD_RES_BUF_INIT_SIZE);
@@ -369,7 +363,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 SPPWK_RET_NG;
@@ -553,7 +547,7 @@ append_core_value(const char *name, char **output,
 /* 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 *list,
 		void *tmp)
 {
 	int ret = SPPWK_RET_NG;
@@ -612,14 +606,14 @@ append_response_list_value(char **output,
 #define COMMAND_RESP_TAG_LIST_EMPTY { "", NULL }
 
 /* command response result string list */
-struct command_response_list response_result_list[] = {
+struct cmd_response response_result_list[] = {
 	{ "result",        append_result_value },
 	{ "error_details", append_error_details_value },
 	COMMAND_RESP_TAG_LIST_EMPTY
 };
 
 /* command response status information string list */
-struct command_response_list response_info_list[] = {
+struct cmd_response response_info_list[] = {
 	{ "client-id",        append_client_id_value },
 	{ "status",           append_capture_status_value },
 	{ "master-lcore",     append_master_lcore_value },
@@ -630,7 +624,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 = SPPWK_RET_NG;
 	int i;
@@ -706,7 +700,7 @@ append_info_value(const char *name, char **output)
 static void
 send_parse_error_response(int *sock,
 		const struct spp_command_request *request,
-		struct command_result *command_results)
+		struct cmd_result *command_results)
 {
 	int ret = SPPWK_RET_NG;
 	char *msg, *tmp_buff;
@@ -762,7 +756,7 @@ send_parse_error_response(int *sock,
 static void
 send_command_result_response(int *sock,
 		const struct spp_command_request *request,
-		struct command_result *command_results)
+		struct cmd_result *command_results)
 {
 	int ret = SPPWK_RET_NG;
 	char *msg, *tmp_buff;
@@ -863,7 +857,7 @@ process_request(int *sock, const char *request_str, size_t request_str_len)
 
 	struct spp_command_request request;
 	struct sppwk_parse_err_msg parse_error;
-	struct command_result command_results[SPPWK_MAX_CMDS];
+	struct cmd_result command_results[SPPWK_MAX_CMDS];
 
 	memset(&request, 0, sizeof(struct spp_command_request));
 	memset(&parse_error, 0, sizeof(struct sppwk_parse_err_msg));
diff --git a/src/shared/secondary/spp_worker_th/cmd_runner.c b/src/shared/secondary/spp_worker_th/cmd_runner.c
index 8672a25..ba3fdc6 100644
--- a/src/shared/secondary/spp_worker_th/cmd_runner.c
+++ b/src/shared/secondary/spp_worker_th/cmd_runner.c
@@ -34,7 +34,7 @@
 #define JSON_APPEND_BLOCK         "%s\"%s\": { %s }"
 #define JSON_APPEND_BLOCK_NONAME  "%s%s{ %s }"
 
-enum cmd_res_codes {
+enum cmd_res_code {
 	CMD_SUCCESS = 0,
 	CMD_FAILED,
 	CMD_INVALID,
-- 
2.17.1


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

* [spp] [PATCH 04/10] spp_pcap: revise parser functions
  2019-05-31  8:52 [spp] [PATCH 00/10] Refactor cmd parser in spp_pcap ogawa.yasufumi
                   ` (2 preceding siblings ...)
  2019-05-31  8:52 ` [spp] [PATCH 03/10] spp_pcap: revise command response codes ogawa.yasufumi
@ 2019-05-31  8:52 ` ogawa.yasufumi
  2019-05-31  8:52 ` [spp] [PATCH 05/10] spp_pcap: refactor func for splitting cmd tokens ogawa.yasufumi
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: ogawa.yasufumi @ 2019-05-31  8:52 UTC (permalink / raw)
  To: spp, ferruh.yigit, ogawa.yasufumi

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

This update is to revise name of functions, struct, and variables for
refactoring.

Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
 src/pcap/cmd_parser.c                         | 99 ++++++++++---------
 src/pcap/cmd_parser.h                         |  2 +-
 src/pcap/cmd_runner.c                         |  4 +-
 .../secondary/spp_worker_th/cmd_parser.c      |  4 +-
 4 files changed, 58 insertions(+), 51 deletions(-)

diff --git a/src/pcap/cmd_parser.c b/src/pcap/cmd_parser.c
index 1624347..577e952 100644
--- a/src/pcap/cmd_parser.c
+++ b/src/pcap/cmd_parser.c
@@ -38,41 +38,47 @@ set_string_value_parse_error(struct sppwk_parse_err_msg *wk_err_msg,
 
 /* Split command line parameter with spaces */
 static int
-parse_parameter_value(char *string, int max, int *argc, char *argv[])
+parse_parameter_value(char *string, int max, int *nof_tokens, char *tokens[])
 {
 	int cnt = 0;
 	const char *delim = " ";
-	char *argv_tok = NULL;
+	char *token = NULL;
 	char *saveptr = NULL;
 
-	argv_tok = strtok_r(string, delim, &saveptr);
-	while (argv_tok != NULL) {
+	token = strtok_r(string, delim, &saveptr);
+	while (token != NULL) {
 		if (cnt >= max)
 			return SPPWK_RET_NG;
-		argv[cnt] = argv_tok;
+		tokens[cnt] = token;
 		cnt++;
-		argv_tok = strtok_r(NULL, delim, &saveptr);
+		token = strtok_r(NULL, delim, &saveptr);
 	}
-	*argc = cnt;
+	*nof_tokens = cnt;
 
 	return SPPWK_RET_OK;
 }
 
-/* command list for parse */
-struct parse_command_list {
-	const char *name;       /* Command name */
-	int   param_min;        /* Min number of parameters */
-	int   param_max;        /* Max number of parameters */
-	int (*func)(struct spp_command_request *request, int argc,
-			char *argv[], struct sppwk_parse_err_msg *error,
-			int maxargc);
+/**
+ * A set of attributes of commands for parsing. The fourth member of function
+ * pointer is the operator function for the command.
+ */
+struct pcap_cmd_parse_attr {
+	const char *cmd_name;
+	int nof_params_min;
+	int nof_params_max;
+	int (*func)(struct spp_command_request *request, int nof_tokens,
+			char *tokens[], struct sppwk_parse_err_msg *error,
+			int nof_max_tokens);
 				/* Pointer to command handling function */
 	enum pcap_cmd_type type;
 				/* Command type */
 };
 
-/* command list */
-static struct parse_command_list command_list_pcap[] = {
+/**
+ * List of command attributes defines the name of command, number of params
+ * and operator functions.
+ */
+static struct pcap_cmd_parse_attr pcap_cmd_attrs[] = {
 	{ "_get_client_id", 1, 1, NULL, PCAP_CMDTYPE_CLIENT_ID },
 	{ "status", 1, 1, NULL, PCAP_CMDTYPE_STATUS },
 	{ "exit",  1, 1, NULL, PCAP_CMDTYPE_EXIT },
@@ -81,53 +87,54 @@ static struct parse_command_list command_list_pcap[] = {
 	{ "", 0, 0, NULL, 0 }  /* termination */
 };
 
-/* Parse command line parameters */
+/* Parse command requested from spp-ctl. */
 static int
-parse_command_in_list(struct spp_command_request *request,
-			const char *request_str,
-			struct sppwk_parse_err_msg *wk_err_msg)
+parse_pcap_cmd(struct spp_command_request *request,
+		const char *request_str,
+		struct sppwk_parse_err_msg *wk_err_msg)
 {
-	int ret = SPPWK_RET_OK;
-	int command_name_check = 0;
-	struct parse_command_list *list = NULL;
-	int i = 0;
-	int argc = 0;
-	char *argv[SPPWK_MAX_PARAMS];
+	int is_invalid_cmd = 0;
+	struct pcap_cmd_parse_attr *cmd_attr = NULL;
+	int ret;
+	int i;
+	char *tokens[SPPWK_MAX_PARAMS];
+	int nof_tokens = 0;
 	char tmp_str[SPPWK_MAX_PARAMS * SPPWK_VAL_BUFSZ];
-	memset(argv, 0x00, sizeof(argv));
+	memset(tokens, 0x00, sizeof(tokens));
 	memset(tmp_str, 0x00, sizeof(tmp_str));
 
 	strcpy(tmp_str, request_str);
 	ret = parse_parameter_value(tmp_str, SPPWK_MAX_PARAMS,
-			&argc, argv);
+			&nof_tokens, tokens);
 	if (ret < SPPWK_RET_OK) {
 		RTE_LOG(ERR, PCAP_PARSER, "Parameter number over limit."
 				"request_str=%s\n", request_str);
 		return set_parse_error(wk_err_msg,
 				SPPWK_PARSE_WRONG_FORMAT, NULL);
 	}
-	RTE_LOG(DEBUG, PCAP_PARSER, "Decode array. num=%d\n", argc);
+	RTE_LOG(DEBUG, PCAP_PARSER, "Decode array. num=%d\n", nof_tokens);
 
-	for (i = 0; command_list_pcap[i].name[0] != '\0'; i++) {
-		list = &command_list_pcap[i];
-		if (strcmp(argv[0], list->name) != 0)
+	for (i = 0; pcap_cmd_attrs[i].cmd_name[0] != '\0'; i++) {
+		cmd_attr = &pcap_cmd_attrs[i];
+		if (strcmp(tokens[0], cmd_attr->cmd_name) != 0)
 			continue;
 
-		if (unlikely(argc < list->param_min) ||
-				unlikely(list->param_max < argc)) {
-			command_name_check = 1;
+		if (unlikely(nof_tokens < cmd_attr->nof_params_min) ||
+			unlikely(cmd_attr->nof_params_max < nof_tokens)) {
+			is_invalid_cmd = 1;
 			continue;
 		}
 
-		request->cmd_attrs[0].type = command_list_pcap[i].type;
-		if (list->func != NULL)
-			return (*list->func)(request, argc, argv, wk_err_msg,
-							list->param_max);
+		request->cmd_attrs[0].type = pcap_cmd_attrs[i].type;
+		if (cmd_attr->func != NULL)
+			return (*cmd_attr->func)(
+					request, nof_tokens, tokens, wk_err_msg,
+					cmd_attr->nof_params_max);
 
 		return SPPWK_RET_OK;
 	}
 
-	if (command_name_check != 0) {
+	if (is_invalid_cmd != 0) {
 		RTE_LOG(ERR, PCAP_PARSER, "Parameter number out of range."
 				"request_str=%s\n", request_str);
 		return set_parse_error(wk_err_msg,
@@ -136,13 +143,13 @@ parse_command_in_list(struct spp_command_request *request,
 
 	RTE_LOG(ERR, PCAP_PARSER,
 			"Unknown command. command=%s, request_str=%s\n",
-			argv[0], request_str);
-	return set_string_value_parse_error(wk_err_msg, argv[0], "command");
+			tokens[0], request_str);
+	return set_string_value_parse_error(wk_err_msg, tokens[0], "command");
 }
 
-/* parse request from no-null-terminated string */
+/* Parse request of non null terminated string. */
 int
-spp_command_parse_request(
+sppwk_parse_req(
 		struct spp_command_request *request,
 		const char *request_str, size_t request_str_len,
 		struct sppwk_parse_err_msg *wk_err_msg)
@@ -152,7 +159,7 @@ spp_command_parse_request(
 
 	/* parse request */
 	request->num_command = 1;
-	ret = parse_command_in_list(request, request_str, wk_err_msg);
+	ret = parse_pcap_cmd(request, request_str, wk_err_msg);
 	if (unlikely(ret != SPPWK_RET_OK)) {
 		RTE_LOG(ERR, PCAP_PARSER,
 				"Cannot parse command request. "
diff --git a/src/pcap/cmd_parser.h b/src/pcap/cmd_parser.h
index 706144d..9a91216 100644
--- a/src/pcap/cmd_parser.h
+++ b/src/pcap/cmd_parser.h
@@ -93,7 +93,7 @@ struct sppwk_parse_err_msg {
  * @retval SPPWK_RET_OK succeeded.
  * @retval !0 failed.
  */
-int spp_command_parse_request(struct spp_command_request *request,
+int sppwk_parse_req(struct spp_command_request *request,
 		const char *request_str, size_t request_str_len,
 		struct sppwk_parse_err_msg *error);
 
diff --git a/src/pcap/cmd_runner.c b/src/pcap/cmd_runner.c
index 9d63bde..3080242 100644
--- a/src/pcap/cmd_runner.c
+++ b/src/pcap/cmd_runner.c
@@ -868,8 +868,8 @@ process_request(int *sock, const char *request_str, size_t request_str_len)
 			(int)request_str_len, request_str);
 
 	/* parse request message */
-	ret = spp_command_parse_request(
-			&request, request_str, request_str_len, &parse_error);
+	ret = sppwk_parse_req(&request, request_str, request_str_len,
+			&parse_error);
 	if (unlikely(ret != SPPWK_RET_OK)) {
 		/* send error response */
 		set_parse_error_to_results(command_results, &request,
diff --git a/src/shared/secondary/spp_worker_th/cmd_parser.c b/src/shared/secondary/spp_worker_th/cmd_parser.c
index 621bea4..9fc88dd 100644
--- a/src/shared/secondary/spp_worker_th/cmd_parser.c
+++ b/src/shared/secondary/spp_worker_th/cmd_parser.c
@@ -1016,8 +1016,8 @@ parse_cmd_port(struct sppwk_cmd_req *request, int argc, char *argv[],
 }
 
 /**
- * Attributes of commands for parsing. The last member of function pointer
- * is the operator function for the command.
+ * A set of attributes of commands for parsing. The last member of function
+ * pointer is the operator function for the command.
  */
 struct cmd_parse_attrs {
 	const char *cmd_name;
-- 
2.17.1


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

* [spp] [PATCH 05/10] spp_pcap: refactor func for splitting cmd tokens
  2019-05-31  8:52 [spp] [PATCH 00/10] Refactor cmd parser in spp_pcap ogawa.yasufumi
                   ` (3 preceding siblings ...)
  2019-05-31  8:52 ` [spp] [PATCH 04/10] spp_pcap: revise parser functions ogawa.yasufumi
@ 2019-05-31  8:52 ` ogawa.yasufumi
  2019-05-31  8:52 ` [spp] [PATCH 06/10] spp_pcap: revise log msgs in parser func ogawa.yasufumi
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: ogawa.yasufumi @ 2019-05-31  8:52 UTC (permalink / raw)
  To: spp, ferruh.yigit, ogawa.yasufumi

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

Refactor function for splitting pcap command into tokens in which the
name of vars are misunderstandable.

Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
 src/pcap/cmd_parser.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/src/pcap/cmd_parser.c b/src/pcap/cmd_parser.c
index 577e952..cf1f2b9 100644
--- a/src/pcap/cmd_parser.c
+++ b/src/pcap/cmd_parser.c
@@ -36,22 +36,29 @@ set_string_value_parse_error(struct sppwk_parse_err_msg *wk_err_msg,
 	return set_parse_error(wk_err_msg, SPPWK_PARSE_INVALID_VALUE, err_msg);
 }
 
-/* Split command line parameter with spaces */
+/* Split tokens of pcap command with delimiter. */
+/* TODO(yasufum) consider this func can be moved to shared. */
 static int
-parse_parameter_value(char *string, int max, int *nof_tokens, char *tokens[])
+split_cmd_tokens(char *tokens_str, int max_tokens, int *nof_tokens,
+		char *tokens[])
 {
-	int cnt = 0;
 	const char *delim = " ";
 	char *token = NULL;
-	char *saveptr = NULL;
+	char *t_ptr = NULL;
+	int cnt = 0;
 
-	token = strtok_r(string, delim, &saveptr);
+	token = strtok_r(tokens_str, delim, &t_ptr);
 	while (token != NULL) {
-		if (cnt >= max)
+		if (cnt >= max_tokens) {
+			RTE_LOG(ERR, PCAP_PARSER,
+					"Invalid num of tokens in %s."
+					"It should be less than %d.\n",
+					tokens_str, max_tokens);
 			return SPPWK_RET_NG;
+		}
 		tokens[cnt] = token;
 		cnt++;
-		token = strtok_r(NULL, delim, &saveptr);
+		token = strtok_r(NULL, delim, &t_ptr);
 	}
 	*nof_tokens = cnt;
 
@@ -104,7 +111,7 @@ parse_pcap_cmd(struct spp_command_request *request,
 	memset(tmp_str, 0x00, sizeof(tmp_str));
 
 	strcpy(tmp_str, request_str);
-	ret = parse_parameter_value(tmp_str, SPPWK_MAX_PARAMS,
+	ret = split_cmd_tokens(tmp_str, SPPWK_MAX_PARAMS,
 			&nof_tokens, tokens);
 	if (ret < SPPWK_RET_OK) {
 		RTE_LOG(ERR, PCAP_PARSER, "Parameter number over limit."
-- 
2.17.1


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

* [spp] [PATCH 06/10] spp_pcap: revise log msgs in parser func
  2019-05-31  8:52 [spp] [PATCH 00/10] Refactor cmd parser in spp_pcap ogawa.yasufumi
                   ` (4 preceding siblings ...)
  2019-05-31  8:52 ` [spp] [PATCH 05/10] spp_pcap: refactor func for splitting cmd tokens ogawa.yasufumi
@ 2019-05-31  8:52 ` ogawa.yasufumi
  2019-05-31  8:52 ` [spp] [PATCH 07/10] spp_pcap: remove unused string list ogawa.yasufumi
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: ogawa.yasufumi @ 2019-05-31  8:52 UTC (permalink / raw)
  To: spp, ferruh.yigit, ogawa.yasufumi

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

This update is to revise redundant or no meaning log messages in
function parse_pcap_cmd().

Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
 src/pcap/cmd_parser.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/src/pcap/cmd_parser.c b/src/pcap/cmd_parser.c
index cf1f2b9..c6f3387 100644
--- a/src/pcap/cmd_parser.c
+++ b/src/pcap/cmd_parser.c
@@ -97,7 +97,7 @@ static struct pcap_cmd_parse_attr pcap_cmd_attrs[] = {
 /* Parse command requested from spp-ctl. */
 static int
 parse_pcap_cmd(struct spp_command_request *request,
-		const char *request_str,
+		const char *cmd_str,
 		struct sppwk_parse_err_msg *wk_err_msg)
 {
 	int is_invalid_cmd = 0;
@@ -110,16 +110,18 @@ parse_pcap_cmd(struct spp_command_request *request,
 	memset(tokens, 0x00, sizeof(tokens));
 	memset(tmp_str, 0x00, sizeof(tmp_str));
 
-	strcpy(tmp_str, request_str);
+	strcpy(tmp_str, cmd_str);
 	ret = split_cmd_tokens(tmp_str, SPPWK_MAX_PARAMS,
 			&nof_tokens, tokens);
 	if (ret < SPPWK_RET_OK) {
-		RTE_LOG(ERR, PCAP_PARSER, "Parameter number over limit."
-				"request_str=%s\n", request_str);
+		RTE_LOG(ERR, PCAP_PARSER, "Invalid cmd '%s', "
+				"num of tokens is over SPPWK_MAX_PARAMS.\n",
+				cmd_str);
 		return set_parse_error(wk_err_msg,
 				SPPWK_PARSE_WRONG_FORMAT, NULL);
 	}
-	RTE_LOG(DEBUG, PCAP_PARSER, "Decode array. num=%d\n", nof_tokens);
+	RTE_LOG(DEBUG, PCAP_PARSER, "Parsed cmd '%s', nof token is %d\n",
+			cmd_str, nof_tokens);
 
 	for (i = 0; pcap_cmd_attrs[i].cmd_name[0] != '\0'; i++) {
 		cmd_attr = &pcap_cmd_attrs[i];
@@ -142,15 +144,14 @@ parse_pcap_cmd(struct spp_command_request *request,
 	}
 
 	if (is_invalid_cmd != 0) {
-		RTE_LOG(ERR, PCAP_PARSER, "Parameter number out of range."
-				"request_str=%s\n", request_str);
+		RTE_LOG(ERR, PCAP_PARSER, "Invalid cmd '%s', "
+				"num of params is out of range.\n", cmd_str);
 		return set_parse_error(wk_err_msg,
 				SPPWK_PARSE_WRONG_FORMAT, NULL);
 	}
 
 	RTE_LOG(ERR, PCAP_PARSER,
-			"Unknown command. command=%s, request_str=%s\n",
-			tokens[0], request_str);
+			"Unknown cmd '%s' in '%s'.\n", tokens[0], cmd_str);
 	return set_string_value_parse_error(wk_err_msg, tokens[0], "command");
 }
 
-- 
2.17.1


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

* [spp] [PATCH 07/10] spp_pcap: remove unused string list
  2019-05-31  8:52 [spp] [PATCH 00/10] Refactor cmd parser in spp_pcap ogawa.yasufumi
                   ` (5 preceding siblings ...)
  2019-05-31  8:52 ` [spp] [PATCH 06/10] spp_pcap: revise log msgs in parser func ogawa.yasufumi
@ 2019-05-31  8:52 ` ogawa.yasufumi
  2019-05-31  8:52 ` [spp] [PATCH 08/10] spp_pcap: refactor comments of spp_pcap ogawa.yasufumi
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: ogawa.yasufumi @ 2019-05-31  8:52 UTC (permalink / raw)
  To: spp, ferruh.yigit, ogawa.yasufumi

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

Remove definition of `const char *CAPTURE_THREAD_TYPE_STRINGS[]` which
is not used actually.

Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
 src/pcap/spp_pcap.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/src/pcap/spp_pcap.c b/src/pcap/spp_pcap.c
index 84b42e4..7c66f16 100644
--- a/src/pcap/spp_pcap.c
+++ b/src/pcap/spp_pcap.c
@@ -67,14 +67,6 @@ enum comp_file_generate_mode {
 	CLOSE_MODE   /* close mode which is used when capture is stopped */
 };
 
-/* capture thread name string  */
-const char *CAPTURE_THREAD_TYPE_STRINGS[] = {
-	"unuse",
-	"receive",
-	"write",
-	/* termination */ "",
-};
-
 /* lz4 preferences */
 static const LZ4F_preferences_t g_kprefs = {
 	{
-- 
2.17.1


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

* [spp] [PATCH 08/10] spp_pcap: refactor comments of spp_pcap
  2019-05-31  8:52 [spp] [PATCH 00/10] Refactor cmd parser in spp_pcap ogawa.yasufumi
                   ` (6 preceding siblings ...)
  2019-05-31  8:52 ` [spp] [PATCH 07/10] spp_pcap: remove unused string list ogawa.yasufumi
@ 2019-05-31  8:52 ` ogawa.yasufumi
  2019-05-31  8:52 ` [spp] [PATCH 09/10] spp_pcap: rename spp_get_core_status ogawa.yasufumi
  2019-05-31  8:52 ` [spp] [PATCH 10/10] spp_pcap: revise name of vars for parser error ogawa.yasufumi
  9 siblings, 0 replies; 11+ messages in thread
From: ogawa.yasufumi @ 2019-05-31  8:52 UTC (permalink / raw)
  To: spp, ferruh.yigit, ogawa.yasufumi

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

This update is to revise comments in `spp_pcap.c` and `spp_pcap.h` for
refactoring.

Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
 src/pcap/spp_pcap.c | 66 +++++++++++++++++++++------------------------
 src/pcap/spp_pcap.h | 12 +--------
 2 files changed, 31 insertions(+), 47 deletions(-)

diff --git a/src/pcap/spp_pcap.c b/src/pcap/spp_pcap.c
index 7c66f16..a7f95e9 100644
--- a/src/pcap/spp_pcap.c
+++ b/src/pcap/spp_pcap.c
@@ -56,15 +56,9 @@ enum worker_thread_type {
 
 /* compress file generate mode */
 enum comp_file_generate_mode {
-	INIT_MODE,   /**
-		       * initial generation mode which is used
-		       * when capture is started
-		       */
-	UPDATE_MODE, /**
-		       * update generation mode which is used
-		       * when capture file reached max size
-		       */
-	CLOSE_MODE   /* close mode which is used when capture is stopped */
+	INIT_MODE,  /** Initial gen mode while capture is starting. */
+	UPDATE_MODE,  /** Update gen mode when cap file size reached max. */
+	CLOSE_MODE   /* Close mode used when capture is stopped. */
 };
 
 /* lz4 preferences */
@@ -74,45 +68,45 @@ static const LZ4F_preferences_t g_kprefs = {
 		LZ4F_blockLinked,
 		LZ4F_noContentChecksum,
 		LZ4F_frame,
-		0,                   /* unknown content size */
-		{ 0, 0},             /* reserved, must be set to 0 */
+		0,  /* unknown content size */
+		{ 0, 0},  /* reserved, must be set to 0 */
 	},
-	0,                           /* compression level; 0 == default */
-	0,                           /* autoflush */
-	{ 0, 0, 0, 0},               /* reserved, must be set to 0 */
+	0,  /* compression level; 0 == default */
+	0,  /* autoflush */
+	{ 0, 0, 0, 0},   /* reserved, must be set to 0 */
 };
 
 /* pcap file header */
 struct __attribute__((__packed__)) pcap_header {
 	uint32_t magic_number;  /* magic number */
-	uint16_t version_major; /* major version number */
-	uint16_t version_minor; /* minor version number */
-	int32_t  thiszone;      /* GMT to local correction */
-	uint32_t sigfigs;       /* accuracy of timestamps */
-	uint32_t snaplen;       /* max length of captured packets, in octets */
-	uint32_t network;       /* data link type */
+	uint16_t major_ver;  /* major version */
+	uint16_t minor_ver;  /* minor version */
+	int32_t thiszone;  /* GMT to local correction */
+	uint32_t sigfigs;  /* accuracy of timestamps */
+	uint32_t snaplen;  /* max length of captured packets, in octets */
+	uint32_t network;  /* data link type */
 };
 
 /* pcap packet header */
 struct pcap_packet_header {
-	uint32_t ts_sec;        /* time stamp seconds */
-	uint32_t ts_usec;       /* time stamp micro seconds */
-	uint32_t write_len;     /* write length */
-	uint32_t packet_len;    /* packet length */
+	uint32_t ts_sec;   /* time stamp seconds */
+	uint32_t ts_usec;  /* time stamp micro seconds */
+	uint32_t write_len;   /* write length */
+	uint32_t packet_len;  /* packet length */
 };
 
 /* Option for pcap. */
 struct pcap_option {
-	struct timespec start_time; /* start time */
-	uint64_t fsize_limit;        /* file size limit */
-	char compress_file_path[PCAP_FPATH_STRLEN]; /* file path */
-	char compress_file_date[PCAP_FDATE_STRLEN]; /* file name date */
+	struct timespec start_time;  /* start time */
+	uint64_t fsize_limit;  /* file size limit */
+	char compress_file_path[PCAP_FPATH_STRLEN];  /* file path */
+	char compress_file_date[PCAP_FDATE_STRLEN];  /* file name date */
 	struct sppwk_port_info port_cap;  /* capture port */
-	struct rte_ring *cap_ring;      /* RTE ring structure */
+	struct rte_ring *cap_ring;  /* RTE ring structure */
 };
 
 /**
- * pcap management info which stores attributes
+ * pcap management info which stores attributes.
  * (e.g. worker thread type, file number, pointer to writing file etc) per core
  */
 struct pcap_mng_info {
@@ -130,14 +124,14 @@ struct pcap_mng_info {
 
 /* Pcap status info. */
 struct pcap_status_info {
-	int thread_cnt;		/* thread count */
-	int start_up_cnt;	/* thread start up count */
+	int thread_cnt;  /* thread count */
+	int start_up_cnt;  /* thread start up count */
 };
 
-/* Logical core ID for main thread */
+/* Lcore ID of main thread. */
 static unsigned int g_main_lcore_id = 0xffffffff;
 
-/* Execution parameter of spp_pcap */
+/* Arguments for spp_pcap process. */
 static struct startup_param g_startup_param;
 
 /* Interface management information */
@@ -662,8 +656,8 @@ static int file_compression_operation(struct pcap_mng_info *info,
 
 	/* init the common pcap header */
 	pcap_h.magic_number = TCPDUMP_MAGIC;
-	pcap_h.version_major = PCAP_VERSION_MAJOR;
-	pcap_h.version_minor = PCAP_VERSION_MINOR;
+	pcap_h.major_ver = PCAP_VERSION_MAJOR;
+	pcap_h.minor_ver = PCAP_VERSION_MINOR;
 	pcap_h.thiszone = 0;
 	pcap_h.sigfigs = 0;
 	pcap_h.snaplen = PCAP_SNAPLEN_MAX;
diff --git a/src/pcap/spp_pcap.h b/src/pcap/spp_pcap.h
index bf2eec1..b23fbbc 100644
--- a/src/pcap/spp_pcap.h
+++ b/src/pcap/spp_pcap.h
@@ -7,20 +7,10 @@
 
 #include "cmd_utils.h"
 
-/**
- * @file
- * SPP_PCAP main
- *
- * Main function of spp_pcap.
- * This provides the function for initializing and starting the threads.
- *
- */
-
 /**
  * Pcap get core status
  *
- * @param lcore_id
- *  The logical core ID for forwarder and merger.
+ * @param lcore_id The logical core ID for forwarder and merger.
  * @param params
  *  The pointer to struct spp_iterate_core_params.@n
  *  Detailed data of pcap status.
-- 
2.17.1


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

* [spp] [PATCH 09/10] spp_pcap: rename spp_get_core_status
  2019-05-31  8:52 [spp] [PATCH 00/10] Refactor cmd parser in spp_pcap ogawa.yasufumi
                   ` (7 preceding siblings ...)
  2019-05-31  8:52 ` [spp] [PATCH 08/10] spp_pcap: refactor comments of spp_pcap ogawa.yasufumi
@ 2019-05-31  8:52 ` ogawa.yasufumi
  2019-05-31  8:52 ` [spp] [PATCH 10/10] spp_pcap: revise name of vars for parser error ogawa.yasufumi
  9 siblings, 0 replies; 11+ messages in thread
From: ogawa.yasufumi @ 2019-05-31  8:52 UTC (permalink / raw)
  To: spp, ferruh.yigit, ogawa.yasufumi

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

Revise misleading function names in spp_pcap, spp_get_core_status() and
spp_pcap_get_core_status(). First one is to get lcore status actually,
but second one is to thread info on the lcore and the name is not
appropriate. This update is to rename first one to be more specific.
The second one is to be revise in the next patch.

Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
 src/pcap/cmd_runner.c |  2 +-
 src/pcap/cmd_utils.c  |  4 ++--
 src/pcap/cmd_utils.h  | 28 +++++++++++-----------------
 src/pcap/spp_pcap.c   | 21 ++++++++++-----------
 4 files changed, 24 insertions(+), 31 deletions(-)

diff --git a/src/pcap/cmd_runner.c b/src/pcap/cmd_runner.c
index 3080242..f94deea 100644
--- a/src/pcap/cmd_runner.c
+++ b/src/pcap/cmd_runner.c
@@ -79,7 +79,7 @@ spp_iterate_core_info(struct spp_iterate_core_params *params)
 	int lcore_id;
 
 	RTE_LCORE_FOREACH_SLAVE(lcore_id) {
-		if (spp_get_core_status(lcore_id) == SPP_CORE_UNUSE)
+		if (sppwk_get_lcore_status(lcore_id) == SPP_CORE_UNUSE)
 			continue;
 
 		ret = spp_pcap_get_core_status(lcore_id, params);
diff --git a/src/pcap/cmd_utils.c b/src/pcap/cmd_utils.c
index 084b3ca..726ad9e 100644
--- a/src/pcap/cmd_utils.c
+++ b/src/pcap/cmd_utils.c
@@ -63,9 +63,9 @@ add_ring_pmd(int ring_id)
 	return ring_port_id;
 }
 
-/* Get core status */
+/* Get status of lcore of given ID from global management info. */
 enum sppwk_lcore_status
-spp_get_core_status(unsigned int lcore_id)
+sppwk_get_lcore_status(unsigned int lcore_id)
 {
 	return (g_mng_data_addr.p_core_info + lcore_id)->status;
 }
diff --git a/src/pcap/cmd_utils.h b/src/pcap/cmd_utils.h
index 8487c23..5528e00 100644
--- a/src/pcap/cmd_utils.h
+++ b/src/pcap/cmd_utils.h
@@ -186,6 +186,7 @@ struct core_mng_info {
 	volatile enum sppwk_lcore_status status;
 };
 
+/* TODO(yasufum) refactor name of func and vars, and comments. */
 struct spp_iterate_core_params;
 /**
  * definition of iterated core element procedure function
@@ -203,6 +204,7 @@ typedef int (*spp_iterate_core_element_proc)(
 		const int num_tx,
 		const struct sppwk_port_idx *tx_ports);
 
+/* TODO(yasufum) refactor name of func and vars, and comments. */
 /**
  * iterate core table parameters which is
  * used when listing core table content
@@ -228,36 +230,28 @@ struct spp_iterate_core_params {
 int add_ring_pmd(int ring_id);
 
 /**
- * Get core status
+ * Get status of lcore of given ID from global management info.
  *
- * @param lcore_id
- *  Logical core ID.
- *
- * @return
- *  Status of specified logical core.
+ * @param[in] lcore_id Logical core ID.
+ * @return Status of specified logical core.
  */
-enum sppwk_lcore_status spp_get_core_status(unsigned int lcore_id);
+enum sppwk_lcore_status sppwk_get_lcore_status(unsigned int lcore_id);
 
 /**
  * Run check_core_status() for SPP_CORE_STATUS_CHECK_MAX times with
  * interval time (1sec)
  *
- * @param status
- *  wait check status.
- *
- * @retval 0  succeeded.
- * @retval -1 failed.
+ * @param status Wait check status.
+ * @retval 0  If succeeded.
+ * @retval -1 If failed.
  */
 int check_core_status_wait(enum sppwk_lcore_status status);
 
 /**
  * Set core status
  *
- * @param lcore_id
- *  Logical core ID.
- * @param status
- *  set status.
- *
+ * @param lcore_id Logical core ID.
+ * @param status Set status.
  */
 void set_core_status(unsigned int lcore_id, enum sppwk_lcore_status status);
 
diff --git a/src/pcap/spp_pcap.c b/src/pcap/spp_pcap.c
index a7f95e9..dec3f37 100644
--- a/src/pcap/spp_pcap.c
+++ b/src/pcap/spp_pcap.c
@@ -322,8 +322,6 @@ parse_args(int argc, char *argv[])
 	g_pcap_option.fsize_limit = DEFAULT_FILE_LIMIT;
 
 	/* Check options of application */
-	optind = 0;
-	opterr = 0;
 	while ((opt = getopt_long(argc, argvopt, "c:s:", lgopts,
 			&option_index)) != EOF) {
 		switch (opt) {
@@ -396,18 +394,22 @@ parse_args(int argc, char *argv[])
 	return SPPWK_RET_OK;
 }
 
-/* Pcap get core status */
+/* TODO(yasufum) refactor name of func and vars, and comments. */
+/**
+ * Get each of attrs such as name, type or nof ports of a thread on a lcore.
+ * MEMO: This func is not for getting core status, but thread info actually.
+ */
 int
 spp_pcap_get_core_status(
 		unsigned int lcore_id,
 		struct spp_iterate_core_params *params)
 {
-	int ret = SPPWK_RET_NG;
 	char role_type[8];
 	struct pcap_mng_info *info = &g_pcap_info[lcore_id];
 	char name[PCAP_FPATH_STRLEN + PCAP_FDATE_STRLEN];
 	struct sppwk_port_idx rx_ports[1];
 	int rx_num = 0;
+	int res;
 
 	RTE_LOG(DEBUG, SPP_PCAP, "status core[%d]\n", lcore_id);
 	if (info->type == PCAP_RECEIVE) {
@@ -426,13 +428,10 @@ spp_pcap_get_core_status(
 		strcpy(role_type, "write");
 	}
 
-
-	/* Set the information with the function specified by the command. */
-	ret = (*params->element_proc)(
-		params, lcore_id,
-		name, role_type,
+	/* Set information with specified by the command. */
+	res = (*params->element_proc)(params, lcore_id, name, role_type,
 		rx_num, rx_ports, 0, NULL);
-	if (unlikely(ret != 0))
+	if (unlikely(res != 0))
 		return SPPWK_RET_NG;
 
 	return SPPWK_RET_OK;
@@ -915,7 +914,7 @@ slave_main(void *arg __attribute__ ((unused)))
 	set_core_status(lcore_id, SPP_CORE_IDLE);
 
 	while (1) {
-		if (spp_get_core_status(lcore_id) == SPP_CORE_STOP_REQUEST) {
+		if (sppwk_get_lcore_status(lcore_id) == SPP_CORE_STOP_REQUEST) {
 			if (pcap_info->status == SPP_CAPTURE_IDLE)
 				break;
 			if (pcap_info->type == PCAP_RECEIVE)
-- 
2.17.1


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

* [spp] [PATCH 10/10] spp_pcap: revise name of vars for parser error
  2019-05-31  8:52 [spp] [PATCH 00/10] Refactor cmd parser in spp_pcap ogawa.yasufumi
                   ` (8 preceding siblings ...)
  2019-05-31  8:52 ` [spp] [PATCH 09/10] spp_pcap: rename spp_get_core_status ogawa.yasufumi
@ 2019-05-31  8:52 ` ogawa.yasufumi
  9 siblings, 0 replies; 11+ messages in thread
From: ogawa.yasufumi @ 2019-05-31  8:52 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 vars for handling parse error to be
more concisely.

Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
 src/pcap/cmd_runner.c                         | 57 ++++++++++---------
 .../secondary/spp_worker_th/cmd_runner.c      |  6 +-
 2 files changed, 31 insertions(+), 32 deletions(-)

diff --git a/src/pcap/cmd_runner.c b/src/pcap/cmd_runner.c
index f94deea..d623d8c 100644
--- a/src/pcap/cmd_runner.c
+++ b/src/pcap/cmd_runner.c
@@ -16,13 +16,14 @@
 #define RTE_LOGTYPE_PCAP_RUNNER RTE_LOGTYPE_USER2
 
 /* request message initial size */
-#define CMD_RES_ERR_MSG_SIZE  128
+#define CMD_ERR_MSG_SIZE  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) confirm why using "" for the alternative of comma? */
 #define JSON_APPEND_COMMA(flg)    ((flg)?", ":"")
 #define JSON_APPEND_VALUE(format) "%s\"%s\": "format
 #define JSON_APPEND_ARRAY         "%s\"%s\": [ %s ]"
@@ -40,8 +41,8 @@ enum cmd_res_code {
 /* TODO(yasufum) merge it to the same definition in shared/.../cmd_runner.c */
 struct cmd_result {
 	int code;  /* Response code */
-	char msg[SPPWK_NAME_BUFSZ];  /* Response msg in short. */
-	char err_msg[CMD_RES_ERR_MSG_SIZE];  /* Used only if cmd is failed. */
+	char result[SPPWK_NAME_BUFSZ];  /* Response msg in short. */
+	char err_msg[CMD_ERR_MSG_SIZE];  /* Used only if cmd is failed. */
 };
 
 /* command response list control structure */
@@ -290,47 +291,47 @@ parse_error_message(
 
 /* set the command result */
 static inline void
-set_command_results(struct cmd_result *result,
+set_command_results(struct cmd_result *cmd_res,
 		int code, const char *error_messege)
 {
-	result->code = code;
+	cmd_res->code = code;
 	switch (code) {
 	case CMD_SUCCESS:
-		strcpy(result->msg, "success");
-		memset(result->err_msg, 0x00, CMD_RES_ERR_MSG_SIZE);
+		strcpy(cmd_res->result, "success");
+		memset(cmd_res->err_msg, 0x00, CMD_ERR_MSG_SIZE);
 		break;
 	case CMD_FAILURE:
-		strcpy(result->msg, "error");
-		strcpy(result->err_msg, error_messege);
+		strcpy(cmd_res->result, "error");
+		strcpy(cmd_res->err_msg, error_messege);
 		break;
 	case CMD_INVALID: /* FALLTHROUGH */
 	default:
-		strcpy(result->msg, "invalid");
-		memset(result->err_msg, 0x00, CMD_RES_ERR_MSG_SIZE);
+		strcpy(cmd_res->result, "invalid");
+		memset(cmd_res->err_msg, 0x00, CMD_ERR_MSG_SIZE);
 		break;
 	}
 }
 
 /* set parse error to command result */
 static void
-set_parse_error_to_results(struct cmd_result *results,
+set_parse_error_to_results(struct cmd_result *cmd_results,
 		const struct spp_command_request *request,
 		const struct sppwk_parse_err_msg *wk_err_msg)
 {
 	int i;
 	const char *tmp_buff;
-	char err_msg[CMD_RES_ERR_MSG_SIZE];
+	char err_msg[CMD_ERR_MSG_SIZE];
 
 	for (i = 0; i < request->num_command; i++) {
 		if (wk_err_msg->code == 0)
-			set_command_results(&results[i], CMD_SUCCESS, "");
+			set_command_results(&cmd_results[i], CMD_SUCCESS, "");
 		else
-			set_command_results(&results[i], CMD_INVALID, "");
+			set_command_results(&cmd_results[i], CMD_INVALID, "");
 	}
 
 	if (wk_err_msg->code != 0) {
 		tmp_buff = parse_error_message(wk_err_msg, err_msg);
-		set_command_results(&results[request->num_valid_command],
+		set_command_results(&cmd_results[request->num_valid_command],
 				CMD_FAILURE, tmp_buff);
 	}
 }
@@ -339,8 +340,8 @@ set_parse_error_to_results(struct cmd_result *results,
 static int
 append_result_value(const char *name, char **output, void *tmp)
 {
-	const struct cmd_result *result = tmp;
-	return append_json_str_value(name, output, result->msg);
+	const struct cmd_result *res = tmp;
+	return append_json_str_value(name, output, res->result);
 }
 
 /* append error details for JSON format */
@@ -857,11 +858,11 @@ process_request(int *sock, const char *request_str, size_t request_str_len)
 
 	struct spp_command_request request;
 	struct sppwk_parse_err_msg parse_error;
-	struct cmd_result command_results[SPPWK_MAX_CMDS];
+	struct cmd_result cmd_results[SPPWK_MAX_CMDS];
 
 	memset(&request, 0, sizeof(struct spp_command_request));
 	memset(&parse_error, 0, sizeof(struct sppwk_parse_err_msg));
-	memset(command_results, 0, sizeof(command_results));
+	memset(cmd_results, 0, sizeof(cmd_results));
 
 	RTE_LOG(DEBUG, PCAP_RUNNER, "Start command request processing. "
 			"request_str=\n%.*s\n",
@@ -872,9 +873,9 @@ process_request(int *sock, const char *request_str, size_t request_str_len)
 			&parse_error);
 	if (unlikely(ret != SPPWK_RET_OK)) {
 		/* send error response */
-		set_parse_error_to_results(command_results, &request,
+		set_parse_error_to_results(cmd_results, &request,
 				&parse_error);
-		send_parse_error_response(sock, &request, command_results);
+		send_parse_error_response(sock, &request, cmd_results);
 		RTE_LOG(DEBUG, PCAP_RUNNER,
 				"End command request processing.\n");
 		return SPPWK_RET_OK;
@@ -888,32 +889,32 @@ 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.cmd_attrs + i);
 		if (unlikely(ret != SPPWK_RET_OK)) {
-			set_command_results(&command_results[i], CMD_FAILURE,
+			set_command_results(&cmd_results[i], CMD_FAILURE,
 					"error occur");
 
 			/* not execute remaining commands */
 			for (++i; i < request.num_command ; ++i)
-				set_command_results(&command_results[i],
+				set_command_results(&cmd_results[i],
 					CMD_INVALID, "");
 
 			break;
 		}
 
-		set_command_results(&command_results[i], CMD_SUCCESS, "");
+		set_command_results(&cmd_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, "");
-		send_command_result_response(sock, &request, command_results);
+		set_command_results(&cmd_results[0], CMD_SUCCESS, "");
+		send_command_result_response(sock, &request, cmd_results);
 		RTE_LOG(INFO, PCAP_RUNNER,
 				"Terminate process for exit.\n");
 		return SPPWK_RET_NG;
 	}
 
 	/* send response */
-	send_command_result_response(sock, &request, command_results);
+	send_command_result_response(sock, &request, cmd_results);
 
 	RTE_LOG(DEBUG, PCAP_RUNNER, "End command request processing.\n");
 
diff --git a/src/shared/secondary/spp_worker_th/cmd_runner.c b/src/shared/secondary/spp_worker_th/cmd_runner.c
index ba3fdc6..ef3b2d9 100644
--- a/src/shared/secondary/spp_worker_th/cmd_runner.c
+++ b/src/shared/secondary/spp_worker_th/cmd_runner.c
@@ -25,10 +25,8 @@
 #define CMD_REQ_BUF_INIT_SIZE 2048
 #define CMD_RES_BUF_INIT_SIZE 2048
 
-/* 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_COMMA(flg)    ((flg)?", ":"")
 #define JSON_APPEND_VALUE(format) "%s\"%s\": "format
 #define JSON_APPEND_ARRAY         "%s\"%s\": [ %s ]"
 #define JSON_APPEND_BLOCK         "%s\"%s\": { %s }"
@@ -610,7 +608,7 @@ sppwk_get_ethdev_port_id(enum port_type iface_type, int iface_no)
 static int
 append_json_comma(char **output)
 {
-	*output = spp_strbuf_append(*output, JSON_COMMA, strlen(JSON_COMMA));
+	*output = spp_strbuf_append(*output, ", ", strlen(", "));
 	if (unlikely(*output == NULL)) {
 		RTE_LOG(ERR, WK_CMD_RUNNER,
 				"JSON's comma failed to add.\n");
-- 
2.17.1


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

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

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-31  8:52 [spp] [PATCH 00/10] Refactor cmd parser in spp_pcap ogawa.yasufumi
2019-05-31  8:52 ` [spp] [PATCH 01/10] spp_pcap: rename enum spp_command_type ogawa.yasufumi
2019-05-31  8:52 ` [spp] [PATCH 02/10] spp_pcap: rename struct spp_command ogawa.yasufumi
2019-05-31  8:52 ` [spp] [PATCH 03/10] spp_pcap: revise command response codes ogawa.yasufumi
2019-05-31  8:52 ` [spp] [PATCH 04/10] spp_pcap: revise parser functions ogawa.yasufumi
2019-05-31  8:52 ` [spp] [PATCH 05/10] spp_pcap: refactor func for splitting cmd tokens ogawa.yasufumi
2019-05-31  8:52 ` [spp] [PATCH 06/10] spp_pcap: revise log msgs in parser func ogawa.yasufumi
2019-05-31  8:52 ` [spp] [PATCH 07/10] spp_pcap: remove unused string list ogawa.yasufumi
2019-05-31  8:52 ` [spp] [PATCH 08/10] spp_pcap: refactor comments of spp_pcap ogawa.yasufumi
2019-05-31  8:52 ` [spp] [PATCH 09/10] spp_pcap: rename spp_get_core_status ogawa.yasufumi
2019-05-31  8:52 ` [spp] [PATCH 10/10] spp_pcap: revise name of vars for parser error 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).