Soft Patch Panel
 help / color / Atom feed
* [spp] [PATCH] spp_pcap: remove global variable client_id
@ 2019-06-24 10:45 yasufum.o
  0 siblings, 0 replies; only message in thread
From: yasufum.o @ 2019-06-24 10:45 UTC (permalink / raw)
  To: spp, ferruh.yigit, yasufum.o

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

`client_id` is defined as a global variable, but it is better to use
get_client_id() instead. This update is to replace it.

This update is also try block in SPP controller to avoid CLI is
terminated if entry `client-id` is not found in status message.

Signed-off-by: Yasufumi Ogawa <yasufum.o@gmail.com>
---
 src/controller/commands/pri.py |  9 +++--
 src/controller/shell.py        | 66 ++++++++++++++++++----------------
 src/pcap/Makefile              |  1 +
 src/pcap/cmd_runner.c          | 19 +++-------
 src/pcap/cmd_utils.h           |  2 +-
 src/pcap/spp_pcap.c            | 35 ++++--------------
 src/spp-ctl/spp_ctl.py         |  1 +
 7 files changed, 56 insertions(+), 77 deletions(-)

diff --git a/src/controller/commands/pri.py b/src/controller/commands/pri.py
index b119a5c..6089137 100644
--- a/src/controller/commands/pri.py
+++ b/src/controller/commands/pri.py
@@ -301,9 +301,12 @@ class SppPrimary(object):
         res = self.spp_ctl_cli.get('processes')
         if res is not None:
             if res.status_code == 200:
-                for proc in res.json():
-                    if proc['type'] != 'primary':
-                        sec_ids.append(proc['client-id'])
+                try:
+                    for proc in res.json():
+                        if proc['type'] != 'primary':
+                            sec_ids.append(proc['client-id'])
+                except KeyError as e:
+                    print('Error: {} is not defined!'.format(e))
             elif res.status_code in self.spp_ctl_cli.rest_common_error_codes:
                 # Print default error message
                 pass
diff --git a/src/controller/shell.py b/src/controller/shell.py
index 3f4d95b..2288e1d 100644
--- a/src/controller/shell.py
+++ b/src/controller/shell.py
@@ -142,9 +142,12 @@ class Shell(cmd.Cmd, object):
         res = self.spp_ctl_cli.get('processes')
         if res is not None:
             if res.status_code == 200:
-                for ent in res.json():
-                    if ent['type'] == ptype:
-                        ids.append(ent['client-id'])
+                try:
+                    for ent in res.json():
+                        if ent['type'] == ptype:
+                            ids.append(ent['client-id'])
+                except KeyError as e:
+                    print('Error: {} is not defined!'.format(e))
         return ids
 
     def print_status(self):
@@ -156,34 +159,37 @@ class Shell(cmd.Cmd, object):
         res = self.spp_ctl_cli.get('processes')
         if res is not None:
             if res.status_code == 200:
-                proc_objs = res.json()
-                pri_obj = None
-                sec_obj = {}
-                sec_obj['nfv'] = []
-                sec_obj['vf'] = []
-                sec_obj['mirror'] = []
-                sec_obj['pcap'] = []
-
-                for proc_obj in proc_objs:
-                    if proc_obj['type'] == 'primary':
-                        pri_obj = proc_obj
+                try:
+                    proc_objs = res.json()
+                    pri_obj = None
+                    sec_obj = {}
+                    sec_obj['nfv'] = []
+                    sec_obj['vf'] = []
+                    sec_obj['mirror'] = []
+                    sec_obj['pcap'] = []
+
+                    for proc_obj in proc_objs:
+                        if proc_obj['type'] == 'primary':
+                            pri_obj = proc_obj
+                        else:
+                            sec_obj[proc_obj['type']].append(proc_obj)
+
+                    print('- primary:')
+                    if pri_obj is not None:
+                        print('  - status: running')
                     else:
