Soft Patch Panel
 help / color / mirror / Atom feed
* [spp] [PATCH 00/13] Move JSON utils from libs for running cmds
@ 2019-06-24  4:36 yasufum.o
  2019-06-24  4:36 ` [spp] [PATCH 01/13] shared/sec: rename ops for setup cmd response yasufum.o
                   ` (12 more replies)
  0 siblings, 13 replies; 14+ messages in thread
From: yasufum.o @ 2019-06-24  4:36 UTC (permalink / raw)
  To: spp, ferruh.yigit, yasufum.o

From: Yasufumi Ogawa <yasufum.o@gmail.com>

There are many JSON utils in shared lib for running commands, and
hard to maintain because several features are defined in a huge file.
This series of patches is to separate JSON utils from other functions
and fix complex dependencies among them.

Yasufumi Ogawa (13):
  shared/sec: rename ops for setup cmd response
  shared/sec: rename functions for spp_mirror
  shared/sec: move principle JSON formatter funcs
  shared/sec: change order of args of JSON fmtters
  shared/sec: move JSON formatter to shard/secondary
  shared/sec: revise including headers
  shared/sec: move JSON formatters from cmd_runner
  shared/sec: move rest of JSON formatters
  shared/sec: move lcore funcs in response_info_list
  shared/sec: move ope cli-id in response_info_list
  shared/sec: move rest of ops in response_info_list
  shared/sec: remove local funcs from header
  shared/sec: refactor comments for JSON formatter

 src/mirror/Makefile                           |  11 +-
 src/mirror/spp_mirror.c                       |  75 +-
 src/mirror/spp_mirror.h                       |  26 -
 src/pcap/Makefile                             |   3 +-
 src/pcap/cmd_runner.c                         |   2 +-
 src/shared/secondary/json_helper.c            | 136 +++
 src/shared/secondary/json_helper.h            | 101 ++
 src/shared/secondary/return_codes.h           |  13 +
 .../secondary/spp_worker_th/cmd_parser.c      |   1 +
 .../spp_worker_th/cmd_res_formatter.c         | 831 +++++++++++++++
 .../spp_worker_th/cmd_res_formatter.h         |  83 ++
 .../secondary/spp_worker_th/cmd_runner.c      | 952 +-----------------
 .../secondary/spp_worker_th/cmd_utils.c       |   6 +-
 .../secondary/spp_worker_th/cmd_utils.h       |   8 +-
 .../secondary/spp_worker_th/conn_spp_ctl.c    |   3 +-
 .../secondary/spp_worker_th/mirror_deps.h     |  33 +-
 src/shared/secondary/spp_worker_th/spp_port.c |   1 +
 src/shared/secondary/spp_worker_th/vf_deps.h  |   2 +-
 .../{spp_worker_th => }/string_buffer.c       |   0
 .../{spp_worker_th => }/string_buffer.h       |   2 +
 src/vf/Makefile                               |   7 +-
 src/vf/classifier_mac.c                       |   6 +-
 src/vf/spp_forward.c                          |   5 +-
 src/vf/spp_vf.c                               |   3 +-
 24 files changed, 1260 insertions(+), 1050 deletions(-)
 delete mode 100644 src/mirror/spp_mirror.h
 create mode 100644 src/shared/secondary/json_helper.c
 create mode 100644 src/shared/secondary/json_helper.h
 create mode 100644 src/shared/secondary/return_codes.h
 create mode 100644 src/shared/secondary/spp_worker_th/cmd_res_formatter.c
 create mode 100644 src/shared/secondary/spp_worker_th/cmd_res_formatter.h
 rename src/shared/secondary/{spp_worker_th => }/string_buffer.c (100%)
 rename src/shared/secondary/{spp_worker_th => }/string_buffer.h (98%)

-- 
2.17.1


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

* [spp] [PATCH 01/13] shared/sec: rename ops for setup cmd response
  2019-06-24  4:36 [spp] [PATCH 00/13] Move JSON utils from libs for running cmds yasufum.o
@ 2019-06-24  4:36 ` yasufum.o
  2019-06-24  4:36 ` [spp] [PATCH 02/13] shared/sec: rename functions for spp_mirror yasufum.o
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: yasufum.o @ 2019-06-24  4:36 UTC (permalink / raw)
  To: spp, ferruh.yigit, yasufum.o

From: Yasufumi Ogawa <yasufum.o@gmail.com>

For refactoring, rename operation functions in `response_info_list`
of struct cmd_response. It is for assembling a JSON response message
and starts with `append_`. However, There are many functions start
with `append_` other than these operation functions and it is so
confusing. This update is to rename the prefix to `add_`, and revise
comments and vars.

Signed-off-by: Yasufumi Ogawa <yasufum.o@gmail.com>
---
 .../secondary/spp_worker_th/cmd_runner.c      | 71 ++++++++++---------
 1 file changed, 38 insertions(+), 33 deletions(-)

diff --git a/src/shared/secondary/spp_worker_th/cmd_runner.c b/src/shared/secondary/spp_worker_th/cmd_runner.c
index bc3ae06..f686209 100644
--- a/src/shared/secondary/spp_worker_th/cmd_runner.c
+++ b/src/shared/secondary/spp_worker_th/cmd_runner.c
@@ -561,7 +561,7 @@ spp_iterate_core_info(struct spp_iterate_core_params *params)
 /* Iterate classifier_table to create response to status command */
 #ifdef SPP_VF_MODULE
 static int
