Soft Patch Panel
 help / color / mirror / Atom feed
* [spp] [PATCH 0/5] Refactor source of cmd parser port utils
@ 2019-05-10  8:35 ogawa.yasufumi
  2019-05-10  8:35 ` [spp] [PATCH 1/5] shared/sec: rename src command_dec to cmd_parser ogawa.yasufumi
                   ` (5 more replies)
  0 siblings, 6 replies; 12+ messages in thread
From: ogawa.yasufumi @ 2019-05-10  8:35 UTC (permalink / raw)
  To: spp, ferruh.yigit, ogawa.yasufumi

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

This series of update is to change the name `command_dec.c` to
`cmd_parser.c` and refactor vars and functions mainly for port
management defined in the file.

Yasufumi Ogawa (5):
  shared/sec: rename src command_dec to cmd_parser
  shared/sec: remove no meaning str defines
  shared/sec: rename lists of fixed strings
  shared/sec: rename struct for attrs of classify
  shared/sec: rename sppwk port util functions

 src/mirror/Makefile                           |   2 +-
 .../{command_dec.c => cmd_parser.c}           | 234 +++++++-----------
 .../secondary/spp_worker_th/cmd_parser.h      |   6 +-
 .../secondary/spp_worker_th/command_proc.c    |  75 +++---
 src/shared/secondary/spp_worker_th/spp_proc.c |  33 ++-
 src/shared/secondary/spp_worker_th/spp_proc.h |  28 +--
 src/vf/Makefile                               |   2 +-
 src/vf/classifier_mac.c                       |   8 +-
 8 files changed, 165 insertions(+), 223 deletions(-)
 rename src/shared/secondary/spp_worker_th/{command_dec.c => cmd_parser.c} (81%)

-- 
2.17.1


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

* [spp] [PATCH 1/5] shared/sec: rename src command_dec to cmd_parser
  2019-05-10  8:35 [spp] [PATCH 0/5] Refactor source of cmd parser port utils ogawa.yasufumi
@ 2019-05-10  8:35 ` ogawa.yasufumi
  2019-05-10  8:35 ` [spp] [PATCH 2/5] shared/sec: remove no meaning str defines ogawa.yasufumi
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: ogawa.yasufumi @ 2019-05-10  8:35 UTC (permalink / raw)
  To: spp, ferruh.yigit, ogawa.yasufumi

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

For refactoring, rename source file `command_dec.c` to `cmd_parser.c`
because it defines vars and functions for parsing command, not decoding.

Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
 src/mirror/Makefile                                             | 2 +-
 .../secondary/spp_worker_th/{command_dec.c => cmd_parser.c}     | 0
 src/shared/secondary/spp_worker_th/cmd_parser.h                 | 2 +-
 src/vf/Makefile                                                 | 2 +-
 4 files changed, 3 insertions(+), 3 deletions(-)
 rename src/shared/secondary/spp_worker_th/{command_dec.c => cmd_parser.c} (100%)

diff --git a/src/mirror/Makefile b/src/mirror/Makefile
index be58e3a..3104059 100644
--- a/src/mirror/Makefile
+++ b/src/mirror/Makefile
@@ -20,7 +20,7 @@ SRCS-y += ../shared/secondary/utils.c ../shared/secondary/add_port.c
 SRCS-y += $(SPP_WKT_DIR)/spp_proc.c
 SRCS-y += $(SPP_WKT_DIR)/spp_port.c
 SRCS-y += $(SPP_WKT_DIR)/command_conn.c
-SRCS-y += $(SPP_WKT_DIR)/command_dec.c
+SRCS-y += $(SPP_WKT_DIR)/cmd_parser.c
 SRCS-y += $(SPP_WKT_DIR)/command_proc.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/command_dec.c b/src/shared/secondary/spp_worker_th/cmd_parser.c