-                        sec_obj[proc_obj['type']].append(proc_obj)
-
-                print('- primary:')
-                if pri_obj is not None:
-                    print('  - status: running')
-                else:
-                    print('  - status: not running')
-
-                print('- secondary:')
-                print('  - processes:')
-                cnt = 1
-                for pt in ['nfv', 'vf', 'mirror', 'pcap']:
-                    for obj in sec_obj[pt]:
-                        print('    %d: %s:%s' %
-                              (cnt, obj['type'], obj['client-id']))
-                        cnt += 1
+                        print('  - status: not running')
+
+                    print('- secondary:')
+                    print('  - processes:')
+                    cnt = 1
+                    for pt in ['nfv', 'vf', 'mirror', 'pcap']:
+                        for obj in sec_obj[pt]:
+                            print('    %d: %s:%s' %
+                                  (cnt, obj['type'], obj['client-id']))
+                            cnt += 1
+                except KeyError as e:
+                    print('Error: {} is not defined!'.format(e))
             elif res.status_code in self.spp_ctl_cli.rest_common_error_codes:
                 pass
             else:
diff --git a/src/pcap/Makefile b/src/pcap/Makefile
index 852d036..784ee1b 100644
--- a/src/pcap/Makefile
+++ b/src/pcap/Makefile
@@ -19,6 +19,7 @@ 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)/utils.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
diff --git a/src/pcap/cmd_runner.c b/src/pcap/cmd_runner.c
index 7f17d40..4c1891e 100644
--- a/src/pcap/cmd_runner.c
+++ b/src/pcap/cmd_runner.c
@@ -7,11 +7,12 @@
 
 #include <rte_log.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"
 #include "cmd_runner.h"
+#include "spp_pcap.h"
+#include "shared/secondary/utils.h"
+#include "shared/secondary/string_buffer.h"
+#include "shared/secondary/spp_worker_th/conn_spp_ctl.h"
 
 #define RTE_LOGTYPE_PCAP_RUNNER RTE_LOGTYPE_USER2
 
@@ -59,16 +60,6 @@ const char *CAPTURE_STATUS_STRINGS[] = {
 	"", /* termination */
 };
 
-/* get client id */
-static int
-spp_get_client_id(void)
-{
-	struct startup_param *startup_param;
-
-	spp_get_mng_data_addr(&startup_param, NULL, NULL, NULL, NULL);
-	return startup_param->client_id;
-}
-
 /**
  * Iterate core information for number of available cores to
  * append response for status command.
@@ -393,7 +384,7 @@ static int
 append_client_id_value(const char *name, char **output,
 		void *tmp __attribute__ ((unused)))
 {
-	return append_json_int_value(name, output, spp_get_client_id());
+	return append_json_int_value(name, output, get_client_id());
 }
 
 /* append a block of port entry for JSON format */
diff --git a/src/pcap/cmd_utils.h b/src/pcap/cmd_utils.h
index 46747e9..8281a3b 100644
--- a/src/pcap/cmd_utils.h
+++ b/src/pcap/cmd_utils.h
@@ -157,7 +157,7 @@ struct sppwk_comp_info {
 
 /* Manage given options as global variable */
 struct startup_param {
-	int client_id;  /* Client ID */
+	//int client_id;  /* Client ID */
 	char server_ip[INET_ADDRSTRLEN];  /* IP address of spp-ctl */
 	int server_port;  /* Port Number of spp-ctl */
 };
diff --git a/src/pcap/spp_pcap.c b/src/pcap/spp_pcap.c
index 57cc0bb..b69ffdd 100644
--- a/src/pcap/spp_pcap.c
+++ b/src/pcap/spp_pcap.c
@@ -177,28 +177,6 @@ usage(const char *progname)
 		, progname);
 }
 