-add_classifier_table(
+_add_classifier_table(
 		struct spp_iterate_classifier_table_params *params)
 {
 	int ret;
@@ -903,9 +903,9 @@ append_error_details_value(const char *name, char **output, void *tmp)
 	return ret;
 }
 
-/* append a client id for JSON format */
+/* Add entry of client ID to a response in JSON. */
 static int
-append_client_id_value(const char *name, char **output,
+add_client_id(const char *name, char **output,
 		void *tmp __attribute__ ((unused)))
 {
 	return append_json_int_value(name, output, sppwk_get_client_id());
@@ -947,9 +947,9 @@ append_process_type_value(const char *name, char **output,
 			SPPWK_PROC_TYPE_LIST[sppwk_get_proc_type()]);
 }
 
-/* append a list of interface numbers for JSON format */
+/* Add entry of port to a response in JSON such as "phy:0". */
 static int
-append_interface_value(const char *name, char **output,
+add_interface(const char *name, char **output,
 		void *tmp __attribute__ ((unused)))
 {
 	int ret = SPP_RET_NG;
@@ -1115,7 +1115,7 @@ append_port_array(const char *name, char **output, const int num,
 }
 
 /**
- * TODO(yasufum) add usages called from `append_core_value` or refactor
+ * TODO(yasufum) add usages called from `add_core` or refactor
  * confusing function names.
  */
 /* append one element of core information for JSON format */
@@ -1179,9 +1179,9 @@ append_core_element_value(
 	return ret;
 }
 
-/* append master lcore in JSON format */
+/* Add entry of master lcore to a response in JSON. */
 static int
-append_master_lcore_value(const char *name, char **output,
+add_master_lcore(const char *name, char **output,
 		void *tmp __attribute__ ((unused)))
 {
 	int ret = SPP_RET_NG;
@@ -1189,9 +1189,9 @@ append_master_lcore_value(const char *name, char **output,
 	return ret;
 }
 
-/* append a list of core information for JSON format */
+/* Add entry of core info of worker to a response in JSON such as "core:0". */
 static int
-append_core_value(const char *name, char **output,
+add_core(const char *name, char **output,
 		void *tmp __attribute__ ((unused)))
 {
 	int ret = SPP_RET_NG;
@@ -1280,17 +1280,16 @@ append_classifier_element_value(
 #endif /* SPP_VF_MODULE */
 
 /**
- * Append entries of classifier table in JSON. Before iterating the entries,
+ * Add entries of classifier table in JSON. Before iterating the entries,
  * this function calls several nested functions.
- *   append_classifier_table()  // This function.
- *     -> add_classifier_table()  // Wrapper and doesn't almost nothing.
+ *   add_classifier_table()  // This function.
+ *     -> _add_classifier_table()  // Wrapper and doesn't almost nothing.
  *       -> add_classifier_table_val()  // Setup data and call iterator.
  *         -> iterate_adding_mac_entry()
- *
  */
 #ifdef SPP_VF_MODULE
 static int
-append_classifier_table(const char *name, char **output,
+add_classifier_table(const char *name, char **output,
 		void *tmp __attribute__ ((unused)))
 {
 	int ret = SPP_RET_NG;
@@ -1307,7 +1306,7 @@ append_classifier_table(const char *name, char **output,
 	itr_params.output = tmp_buff;
 	itr_params.element_proc = append_classifier_element_value;
 
-	ret = add_classifier_table(&itr_params);
+	ret = _add_classifier_table(&itr_params);
 	if (unlikely(ret != SPP_RET_OK)) {
 		spp_strbuf_free(itr_params.output);
 		return SPP_RET_NG;
@@ -1386,20 +1385,26 @@ struct cmd_response response_result_list[] = {
 };
 
 /**
- * TODO(yasufum) Add desc why it is needed and how to be used. At least, func
- * name is not appropriate because not for reponse, but name of funcs returns
- * response.
+ * List of combination of tag and operator function. It is used to assemble
+ * a result of command in JSON like as following.
+ *
+ *     {
+ *         "client-id": 1,
+ *         "ports": ["phy:0", "phy:1", "vhost:0", "ring:0"],
+ *         "components": [
+ *             {
+ *                 "core": 2,
+ *                 ...
  */
-/* command response status information string list */
 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 },
+	{ "client-id", add_client_id },
+	{ "phy", add_interface },
+	{ "vhost", add_interface },
+	{ "ring", add_interface },
+	{ "master-lcore", add_master_lcore},
+	{ "core", add_core},
 #ifdef SPP_VF_MODULE
-	{ "classifier_table", append_classifier_table},
+	{ "classifier_table", add_classifier_table},
 #endif /* SPP_VF_MODULE */
 	{ "", NULL }
 };
@@ -1541,9 +1546,9 @@ send_decode_error_response(int *sock,
 	spp_strbuf_free(msg);
 }
 
-/* send response for command execution result */
+/* Send the result of command to spp-ctl. */
 static void
-send_command_result_response(int *sock,
+send_result_spp_ctl(int *sock,
 		const struct sppwk_cmd_req *request,
 		struct cmd_result *cmd_results)
 {
@@ -1569,7 +1574,7 @@ send_command_result_response(int *sock,
 
 	/* append client id information value */
 	if (request->is_requested_client_id) {
-		ret = append_client_id_value("client_id", &tmp_buff, NULL);
+		ret = add_client_id("client_id", &tmp_buff, NULL);
 		if (unlikely(ret < SPP_RET_OK)) {
 			spp_strbuf_free(tmp_buff);
 			RTE_LOG(ERR, WK_CMD_RUNNER, "Failed to make "
@@ -1675,14 +1680,14 @@ exec_cmds(int *sock, const char *req_str, size_t req_str_len)
 	/* Exec exit command. */
 	if (cmd_req.is_requested_exit) {
 		set_cmd_result(&cmd_results[0], CMD_SUCCESS, "");
-		send_command_result_response(sock, &cmd_req, cmd_results);
+		send_result_spp_ctl(sock, &cmd_req, cmd_results);
 		RTE_LOG(INFO, WK_CMD_RUNNER,
 				"Process is terminated with exit cmd.\n");
 		return SPP_RET_NG;
 	}
 
-	/* send response */
-	send_command_result_response(sock, &cmd_req, cmd_results);
+	/* Send response to spp-ctl. */
+	send_result_spp_ctl(sock, &cmd_req, cmd_results);
 
 	RTE_LOG(DEBUG, WK_CMD_RUNNER, "End command request processing.\n");
 
-- 
2.17.1


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

* [spp] [PATCH 02/13] shared/sec: rename functions for spp_mirror
  2019-06-24  4:36 [spp] [PATCH 00/13] Move JSON utils from libs for running cmds yasufum.o
  2019-06-24  4:36 ` [spp] [PATCH 01/13] shared/sec: rename ops for setup cmd response yasufum.o
@ 2019-06-24  4:36 ` yasufum.o
  2019-06-24  4:36 ` [spp] [PATCH 03/13] shared/sec: move principle JSON formatter funcs yasufum.o
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: yasufum.o @ 2019-06-24  4:36 UTC (permalink / raw)
  To: spp, ferruh.yigit, yasufum.o

From: Yasufumi Ogawa <yasufum.o@gmail.com>

Rename spp_mirror_get_component_status() to get_mirror_status(), and
spp_mirror_update() to update_mirror(). This update also revises
comments for the functions.

Signed-off-by: Yasufumi Ogawa <yasufum.o@gmail.com>
---
 src/mirror/spp_mirror.c                       | 72 ++++++++-----------
 .../secondary/spp_worker_th/cmd_runner.c      |  6 +-
 .../secondary/spp_worker_th/cmd_utils.c       |  2 +-
 .../secondary/spp_worker_th/mirror_deps.h     | 33 ++++-----
 src/shared/secondary/spp_worker_th/vf_deps.h  |  2 +-
 5 files changed, 47 insertions(+), 68 deletions(-)

diff --git a/src/mirror/spp_mirror.c b/src/mirror/spp_mirror.c
index f62ae7e..c2ec09d 100644
--- a/src/mirror/spp_mirror.c
+++ b/src/mirror/spp_mirror.c
@@ -277,50 +277,45 @@ mirror_proc_init(void)
 
 /* Update mirror info */
 int
-spp_mirror_update(struct sppwk_comp_info *component)
+update_mirror(struct sppwk_comp_info *wk_comp)
 {
 	int cnt = 0;
-	int num_rx = component->nof_rx;
-	int num_tx = component->nof_tx;
-	struct mirror_info *info = &g_mirror_info[component->comp_id];
+	int nof_rx = wk_comp->nof_rx;
+	int nof_tx = wk_comp->nof_tx;
+	struct mirror_info *info = &g_mirror_info[wk_comp->comp_id];
 	struct mirror_path *path = &info->path[info->upd_index];
 
-	/* mirror component allows only one receiving port. */
-	if (unlikely(num_rx > 1)) {
+	/* Check mirror has just one RX and two TX port. */
+	if (unlikely(nof_rx > 1)) {
 		RTE_LOG(ERR, MIRROR,
-			"Component[%d] Setting error. (type = %d, rx = %d)\n",
-			component->comp_id, component->wk_type, num_rx);
+			"Invalid num of RX (id=%d, type=%d, nof_rx=%d)\n",
+			wk_comp->comp_id, wk_comp->wk_type, nof_rx);
 		return SPP_RET_NG;
 	}
-
-	/* Component allows only tewo transmit port. */
-	if (unlikely(num_tx > 2)) {
+	if (unlikely(nof_tx > 2)) {
 		RTE_LOG(ERR, MIRROR,
-			"Component[%d] Setting error. (type = %d, tx = %d)\n",
-			component->comp_id, component->wk_type, num_tx);
+			"Invalid num of TX (id=%d, type=%d, nof_tx=%d)\n",
+			wk_comp->comp_id, wk_comp->wk_type, nof_tx);
 		return SPP_RET_NG;
 	}
 
 	memset(path, 0x00, sizeof(struct mirror_path));
 
 	RTE_LOG(INFO, MIRROR,
-			"Component[%d] Start update component. "
-			"(name = %s, type = %d)\n",
-			component->comp_id,
-			component->name,
-			component->wk_type);
-
-	memcpy(&path->name, component->name, STR_LEN_NAME);
-	path->wk_type = component->wk_type;
-	path->nof_rx = component->nof_rx;
-	path->nof_tx = component->nof_tx;
-	for (cnt = 0; cnt < num_rx; cnt++)
-		memcpy(&path->ports[cnt].rx, component->rx_ports[cnt],
+			"Start updating mirror (id=%d, name=%s, type=%d)\n",
+			wk_comp->comp_id, wk_comp->name, wk_comp->wk_type);
+
+	memcpy(&path->name, wk_comp->name, STR_LEN_NAME);
+	path->wk_type = wk_comp->wk_type;
+	path->nof_rx = wk_comp->nof_rx;
+	path->nof_tx = wk_comp->nof_tx;
+	for (cnt = 0; cnt < nof_rx; cnt++)
+		memcpy(&path->ports[cnt].rx, wk_comp->rx_ports[cnt],
 				sizeof(struct sppwk_port_info));
 
-	/* Transmit port is set according with larger num_rx / num_tx. */
-	for (cnt = 0; cnt < num_tx; cnt++)
-		memcpy(&path->ports[cnt].tx, component->tx_ports[cnt],
+	/* Transmit port is set according with larger nof_rx / nof_tx. */
+	for (cnt = 0; cnt < nof_tx; cnt++)
+		memcpy(&path->ports[cnt].tx, wk_comp->tx_ports[cnt],
 				sizeof(struct sppwk_port_info));
 
 	info->upd_index = info->ref_index;
@@ -328,10 +323,8 @@ spp_mirror_update(struct sppwk_comp_info *component)
 		rte_delay_us_block(SPP_CHANGE_UPDATE_INTERVAL);
 
 	RTE_LOG(INFO, MIRROR,
-			"Component[%d] Complete update component. "
-			"(name = %s, type = %d)\n",
-			component->comp_id, component->name,
-			component->wk_type);
+			"Done update mirror (id=%d, name=%s, type=%d)\n",
+			wk_comp->comp_id, wk_comp->name, wk_comp->wk_type);
 
 	return SPP_RET_OK;
 }
@@ -465,8 +458,7 @@ mirror_proc(int id)
 
 /* Mirror get component status */
 int
-spp_mirror_get_component_status(
-		unsigned int lcore_id, int id,
+get_mirror_status(unsigned int lcore_id, int id,
 		struct spp_iterate_core_params *params)
 {
 	int ret = SPP_RET_NG;
@@ -479,9 +471,8 @@ spp_mirror_get_component_status(
 
 	if (unlikely(path->wk_type == SPPWK_TYPE_NONE)) {
 		RTE_LOG(ERR, MIRROR,
-				"Component[%d] Not used. "
-				"(status)(core = %d, type = %d)\n",
-				id, lcore_id, path->wk_type);
+			"Mirror is not used. (id=%d, lcore=%d, type=%d)\n",
+			id, lcore_id, path->wk_type);
 		return SPP_RET_NG;
 	}
 
@@ -500,10 +491,9 @@ spp_mirror_get_component_status(
 	}
 
 	/* Set the information with the function specified by the command. */
-	ret = (*params->element_proc)(
-		params, lcore_id,
-		path->name, component_type,
-		path->nof_rx, rx_ports, path->nof_tx, tx_ports);
+	ret = (*params->element_proc)(params, lcore_id, path->name,
+			component_type, path->nof_rx, rx_ports, path->nof_tx,
+			tx_ports);
 	if (unlikely(ret != 0))
 		return SPP_RET_NG;
 
diff --git a/src/shared/secondary/spp_worker_th/cmd_runner.c b/src/shared/secondary/spp_worker_th/cmd_runner.c
index f686209..4c4abd8 100644
--- a/src/shared/secondary/spp_worker_th/cmd_runner.c
+++ b/src/shared/secondary/spp_worker_th/cmd_runner.c
@@ -540,10 +540,8 @@ spp_iterate_core_info(struct spp_iterate_core_params *params)
 			}
 #endif /* SPP_VF_MODULE */
 #ifdef SPP_MIRROR_MODULE
-			ret = spp_mirror_get_component_status(
-						lcore_id,
-						core->id[cnt],
-						params);
+			ret = get_mirror_status(lcore_id, core->id[cnt],
+					params);
 #endif /* SPP_MIRROR_MODULE */
 			if (unlikely(ret != 0)) {
 				RTE_LOG(ERR, WK_CMD_RUNNER, "Cannot iterate core "
diff --git a/src/shared/secondary/spp_worker_th/cmd_utils.c b/src/shared/secondary/spp_worker_th/cmd_utils.c
index 709c3da..79dd2e7 100644
--- a/src/shared/secondary/spp_worker_th/cmd_utils.c
+++ b/src/shared/secondary/spp_worker_th/cmd_utils.c
@@ -897,7 +897,7 @@ update_comp_info(void)
 #endif /* SPP_VF_MODULE */
 
 #ifdef SPP_MIRROR_MODULE
-		ret = spp_mirror_update(comp_info);
+		ret = update_mirror(comp_info);
 #endif /* SPP_MIRROR_MODULE */
 
 		if (unlikely(ret < 0)) {
diff --git a/src/shared/secondary/spp_worker_th/mirror_deps.h b/src/shared/secondary/spp_worker_th/mirror_deps.h
index 83edc1f..bfa715d 100644
--- a/src/shared/secondary/spp_worker_th/mirror_deps.h
+++ b/src/shared/secondary/spp_worker_th/mirror_deps.h
@@ -8,33 +8,24 @@
 #include "cmd_utils.h"
 
 /**
- * Update Mirror info
+ * Update mirror info.
  *
- * @param component
- *  The pointer to struct sppwk_comp_info.@n
- *  The data for updating the internal data of mirror.
- *
- * @retval SPP_RET_OK succeeded.
- * @retval SPP_RET_NG failed.
+ * @param wk_comp_info Pointer to internal data of mirror.
+ * @retval SPP_RET_OK If succeeded.
+ * @retval SPP_RET_NG If failed.
  */
-int spp_mirror_update(struct sppwk_comp_info *component);
+int update_mirror(struct sppwk_comp_info *wk_comp_info);
 
 /**
- * Mirror get component status
- *
- * @param lcore_id
- *  The logical core ID for forwarder and merger.
- * @param id
- *  The unique component ID.
- * @param params
- *  The pointer to struct spp_iterate_core_params.@n
- *  Detailed data of mirror status.
+ * Get mirror status.
  *
- * @retval SPP_RET_OK succeeded.
- * @retval SPP_RET_NG failed.
+ * @param lcore_id Lcore ID for forwarder and merger.
+ * @param id Unique component ID.
+ * @param params Pointer to detailed data of mirror status.
+ * @retval SPP_RET_OK If succeeded.
+ * @retval SPP_RET_NG If failed.
  */
-int spp_mirror_get_component_status(
-		unsigned int lcore_id, int id,
+int get_mirror_status(unsigned int lcore_id, int id,
 		struct spp_iterate_core_params *params);
 
 #endif  /* __SPP_WORKER_TH_MIRROR_DEPS_H__ */
diff --git a/src/shared/secondary/spp_worker_th/vf_deps.h b/src/shared/secondary/spp_worker_th/vf_deps.h
index beafc64..73e0f9a 100644
--- a/src/shared/secondary/spp_worker_th/vf_deps.h
+++ b/src/shared/secondary/spp_worker_th/vf_deps.h
@@ -56,7 +56,7 @@ free_mac_classifier(struct mac_classifier *mac_clf)
 /**
  * Update classifier info.
  *
- * @param comp_info Pointer to internal data of classifier.
+ * @param wk_comp_info Pointer to internal data of classifier.
  * @retval SPP_RET_OK If succeeded.
  * @retval SPP_RET_NG If failed.
  */
-- 
2.17.1


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

* [spp] [PATCH 03/13] shared/sec: move principle JSON formatter funcs
  2019-06-24  4:36 [spp] [PATCH 00/13] Move JSON utils from libs for running cmds yasufum.o
  2019-06-24  4:36 ` [spp] [PATCH 01/13] shared/sec: rename ops for setup cmd response yasufum.o
  2019-06-24  4:36 ` [spp] [PATCH 02/13] shared/sec: rename functions for spp_mirror yasufum.o
@ 2019-06-24  4:36 ` yasufum.o
  2019-06-24  4:36 ` [spp] [PATCH 04/13] shared/sec: change order of args of JSON fmtters yasufum.o
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: yasufum.o @ 2019-06-24  4:36 UTC (permalink / raw)
  To: spp, ferruh.yigit, yasufum.o

From: Yasufumi Ogawa <yasufum.o@gmail.com>

This update is to move JSON formatter functions start with `append_json`
which have principle features, such as appending int, uint or so, to
separate other formatters have prefix `append_` but not principle
features. Added files are `json_helper.c` and json_helper.h` for the
moved functions.

Signed-off-by: Yasufumi Ogawa <yasufum.o@gmail.com>
---
 src/mirror/Makefile                           |   1 +
 .../secondary/spp_worker_th/cmd_runner.c      | 127 +-----------------
 .../secondary/spp_worker_th/json_helper.c     | 125 +++++++++++++++++
 .../secondary/spp_worker_th/json_helper.h     |  38 ++++++
 .../secondary/spp_worker_th/string_buffer.h   |   2 +
 src/vf/Makefile                               |   1 +
 6 files changed, 168 insertions(+), 126 deletions(-)
 create mode 100644 src/shared/secondary/spp_worker_th/json_helper.c
 create mode 100644 src/shared/secondary/spp_worker_th/json_helper.h

diff --git a/src/mirror/Makefile b/src/mirror/Makefile
index 3e31cfa..6b6b9b9 100644
--- a/src/mirror/Makefile
+++ b/src/mirror/Makefile
@@ -22,6 +22,7 @@ SRCS-y += $(SPP_WKT_DIR)/spp_port.c
 SRCS-y += $(SPP_WKT_DIR)/conn_spp_ctl.c
 SRCS-y += $(SPP_WKT_DIR)/cmd_parser.c
 SRCS-y += $(SPP_WKT_DIR)/cmd_runner.c
+SRCS-y += $(SPP_WKT_DIR)/json_helper.c
 SRCS-y += $(SPP_WKT_DIR)/string_buffer.c
 SRCS-y += $(SPP_WKT_DIR)/ringlatencystats.c
 
diff --git a/src/shared/secondary/spp_worker_th/cmd_runner.c b/src/shared/secondary/spp_worker_th/cmd_runner.c
index 4c4abd8..423774b 100644
--- a/src/shared/secondary/spp_worker_th/cmd_runner.c
+++ b/src/shared/secondary/spp_worker_th/cmd_runner.c
@@ -13,6 +13,7 @@
 #include "spp_port.h"
 #include "string_buffer.h"
 
+#include "json_helper.h"
 #include "conn_spp_ctl.h"
 #include "cmd_parser.h"
 #include "cmd_runner.h"
@@ -21,17 +22,9 @@
 
 /* request message initial size */
 #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
 
-/* 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 ]"
-#define JSON_APPEND_BLOCK         "%s\"%s\": { %s }"
-#define JSON_APPEND_BLOCK_NONAME  "%s%s{ %s }"
-
 enum cmd_res_code {
 	CMD_SUCCESS = 0,
 	CMD_FAILED,
@@ -597,124 +590,6 @@ sppwk_get_ethdev_port_id(enum port_type iface_type, int iface_no)
 	}
 }
 
-/* append a comma for JSON format */
-static int
-append_json_comma(char **output)
-{
-	*output = spp_strbuf_append(*output, ", ", strlen(", "));
-	if (unlikely(*output == NULL)) {
-		RTE_LOG(ERR, WK_CMD_RUNNER,
-				"JSON's comma failed to add.\n");
-		return SPP_RET_NG;
-	}
-
-	return SPP_RET_OK;
-}
-
-/* append data of unsigned integral type for JSON format */
-static int
-append_json_uint_value(const char *name, char **output, unsigned int value)
-{
-	int len = strlen(*output);
-	/* extend the buffer */
-	*output = spp_strbuf_append(*output, "",
-			strlen(name) + CMD_TAG_APPEND_SIZE*2);
-	if (unlikely(*output == NULL)) {
-		RTE_LOG(ERR, WK_CMD_RUNNER,
-				"JSON's numeric format failed to add. "
-				"(name = %s, uint = %u)\n", name, value);
-		return SPP_RET_NG;
-	}
-
-	sprintf(&(*output)[len], JSON_APPEND_VALUE("%u"),
-			JSON_APPEND_COMMA(len), name, value);
-	return SPP_RET_OK;
-}
-
-/* append data of integral type for JSON format */
-static int
-append_json_int_value(const char *name, char **output, int value)
-{
-	int len = strlen(*output);
-	/* extend the buffer */
-	*output = spp_strbuf_append(*output, "",
-			strlen(name) + CMD_TAG_APPEND_SIZE*2);
-	if (unlikely(*output == NULL)) {
-		RTE_LOG(ERR, WK_CMD_RUNNER,
-				"JSON's numeric format failed to add. "
-				"(name = %s, int = %d)\n", name, value);
-		return SPP_RET_NG;
-	}
-
-	sprintf(&(*output)[len], JSON_APPEND_VALUE("%d"),
-			JSON_APPEND_COMMA(len), name, value);
-	return SPP_RET_OK;
-}
-
-/* append data of string type for JSON format */
-static int
-append_json_str_value(const char *name, char **output, const char *str)
-{
-	int len = strlen(*output);
-	/* extend the buffer */
-	*output = spp_strbuf_append(*output, "",
-			strlen(name) + strlen(str) + CMD_TAG_APPEND_SIZE);
-	if (unlikely(*output == NULL)) {
-		RTE_LOG(ERR, WK_CMD_RUNNER,
-				"JSON's string format failed to add. "
-				"(name = %s, str = %s)\n", name, str);
-		return SPP_RET_NG;
-	}
-
-	sprintf(&(*output)[len], JSON_APPEND_VALUE("\"%s\""),
-			JSON_APPEND_COMMA(len), name, str);
-	return SPP_RET_OK;
-}
-
-/* append brackets of the array for JSON format */
-static int
-append_json_array_brackets(const char *name, char **output, const char *str)
-{
-	int len = strlen(*output);
-	/* extend the buffer */
-	*output = spp_strbuf_append(*output, "",
-			strlen(name) + strlen(str) + CMD_TAG_APPEND_SIZE);
-	if (unlikely(*output == NULL)) {
-		RTE_LOG(ERR, WK_CMD_RUNNER,
-				"JSON's square bracket failed to add. "
-				"(name = %s, str = %s)\n", name, str);
-		return SPP_RET_NG;
-	}
-
-	sprintf(&(*output)[len], JSON_APPEND_ARRAY,
-			JSON_APPEND_COMMA(len), name, str);
-	return SPP_RET_OK;
-}
-
-/* append brackets of the blocks for JSON format */
-static int
-append_json_block_brackets(const char *name, char **output, const char *str)
-{
-	int len = strlen(*output);
-	/* extend the buffer */
-	*output = spp_strbuf_append(*output, "",
-			strlen(name) + strlen(str) + CMD_TAG_APPEND_SIZE);
-	if (unlikely(*output == NULL)) {
-		RTE_LOG(ERR, WK_CMD_RUNNER,
-				"JSON's curly bracket failed to add. "
-				"(name = %s, str = %s)\n", name, str);
-		return SPP_RET_NG;
-	}
-
-	if (name[0] == '\0')
-		sprintf(&(*output)[len], JSON_APPEND_BLOCK_NONAME,
-				JSON_APPEND_COMMA(len), name, str);
-	else
-		sprintf(&(*output)[len], JSON_APPEND_BLOCK,
-				JSON_APPEND_COMMA(len), name, str);
-	return SPP_RET_OK;
-}
-
 /* Execute one command. */
 static int
 exec_one_cmd(const struct sppwk_cmd_attrs *cmd)
diff --git a/src/shared/secondary/spp_worker_th/json_helper.c b/src/shared/secondary/spp_worker_th/json_helper.c
new file mode 100644
index 0000000..4c1baa3
--- /dev/null
+++ b/src/shared/secondary/spp_worker_th/json_helper.c
@@ -0,0 +1,125 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2019 Nippon Telegraph and Telephone Corporation
+ */
+
+#include "string_buffer.h"
+#include "json_helper.h"
+
+#define RTE_LOGTYPE_WK_JSON_HELPER RTE_LOGTYPE_USER1
+
+int
+append_json_comma(char **output)
+{
+	*output = spp_strbuf_append(*output, ", ", strlen(", "));
+	if (unlikely(*output == NULL)) {
+		RTE_LOG(ERR, WK_JSON_HELPER,
+				"JSON's comma failed to add.\n");
+		return SPP_RET_NG;
+	}
+
+	return SPP_RET_OK;
+}
+
+/* append data of unsigned integral type for JSON format */
+int
+append_json_uint_value(const char *name, char **output, unsigned int value)
+{
+	int len = strlen(*output);
+	/* extend the buffer */
+	*output = spp_strbuf_append(*output, "",
+			strlen(name) + CMD_TAG_APPEND_SIZE*2);
+	if (unlikely(*output == NULL)) {
+		RTE_LOG(ERR, WK_JSON_HELPER,
+				"JSON's numeric format failed to add. "
+				"(name = %s, uint = %u)\n", name, value);
+		return SPP_RET_NG;
+	}
+
+	sprintf(&(*output)[len], JSON_APPEND_VALUE("%u"),
+			JSON_APPEND_COMMA(len), name, value);
+	return SPP_RET_OK;
+}
+
+/* append data of integral type for JSON format */
+int
+append_json_int_value(const char *name, char **output, int value)
+{
+	int len = strlen(*output);
+	/* extend the buffer */
+	*output = spp_strbuf_append(*output, "",
+			strlen(name) + CMD_TAG_APPEND_SIZE*2);
+	if (unlikely(*output == NULL)) {
+		RTE_LOG(ERR, WK_JSON_HELPER,
+				"JSON's numeric format failed to add. "
+				"(name = %s, int = %d)\n", name, value);
+		return SPP_RET_NG;
+	}
+
+	sprintf(&(*output)[len], JSON_APPEND_VALUE("%d"),
+			JSON_APPEND_COMMA(len), name, value);
+	return SPP_RET_OK;
+}
+
+/* append data of string type for JSON format */
+int
+append_json_str_value(const char *name, char **output, const char *str)
+{
+	int len = strlen(*output);
+	/* extend the buffer */
+	*output = spp_strbuf_append(*output, "",
+			strlen(name) + strlen(str) + CMD_TAG_APPEND_SIZE);
+	if (unlikely(*output == NULL)) {
+		RTE_LOG(ERR, WK_JSON_HELPER,
+				"JSON's string format failed to add. "
+				"(name = %s, str = %s)\n", name, str);
+		return SPP_RET_NG;
+	}
+
+	sprintf(&(*output)[len], JSON_APPEND_VALUE("\"%s\""),
+			JSON_APPEND_COMMA(len), name, str);
+	return SPP_RET_OK;
+}
+
+/* append brackets of the array for JSON format */
+int
+append_json_array_brackets(const char *name, char **output, const char *str)
+{
+	int len = strlen(*output);
+	/* extend the buffer */
+	*output = spp_strbuf_append(*output, "",
+			strlen(name) + strlen(str) + CMD_TAG_APPEND_SIZE);
+	if (unlikely(*output == NULL)) {
+		RTE_LOG(ERR, WK_JSON_HELPER,
+				"JSON's square bracket failed to add. "
+				"(name = %s, str = %s)\n", name, str);
+		return SPP_RET_NG;
+	}
+
+	sprintf(&(*output)[len], JSON_APPEND_ARRAY,
+			JSON_APPEND_COMMA(len), name, str);
+	return SPP_RET_OK;
+}
+
+/* append brackets of the blocks for JSON format */
+int
+append_json_block_brackets(const char *name, char **output, const char *str)
+{
+	int len = strlen(*output);
+	/* extend the buffer */
+	*output = spp_strbuf_append(*output, "",
+			strlen(name) + strlen(str) + CMD_TAG_APPEND_SIZE);
+	if (unlikely(*output == NULL)) {
+		RTE_LOG(ERR, WK_JSON_HELPER,
+				"JSON's curly bracket failed to add. "
+				"(name = %s, str = %s)\n", name, str);
+		return SPP_RET_NG;
+	}
+
+	if (name[0] == '\0')
+		sprintf(&(*output)[len], JSON_APPEND_BLOCK_NONAME,
+				JSON_APPEND_COMMA(len), name, str);
+	else
+		sprintf(&(*output)[len], JSON_APPEND_BLOCK,
+				JSON_APPEND_COMMA(len), name, str);
+	return SPP_RET_OK;
+}
diff --git a/src/shared/secondary/spp_worker_th/json_helper.h b/src/shared/secondary/spp_worker_th/json_helper.h
new file mode 100644
index 0000000..f286404
--- /dev/null
+++ b/src/shared/secondary/spp_worker_th/json_helper.h
@@ -0,0 +1,38 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2019 Nippon Telegraph and Telephone Corporation
+ */
+
+#ifndef _SPPWK_JSON_HELPER_H_
+#define _SPPWK_JSON_HELPER_H_
+
+#include "cmd_utils.h"
+
+#define CMD_TAG_APPEND_SIZE 16
+
+#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_NONAME  "%s%s{ %s }"
+#define JSON_APPEND_BLOCK         "%s\"%s\": { %s }"
+
+int append_json_comma(char **output);
+
+int append_json_uint_value(const char *name, char **output,
+		unsigned int value);
+
+int append_json_int_value(const char *name, char **output,
+		int value);
+
+int append_json_str_value(const char *name, char **output,
+		const char *str);
+
+int append_json_array_brackets(const char *name, char **output,
+		const char *str);
+
+int append_json_block_brackets(const char *name, char **output,
+		const char *str);
+
+#endif
diff --git a/src/shared/secondary/spp_worker_th/string_buffer.h b/src/shared/secondary/spp_worker_th/string_buffer.h
index 34ee6cb..951f0ae 100644
--- a/src/shared/secondary/spp_worker_th/string_buffer.h
+++ b/src/shared/secondary/spp_worker_th/string_buffer.h
@@ -5,6 +5,8 @@
 #ifndef _STRING_BUFFER_H_
 #define _STRING_BUFFER_H_
 
+#include <stdlib.h>
+
 /**
  * @file
  * SPP String buffer management
diff --git a/src/vf/Makefile b/src/vf/Makefile
index faf72ee..ca8d2b3 100644
--- a/src/vf/Makefile
+++ b/src/vf/Makefile
@@ -22,6 +22,7 @@ SRCS-y += $(SPP_WKT_DIR)/conn_spp_ctl.c
 SRCS-y += $(SPP_WKT_DIR)/cmd_parser.c
 SRCS-y += $(SPP_WKT_DIR)/cmd_runner.c
 SRCS-y += $(SPP_WKT_DIR)/cmd_utils.c
+SRCS-y += $(SPP_WKT_DIR)/json_helper.c
 SRCS-y += ../shared/common.c
 SRCS-y += ../shared/secondary/utils.c ../shared/secondary/add_port.c
 
-- 
2.17.1


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

* [spp] [PATCH 04/13] shared/sec: change order of args of JSON fmtters
  2019-06-24  4:36 [spp] [PATCH 00/13] Move JSON utils from libs for running cmds yasufum.o
                   ` (2 preceding siblings ...)
  2019-06-24  4:36 ` [spp] [PATCH 03/13] shared/sec: move principle JSON formatter funcs yasufum.o
@ 2019-06-24  4:36 ` yasufum.o
  2019-06-24  4:36 ` [spp] [PATCH 05/13] shared/sec: move JSON formatter to shard/secondary yasufum.o
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: yasufum.o @ 2019-06-24  4:36 UTC (permalink / raw)
  To: spp, ferruh.yigit, yasufum.o

From: Yasufumi Ogawa <yasufum.o@gmail.com>

Formatter functions defined in `json_helper.h` takes three args, a pair
of name and value of JSON and a string buffer as a placeholder. But the
order of args `func(name, buf, value)` is not well. This update is to
move `buf` at first as similar to other C functions such as sprintf().

Signed-off-by: Yasufumi Ogawa <yasufum.o@gmail.com>
---
 .../secondary/spp_worker_th/cmd_runner.c      | 61 +++++++------
 .../secondary/spp_worker_th/json_helper.c     | 61 +++++++------
 .../secondary/spp_worker_th/json_helper.h     | 85 ++++++++++++++++---
 3 files changed, 138 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 423774b..e41fd85 100644
--- a/src/shared/secondary/spp_worker_th/cmd_runner.c
+++ b/src/shared/secondary/spp_worker_th/cmd_runner.c
@@ -21,6 +21,7 @@
 #define RTE_LOGTYPE_WK_CMD_RUNNER RTE_LOGTYPE_USER1
 
 /* request message initial size */
+#define CMD_TAG_APPEND_SIZE 16
 #define CMD_ERR_MSG_LEN 128
 #define CMD_REQ_BUF_INIT_SIZE 2048
 #define CMD_RES_BUF_INIT_SIZE 2048
@@ -741,7 +742,7 @@ 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->result);
+	return append_json_str_value(output, name, result->result);
 }
 
 /* append error details for JSON format */
@@ -764,14 +765,13 @@ append_error_details_value(const char *name, char **output, void *tmp)
 		return SPP_RET_NG;
 	}
 
-	ret = append_json_str_value("message", &tmp_buff,
-			result->err_msg);
+	ret = append_json_str_value(&tmp_buff, "message", result->err_msg);
 	if (unlikely(ret < 0)) {
 		spp_strbuf_free(tmp_buff);
 		return SPP_RET_NG;
 	}
 
-	ret = append_json_block_brackets(name, output, tmp_buff);
+	ret = append_json_block_brackets(output, name, tmp_buff);
 	spp_strbuf_free(tmp_buff);
 	return ret;
 }
@@ -781,7 +781,7 @@ static int
 add_client_id(const char *name, char **output,
 		void *tmp __attribute__ ((unused)))
 {
-	return append_json_int_value(name, output, sppwk_get_client_id());
+	return append_json_int_value(output, name, sppwk_get_client_id());
 }
 
 /* append a list of interface numbers */
@@ -816,7 +816,7 @@ static int
 append_process_type_value(const char *name, char **output,
 		void *tmp __attribute__ ((unused)))
 {
-	return append_json_str_value(name, output,
+	return append_json_str_value(output, name,
 			SPPWK_PROC_TYPE_LIST[sppwk_get_proc_type()]);
 }
 
@@ -849,7 +849,7 @@ add_interface(const char *name, char **output,
 		return SPP_RET_NG;
 	}
 
-	ret = append_json_array_brackets(name, output, tmp_buff);
+	ret = append_json_array_brackets(output, name, tmp_buff);
 	spp_strbuf_free(tmp_buff);
 	return ret;
 }
@@ -859,16 +859,16 @@ static int
 append_vlan_value(char **output, const int ope, const int vid, const int pcp)
 {
 	int ret = SPP_RET_OK;
-	ret = append_json_str_value("operation", output,
+	ret = append_json_str_value(output, "operation",
 			PORT_ABILITY_STAT_LIST[ope]);
 	if (unlikely(ret < SPP_RET_OK))
 		return SPP_RET_NG;
 
-	ret = append_json_int_value("id", output, vid);
+	ret = append_json_int_value(output, "id", vid);
 	if (unlikely(ret < 0))
 		return SPP_RET_NG;
 
-	ret = append_json_int_value("pcp", output, pcp);
+	ret = append_json_int_value(output, "pcp", pcp);
 	if (unlikely(ret < 0))
 		return SPP_RET_NG;
 
@@ -922,7 +922,7 @@ append_vlan_block(const char *name, char **output,
 			return SPP_RET_NG;
 	}
 
-	ret = append_json_block_brackets(name, output, tmp_buff);
+	ret = append_json_block_brackets(output, name, tmp_buff);
 	spp_strbuf_free(tmp_buff);
 	return ret;
 }
@@ -943,7 +943,7 @@ append_port_block(char **output, const struct sppwk_port_idx *port,
 	}
 
 	spp_format_port_string(port_str, port->iface_type, port->iface_no);
-	ret = append_json_str_value("port", &tmp_buff, port_str);
+	ret = append_json_str_value(&tmp_buff, "port", port_str);
 	if (unlikely(ret < SPP_RET_OK))
 		return SPP_RET_NG;
 
@@ -954,7 +954,7 @@ append_port_block(char **output, const struct sppwk_port_idx *port,
 	if (unlikely(ret < SPP_RET_OK))
 		return SPP_RET_NG;
 
-	ret = append_json_block_brackets("", output, tmp_buff);
+	ret = append_json_block_brackets(output, "", tmp_buff);
 	spp_strbuf_free(tmp_buff);
 	return ret;
 }
@@ -982,7 +982,7 @@ append_port_array(const char *name, char **output, const int num,
 			return SPP_RET_NG;
 	}
 
-	ret = append_json_array_brackets(name, output, tmp_buff);
+	ret = append_json_array_brackets(output, name, tmp_buff);
 	spp_strbuf_free(tmp_buff);
 	return ret;
 }
@@ -1020,17 +1020,17 @@ append_core_element_value(
 	 * TODO(yasufum) change ambiguous "core" to more specific one such as
 	 * "worker-lcores" or "slave-lcores".
 	 */
-	ret = append_json_uint_value("core", &tmp_buff, lcore_id);
+	ret = append_json_uint_value(&tmp_buff, "core", lcore_id);
 	if (unlikely(ret < SPP_RET_OK))
 		return ret;
 
 	if (unuse_flg) {
-		ret = append_json_str_value("name", &tmp_buff, name);
+		ret = append_json_str_value(&tmp_buff, "name", name);
 		if (unlikely(ret < 0))
 			return ret;
 	}
 
-	ret = append_json_str_value("type", &tmp_buff, type);
+	ret = append_json_str_value(&tmp_buff, "type", type);
 	if (unlikely(ret < SPP_RET_OK))
 		return ret;
 
@@ -1046,7 +1046,7 @@ append_core_element_value(
 			return ret;
 	}
 
-	ret = append_json_block_brackets("", &buff, tmp_buff);
+	ret = append_json_block_brackets(&buff, "", tmp_buff);
 	spp_strbuf_free(tmp_buff);
 	params->output = buff;
 	return ret;
@@ -1058,7 +1058,7 @@ add_master_lcore(const char *name, char **output,
 		void *tmp __attribute__ ((unused)))
 {
 	int ret = SPP_RET_NG;
-	ret = append_json_int_value(name, output, rte_get_master_lcore());
+	ret = append_json_int_value(output, name, rte_get_master_lcore());
 	return ret;
 }
 
@@ -1087,7 +1087,7 @@ add_core(const char *name, char **output,
 		return SPP_RET_NG;
 	}
 
-	ret = append_json_array_brackets(name, output, itr_params.output);
+	ret = append_json_array_brackets(output, name, itr_params.output);
 	spp_strbuf_free(itr_params.output);
 	return ret;
 }
@@ -1119,8 +1119,7 @@ append_classifier_element_value(
 
 	spp_format_port_string(port_str, port->iface_type, port->iface_no);
 
-	ret = append_json_str_value("type", &tmp_buff,
-			CLS_TYPE_A_LIST[type]);
+	ret = append_json_str_value(&tmp_buff, "type", CLS_TYPE_A_LIST[type]);
 	if (unlikely(ret < SPP_RET_OK))
 		return ret;
 
@@ -1137,15 +1136,15 @@ append_classifier_element_value(
 		break;
 	}
 
-	ret = append_json_str_value("value", &tmp_buff, value_str);
+	ret = append_json_str_value(&tmp_buff, "value", value_str);
 	if (unlikely(ret < 0))
 		return ret;
 
-	ret = append_json_str_value("port", &tmp_buff, port_str);
+	ret = append_json_str_value(&tmp_buff, "port", port_str);
 	if (unlikely(ret < SPP_RET_OK))
 		return ret;
 
-	ret = append_json_block_brackets("", &buff, tmp_buff);
+	ret = append_json_block_brackets(&buff, "", tmp_buff);
 	spp_strbuf_free(tmp_buff);
 	params->output = buff;
 	return ret;
@@ -1185,7 +1184,7 @@ add_classifier_table(const char *name, char **output,
 		return SPP_RET_NG;
 	}
 
-	ret = append_json_array_brackets(name, output, itr_params.output);
+	ret = append_json_array_brackets(output, name, itr_params.output);
 	spp_strbuf_free(itr_params.output);
 	return ret;
 }
@@ -1319,7 +1318,7 @@ append_command_results_value(const char *name, char **output,
 			return SPP_RET_NG;
 		}
 
-		ret = append_json_block_brackets("", &tmp_buff2, tmp_buff1);
+		ret = append_json_block_brackets(&tmp_buff2, "", tmp_buff1);
 		if (unlikely(ret < 0)) {
 			spp_strbuf_free(tmp_buff1);
 			spp_strbuf_free(tmp_buff2);
@@ -1328,7 +1327,7 @@ append_command_results_value(const char *name, char **output,
 
 	}
 
-	ret = append_json_array_brackets(name, output, tmp_buff2);
+	ret = append_json_array_brackets(output, name, tmp_buff2);
 	spp_strbuf_free(tmp_buff1);
 	spp_strbuf_free(tmp_buff2);
 	return ret;
@@ -1355,7 +1354,7 @@ append_info_value(const char *name, char **output)
 		return SPP_RET_NG;
 	}
 
-	ret = append_json_block_brackets(name, output, tmp_buff);
+	ret = append_json_block_brackets(output, name, tmp_buff);
 	spp_strbuf_free(tmp_buff);
 	return ret;
 }
@@ -1394,7 +1393,7 @@ send_decode_error_response(int *sock,
 				"(name = decode_error_response)\n");
 		return;
 	}
-	ret = append_json_block_brackets("", &msg, tmp_buff);
+	ret = append_json_block_brackets(&msg, "", tmp_buff);
 	spp_strbuf_free(tmp_buff);
 	if (unlikely(ret < SPP_RET_OK)) {
 		spp_strbuf_free(msg);
@@ -1477,7 +1476,7 @@ send_result_spp_ctl(int *sock,
 				"allocate error. (name = result_response)\n");
 		return;
 	}
-	ret = append_json_block_brackets("", &msg, tmp_buff);
+	ret = append_json_block_brackets(&msg, "", tmp_buff);
 	spp_strbuf_free(tmp_buff);
 	if (unlikely(ret < SPP_RET_OK)) {
 		spp_strbuf_free(msg);
diff --git a/src/shared/secondary/spp_worker_th/json_helper.c b/src/shared/secondary/spp_worker_th/json_helper.c
index 4c1baa3..20badb0 100644
--- a/src/shared/secondary/spp_worker_th/json_helper.c
+++ b/src/shared/secondary/spp_worker_th/json_helper.c
@@ -7,27 +7,27 @@
 
 #define RTE_LOGTYPE_WK_JSON_HELPER RTE_LOGTYPE_USER1
 
+/* Add a comma to given JSON string, or nothing if the end of the msg. */
 int
 append_json_comma(char **output)
 {
 	*output = spp_strbuf_append(*output, ", ", strlen(", "));
 	if (unlikely(*output == NULL)) {
-		RTE_LOG(ERR, WK_JSON_HELPER,
-				"JSON's comma failed to add.\n");
+		RTE_LOG(ERR, WK_JSON_HELPER, "Failed to add comma.\n");
 		return SPP_RET_NG;
 	}
 
 	return SPP_RET_OK;
 }
 
-/* append data of unsigned integral type for JSON format */
+/* Add a uint value to given JSON string. */
 int
-append_json_uint_value(const char *name, char **output, unsigned int value)
+append_json_uint_value(char **output, const char *name, unsigned int value)
 {
 	int len = strlen(*output);
-	/* extend the buffer */
+
 	*output = spp_strbuf_append(*output, "",
-			strlen(name) + CMD_TAG_APPEND_SIZE*2);
+			strlen(name) + JSON_APPEND_LEN*2);
 	if (unlikely(*output == NULL)) {
 		RTE_LOG(ERR, WK_JSON_HELPER,
 				"JSON's numeric format failed to add. "
@@ -40,14 +40,14 @@ append_json_uint_value(const char *name, char **output, unsigned int value)
 	return SPP_RET_OK;
 }
 
-/* append data of integral type for JSON format */
+/* Add an int value to given JSON string. */
 int
-append_json_int_value(const char *name, char **output, int value)
+append_json_int_value(char **output, const char *name, int value)
 {
 	int len = strlen(*output);
 	/* extend the buffer */
 	*output = spp_strbuf_append(*output, "",
-			strlen(name) + CMD_TAG_APPEND_SIZE*2);
+			strlen(name) + JSON_APPEND_LEN*2);
 	if (unlikely(*output == NULL)) {
 		RTE_LOG(ERR, WK_JSON_HELPER,
 				"JSON's numeric format failed to add. "
@@ -60,66 +60,77 @@ append_json_int_value(const char *name, char **output, int value)
 	return SPP_RET_OK;
 }
 
-/* append data of string type for JSON format */
+/* Add a string value to given JSON string. */
 int
-append_json_str_value(const char *name, char **output, const char *str)
+append_json_str_value(char **output, const char *name, const char *val)
 {
 	int len = strlen(*output);
 	/* extend the buffer */
 	*output = spp_strbuf_append(*output, "",
-			strlen(name) + strlen(str) + CMD_TAG_APPEND_SIZE);
+			strlen(name) + strlen(val) + JSON_APPEND_LEN);
 	if (unlikely(*output == NULL)) {
 		RTE_LOG(ERR, WK_JSON_HELPER,
 				"JSON's string format failed to add. "
-				"(name = %s, str = %s)\n", name, str);
+				"(name = %s, val= %s)\n", name, val);
 		return SPP_RET_NG;
 	}
 
 	sprintf(&(*output)[len], JSON_APPEND_VALUE("\"%s\""),
-			JSON_APPEND_COMMA(len), name, str);
+			JSON_APPEND_COMMA(len), name, val);
 	return SPP_RET_OK;
 }
 
-/* append brackets of the array for JSON format */
+/**
+ * Add an entry of array by surrounding given value with '[' and ']' to make
+ * it an array entry. The added entry `"key": [ val ]"` is defined as macro
+ * `JSON_APPEND_ARRAY`.
+ */
 int
-append_json_array_brackets(const char *name, char **output, const char *str)
+append_json_array_brackets(char **output, const char *name, const char *val)
 {
 	int len = strlen(*output);
 	/* extend the buffer */
 	*output = spp_strbuf_append(*output, "",
-			strlen(name) + strlen(str) + CMD_TAG_APPEND_SIZE);
+			strlen(name) + strlen(val) + JSON_APPEND_LEN);
 	if (unlikely(*output == NULL)) {
 		RTE_LOG(ERR, WK_JSON_HELPER,
 				"JSON's square bracket failed to add. "
-				"(name = %s, str = %s)\n", name, str);
+				"(name = %s, val= %s)\n", name, val);
 		return SPP_RET_NG;
 	}
 
 	sprintf(&(*output)[len], JSON_APPEND_ARRAY,
-			JSON_APPEND_COMMA(len), name, str);
+			JSON_APPEND_COMMA(len), name, val);
 	return SPP_RET_OK;
 }
 
-/* append brackets of the blocks for JSON format */
+/**
+ * Add an entry of hash by surrounding given value with '{' and '}' to make
+ * it a hash entry. The added entry `"key": { val }"` is defined as macro
+ * `JSON_APPEND_BLOCK`.
+ *
+ * This function is also used to make a block without key `{ val }` if given
+ * key is `""`.
+ */
 int
-append_json_block_brackets(const char *name, char **output, const char *str)
+append_json_block_brackets(char **output, const char *name, const char *val)
 {
 	int len = strlen(*output);
 	/* extend the buffer */
 	*output = spp_strbuf_append(*output, "",
-			strlen(name) + strlen(str) + CMD_TAG_APPEND_SIZE);
+			strlen(name) + strlen(val) + JSON_APPEND_LEN);
 	if (unlikely(*output == NULL)) {
 		RTE_LOG(ERR, WK_JSON_HELPER,
 				"JSON's curly bracket failed to add. "
-				"(name = %s, str = %s)\n", name, str);
+				"(name = %s, val= %s)\n", name, val);
 		return SPP_RET_NG;
 	}
 
 	if (name[0] == '\0')
 		sprintf(&(*output)[len], JSON_APPEND_BLOCK_NONAME,
-				JSON_APPEND_COMMA(len), name, str);
+				JSON_APPEND_COMMA(len), name, val);
 	else
 		sprintf(&(*output)[len], JSON_APPEND_BLOCK,
-				JSON_APPEND_COMMA(len), name, str);
+				JSON_APPEND_COMMA(len), name, val);
 	return SPP_RET_OK;
 }
diff --git a/src/shared/secondary/spp_worker_th/json_helper.h b/src/shared/secondary/spp_worker_th/json_helper.h
index f286404..0bfe0bf 100644
--- a/src/shared/secondary/spp_worker_th/json_helper.h
+++ b/src/shared/secondary/spp_worker_th/json_helper.h
@@ -7,32 +7,91 @@
 
 #include "cmd_utils.h"
 
-#define CMD_TAG_APPEND_SIZE 16
+/* TODO(yasufum) revise name considering the usage. */
+#define JSON_APPEND_LEN 16
 
+/* Add comma at the end of JSON statement, or do nothing. */
 #define JSON_APPEND_COMMA(flg)    ((flg)?", ":"")
 
+/**
+ * Defines of macro for JSON formatter. There are two ro three strings, the
+ * first one is placehodler for JSON msg, second one is key and third one is
+ * its value. Key and value are appended at the end of the placeholder.
+ */
 #define JSON_APPEND_VALUE(format) "%s\"%s\": "format
-
 #define JSON_APPEND_ARRAY         "%s\"%s\": [ %s ]"
-
 #define JSON_APPEND_BLOCK_NONAME  "%s%s{ %s }"
 #define JSON_APPEND_BLOCK         "%s\"%s\": { %s }"
 
+/**
+ * Add a comma to given JSON string, or nothing if the end of the statement.
+ *
+ * @param[in,out] output Placeholder of JSON msg.
+ */
 int append_json_comma(char **output);
 
-int append_json_uint_value(const char *name, char **output,
-		unsigned int value);
+/**
+ * Add a uint value to given JSON string.
+ *
+ * @param[in,out] output Placeholder of JSON msg.
+ * @param[in] name Name as a key.
+ * @param[in] val Uint value of the key.
+ * @retval SPP_RET_OK if succeeded.
+ * @retval SPP_RET_NG if failed.
+ */
+int append_json_uint_value(char **output, const char *name, unsigned int val);
 
-int append_json_int_value(const char *name, char **output,
-		int value);
+/**
+ * Add an int value to given JSON string.
+ *
+ * @param[in,out] output Placeholder of JSON msg.
+ * @param[in] name Name as a key.
+ * @param[in] val Int value of the key.
+ * @retval SPP_RET_OK if succeeded.
+ * @retval SPP_RET_NG if failed.
+ */
+int append_json_int_value(char **output, const char *name, int val);
 
-int append_json_str_value(const char *name, char **output,
-		const char *str);
+/**
+ * Add a string value to given JSON string.
+ *
+ * @param[in,out] output Placeholder of JSON msg.
+ * @param[in] name Name as a key.
+ * @param[in] val String value of the key.
+ * @retval SPP_RET_OK if succeeded.
+ * @retval SPP_RET_NG if failed.
+ */
+int append_json_str_value(char **output, const char *name, const char *val);
 
-int append_json_array_brackets(const char *name, char **output,
-		const char *str);
+/**
+ * Add an entry of array by surrounding given value with '[' and ']' to make
+ * it an array entry. The added entry `"key": [ val ]"` is defined as macro
+ * `JSON_APPEND_ARRAY`.
+ *
+ * @param[in,out] output Placeholder of JSON msg.
+ * @param[in] name Name as a key.
+ * @param[in] val String value of the key.
+ * @retval SPP_RET_OK if succeeded.
+ * @retval SPP_RET_NG if failed.
+ */
+int append_json_array_brackets(char **output, const char *name,
+		const char *val);
 
-int append_json_block_brackets(const char *name, char **output,
-		const char *str);
+/**
+ * Add an entry of hash by surrounding given value with '{' and '}' to make
+ * it a hash entry. The added entry `"key": { val }"` is defined as macro
+ * `JSON_APPEND_BLOCK`.
+ *
+ * This function is also used to make a block without key `{ val }` if given
+ * key is `""`.
+ *
+ * @param[in,out] output Placeholder of JSON msg.
+ * @param[in] name Name as a key.
+ * @param[in] val String value of the key.
+ * @retval SPP_RET_OK if succeeded.
+ * @retval SPP_RET_NG if failed.
+ */
+int append_json_block_brackets(char **output, const char *name,
+		const char *val);
 
 #endif
-- 
2.17.1


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

* [spp] [PATCH 05/13] shared/sec: move JSON formatter to shard/secondary
  2019-06-24  4:36 [spp] [PATCH 00/13] Move JSON utils from libs for running cmds yasufum.o
                   ` (3 preceding siblings ...)
  2019-06-24  4:36 ` [spp] [PATCH 04/13] shared/sec: change order of args of JSON fmtters yasufum.o
@ 2019-06-24  4:36 ` yasufum.o
  2019-06-24  4:36 ` [spp] [PATCH 06/13] shared/sec: revise including headers yasufum.o
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: yasufum.o @ 2019-06-24  4:36 UTC (permalink / raw)
  To: spp, ferruh.yigit, yasufum.o

From: Yasufumi Ogawa <yasufum.o@gmail.com>

Principle JSON formatters, such as append_json_int_value(), can be used
from all of SPP secondary processes. This update is to move files
`json_helper.*` and `string_buffer.*` to the parent directory
`shared/secondary`. Definition of return codes of `SPP_RET_OK` and
`SPP_RET_NG` is also moved to the directory for the change.

Signed-off-by: Yasufumi Ogawa <yasufum.o@gmail.com>
---
 src/mirror/Makefile                                 |  7 ++++---
 src/pcap/Makefile                                   |  3 ++-
 src/pcap/cmd_runner.c                               |  2 +-
 .../secondary/{spp_worker_th => }/json_helper.c     |  0
 .../secondary/{spp_worker_th => }/json_helper.h     |  6 +++++-
 src/shared/secondary/return_codes.h                 | 13 +++++++++++++
 src/shared/secondary/spp_worker_th/cmd_runner.c     |  4 ++--
 src/shared/secondary/spp_worker_th/cmd_utils.h      |  6 +-----
 src/shared/secondary/spp_worker_th/conn_spp_ctl.c   |  2 +-
 .../secondary/{spp_worker_th => }/string_buffer.c   |  0
 .../secondary/{spp_worker_th => }/string_buffer.h   |  0
 src/vf/Makefile                                     |  7 ++++---
 12 files changed, 33 insertions(+), 17 deletions(-)
 rename src/shared/secondary/{spp_worker_th => }/json_helper.c (100%)
 rename src/shared/secondary/{spp_worker_th => }/json_helper.h (95%)
 create mode 100644 src/shared/secondary/return_codes.h
 rename src/shared/secondary/{spp_worker_th => }/string_buffer.c (100%)
 rename src/shared/secondary/{spp_worker_th => }/string_buffer.h (100%)

diff --git a/src/mirror/Makefile b/src/mirror/Makefile
index 6b6b9b9..b31e116 100644
--- a/src/mirror/Makefile
+++ b/src/mirror/Makefile
@@ -11,19 +11,20 @@ include $(RTE_SDK)/mk/rte.vars.mk
 # binary name
 APP = spp_mirror
 
+SPP_SEC_DIR = ../shared/secondary
 SPP_WKT_DIR = ../shared/secondary/spp_worker_th
 
 # all source are stored in SRCS-y
 SRCS-y := spp_mirror.c
 SRCS-y += ../shared/common.c
-SRCS-y += ../shared/secondary/utils.c ../shared/secondary/add_port.c
+SRCS-y += $(SPP_SEC_DIR)/utils.c $(SPP_SEC_DIR)/add_port.c
+SRCS-y += $(SPP_SEC_DIR)/json_helper.c
+SRCS-y += $(SPP_SEC_DIR)/string_buffer.c
 SRCS-y += $(SPP_WKT_DIR)/cmd_utils.c
 SRCS-y += $(SPP_WKT_DIR)/spp_port.c
 SRCS-y += $(SPP_WKT_DIR)/conn_spp_ctl.c
 SRCS-y += $(SPP_WKT_DIR)/cmd_parser.c
 SRCS-y += $(SPP_WKT_DIR)/cmd_runner.c
-SRCS-y += $(SPP_WKT_DIR)/json_helper.c
-SRCS-y += $(SPP_WKT_DIR)/string_buffer.c
 SRCS-y += $(SPP_WKT_DIR)/ringlatencystats.c
 
 CFLAGS += -DALLOW_EXPERIMENTAL_API
diff --git a/src/pcap/Makefile b/src/pcap/Makefile
index c3d5ae4..852d036 100644
--- a/src/pcap/Makefile
+++ b/src/pcap/Makefile
@@ -11,6 +11,7 @@ include $(RTE_SDK)/mk/rte.vars.mk
 # binary name
 APP = spp_pcap
 
+SPP_SEC_DIR = ../shared/secondary
 SPP_WKT_DIR = ../shared/secondary/spp_worker_th
 
 # all source are stored in SRCS-y
@@ -18,10 +19,10 @@ SRCS-y := spp_pcap.c
 SRCS-y += cmd_utils.c
 SRCS-y += cmd_runner.c cmd_parser.c
 SRCS-y += ../shared/common.c
+SRCS-y += $(SPP_SEC_DIR)/string_buffer.c
 SRCS-y += $(SPP_WKT_DIR)/conn_spp_ctl.c
 SRCS-y += $(SPP_WKT_DIR)/spp_port.c
 SRCS-y += $(SPP_WKT_DIR)/ringlatencystats.c
-SRCS-y += $(SPP_WKT_DIR)/string_buffer.c
 
 CFLAGS += -DALLOW_EXPERIMENTAL_API
 CFLAGS += $(WERROR_FLAGS) -O3 -MMD
diff --git a/src/pcap/cmd_runner.c b/src/pcap/cmd_runner.c
index 8a1fb87..55f15bf 100644
--- a/src/pcap/cmd_runner.c
+++ b/src/pcap/cmd_runner.c
@@ -7,7 +7,7 @@
 
 #include <rte_log.h>
 
-#include "shared/secondary/spp_worker_th/string_buffer.h"
+#include "shared/secondary/string_buffer.h"
 #include "spp_pcap.h"
 #include "shared/secondary/spp_worker_th/conn_spp_ctl.h"
 #include "cmd_parser.h"
diff --git a/src/shared/secondary/spp_worker_th/json_helper.c b/src/shared/secondary/json_helper.c
similarity index 100%
rename from src/shared/secondary/spp_worker_th/json_helper.c
rename to src/shared/secondary/json_helper.c
diff --git a/src/shared/secondary/spp_worker_th/json_helper.h b/src/shared/secondary/json_helper.h
similarity index 95%
rename from src/shared/secondary/spp_worker_th/json_helper.h
rename to src/shared/secondary/json_helper.h
index 0bfe0bf..b004428 100644
--- a/src/shared/secondary/spp_worker_th/json_helper.h
+++ b/src/shared/secondary/json_helper.h
@@ -5,7 +5,11 @@
 #ifndef _SPPWK_JSON_HELPER_H_
 #define _SPPWK_JSON_HELPER_H_
 
-#include "cmd_utils.h"
+#include <string.h>
+#include <rte_branch_prediction.h>
+#include <rte_log.h>
+#include "return_codes.h"
+#include "string_buffer.h"
 
 /* TODO(yasufum) revise name considering the usage. */
 #define JSON_APPEND_LEN 16
diff --git a/src/shared/secondary/return_codes.h b/src/shared/secondary/return_codes.h
new file mode 100644
index 0000000..18b4711
--- /dev/null
+++ b/src/shared/secondary/return_codes.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2019 Nippon Telegraph and Telephone Corporation
+ */
+
+#ifndef _SPP_SEC_RETURN_CODES_H_
+#define _SPP_SEC_RETURN_CODES_H_
+
+enum spp_return_val {
+	SPP_RET_OK = 0,  /**< succeeded */
+	SPP_RET_NG = -1, /**< failed */
+};
+
+#endif
diff --git a/src/shared/secondary/spp_worker_th/cmd_runner.c b/src/shared/secondary/spp_worker_th/cmd_runner.c
index e41fd85..b1718fa 100644
--- a/src/shared/secondary/spp_worker_th/cmd_runner.c
+++ b/src/shared/secondary/spp_worker_th/cmd_runner.c
@@ -11,9 +11,9 @@
 #include "vf_deps.h"
 #include "mirror_deps.h"
 #include "spp_port.h"
-#include "string_buffer.h"
+#include "shared/secondary/string_buffer.h"
 
-#include "json_helper.h"
+#include "shared/secondary/json_helper.h"
 #include "conn_spp_ctl.h"
 #include "cmd_parser.h"
 #include "cmd_runner.h"
diff --git a/src/shared/secondary/spp_worker_th/cmd_utils.h b/src/shared/secondary/spp_worker_th/cmd_utils.h
index b8ab10c..3ee3142 100644
--- a/src/shared/secondary/spp_worker_th/cmd_utils.h
+++ b/src/shared/secondary/spp_worker_th/cmd_utils.h
@@ -12,6 +12,7 @@
  */
 
 #include <netinet/in.h>
+#include "shared/secondary/return_codes.h"
 #include "shared/common.h"
 
 /**
@@ -94,11 +95,6 @@ enum spp_classifier_type {
 	SPP_CLASSIFIER_TYPE_VLAN  /**< VLAN ID */
 };
 
-enum sppwk_return_val {
-	SPP_RET_OK = 0,  /**< succeeded */
-	SPP_RET_NG = -1, /**< failed */
-};
-
 /**
  * Port type (rx or tx) to indicate which direction packet goes
  * (e.g. receiving or transmitting)
diff --git a/src/shared/secondary/spp_worker_th/conn_spp_ctl.c b/src/shared/secondary/spp_worker_th/conn_spp_ctl.c
index 030da92..70e06f9 100644
--- a/src/shared/secondary/spp_worker_th/conn_spp_ctl.c
+++ b/src/shared/secondary/spp_worker_th/conn_spp_ctl.c
@@ -13,7 +13,7 @@
 #include <rte_branch_prediction.h>
 
 #include "shared/common.h"
-#include "string_buffer.h"
+#include "shared/secondary/string_buffer.h"
 #include "conn_spp_ctl.h"
 
 #define RTE_LOGTYPE_SPP_COMMAND_PROC RTE_LOGTYPE_USER1
diff --git a/src/shared/secondary/spp_worker_th/string_buffer.c b/src/shared/secondary/string_buffer.c
similarity index 100%
rename from src/shared/secondary/spp_worker_th/string_buffer.c
rename to src/shared/secondary/string_buffer.c
diff --git a/src/shared/secondary/spp_worker_th/string_buffer.h b/src/shared/secondary/string_buffer.h
similarity index 100%
rename from src/shared/secondary/spp_worker_th/string_buffer.h
rename to src/shared/secondary/string_buffer.h
diff --git a/src/vf/Makefile b/src/vf/Makefile
index ca8d2b3..3cbdb01 100644
--- a/src/vf/Makefile
+++ b/src/vf/Makefile
@@ -11,20 +11,21 @@ include $(RTE_SDK)/mk/rte.vars.mk
 # binary name
 APP = spp_vf
 
+SPP_SEC_DIR = ../shared/secondary
 SPP_WKT_DIR = ../shared/secondary/spp_worker_th
 
 # all source are stored in SRCS-y
 SRCS-y := spp_vf.c classifier_mac.c spp_forward.c
-SRCS-y += $(SPP_WKT_DIR)/string_buffer.c
+SRCS-y += $(SPP_SEC_DIR)/string_buffer.c
+SRCS-y += $(SPP_SEC_DIR)/json_helper.c
+SRCS-y += $(SPP_SEC_DIR)/utils.c $(SPP_SEC_DIR)/add_port.c
 SRCS-y += $(SPP_WKT_DIR)/ringlatencystats.c
 SRCS-y += $(SPP_WKT_DIR)/spp_port.c
 SRCS-y += $(SPP_WKT_DIR)/conn_spp_ctl.c
 SRCS-y += $(SPP_WKT_DIR)/cmd_parser.c
 SRCS-y += $(SPP_WKT_DIR)/cmd_runner.c
 SRCS-y += $(SPP_WKT_DIR)/cmd_utils.c
-SRCS-y += $(SPP_WKT_DIR)/json_helper.c
 SRCS-y += ../shared/common.c
-SRCS-y += ../shared/secondary/utils.c ../shared/secondary/add_port.c
 
 CFLAGS += -DALLOW_EXPERIMENTAL_API
 CFLAGS += $(WERROR_FLAGS) -O3 -MMD
-- 
2.17.1


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

* [spp] [PATCH 06/13] shared/sec: revise including headers
  2019-06-24  4:36 [spp] [PATCH 00/13] Move JSON utils from libs for running cmds yasufum.o
                   ` (4 preceding siblings ...)
  2019-06-24  4:36 ` [spp] [PATCH 05/13] shared/sec: move JSON formatter to shard/secondary yasufum.o
@ 2019-06-24  4:36 ` yasufum.o
  2019-06-24  4:36 ` [spp] [PATCH 07/13] shared/sec: move JSON formatters from cmd_runner yasufum.o
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: yasufum.o @ 2019-06-24  4:36 UTC (permalink / raw)
  To: spp, ferruh.yigit, yasufum.o

From: Yasufumi Ogawa <yasufum.o@gmail.com>

Some of including headers in SPP secondary are roughly described in
header files, so dependency is not obvious in each of source code. This
update is to refactor this situation by revising where including header
files.

Signed-off-by: Yasufumi Ogawa <yasufum.o@gmail.com>
---
 src/mirror/Makefile                           |  4 +++
 src/mirror/spp_mirror.c                       |  3 ++-
 src/mirror/spp_mirror.h                       | 26 -------------------
 .../secondary/spp_worker_th/cmd_parser.c      |  1 +
 .../secondary/spp_worker_th/cmd_utils.c       |  4 ++-
 .../secondary/spp_worker_th/cmd_utils.h       |  4 +--
 .../secondary/spp_worker_th/conn_spp_ctl.c    |  1 +
 src/shared/secondary/spp_worker_th/spp_port.c |  1 +
 src/vf/classifier_mac.c                       |  6 +++--
 src/vf/spp_forward.c                          |  5 ++--
 src/vf/spp_vf.c                               |  3 ++-
 11 files changed, 23 insertions(+), 35 deletions(-)
 delete mode 100644 src/mirror/spp_mirror.h

diff --git a/src/mirror/Makefile b/src/mirror/Makefile
index b31e116..fc18d1e 100644
--- a/src/mirror/Makefile
+++ b/src/mirror/Makefile
@@ -32,6 +32,10 @@ CFLAGS += $(WERROR_FLAGS) -O3 -MMD
 CFLAGS += -I$(SRCDIR)/../
 CFLAGS += -DSPP_MIRROR_MODULE
 
+# There are two kinds of copy mode, deep copy and shallow copy. If this
+# `DSPP_MIRROR_SHALLOWCOPY` is commented out, spp_mirror runs as in
+# deep copy mode.
+# Default mode is shallow copy.
 CFLAGS += -DSPP_MIRROR_SHALLOWCOPY
 
 # Optional Settings
diff --git a/src/mirror/spp_mirror.c b/src/mirror/spp_mirror.c
index c2ec09d..c00cad6 100644
--- a/src/mirror/spp_mirror.c
+++ b/src/mirror/spp_mirror.c
@@ -9,9 +9,10 @@
 #include <rte_common.h>
 #include <rte_cycles.h>
 
-#include "shared/secondary/spp_worker_th/mirror_deps.h"
 #include "shared/common.h"
+#include "shared/secondary/return_codes.h"
 #include "shared/secondary/utils.h"
+#include "shared/secondary/spp_worker_th/mirror_deps.h"
 #include "shared/secondary/spp_worker_th/cmd_runner.h"
 #include "shared/secondary/spp_worker_th/cmd_parser.h"
 #include "shared/secondary/spp_worker_th/cmd_utils.h"
diff --git a/src/mirror/spp_mirror.h b/src/mirror/spp_mirror.h
deleted file mode 100644
index 17fa522..0000000
--- a/src/mirror/spp_mirror.h
+++ /dev/null
@@ -1,26 +0,0 @@
-/* SPDX-License-Identifier: BSD-3-Clause
- * Copyright(c) 2018 Nippon Telegraph and Telephone Corporation
- */
-
-#ifndef __SPP_MIRROR_H__
-#define __SPP_MIRROR_H__
-
-#include "shared/secondary/spp_worker_th/spp_proc.h"
-
-/**
- * @file
- * SPP_MIRROR main
- *
- * Main function of spp_mirror.
- * This provides the function for initializing and starting the threads.
- *
- * There is two kinds of reproduction classification. I choose it by a
- * compilation switch.
- *  -DeepCopy
- *  -ShallowCopy
- *
- * Attention
- *  I do not do the deletion of the VLAN tag, the addition.
- */
-
-#endif /* __SPP_MIRROR_H__ */
diff --git a/src/shared/secondary/spp_worker_th/cmd_parser.c b/src/shared/secondary/spp_worker_th/cmd_parser.c
index 13e7013..200e41b 100644
--- a/src/shared/secondary/spp_worker_th/cmd_parser.c
+++ b/src/shared/secondary/spp_worker_th/cmd_parser.c
@@ -10,6 +10,7 @@
 #include <rte_branch_prediction.h>
 
 #include "cmd_parser.h"
+#include "shared/secondary/return_codes.h"
 
 #define RTE_LOGTYPE_WK_CMD_PARSER RTE_LOGTYPE_USER1
 
diff --git a/src/shared/secondary/spp_worker_th/cmd_utils.c b/src/shared/secondary/spp_worker_th/cmd_utils.c
index 79dd2e7..098c8c1 100644
--- a/src/shared/secondary/spp_worker_th/cmd_utils.c
+++ b/src/shared/secondary/spp_worker_th/cmd_utils.c
@@ -2,8 +2,8 @@
  * Copyright(c) 2018-2019 Nippon Telegraph and Telephone Corporation
  */
 
-#include <unistd.h>
 #include <string.h>
+#include <unistd.h>
 
 #include <rte_eth_ring.h>
 #include <rte_eth_vhost.h>
@@ -13,12 +13,14 @@
 
 #include "vf_deps.h"
 #include "mirror_deps.h"
+#include "shared/secondary/return_codes.h"
 #include "cmd_utils.h"
 #include "spp_port.h"
 
 #include "shared/secondary/add_port.h"
 #include "shared/secondary/utils.h"
 
+
 /* TODO(yasufum) change log label after filename is revised. */
 #define RTE_LOGTYPE_APP RTE_LOGTYPE_USER1
 
diff --git a/src/shared/secondary/spp_worker_th/cmd_utils.h b/src/shared/secondary/spp_worker_th/cmd_utils.h
index 3ee3142..f9d7015 100644
--- a/src/shared/secondary/spp_worker_th/cmd_utils.h
+++ b/src/shared/secondary/spp_worker_th/cmd_utils.h
@@ -5,14 +5,14 @@
 #ifndef _SPPWK_CMD_UTILS_H_
 #define _SPPWK_CMD_UTILS_H_
 
+#include <netinet/in.h>
+
 /**
  * @file cmd_utils.h
  *
  * Command utility functions for SPP worker thread.
  */
 
-#include <netinet/in.h>
-#include "shared/secondary/return_codes.h"
 #include "shared/common.h"
 
 /**
diff --git a/src/shared/secondary/spp_worker_th/conn_spp_ctl.c b/src/shared/secondary/spp_worker_th/conn_spp_ctl.c
index 70e06f9..a67cd10 100644
--- a/src/shared/secondary/spp_worker_th/conn_spp_ctl.c
+++ b/src/shared/secondary/spp_worker_th/conn_spp_ctl.c
@@ -15,6 +15,7 @@
 #include "shared/common.h"
 #include "shared/secondary/string_buffer.h"
 #include "conn_spp_ctl.h"
+#include "shared/secondary/return_codes.h"
 
 #define RTE_LOGTYPE_SPP_COMMAND_PROC RTE_LOGTYPE_USER1
 
diff --git a/src/shared/secondary/spp_worker_th/spp_port.c b/src/shared/secondary/spp_worker_th/spp_port.c
index 7a8a088..3163274 100644
--- a/src/shared/secondary/spp_worker_th/spp_port.c
+++ b/src/shared/secondary/spp_worker_th/spp_port.c
@@ -9,6 +9,7 @@
 #include <rte_net_crc.h>
 
 #include "spp_port.h"
+#include "shared/secondary/return_codes.h"
 #include "ringlatencystats.h"
 
 /* Port ability management information */
diff --git a/src/vf/classifier_mac.c b/src/vf/classifier_mac.c
index 8d677a9..6d8e664 100644
--- a/src/vf/classifier_mac.c
+++ b/src/vf/classifier_mac.c
@@ -19,11 +19,13 @@
 #include <rte_per_lcore.h>
 #include <rte_eal.h>
 #include <rte_launch.h>
+#include <netinet/in.h>
 
+#include "classifier_mac.h"
+#include "spp_vf.h"
+#include "shared/secondary/return_codes.h"
 #include "shared/secondary/spp_worker_th/vf_deps.h"
 #include "shared/secondary/spp_worker_th/spp_port.h"
-#include "spp_vf.h"
-#include "classifier_mac.h"
 
 #define RTE_LOGTYPE_SPP_CLASSIFIER_MAC RTE_LOGTYPE_USER1
 
diff --git a/src/vf/spp_forward.c b/src/vf/spp_forward.c
index 3d40951..d381c92 100644
--- a/src/vf/spp_forward.c
+++ b/src/vf/spp_forward.c
@@ -4,10 +4,11 @@
 
 #include <rte_cycles.h>
 
+#include "spp_forward.h"
+#include "spp_vf.h"
+#include "shared/secondary/return_codes.h"
 #include "shared/secondary/spp_worker_th/vf_deps.h"
 #include "shared/secondary/spp_worker_th/spp_port.h"
-#include "spp_vf.h"
-#include "spp_forward.h"
 
 #define RTE_LOGTYPE_FORWARD RTE_LOGTYPE_USER1
 
diff --git a/src/vf/spp_vf.c b/src/vf/spp_vf.c
index 94da67a..e55e3f0 100644
--- a/src/vf/spp_vf.c
+++ b/src/vf/spp_vf.c
@@ -6,10 +6,11 @@
 #include <arpa/inet.h>
 #include <getopt.h>
 
-#include "shared/secondary/spp_worker_th/cmd_utils.h"
 #include "spp_vf.h"
+#include "shared/secondary/spp_worker_th/cmd_utils.h"
 #include "classifier_mac.h"
 #include "spp_forward.h"
+#include "shared/secondary/return_codes.h"
 #include "shared/secondary/spp_worker_th/cmd_runner.h"
 #include "shared/secondary/spp_worker_th/cmd_parser.h"
 #include "shared/secondary/spp_worker_th/spp_port.h"
-- 
2.17.1


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

* [spp] [PATCH 07/13] shared/sec: move JSON formatters from cmd_runner
  2019-06-24  4:36 [spp] [PATCH 00/13] Move JSON utils from libs for running cmds yasufum.o
                   ` (5 preceding siblings ...)
  2019-06-24  4:36 ` [spp] [PATCH 06/13] shared/sec: revise including headers yasufum.o
@ 2019-06-24  4:36 ` yasufum.o
  2019-06-24  4:36 ` [spp] [PATCH 08/13] shared/sec: move rest of JSON formatters yasufum.o
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: yasufum.o @ 2019-06-24  4:36 UTC (permalink / raw)
  To: spp, ferruh.yigit, yasufum.o

From: Yasufumi Ogawa <yasufum.o@gmail.com>

JSON formatter functions for making a command result are defined in
`cmd_runner.c`, but better to move another file because this file is too
large and functions are not for running command.

This update is to move following functions to `cmd_res_formatter.c` and
its header. The rest of functions are moved in the next patch.

* append_result_value()
* append_error_details_value()
* is_port_flushed()
* append_interface_array()
* append_process_type_value()
* sppwk_get_proc_type()
* append_vlan_value()

Signed-off-by: Yasufumi Ogawa <yasufum.o@gmail.com>
---
 src/mirror/Makefile                           |   1 +
 .../spp_worker_th/cmd_res_formatter.c         | 147 +++++++++++++++++
 .../spp_worker_th/cmd_res_formatter.h         |  38 +++++
 .../secondary/spp_worker_th/cmd_runner.c      | 150 +-----------------
 src/vf/Makefile                               |   1 +
 5 files changed, 189 insertions(+), 148 deletions(-)
 create mode 100644 src/shared/secondary/spp_worker_th/cmd_res_formatter.c
 create mode 100644 src/shared/secondary/spp_worker_th/cmd_res_formatter.h

diff --git a/src/mirror/Makefile b/src/mirror/Makefile
index fc18d1e..7e666f5 100644
--- a/src/mirror/Makefile
+++ b/src/mirror/Makefile
@@ -26,6 +26,7 @@ SRCS-y += $(SPP_WKT_DIR)/conn_spp_ctl.c
 SRCS-y += $(SPP_WKT_DIR)/cmd_parser.c
 SRCS-y += $(SPP_WKT_DIR)/cmd_runner.c
 SRCS-y += $(SPP_WKT_DIR)/ringlatencystats.c
+SRCS-y += $(SPP_WKT_DIR)/cmd_res_formatter.c
 
 CFLAGS += -DALLOW_EXPERIMENTAL_API
 CFLAGS += $(WERROR_FLAGS) -O3 -MMD
diff --git a/src/shared/secondary/spp_worker_th/cmd_res_formatter.c b/src/shared/secondary/spp_worker_th/cmd_res_formatter.c
new file mode 100644
index 0000000..f66f97f
--- /dev/null
+++ b/src/shared/secondary/spp_worker_th/cmd_res_formatter.c
@@ -0,0 +1,147 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2019 Nippon Telegraph and Telephone Corporation
+ */
+
+#include "cmd_res_formatter.h"
+#include "cmd_utils.h"
+#include "shared/secondary/json_helper.h"
+
+#define RTE_LOGTYPE_WK_CMD_RES_FMT RTE_LOGTYPE_USER1
+
+/**
+ * List of worker process type. The order of items should be same as the order
+ * of enum `secondary_type` in cmd_utils.h.
+ */
+/* TODO(yasufum) rename `secondary_type` to `sppwk_proc_type`. */
+const char *SPPWK_PROC_TYPE_LIST[] = {
+	"none",
+	"vf",
+	"mirror",
+	"",  /* termination */
+};
+
+/**
+ * List of port abilities. The order of items should be same as the order of
+ * enum `spp_port_ability_type` in spp_vf.h.
+ */
+const char *PORT_ABILITY_STAT_LIST[] = {
+	"none",
+	"add",
+	"del",
+	"",  /* termination */
+};
+
+/* append a command result for JSON format */
+int
+append_result_value(const char *name, char **output, void *tmp)
+{
+	const struct cmd_result *result = tmp;
+	return append_json_str_value(output, name, result->result);
+}
+
+/* append error details for JSON format */
+int
+append_error_details_value(const char *name, char **output, void *tmp)
+{
+	int ret = SPP_RET_NG;
+	const struct cmd_result *result = tmp;
+	char *tmp_buff;
+	/* string is empty, except for errors */
+	if (result->err_msg[0] == '\0')
+		return SPP_RET_OK;
+
+	tmp_buff = spp_strbuf_allocate(CMD_RES_BUF_INIT_SIZE);
+	if (unlikely(tmp_buff == NULL)) {
+		RTE_LOG(ERR, WK_CMD_RES_FMT,
+				/* TODO(yasufum) refactor no meaning err msg */
+				"allocate error. (name = %s)\n",
+				name);
+		return SPP_RET_NG;
+	}
+
+	ret = append_json_str_value(&tmp_buff, "message", result->err_msg);
+	if (unlikely(ret < 0)) {
+		spp_strbuf_free(tmp_buff);
+		return SPP_RET_NG;
+	}
+
+	ret = append_json_block_brackets(output, name, tmp_buff);
+	spp_strbuf_free(tmp_buff);
+	return ret;
+}
+
+/* Check if port is already flushed. */
+int
+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;
+}
+
+/* append a list of interface numbers */
+int
+append_interface_array(char **output, const enum port_type type)
+{
+	int i, port_cnt = 0;
+	char tmp_str[CMD_TAG_APPEND_SIZE];
+
+	for (i = 0; i < RTE_MAX_ETHPORTS; i++) {
+		if (!is_port_flushed(type, i))
+			continue;
+
+		sprintf(tmp_str, "%s%d", JSON_APPEND_COMMA(port_cnt), i);
+
+		*output = spp_strbuf_append(*output, tmp_str, strlen(tmp_str));
+		if (unlikely(*output == NULL)) {
+			RTE_LOG(ERR, WK_CMD_RES_FMT,
+					"Interface number failed to add. "
+					"(type = %d)\n", type);
+			return SPP_RET_NG;
+		}
+
+		port_cnt++;
+	}
+
+	return SPP_RET_OK;
+}
+
+/* TODO(yasufum) move to another file for util funcs. */
+/* Get proc type from global command params. */
+int
+sppwk_get_proc_type(void)
+{
+	struct startup_param *params;
+	sppwk_get_mng_data(&params, NULL, NULL, NULL, NULL, NULL, NULL);
+	return params->secondary_type;
+}
+
+/* append a secondary process type for JSON format */
+int
+append_process_type_value(const char *name, char **output,
+		void *tmp __attribute__ ((unused)))
+{
+	return append_json_str_value(output, name,
+			SPPWK_PROC_TYPE_LIST[sppwk_get_proc_type()]);
+}
+
+/* append a value of vlan for JSON format */
+int
+append_vlan_value(char **output, const int ope, const int vid, const int pcp)
+{
+	int ret = SPP_RET_OK;
+	ret = append_json_str_value(output, "operation",
+			PORT_ABILITY_STAT_LIST[ope]);
+	if (unlikely(ret < SPP_RET_OK))
+		return SPP_RET_NG;
+
+	ret = append_json_int_value(output, "id", vid);
+	if (unlikely(ret < 0))
+		return SPP_RET_NG;
+
+	ret = append_json_int_value(output, "pcp", pcp);
+	if (unlikely(ret < 0))
+		return SPP_RET_NG;
+
+	return SPP_RET_OK;
+}
+
diff --git a/src/shared/secondary/spp_worker_th/cmd_res_formatter.h b/src/shared/secondary/spp_worker_th/cmd_res_formatter.h
new file mode 100644
index 0000000..c2da2ee
--- /dev/null
+++ b/src/shared/secondary/spp_worker_th/cmd_res_formatter.h
@@ -0,0 +1,38 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2019 Nippon Telegraph and Telephone Corporation
+ */
+
+#ifndef _SPPWK_CMD_RES_FORMATTER_H_
+#define _SPPWK_CMD_RES_FORMATTER_H_
+
+#include "shared/common.h"
+
+#define CMD_RES_LEN  32  /* Size of message including null char. */
+#define CMD_ERR_MSG_LEN 128
+
+#define CMD_RES_BUF_INIT_SIZE 2048
+#define CMD_TAG_APPEND_SIZE 16
+
+struct cmd_result {
+	int code;  /* Response code. */
+	char result[CMD_RES_LEN];  /* Response msg in short. */
+	char err_msg[CMD_ERR_MSG_LEN];  /* Used only if cmd is failed. */
+};
+
+int append_result_value(const char *name, char **output, void *tmp);
+
+int append_error_details_value(const char *name, char **output, void *tmp);
+
+int is_port_flushed(enum port_type iface_type, int iface_no);
+
+int append_interface_array(char **output, const enum port_type type);
+
+int append_process_type_value(const char *name, char **output,
+		void *tmp __attribute__ ((unused)));
+
+int sppwk_get_proc_type(void);
+
+int append_vlan_value(char **output, const int ope, const int vid,
+		const int pcp);
+
+#endif
diff --git a/src/shared/secondary/spp_worker_th/cmd_runner.c b/src/shared/secondary/spp_worker_th/cmd_runner.c
index b1718fa..7145045 100644
--- a/src/shared/secondary/spp_worker_th/cmd_runner.c
+++ b/src/shared/secondary/spp_worker_th/cmd_runner.c
@@ -8,23 +8,20 @@
 #include <rte_log.h>
 #include <rte_branch_prediction.h>
 
+#include "cmd_runner.h"
 #include "vf_deps.h"
 #include "mirror_deps.h"
 #include "spp_port.h"
 #include "shared/secondary/string_buffer.h"
-
 #include "shared/secondary/json_helper.h"
+#include "cmd_res_formatter.h"
 #include "conn_spp_ctl.h"
 #include "cmd_parser.h"
-#include "cmd_runner.h"
 
 #define RTE_LOGTYPE_WK_CMD_RUNNER RTE_LOGTYPE_USER1
 
 /* request message initial size */
-#define CMD_TAG_APPEND_SIZE 16
-#define CMD_ERR_MSG_LEN 128
 #define CMD_REQ_BUF_INIT_SIZE 2048
-#define CMD_RES_BUF_INIT_SIZE 2048
 
 enum cmd_res_code {
 	CMD_SUCCESS = 0,
@@ -32,12 +29,6 @@ enum cmd_res_code {
 	CMD_INVALID,
 };
 
-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. */
-};
-
 /**
  * Contains command response nad operator func for. It is used as an array of
  * this struct.
@@ -48,29 +39,6 @@ struct cmd_response {
 	int (*func)(const char *name, char **output, void *tmp);
 };
 
-/**
- * List of worker process type. The order of items should be same as the order
- * of enum `secondary_type` in cmd_utils.h.
- */
-/* TODO(yasufum) rename `secondary_type` to `sppwk_proc_type`. */
-const char *SPPWK_PROC_TYPE_LIST[] = {
-	"none",
-	"vf",
-	"mirror",
-	"",  /* termination */
-};
-
-/**
- * List of port abilities. The order of items should be same as the order of
- * enum `spp_port_ability_type` in spp_vf.h.
- */
-const char *PORT_ABILITY_STAT_LIST[] = {
-	"none",
-	"add",
-	"del",
-	"",  /* termination */
-};
-
 /**
  * List of classifier type. The order of items should be same as the order of
  * enum `spp_classifier_type` defined in cmd_utils.h.
@@ -93,24 +61,6 @@ 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)
-{
-	struct startup_param *params;
-	sppwk_get_mng_data(&params, NULL, NULL, NULL, NULL, NULL, NULL);
-	return params->secondary_type;
-}
-
-/* Check if port is already flushed. */
-static int
-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;
-}
-
 /* Update classifier table with given action, add or del. */
 static int
 update_cls_table(enum sppwk_action wk_action,
@@ -737,45 +687,6 @@ prepare_parse_err_msg(struct cmd_result *results,
 	}
 }
 
-/* append a command result for JSON format */
-static int
-append_result_value(const char *name, char **output, void *tmp)
-{
-	const struct cmd_result *result = tmp;
-	return append_json_str_value(output, name, result->result);
-}
-
-/* append error details for JSON format */
-static int
-append_error_details_value(const char *name, char **output, void *tmp)
-{
-	int ret = SPP_RET_NG;
-	const struct cmd_result *result = tmp;
-	char *tmp_buff;
-	/* string is empty, except for errors */
-	if (result->err_msg[0] == '\0')
-		return SPP_RET_OK;
-
-	tmp_buff = spp_strbuf_allocate(CMD_RES_BUF_INIT_SIZE);
-	if (unlikely(tmp_buff == NULL)) {
-		RTE_LOG(ERR, WK_CMD_RUNNER,
-				/* TODO(yasufum) refactor no meaning err msg */
-				"allocate error. (name = %s)\n",
-				name);
-		return SPP_RET_NG;
-	}
-
-	ret = append_json_str_value(&tmp_buff, "message", result->err_msg);
-	if (unlikely(ret < 0)) {
-		spp_strbuf_free(tmp_buff);
-		return SPP_RET_NG;
-	}
-
-	ret = append_json_block_brackets(output, name, tmp_buff);
-	spp_strbuf_free(tmp_buff);
-	return ret;
-}
-
 /* Add entry of client ID to a response in JSON. */
 static int
 add_client_id(const char *name, char **output,
@@ -784,42 +695,6 @@ add_client_id(const char *name, char **output,
 	return append_json_int_value(output, name, sppwk_get_client_id());
 }
 
-/* append a list of interface numbers */
-static int
-append_interface_array(char **output, const enum port_type type)
-{
-	int i, port_cnt = 0;
-	char tmp_str[CMD_TAG_APPEND_SIZE];
-
-	for (i = 0; i < RTE_MAX_ETHPORTS; i++) {
-		if (!is_port_flushed(type, i))
-			continue;
-
-		sprintf(tmp_str, "%s%d", JSON_APPEND_COMMA(port_cnt), i);
-
-		*output = spp_strbuf_append(*output, tmp_str, strlen(tmp_str));
-		if (unlikely(*output == NULL)) {
-			RTE_LOG(ERR, WK_CMD_RUNNER,
-					"Interface number failed to add. "
-					"(type = %d)\n", type);
-			return SPP_RET_NG;
-		}
-
-		port_cnt++;
-	}
-
-	return SPP_RET_OK;
-}
-
-/* append a secondary process type for JSON format */
-static int
-append_process_type_value(const char *name, char **output,
-		void *tmp __attribute__ ((unused)))
-{
-	return append_json_str_value(output, name,
-			SPPWK_PROC_TYPE_LIST[sppwk_get_proc_type()]);
-}
-
 /* Add entry of port to a response in JSON such as "phy:0". */
 static int
 add_interface(const char *name, char **output,
@@ -854,27 +729,6 @@ add_interface(const char *name, char **output,
 	return ret;
 }
 
-/* append a value of vlan for JSON format */
-static int
-append_vlan_value(char **output, const int ope, const int vid, const int pcp)
-{
-	int ret = SPP_RET_OK;
-	ret = append_json_str_value(output, "operation",
-			PORT_ABILITY_STAT_LIST[ope]);
-	if (unlikely(ret < SPP_RET_OK))
-		return SPP_RET_NG;
-
-	ret = append_json_int_value(output, "id", vid);
-	if (unlikely(ret < 0))
-		return SPP_RET_NG;
-
-	ret = append_json_int_value(output, "pcp", pcp);
-	if (unlikely(ret < 0))
-		return SPP_RET_NG;
-
-	return SPP_RET_OK;
-}
-
 /* append a block of vlan for JSON format */
 static int
 append_vlan_block(const char *name, char **output,
diff --git a/src/vf/Makefile b/src/vf/Makefile
index 3cbdb01..dd5a100 100644
--- a/src/vf/Makefile
+++ b/src/vf/Makefile
@@ -25,6 +25,7 @@ SRCS-y += $(SPP_WKT_DIR)/conn_spp_ctl.c
 SRCS-y += $(SPP_WKT_DIR)/cmd_parser.c
 SRCS-y += $(SPP_WKT_DIR)/cmd_runner.c
 SRCS-y += $(SPP_WKT_DIR)/cmd_utils.c
+SRCS-y += $(SPP_WKT_DIR)/cmd_res_formatter.c
 SRCS-y += ../shared/common.c
 
 CFLAGS += -DALLOW_EXPERIMENTAL_API
-- 
2.17.1


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

* [spp] [PATCH 08/13] shared/sec: move rest of JSON formatters
  2019-06-24  4:36 [spp] [PATCH 00/13] Move JSON utils from libs for running cmds yasufum.o
                   ` (6 preceding siblings ...)
  2019-06-24  4:36 ` [spp] [PATCH 07/13] shared/sec: move JSON formatters from cmd_runner yasufum.o
@ 2019-06-24  4:36 ` yasufum.o
  2019-06-24  4:36 ` [spp] [PATCH 09/13] shared/sec: move lcore funcs in response_info_list yasufum.o
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: yasufum.o @ 2019-06-24  4:36 UTC (permalink / raw)
  To: spp, ferruh.yigit, yasufum.o

From: Yasufumi Ogawa <yasufum.o@gmail.com>

This update is to move the rest of JSON formatters defined in
`cmd_runner.c`.

Signed-off-by: Yasufumi Ogawa <yasufum.o@gmail.com>
---
 .../spp_worker_th/cmd_res_formatter.c         | 396 ++++++++++++++++-
 .../spp_worker_th/cmd_res_formatter.h         |  45 +-
 .../secondary/spp_worker_th/cmd_runner.c      | 399 ------------------
 3 files changed, 433 insertions(+), 407 deletions(-)

diff --git a/src/shared/secondary/spp_worker_th/cmd_res_formatter.c b/src/shared/secondary/spp_worker_th/cmd_res_formatter.c
index f66f97f..b94cb29 100644
--- a/src/shared/secondary/spp_worker_th/cmd_res_formatter.c
+++ b/src/shared/secondary/spp_worker_th/cmd_res_formatter.c
@@ -31,6 +31,25 @@ const char *PORT_ABILITY_STAT_LIST[] = {
 	"",  /* termination */
 };
 
+/**
+ * List of classifier type. The order of items should be same as the order of
+ * enum `spp_classifier_type` defined in cmd_utils.h.
+ */
+/* TODO(yasufum) fix similar var in cmd_parser.c */
+const char *CLS_TYPE_A_LIST[] = {
+	"none",
+	"mac",
+	"vlan",
+	"",  /* termination */
+};
+
+/* command response result string list */
+struct cmd_response response_result_list[] = {
+	{ "result", append_result_value },
+	{ "error_details", append_error_details_value },
+	{ "", NULL }
+};
+
 /* append a command result for JSON format */
 int
 append_result_value(const char *name, char **output, void *tmp)
@@ -71,7 +90,7 @@ append_error_details_value(const char *name, char **output, void *tmp)
 }
 
 /* Check if port is already flushed. */
-int
+static int
 is_port_flushed(enum port_type iface_type, int iface_no)
 {
 	struct sppwk_port_info *port = get_sppwk_port(iface_type, iface_no);
@@ -107,8 +126,8 @@ append_interface_array(char **output, const enum port_type type)
 
 /* TODO(yasufum) move to another file for util funcs. */
 /* Get proc type from global command params. */
-int
-sppwk_get_proc_type(void)
+static int
+get_wk_type(void)
 {
 	struct startup_param *params;
 	sppwk_get_mng_data(&params, NULL, NULL, NULL, NULL, NULL, NULL);
@@ -121,7 +140,7 @@ append_process_type_value(const char *name, char **output,
 		void *tmp __attribute__ ((unused)))
 {
 	return append_json_str_value(output, name,
-			SPPWK_PROC_TYPE_LIST[sppwk_get_proc_type()]);
+			SPPWK_PROC_TYPE_LIST[get_wk_type()]);
 }
 
 /* append a value of vlan for JSON format */
@@ -145,3 +164,372 @@ append_vlan_value(char **output, const int ope, const int vid, const int pcp)
 	return SPP_RET_OK;
 }
 
+/* append a block of vlan for JSON format */
+int
+append_vlan_block(const char *name, char **output,
+		const int port_id, const enum spp_port_rxtx rxtx)
+{
+	int ret = SPP_RET_NG;
+	int i = 0;
+	struct spp_port_ability *info = NULL;
+	char *tmp_buff = spp_strbuf_allocate(CMD_RES_BUF_INIT_SIZE);
+	if (unlikely(tmp_buff == NULL)) {
+		RTE_LOG(ERR, WK_CMD_RES_FMT,
+				/* TODO(yasufum) refactor no meaning err msg */
+				"allocate error. (name = %s)\n",
+				name);
+		return SPP_RET_NG;
+	}
+
+	spp_port_ability_get_info(port_id, rxtx, &info);
+	for (i = 0; i < SPP_PORT_ABILITY_MAX; i++) {
+		switch (info[i].ops) {
+		case SPPWK_PORT_ABL_OPS_ADD_VLANTAG:
+		case SPPWK_PORT_ABL_OPS_DEL_VLANTAG:
+			ret = append_vlan_value(&tmp_buff, info[i].ops,
+					info[i].data.vlantag.vid,
+					info[i].data.vlantag.pcp);
+			if (unlikely(ret < SPP_RET_OK))
+				return SPP_RET_NG;
+
+			/*
+			 * Change counter to "maximum+1" for exit the loop.
+			 * An if statement after loop termination is false
+			 * by "maximum+1 ".
+			 */
+			i = SPP_PORT_ABILITY_MAX + 1;
+			break;
+		default:
+			/* not used */
+			break;
+		}
+	}
+	if (i == SPP_PORT_ABILITY_MAX) {
+		ret = append_vlan_value(&tmp_buff, SPPWK_PORT_ABL_OPS_NONE,
+				0, 0);
+		if (unlikely(ret < SPP_RET_OK))
+			return SPP_RET_NG;
+	}
+
+	ret = append_json_block_brackets(output, name, tmp_buff);
+	spp_strbuf_free(tmp_buff);
+	return ret;
+}
+
+/**
+ * Get consistent port ID of rte ethdev from resource UID such as `phy:0`.
+ * It returns a port ID, or error code if it's failed to.
+ */
+static int
+get_ethdev_port_id(enum port_type iface_type, int iface_no)
+{
+	struct iface_info *iface_info = NULL;
+
+	sppwk_get_mng_data(NULL, &iface_info,
+				NULL, NULL, NULL, NULL, NULL);
+	switch (iface_type) {
+	case PHY:
+		return iface_info->nic[iface_no].ethdev_port_id;
+	case RING:
+		return iface_info->ring[iface_no].ethdev_port_id;
+	case VHOST:
+		return iface_info->vhost[iface_no].ethdev_port_id;
+	default:
+		return SPP_RET_NG;
+	}
+}
+
+/* append a block of port numbers for JSON format */
+int
+append_port_block(char **output, const struct sppwk_port_idx *port,
+		const enum spp_port_rxtx rxtx)
+{
+	int ret = SPP_RET_NG;
+	char port_str[CMD_TAG_APPEND_SIZE];
+	char *tmp_buff = spp_strbuf_allocate(CMD_RES_BUF_INIT_SIZE);
+	if (unlikely(tmp_buff == NULL)) {
+		RTE_LOG(ERR, WK_CMD_RES_FMT,
+				/* TODO(yasufum) refactor no meaning err msg */
+				"allocate error. (name = port_block)\n");
+		return SPP_RET_NG;
+	}
+
+	spp_format_port_string(port_str, port->iface_type, port->iface_no);
+	ret = append_json_str_value(&tmp_buff, "port", port_str);
+	if (unlikely(ret < SPP_RET_OK))
+		return SPP_RET_NG;
+
+	ret = append_vlan_block("vlan", &tmp_buff,
+			get_ethdev_port_id(
+				port->iface_type, port->iface_no),
+			rxtx);
+	if (unlikely(ret < SPP_RET_OK))
+		return SPP_RET_NG;
+
+	ret = append_json_block_brackets(output, "", tmp_buff);
+	spp_strbuf_free(tmp_buff);
+	return ret;
+}
+
+/* append a list of port numbers for JSON format */
+int
+append_port_array(const char *name, char **output, const int num,
+		const struct sppwk_port_idx *ports,
+		const enum spp_port_rxtx rxtx)
+{
+	int ret = SPP_RET_NG;
+	int i = 0;
+	char *tmp_buff = spp_strbuf_allocate(CMD_RES_BUF_INIT_SIZE);
+	if (unlikely(tmp_buff == NULL)) {
+		RTE_LOG(ERR, WK_CMD_RES_FMT,
+				/* TODO(yasufum) refactor no meaning err msg */
+				"allocate error. (name = %s)\n",
+				name);
+		return SPP_RET_NG;
+	}
+
+	for (i = 0; i < num; i++) {
+		ret = append_port_block(&tmp_buff, &ports[i], rxtx);
+		if (unlikely(ret < SPP_RET_OK))
+			return SPP_RET_NG;
+	}
+
+	ret = append_json_array_brackets(output, name, tmp_buff);
+	spp_strbuf_free(tmp_buff);
+	return ret;
+}
+
+/**
+ * TODO(yasufum) add usages called from `add_core` or refactor
+ * confusing function names.
+ */
+/* append one element of core information for JSON format */
+int
+append_core_element_value(
+		struct spp_iterate_core_params *params,
+		const unsigned int lcore_id,
+		const char *name, const char *type,
+		const int num_rx, const struct sppwk_port_idx *rx_ports,
+		const int num_tx, const struct sppwk_port_idx *tx_ports)
+{
+	int ret = SPP_RET_NG;
+	int unuse_flg = 0;
+	char *buff, *tmp_buff;
+	buff = params->output;
+	tmp_buff = spp_strbuf_allocate(CMD_RES_BUF_INIT_SIZE);
+	if (unlikely(tmp_buff == NULL)) {
+		/* TODO(yasufum) refactor no meaning err msg */
+		RTE_LOG(ERR, WK_CMD_RES_FMT,
+				"allocate error. (name = %s)\n",
+				name);
+		return ret;
+	}
+
+	/* there is unnecessary data when "unuse" by type */
+	unuse_flg = strcmp(type, SPPWK_TYPE_NONE_STR);
+
+	/**
+	 * TODO(yasufum) change ambiguous "core" to more specific one such as
+	 * "worker-lcores" or "slave-lcores".
+	 */
+	ret = append_json_uint_value(&tmp_buff, "core", lcore_id);
+	if (unlikely(ret < SPP_RET_OK))
+		return ret;
+
+	if (unuse_flg) {
+		ret = append_json_str_value(&tmp_buff, "name", name);
+		if (unlikely(ret < 0))
+			return ret;
+	}
+
+	ret = append_json_str_value(&tmp_buff, "type", type);
+	if (unlikely(ret < SPP_RET_OK))
+		return ret;
+
+	if (unuse_flg) {
+		ret = append_port_array("rx_port", &tmp_buff,
+				num_rx, rx_ports, SPP_PORT_RXTX_RX);
+		if (unlikely(ret < 0))
+			return ret;
+
+		ret = append_port_array("tx_port", &tmp_buff,
+				num_tx, tx_ports, SPP_PORT_RXTX_TX);
+		if (unlikely(ret < SPP_RET_OK))
+			return ret;
+	}
+
+	ret = append_json_block_brackets(&buff, "", tmp_buff);
+	spp_strbuf_free(tmp_buff);
+	params->output = buff;
+	return ret;
+}
+
+#ifdef SPP_VF_MODULE
+/**
+ * Operator function called in iterator for getting each of entries of
+ * classifier table named as iterate_adding_mac_entry().
+ */
+int
+append_classifier_element_value(
+		struct spp_iterate_classifier_table_params *params,
+		enum spp_classifier_type type,
+		int vid, const char *mac,
+		const struct sppwk_port_idx *port)
+{
+	int ret = SPP_RET_NG;
+	char *buff, *tmp_buff;
+	char port_str[CMD_TAG_APPEND_SIZE];
+	char value_str[STR_LEN_SHORT];
+	buff = params->output;
+	tmp_buff = spp_strbuf_allocate(CMD_RES_BUF_INIT_SIZE);
+	if (unlikely(tmp_buff == NULL)) {
+		RTE_LOG(ERR, WK_CMD_RES_FMT,
+				/* TODO(yasufum) refactor no meaning err msg */
+				"allocate error. (name = classifier_table)\n");
+		return ret;
+	}
+
+	spp_format_port_string(port_str, port->iface_type, port->iface_no);
+
+	ret = append_json_str_value(&tmp_buff, "type", CLS_TYPE_A_LIST[type]);
+	if (unlikely(ret < SPP_RET_OK))
+		return ret;
+
+	memset(value_str, 0x00, STR_LEN_SHORT);
+	switch (type) {
+	case SPP_CLASSIFIER_TYPE_MAC:
+		sprintf(value_str, "%s", mac);
+		break;
+	case SPP_CLASSIFIER_TYPE_VLAN:
+		sprintf(value_str, "%d/%s", vid, mac);
+		break;
+	default:
+		/* not used */
+		break;
+	}
+
+	ret = append_json_str_value(&tmp_buff, "value", value_str);
+	if (unlikely(ret < 0))
+		return ret;
+
+	ret = append_json_str_value(&tmp_buff, "port", port_str);
+	if (unlikely(ret < SPP_RET_OK))
+		return ret;
+
+	ret = append_json_block_brackets(&buff, "", tmp_buff);
+	spp_strbuf_free(tmp_buff);
+	params->output = buff;
+	return ret;
+}
+#endif /* SPP_VF_MODULE */
+
+/* append string of command response list for JSON format */
+int
+append_response_list_value(char **output, struct cmd_response *responses,
+		void *tmp)
+{
+	int ret = SPP_RET_NG;
+	int i;
+	char *tmp_buff;
+	tmp_buff = spp_strbuf_allocate(CMD_RES_BUF_INIT_SIZE);
+	if (unlikely(tmp_buff == NULL)) {
+		RTE_LOG(ERR, WK_CMD_RES_FMT,
+				/* TODO(yasufum) refactor no meaning err msg */
+				"allocate error. (name = response_list)\n");
+		return SPP_RET_NG;
+	}
+
+	for (i = 0; responses[i].tag_name[0] != '\0'; i++) {
+		tmp_buff[0] = '\0';
+		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, WK_CMD_RES_FMT,
+					"Failed to get reply string. "
+					"(tag = %s)\n", responses[i].tag_name);
+			return SPP_RET_NG;
+		}
+
+		if (tmp_buff[0] == '\0')
+			continue;
+
+		if ((*output)[0] != '\0') {
+			ret = append_json_comma(output);
+			if (unlikely(ret < SPP_RET_OK)) {
+				spp_strbuf_free(tmp_buff);
+				RTE_LOG(ERR, WK_CMD_RES_FMT,
+						"Failed to add commas. "
+						"(tag = %s)\n",
+						responses[i].tag_name);
+				return SPP_RET_NG;
+			}
+		}
+
+		*output = spp_strbuf_append(*output, tmp_buff,
+				strlen(tmp_buff));
+		if (unlikely(*output == NULL)) {
+			spp_strbuf_free(tmp_buff);
+			RTE_LOG(ERR, WK_CMD_RES_FMT,
+					"Failed to add reply string. "
+					"(tag = %s)\n",
+					responses[i].tag_name);
+			return SPP_RET_NG;
+		}
+	}
+
+	spp_strbuf_free(tmp_buff);
+	return SPP_RET_OK;
+}
+
+/* append a list of command results for JSON format. */
+int
+append_command_results_value(const char *name, char **output,
+		int num, struct cmd_result *results)
+{
+	int ret = SPP_RET_NG;
+	int i;
+	char *tmp_buff1, *tmp_buff2;
+	tmp_buff1 = spp_strbuf_allocate(CMD_RES_BUF_INIT_SIZE);
+	if (unlikely(tmp_buff1 == NULL)) {
+		RTE_LOG(ERR, WK_CMD_RES_FMT,
+				/* TODO(yasufum) refactor no meaning err msg */
+				"allocate error. (name = %s, buff=1)\n",
+				name);
+		return SPP_RET_NG;
+	}
+
+	tmp_buff2 = spp_strbuf_allocate(CMD_RES_BUF_INIT_SIZE);
+	if (unlikely(tmp_buff2 == NULL)) {
+		spp_strbuf_free(tmp_buff1);
+		RTE_LOG(ERR, WK_CMD_RES_FMT,
+				/* TODO(yasufum) refactor no meaning err msg */
+				"allocate error. (name = %s, buff=2)\n",
+				name);
+		return SPP_RET_NG;
+	}
+
+	for (i = 0; i < num; i++) {
+		tmp_buff1[0] = '\0';
+		ret = append_response_list_value(&tmp_buff1,
+				response_result_list, &results[i]);
+		if (unlikely(ret < 0)) {
+			spp_strbuf_free(tmp_buff1);
+			spp_strbuf_free(tmp_buff2);
+			return SPP_RET_NG;
+		}
+
+		ret = append_json_block_brackets(&tmp_buff2, "", tmp_buff1);
+		if (unlikely(ret < 0)) {
+			spp_strbuf_free(tmp_buff1);
+			spp_strbuf_free(tmp_buff2);
+			return SPP_RET_NG;
+		}
+
+	}
+
+	ret = append_json_array_brackets(output, name, tmp_buff2);
+	spp_strbuf_free(tmp_buff1);
+	spp_strbuf_free(tmp_buff2);
+	return ret;
+}
+
diff --git a/src/shared/secondary/spp_worker_th/cmd_res_formatter.h b/src/shared/secondary/spp_worker_th/cmd_res_formatter.h
index c2da2ee..f3e9879 100644
--- a/src/shared/secondary/spp_worker_th/cmd_res_formatter.h
+++ b/src/shared/secondary/spp_worker_th/cmd_res_formatter.h
@@ -5,9 +5,12 @@
 #ifndef _SPPWK_CMD_RES_FORMATTER_H_
 #define _SPPWK_CMD_RES_FORMATTER_H_
 
+#include "cmd_utils.h"
+#include "spp_port.h"
 #include "shared/common.h"
 
 #define CMD_RES_LEN  32  /* Size of message including null char. */
+#define CMD_RES_TAG_LEN  32
 #define CMD_ERR_MSG_LEN 128
 
 #define CMD_RES_BUF_INIT_SIZE 2048
@@ -19,20 +22,54 @@ struct cmd_result {
 	char err_msg[CMD_ERR_MSG_LEN];  /* Used only if cmd is failed. */
 };
 
+/**
+ * Contains command response and 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[CMD_RES_TAG_LEN];
+	int (*func)(const char *name, char **output, void *tmp);
+};
+
 int append_result_value(const char *name, char **output, void *tmp);
 
 int append_error_details_value(const char *name, char **output, void *tmp);
 
-int is_port_flushed(enum port_type iface_type, int iface_no);
-
 int append_interface_array(char **output, const enum port_type type);
 
 int append_process_type_value(const char *name, char **output,
 		void *tmp __attribute__ ((unused)));
 
-int sppwk_get_proc_type(void);
-
 int append_vlan_value(char **output, const int ope, const int vid,
 		const int pcp);
 
+int append_vlan_block(const char *name, char **output,
+		const int port_id, const enum spp_port_rxtx rxtx);
+
+int append_port_block(char **output, const struct sppwk_port_idx *port,
+		const enum spp_port_rxtx rxtx);
+
+int append_port_array(const char *name, char **output, const int num,
+		const struct sppwk_port_idx *ports,
+		const enum spp_port_rxtx rxtx);
+
+int append_core_element_value(struct spp_iterate_core_params *params,
+		const unsigned int lcore_id,
+		const char *name, const char *type,
+		const int num_rx, const struct sppwk_port_idx *rx_ports,
+		const int num_tx, const struct sppwk_port_idx *tx_ports);
+
+int append_classifier_element_value(
+		struct spp_iterate_classifier_table_params *params,
+		enum spp_classifier_type type,
+		int vid, const char *mac,
+		const struct sppwk_port_idx *port);
+
+int append_response_list_value(char **output, struct cmd_response *responses,
+		void *tmp);
+
+int append_command_results_value(const char *name, char **output,
+		int num, struct cmd_result *results);
+
 #endif
diff --git a/src/shared/secondary/spp_worker_th/cmd_runner.c b/src/shared/secondary/spp_worker_th/cmd_runner.c
index 7145045..5644aec 100644
--- a/src/shared/secondary/spp_worker_th/cmd_runner.c
+++ b/src/shared/secondary/spp_worker_th/cmd_runner.c
@@ -29,28 +29,6 @@ enum cmd_res_code {
 	CMD_INVALID,
 };
 
-/**
- * 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];
-	int (*func)(const char *name, char **output, void *tmp);
-};
-
-/**
- * List of classifier type. The order of items should be same as the order of
- * enum `spp_classifier_type` defined in cmd_utils.h.
- */
-/* TODO(yasufum) fix similar var in cmd_parser.c */
-const char *CLS_TYPE_A_LIST[] = {
-	"none",
-	"mac",
-	"vlan",
-	"",  /* termination */
-};
-
 /* TODO(yasufum) move to another file for util funcs. */
 /* Get client ID from global command params. */
 static int
@@ -518,29 +496,6 @@ _add_classifier_table(
 }
 #endif /* SPP_VF_MODULE */
 
-/**
- * Get consistent port ID of rte ethdev from resource UID such as `phy:0`.
- * It returns a port ID, or error code if it's failed to.
- */
-static int
-sppwk_get_ethdev_port_id(enum port_type iface_type, int iface_no)
-{
-	struct iface_info *iface_info = NULL;
-
-	sppwk_get_mng_data(NULL, &iface_info,
-				NULL, NULL, NULL, NULL, NULL);
-	switch (iface_type) {
-	case PHY:
-		return iface_info->nic[iface_no].ethdev_port_id;
-	case RING:
-		return iface_info->ring[iface_no].ethdev_port_id;
-	case VHOST:
-		return iface_info->vhost[iface_no].ethdev_port_id;
-	default:
-		return SPP_RET_NG;
-	}
-}
-
 /* Execute one command. */
 static int
 exec_one_cmd(const struct sppwk_cmd_attrs *cmd)
@@ -729,183 +684,6 @@ add_interface(const char *name, char **output,
 	return ret;
 }
 
-/* append a block of vlan for JSON format */
-static int
-append_vlan_block(const char *name, char **output,
-		const int port_id, const enum spp_port_rxtx rxtx)
-{
-	int ret = SPP_RET_NG;
-	int i = 0;
-	struct spp_port_ability *info = NULL;
-	char *tmp_buff = spp_strbuf_allocate(CMD_RES_BUF_INIT_SIZE);
-	if (unlikely(tmp_buff == NULL)) {
-		RTE_LOG(ERR, WK_CMD_RUNNER,
-				/* TODO(yasufum) refactor no meaning err msg */
-				"allocate error. (name = %s)\n",
-				name);
-		return SPP_RET_NG;
-	}
-
-	spp_port_ability_get_info(port_id, rxtx, &info);
-	for (i = 0; i < SPP_PORT_ABILITY_MAX; i++) {
-		switch (info[i].ops) {
-		case SPPWK_PORT_ABL_OPS_ADD_VLANTAG:
-		case SPPWK_PORT_ABL_OPS_DEL_VLANTAG:
-			ret = append_vlan_value(&tmp_buff, info[i].ops,
-					info[i].data.vlantag.vid,
-					info[i].data.vlantag.pcp);
-			if (unlikely(ret < SPP_RET_OK))
-				return SPP_RET_NG;
-
-			/*
-			 * Change counter to "maximum+1" for exit the loop.
-			 * An if statement after loop termination is false
-			 * by "maximum+1 ".
-			 */
-			i = SPP_PORT_ABILITY_MAX + 1;
-			break;
-		default:
-			/* not used */
-			break;
-		}
-	}
-	if (i == SPP_PORT_ABILITY_MAX) {
-		ret = append_vlan_value(&tmp_buff, SPPWK_PORT_ABL_OPS_NONE,
-				0, 0);
-		if (unlikely(ret < SPP_RET_OK))
-			return SPP_RET_NG;
-	}
-
-	ret = append_json_block_brackets(output, name, tmp_buff);
-	spp_strbuf_free(tmp_buff);
-	return ret;
-}
-
-/* append a block of port numbers for JSON format */
-static int
-append_port_block(char **output, const struct sppwk_port_idx *port,
-		const enum spp_port_rxtx rxtx)
-{
-	int ret = SPP_RET_NG;
-	char port_str[CMD_TAG_APPEND_SIZE];
-	char *tmp_buff = spp_strbuf_allocate(CMD_RES_BUF_INIT_SIZE);
-	if (unlikely(tmp_buff == NULL)) {
-		RTE_LOG(ERR, WK_CMD_RUNNER,
-				/* TODO(yasufum) refactor no meaning err msg */
-				"allocate error. (name = port_block)\n");
-		return SPP_RET_NG;
-	}
-
-	spp_format_port_string(port_str, port->iface_type, port->iface_no);
-	ret = append_json_str_value(&tmp_buff, "port", port_str);
-	if (unlikely(ret < SPP_RET_OK))
-		return SPP_RET_NG;
-
-	ret = append_vlan_block("vlan", &tmp_buff,
-			sppwk_get_ethdev_port_id(
-				port->iface_type, port->iface_no),
-			rxtx);
-	if (unlikely(ret < SPP_RET_OK))
-		return SPP_RET_NG;
-
-	ret = append_json_block_brackets(output, "", tmp_buff);
-	spp_strbuf_free(tmp_buff);
-	return ret;
-}
-
-/* append a list of port numbers for JSON format */
-static int
-append_port_array(const char *name, char **output, const int num,
-		const struct sppwk_port_idx *ports,
-		const enum spp_port_rxtx rxtx)
-{
-	int ret = SPP_RET_NG;
-	int i = 0;
-	char *tmp_buff = spp_strbuf_allocate(CMD_RES_BUF_INIT_SIZE);
-	if (unlikely(tmp_buff == NULL)) {
-		RTE_LOG(ERR, WK_CMD_RUNNER,
-				/* TODO(yasufum) refactor no meaning err msg */
-				"allocate error. (name = %s)\n",
-				name);
-		return SPP_RET_NG;
-	}
-
-	for (i = 0; i < num; i++) {
-		ret = append_port_block(&tmp_buff, &ports[i], rxtx);
-		if (unlikely(ret < SPP_RET_OK))
-			return SPP_RET_NG;
-	}
-
-	ret = append_json_array_brackets(output, name, tmp_buff);
-	spp_strbuf_free(tmp_buff);
-	return ret;
-}
-
-/**
- * TODO(yasufum) add usages called from `add_core` or refactor
- * confusing function names.
- */
-/* append one element of core information for JSON format */
-static int
-append_core_element_value(
-		struct spp_iterate_core_params *params,
-		const unsigned int lcore_id,
-		const char *name, const char *type,
-		const int num_rx, const struct sppwk_port_idx *rx_ports,
-		const int num_tx, const struct sppwk_port_idx *tx_ports)
-{
-	int ret = SPP_RET_NG;
-	int unuse_flg = 0;
-	char *buff, *tmp_buff;
-	buff = params->output;
-	tmp_buff = spp_strbuf_allocate(CMD_RES_BUF_INIT_SIZE);
-	if (unlikely(tmp_buff == NULL)) {
-		/* TODO(yasufum) refactor no meaning err msg */
-		RTE_LOG(ERR, WK_CMD_RUNNER,
-				"allocate error. (name = %s)\n",
-				name);
-		return ret;
-	}
-
-	/* there is unnecessary data when "unuse" by type */
-	unuse_flg = strcmp(type, SPPWK_TYPE_NONE_STR);
-
-	/**
-	 * TODO(yasufum) change ambiguous "core" to more specific one such as
-	 * "worker-lcores" or "slave-lcores".
-	 */
-	ret = append_json_uint_value(&tmp_buff, "core", lcore_id);
-	if (unlikely(ret < SPP_RET_OK))
-		return ret;
-
-	if (unuse_flg) {
-		ret = append_json_str_value(&tmp_buff, "name", name);
-		if (unlikely(ret < 0))
-			return ret;
-	}
-
-	ret = append_json_str_value(&tmp_buff, "type", type);
-	if (unlikely(ret < SPP_RET_OK))
-		return ret;
-
-	if (unuse_flg) {
-		ret = append_port_array("rx_port", &tmp_buff,
-				num_rx, rx_ports, SPP_PORT_RXTX_RX);
-		if (unlikely(ret < 0))
-			return ret;
-
-		ret = append_port_array("tx_port", &tmp_buff,
-				num_tx, tx_ports, SPP_PORT_RXTX_TX);
-		if (unlikely(ret < SPP_RET_OK))
-			return ret;
-	}
-
-	ret = append_json_block_brackets(&buff, "", tmp_buff);
-	spp_strbuf_free(tmp_buff);
-	params->output = buff;
-	return ret;
-}
-
 /* Add entry of master lcore to a response in JSON. */
 static int
 add_master_lcore(const char *name, char **output,
@@ -946,65 +724,7 @@ add_core(const char *name, char **output,
 	return ret;
 }
 
-/**
- * Operator function called in iterator for getting each of entries of
- * classifier table named as iterate_adding_mac_entry().
- */
 #ifdef SPP_VF_MODULE
-static int
-append_classifier_element_value(
-		struct spp_iterate_classifier_table_params *params,
-		enum spp_classifier_type type,
-		int vid, const char *mac,
-		const struct sppwk_port_idx *port)
-{
-	int ret = SPP_RET_NG;
-	char *buff, *tmp_buff;
-	char port_str[CMD_TAG_APPEND_SIZE];
-	char value_str[STR_LEN_SHORT];
-	buff = params->output;
-	tmp_buff = spp_strbuf_allocate(CMD_RES_BUF_INIT_SIZE);
-	if (unlikely(tmp_buff == NULL)) {
-		RTE_LOG(ERR, WK_CMD_RUNNER,
-				/* TODO(yasufum) refactor no meaning err msg */
-				"allocate error. (name = classifier_table)\n");
-		return ret;
-	}
-
-	spp_format_port_string(port_str, port->iface_type, port->iface_no);
-
-	ret = append_json_str_value(&tmp_buff, "type", CLS_TYPE_A_LIST[type]);
-	if (unlikely(ret < SPP_RET_OK))
-		return ret;
-
-	memset(value_str, 0x00, STR_LEN_SHORT);
-	switch (type) {
-	case SPP_CLASSIFIER_TYPE_MAC:
-		sprintf(value_str, "%s", mac);
-		break;
-	case SPP_CLASSIFIER_TYPE_VLAN:
-		sprintf(value_str, "%d/%s", vid, mac);
-		break;
-	default:
-		/* not used */
-		break;
-	}
-
-	ret = append_json_str_value(&tmp_buff, "value", value_str);
-	if (unlikely(ret < 0))
-		return ret;
-
-	ret = append_json_str_value(&tmp_buff, "port", port_str);
-	if (unlikely(ret < SPP_RET_OK))
-		return ret;
-
-	ret = append_json_block_brackets(&buff, "", tmp_buff);
-	spp_strbuf_free(tmp_buff);
-	params->output = buff;
-	return ret;
-}
-#endif /* SPP_VF_MODULE */
-
 /**
  * Add entries of classifier table in JSON. Before iterating the entries,
  * this function calls several nested functions.
@@ -1013,7 +733,6 @@ append_classifier_element_value(
  *       -> add_classifier_table_val()  // Setup data and call iterator.
  *         -> iterate_adding_mac_entry()
  */
-#ifdef SPP_VF_MODULE
 static int
 add_classifier_table(const char *name, char **output,
 		void *tmp __attribute__ ((unused)))
@@ -1044,72 +763,6 @@ add_classifier_table(const char *name, char **output,
 }
 #endif /* SPP_VF_MODULE */
 
-/* append string of command response list for JSON format */
-static int
-append_response_list_value(char **output,
-		struct cmd_response *responses,
-		void *tmp)
-{
-	int ret = SPP_RET_NG;
-	int i;
-	char *tmp_buff;
-	tmp_buff = spp_strbuf_allocate(CMD_RES_BUF_INIT_SIZE);
-	if (unlikely(tmp_buff == NULL)) {
-		RTE_LOG(ERR, WK_CMD_RUNNER,
-				/* TODO(yasufum) refactor no meaning err msg */
-				"allocate error. (name = response_list)\n");
-		return SPP_RET_NG;
-	}
-
-	for (i = 0; responses[i].tag_name[0] != '\0'; i++) {
-		tmp_buff[0] = '\0';
-		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, WK_CMD_RUNNER,
-					"Failed to get reply string. "
-					"(tag = %s)\n", responses[i].tag_name);
-			return SPP_RET_NG;
-		}
-
-		if (tmp_buff[0] == '\0')
-			continue;
-
-		if ((*output)[0] != '\0') {
-			ret = append_json_comma(output);
-			if (unlikely(ret < SPP_RET_OK)) {
-				spp_strbuf_free(tmp_buff);
-				RTE_LOG(ERR, WK_CMD_RUNNER,
-						"Failed to add commas. "
-						"(tag = %s)\n",
-						responses[i].tag_name);
-				return SPP_RET_NG;
-			}
-		}
-
-		*output = spp_strbuf_append(*output, tmp_buff,
-				strlen(tmp_buff));
-		if (unlikely(*output == NULL)) {
-			spp_strbuf_free(tmp_buff);
-			RTE_LOG(ERR, WK_CMD_RUNNER,
-					"Failed to add reply string. "
-					"(tag = %s)\n",
-					responses[i].tag_name);
-			return SPP_RET_NG;
-		}
-	}
-
-	spp_strbuf_free(tmp_buff);
-	return SPP_RET_OK;
-}
-
-/* command response result string list */
-struct cmd_response response_result_list[] = {
-	{ "result", append_result_value },
-	{ "error_details", append_error_details_value },
-	{ "", NULL }
-};
-
 /**
  * List of combination of tag and operator function. It is used to assemble
  * a result of command in JSON like as following.
@@ -1135,58 +788,6 @@ struct cmd_response response_info_list[] = {
 	{ "", NULL }
 };
 
-/* append a list of command results for JSON format. */
-static int
-append_command_results_value(const char *name, char **output,
-		int num, struct cmd_result *results)
-{
-	int ret = SPP_RET_NG;
-	int i;
-	char *tmp_buff1, *tmp_buff2;
-	tmp_buff1 = spp_strbuf_allocate(CMD_RES_BUF_INIT_SIZE);
-	if (unlikely(tmp_buff1 == NULL)) {
-		RTE_LOG(ERR, WK_CMD_RUNNER,
-				/* TODO(yasufum) refactor no meaning err msg */
-				"allocate error. (name = %s, buff=1)\n",
-				name);
-		return SPP_RET_NG;
-	}
-
-	tmp_buff2 = spp_strbuf_allocate(CMD_RES_BUF_INIT_SIZE);
-	if (unlikely(tmp_buff2 == NULL)) {
-		spp_strbuf_free(tmp_buff1);
-		RTE_LOG(ERR, WK_CMD_RUNNER,
-				/* TODO(yasufum) refactor no meaning err msg */
-				"allocate error. (name = %s, buff=2)\n",
-				name);
-		return SPP_RET_NG;
-	}
-
-	for (i = 0; i < num; i++) {
-		tmp_buff1[0] = '\0';
-		ret = append_response_list_value(&tmp_buff1,
-				response_result_list, &results[i]);
-		if (unlikely(ret < 0)) {
-			spp_strbuf_free(tmp_buff1);
-			spp_strbuf_free(tmp_buff2);
-			return SPP_RET_NG;
-		}
-
-		ret = append_json_block_brackets(&tmp_buff2, "", tmp_buff1);
-		if (unlikely(ret < 0)) {
-			spp_strbuf_free(tmp_buff1);
-			spp_strbuf_free(tmp_buff2);
-			return SPP_RET_NG;
-		}
-
-	}
-
-	ret = append_json_array_brackets(output, name, tmp_buff2);
-	spp_strbuf_free(tmp_buff1);
-	spp_strbuf_free(tmp_buff2);
-	return ret;
-}
-
 /* append a list of status information for JSON format. */
 static int
 append_info_value(const char *name, char **output)