similarity index 100%
rename from src/shared/secondary/spp_worker_th/command_dec.c
rename to src/shared/secondary/spp_worker_th/cmd_parser.c
diff --git a/src/shared/secondary/spp_worker_th/cmd_parser.h b/src/shared/secondary/spp_worker_th/cmd_parser.h
index 785fffe..b03a920 100644
--- a/src/shared/secondary/spp_worker_th/cmd_parser.h
+++ b/src/shared/secondary/spp_worker_th/cmd_parser.h
@@ -55,7 +55,7 @@ enum sppwk_action {
  * SPP command type.
  *
  * @attention This enumerated type must have the same order of command_list
- *            defined in command_dec.c
+ *            defined in cmd_parser.c
  */
 /*
  * TODO(yasufum) consider to divide because each of target of scope is
diff --git a/src/vf/Makefile b/src/vf/Makefile
index a8b60fb..8c1cb2b 100644
--- a/src/vf/Makefile
+++ b/src/vf/Makefile
@@ -19,7 +19,7 @@ SRCS-y += $(SPP_WKT_DIR)/string_buffer.c
 SRCS-y += $(SPP_WKT_DIR)/ringlatencystats.c
 SRCS-y += $(SPP_WKT_DIR)/spp_port.c
 SRCS-y += $(SPP_WKT_DIR)/command_conn.c
-SRCS-y += $(SPP_WKT_DIR)/command_dec.c
+SRCS-y += $(SPP_WKT_DIR)/cmd_parser.c
 SRCS-y += $(SPP_WKT_DIR)/command_proc.c
 SRCS-y += $(SPP_WKT_DIR)/spp_proc.c
 SRCS-y += ../shared/common.c
-- 
2.17.1


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

* [spp] [PATCH 2/5] shared/sec: remove no meaning str defines
  2019-05-10  8:35 [spp] [PATCH 0/5] Refactor source of cmd parser port utils ogawa.yasufumi
  2019-05-10  8:35 ` [spp] [PATCH 1/5] shared/sec: rename src command_dec to cmd_parser ogawa.yasufumi
@ 2019-05-10  8:35 ` ogawa.yasufumi
  2019-05-10  8:35 ` [spp] [PATCH 3/5] shared/sec: rename lists of fixed strings ogawa.yasufumi
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: ogawa.yasufumi @ 2019-05-10  8:35 UTC (permalink / raw)
  To: spp, ferruh.yigit, ogawa.yasufumi

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

Defines of fixed string, such as `#define SPP_COMMAND_EXIT "exit"`, are
no meaning because this long define is referred just one time. In
addition, there are other similar defines which are actually required.
It makes code complex and hard to maintain. This update is to remove
such kind of defines.

Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
 .../secondary/spp_worker_th/cmd_parser.c      | 97 ++++++-------------
 1 file changed, 29 insertions(+), 68 deletions(-)

diff --git a/src/shared/secondary/spp_worker_th/cmd_parser.c b/src/shared/secondary/spp_worker_th/cmd_parser.c
index 723d8c4..8efbfdb 100644
--- a/src/shared/secondary/spp_worker_th/cmd_parser.c
+++ b/src/shared/secondary/spp_worker_th/cmd_parser.c
@@ -14,46 +14,15 @@
 #define RTE_LOGTYPE_SPP_COMMAND_PROC RTE_LOGTYPE_USER1
 #define RTE_LOGTYPE_APP RTE_LOGTYPE_USER2
 
-/* command string  */
-#define SPP_COMMAND_CLASSFIER_TABLE_STR "classifier_table"
-#define SPP_COMMAND_GET_CLIENT_ID_STR   "_get_client_id"
-#define SPP_COMMAND_STATUS_STR          "status"
-#define SPP_COMMAND_EXIT_STR            "exit"
-#define SPP_COMMAND_COMPONENT_STR       "component"
-#define SPP_COMMAND_PORT_STR            "port"
-
-/* classifiler_type string */
-#define SPP_CLASSIFLER_NONE_STR         "none"
-#define SPP_CLASSIFLER_MAC_STR          "mac"
-#define SPP_CLASSIFLER_VLAN_STR         "vlan"
-
-/* command action string */
-#define SPP_ACTION_NONE_STR             "none"
-#define SPP_ACTION_START_STR            "start"
-#define SPP_ACTION_STOP_STR             "stop"
-#define SPP_ACTION_ADD_STR              "add"
-#define SPP_ACTION_DEL_STR              "del"
-
-/* port rx/tx string */
-#define SPP_PORT_RXTX_NONE_STR          "none"
-#define SPP_PORT_RXTX_RX_STR            "rx"
-#define SPP_PORT_RXTX_TX_STR            "tx"
-
-/* port ability string */
-#define SPP_ABILITY_NONE_STR            "none"
-#define SPP_ABILITY_ADD_VLANTAG_STR     "add_vlantag"
-#define SPP_ABILITY_DEL_VLANTAG_STR     "del_vlantag"
-
 /*
- * classifier type string list
- * do it same as the order of enum spp_classifier_type (spp_proc.h)
+ * List of classifier type. The order of items should be same as the order of
+ * enum `spp_classifier_type` defined in spp_proc.h.
  */
 const char *CLASSIFILER_TYPE_STRINGS[] = {
-	SPP_CLASSIFLER_NONE_STR,
-	SPP_CLASSIFLER_MAC_STR,
-	SPP_CLASSIFLER_VLAN_STR,
-
-	/* termination */ "",
+	"none",
+	"mac",
+	"vlan",
+	"",  /* termination */
 };
 
 /**
@@ -61,24 +30,23 @@ const char *CLASSIFILER_TYPE_STRINGS[] = {
  * enum `sppwk_action` in cmd_parser.h.
  */
 const char *COMMAND_ACTION_STRINGS[] = {
-	SPP_ACTION_NONE_STR,
-	SPP_ACTION_START_STR,
-	SPP_ACTION_STOP_STR,
-	SPP_ACTION_ADD_STR,
-	SPP_ACTION_DEL_STR,
+	"none",
+	"start",
+	"stop",
+	"add",
+	"del",
 	"",  /* termination */
 };
 
 /*
- * port rxtx string list
- * do it same as the order of enum spp_port_rxtx (spp_vf.h)
+ * List of port type. The order of items should be same as the order of
+ * enum `spp_port_rxtx` in spp_vf.h.
  */
 const char *PORT_RXTX_STRINGS[] = {
-	SPP_PORT_RXTX_NONE_STR,
-	SPP_PORT_RXTX_RX_STR,
-	SPP_PORT_RXTX_TX_STR,
-
-	/* termination */ "",
+	"none",
+	"rx",
+	"tx",
+	"",  /* termination */
 };
 
 /*
@@ -86,11 +54,10 @@ const char *PORT_RXTX_STRINGS[] = {
  * do it same as the order of enum spp_port_ability_type (spp_vf.h)
  */
 const char *PORT_ABILITY_STRINGS[] = {
-	SPP_ABILITY_NONE_STR,
-	SPP_ABILITY_ADD_VLANTAG_STR,
-	SPP_ABILITY_DEL_VLANTAG_STR,
-
-	/* termination */ "",
+	"none",
+	"add_vlantag",
+	"del_vlantag",
+	"",  /* termination */
 };
 
 /* Check mac address used on the port for registering or removing */
@@ -1005,20 +972,14 @@ struct decode_command_list {
 
 /* command list */
 static struct decode_command_list command_list[] = {
-	{ SPP_COMMAND_CLASSFIER_TABLE_STR, 5, 5,
-		decode_command_parameter_cls_table  },
-						/* classifier_table(mac)  */
-	{ SPP_COMMAND_CLASSFIER_TABLE_STR, 6, 6,
-		decode_command_parameter_cls_table_vlan },
-						/* classifier_table(vlan) */
-	{ SPP_COMMAND_GET_CLIENT_ID_STR, 1, 1, NULL }, /* _get_client_id  */
-	{ SPP_COMMAND_STATUS_STR,	 1, 1, NULL }, /* status          */
-	{ SPP_COMMAND_EXIT_STR,		 1, 1, NULL }, /* exit            */
-	{ SPP_COMMAND_COMPONENT_STR,	 3, 5,
-		decode_command_parameter_component  }, /* component       */
-	{ SPP_COMMAND_PORT_STR,		 5, 8,
-		decode_command_parameter_port       }, /* port            */
-	{ "",				 0, 0, NULL }  /* termination     */
+	{ "classifier_table", 5, 5, decode_command_parameter_cls_table },
+	{ "classifier_table", 6, 6, decode_command_parameter_cls_table_vlan },
+	{ "_get_client_id", 1, 1, NULL },
+	{ "static", 1, 1, NULL },
+	{ "exit", 1, 1, NULL },
+	{ "component", 3, 5, decode_command_parameter_component },
+	{ "port", 5, 8, decode_command_parameter_port },
+	{ "", 0, 0, NULL }  /* termination */
 };
 
 /* Parse command line parameters. */
-- 
2.17.1


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

* [spp] [PATCH 3/5] shared/sec: rename lists of fixed strings
  2019-05-10  8:35 [spp] [PATCH 0/5] Refactor source of cmd parser port utils ogawa.yasufumi
  2019-05-10  8:35 ` [spp] [PATCH 1/5] shared/sec: rename src command_dec to cmd_parser ogawa.yasufumi
  2019-05-10  8:35 ` [spp] [PATCH 2/5] shared/sec: remove no meaning str defines ogawa.yasufumi
@ 2019-05-10  8:35 ` ogawa.yasufumi
  2019-05-10  8:35 ` [spp] [PATCH 4/5] shared/sec: rename struct for attrs of classify ogawa.yasufumi
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: ogawa.yasufumi @ 2019-05-10  8:35 UTC (permalink / raw)
  To: spp, ferruh.yigit, ogawa.yasufumi

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

As redundant defines are removed, rename lists of fixed strings named
`*_STRINGS`, such as `PORT_ABILITY_STATUS_STRINGS`, to `*_LIST` because
it is not needed to declare as string explicitly.

Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
 .../secondary/spp_worker_th/cmd_parser.c      | 51 ++++++++++---------
 .../secondary/spp_worker_th/command_proc.c    | 41 ++++++++-------
 src/shared/secondary/spp_worker_th/spp_proc.h |  1 +
 3 files changed, 47 insertions(+), 46 deletions(-)

diff --git a/src/shared/secondary/spp_worker_th/cmd_parser.c b/src/shared/secondary/spp_worker_th/cmd_parser.c
index 8efbfdb..b9e3fbe 100644
--- a/src/shared/secondary/spp_worker_th/cmd_parser.c
+++ b/src/shared/secondary/spp_worker_th/cmd_parser.c
@@ -14,22 +14,11 @@
 #define RTE_LOGTYPE_SPP_COMMAND_PROC RTE_LOGTYPE_USER1
 #define RTE_LOGTYPE_APP RTE_LOGTYPE_USER2
 
-/*
- * List of classifier type. The order of items should be same as the order of
- * enum `spp_classifier_type` defined in spp_proc.h.
- */
-const char *CLASSIFILER_TYPE_STRINGS[] = {
-	"none",
-	"mac",
-	"vlan",
-	"",  /* termination */
-};
-
 /**
  * List of command action. The order of items should be same as the order of
  * enum `sppwk_action` in cmd_parser.h.
  */
-const char *COMMAND_ACTION_STRINGS[] = {
+const char *CMD_ACT_LIST[] = {
 	"none",
 	"start",
 	"stop",
@@ -38,22 +27,34 @@ const char *COMMAND_ACTION_STRINGS[] = {
 	"",  /* termination */
 };
 
-/*
- * List of port type. The order of items should be same as the order of
+/**
+ * List of classifier type. The order of items should be same as the order of
+ * enum `spp_classifier_type` defined in spp_proc.h.
+ */
+/* TODO(yasufum) fix sinmilar var in command_proc.c */
+const char *CLS_TYPE_LIST[] = {
+	"none",
+	"mac",
+	"vlan",
+	"",  /* termination */
+};
+
+/**
+ * List of port direction. The order of items should be same as the order of
  * enum `spp_port_rxtx` in spp_vf.h.
  */
-const char *PORT_RXTX_STRINGS[] = {
+const char *PORT_DIR_LIST[] = {
 	"none",
 	"rx",
 	"tx",
 	"",  /* termination */
 };
 
-/*
- * port ability string list
- * do it same as the order of enum spp_port_ability_type (spp_vf.h)
+/**
+ * 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_STRINGS[] = {
+const char *PORT_ABILITY_LIST[] = {
 	"none",
 	"add_vlantag",
 	"del_vlantag",
@@ -311,7 +312,7 @@ decode_component_action_value(void *output, const char *arg_val,
 				int allow_override __attribute__ ((unused)))
 {
 	int ret = SPP_RET_OK;
-	ret = get_arrary_index(arg_val, COMMAND_ACTION_STRINGS);
+	ret = get_arrary_index(arg_val, CMD_ACT_LIST);
 	if (unlikely(ret <= 0)) {
 		RTE_LOG(ERR, SPP_COMMAND_PROC,
 				"Unknown component action. val=%s\n",
@@ -397,7 +398,7 @@ decode_port_action_value(void *output, const char *arg_val,
 				int allow_override __attribute__ ((unused)))
 {
 	int ret = SPP_RET_OK;
-	ret = get_arrary_index(arg_val, COMMAND_ACTION_STRINGS);
+	ret = get_arrary_index(arg_val, CMD_ACT_LIST);
 	if (unlikely(ret <= 0)) {
 		RTE_LOG(ERR, SPP_COMMAND_PROC,
 				"Unknown port action. val=%s\n",
@@ -457,7 +458,7 @@ decode_port_rxtx_value(void *output, const char *arg_val, int allow_override)
 	int ret = SPP_RET_OK;
 	struct sppwk_cmd_port *port = output;
 
-	ret = get_arrary_index(arg_val, PORT_RXTX_STRINGS);
+	ret = get_arrary_index(arg_val, PORT_DIR_LIST);
 	if (unlikely(ret <= 0)) {
 		RTE_LOG(ERR, SPP_COMMAND_PROC, "Unknown port rxtx. val=%s\n",
 				arg_val);
@@ -508,7 +509,7 @@ decode_port_vlan_operation(void *output, const char *arg_val,
 
 	switch (ability->ope) {
 	case SPP_PORT_ABILITY_OPE_NONE:
-		ret = get_arrary_index(arg_val, PORT_ABILITY_STRINGS);
+		ret = get_arrary_index(arg_val, PORT_ABILITY_LIST);
 		if (unlikely(ret <= 0)) {
 			RTE_LOG(ERR, SPP_COMMAND_PROC,
 					"Unknown port ability. val=%s\n",
@@ -613,7 +614,7 @@ decode_classifier_action_value(void *output, const char *arg_val,
 				int allow_override __attribute__ ((unused)))
 {
 	int ret = SPP_RET_OK;
-	ret = get_arrary_index(arg_val, COMMAND_ACTION_STRINGS);
+	ret = get_arrary_index(arg_val, CMD_ACT_LIST);
 	if (unlikely(ret <= 0)) {
 		RTE_LOG(ERR, SPP_COMMAND_PROC, "Unknown port action. val=%s\n",
 				arg_val);
@@ -637,7 +638,7 @@ decode_classifier_type_value(void *output, const char *arg_val,
 				int allow_override __attribute__ ((unused)))
 {
 	int ret = SPP_RET_OK;
-	ret = get_arrary_index(arg_val, CLASSIFILER_TYPE_STRINGS);
+	ret = get_arrary_index(arg_val, CLS_TYPE_LIST);
 	if (unlikely(ret <= 0)) {
 		RTE_LOG(ERR, SPP_COMMAND_PROC,
 				"Unknown classifier type. val=%s\n",
diff --git a/src/shared/secondary/spp_worker_th/command_proc.c b/src/shared/secondary/spp_worker_th/command_proc.c
index f345759..9df4cdc 100644
--- a/src/shared/secondary/spp_worker_th/command_proc.c
+++ b/src/shared/secondary/spp_worker_th/command_proc.c
@@ -63,40 +63,39 @@ struct command_response_list {
 	int (*func)(const char *name, char **output, void *tmp);
 };
 
-/*
- * seconary type string list
- * do it same the order enum secondary_type (spp_proc.h)
+/**
+ * List of worker process type. The order of items should be same as the order
+ * of enum `secondary_type` in spp_proc.h.
  */
-const char *SECONDARY_PROCESS_TYPE_SRINGS[] = {
+/* TODO(yasufum) rename `secondary_type` to `sppwk_proc_type`. */
+const char *SPPWK_PROC_TYPE_LIST[] = {
 	"none",
 	"vf",
 	"mirror",
-
-	/* termination */ "",
+	"",  /* termination */
 };
 
-/*
- * port ability string list
- * do it same as the order of enum spp_port_ability_type (spp_vf.h)
+/**
+ * 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_STATUS_STRINGS[] = {
+const char *PORT_ABILITY_STAT_LIST[] = {
 	"none",
 	"add",
 	"del",
-
-	/* termination */ "",
+	"",  /* termination */
 };
 
-/*
- * classifier type string list
- * do it same as the order of enum spp_classifier_type (spp_vf.h)
+/**
+ * List of classifier type. The order of items should be same as the order of
+ * enum `spp_classifier_type` defined in spp_proc.h.
  */
-const char *CLASSIFILER_TYPE_STATUS_STRINGS[] = {
+/* TODO(yasufum) fix similar var in cmd_parser.c */
+const char *CLS_TYPE_A_LIST[] = {
 	"none",
 	"mac",
 	"vlan",
-
-	/* termination */ "",
+	"",  /* termination */
 };
 
 /* get client id */
@@ -969,7 +968,7 @@ append_process_type_value(const char *name, char **output,
 		void *tmp __attribute__ ((unused)))
 {
 	return append_json_str_value(name, output,
-			SECONDARY_PROCESS_TYPE_SRINGS[spp_get_process_type()]);
+			SPPWK_PROC_TYPE_LIST[spp_get_process_type()]);
 }
 
 /* append a list of interface numbers for JSON format */
@@ -1012,7 +1011,7 @@ 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,
-			PORT_ABILITY_STATUS_STRINGS[ope]);
+			PORT_ABILITY_STAT_LIST[ope]);
 	if (unlikely(ret < SPP_RET_OK))
 		return SPP_RET_NG;
 
@@ -1269,7 +1268,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,
-			CLASSIFILER_TYPE_STATUS_STRINGS[type]);
+			CLS_TYPE_A_LIST[type]);
 	if (unlikely(ret < SPP_RET_OK))
 		return ret;
 
diff --git a/src/shared/secondary/spp_worker_th/spp_proc.h b/src/shared/secondary/spp_worker_th/spp_proc.h
index d7952fb..bca2c0e 100644
--- a/src/shared/secondary/spp_worker_th/spp_proc.h
+++ b/src/shared/secondary/spp_worker_th/spp_proc.h
@@ -149,6 +149,7 @@ enum copy_mng_flg {
 };
 
 /* secondary process type used only from spp_vf and spp_mirror */
+/* TODO(yasufum) rename `secondary_type` to `sppwk_proc_type`. */
 enum secondary_type {
 	SECONDARY_TYPE_NONE,
 	SECONDARY_TYPE_VF,
-- 
2.17.1


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

* [spp] [PATCH 4/5] shared/sec: rename struct for attrs of classify
  2019-05-10  8:35 [spp] [PATCH 0/5] Refactor source of cmd parser port utils ogawa.yasufumi
                   ` (2 preceding siblings ...)
  2019-05-10  8:35 ` [spp] [PATCH 3/5] shared/sec: rename lists of fixed strings ogawa.yasufumi
@ 2019-05-10  8:35 ` ogawa.yasufumi
  2019-05-10  8:35 ` [spp] [PATCH 5/5] shared/sec: rename sppwk port util functions ogawa.yasufumi
  2019-05-15  5:44 ` [spp] [PATCH v2 0/5] Fix typo of status command ogawa.yasufumi
  5 siblings, 0 replies; 12+ messages in thread
From: ogawa.yasufumi @ 2019-05-10  8:35 UTC (permalink / raw)
  To: spp, ferruh.yigit, ogawa.yasufumi

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

Struct `spp_port_class_identifier` has members of MAC address and VLAN
tag used for classifying, so the name is not appropriate because it is
not an identifier but a set of attributes. This update is to fix the
issue.

Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
 .../secondary/spp_worker_th/cmd_parser.c      | 31 +++++++++----------
 .../secondary/spp_worker_th/cmd_parser.h      |  4 +--
 .../secondary/spp_worker_th/command_proc.c    | 28 ++++++++---------
 src/shared/secondary/spp_worker_th/spp_proc.c | 24 +++++++-------
 src/shared/secondary/spp_worker_th/spp_proc.h | 12 +++----
 src/vf/classifier_mac.c                       |  8 ++---
 6 files changed, 53 insertions(+), 54 deletions(-)

diff --git a/src/shared/secondary/spp_worker_th/cmd_parser.c b/src/shared/secondary/spp_worker_th/cmd_parser.c
index b9e3fbe..6e536eb 100644
--- a/src/shared/secondary/spp_worker_th/cmd_parser.c
+++ b/src/shared/secondary/spp_worker_th/cmd_parser.c
@@ -61,18 +61,17 @@ const char *PORT_ABILITY_LIST[] = {
 	"",  /* termination */
 };
 
-/* Check mac address used on the port for registering or removing */
+/* Return 1 as true if port is used with given mac_addr and vid. */
 static int
-spp_check_classid_used_port(
+is_used_with_addr(
 		int vid, uint64_t mac_addr,
 		enum port_type iface_type, int iface_no)
 {
-	struct sppwk_port_info *port_info = get_iface_info(
+	struct sppwk_port_info *wk_port = get_iface_info(
 			iface_type, iface_no);
 
-	/* Return true if given mac_addr matches with port_info, and vid. */
-	return ((mac_addr == port_info->class_id.mac_addr) &&
-		(vid == port_info->class_id.vlantag.vid));
+	return ((mac_addr == wk_port->cls_attrs.mac_addr) &&
+		(vid == wk_port->cls_attrs.vlantag.vid));
 }
 
 /* Check if port has been added. */
@@ -671,7 +670,7 @@ parse_cls_port(void *cls_cmd_attr, const char *arg_val,
 		int allow_override __attribute__ ((unused)))
 {
 	int ret = SPP_RET_OK;
-	struct sppwk_cls_cmd_attr *cls_attr = cls_cmd_attr;
+	struct sppwk_cls_cmd_attrs *cls_attrs = cls_cmd_attr;
 	struct sppwk_port_idx tmp_port;
 	int64_t mac_addr = 0;
 
@@ -686,23 +685,23 @@ parse_cls_port(void *cls_cmd_attr, const char *arg_val,
 		return SPP_RET_NG;
 	}
 
-	if (cls_attr->type == SPP_CLASSIFIER_TYPE_MAC)
-		cls_attr->vid = ETH_VLAN_ID_MAX;
+	if (cls_attrs->type == SPP_CLASSIFIER_TYPE_MAC)
+		cls_attrs->vid = ETH_VLAN_ID_MAX;
 
-	if (unlikely(cls_attr->wk_action == SPPWK_ACT_ADD)) {
-		if (!spp_check_classid_used_port(ETH_VLAN_ID_MAX, 0,
+	if (unlikely(cls_attrs->wk_action == SPPWK_ACT_ADD)) {
+		if (!is_used_with_addr(ETH_VLAN_ID_MAX, 0,
 				tmp_port.iface_type, tmp_port.iface_no)) {
 			RTE_LOG(ERR, SPP_COMMAND_PROC, "Port in used. "
 					"(classifier_table command) val=%s\n",
 					arg_val);
 			return SPP_RET_NG;
 		}
-	} else if (unlikely(cls_attr->wk_action == SPPWK_ACT_DEL)) {
-		mac_addr = spp_change_mac_str_to_int64(cls_attr->mac);
+	} else if (unlikely(cls_attrs->wk_action == SPPWK_ACT_DEL)) {
+		mac_addr = spp_change_mac_str_to_int64(cls_attrs->mac);
 		if (mac_addr < 0)
 			return SPP_RET_NG;
 
-		if (!spp_check_classid_used_port(cls_attr->vid,
+		if (!is_used_with_addr(cls_attrs->vid,
 				(uint64_t)mac_addr,
 				tmp_port.iface_type, tmp_port.iface_no)) {
 			RTE_LOG(ERR, SPP_COMMAND_PROC, "Port in used. "
@@ -712,8 +711,8 @@ parse_cls_port(void *cls_cmd_attr, const char *arg_val,
 		}
 	}
 
-	cls_attr->port.iface_type = tmp_port.iface_type;
-	cls_attr->port.iface_no   = tmp_port.iface_no;
+	cls_attrs->port.iface_type = tmp_port.iface_type;
+	cls_attrs->port.iface_no   = tmp_port.iface_no;
 	return SPP_RET_OK;
 }
 
diff --git a/src/shared/secondary/spp_worker_th/cmd_parser.h b/src/shared/secondary/spp_worker_th/cmd_parser.h
index b03a920..58e39a9 100644
--- a/src/shared/secondary/spp_worker_th/cmd_parser.h
+++ b/src/shared/secondary/spp_worker_th/cmd_parser.h
@@ -73,7 +73,7 @@ enum sppwk_cmd_type {
 };
 
 /* `classifier_table` command specific parameters. */
-struct sppwk_cls_cmd_attr {
+struct sppwk_cls_cmd_attrs {
 	enum sppwk_action wk_action;  /**< add or del */
 	enum spp_classifier_type type;  /**< currently only for mac */
 	int vid;  /**< VLAN ID  */
@@ -108,7 +108,7 @@ struct spp_command {
 	enum sppwk_cmd_type type; /**< command type */
 
 	union {  /**< command descriptors */
-		struct sppwk_cls_cmd_attr cls_table;
+		struct sppwk_cls_cmd_attrs cls_table;
 		struct sppwk_cmd_flush flush;
 		struct sppwk_cmd_comp comp;
 		struct sppwk_cmd_port port;
diff --git a/src/shared/secondary/spp_worker_th/command_proc.c b/src/shared/secondary/spp_worker_th/command_proc.c
index 9df4cdc..ab0bbb3 100644
--- a/src/shared/secondary/spp_worker_th/command_proc.c
+++ b/src/shared/secondary/spp_worker_th/command_proc.c
@@ -167,48 +167,48 @@ spp_update_classifier_table(
 
 	if (wk_action == SPPWK_ACT_DEL) {
 		/* Delete */
-		if ((port_info->class_id.vlantag.vid != 0) &&
-				unlikely(port_info->class_id.vlantag.vid !=
+		if ((port_info->cls_attrs.vlantag.vid != 0) &&
+				unlikely(port_info->cls_attrs.vlantag.vid !=
 				vid)) {
 			RTE_LOG(ERR, APP, "VLAN ID is different. "
 					"( vid = %d )\n", vid);
 			return SPP_RET_NG;
 		}
-		if ((port_info->class_id.mac_addr != 0) &&
-			unlikely(port_info->class_id.mac_addr !=
+		if ((port_info->cls_attrs.mac_addr != 0) &&
+			unlikely(port_info->cls_attrs.mac_addr !=
 					mac_addr)) {
 			RTE_LOG(ERR, APP, "MAC address is different. "
 					"( mac = %s )\n", mac_addr_str);
 			return SPP_RET_NG;
 		}
 
-		port_info->class_id.vlantag.vid = ETH_VLAN_ID_MAX;
-		port_info->class_id.mac_addr    = 0;
-		memset(port_info->class_id.mac_addr_str, 0x00,
+		port_info->cls_attrs.vlantag.vid = ETH_VLAN_ID_MAX;
+		port_info->cls_attrs.mac_addr    = 0;
+		memset(port_info->cls_attrs.mac_addr_str, 0x00,
 							SPP_MIN_STR_LEN);
 
 	} else if (wk_action == SPPWK_ACT_ADD) {
 		/* Setting */
-		if (unlikely(port_info->class_id.vlantag.vid !=
+		if (unlikely(port_info->cls_attrs.vlantag.vid !=
 				ETH_VLAN_ID_MAX)) {
 			RTE_LOG(ERR, APP, "Port in used. "
 					"( port = %d:%d, vlan = %d != %d )\n",
 					port->iface_type, port->iface_no,
-					port_info->class_id.vlantag.vid, vid);
+					port_info->cls_attrs.vlantag.vid, vid);
 			return SPP_RET_NG;
 		}
-		if (unlikely(port_info->class_id.mac_addr != 0)) {
+		if (unlikely(port_info->cls_attrs.mac_addr != 0)) {
 			RTE_LOG(ERR, APP, "Port in used. "
 					"( port = %d:%d, mac = %s != %s )\n",
 					port->iface_type, port->iface_no,
-					port_info->class_id.mac_addr_str,
+					port_info->cls_attrs.mac_addr_str,
 					mac_addr_str);
 			return SPP_RET_NG;
 		}
 
-		port_info->class_id.vlantag.vid = vid;
-		port_info->class_id.mac_addr    = mac_addr;
-		strcpy(port_info->class_id.mac_addr_str, mac_addr_str);
+		port_info->cls_attrs.vlantag.vid = vid;
+		port_info->cls_attrs.mac_addr    = mac_addr;
+		strcpy(port_info->cls_attrs.mac_addr_str, mac_addr_str);
 	}
 
 	set_component_change_port(port_info, SPP_PORT_RXTX_TX);
diff --git a/src/shared/secondary/spp_worker_th/spp_proc.c b/src/shared/secondary/spp_worker_th/spp_proc.c
index 18acd64..370f071 100644
--- a/src/shared/secondary/spp_worker_th/spp_proc.c
+++ b/src/shared/secondary/spp_worker_th/spp_proc.c
@@ -355,9 +355,9 @@ dump_interface_info(const struct iface_info *iface_info)
 				"vid = %u, mac=%08lx(%s)\n",
 				cnt, port->iface_type, port->iface_no,
 				port->ethdev_port_id,
-				port->class_id.vlantag.vid,
-				port->class_id.mac_addr,
-				port->class_id.mac_addr_str);
+				port->cls_attrs.vlantag.vid,
+				port->cls_attrs.mac_addr,
+				port->cls_attrs.mac_addr_str);
 	}
 	for (cnt = 0; cnt < RTE_MAX_ETHPORTS; cnt++) {
 		port = &iface_info->vhost[cnt];
@@ -368,9 +368,9 @@ dump_interface_info(const struct iface_info *iface_info)
 				"vid = %u, mac=%08lx(%s)\n",
 				cnt, port->iface_type, port->iface_no,
 				port->ethdev_port_id,
-				port->class_id.vlantag.vid,
-				port->class_id.mac_addr,
-				port->class_id.mac_addr_str);
+				port->cls_attrs.vlantag.vid,
+				port->cls_attrs.mac_addr,
+				port->cls_attrs.mac_addr_str);
 	}
 	for (cnt = 0; cnt < RTE_MAX_ETHPORTS; cnt++) {
 		port = &iface_info->ring[cnt];
@@ -381,9 +381,9 @@ dump_interface_info(const struct iface_info *iface_info)
 				"vid = %u, mac=%08lx(%s)\n",
 				cnt, port->iface_type, port->iface_no,
 				port->ethdev_port_id,
-				port->class_id.vlantag.vid,
-				port->class_id.mac_addr,
-				port->class_id.mac_addr_str);
+				port->cls_attrs.vlantag.vid,
+				port->cls_attrs.mac_addr,
+				port->cls_attrs.mac_addr_str);
 	}
 }
 
@@ -476,17 +476,17 @@ init_iface_info(void)
 		p_iface_info->nic[port_cnt].iface_type = UNDEF;
 		p_iface_info->nic[port_cnt].iface_no = port_cnt;
 		p_iface_info->nic[port_cnt].ethdev_port_id = -1;
-		p_iface_info->nic[port_cnt].class_id.vlantag.vid =
+		p_iface_info->nic[port_cnt].cls_attrs.vlantag.vid =
 			ETH_VLAN_ID_MAX;
 		p_iface_info->vhost[port_cnt].iface_type = UNDEF;
 		p_iface_info->vhost[port_cnt].iface_no = port_cnt;
 		p_iface_info->vhost[port_cnt].ethdev_port_id = -1;
-		p_iface_info->vhost[port_cnt].class_id.vlantag.vid =
+		p_iface_info->vhost[port_cnt].cls_attrs.vlantag.vid =
 			ETH_VLAN_ID_MAX;
 		p_iface_info->ring[port_cnt].iface_type = UNDEF;
 		p_iface_info->ring[port_cnt].iface_no = port_cnt;
 		p_iface_info->ring[port_cnt].ethdev_port_id = -1;
-		p_iface_info->ring[port_cnt].class_id.vlantag.vid =
+		p_iface_info->ring[port_cnt].cls_attrs.vlantag.vid =
 			ETH_VLAN_ID_MAX;
 	}
 }
diff --git a/src/shared/secondary/spp_worker_th/spp_proc.h b/src/shared/secondary/spp_worker_th/spp_proc.h
index bca2c0e..abf16e7 100644
--- a/src/shared/secondary/spp_worker_th/spp_proc.h
+++ b/src/shared/secondary/spp_worker_th/spp_proc.h
@@ -179,11 +179,11 @@ struct spp_port_ability {
 	union spp_ability_data data;   /**< Port ability data */
 };
 
-/** Port class identifier for classifying */
-struct spp_port_class_identifier {
-	uint64_t mac_addr;                      /**< Mac address (binary) */
-	char     mac_addr_str[SPP_MIN_STR_LEN]; /**< Mac address (text) */
-	struct spp_vlantag_info vlantag;        /**< VLAN tag information */
+/* Attributes for classifying . */
+struct sppwk_cls_attrs {
+	uint64_t mac_addr;  /**< Mac address (binary) */
+	char mac_addr_str[SPP_MIN_STR_LEN];  /**< Mac address (text) */
+	struct spp_vlantag_info vlantag;   /**< VLAN tag information */
 };
 
 /**
@@ -200,7 +200,7 @@ struct sppwk_port_info {
 	enum port_type iface_type;  /**< phy, vhost or ring */
 	int iface_no;
 	int ethdev_port_id;  /**< Consistent ID of ethdev */
-	struct spp_port_class_identifier class_id;
+	struct sppwk_cls_attrs cls_attrs;
 	struct spp_port_ability ability[SPP_PORT_ABILITY_MAX];
 };
 
diff --git a/src/vf/classifier_mac.c b/src/vf/classifier_mac.c
index 1e98636..9fd8da1 100644
--- a/src/vf/classifier_mac.c
+++ b/src/vf/classifier_mac.c
@@ -356,7 +356,7 @@ init_component_info(struct component_info *cmp_info,
 	cmp_info->mac_addr_entry = 0;
 	for (i = 0; i < component_info->num_tx_port; i++) {
 		tx_port = component_info->tx_ports[i];
-		vid = tx_port->class_id.vlantag.vid;
+		vid = tx_port->cls_attrs.vlantag.vid;
 
 		/* store ports information */
 		clsd_data_tx[i].iface_type      = tx_port->iface_type;
@@ -365,7 +365,7 @@ init_component_info(struct component_info *cmp_info,
 		clsd_data_tx[i].port            = tx_port->ethdev_port_id;
 		clsd_data_tx[i].num_pkt         = 0;
 
-		if (tx_port->class_id.mac_addr == 0)
+		if (tx_port->cls_attrs.mac_addr == 0)
 			continue;
 
 		/* if mac classification is NULL, make instance */
@@ -389,7 +389,7 @@ init_component_info(struct component_info *cmp_info,
 		cmp_info->mac_addr_entry = 1;
 
 		/* store default classified */
-		if (unlikely(tx_port->class_id.mac_addr ==
+		if (unlikely(tx_port->cls_attrs.mac_addr ==
 				SPP_DEFAULT_CLASSIFIED_DMY_ADDR)) {
 			mac_cls->default_classified = i;
 			RTE_LOG(INFO, SPP_CLASSIFIER_MAC,
@@ -404,7 +404,7 @@ init_component_info(struct component_info *cmp_info,
 		}
 
 		/* add entry to classifier mac table */
-		rte_memcpy(&eth_addr, &tx_port->class_id.mac_addr,
+		rte_memcpy(&eth_addr, &tx_port->cls_attrs.mac_addr,
 				ETHER_ADDR_LEN);
 		ether_format_addr(mac_addr_str, sizeof(mac_addr_str),
 				&eth_addr);
-- 
2.17.1


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

* [spp] [PATCH 5/5] shared/sec: rename sppwk port util functions
  2019-05-10  8:35 [spp] [PATCH 0/5] Refactor source of cmd parser port utils ogawa.yasufumi
                   ` (3 preceding siblings ...)
  2019-05-10  8:35 ` [spp] [PATCH 4/5] shared/sec: rename struct for attrs of classify ogawa.yasufumi
@ 2019-05-10  8:35 ` ogawa.yasufumi
  2019-05-15  5:44 ` [spp] [PATCH v2 0/5] Fix typo of status command ogawa.yasufumi
  5 siblings, 0 replies; 12+ messages in thread
From: ogawa.yasufumi @ 2019-05-10  8:35 UTC (permalink / raw)
  To: spp, ferruh.yigit, ogawa.yasufumi

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

Some functions for managing sppwk port are prefixed `spp_` but file
local, or the name is not so intuitive. This update is to rename the
names.

* Change file local `spp_check_added_port()` to `is_added_port()`.

* Change file local `spp_convert_port_to_iface()` to
  `parse_resource_uid()`.

* Change `get_iface_info()` to `get_sppwk_port()` because the word
  `iface info` is ambiguous.

Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
 .../secondary/spp_worker_th/cmd_parser.c      | 73 +++++++++----------
 .../secondary/spp_worker_th/command_proc.c    |  6 +-
 src/shared/secondary/spp_worker_th/spp_proc.c |  9 +--
 src/shared/secondary/spp_worker_th/spp_proc.h | 15 +---
 4 files changed, 42 insertions(+), 61 deletions(-)

diff --git a/src/shared/secondary/spp_worker_th/cmd_parser.c b/src/shared/secondary/spp_worker_th/cmd_parser.c
index 6e536eb..ac1b035 100644
--- a/src/shared/secondary/spp_worker_th/cmd_parser.c
+++ b/src/shared/secondary/spp_worker_th/cmd_parser.c
@@ -67,72 +67,67 @@ is_used_with_addr(
 		int vid, uint64_t mac_addr,
 		enum port_type iface_type, int iface_no)
 {
-	struct sppwk_port_info *wk_port = get_iface_info(
+	struct sppwk_port_info *wk_port = get_sppwk_port(
 			iface_type, iface_no);
 
 	return ((mac_addr == wk_port->cls_attrs.mac_addr) &&
 		(vid == wk_port->cls_attrs.vlantag.vid));
 }
 
-/* Check if port has been added. */
+/* Return 1 as true if given port is already used. */
 static int
-spp_check_added_port(enum port_type iface_type, int iface_no)
+is_added_port(enum port_type iface_type, int iface_no)
 {
-	struct sppwk_port_info *port = get_iface_info(iface_type, iface_no);
+	struct sppwk_port_info *port = get_sppwk_port(iface_type, iface_no);
 	return port->iface_type != UNDEF;
 }
 
 /**
- * Separate port id of combination of iface type and number and
- * assign to given argument, iface_type and iface_no.
- *
- * For instance, 'ring:0' is separated to 'ring' and '0'.
+ * Separate resource UID of combination of iface type and number and assign to
+ * given argument, iface_type and iface_no. For instance, 'ring:0' is separated
+ * to 'ring' and '0'. The supported types are `phy`, `vhost` and `ring`.
  */
 static int
-spp_convert_port_to_iface(const char *port,
+parse_resource_uid(const char *res_uid,
 		    enum port_type *iface_type,
 		    int *iface_no)
 {
-	enum port_type type = UNDEF;
-	const char *no_str = NULL;
+	enum port_type ptype = UNDEF;
+	const char *iface_no_str = NULL;
 	char *endptr = NULL;
 
-	/* Find out which type of interface from port */
-	if (strncmp(port, SPP_IFTYPE_NIC_STR ":",
+	/**
+	 * TODO(yasufum) consider this checking of zero value is recommended
+	 * way, or should be changed.
+	 */
+	if (strncmp(res_uid, SPP_IFTYPE_NIC_STR ":",
 			strlen(SPP_IFTYPE_NIC_STR)+1) == 0) {
-		/* NIC */
-		type = PHY;
-		no_str = &port[strlen(SPP_IFTYPE_NIC_STR)+1];
-	} else if (strncmp(port, SPP_IFTYPE_VHOST_STR ":",
+		ptype = PHY;
+		iface_no_str = &res_uid[strlen(SPP_IFTYPE_NIC_STR)+1];
+	} else if (strncmp(res_uid, SPP_IFTYPE_VHOST_STR ":",
 			strlen(SPP_IFTYPE_VHOST_STR)+1) == 0) {
-		/* VHOST */
-		type = VHOST;
-		no_str = &port[strlen(SPP_IFTYPE_VHOST_STR)+1];
-	} else if (strncmp(port, SPP_IFTYPE_RING_STR ":",
+		ptype = VHOST;
+		iface_no_str = &res_uid[strlen(SPP_IFTYPE_VHOST_STR)+1];
+	} else if (strncmp(res_uid, SPP_IFTYPE_RING_STR ":",
 			strlen(SPP_IFTYPE_RING_STR)+1) == 0) {
-		/* RING */
-		type = RING;
-		no_str = &port[strlen(SPP_IFTYPE_RING_STR)+1];
+		ptype = RING;
+		iface_no_str = &res_uid[strlen(SPP_IFTYPE_RING_STR)+1];
 	} else {
-		/* OTHER */
-		RTE_LOG(ERR, APP, "Unknown interface type. (port = %s)\n",
-				port);
+		RTE_LOG(ERR, APP, "Unexpected port type in '%s'.\n", res_uid);
 		return SPP_RET_NG;
 	}
 
-	/* Change type of number of interface */
-	int ret_no = strtol(no_str, &endptr, 0);
-	if (unlikely(no_str == endptr) || unlikely(*endptr != '\0')) {
-		/* No IF number */
-		RTE_LOG(ERR, APP, "No interface number. (port = %s)\n", port);
+	int port_id = strtol(iface_no_str, &endptr, 0);
+	if (unlikely(iface_no_str == endptr) || unlikely(*endptr != '\0')) {
+		RTE_LOG(ERR, APP, "No interface number in '%s'.\n", res_uid);
 		return SPP_RET_NG;
 	}
 
-	*iface_type = type;
-	*iface_no = ret_no;
+	*iface_type = ptype;
+	*iface_no = port_id;
 
-	RTE_LOG(DEBUG, APP, "Port = %s => Type = %d No = %d\n",
-			port, *iface_type, *iface_no);
+	RTE_LOG(DEBUG, APP, "Parsed '%s' to '%d' and '%d'.\n",
+			res_uid, *iface_type, *iface_no);
 	return SPP_RET_OK;
 }
 
@@ -280,8 +275,7 @@ decode_port_value(void *output, const char *arg_val)
 {
 	int ret = SPP_RET_OK;
 	struct sppwk_port_idx *port = output;
-	ret = spp_convert_port_to_iface(arg_val, &port->iface_type,
-							&port->iface_no);
+	ret = parse_resource_uid(arg_val, &port->iface_type, &port->iface_no);
 	if (unlikely(ret != 0)) {
 		RTE_LOG(ERR, SPP_COMMAND_PROC, "Bad port. val=%s\n", arg_val);
 		return SPP_RET_NG;
@@ -678,8 +672,7 @@ parse_cls_port(void *cls_cmd_attr, const char *arg_val,
 	if (ret < SPP_RET_OK)
 		return SPP_RET_NG;
 
-	if (spp_check_added_port(tmp_port.iface_type,
-					tmp_port.iface_no) == 0) {
+	if (is_added_port(tmp_port.iface_type, tmp_port.iface_no) == 0) {
 		RTE_LOG(ERR, SPP_COMMAND_PROC, "Port not added. val=%s\n",
 				arg_val);
 		return SPP_RET_NG;
diff --git a/src/shared/secondary/spp_worker_th/command_proc.c b/src/shared/secondary/spp_worker_th/command_proc.c
index ab0bbb3..19f96d7 100644
--- a/src/shared/secondary/spp_worker_th/command_proc.c
+++ b/src/shared/secondary/spp_worker_th/command_proc.c
@@ -124,7 +124,7 @@ spp_get_process_type(void)
 static int
 spp_check_flush_port(enum port_type iface_type, int iface_no)
 {
-	struct sppwk_port_info *port = get_iface_info(iface_type, iface_no);
+	struct sppwk_port_info *port = get_sppwk_port(iface_type, iface_no);
 	return port->ethdev_port_id >= 0;
 }
 
@@ -153,7 +153,7 @@ spp_update_classifier_table(
 	}
 	mac_addr = (uint64_t)ret_mac;
 
-	port_info = get_iface_info(port->iface_type, port->iface_no);
+	port_info = get_sppwk_port(port->iface_type, port->iface_no);
 	if (unlikely(port_info == NULL)) {
 		RTE_LOG(ERR, APP, "No port. ( port = %d:%d )\n",
 				port->iface_type, port->iface_no);
@@ -390,7 +390,7 @@ spp_update_port(enum sppwk_action wk_action,
 	spp_get_mng_data_addr(NULL, NULL,
 			&comp_info_base, NULL, NULL, &change_component, NULL);
 	comp_info = (comp_info_base + component_id);
-	port_info = get_iface_info(port->iface_type, port->iface_no);
+	port_info = get_sppwk_port(port->iface_type, port->iface_no);
 	if (rxtx == SPP_PORT_RXTX_RX) {
 		num = &comp_info->num_rx_port;
 		ports = comp_info->rx_ports;
diff --git a/src/shared/secondary/spp_worker_th/spp_proc.c b/src/shared/secondary/spp_worker_th/spp_proc.c
index 370f071..53dd3f8 100644
--- a/src/shared/secondary/spp_worker_th/spp_proc.c
+++ b/src/shared/secondary/spp_worker_th/spp_proc.c
@@ -265,12 +265,11 @@ stop_process(int signal)
 }
 
 /**
- * Return port info of given type and num of interface
- *
- * It returns NULL value if given type is invalid.
+ * Return sppwk_port_info of given type and num of interface. It returns NULL
+ * if given type is invalid.
  */
 struct sppwk_port_info *
-get_iface_info(enum port_type iface_type, int iface_no)
+get_sppwk_port(enum port_type iface_type, int iface_no)
 {
 	struct iface_info *iface_info = g_mng_data_addr.p_iface_info;
 
@@ -663,7 +662,7 @@ spp_check_used_port(
 	int cnt, port_cnt, max = 0;
 	struct spp_component_info *component = NULL;
 	struct sppwk_port_info **port_array = NULL;
-	struct sppwk_port_info *port = get_iface_info(iface_type, iface_no);
+	struct sppwk_port_info *port = get_sppwk_port(iface_type, iface_no);
 	struct spp_component_info *component_info =
 					g_mng_data_addr.p_component_info;
 
diff --git a/src/shared/secondary/spp_worker_th/spp_proc.h b/src/shared/secondary/spp_worker_th/spp_proc.h
index abf16e7..aeb8e92 100644
--- a/src/shared/secondary/spp_worker_th/spp_proc.h
+++ b/src/shared/secondary/spp_worker_th/spp_proc.h
@@ -428,23 +428,12 @@ void set_all_core_status(enum spp_core_status status);
  *
  * @param signl
  *  received signal.
- *
  */
 void stop_process(int signal);
 
-/**
- * Return port info of given type and num of interface
- *
- * @param iface_type
- *  Interface type to be validated.
- * @param iface_no
- *  Interface number to be validated.
- *
- * @retval !NULL  sppwk_port_info.
- * @retval NULL   failed.
- */
+/* Return sppwk_port_info of given type and num of interface. */
 struct sppwk_port_info *
-get_iface_info(enum port_type iface_type, int iface_no);
+get_sppwk_port(enum port_type iface_type, int iface_no);
 
 /* Dump of core information */
 void dump_core_info(const struct core_mng_info *core_info);
-- 
2.17.1


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

* [spp] [PATCH v2 0/5] Fix typo of status command
  2019-05-10  8:35 [spp] [PATCH 0/5] Refactor source of cmd parser port utils ogawa.yasufumi
                   ` (4 preceding siblings ...)
  2019-05-10  8:35 ` [spp] [PATCH 5/5] shared/sec: rename sppwk port util functions ogawa.yasufumi
@ 2019-05-15  5:44 ` ogawa.yasufumi
  2019-05-15  5:44   ` [spp] [PATCH v2 1/5] shared/sec: rename src command_dec to cmd_parser ogawa.yasufumi
                     ` (4 more replies)
  5 siblings, 5 replies; 12+ messages in thread
From: ogawa.yasufumi @ 2019-05-15  5:44 UTC (permalink / raw)
  To: spp, ferruh.yigit, ogawa.yasufumi

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

This v2 patches is to fix type of status command in the second patch to
remove defines.

Yasufumi Ogawa (5):
  shared/sec: rename src command_dec to cmd_parser
  shared/sec: remove no meaning str defines
  shared/sec: rename lists of fixed strings
  shared/sec: rename struct for attrs of classify
  shared/sec: rename sppwk port util functions

 src/mirror/Makefile                           |   2 +-
 .../{command_dec.c => cmd_parser.c}           | 234 +++++++-----------
 .../secondary/spp_worker_th/cmd_parser.h      |   6 +-
 .../secondary/spp_worker_th/command_proc.c    |  75 +++---
 src/shared/secondary/spp_worker_th/spp_proc.c |  33 ++-
 src/shared/secondary/spp_worker_th/spp_proc.h |  28 +--
 src/vf/Makefile                               |   2 +-
 src/vf/classifier_mac.c                       |   8 +-
 8 files changed, 165 insertions(+), 223 deletions(-)
 rename src/shared/secondary/spp_worker_th/{command_dec.c => cmd_parser.c} (81%)

-- 
2.17.1


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

* [spp] [PATCH v2 1/5] shared/sec: rename src command_dec to cmd_parser
  2019-05-15  5:44 ` [spp] [PATCH v2 0/5] Fix typo of status command ogawa.yasufumi
@ 2019-05-15  5:44   ` ogawa.yasufumi
  2019-05-15  5:44   ` [spp] [PATCH v2 2/5] shared/sec: remove no meaning str defines ogawa.yasufumi
                     ` (3 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: ogawa.yasufumi @ 2019-05-15  5:44 UTC (permalink / raw)
  To: spp, ferruh.yigit, ogawa.yasufumi

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

For refactoring, rename source file `command_dec.c` to `cmd_parser.c`
because it defines vars and functions for parsing command, not decoding.

Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
 src/mirror/Makefile                                             | 2 +-
 .../secondary/spp_worker_th/{command_dec.c => cmd_parser.c}     | 0
 src/shared/secondary/spp_worker_th/cmd_parser.h                 | 2 +-
 src/vf/Makefile                                                 | 2 +-
 4 files changed, 3 insertions(+), 3 deletions(-)
 rename src/shared/secondary/spp_worker_th/{command_dec.c => cmd_parser.c} (100%)

diff --git a/src/mirror/Makefile b/src/mirror/Makefile
index be58e3a..3104059 100644
--- a/src/mirror/Makefile
+++ b/src/mirror/Makefile
@@ -20,7 +20,7 @@ SRCS-y += ../shared/secondary/utils.c ../shared/secondary/add_port.c
 SRCS-y += $(SPP_WKT_DIR)/spp_proc.c
 SRCS-y += $(SPP_WKT_DIR)/spp_port.c
 SRCS-y += $(SPP_WKT_DIR)/command_conn.c
-SRCS-y += $(SPP_WKT_DIR)/command_dec.c
+SRCS-y += $(SPP_WKT_DIR)/cmd_parser.c
 SRCS-y += $(SPP_WKT_DIR)/command_proc.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/command_dec.c b/src/shared/secondary/spp_worker_th/cmd_parser.c
similarity index 100%
rename from src/shared/secondary/spp_worker_th/command_dec.c
rename to src/shared/secondary/spp_worker_th/cmd_parser.c
diff --git a/src/shared/secondary/spp_worker_th/cmd_parser.h b/src/shared/secondary/spp_worker_th/cmd_parser.h
index 785fffe..b03a920 100644
--- a/src/shared/secondary/spp_worker_th/cmd_parser.h
+++ b/src/shared/secondary/spp_worker_th/cmd_parser.h
@@ -55,7 +55,7 @@ enum sppwk_action {
  * SPP command type.
  *
  * @attention This enumerated type must have the same order of command_list
- *            defined in command_dec.c
+ *            defined in cmd_parser.c
  */
 /*
  * TODO(yasufum) consider to divide because each of target of scope is
diff --git a/src/vf/Makefile b/src/vf/Makefile
index a8b60fb..8c1cb2b 100644
--- a/src/vf/Makefile
+++ b/src/vf/Makefile
@@ -19,7 +19,7 @@ SRCS-y += $(SPP_WKT_DIR)/string_buffer.c
 SRCS-y += $(SPP_WKT_DIR)/ringlatencystats.c
 SRCS-y += $(SPP_WKT_DIR)/spp_port.c
 SRCS-y += $(SPP_WKT_DIR)/command_conn.c
-SRCS-y += $(SPP_WKT_DIR)/command_dec.c
+SRCS-y += $(SPP_WKT_DIR)/cmd_parser.c
 SRCS-y += $(SPP_WKT_DIR)/command_proc.c
 SRCS-y += $(SPP_WKT_DIR)/spp_proc.c
 SRCS-y += ../shared/common.c
-- 
2.17.1


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

* [spp] [PATCH v2 2/5] shared/sec: remove no meaning str defines
  2019-05-15  5:44 ` [spp] [PATCH v2 0/5] Fix typo of status command ogawa.yasufumi
  2019-05-15  5:44   ` [spp] [PATCH v2 1/5] shared/sec: rename src command_dec to cmd_parser ogawa.yasufumi
@ 2019-05-15  5:44   ` ogawa.yasufumi
  2019-05-15  5:44   ` [spp] [PATCH v2 3/5] shared/sec: rename lists of fixed strings ogawa.yasufumi
                     ` (2 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: ogawa.yasufumi @ 2019-05-15  5:44 UTC (permalink / raw)
  To: spp, ferruh.yigit, ogawa.yasufumi

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

Defines of fixed string, such as `#define SPP_COMMAND_EXIT "exit"`, are
no meaning because this long define is referred just one time. In
addition, there are other similar defines which are actually required.
It makes code complex and hard to maintain. This update is to remove
such kind of defines.

Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
 .../secondary/spp_worker_th/cmd_parser.c      | 97 ++++++-------------
 1 file changed, 29 insertions(+), 68 deletions(-)

diff --git a/src/shared/secondary/spp_worker_th/cmd_parser.c b/src/shared/secondary/spp_worker_th/cmd_parser.c
index 723d8c4..6ce60f9 100644
--- a/src/shared/secondary/spp_worker_th/cmd_parser.c
+++ b/src/shared/secondary/spp_worker_th/cmd_parser.c
@@ -14,46 +14,15 @@
 #define RTE_LOGTYPE_SPP_COMMAND_PROC RTE_LOGTYPE_USER1
 #define RTE_LOGTYPE_APP RTE_LOGTYPE_USER2
 
-/* command string  */
-#define SPP_COMMAND_CLASSFIER_TABLE_STR "classifier_table"
-#define SPP_COMMAND_GET_CLIENT_ID_STR   "_get_client_id"
-#define SPP_COMMAND_STATUS_STR          "status"
-#define SPP_COMMAND_EXIT_STR            "exit"
-#define SPP_COMMAND_COMPONENT_STR       "component"
-#define SPP_COMMAND_PORT_STR            "port"
-
-/* classifiler_type string */
-#define SPP_CLASSIFLER_NONE_STR         "none"
-#define SPP_CLASSIFLER_MAC_STR          "mac"
-#define SPP_CLASSIFLER_VLAN_STR         "vlan"
-
-/* command action string */
-#define SPP_ACTION_NONE_STR             "none"
-#define SPP_ACTION_START_STR            "start"
-#define SPP_ACTION_STOP_STR             "stop"
-#define SPP_ACTION_ADD_STR              "add"
-#define SPP_ACTION_DEL_STR              "del"
-
-/* port rx/tx string */
-#define SPP_PORT_RXTX_NONE_STR          "none"
-#define SPP_PORT_RXTX_RX_STR            "rx"
-#define SPP_PORT_RXTX_TX_STR            "tx"
-
-/* port ability string */
-#define SPP_ABILITY_NONE_STR            "none"
-#define SPP_ABILITY_ADD_VLANTAG_STR     "add_vlantag"
-#define SPP_ABILITY_DEL_VLANTAG_STR     "del_vlantag"
-
 /*
- * classifier type string list
- * do it same as the order of enum spp_classifier_type (spp_proc.h)
+ * List of classifier type. The order of items should be same as the order of
+ * enum `spp_classifier_type` defined in spp_proc.h.
  */
 const char *CLASSIFILER_TYPE_STRINGS[] = {
-	SPP_CLASSIFLER_NONE_STR,
-	SPP_CLASSIFLER_MAC_STR,
-	SPP_CLASSIFLER_VLAN_STR,
-
-	/* termination */ "",
+	"none",
+	"mac",
+	"vlan",
+	"",  /* termination */
 };
 
 /**
@@ -61,24 +30,23 @@ const char *CLASSIFILER_TYPE_STRINGS[] = {
  * enum `sppwk_action` in cmd_parser.h.
  */
 const char *COMMAND_ACTION_STRINGS[] = {
-	SPP_ACTION_NONE_STR,
-	SPP_ACTION_START_STR,
-	SPP_ACTION_STOP_STR,
-	SPP_ACTION_ADD_STR,
-	SPP_ACTION_DEL_STR,
+	"none",
+	"start",
+	"stop",
+	"add",
+	"del",
 	"",  /* termination */
 };
 
 /*
- * port rxtx string list
- * do it same as the order of enum spp_port_rxtx (spp_vf.h)
+ * List of port type. The order of items should be same as the order of
+ * enum `spp_port_rxtx` in spp_vf.h.
  */
 const char *PORT_RXTX_STRINGS[] = {
-	SPP_PORT_RXTX_NONE_STR,
-	SPP_PORT_RXTX_RX_STR,
-	SPP_PORT_RXTX_TX_STR,
-
-	/* termination */ "",
+	"none",
+	"rx",
+	"tx",
+	"",  /* termination */
 };
 
 /*
@@ -86,11 +54,10 @@ const char *PORT_RXTX_STRINGS[] = {
  * do it same as the order of enum spp_port_ability_type (spp_vf.h)
  */
 const char *PORT_ABILITY_STRINGS[] = {
-	SPP_ABILITY_NONE_STR,
-	SPP_ABILITY_ADD_VLANTAG_STR,
-	SPP_ABILITY_DEL_VLANTAG_STR,
-
-	/* termination */ "",
+	"none",
+	"add_vlantag",
+	"del_vlantag",
+	"",  /* termination */
 };
 
 /* Check mac address used on the port for registering or removing */
@@ -1005,20 +972,14 @@ struct decode_command_list {
 
 /* command list */
 static struct decode_command_list command_list[] = {
-	{ SPP_COMMAND_CLASSFIER_TABLE_STR, 5, 5,
-		decode_command_parameter_cls_table  },
-						/* classifier_table(mac)  */
-	{ SPP_COMMAND_CLASSFIER_TABLE_STR, 6, 6,
-		decode_command_parameter_cls_table_vlan },
-						/* classifier_table(vlan) */
-	{ SPP_COMMAND_GET_CLIENT_ID_STR, 1, 1, NULL }, /* _get_client_id  */
-	{ SPP_COMMAND_STATUS_STR,	 1, 1, NULL }, /* status          */
-	{ SPP_COMMAND_EXIT_STR,		 1, 1, NULL }, /* exit            */
-	{ SPP_COMMAND_COMPONENT_STR,	 3, 5,
-		decode_command_parameter_component  }, /* component       */
-	{ SPP_COMMAND_PORT_STR,		 5, 8,
-		decode_command_parameter_port       }, /* port            */
-	{ "",				 0, 0, NULL }  /* termination     */
+	{ "classifier_table", 5, 5, decode_command_parameter_cls_table },
+	{ "classifier_table", 6, 6, decode_command_parameter_cls_table_vlan },
+	{ "_get_client_id", 1, 1, NULL },
+	{ "status", 1, 1, NULL },
+	{ "exit", 1, 1, NULL },
+	{ "component", 3, 5, decode_command_parameter_component },
+	{ "port", 5, 8, decode_command_parameter_port },
+	{ "", 0, 0, NULL }  /* termination */
 };
 
 /* Parse command line parameters. */
-- 
2.17.1


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

* [spp] [PATCH v2 3/5] shared/sec: rename lists of fixed strings
  2019-05-15  5:44 ` [spp] [PATCH v2 0/5] Fix typo of status command ogawa.yasufumi
  2019-05-15  5:44   ` [spp] [PATCH v2 1/5] shared/sec: rename src command_dec to cmd_parser ogawa.yasufumi
  2019-05-15  5:44   ` [spp] [PATCH v2 2/5] shared/sec: remove no meaning str defines ogawa.yasufumi
@ 2019-05-15  5:44   ` ogawa.yasufumi
  2019-05-15  5:44   ` [spp] [PATCH v2 4/5] shared/sec: rename struct for attrs of classify ogawa.yasufumi
  2019-05-15  5:44   ` [spp] [PATCH v2 5/5] shared/sec: rename sppwk port util functions ogawa.yasufumi
  4 siblings, 0 replies; 12+ messages in thread
From: ogawa.yasufumi @ 2019-05-15  5:44 UTC (permalink / raw)
  To: spp, ferruh.yigit, ogawa.yasufumi

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

As redundant defines are removed, rename lists of fixed strings named
`*_STRINGS`, such as `PORT_ABILITY_STATUS_STRINGS`, to `*_LIST` because
it is not needed to declare as string explicitly.

Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
 .../secondary/spp_worker_th/cmd_parser.c      | 51 ++++++++++---------
 .../secondary/spp_worker_th/command_proc.c    | 41 ++++++++-------
 src/shared/secondary/spp_worker_th/spp_proc.h |  1 +
 3 files changed, 47 insertions(+), 46 deletions(-)

diff --git a/src/shared/secondary/spp_worker_th/cmd_parser.c b/src/shared/secondary/spp_worker_th/cmd_parser.c
index 6ce60f9..d617276 100644
--- a/src/shared/secondary/spp_worker_th/cmd_parser.c
+++ b/src/shared/secondary/spp_worker_th/cmd_parser.c
@@ -14,22 +14,11 @@
 #define RTE_LOGTYPE_SPP_COMMAND_PROC RTE_LOGTYPE_USER1
 #define RTE_LOGTYPE_APP RTE_LOGTYPE_USER2
 
-/*
- * List of classifier type. The order of items should be same as the order of
- * enum `spp_classifier_type` defined in spp_proc.h.
- */
-const char *CLASSIFILER_TYPE_STRINGS[] = {
-	"none",
-	"mac",
-	"vlan",
-	"",  /* termination */
-};
-
 /**
  * List of command action. The order of items should be same as the order of
  * enum `sppwk_action` in cmd_parser.h.
  */
-const char *COMMAND_ACTION_STRINGS[] = {
+const char *CMD_ACT_LIST[] = {
 	"none",
 	"start",
 	"stop",
@@ -38,22 +27,34 @@ const char *COMMAND_ACTION_STRINGS[] = {
 	"",  /* termination */
 };
 
-/*
- * List of port type. The order of items should be same as the order of
+/**
+ * List of classifier type. The order of items should be same as the order of
+ * enum `spp_classifier_type` defined in spp_proc.h.
+ */
+/* TODO(yasufum) fix sinmilar var in command_proc.c */
+const char *CLS_TYPE_LIST[] = {
+	"none",
+	"mac",
+	"vlan",
+	"",  /* termination */
+};
+
+/**
+ * List of port direction. The order of items should be same as the order of
  * enum `spp_port_rxtx` in spp_vf.h.
  */
-const char *PORT_RXTX_STRINGS[] = {
+const char *PORT_DIR_LIST[] = {
 	"none",
 	"rx",
 	"tx",
 	"",  /* termination */
 };
 
-/*
- * port ability string list
- * do it same as the order of enum spp_port_ability_type (spp_vf.h)
+/**
+ * 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_STRINGS[] = {
+const char *PORT_ABILITY_LIST[] = {
 	"none",
 	"add_vlantag",
 	"del_vlantag",
@@ -311,7 +312,7 @@ decode_component_action_value(void *output, const char *arg_val,
 				int allow_override __attribute__ ((unused)))
 {
 	int ret = SPP_RET_OK;
-	ret = get_arrary_index(arg_val, COMMAND_ACTION_STRINGS);
+	ret = get_arrary_index(arg_val, CMD_ACT_LIST);
 	if (unlikely(ret <= 0)) {
 		RTE_LOG(ERR, SPP_COMMAND_PROC,
 				"Unknown component action. val=%s\n",
@@ -397,7 +398,7 @@ decode_port_action_value(void *output, const char *arg_val,
 				int allow_override __attribute__ ((unused)))
 {
 	int ret = SPP_RET_OK;
-	ret = get_arrary_index(arg_val, COMMAND_ACTION_STRINGS);
+	ret = get_arrary_index(arg_val, CMD_ACT_LIST);
 	if (unlikely(ret <= 0)) {
 		RTE_LOG(ERR, SPP_COMMAND_PROC,
 				"Unknown port action. val=%s\n",
@@ -457,7 +458,7 @@ decode_port_rxtx_value(void *output, const char *arg_val, int allow_override)
 	int ret = SPP_RET_OK;
 	struct sppwk_cmd_port *port = output;
 
-	ret = get_arrary_index(arg_val, PORT_RXTX_STRINGS);
+	ret = get_arrary_index(arg_val, PORT_DIR_LIST);
 	if (unlikely(ret <= 0)) {
 		RTE_LOG(ERR, SPP_COMMAND_PROC, "Unknown port rxtx. val=%s\n",
 				arg_val);
@@ -508,7 +509,7 @@ decode_port_vlan_operation(void *output, const char *arg_val,
 
 	switch (ability->ope) {
 	case SPP_PORT_ABILITY_OPE_NONE:
-		ret = get_arrary_index(arg_val, PORT_ABILITY_STRINGS);
+		ret = get_arrary_index(arg_val, PORT_ABILITY_LIST);
 		if (unlikely(ret <= 0)) {
 			RTE_LOG(ERR, SPP_COMMAND_PROC,
 					"Unknown port ability. val=%s\n",
@@ -613,7 +614,7 @@ decode_classifier_action_value(void *output, const char *arg_val,
 				int allow_override __attribute__ ((unused)))
 {
 	int ret = SPP_RET_OK;
-	ret = get_arrary_index(arg_val, COMMAND_ACTION_STRINGS);
+	ret = get_arrary_index(arg_val, CMD_ACT_LIST);
 	if (unlikely(ret <= 0)) {
 		RTE_LOG(ERR, SPP_COMMAND_PROC, "Unknown port action. val=%s\n",
 				arg_val);
@@ -637,7 +638,7 @@ decode_classifier_type_value(void *output, const char *arg_val,
 				int allow_override __attribute__ ((unused)))
 {
 	int ret = SPP_RET_OK;
-	ret = get_arrary_index(arg_val, CLASSIFILER_TYPE_STRINGS);
+	ret = get_arrary_index(arg_val, CLS_TYPE_LIST);
 	if (unlikely(ret <= 0)) {
 		RTE_LOG(ERR, SPP_COMMAND_PROC,
 				"Unknown classifier type. val=%s\n",
diff --git a/src/shared/secondary/spp_worker_th/command_proc.c b/src/shared/secondary/spp_worker_th/command_proc.c
index f345759..9df4cdc 100644
--- a/src/shared/secondary/spp_worker_th/command_proc.c
+++ b/src/shared/secondary/spp_worker_th/command_proc.c
@@ -63,40 +63,39 @@ struct command_response_list {
 	int (*func)(const char *name, char **output, void *tmp);
 };
 
-/*
- * seconary type string list
- * do it same the order enum secondary_type (spp_proc.h)
+/**
+ * List of worker process type. The order of items should be same as the order
+ * of enum `secondary_type` in spp_proc.h.
  */
-const char *SECONDARY_PROCESS_TYPE_SRINGS[] = {
+/* TODO(yasufum) rename `secondary_type` to `sppwk_proc_type`. */
+const char *SPPWK_PROC_TYPE_LIST[] = {
 	"none",
 	"vf",
 	"mirror",
-
-	/* termination */ "",
+	"",  /* termination */
 };
 
-/*
- * port ability string list
- * do it same as the order of enum spp_port_ability_type (spp_vf.h)
+/**
+ * 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_STATUS_STRINGS[] = {
+const char *PORT_ABILITY_STAT_LIST[] = {
 	"none",
 	"add",
 	"del",
-
-	/* termination */ "",
+	"",  /* termination */
 };
 
-/*
- * classifier type string list
- * do it same as the order of enum spp_classifier_type (spp_vf.h)
+/**
+ * List of classifier type. The order of items should be same as the order of
+ * enum `spp_classifier_type` defined in spp_proc.h.
  */
-const char *CLASSIFILER_TYPE_STATUS_STRINGS[] = {
+/* TODO(yasufum) fix similar var in cmd_parser.c */
+const char *CLS_TYPE_A_LIST[] = {
 	"none",
 	"mac",
 	"vlan",
-
-	/* termination */ "",
+	"",  /* termination */
 };
 
 /* get client id */
@@ -969,7 +968,7 @@ append_process_type_value(const char *name, char **output,
 		void *tmp __attribute__ ((unused)))
 {
 	return append_json_str_value(name, output,
-			SECONDARY_PROCESS_TYPE_SRINGS[spp_get_process_type()]);
+			SPPWK_PROC_TYPE_LIST[spp_get_process_type()]);
 }
 
 /* append a list of interface numbers for JSON format */
@@ -1012,7 +1011,7 @@ 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,
-			PORT_ABILITY_STATUS_STRINGS[ope]);
+			PORT_ABILITY_STAT_LIST[ope]);
 	if (unlikely(ret < SPP_RET_OK))
 		return SPP_RET_NG;
 
@@ -1269,7 +1268,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,
-			CLASSIFILER_TYPE_STATUS_STRINGS[type]);
+			CLS_TYPE_A_LIST[type]);
 	if (unlikely(ret < SPP_RET_OK))
 		return ret;
 
diff --git a/src/shared/secondary/spp_worker_th/spp_proc.h b/src/shared/secondary/spp_worker_th/spp_proc.h
index d7952fb..bca2c0e 100644
--- a/src/shared/secondary/spp_worker_th/spp_proc.h
+++ b/src/shared/secondary/spp_worker_th/spp_proc.h
@@ -149,6 +149,7 @@ enum copy_mng_flg {
 };
 
 /* secondary process type used only from spp_vf and spp_mirror */
+/* TODO(yasufum) rename `secondary_type` to `sppwk_proc_type`. */
 enum secondary_type {
 	SECONDARY_TYPE_NONE,
 	SECONDARY_TYPE_VF,
-- 
2.17.1


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

* [spp] [PATCH v2 4/5] shared/sec: rename struct for attrs of classify
  2019-05-15  5:44 ` [spp] [PATCH v2 0/5] Fix typo of status command ogawa.yasufumi
                     ` (2 preceding siblings ...)
  2019-05-15  5:44   ` [spp] [PATCH v2 3/5] shared/sec: rename lists of fixed strings ogawa.yasufumi
@ 2019-05-15  5:44   ` ogawa.yasufumi
  2019-05-15  5:44   ` [spp] [PATCH v2 5/5] shared/sec: rename sppwk port util functions ogawa.yasufumi
  4 siblings, 0 replies; 12+ messages in thread
From: ogawa.yasufumi @ 2019-05-15  5:44 UTC (permalink / raw)
  To: spp, ferruh.yigit, ogawa.yasufumi

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

Struct `spp_port_class_identifier` has members of MAC address and VLAN
tag used for classifying, so the name is not appropriate because it is
not an identifier but a set of attributes. This update is to fix the
issue.

Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
 .../secondary/spp_worker_th/cmd_parser.c      | 31 +++++++++----------
 .../secondary/spp_worker_th/cmd_parser.h      |  4 +--
 .../secondary/spp_worker_th/command_proc.c    | 28 ++++++++---------
 src/shared/secondary/spp_worker_th/spp_proc.c | 24 +++++++-------
 src/shared/secondary/spp_worker_th/spp_proc.h | 12 +++----
 src/vf/classifier_mac.c                       |  8 ++---
 6 files changed, 53 insertions(+), 54 deletions(-)

diff --git a/src/shared/secondary/spp_worker_th/cmd_parser.c b/src/shared/secondary/spp_worker_th/cmd_parser.c
index d617276..4c5a9d2 100644
--- a/src/shared/secondary/spp_worker_th/cmd_parser.c
+++ b/src/shared/secondary/spp_worker_th/cmd_parser.c
@@ -61,18 +61,17 @@ const char *PORT_ABILITY_LIST[] = {
 	"",  /* termination */
 };
 
-/* Check mac address used on the port for registering or removing */
+/* Return 1 as true if port is used with given mac_addr and vid. */
 static int
-spp_check_classid_used_port(
+is_used_with_addr(
 		int vid, uint64_t mac_addr,
 		enum port_type iface_type, int iface_no)
 {
-	struct sppwk_port_info *port_info = get_iface_info(
+	struct sppwk_port_info *wk_port = get_iface_info(
 			iface_type, iface_no);
 
-	/* Return true if given mac_addr matches with port_info, and vid. */
-	return ((mac_addr == port_info->class_id.mac_addr) &&
-		(vid == port_info->class_id.vlantag.vid));
+	return ((mac_addr == wk_port->cls_attrs.mac_addr) &&
+		(vid == wk_port->cls_attrs.vlantag.vid));
 }
 
 /* Check if port has been added. */
@@ -671,7 +670,7 @@ parse_cls_port(void *cls_cmd_attr, const char *arg_val,
 		int allow_override __attribute__ ((unused)))
 {
 	int ret = SPP_RET_OK;
-	struct sppwk_cls_cmd_attr *cls_attr = cls_cmd_attr;
+	struct sppwk_cls_cmd_attrs *cls_attrs = cls_cmd_attr;
 	struct sppwk_port_idx tmp_port;
 	int64_t mac_addr = 0;
 
@@ -686,23 +685,23 @@ parse_cls_port(void *cls_cmd_attr, const char *arg_val,
 		return SPP_RET_NG;
 	}
 
-	if (cls_attr->type == SPP_CLASSIFIER_TYPE_MAC)
-		cls_attr->vid = ETH_VLAN_ID_MAX;
+	if (cls_attrs->type == SPP_CLASSIFIER_TYPE_MAC)
+		cls_attrs->vid = ETH_VLAN_ID_MAX;
 
-	if (unlikely(cls_attr->wk_action == SPPWK_ACT_ADD)) {
-		if (!spp_check_classid_used_port(ETH_VLAN_ID_MAX, 0,
+	if (unlikely(cls_attrs->wk_action == SPPWK_ACT_ADD)) {
+		if (!is_used_with_addr(ETH_VLAN_ID_MAX, 0,
 				tmp_port.iface_type, tmp_port.iface_no)) {
 			RTE_LOG(ERR, SPP_COMMAND_PROC, "Port in used. "
 					"(classifier_table command) val=%s\n",
 					arg_val);
 			return SPP_RET_NG;
 		}
-	} else if (unlikely(cls_attr->wk_action == SPPWK_ACT_DEL)) {
-		mac_addr = spp_change_mac_str_to_int64(cls_attr->mac);
+	} else if (unlikely(cls_attrs->wk_action == SPPWK_ACT_DEL)) {
+		mac_addr = spp_change_mac_str_to_int64(cls_attrs->mac);
 		if (mac_addr < 0)
 			return SPP_RET_NG;
 
-		if (!spp_check_classid_used_port(cls_attr->vid,
+		if (!is_used_with_addr(cls_attrs->vid,
 				(uint64_t)mac_addr,
 				tmp_port.iface_type, tmp_port.iface_no)) {
 			RTE_LOG(ERR, SPP_COMMAND_PROC, "Port in used. "
@@ -712,8 +711,8 @@ parse_cls_port(void *cls_cmd_attr, const char *arg_val,
 		}
 	}
 
-	cls_attr->port.iface_type = tmp_port.iface_type;
-	cls_attr->port.iface_no   = tmp_port.iface_no;
+	cls_attrs->port.iface_type = tmp_port.iface_type;
+	cls_attrs->port.iface_no   = tmp_port.iface_no;
 	return SPP_RET_OK;
 }
 
diff --git a/src/shared/secondary/spp_worker_th/cmd_parser.h b/src/shared/secondary/spp_worker_th/cmd_parser.h
index b03a920..58e39a9 100644
--- a/src/shared/secondary/spp_worker_th/cmd_parser.h
+++ b/src/shared/secondary/spp_worker_th/cmd_parser.h
@@ -73,7 +73,7 @@ enum sppwk_cmd_type {
 };
 
 /* `classifier_table` command specific parameters. */
-struct sppwk_cls_cmd_attr {
+struct sppwk_cls_cmd_attrs {
 	enum sppwk_action wk_action;  /**< add or del */
 	enum spp_classifier_type type;  /**< currently only for mac */
 	int vid;  /**< VLAN ID  */
@@ -108,7 +108,7 @@ struct spp_command {
 	enum sppwk_cmd_type type; /**< command type */
 
 	union {  /**< command descriptors */
-		struct sppwk_cls_cmd_attr cls_table;
+		struct sppwk_cls_cmd_attrs cls_table;
 		struct sppwk_cmd_flush flush;
 		struct sppwk_cmd_comp comp;
 		struct sppwk_cmd_port port;
diff --git a/src/shared/secondary/spp_worker_th/command_proc.c b/src/shared/secondary/spp_worker_th/command_proc.c
index 9df4cdc..ab0bbb3 100644
--- a/src/shared/secondary/spp_worker_th/command_proc.c
+++ b/src/shared/secondary/spp_worker_th/command_proc.c
@@ -167,48 +167,48 @@ spp_update_classifier_table(
 
 	if (wk_action == SPPWK_ACT_DEL) {
 		/* Delete */
-		if ((port_info->class_id.vlantag.vid != 0) &&
-				unlikely(port_info->class_id.vlantag.vid !=
+		if ((port_info->cls_attrs.vlantag.vid != 0) &&
+				unlikely(port_info->cls_attrs.vlantag.vid !=
 				vid)) {
 			RTE_LOG(ERR, APP, "VLAN ID is different. "
 					"( vid = %d )\n", vid);
 			return SPP_RET_NG;
 		}
-		if ((port_info->class_id.mac_addr != 0) &&
-			unlikely(port_info->class_id.mac_addr !=
+		if ((port_info->cls_attrs.mac_addr != 0) &&
+			unlikely(port_info->cls_attrs.mac_addr !=
 					mac_addr)) {
 			RTE_LOG(ERR, APP, "MAC address is different. "
 					"( mac = %s )\n", mac_addr_str);
 			return SPP_RET_NG;
 		}
 
-		port_info->class_id.vlantag.vid = ETH_VLAN_ID_MAX;
-		port_info->class_id.mac_addr    = 0;
-		memset(port_info->class_id.mac_addr_str, 0x00,
+		port_info->cls_attrs.vlantag.vid = ETH_VLAN_ID_MAX;
+		port_info->cls_attrs.mac_addr    = 0;
+		memset(port_info->cls_attrs.mac_addr_str, 0x00,
 							SPP_MIN_STR_LEN);
 
 	} else if (wk_action == SPPWK_ACT_ADD) {
 		/* Setting */
-		if (unlikely(port_info->class_id.vlantag.vid !=
+		if (unlikely(port_info->cls_attrs.vlantag.vid !=
 				ETH_VLAN_ID_MAX)) {
 			RTE_LOG(ERR, APP, "Port in used. "
 					"( port = %d:%d, vlan = %d != %d )\n",
 					port->iface_type, port->iface_no,
-					port_info->class_id.vlantag.vid, vid);
+					port_info->cls_attrs.vlantag.vid, vid);
 			return SPP_RET_NG;
 		}
-		if (unlikely(port_info->class_id.mac_addr != 0)) {
+		if (unlikely(port_info->cls_attrs.mac_addr != 0)) {
 			RTE_LOG(ERR, APP, "Port in used. "
 					"( port = %d:%d, mac = %s != %s )\n",
 					port->iface_type, port->iface_no,
-					port_info->class_id.mac_addr_str,
+					port_info->cls_attrs.mac_addr_str,
 					mac_addr_str);
 			return SPP_RET_NG;
 		}
 
-		port_info->class_id.vlantag.vid = vid;
-		port_info->class_id.mac_addr    = mac_addr;
-		strcpy(port_info->class_id.mac_addr_str, mac_addr_str);
+		port_info->cls_attrs.vlantag.vid = vid;
+		port_info->cls_attrs.mac_addr    = mac_addr;
+		strcpy(port_info->cls_attrs.mac_addr_str, mac_addr_str);
 	}
 
 	set_component_change_port(port_info, SPP_PORT_RXTX_TX);
diff --git a/src/shared/secondary/spp_worker_th/spp_proc.c b/src/shared/secondary/spp_worker_th/spp_proc.c
index 18acd64..370f071 100644
--- a/src/shared/secondary/spp_worker_th/spp_proc.c
+++ b/src/shared/secondary/spp_worker_th/spp_proc.c
@@ -355,9 +355,9 @@ dump_interface_info(const struct iface_info *iface_info)
 				"vid = %u, mac=%08lx(%s)\n",
 				cnt, port->iface_type, port->iface_no,
 				port->ethdev_port_id,
-				port->class_id.vlantag.vid,
-				port->class_id.mac_addr,
-				port->class_id.mac_addr_str);
+				port->cls_attrs.vlantag.vid,
+				port->cls_attrs.mac_addr,
+				port->cls_attrs.mac_addr_str);
 	}
 	for (cnt = 0; cnt < RTE_MAX_ETHPORTS; cnt++) {
 		port = &iface_info->vhost[cnt];
@@ -368,9 +368,9 @@ dump_interface_info(const struct iface_info *iface_info)
 				"vid = %u, mac=%08lx(%s)\n",
 				cnt, port->iface_type, port->iface_no,
 				port->ethdev_port_id,
-				port->class_id.vlantag.vid,
-				port->class_id.mac_addr,
-				port->class_id.mac_addr_str);
+				port->cls_attrs.vlantag.vid,
+				port->cls_attrs.mac_addr,
+				port->cls_attrs.mac_addr_str);
 	}
 	for (cnt = 0; cnt < RTE_MAX_ETHPORTS; cnt++) {
 		port = &iface_info->ring[cnt];
@@ -381,9 +381,9 @@ dump_interface_info(const struct iface_info *iface_info)
 				"vid = %u, mac=%08lx(%s)\n",
 				cnt, port->iface_type, port->iface_no,
 				port->ethdev_port_id,
-				port->class_id.vlantag.vid,
-				port->class_id.mac_addr,
-				port->class_id.mac_addr_str);
+				port->cls_attrs.vlantag.vid,
+				port->cls_attrs.mac_addr,
+				port->cls_attrs.mac_addr_str);
 	}
 }
 
@@ -476,17 +476,17 @@ init_iface_info(void)
 		p_iface_info->nic[port_cnt].iface_type = UNDEF;
 		p_iface_info->nic[port_cnt].iface_no = port_cnt;
 		p_iface_info->nic[port_cnt].ethdev_port_id = -1;
-		p_iface_info->nic[port_cnt].class_id.vlantag.vid =
+		p_iface_info->nic[port_cnt].cls_attrs.vlantag.vid =
 			ETH_VLAN_ID_MAX;
 		p_iface_info->vhost[port_cnt].iface_type = UNDEF;
 		p_iface_info->vhost[port_cnt].iface_no = port_cnt;
 		p_iface_info->vhost[port_cnt].ethdev_port_id = -1;
-		p_iface_info->vhost[port_cnt].class_id.vlantag.vid =
+		p_iface_info->vhost[port_cnt].cls_attrs.vlantag.vid =
 			ETH_VLAN_ID_MAX;
 		p_iface_info->ring[port_cnt].iface_type = UNDEF;
 		p_iface_info->ring[port_cnt].iface_no = port_cnt;
 		p_iface_info->ring[port_cnt].ethdev_port_id = -1;
-		p_iface_info->ring[port_cnt].class_id.vlantag.vid =
+		p_iface_info->ring[port_cnt].cls_attrs.vlantag.vid =
 			ETH_VLAN_ID_MAX;
 	}
 }
diff --git a/src/shared/secondary/spp_worker_th/spp_proc.h b/src/shared/secondary/spp_worker_th/spp_proc.h
index bca2c0e..abf16e7 100644
--- a/src/shared/secondary/spp_worker_th/spp_proc.h
+++ b/src/shared/secondary/spp_worker_th/spp_proc.h
@@ -179,11 +179,11 @@ struct spp_port_ability {
 	union spp_ability_data data;   /**< Port ability data */
 };
 
-/** Port class identifier for classifying */
-struct spp_port_class_identifier {
-	uint64_t mac_addr;                      /**< Mac address (binary) */
-	char     mac_addr_str[SPP_MIN_STR_LEN]; /**< Mac address (text) */
-	struct spp_vlantag_info vlantag;        /**< VLAN tag information */
+/* Attributes for classifying . */
+struct sppwk_cls_attrs {
+	uint64_t mac_addr;  /**< Mac address (binary) */
+	char mac_addr_str[SPP_MIN_STR_LEN];  /**< Mac address (text) */
+	struct spp_vlantag_info vlantag;   /**< VLAN tag information */
 };
 
 /**
@@ -200,7 +200,7 @@ struct sppwk_port_info {
 	enum port_type iface_type;  /**< phy, vhost or ring */
 	int iface_no;
 	int ethdev_port_id;  /**< Consistent ID of ethdev */
-	struct spp_port_class_identifier class_id;
+	struct sppwk_cls_attrs cls_attrs;
 	struct spp_port_ability ability[SPP_PORT_ABILITY_MAX];
 };
 
diff --git a/src/vf/classifier_mac.c b/src/vf/classifier_mac.c
index 1e98636..9fd8da1 100644
--- a/src/vf/classifier_mac.c
+++ b/src/vf/classifier_mac.c
@@ -356,7 +356,7 @@ init_component_info(struct component_info *cmp_info,
 	cmp_info->mac_addr_entry = 0;
 	for (i = 0; i < component_info->num_tx_port; i++) {
 		tx_port = component_info->tx_ports[i];
-		vid = tx_port->class_id.vlantag.vid;
+		vid = tx_port->cls_attrs.vlantag.vid;
 
 		/* store ports information */
 		clsd_data_tx[i].iface_type      = tx_port->iface_type;
@@ -365,7 +365,7 @@ init_component_info(struct component_info *cmp_info,
 		clsd_data_tx[i].port            = tx_port->ethdev_port_id;
 		clsd_data_tx[i].num_pkt         = 0;
 
-		if (tx_port->class_id.mac_addr == 0)
+		if (tx_port->cls_attrs.mac_addr == 0)
 			continue;
 
 		/* if mac classification is NULL, make instance */
@@ -389,7 +389,7 @@ init_component_info(struct component_info *cmp_info,
 		cmp_info->mac_addr_entry = 1;
 
 		/* store default classified */
-		if (unlikely(tx_port->class_id.mac_addr ==
+		if (unlikely(tx_port->cls_attrs.mac_addr ==
 				SPP_DEFAULT_CLASSIFIED_DMY_ADDR)) {
 			mac_cls->default_classified = i;
 			RTE_LOG(INFO, SPP_CLASSIFIER_MAC,
@@ -404,7 +404,7 @@ init_component_info(struct component_info *cmp_info,
 		}
 
 		/* add entry to classifier mac table */
-		rte_memcpy(&eth_addr, &tx_port->class_id.mac_addr,
+		rte_memcpy(&eth_addr, &tx_port->cls_attrs.mac_addr,
 				ETHER_ADDR_LEN);
 		ether_format_addr(mac_addr_str, sizeof(mac_addr_str),
 				&eth_addr);
-- 
2.17.1


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

* [spp] [PATCH v2 5/5] shared/sec: rename sppwk port util functions
  2019-05-15  5:44 ` [spp] [PATCH v2 0/5] Fix typo of status command ogawa.yasufumi
                     ` (3 preceding siblings ...)
  2019-05-15  5:44   ` [spp] [PATCH v2 4/5] shared/sec: rename struct for attrs of classify ogawa.yasufumi
@ 2019-05-15  5:44   ` ogawa.yasufumi
  4 siblings, 0 replies; 12+ messages in thread
From: ogawa.yasufumi @ 2019-05-15  5:44 UTC (permalink / raw)
  To: spp, ferruh.yigit, ogawa.yasufumi

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

Some functions for managing sppwk port are prefixed `spp_` but file
local, or the name is not so intuitive. This update is to rename the
names.

* Change file local `spp_check_added_port()` to `is_added_port()`.

* Change file local `spp_convert_port_to_iface()` to
  `parse_resource_uid()`.

* Change `get_iface_info()` to `get_sppwk_port()` because the word
  `iface info` is ambiguous.

Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
 .../secondary/spp_worker_th/cmd_parser.c      | 73 +++++++++----------
 .../secondary/spp_worker_th/command_proc.c    |  6 +-
 src/shared/secondary/spp_worker_th/spp_proc.c |  9 +--
 src/shared/secondary/spp_worker_th/spp_proc.h | 15 +---
 4 files changed, 42 insertions(+), 61 deletions(-)

diff --git a/src/shared/secondary/spp_worker_th/cmd_parser.c b/src/shared/secondary/spp_worker_th/cmd_parser.c
index 4c5a9d2..6813666 100644
--- a/src/shared/secondary/spp_worker_th/cmd_parser.c
+++ b/src/shared/secondary/spp_worker_th/cmd_parser.c
@@ -67,72 +67,67 @@ is_used_with_addr(
 		int vid, uint64_t mac_addr,
 		enum port_type iface_type, int iface_no)
 {
-	struct sppwk_port_info *wk_port = get_iface_info(
+	struct sppwk_port_info *wk_port = get_sppwk_port(
 			iface_type, iface_no);
 
 	return ((mac_addr == wk_port->cls_attrs.mac_addr) &&
 		(vid == wk_port->cls_attrs.vlantag.vid));
 }
 
-/* Check if port has been added. */
+/* Return 1 as true if given port is already used. */
 static int
-spp_check_added_port(enum port_type iface_type, int iface_no)
+is_added_port(enum port_type iface_type, int iface_no)
 {
-	struct sppwk_port_info *port = get_iface_info(iface_type, iface_no);
+	struct sppwk_port_info *port = get_sppwk_port(iface_type, iface_no);
 	return port->iface_type != UNDEF;
 }
 
 /**
- * Separate port id of combination of iface type and number and
- * assign to given argument, iface_type and iface_no.
- *
- * For instance, 'ring:0' is separated to 'ring' and '0'.
+ * Separate resource UID of combination of iface type and number and assign to
+ * given argument, iface_type and iface_no. For instance, 'ring:0' is separated
+ * to 'ring' and '0'. The supported types are `phy`, `vhost` and `ring`.
  */
 static int
-spp_convert_port_to_iface(const char *port,
+parse_resource_uid(const char *res_uid,
 		    enum port_type *iface_type,
 		    int *iface_no)
 {
-	enum port_type type = UNDEF;
-	const char *no_str = NULL;
+	enum port_type ptype = UNDEF;
+	const char *iface_no_str = NULL;
 	char *endptr = NULL;
 
-	/* Find out which type of interface from port */
-	if (strncmp(port, SPP_IFTYPE_NIC_STR ":",
+	/**
+	 * TODO(yasufum) consider this checking of zero value is recommended
+	 * way, or should be changed.
+	 */
+	if (strncmp(res_uid, SPP_IFTYPE_NIC_STR ":",
 			strlen(SPP_IFTYPE_NIC_STR)+1) == 0) {
-		/* NIC */
-		type = PHY;
-		no_str = &port[strlen(SPP_IFTYPE_NIC_STR)+1];
-	} else if (strncmp(port, SPP_IFTYPE_VHOST_STR ":",
+		ptype = PHY;
+		iface_no_str = &res_uid[strlen(SPP_IFTYPE_NIC_STR)+1];
+	} else if (strncmp(res_uid, SPP_IFTYPE_VHOST_STR ":",
 			strlen(SPP_IFTYPE_VHOST_STR)+1) == 0) {
-		/* VHOST */
-		type = VHOST;
-		no_str = &port[strlen(SPP_IFTYPE_VHOST_STR)+1];
-	} else if (strncmp(port, SPP_IFTYPE_RING_STR ":",
+		ptype = VHOST;
+		iface_no_str = &res_uid[strlen(SPP_IFTYPE_VHOST_STR)+1];
+	} else if (strncmp(res_uid, SPP_IFTYPE_RING_STR ":",
 			strlen(SPP_IFTYPE_RING_STR)+1) == 0) {
-		/* RING */
-		type = RING;
-		no_str = &port[strlen(SPP_IFTYPE_RING_STR)+1];
+		ptype = RING;
+		iface_no_str = &res_uid[strlen(SPP_IFTYPE_RING_STR)+1];
 	} else {
-		/* OTHER */
-		RTE_LOG(ERR, APP, "Unknown interface type. (port = %s)\n",
-				port);
+		RTE_LOG(ERR, APP, "Unexpected port type in '%s'.\n", res_uid);
 		return SPP_RET_NG;
 	}
 
-	/* Change type of number of interface */
-	int ret_no = strtol(no_str, &endptr, 0);
-	if (unlikely(no_str == endptr) || unlikely(*endptr != '\0')) {
-		/* No IF number */
-		RTE_LOG(ERR, APP, "No interface number. (port = %s)\n", port);
+	int port_id = strtol(iface_no_str, &endptr, 0);
+	if (unlikely(iface_no_str == endptr) || unlikely(*endptr != '\0')) {
+		RTE_LOG(ERR, APP, "No interface number in '%s'.\n", res_uid);
 		return SPP_RET_NG;
 	}
 
-	*iface_type = type;
-	*iface_no = ret_no;
+	*iface_type = ptype;
+	*iface_no = port_id;
 
-	RTE_LOG(DEBUG, APP, "Port = %s => Type = %d No = %d\n",
-			port, *iface_type, *iface_no);
+	RTE_LOG(DEBUG, APP, "Parsed '%s' to '%d' and '%d'.\n",
+			res_uid, *iface_type, *iface_no);
 	return SPP_RET_OK;
 }
 
@@ -280,8 +275,7 @@ decode_port_value(void *output, const char *arg_val)
 {
 	int ret = SPP_RET_OK;
 	struct sppwk_port_idx *port = output;
-	ret = spp_convert_port_to_iface(arg_val, &port->iface_type,
-							&port->iface_no);
+	ret = parse_resource_uid(arg_val, &port->iface_type, &port->iface_no);
 	if (unlikely(ret != 0)) {
 		RTE_LOG(ERR, SPP_COMMAND_PROC, "Bad port. val=%s\n", arg_val);
 		return SPP_RET_NG;
@@ -678,8 +672,7 @@ parse_cls_port(void *cls_cmd_attr, const char *arg_val,
 	if (ret < SPP_RET_OK)
 		return SPP_RET_NG;
 
-	if (spp_check_added_port(tmp_port.iface_type,
-					tmp_port.iface_no) == 0) {
+	if (is_added_port(tmp_port.iface_type, tmp_port.iface_no) == 0) {
 		RTE_LOG(ERR, SPP_COMMAND_PROC, "Port not added. val=%s\n",
 				arg_val);
 		return SPP_RET_NG;
diff --git a/src/shared/secondary/spp_worker_th/command_proc.c b/src/shared/secondary/spp_worker_th/command_proc.c
index ab0bbb3..19f96d7 100644
--- a/src/shared/secondary/spp_worker_th/command_proc.c
+++ b/src/shared/secondary/spp_worker_th/command_proc.c
@@ -124,7 +124,7 @@ spp_get_process_type(void)
 static int
 spp_check_flush_port(enum port_type iface_type, int iface_no)
 {
-	struct sppwk_port_info *port = get_iface_info(iface_type, iface_no);
+	struct sppwk_port_info *port = get_sppwk_port(iface_type, iface_no);
 	return port->ethdev_port_id >= 0;
 }
 
@@ -153,7 +153,7 @@ spp_update_classifier_table(
 	}
 	mac_addr = (uint64_t)ret_mac;
 
-	port_info = get_iface_info(port->iface_type, port->iface_no);
+	port_info = get_sppwk_port(port->iface_type, port->iface_no);
 	if (unlikely(port_info == NULL)) {
 		RTE_LOG(ERR, APP, "No port. ( port = %d:%d )\n",
 				port->iface_type, port->iface_no);
@@ -390,7 +390,7 @@ spp_update_port(enum sppwk_action wk_action,
 	spp_get_mng_data_addr(NULL, NULL,
 			&comp_info_base, NULL, NULL, &change_component, NULL);
 	comp_info = (comp_info_base + component_id);
-	port_info = get_iface_info(port->iface_type, port->iface_no);
+	port_info = get_sppwk_port(port->iface_type, port->iface_no);
 	if (rxtx == SPP_PORT_RXTX_RX) {
 		num = &comp_info->num_rx_port;
 		ports = comp_info->rx_ports;
diff --git a/src/shared/secondary/spp_worker_th/spp_proc.c b/src/shared/secondary/spp_worker_th/spp_proc.c
index 370f071..53dd3f8 100644
--- a/src/shared/secondary/spp_worker_th/spp_proc.c
+++ b/src/shared/secondary/spp_worker_th/spp_proc.c
@@ -265,12 +265,11 @@ stop_process(int signal)
 }
 
 /**
- * Return port info of given type and num of interface
- *
- * It returns NULL value if given type is invalid.
+ * Return sppwk_port_info of given type and num of interface. It returns NULL
+ * if given type is invalid.
  */
 struct sppwk_port_info *
-get_iface_info(enum port_type iface_type, int iface_no)
+get_sppwk_port(enum port_type iface_type, int iface_no)
 {
 	struct iface_info *iface_info = g_mng_data_addr.p_iface_info;
 
@@ -663,7 +662,7 @@ spp_check_used_port(
 	int cnt, port_cnt, max = 0;
 	struct spp_component_info *component = NULL;
 	struct sppwk_port_info **port_array = NULL;
-	struct sppwk_port_info *port = get_iface_info(iface_type, iface_no);
+	struct sppwk_port_info *port = get_sppwk_port(iface_type, iface_no);
 	struct spp_component_info *component_info =
 					g_mng_data_addr.p_component_info;
 
diff --git a/src/shared/secondary/spp_worker_th/spp_proc.h b/src/shared/secondary/spp_worker_th/spp_proc.h
index abf16e7..aeb8e92 100644
--- a/src/shared/secondary/spp_worker_th/spp_proc.h
+++ b/src/shared/secondary/spp_worker_th/spp_proc.h
@@ -428,23 +428,12 @@ void set_all_core_status(enum spp_core_status status);
  *
  * @param signl
  *  received signal.
- *
  */
 void stop_process(int signal);
 
-/**
- * Return port info of given type and num of interface
- *
- * @param iface_type
- *  Interface type to be validated.
- * @param iface_no
- *  Interface number to be validated.
- *
- * @retval !NULL  sppwk_port_info.
- * @retval NULL   failed.
- */
+/* Return sppwk_port_info of given type and num of interface. */
 struct sppwk_port_info *
-get_iface_info(enum port_type iface_type, int iface_no);
+get_sppwk_port(enum port_type iface_type, int iface_no);
 
 /* Dump of core information */
 void dump_core_info(const struct core_mng_info *core_info);
-- 
2.17.1


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

end of thread, other threads:[~2019-05-15  5:47 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-10  8:35 [spp] [PATCH 0/5] Refactor source of cmd parser port utils ogawa.yasufumi
2019-05-10  8:35 ` [spp] [PATCH 1/5] shared/sec: rename src command_dec to cmd_parser ogawa.yasufumi
2019-05-10  8:35 ` [spp] [PATCH 2/5] shared/sec: remove no meaning str defines ogawa.yasufumi
2019-05-10  8:35 ` [spp] [PATCH 3/5] shared/sec: rename lists of fixed strings ogawa.yasufumi
2019-05-10  8:35 ` [spp] [PATCH 4/5] shared/sec: rename struct for attrs of classify ogawa.yasufumi
2019-05-10  8:35 ` [spp] [PATCH 5/5] shared/sec: rename sppwk port util functions ogawa.yasufumi
2019-05-15  5:44 ` [spp] [PATCH v2 0/5] Fix typo of status command ogawa.yasufumi
2019-05-15  5:44   ` [spp] [PATCH v2 1/5] shared/sec: rename src command_dec to cmd_parser ogawa.yasufumi
2019-05-15  5:44   ` [spp] [PATCH v2 2/5] shared/sec: remove no meaning str defines ogawa.yasufumi
2019-05-15  5:44   ` [spp] [PATCH v2 3/5] shared/sec: rename lists of fixed strings ogawa.yasufumi
2019-05-15  5:44   ` [spp] [PATCH v2 4/5] shared/sec: rename struct for attrs of classify ogawa.yasufumi
2019-05-15  5:44   ` [spp] [PATCH v2 5/5] shared/sec: rename sppwk port util functions ogawa.yasufumi

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