[spp] [PATCH 0/6] Update for spp_pcap

[spp] [PATCH 0/6] Update for spp_pcap

[ogawa.yasufumi]

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

This series of patches is to add spp_pcap support, update GNUmakefile
and for SPP CLI and refactor spp_pcap's options. It also includes
refactoring for the name of functions and variables.

Yasufumi Ogawa (6):
  spp_pcap: refactor app options
  spp-ctl: update spp_pcap opts for validation
  controller: remove local default config
  controller: add pcap support for pri completion
  spp_pcap: refactor name of funcs and vars
  makefile: update dist-clean task

 GNUmakefile                       |  3 +
 src/controller/commands/pri.py    | 42 ++++----------
 src/controller/config/default.yml |  3 +
 src/pcap/spp_pcap.c               | 91 +++++++++++++++----------------
 src/pcap/spp_proc.h               |  6 +-
 src/spp-ctl/spp_proc.py           | 13 +++--
 6 files changed, 70 insertions(+), 88 deletions(-)

-- 
2.17.1

[spp] [PATCH 1/6] spp_pcap: refactor app options

[ogawa.yasufumi]

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

Some of app options of spp_pcap are not user friendly or violating
naming rule. This patch is to modify options, logs and help messages.

* `-i` is changed to `-c` because `-i` is ambiguous for the purpose.

* `--output` is changed to `--out-dir` because it is directory.

* `--limit_file_size` is changed to `--fsize` because it should be used
  `-` instead of `_` and simpler name is enough.

Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
 src/pcap/spp_pcap.c | 48 ++++++++++++++++++++++-----------------------
 src/pcap/spp_proc.h |  6 +++---
 2 files changed, 26 insertions(+), 28 deletions(-)

diff --git a/src/pcap/spp_pcap.c b/src/pcap/spp_pcap.c
index 2716c01..c69207b 100644
--- a/src/pcap/spp_pcap.c
+++ b/src/pcap/spp_pcap.c
@@ -167,17 +167,15 @@ usage(const char *progname)
 {
 	RTE_LOG(INFO, SPP_PCAP, "Usage: %s [EAL args] --"
 		" --client-id CLIENT_ID"
-		" -s SERVER_IP:SERVER_PORT"
-		" -i INPUT_PORT"
-		" [--output FILE_OUT_PUT_PATH]"
-		" [--limit_file_size LIMIT_FILE_SIZE]\n"
-		" --client-id CLIENT_ID   : My client ID\n"
-		" -s SERVER_IP:SERVER_PORT  : "
-				"Access information to the server\n"
-		" -i                        : capture port(phy,ring)\n"
-		" --output                  : file path(default:/tmp)\n"
-		" --limit_file_size         : "
-				"file limit size(default:1073741824 Byte)\n"
+		" -s IPADDR:PORT"
+		" -c CAP_PORT"
+		" [--out-dir DIR_OUTPUT]"
+		" [--fsize MAX_FILE_SIZE]\n"
+		" --client-id CLIENT_ID: My client ID\n"
+		" -s IPADDR:PORT: IP addr and sec port for spp-ctl\n"
+		" -c: Captured port (e.g. 'phy:0' or 'ring:1')\n"
+		" --out-dir: Output dir (Default is /tmp)\n"
+		" --fsize: Maximum captured file size (Default is 1GiB)\n"
 		, progname);
 }
 
@@ -305,13 +303,13 @@ parse_args(int argc, char *argv[])
 	const char *progname = argv[0];
 	char port_str[PORT_STR_SIZE];
 	static struct option lgopts[] = {
-			{ "client-id",       required_argument, NULL,
-					SPP_LONGOPT_RETVAL_CLIENT_ID },
-			{ "output",          required_argument, NULL,
-					SPP_LONGOPT_RETVAL_OUTPUT },
-			{ "limit_file_size", required_argument, NULL,
-					SPP_LONGOPT_RETVAL_LIMIT_FILE_SIZE},
-			{ 0 },
+		{ "client-id", required_argument, NULL,
+			SPP_LONGOPT_RETVAL_CLIENT_ID },
+		{ "out-dir", required_argument, NULL,
+			SPP_LONGOPT_RETVAL_OUT_DIR },
+		{ "fsize", required_argument, NULL,
+			SPP_LONGOPT_RETVAL_FILE_SIZE},
+		{ 0 },
 	};
 	/**
 	 * Save argv to argvopt to avoid losing the order of options
@@ -331,7 +329,7 @@ parse_args(int argc, char *argv[])
 	/* Check options of application */
 	optind = 0;
 	opterr = 0;