-- 
2.17.1


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

* [spp] [PATCH 09/13] shared/sec: move lcore funcs in response_info_list
  2019-06-24  4:36 [spp] [PATCH 00/13] Move JSON utils from libs for running cmds yasufum.o
                   ` (7 preceding siblings ...)
  2019-06-24  4:36 ` [spp] [PATCH 08/13] shared/sec: move rest of JSON formatters yasufum.o
@ 2019-06-24  4:36 ` yasufum.o
  2019-06-24  4:36 ` [spp] [PATCH 10/13] shared/sec: move ope cli-id " yasufum.o
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: yasufum.o @ 2019-06-24  4:36 UTC (permalink / raw)
  To: spp, ferruh.yigit, yasufum.o

From: Yasufumi Ogawa <yasufum.o@gmail.com>

This update is to move operation functions for making command response
which is defined in `cmd_runner.c` to `cmd_res_formatter.c`. Operation
functions are defined as following, and this update moves
`add_master_lcore` and `add_core`. The rest of operation functions are
moved in next patches.

    struct cmd_response response_info_list[] = {
            { "client-id", add_client_id },
            { "phy", add_interface },
            { "vhost", add_interface },
            { "ring", add_interface },
            { "master-lcore", add_master_lcore},
            { "core", add_core},
    #ifdef SPP_VF_MODULE
            { "classifier_table", add_classifier_table},
    #endif /* SPP_VF_MODULE */
            { "", NULL }
    };