-/**
- * Convert string type of client ID to integer and return SPPWK_RET_OK, or
- * SPPWK_RET_NG if failed.
- */
-static int
-client_id_toi(const char *client_id_str, int *client_id)
-{
-	int id = 0;
-	char *endptr = NULL;
-
-	id = strtol(client_id_str, &endptr, 0);
-	if (unlikely(client_id_str == endptr) || unlikely(*endptr != '\0'))
-		return SPPWK_RET_NG;
-
-	if (id >= RTE_MAX_LCORE)
-		return SPPWK_RET_NG;
-
-	*client_id = id;
-	RTE_LOG(DEBUG, SPP_PCAP, "Set client id = %d\n", *client_id);
-	return SPPWK_RET_OK;
-}
-
 /* Parse `--fsize` option and get the value */
 static int
 parse_fsize(const char *fsize_str, uint64_t *fsize)
@@ -263,6 +241,7 @@ parse_captured_port(const char *port_str, enum port_type *iface_type,
 static int
 parse_app_args(int argc, char *argv[])
 {
+	int cli_id;  /* Client ID. */
 	char *ctl_ip;  /* IP address of spp_ctl. */
 	int ctl_port;  /* Port num to connect spp_ctl. */
 	char cap_port_str[PORT_STR_SIZE];  /* Captured port. */
@@ -307,12 +286,11 @@ parse_app_args(int argc, char *argv[])
 			&option_index)) != EOF) {
 		switch (opt) {
 		case SPP_LONGOPT_RETVAL_CLIENT_ID:
-			if (client_id_toi(optarg,
-					&g_startup_param.client_id) !=
-								SPPWK_RET_OK) {
+			if (parse_client_id(&cli_id, optarg) != SPPWK_RET_OK) {
 				usage(progname);
 				return SPPWK_RET_NG;
 			}
+			set_client_id(cli_id);
 			proc_flg = 1;
 			break;
 		case SPP_LONGOPT_RETVAL_OUT_DIR:
@@ -367,8 +345,7 @@ parse_app_args(int argc, char *argv[])
 	RTE_LOG(INFO, SPP_PCAP,
 			"Parsed app args ('--client-id %d', '-s %s:%d', "
 			"'-c %s', '--out-dir %s', '--fsize %ld')\n",
-			g_startup_param.client_id, ctl_ip, ctl_port,
-			cap_port_str,
+			cli_id, ctl_ip, ctl_port, cap_port_str,
 			g_pcap_option.compress_file_path,
 			g_pcap_option.fsize_limit);
 	return SPPWK_RET_OK;
@@ -1031,8 +1008,8 @@ main(int argc, char *argv[])
 		/* create ring */
 		char ring_name[PORT_STR_SIZE];
 		memset(ring_name, 0x00, PORT_STR_SIZE);
-		snprintf(ring_name, PORT_STR_SIZE,  "cap_ring_%d",
-						g_startup_param.client_id);
+		snprintf(ring_name, PORT_STR_SIZE, "cap_ring_%d",
+				get_client_id());
 		g_pcap_option.cap_ring = rte_ring_create(ring_name,
 					rte_align32pow2(RING_SIZE),
 					rte_socket_id(), 0);
diff --git a/src/spp-ctl/spp_ctl.py b/src/spp-ctl/spp_ctl.py
index f276442..ce1a2e2 100644
--- a/src/spp-ctl/spp_ctl.py
+++ b/src/spp-ctl/spp_ctl.py
@@ -139,6 +139,7 @@ class Controller(object):
 
     def get_processes(self):
         procs = []
+        LOG.info('get_proesses: {}'.format(self.procs.values()))
         for proc in self.procs.values():
             p = {"type": proc.type}
             if proc.id != spp_proc.ID_PRIMARY:
-- 
2.17.1


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, back to index

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-24 10:45 [spp] [PATCH] spp_pcap: remove global variable client_id yasufum.o

Soft Patch Panel

Archives are clonable:
	git clone --mirror http://inbox.dpdk.org/spp/0 spp/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 spp spp/ http://inbox.dpdk.org/spp \
		spp@dpdk.org
	public-inbox-index spp


Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.spp


AGPL code for this site: git clone https://public-inbox.org/ public-inbox