-	while ((opt = getopt_long(argc, argvopt, "i:s:", lgopts,
+	while ((opt = getopt_long(argc, argvopt, "c:s:", lgopts,
 			&option_index)) != EOF) {
 		switch (opt) {
 		case SPP_LONGOPT_RETVAL_CLIENT_ID:
@@ -343,7 +341,7 @@ parse_args(int argc, char *argv[])
 			}
 			proc_flg = 1;
 			break;
-		case SPP_LONGOPT_RETVAL_OUTPUT:
+		case SPP_LONGOPT_RETVAL_OUT_DIR:
 			strcpy(g_pcap_option.compress_file_path, optarg);
 			struct stat statBuf;
 			if (g_pcap_option.compress_file_path[0] == '\0' ||
@@ -352,7 +350,7 @@ parse_args(int argc, char *argv[])
 				return SPP_RET_NG;
 			}
 			break;
-		case SPP_LONGOPT_RETVAL_LIMIT_FILE_SIZE:
+		case SPP_LONGOPT_RETVAL_FILE_SIZE:
 			if (decode_limit_file_size(optarg,
 						&g_pcap_option.file_limit) !=
 						SPP_RET_OK) {
@@ -360,7 +358,7 @@ parse_args(int argc, char *argv[])
 				return SPP_RET_NG;
 			}
 			break;
-		case 'i':
+		case 'c':  /* captured port */
 			strcpy(port_str, optarg);
 			if (decode_capture_port(optarg,
 					&g_pcap_option.port_cap.iface_type,
@@ -371,7 +369,7 @@ parse_args(int argc, char *argv[])
 			}
 			port_flg = 1;
 			break;
-		case 's':
+		case 's':  /* server addr */
 			if (parse_server_ip(optarg, g_startup_param.server_ip,
 					&g_startup_param.server_port) !=
 								SPP_RET_OK) {
@@ -393,8 +391,8 @@ parse_args(int argc, char *argv[])
 	}
 
 	RTE_LOG(INFO, SPP_PCAP,
-			"app opts (client_id=%d,server=%s:%d,"
-			"port=%s,output=%s,limit_file_size=%ld)\n",
+			"App opts: '--client-id %d', '-s %s:%d', "
+			"'-c %s', '--out-dir %s', '--fsize %ld'\n",
 			g_startup_param.client_id,
 			g_startup_param.server_ip,
 			g_startup_param.server_port,
diff --git a/src/pcap/spp_proc.h b/src/pcap/spp_proc.h
index 6da5c5e..2265b21 100644
--- a/src/pcap/spp_proc.h
+++ b/src/pcap/spp_proc.h
@@ -90,9 +90,9 @@ enum SPP_LONGOPT_RETVAL {
 	 * Return value definition for getopt_long()
 	 * Only for long option
 	 */
-	SPP_LONGOPT_RETVAL_CLIENT_ID,      /* --client-id       */
-	SPP_LONGOPT_RETVAL_OUTPUT,         /* --output          */
-	SPP_LONGOPT_RETVAL_LIMIT_FILE_SIZE /* --limit_file_size */
+	SPP_LONGOPT_RETVAL_CLIENT_ID,  /* --client-id */
+	SPP_LONGOPT_RETVAL_OUT_DIR,    /* --out-dir */
+	SPP_LONGOPT_RETVAL_FILE_SIZE   /* --fsize */
 };
 
 /* Interface information structure */
-- 
2.17.1

[spp] [PATCH 2/6] spp-ctl: update spp_pcap opts for validation

[ogawa.yasufumi]

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

This patch is to update `APP_OPTS` used for validation for modified
options.

Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
 src/spp-ctl/spp_proc.py | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/src/spp-ctl/spp_proc.py b/src/spp-ctl/spp_proc.py
index a2d2adf..cdd7b49 100644
--- a/src/spp-ctl/spp_proc.py
+++ b/src/spp-ctl/spp_proc.py
@@ -82,10 +82,10 @@ APP_OPTS = {
         'spp_pcap':
         [
             '--client-id',  # sec ID
-            '-s',  # address nd port
-            '-i',
-            '--output',
-            '--limit_file_size'
+            '-s',  # address and port
+            '-c',  # captured port
+            '--out-dir',  # captured file dir
+            '--fsize'  # max size of captured file
             ]}
 
 
@@ -332,8 +332,9 @@ class PrimaryProc(SppProc):
                     if (val is not None) and (val != ''):
                         app_opts.append(str(val))
 
-        query = "launch {} {} {} -- {}".format(
-                sec_id, proc_name, ' '.join(eal_opts), ' '.join(app_opts))
+        query = "launch {sid} {pname} {eal} -- {app}".format(
+                sid=sec_id, pname=proc_name, eal=' '.join(eal_opts),
+                app=' '.join(app_opts))
 
         LOG.info("Query: {}".format(query))
 
-- 
2.17.1

[spp] [PATCH 3/6] controller: remove local default config

[ogawa.yasufumi]

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

Default config defined in `pri.py` is nouse because all of config is
defined in config file. This update is to remove file local default
config.

Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
 src/controller/commands/pri.py | 35 ++++------------------------------
 1 file changed, 4 insertions(+), 31 deletions(-)

diff --git a/src/controller/commands/pri.py b/src/controller/commands/pri.py
index 0e65883..d76da2e 100644
--- a/src/controller/commands/pri.py
+++ b/src/controller/commands/pri.py
@@ -25,16 +25,6 @@ class SppPrimary(object):
         self.spp_ctl_cli = spp_ctl_cli
 
         # Default args for `pri; launch`, used if given cli_config is invalid
-        # TODO(yasufum) remove local default and add checking config
-        self.launch_default = {
-                'mem': '-m 512',
-                'base_lcore': '1',
-                'vhost_cli': '',
-                'nof_lcores_nfv': '1',
-                'nof_lcores_vf': '3',
-                'nof_lcores_mirror': '2',
-                'nof_lcores_pcap': '2',
-                }
 
         # TODO(yasufum) replace placeholders __XXX__ to {keyword}.
         # Setup template of args for `pri; launch`
@@ -165,6 +155,7 @@ class SppPrimary(object):
                     rid=rports['id'], rx=rports['rx'], tx=rports['tx'],
                     rx_drop=rports['rx_drop'], tx_drop=rports['tx_drop']))
 
+    # TODO(yasufum) add checking for cli_config has keys
     def complete(self, text, line, begidx, endidx, cli_config):
         """Completion for primary process commands.
 
@@ -230,35 +221,23 @@ class SppPrimary(object):
                                 tmpkey = 'sec_nfv_nof_lcores'
                                 nof_workers = int(
                                         cli_config[tmpkey]['val'])
-                            else:
-                                nof_workers = int(
-                                        self.defaults['nof_lcores_nfv'])
 
                         elif ptype == 'vf':
                             if 'sec_vf_nof_lcores' in cli_config.keys():
                                 nof_workers = int(
                                         cli_config['sec_vf_nof_lcores']['val'])
-                            else:
-                                nof_workers = int(
-                                        elf.defaults['nof_lcores_vf'])
 
                         elif ptype == 'mirror':  # two worker cores
                             if 'sec_mirror_nof_lcores' in cli_config.keys():
                                 tmpkey = 'sec_mirror_nof_lcores'
                                 nof_workers = int(
                                         cli_config[tmpkey]['val'])
-                            else:
-                                nof_workers = int(
-                                        self.defaults['nof_lcore_mirror'])
 
                         elif ptype == 'pcap':  # at least two worker cores
                             if 'sec_pcap_nof_lcores' in cli_config.keys():
                                 tmpkey = 'sec_pcap_nof_lcores'
                                 nof_workers = int(
                                         cli_config[tmpkey]['val'])
-                            else:
-                                nof_workers = int(
-                                        elf.defaults['nof_lcore_pcap'])
 
                         last_core = lcore_base + nof_workers - 1
 
@@ -270,8 +249,7 @@ class SppPrimary(object):
                             rest_core = '{}-{}'.format(lcore_base, last_core)
 
                         temp = self._setup_launch_template(
-                                cli_config, self.launch_template,
-                                self.launch_default)
+                                cli_config, self.launch_template)
                         candidates = [temp.format(
                             wlcores=rest_core, opt_sid=opt_sid, sid=sid,
                             sec_addr=server_addr)]
@@ -290,19 +268,16 @@ class SppPrimary(object):
 
         return completions
 
-    def _setup_launch_template(self, cli_config, template, defaults):
+    # TODO(yasufum) add checking for cli_config has keys
+    def _setup_launch_template(self, cli_config, template):
         """Check given `cli_config` for params of launch."""
 
         if 'sec_mem' in cli_config.keys():
             sec_mem = cli_config['sec_mem']['val']
-        else:
-            sec_mem = defaults['mem']
         template = template.replace('__MEM__', sec_mem)
 
         if 'sec_base_lcore' in cli_config.keys():
             sec_base_lcore = cli_config['sec_base_lcore']['val']
-        else:
-            sec_base_lcore = defaults['base_lcore']
         template = template.replace('__BASE_LCORE__', str(sec_base_lcore))
 
         if 'sec_vhost_cli' in cli_config.keys():
@@ -310,8 +285,6 @@ class SppPrimary(object):
                 vhost_client = '--vhost-client'
             else:
                 vhost_client = ''
-        else:
-            vhost_client = defaults['vhost_cli']
         template = template.replace('__VHOST_CLI__', vhost_client)
 
         return template
-- 
2.17.1

[spp] [PATCH 4/6] controller: add pcap support for pri completion

[ogawa.yasufumi]

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

This update is to add `pri; launch` completion for pcap. Optional args,
`--out-dir` and `--fsize`, are expected to be given by user and not
completed. Here is an example.

  # press TAB after `pri; launch pcap 2 `
  spp > pri; launch pcap 2 -l 1,2-3 ... -s 192.168.1.100:6666 -c phy:0

Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
 src/controller/commands/pri.py    | 7 +++++++
 src/controller/config/default.yml | 3 +++
 2 files changed, 10 insertions(+)

diff --git a/src/controller/commands/pri.py b/src/controller/commands/pri.py
index d76da2e..2e3c208 100644
--- a/src/controller/commands/pri.py
+++ b/src/controller/commands/pri.py
@@ -239,6 +239,13 @@ class SppPrimary(object):
                                 nof_workers = int(
                                         cli_config[tmpkey]['val'])
 
+                            if 'sec_pcap_port' in cli_config.keys():
+                                temp = '-c {}'.format(
+                                        cli_config['sec_pcap_port']['val'])
+
+                                self.launch_template = '{} {}'.format(
+                                    self.launch_template, temp)
+
                         last_core = lcore_base + nof_workers - 1
 
                         # Decide lcore option based on configured number of
diff --git a/src/controller/config/default.yml b/src/controller/config/default.yml
index ee42bc3..223359e 100644
--- a/src/controller/config/default.yml
+++ b/src/controller/config/default.yml
@@ -31,3 +31,6 @@ sec_pcap_nof_lcores:
 sec_vhost_cli:
     val: ""
     desc: Vhost client mode, activated if set any of values
+sec_pcap_port:
+    val: "phy:0"
+    desc: Default captured port
-- 
2.17.1

[spp] [PATCH 5/6] spp_pcap: refactor name of funcs and vars

[ogawa.yasufumi]

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

This update is to refactor the name of functions.

* Rename functions starting with `decode_` to `parse_` because it does
  not decode actually.

* Rename member of `pcap_option` of file size to `fsize_limit`.

* Refactor related logs.

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

diff --git a/src/pcap/spp_pcap.c b/src/pcap/spp_pcap.c
index c69207b..7f2e564 100644
--- a/src/pcap/spp_pcap.c
+++ b/src/pcap/spp_pcap.c
@@ -113,7 +113,7 @@ struct pcap_packet_header {
 /* Option for pcap. */
 struct pcap_option {
 	struct timespec start_time; /* start time */
-	uint64_t file_limit;        /* file size limit */
+	uint64_t fsize_limit;        /* file size limit */
 	char compress_file_path[PCAP_FPATH_STRLEN]; /* file path */
 	char compress_file_date[PCAP_FDATE_STRLEN]; /* file name date */
 	struct spp_port_info port_cap;  /* capture port */
@@ -169,7 +169,7 @@ usage(const char *progname)
 		" --client-id CLIENT_ID"
 		" -s IPADDR:PORT"
 		" -c CAP_PORT"
-		" [--out-dir DIR_OUTPUT]"
+		" [--out-dir OUTPUT_DIR]"
 		" [--fsize MAX_FILE_SIZE]\n"
 		" --client-id CLIENT_ID: My client ID\n"
 		" -s IPADDR:PORT: IP addr and sec port for spp-ctl\n"
@@ -180,13 +180,11 @@ usage(const char *progname)
 }
 
 /**
- * Convert string of given client id to integer
- *
- * If succeeded, client id of integer is assigned to client_id and
- * return SPP_RET_OK. Or return -SPP_RET_NG if failed.
+ * Convert string type of client ID to integer and return SPP_RET_OK, or
+ * SPP_RET_NG if failed.
  */
 static int
-decode_client_id(const char *client_id_str, int *client_id)
+client_id_toi(const char *client_id_str, int *client_id)
 {
 	int id = 0;
 	char *endptr = NULL;
@@ -229,25 +227,25 @@ parse_server_ip(const char *server_str, char *server_ip, int *server_port)
 }
 
 
-/* Decode options for limit file size */
+/* Parse `--fsize` option and get the value */
 static int
-decode_limit_file_size(const char *limit_size_str, uint64_t *limit_size)
+parse_fsize(const char *fsize_str, uint64_t *fsize)
 {
-	uint64_t file_limit = 0;
+	uint64_t fs = 0;
 	char *endptr = NULL;
 
-	file_limit = strtoull(limit_size_str, &endptr, 10);
-	if (unlikely(limit_size_str == endptr) || unlikely(*endptr != '\0'))
+	fs = strtoull(fsize_str, &endptr, 10);
+	if (unlikely(fsize_str == endptr) || unlikely(*endptr != '\0'))
 		return SPP_RET_NG;
 
-	*limit_size = file_limit;
-	RTE_LOG(DEBUG, SPP_PCAP, "Set limit file zise = %ld\n", *limit_size);
+	*fsize = fs;
+	RTE_LOG(DEBUG, SPP_PCAP, "Set fzise = %ld\n", *fsize);
 	return SPP_RET_OK;
 }
 
-/* Decode options for port */
+/* Parse `-c` option for captured port and get the port type and ID */
 static int
-decode_capture_port(const char *port_str, enum port_type *iface_type,
+parse_captured_port(const char *port_str, enum port_type *iface_type,
 			int *iface_no)
 {
 	enum port_type type = UNDEF;
@@ -324,7 +322,7 @@ parse_args(int argc, char *argv[])
 	/* option parameters init */
 	memset(&g_pcap_option, 0x00, sizeof(g_pcap_option));
 	strcpy(g_pcap_option.compress_file_path, DEFAULT_OUTPUT_DIR);
-	g_pcap_option.file_limit = DEFAULT_FILE_LIMIT;
+	g_pcap_option.fsize_limit = DEFAULT_FILE_LIMIT;
 
 	/* Check options of application */
 	optind = 0;
@@ -333,7 +331,7 @@ parse_args(int argc, char *argv[])
 			&option_index)) != EOF) {
 		switch (opt) {
 		case SPP_LONGOPT_RETVAL_CLIENT_ID:
-			if (decode_client_id(optarg,
+			if (client_id_toi(optarg,
 					&g_startup_param.client_id) !=
 								SPP_RET_OK) {
 				usage(progname);
@@ -351,16 +349,15 @@ parse_args(int argc, char *argv[])
 			}
 			break;
 		case SPP_LONGOPT_RETVAL_FILE_SIZE:
-			if (decode_limit_file_size(optarg,
-						&g_pcap_option.file_limit) !=
-						SPP_RET_OK) {
+			if (parse_fsize(optarg, &g_pcap_option.fsize_limit) !=
+					SPP_RET_OK) {
 				usage(progname);
 				return SPP_RET_NG;
 			}
 			break;
 		case 'c':  /* captured port */
 			strcpy(port_str, optarg);
-			if (decode_capture_port(optarg,
+			if (parse_captured_port(optarg,
 					&g_pcap_option.port_cap.iface_type,
 					&g_pcap_option.port_cap.iface_no) !=
 					SPP_RET_OK) {
@@ -398,7 +395,7 @@ parse_args(int argc, char *argv[])
 			g_startup_param.server_port,
 			port_str,
 			g_pcap_option.compress_file_path,
-			g_pcap_option.file_limit);
+			g_pcap_option.fsize_limit);
 	return SPP_RET_OK;
 }
 
@@ -697,7 +694,7 @@ static int compress_file_packet(struct pcap_mng_info *info,
 		return SPP_RET_OK;
 
 	/* capture file rool */
-	if (info->file_size > g_pcap_option.file_limit) {
+	if (info->file_size > g_pcap_option.fsize_limit) {
 		if (file_compression_operation(info, UPDATE_MODE)
 							!= SPP_RET_OK)
 			return SPP_RET_NG;
-- 
2.17.1

[spp] [PATCH 6/6] makefile: update dist-clean task

[ogawa.yasufumi]

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

* Remove objects and headers under `src/pcap`.

* Remove python cache files and directories.

Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
 GNUmakefile | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/GNUmakefile b/GNUmakefile
index 8a9fc0c..fff7e77 100644
--- a/GNUmakefile
+++ b/GNUmakefile
@@ -25,8 +25,11 @@ DOC_ROOT = docs/guides
 dist-clean:
 	make clean
 	rm -rf $(wildcard src/*/$(RTE_TARGET))
+	rm -rf $(wildcard src/*/*.pyc)
+	rm -rf $(wildcard src/*/__pycache__)
 	rm -rf $(wildcard src/*/shared)
 	rm -rf $(wildcard src/mirror/vf)
+	rm -rf $(wildcard src/pcap/vf)
 
 .PHONY: doc
 doc: doc-all
-- 
2.17.1