Signed-off-by: Yasufumi Ogawa <yasufum.o@gmail.com>
---
 .../spp_worker_th/cmd_res_formatter.c         | 104 ++++++++++++++++++
 .../spp_worker_th/cmd_res_formatter.h         |   6 +
 .../secondary/spp_worker_th/cmd_runner.c      | 100 -----------------
 3 files changed, 110 insertions(+), 100 deletions(-)

diff --git a/src/shared/secondary/spp_worker_th/cmd_res_formatter.c b/src/shared/secondary/spp_worker_th/cmd_res_formatter.c
index b94cb29..d838a18 100644
--- a/src/shared/secondary/spp_worker_th/cmd_res_formatter.c
+++ b/src/shared/secondary/spp_worker_th/cmd_res_formatter.c
@@ -4,6 +4,8 @@
 
 #include "cmd_res_formatter.h"
 #include "cmd_utils.h"
+#include "vf_deps.h"
+#include "mirror_deps.h"
 #include "shared/secondary/json_helper.h"
 
 #define RTE_LOGTYPE_WK_CMD_RES_FMT RTE_LOGTYPE_USER1
@@ -533,3 +535,105 @@ append_command_results_value(const char *name, char **output,
 	return ret;
 }
 
+/* Iterate core information to create response to status command */
+static int
+spp_iterate_core_info(struct spp_iterate_core_params *params)
+{
+	int ret;
+	int lcore_id, cnt;
+	struct core_info *core = NULL;
+	struct sppwk_comp_info *comp_info_base = NULL;
+	struct sppwk_comp_info *comp_info = NULL;
+
+	RTE_LCORE_FOREACH_SLAVE(lcore_id) {
+		if (spp_get_core_status(lcore_id) == SPP_CORE_UNUSE)
+			continue;
+
+		core = get_core_info(lcore_id);
+		if (core->num == 0) {
+			ret = (*params->element_proc)(
+				params, lcore_id,
+				"", SPPWK_TYPE_NONE_STR,
+				0, NULL, 0, NULL);
+			if (unlikely(ret != 0)) {
+				RTE_LOG(ERR, WK_CMD_RES_FMT,
+						"Cannot iterate core "
+						"information. "
+						"(core = %d, type = %d)\n",
+						lcore_id, SPPWK_TYPE_NONE);
+				return SPP_RET_NG;
+			}
+			continue;
+		}
+
+		for (cnt = 0; cnt < core->num; cnt++) {
+			sppwk_get_mng_data(NULL, NULL, &comp_info_base,
+							NULL, NULL, NULL, NULL);
+			comp_info = (comp_info_base + core->id[cnt]);
+#ifdef SPP_VF_MODULE
+			if (comp_info->wk_type == SPPWK_TYPE_CLS) {
+				ret = get_classifier_status(lcore_id,
+						core->id[cnt], params);
+			} else {
+				ret = get_forwarder_status(lcore_id,
+						core->id[cnt], params);
+			}
+#endif /* SPP_VF_MODULE */
+#ifdef SPP_MIRROR_MODULE
+			ret = get_mirror_status(lcore_id, core->id[cnt],
+					params);
+#endif /* SPP_MIRROR_MODULE */
+			if (unlikely(ret != 0)) {
+				RTE_LOG(ERR, WK_CMD_RES_FMT,
+						"Cannot iterate core "
+						"information. "
+						"(core = %d, type = %d)\n",
+						lcore_id, comp_info->wk_type);
+				return SPP_RET_NG;
+			}
+		}
+	}
+
+	return SPP_RET_OK;
+}
+
+/* Add entry of master lcore to a response in JSON. */
+int
+add_master_lcore(const char *name, char **output,
+		void *tmp __attribute__ ((unused)))
+{
+	int ret = SPP_RET_NG;
+	ret = append_json_int_value(output, name, rte_get_master_lcore());
+	return ret;
+}
+
+/* Add entry of core info of worker to a response in JSON such as "core:0". */
+int
+add_core(const char *name, char **output,
+		void *tmp __attribute__ ((unused)))
+{
+	int ret = SPP_RET_NG;
+	struct spp_iterate_core_params itr_params;
+	char *tmp_buff = spp_strbuf_allocate(CMD_RES_BUF_INIT_SIZE);
+	if (unlikely(tmp_buff == NULL)) {
+		RTE_LOG(ERR, WK_CMD_RES_FMT,
+				/* TODO(yasufum) refactor no meaning err msg */
+				"allocate error. (name = %s)\n",
+				name);
+		return SPP_RET_NG;
+	}
+
+	itr_params.output = tmp_buff;
+	itr_params.element_proc = append_core_element_value;
+
+	ret = spp_iterate_core_info(&itr_params);
+	if (unlikely(ret != SPP_RET_OK)) {
+		spp_strbuf_free(itr_params.output);
+		return SPP_RET_NG;
+	}
+
+	ret = append_json_array_brackets(output, name, itr_params.output);
+	spp_strbuf_free(itr_params.output);
+	return ret;
+}
+
diff --git a/src/shared/secondary/spp_worker_th/cmd_res_formatter.h b/src/shared/secondary/spp_worker_th/cmd_res_formatter.h
index f3e9879..9c77763 100644
--- a/src/shared/secondary/spp_worker_th/cmd_res_formatter.h
+++ b/src/shared/secondary/spp_worker_th/cmd_res_formatter.h
@@ -72,4 +72,10 @@ int append_response_list_value(char **output, struct cmd_response *responses,
 int append_command_results_value(const char *name, char **output,
 		int num, struct cmd_result *results);
 
+int add_master_lcore(const char *name, char **output,
+		void *tmp __attribute__ ((unused)));
+
+int add_core(const char *name, char **output,
+		void *tmp __attribute__ ((unused)));
+
 #endif
diff --git a/src/shared/secondary/spp_worker_th/cmd_runner.c b/src/shared/secondary/spp_worker_th/cmd_runner.c
index 5644aec..a6894fc 100644
--- a/src/shared/secondary/spp_worker_th/cmd_runner.c
+++ b/src/shared/secondary/spp_worker_th/cmd_runner.c
@@ -418,66 +418,6 @@ flush_cmd(void)
 	return ret;
 }
 
-/* Iterate core information to create response to status command */
-static int
-spp_iterate_core_info(struct spp_iterate_core_params *params)
-{
-	int ret;
-	int lcore_id, cnt;
-	struct core_info *core = NULL;
-	struct sppwk_comp_info *comp_info_base = NULL;
-	struct sppwk_comp_info *comp_info = NULL;
-
-	RTE_LCORE_FOREACH_SLAVE(lcore_id) {
-		if (spp_get_core_status(lcore_id) == SPP_CORE_UNUSE)
-			continue;
-
-		core = get_core_info(lcore_id);
-		if (core->num == 0) {
-			ret = (*params->element_proc)(
-				params, lcore_id,
-				"", SPPWK_TYPE_NONE_STR,
-				0, NULL, 0, NULL);
-			if (unlikely(ret != 0)) {
-				RTE_LOG(ERR, WK_CMD_RUNNER, "Cannot iterate core "
-						"information. "
-						"(core = %d, type = %d)\n",
-						lcore_id, SPPWK_TYPE_NONE);
-				return SPP_RET_NG;
-			}
-			continue;
-		}
-
-		for (cnt = 0; cnt < core->num; cnt++) {
-			sppwk_get_mng_data(NULL, NULL, &comp_info_base,
-							NULL, NULL, NULL, NULL);
-			comp_info = (comp_info_base + core->id[cnt]);
-#ifdef SPP_VF_MODULE
-			if (comp_info->wk_type == SPPWK_TYPE_CLS) {
-				ret = get_classifier_status(lcore_id,
-						core->id[cnt], params);
-			} else {
-				ret = get_forwarder_status(lcore_id,
-						core->id[cnt], params);
-			}
-#endif /* SPP_VF_MODULE */
-#ifdef SPP_MIRROR_MODULE
-			ret = get_mirror_status(lcore_id, core->id[cnt],
-					params);
-#endif /* SPP_MIRROR_MODULE */
-			if (unlikely(ret != 0)) {
-				RTE_LOG(ERR, WK_CMD_RUNNER, "Cannot iterate core "
-						"information. "
-						"(core = %d, type = %d)\n",
-						lcore_id, comp_info->wk_type);
-				return SPP_RET_NG;
-			}
-		}
-	}
-
-	return SPP_RET_OK;
-}
-
 /* Iterate classifier_table to create response to status command */
 #ifdef SPP_VF_MODULE
 static int
@@ -684,46 +624,6 @@ add_interface(const char *name, char **output,
 	return ret;
 }
 
-/* Add entry of master lcore to a response in JSON. */
-static int
-add_master_lcore(const char *name, char **output,
-		void *tmp __attribute__ ((unused)))
-{
-	int ret = SPP_RET_NG;
-	ret = append_json_int_value(output, name, rte_get_master_lcore());
-	return ret;
-}
-
-/* Add entry of core info of worker to a response in JSON such as "core:0". */
-static int
-add_core(const char *name, char **output,
-		void *tmp __attribute__ ((unused)))
-{
-	int ret = SPP_RET_NG;
-	struct spp_iterate_core_params itr_params;
-	char *tmp_buff = spp_strbuf_allocate(CMD_RES_BUF_INIT_SIZE);
-	if (unlikely(tmp_buff == NULL)) {
-		RTE_LOG(ERR, WK_CMD_RUNNER,
-				/* TODO(yasufum) refactor no meaning err msg */
-				"allocate error. (name = %s)\n",
-				name);
-		return SPP_RET_NG;
-	}
-
-	itr_params.output = tmp_buff;
-	itr_params.element_proc = append_core_element_value;
-
-	ret = spp_iterate_core_info(&itr_params);
-	if (unlikely(ret != SPP_RET_OK)) {
-		spp_strbuf_free(itr_params.output);
-		return SPP_RET_NG;
-	}
-
-	ret = append_json_array_brackets(output, name, itr_params.output);
-	spp_strbuf_free(itr_params.output);
-	return ret;
-}
-
 #ifdef SPP_VF_MODULE
 /**
  * Add entries of classifier table in JSON. Before iterating the entries,
-- 
2.17.1


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

* [spp] [PATCH 10/13] shared/sec: move ope cli-id in response_info_list
  2019-06-24  4:36 [spp] [PATCH 00/13] Move JSON utils from libs for running cmds yasufum.o
                   ` (8 preceding siblings ...)
  2019-06-24  4:36 ` [spp] [PATCH 09/13] shared/sec: move lcore funcs in response_info_list yasufum.o
@ 2019-06-24  4:36 ` yasufum.o
  2019-06-24  4:36 ` [spp] [PATCH 11/13] shared/sec: move rest of ops " yasufum.o
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: yasufum.o @ 2019-06-24  4:36 UTC (permalink / raw)
  To: spp, ferruh.yigit, yasufum.o

From: Yasufumi Ogawa <yasufum.o@gmail.com>

This update is to move operation functions `add_client_id` and
`add_interface` which is defined in `cmd_runner.c` to
`cmd_res_formatter.c`.

Signed-off-by: Yasufumi Ogawa <yasufum.o@gmail.com>
---
 .../spp_worker_th/cmd_res_formatter.c         | 57 +++++++++++++++++++
 .../spp_worker_th/cmd_res_formatter.h         | 11 ++++
 .../secondary/spp_worker_th/cmd_runner.c      | 52 -----------------
 3 files changed, 68 insertions(+), 52 deletions(-)

diff --git a/src/shared/secondary/spp_worker_th/cmd_res_formatter.c b/src/shared/secondary/spp_worker_th/cmd_res_formatter.c
index d838a18..3476580 100644
--- a/src/shared/secondary/spp_worker_th/cmd_res_formatter.c
+++ b/src/shared/secondary/spp_worker_th/cmd_res_formatter.c
@@ -597,6 +597,63 @@ spp_iterate_core_info(struct spp_iterate_core_params *params)
 	return SPP_RET_OK;
 }
 
+/* TODO(yasufum) move to another file for util funcs. */
+/* Get client ID from global command params. */
+static int
+wk_get_client_id(void)
+{
+	struct startup_param *params;
+	sppwk_get_mng_data(&params, NULL, NULL, NULL, NULL, NULL, NULL);
+	return params->client_id;
+}
+
+/**
+ * Operator functions start with prefix `add_` defined in `response_info_list`
+ * of struct `cmd_response` which are for making each of parts of command
+ * response.
+ */
+/* Add entry of client ID to a response in JSON. */
+int
+add_client_id(const char *name, char **output,
+		void *tmp __attribute__ ((unused)))
+{
+	return append_json_int_value(output, name, wk_get_client_id());
+}
+
+/* Add entry of port to a response in JSON such as "phy:0". */
+int
+add_interface(const char *name, char **output,
+		void *tmp __attribute__ ((unused)))
+{
+	int ret = SPP_RET_NG;
+	char *tmp_buff = spp_strbuf_allocate(CMD_RES_BUF_INIT_SIZE);
+	if (unlikely(tmp_buff == NULL)) {
+		RTE_LOG(ERR, WK_CMD_RES_FMT,
+				/* TODO(yasufum) refactor no meaning err msg */
+				"allocate error. (name = %s)\n",
+				name);
+		return SPP_RET_NG;
+	}
+
+	if (strcmp(name, SPP_IFTYPE_NIC_STR) == 0)
+		ret = append_interface_array(&tmp_buff, PHY);
+
+	else if (strcmp(name, SPP_IFTYPE_VHOST_STR) == 0)
+		ret = append_interface_array(&tmp_buff, VHOST);
+
+	else if (strcmp(name, SPP_IFTYPE_RING_STR) == 0)
+		ret = append_interface_array(&tmp_buff, RING);
+
+	if (unlikely(ret < SPP_RET_OK)) {
+		spp_strbuf_free(tmp_buff);
+		return SPP_RET_NG;
+	}
+
+	ret = append_json_array_brackets(output, name, tmp_buff);
+	spp_strbuf_free(tmp_buff);
+	return ret;
+}
+
 /* Add entry of master lcore to a response in JSON. */
 int
 add_master_lcore(const char *name, char **output,
diff --git a/src/shared/secondary/spp_worker_th/cmd_res_formatter.h b/src/shared/secondary/spp_worker_th/cmd_res_formatter.h
index 9c77763..bc0109c 100644
--- a/src/shared/secondary/spp_worker_th/cmd_res_formatter.h
+++ b/src/shared/secondary/spp_worker_th/cmd_res_formatter.h
@@ -72,6 +72,17 @@ int append_response_list_value(char **output, struct cmd_response *responses,
 int append_command_results_value(const char *name, char **output,
 		int num, struct cmd_result *results);
 
+/**
+ * Operator functions start with prefix `add_` defined in `response_info_list`
+ * of struct `cmd_response` which are for making each of parts of command
+ * response.
+ */
+int add_client_id(const char *name, char **output,
+		void *tmp __attribute__ ((unused)));
+
+int add_interface(const char *name, char **output,
+		void *tmp __attribute__ ((unused)));
+
 int add_master_lcore(const char *name, char **output,
 		void *tmp __attribute__ ((unused)));
 
diff --git a/src/shared/secondary/spp_worker_th/cmd_runner.c b/src/shared/secondary/spp_worker_th/cmd_runner.c
index a6894fc..007d62e 100644
--- a/src/shared/secondary/spp_worker_th/cmd_runner.c
+++ b/src/shared/secondary/spp_worker_th/cmd_runner.c
@@ -29,16 +29,6 @@ enum cmd_res_code {
 	CMD_INVALID,
 };
 
-/* TODO(yasufum) move to another file for util funcs. */
-/* Get client ID from global command params. */
-static int
-sppwk_get_client_id(void)
-{
-	struct startup_param *params;
-	sppwk_get_mng_data(&params, NULL, NULL, NULL, NULL, NULL, NULL);
-	return params->client_id;
-}
-
 /* Update classifier table with given action, add or del. */
 static int
 update_cls_table(enum sppwk_action wk_action,
@@ -582,48 +572,6 @@ prepare_parse_err_msg(struct cmd_result *results,
 	}
 }
 
-/* Add entry of client ID to a response in JSON. */
-static int
-add_client_id(const char *name, char **output,
-		void *tmp __attribute__ ((unused)))
-{
-	return append_json_int_value(output, name, sppwk_get_client_id());
-}
-
-/* Add entry of port to a response in JSON such as "phy:0". */
-static int
-add_interface(const char *name, char **output,
-		void *tmp __attribute__ ((unused)))
-{
-	int ret = SPP_RET_NG;
-	char *tmp_buff = spp_strbuf_allocate(CMD_RES_BUF_INIT_SIZE);
-	if (unlikely(tmp_buff == NULL)) {
-		RTE_LOG(ERR, WK_CMD_RUNNER,
-				/* TODO(yasufum) refactor no meaning err msg */
-				"allocate error. (name = %s)\n",
-				name);
-		return SPP_RET_NG;
-	}
-
-	if (strcmp(name, SPP_IFTYPE_NIC_STR) == 0)
-		ret = append_interface_array(&tmp_buff, PHY);
-
-	else if (strcmp(name, SPP_IFTYPE_VHOST_STR) == 0)
-		ret = append_interface_array(&tmp_buff, VHOST);
-
-	else if (strcmp(name, SPP_IFTYPE_RING_STR) == 0)
-		ret = append_interface_array(&tmp_buff, RING);
-
-	if (unlikely(ret < SPP_RET_OK)) {
-		spp_strbuf_free(tmp_buff);
-		return SPP_RET_NG;
-	}
-
-	ret = append_json_array_brackets(output, name, tmp_buff);
-	spp_strbuf_free(tmp_buff);
-	return ret;
-}
-
 #ifdef SPP_VF_MODULE
 /**
  * Add entries of classifier table in JSON. Before iterating the entries,
-- 
2.17.1


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

* [spp] [PATCH 11/13] shared/sec: move rest of ops in response_info_list
  2019-06-24  4:36 [spp] [PATCH 00/13] Move JSON utils from libs for running cmds yasufum.o
                   ` (9 preceding siblings ...)
  2019-06-24  4:36 ` [spp] [PATCH 10/13] shared/sec: move ope cli-id " yasufum.o
@ 2019-06-24  4:36 ` yasufum.o
  2019-06-24  4:36 ` [spp] [PATCH 12/13] shared/sec: remove local funcs from header yasufum.o
  2019-06-24  4:36 ` [spp] [PATCH 13/13] shared/sec: refactor comments for JSON formatter yasufum.o
  12 siblings, 0 replies; 14+ messages in thread
From: yasufum.o @ 2019-06-24  4:36 UTC (permalink / raw)
  To: spp, ferruh.yigit, yasufum.o

From: Yasufumi Ogawa <yasufum.o@gmail.com>

This update is to move the rest of operation functions for making
command response which is defined in `cmd_runner.c` to
`cmd_res_formatter.c`.

Signed-off-by: Yasufumi Ogawa <yasufum.o@gmail.com>
---
 .../spp_worker_th/cmd_res_formatter.c         | 106 +++++++++++++++++
 .../spp_worker_th/cmd_res_formatter.h         |   4 +
 .../secondary/spp_worker_th/cmd_runner.c      | 108 ------------------
 3 files changed, 110 insertions(+), 108 deletions(-)

diff --git a/src/shared/secondary/spp_worker_th/cmd_res_formatter.c b/src/shared/secondary/spp_worker_th/cmd_res_formatter.c
index 3476580..ab08fcd 100644
--- a/src/shared/secondary/spp_worker_th/cmd_res_formatter.c
+++ b/src/shared/secondary/spp_worker_th/cmd_res_formatter.c
@@ -52,6 +52,31 @@ struct cmd_response response_result_list[] = {
 	{ "", NULL }
 };
 
+/**
+ * List of combination of tag and operator function. It is used to assemble
+ * a result of command in JSON like as following.
+ *
+ *     {
+ *         "client-id": 1,
+ *         "ports": ["phy:0", "phy:1", "vhost:0", "ring:0"],
+ *         "components": [
+ *             {
+ *                 "core": 2,
+ *                 ...
+ */
+struct cmd_response response_info_list[] = {
+	{ "client-id", add_client_id },
+	{ "phy", add_interface },
+	{ "vhost", add_interface },
+	{ "ring", add_interface },
+	{ "master-lcore", add_master_lcore},
+	{ "core", add_core},
+#ifdef SPP_VF_MODULE
+	{ "classifier_table", add_classifier_table},
+#endif /* SPP_VF_MODULE */
+	{ "", NULL }
+};
+
 /* append a command result for JSON format */
 int
 append_result_value(const char *name, char **output, void *tmp)
@@ -535,6 +560,32 @@ append_command_results_value(const char *name, char **output,
 	return ret;
 }
 
+/* append a list of status information for JSON format. */
+int
+append_info_value(const char *name, char **output)
+{
+	int ret = SPP_RET_NG;
+	char *tmp_buff = spp_strbuf_allocate(CMD_RES_BUF_INIT_SIZE);
+	if (unlikely(tmp_buff == NULL)) {
+		RTE_LOG(ERR, WK_CMD_RES_FMT,
+				/* TODO(yasufum) refactor no meaning err msg */
+				"allocate error. (name = %s)\n",
+				name);
+		return SPP_RET_NG;
+	}
+
+	ret = append_response_list_value(&tmp_buff,
+			response_info_list, NULL);
+	if (unlikely(ret < SPP_RET_OK)) {
+		spp_strbuf_free(tmp_buff);
+		return SPP_RET_NG;
+	}
+
+	ret = append_json_block_brackets(output, name, tmp_buff);
+	spp_strbuf_free(tmp_buff);
+	return ret;
+}
+
 /* Iterate core information to create response to status command */
 static int
 spp_iterate_core_info(struct spp_iterate_core_params *params)
@@ -694,3 +745,58 @@ add_core(const char *name, char **output,
 	return ret;
 }
 
+#ifdef SPP_VF_MODULE
+/* Iterate classifier_table to create response to status command */
+static int
+_add_classifier_table(
+		struct spp_iterate_classifier_table_params *params)
+{
+	int ret;
+
+	ret = add_classifier_table_val(params);
+	if (unlikely(ret != 0)) {
+		RTE_LOG(ERR, WK_CMD_RES_FMT,
+				"Cannot iterate classifier_mac_table.\n");
+		return SPP_RET_NG;
+	}
+
+	return SPP_RET_OK;
+}
+
+/**
+ * Add entries of classifier table in JSON. Before iterating the entries,
+ * this function calls several nested functions.
+ *   add_classifier_table()  // This function.
+ *     -> _add_classifier_table()  // Wrapper and doesn't almost nothing.
+ *       -> add_classifier_table_val()  // Setup data and call iterator.
+ *         -> iterate_adding_mac_entry()
+ */
+int
+add_classifier_table(const char *name, char **output,
+		void *tmp __attribute__ ((unused)))
+{
+	int ret = SPP_RET_NG;
+	struct spp_iterate_classifier_table_params itr_params;
+	char *tmp_buff = spp_strbuf_allocate(CMD_RES_BUF_INIT_SIZE);
+	if (unlikely(tmp_buff == NULL)) {
+		RTE_LOG(ERR, WK_CMD_RES_FMT,
+				/* TODO(yasufum) refactor no meaning err msg */
+				"allocate error. (name = %s)\n",
+				name);
+		return SPP_RET_NG;
+	}
+
+	itr_params.output = tmp_buff;
+	itr_params.element_proc = append_classifier_element_value;
+
+	ret = _add_classifier_table(&itr_params);
+	if (unlikely(ret != SPP_RET_OK)) {
+		spp_strbuf_free(itr_params.output);
+		return SPP_RET_NG;
+	}
+
+	ret = append_json_array_brackets(output, name, itr_params.output);
+	spp_strbuf_free(itr_params.output);
+	return ret;
+}
+#endif /* SPP_VF_MODULE */
diff --git a/src/shared/secondary/spp_worker_th/cmd_res_formatter.h b/src/shared/secondary/spp_worker_th/cmd_res_formatter.h
index bc0109c..d9481e3 100644
--- a/src/shared/secondary/spp_worker_th/cmd_res_formatter.h
+++ b/src/shared/secondary/spp_worker_th/cmd_res_formatter.h
@@ -72,6 +72,8 @@ int append_response_list_value(char **output, struct cmd_response *responses,
 int append_command_results_value(const char *name, char **output,
 		int num, struct cmd_result *results);
 
+int append_info_value(const char *name, char **output);
+
 /**
  * Operator functions start with prefix `add_` defined in `response_info_list`
  * of struct `cmd_response` which are for making each of parts of command
@@ -89,4 +91,6 @@ int add_master_lcore(const char *name, char **output,
 int add_core(const char *name, char **output,
 		void *tmp __attribute__ ((unused)));
 
+int add_classifier_table(const char *name, char **output,
+		void *tmp __attribute__ ((unused)));
 #endif
diff --git a/src/shared/secondary/spp_worker_th/cmd_runner.c b/src/shared/secondary/spp_worker_th/cmd_runner.c
index 007d62e..7c4c91c 100644
--- a/src/shared/secondary/spp_worker_th/cmd_runner.c
+++ b/src/shared/secondary/spp_worker_th/cmd_runner.c
@@ -408,24 +408,6 @@ flush_cmd(void)
 	return ret;
 }
 
-/* Iterate classifier_table to create response to status command */
-#ifdef SPP_VF_MODULE
-static int
-_add_classifier_table(
-		struct spp_iterate_classifier_table_params *params)
-{
-	int ret;
-
-	ret = add_classifier_table_val(params);
-	if (unlikely(ret != 0)) {
-		RTE_LOG(ERR, WK_CMD_RUNNER, "Cannot iterate classifier_mac_table.\n");
-		return SPP_RET_NG;
-	}
-
-	return SPP_RET_OK;
-}
-#endif /* SPP_VF_MODULE */
-
 /* Execute one command. */
 static int
 exec_one_cmd(const struct sppwk_cmd_attrs *cmd)
@@ -572,96 +554,6 @@ prepare_parse_err_msg(struct cmd_result *results,
 	}
 }
 
-#ifdef SPP_VF_MODULE
-/**
- * Add entries of classifier table in JSON. Before iterating the entries,
- * this function calls several nested functions.
- *   add_classifier_table()  // This function.
- *     -> _add_classifier_table()  // Wrapper and doesn't almost nothing.
- *       -> add_classifier_table_val()  // Setup data and call iterator.
- *         -> iterate_adding_mac_entry()
- */
-static int
-add_classifier_table(const char *name, char **output,
-		void *tmp __attribute__ ((unused)))
-{
-	int ret = SPP_RET_NG;
-	struct spp_iterate_classifier_table_params itr_params;
-	char *tmp_buff = spp_strbuf_allocate(CMD_RES_BUF_INIT_SIZE);
-	if (unlikely(tmp_buff == NULL)) {
-		RTE_LOG(ERR, WK_CMD_RUNNER,
-				/* TODO(yasufum) refactor no meaning err msg */
-				"allocate error. (name = %s)\n",
-				name);
-		return SPP_RET_NG;
-	}
-
-	itr_params.output = tmp_buff;
-	itr_params.element_proc = append_classifier_element_value;
-
-	ret = _add_classifier_table(&itr_params);
-	if (unlikely(ret != SPP_RET_OK)) {
-		spp_strbuf_free(itr_params.output);
-		return SPP_RET_NG;
-	}
-
-	ret = append_json_array_brackets(output, name, itr_params.output);
-	spp_strbuf_free(itr_params.output);
-	return ret;
-}
-#endif /* SPP_VF_MODULE */
-
-/**
- * List of combination of tag and operator function. It is used to assemble
- * a result of command in JSON like as following.
- *
- *     {
- *         "client-id": 1,
- *         "ports": ["phy:0", "phy:1", "vhost:0", "ring:0"],
- *         "components": [
- *             {
- *                 "core": 2,
- *                 ...
- */
-struct cmd_response response_info_list[] = {
-	{ "client-id", add_client_id },
-	{ "phy", add_interface },
-	{ "vhost", add_interface },
-	{ "ring", add_interface },
-	{ "master-lcore", add_master_lcore},
-	{ "core", add_core},
-#ifdef SPP_VF_MODULE
-	{ "classifier_table", add_classifier_table},
-#endif /* SPP_VF_MODULE */
-	{ "", NULL }
-};
-
-/* append a list of status information for JSON format. */
-static int
-append_info_value(const char *name, char **output)
-{
-	int ret = SPP_RET_NG;
-	char *tmp_buff = spp_strbuf_allocate(CMD_RES_BUF_INIT_SIZE);
-	if (unlikely(tmp_buff == NULL)) {
-		RTE_LOG(ERR, WK_CMD_RUNNER,
-				/* TODO(yasufum) refactor no meaning err msg */
-				"allocate error. (name = %s)\n",
-				name);
-		return SPP_RET_NG;
-	}
-
-	ret = append_response_list_value(&tmp_buff,
-			response_info_list, NULL);
-	if (unlikely(ret < SPP_RET_OK)) {
-		spp_strbuf_free(tmp_buff);
-		return SPP_RET_NG;
-	}
-
-	ret = append_json_block_brackets(output, name, tmp_buff);
-	spp_strbuf_free(tmp_buff);
-	return ret;
-}
-
 /* send response for decode error */
 static void
 send_decode_error_response(int *sock,
-- 
2.17.1


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

* [spp] [PATCH 12/13] shared/sec: remove local funcs from header
  2019-06-24  4:36 [spp] [PATCH 00/13] Move JSON utils from libs for running cmds yasufum.o
                   ` (10 preceding siblings ...)
  2019-06-24  4:36 ` [spp] [PATCH 11/13] shared/sec: move rest of ops " yasufum.o
@ 2019-06-24  4:36 ` yasufum.o
  2019-06-24  4:36 ` [spp] [PATCH 13/13] shared/sec: refactor comments for JSON formatter yasufum.o
  12 siblings, 0 replies; 14+ messages in thread
From: yasufum.o @ 2019-06-24  4:36 UTC (permalink / raw)
  To: spp, ferruh.yigit, yasufum.o

From: Yasufumi Ogawa <yasufum.o@gmail.com>

Some of functions start with `add_` defined in `cmd_res_formatter.h` are
not used from outside. This update is to remove these functions from
header file without add_client_id() and add_classifier_table().

Signed-off-by: Yasufumi Ogawa <yasufum.o@gmail.com>
---
 .../spp_worker_th/cmd_res_formatter.c         | 21 ++++++++++++++-----
 .../spp_worker_th/cmd_res_formatter.h         | 13 ------------
 2 files changed, 16 insertions(+), 18 deletions(-)

diff --git a/src/shared/secondary/spp_worker_th/cmd_res_formatter.c b/src/shared/secondary/spp_worker_th/cmd_res_formatter.c
index ab08fcd..24b5608 100644
--- a/src/shared/secondary/spp_worker_th/cmd_res_formatter.c
+++ b/src/shared/secondary/spp_worker_th/cmd_res_formatter.c
@@ -10,6 +10,17 @@
 
 #define RTE_LOGTYPE_WK_CMD_RES_FMT RTE_LOGTYPE_USER1
 
+/* Proto type declaration for a list of operator functions. */
+static int append_result_value(const char *name, char **output, void *tmp);
+static int append_error_details_value(const char *name, char **output,
+		void *tmp);
+static int add_interface(const char *name, char **output,
+		void *tmp __attribute__ ((unused)));
+static int add_master_lcore(const char *name, char **output,
+		void *tmp __attribute__ ((unused)));
+static int add_core(const char *name, char **output,
+		void *tmp __attribute__ ((unused)));
+
 /**
  * List of worker process type. The order of items should be same as the order
  * of enum `secondary_type` in cmd_utils.h.
@@ -78,7 +89,7 @@ struct cmd_response response_info_list[] = {
 };
 
 /* append a command result for JSON format */
-int
+static int
 append_result_value(const char *name, char **output, void *tmp)
 {
 	const struct cmd_result *result = tmp;
@@ -86,7 +97,7 @@ append_result_value(const char *name, char **output, void *tmp)
 }
 
 /* append error details for JSON format */
-int
+static int
 append_error_details_value(const char *name, char **output, void *tmp)
 {
 	int ret = SPP_RET_NG;
@@ -672,7 +683,7 @@ add_client_id(const char *name, char **output,
 }
 
 /* Add entry of port to a response in JSON such as "phy:0". */
-int
+static int
 add_interface(const char *name, char **output,
 		void *tmp __attribute__ ((unused)))
 {
@@ -706,7 +717,7 @@ add_interface(const char *name, char **output,
 }
 
 /* Add entry of master lcore to a response in JSON. */
-int
+static int
 add_master_lcore(const char *name, char **output,
 		void *tmp __attribute__ ((unused)))
 {
@@ -716,7 +727,7 @@ add_master_lcore(const char *name, char **output,
 }
 
 /* Add entry of core info of worker to a response in JSON such as "core:0". */
-int
+static int
 add_core(const char *name, char **output,
 		void *tmp __attribute__ ((unused)))
 {
diff --git a/src/shared/secondary/spp_worker_th/cmd_res_formatter.h b/src/shared/secondary/spp_worker_th/cmd_res_formatter.h
index d9481e3..cf5f81b 100644
--- a/src/shared/secondary/spp_worker_th/cmd_res_formatter.h
+++ b/src/shared/secondary/spp_worker_th/cmd_res_formatter.h
@@ -32,10 +32,6 @@ struct cmd_response {
 	int (*func)(const char *name, char **output, void *tmp);
 };
 
-int append_result_value(const char *name, char **output, void *tmp);
-
-int append_error_details_value(const char *name, char **output, void *tmp);
-
 int append_interface_array(char **output, const enum port_type type);
 
 int append_process_type_value(const char *name, char **output,
@@ -82,15 +78,6 @@ int append_info_value(const char *name, char **output);
 int add_client_id(const char *name, char **output,
 		void *tmp __attribute__ ((unused)));
 
-int add_interface(const char *name, char **output,
-		void *tmp __attribute__ ((unused)));
-
-int add_master_lcore(const char *name, char **output,
-		void *tmp __attribute__ ((unused)));
-
-int add_core(const char *name, char **output,
-		void *tmp __attribute__ ((unused)));
-
 int add_classifier_table(const char *name, char **output,
 		void *tmp __attribute__ ((unused)));
 #endif
-- 
2.17.1


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

* [spp] [PATCH 13/13] shared/sec: refactor comments for JSON formatter
  2019-06-24  4:36 [spp] [PATCH 00/13] Move JSON utils from libs for running cmds yasufum.o
                   ` (11 preceding siblings ...)
  2019-06-24  4:36 ` [spp] [PATCH 12/13] shared/sec: remove local funcs from header yasufum.o
@ 2019-06-24  4:36 ` yasufum.o
  12 siblings, 0 replies; 14+ messages in thread
From: yasufum.o @ 2019-06-24  4:36 UTC (permalink / raw)
  To: spp, ferruh.yigit, yasufum.o

From: Yasufumi Ogawa <yasufum.o@gmail.com>

This update is to refactor comments for JSON formatter.

Signed-off-by: Yasufumi Ogawa <yasufum.o@gmail.com>
---
 .../spp_worker_th/cmd_res_formatter.c         | 68 ++++++++++++-------
 1 file changed, 43 insertions(+), 25 deletions(-)

diff --git a/src/shared/secondary/spp_worker_th/cmd_res_formatter.c b/src/shared/secondary/spp_worker_th/cmd_res_formatter.c
index 24b5608..21024b9 100644
--- a/src/shared/secondary/spp_worker_th/cmd_res_formatter.c
+++ b/src/shared/secondary/spp_worker_th/cmd_res_formatter.c
@@ -88,7 +88,7 @@ struct cmd_response response_info_list[] = {
 	{ "", NULL }
 };
 
-/* append a command result for JSON format */
+/* Append a command result in JSON format. */
 static int
 append_result_value(const char *name, char **output, void *tmp)
 {
@@ -96,7 +96,7 @@ append_result_value(const char *name, char **output, void *tmp)
 	return append_json_str_value(output, name, result->result);
 }
 
-/* append error details for JSON format */
+/* Append error details in JSON format. */
 static int
 append_error_details_value(const char *name, char **output, void *tmp)
 {
@@ -110,9 +110,7 @@ append_error_details_value(const char *name, char **output, void *tmp)
 	tmp_buff = spp_strbuf_allocate(CMD_RES_BUF_INIT_SIZE);
 	if (unlikely(tmp_buff == NULL)) {
 		RTE_LOG(ERR, WK_CMD_RES_FMT,
-				/* TODO(yasufum) refactor no meaning err msg */
-				"allocate error. (name = %s)\n",
-				name);
+				"Fail to alloc buf for `%s`.\n", name);
 		return SPP_RET_NG;
 	}
 
@@ -135,7 +133,7 @@ is_port_flushed(enum port_type iface_type, int iface_no)
 	return port->ethdev_port_id >= 0;
 }
 
-/* append a list of interface numbers */
+/* Append index number as comma separated format such as `0, 1, ...`. */
 int
 append_interface_array(char **output, const enum port_type type)
 {
@@ -151,14 +149,12 @@ append_interface_array(char **output, const enum port_type type)
 		*output = spp_strbuf_append(*output, tmp_str, strlen(tmp_str));
 		if (unlikely(*output == NULL)) {
 			RTE_LOG(ERR, WK_CMD_RES_FMT,
-					"Interface number failed to add. "
-					"(type = %d)\n", type);
+				/* TODO(yasufum) replace %d to string. */
+				"Failed to add index for type `%d`.\n", type);
 			return SPP_RET_NG;
 		}
-
 		port_cnt++;
 	}
-
 	return SPP_RET_OK;
 }
 
@@ -519,7 +515,10 @@ append_response_list_value(char **output, struct cmd_response *responses,
 	return SPP_RET_OK;
 }
 
-/* append a list of command results for JSON format. */
+/**
+ * Setup `results` section in JSON msg. This is an example.
+ *   "results": [ { "result": "success" } ]
+ */
 int
 append_command_results_value(const char *name, char **output,
 		int num, struct cmd_result *results)
@@ -527,27 +526,26 @@ append_command_results_value(const char *name, char **output,
 	int ret = SPP_RET_NG;
 	int i;
 	char *tmp_buff1, *tmp_buff2;
+
+	/* Setup result statement step by step with two buffers. */
 	tmp_buff1 = spp_strbuf_allocate(CMD_RES_BUF_INIT_SIZE);
 	if (unlikely(tmp_buff1 == NULL)) {
 		RTE_LOG(ERR, WK_CMD_RES_FMT,
-				/* TODO(yasufum) refactor no meaning err msg */
-				"allocate error. (name = %s, buff=1)\n",
-				name);
+				"Faield to alloc 1st buf for `%s`.\n", name);
 		return SPP_RET_NG;
 	}
-
 	tmp_buff2 = spp_strbuf_allocate(CMD_RES_BUF_INIT_SIZE);
 	if (unlikely(tmp_buff2 == NULL)) {
 		spp_strbuf_free(tmp_buff1);
 		RTE_LOG(ERR, WK_CMD_RES_FMT,
-				/* TODO(yasufum) refactor no meaning err msg */
-				"allocate error. (name = %s, buff=2)\n",
-				name);
+				"Faield to alloc 2nd buf for `%s`.\n", name);
 		return SPP_RET_NG;
 	}
 
 	for (i = 0; i < num; i++) {
 		tmp_buff1[0] = '\0';
+
+		/* Setup key-val pair such as `"result": "success"` */
 		ret = append_response_list_value(&tmp_buff1,
 				response_result_list, &results[i]);
 		if (unlikely(ret < 0)) {
@@ -556,22 +554,41 @@ append_command_results_value(const char *name, char **output,
 			return SPP_RET_NG;
 		}
 
+		/* Surround key-val pair such as `{ "result": "success" }`. */
 		ret = append_json_block_brackets(&tmp_buff2, "", tmp_buff1);
 		if (unlikely(ret < 0)) {
 			spp_strbuf_free(tmp_buff1);
 			spp_strbuf_free(tmp_buff2);
 			return SPP_RET_NG;
 		}
-
 	}
 
+	/**
+	 * Setup result statement such as
+	 * `"results": [ { "result": "success" } ]`.
+	 */
 	ret = append_json_array_brackets(output, name, tmp_buff2);
+
 	spp_strbuf_free(tmp_buff1);
 	spp_strbuf_free(tmp_buff2);
 	return ret;
 }
 
-/* append a list of status information for JSON format. */
+/**
+ * Setup response of `status` command.
+ *
+ * This is an example of the response.
+ *   "results": [ { "result": "success" } ],
+ *   "info": {
+ *       "client-id": 2,
+ *       "phy": [ 0, 1 ], "vhost": [  ], "ring": [  ],
+ *       "master-lcore": 1,
+ *       "core": [
+ *           {"core": 2, "type": "unuse"}, {"core": 3, "type": "unuse"}, ...
+ *       ],
+ *       "classifier_table": [  ]
+ *   }
+ */
 int
 append_info_value(const char *name, char **output)
 {
@@ -579,19 +596,19 @@ append_info_value(const char *name, char **output)
 	char *tmp_buff = spp_strbuf_allocate(CMD_RES_BUF_INIT_SIZE);
 	if (unlikely(tmp_buff == NULL)) {
 		RTE_LOG(ERR, WK_CMD_RES_FMT,
-				/* TODO(yasufum) refactor no meaning err msg */
-				"allocate error. (name = %s)\n",
+				"Failed to get empty buf for append `%s`.\n",
 				name);
 		return SPP_RET_NG;
 	}
 
-	ret = append_response_list_value(&tmp_buff,
-			response_info_list, NULL);
+	/* Setup JSON msg in value of `info` key. */
+	ret = append_response_list_value(&tmp_buff, response_info_list, NULL);
 	if (unlikely(ret < SPP_RET_OK)) {
 		spp_strbuf_free(tmp_buff);
 		return SPP_RET_NG;
 	}
 
+	/* Setup response of JSON msg. */
 	ret = append_json_block_brackets(output, name, tmp_buff);
 	spp_strbuf_free(tmp_buff);
 	return ret;
@@ -674,7 +691,8 @@ wk_get_client_id(void)
  * of struct `cmd_response` which are for making each of parts of command
  * response.
  */
-/* Add entry of client ID to a response in JSON. */
+
+/* Add entry of client ID such as `"client-id": 1` to a response in JSON. */
 int
 add_client_id(const char *name, char **output,
 		void *tmp __attribute__ ((unused)))
-- 
2.17.1


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

end of thread, other threads:[~2019-06-24  4:37 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-24  4:36 [spp] [PATCH 00/13] Move JSON utils from libs for running cmds yasufum.o
2019-06-24  4:36 ` [spp] [PATCH 01/13] shared/sec: rename ops for setup cmd response yasufum.o
2019-06-24  4:36 ` [spp] [PATCH 02/13] shared/sec: rename functions for spp_mirror yasufum.o
2019-06-24  4:36 ` [spp] [PATCH 03/13] shared/sec: move principle JSON formatter funcs yasufum.o
2019-06-24  4:36 ` [spp] [PATCH 04/13] shared/sec: change order of args of JSON fmtters yasufum.o
2019-06-24  4:36 ` [spp] [PATCH 05/13] shared/sec: move JSON formatter to shard/secondary yasufum.o
2019-06-24  4:36 ` [spp] [PATCH 06/13] shared/sec: revise including headers yasufum.o
2019-06-24  4:36 ` [spp] [PATCH 07/13] shared/sec: move JSON formatters from cmd_runner yasufum.o
2019-06-24  4:36 ` [spp] [PATCH 08/13] shared/sec: move rest of JSON formatters yasufum.o
2019-06-24  4:36 ` [spp] [PATCH 09/13] shared/sec: move lcore funcs in response_info_list yasufum.o
2019-06-24  4:36 ` [spp] [PATCH 10/13] shared/sec: move ope cli-id " yasufum.o
2019-06-24  4:36 ` [spp] [PATCH 11/13] shared/sec: move rest of ops " yasufum.o
2019-06-24  4:36 ` [spp] [PATCH 12/13] shared/sec: remove local funcs from header yasufum.o
2019-06-24  4:36 ` [spp] [PATCH 13/13] shared/sec: refactor comments for JSON formatter yasufum.o

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).