[spp] [PATCH 0/5] Fix bugs of SPP CLI

[spp] [PATCH 0/5] Fix bugs of SPP CLI

[ogawa.yasufumi]

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

* Add checking for `vf` command to avoid errors of invalid resource
  configuration and invlalid command syntax.

* Correct logging history.

* Correct parsing command line of `pri` for completion.

Yasufumi Ogawa (5):
  controller: fix bug to add vf comps on same core
  controller: add checking syntax for vf port cmd
  controller: fix bug of history command
  controller: fix bug of completion of pri
  controller: refactor configuration of logfile

 src/controller/commands/pri.py | 90 ++++++++++++++++++++++--------------------
 src/controller/commands/vf.py  | 29 ++++++++------
 src/controller/shell.py        | 40 +++++++------------
 src/controller/spp_common.py   |  4 +-
 4 files changed, 82 insertions(+), 81 deletions(-)

-- 
2.7.4

[spp] [PATCH 1/5] controller: fix bug to add vf comps on same core

[ogawa.yasufumi]

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

SPP CLI is terminated if several components are assigned on the same
core because it try to find non-existent ID from list of unused cores
and failed to an error unexpectedly.

This patch is to add checking the required ID exists in the list before
to avoid not found error.

Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
 src/controller/commands/vf.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/controller/commands/vf.py b/src/controller/commands/vf.py
index e715a17..7518c04 100644
--- a/src/controller/commands/vf.py
+++ b/src/controller/commands/vf.py
@@ -250,7 +250,8 @@ class SppVf(object):
                     print("Succeeded to start component '%s' on core:%d"
                           % (req_params['name'], req_params['core']))
                     self.worker_names.append(req_params['name'])
-                    self.unused_core_ids.remove(req_params['core'])
+                    if req_params['core'] in self.unused_core_ids:
+                        self.unused_core_ids.remove(req_params['core'])
                 elif res.status_code in error_codes:
                     pass
                 else:
-- 
2.7.4

[spp] [PATCH 2/5] controller: add checking syntax for vf port cmd

[ogawa.yasufumi]

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

This update is add checking the number of params for `port` sub command
of vf to avoid SPP CLI is terminated if its syntax is invalid.

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

diff --git a/src/controller/commands/vf.py b/src/controller/commands/vf.py
index 7518c04..ff08231 100644
--- a/src/controller/commands/vf.py
+++ b/src/controller/commands/vf.py
@@ -279,6 +279,7 @@ class SppVf(object):
                     print('Error: unknown response.')
 
     def _run_port(self, params):
+        req_params = None
         if len(params) == 4:
             if params[0] == 'add':
                 action = 'attach'
@@ -314,17 +315,20 @@ class SppVf(object):
                           'dir': params[2],
                           'vlan': {'operation': op, 'id': int(params[5]),
                                    'pcp': int(params[6])}}
+        else:
+            print('Error: Invalid syntax.')
 
-        res = self.spp_ctl_cli.put('vfs/%d/components/%s/ports'
-                                   % (self.sec_id, params[3]), req_params)
-        if res is not None:
-            error_codes = self.spp_ctl_cli.rest_common_error_codes
-            if res.status_code == 204:
-                print("Succeeded to %s port" % params[0])
-            elif res.status_code in error_codes:
-                pass
-            else:
-                print('Error: unknown response.')
+        if req_params is not None:
+            res = self.spp_ctl_cli.put('vfs/%d/components/%s/ports'
+                                       % (self.sec_id, params[3]), req_params)
+            if res is not None:
+                error_codes = self.spp_ctl_cli.rest_common_error_codes
+                if res.status_code == 204:
+                    print("Succeeded to %s port" % params[0])
+                elif res.status_code in error_codes:
+                    pass
+                else:
+                    print('Error: unknown response.')
 
     def _run_cls_table(self, params):
         req_params = None
@@ -337,7 +341,7 @@ class SppVf(object):
                           'vlan': params[2], 'mac_address': params[3],
                           'port': params[4]}
         else:
-            print('Error: Invalid params')
+            print('Error: Invalid syntax.')
 
         if req_params is not None:
             req = 'vfs/%d/classifier_table' % self.sec_id
-- 
2.7.4

[spp] [PATCH 3/5] controller: fix bug of history command

[ogawa.yasufumi]

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

In SPP CLI, command is not added to history without running `history`
because setting a hook for flushing from `readline` is inappropriate.
It should be set to when after command is done. This update is to fix
the issue by using `postcmd()` of Cmd class.

Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
 src/controller/shell.py | 39 +++++++++++++--------------------------
 1 file changed, 13 insertions(+), 26 deletions(-)

diff --git a/src/controller/shell.py b/src/controller/shell.py
index 40bafc2..c2a31c6 100644
--- a/src/controller/shell.py
+++ b/src/controller/shell.py
@@ -25,9 +25,11 @@ class Shell(cmd.Cmd, object):
 
     recorded_file = None
     hist_file = os.path.expanduser('~/.spp_history')
+
+    # Commands not included in history
     HIST_EXCEPT = ['bye', 'exit', 'history', 'redo']
 
-    intro = 'Welcome to the spp.   Type help or ? to list commands.\n'
+    intro = 'Welcome to the SPP CLI. Type `help` or `?` to list commands.\n'
     prompt = 'spp > '
 
     PLUGIN_DIR = 'plugins'
@@ -76,10 +78,19 @@ class Shell(cmd.Cmd, object):
             self.secondaries['mirror'][sec_id] = mirror.SppMirror(
                     self.spp_ctl_cli, sec_id)
 
+    # Called everytime after running command. `stop` is returned from `do_*`
+    # method and SPP CLI is terminated if it is True. It means that only
+    # `do_bye` and  `do_exit` return True.
+    def postcmd(self, stop, line):
+        # TODO(yasufum) do not add to history if command is failed.
+        if line.strip().split(' ')[0] not in self.HIST_EXCEPT:
+            readline.write_history_file(self.hist_file)
+        return stop
+
     def default(self, line):
         """Define defualt behaviour.
 
-        If user input is commend styled, controller simply echo
+        If user input is comment styled, controller simply echo
         as a comment.
         """
 
@@ -112,24 +123,6 @@ class Shell(cmd.Cmd, object):
                         ids.append(ent['client-id'])
         return ids
 
-    def clean_hist_file(self):
-        """Remove useless entries in history file."""
-
-        entries = []
-
-        try:
-            for line in open(self.hist_file):
-                line_s = line.strip()
-                if not (line_s.split(' ')[0] in self.HIST_EXCEPT):
-                    entries.append(line_s)
-            f = open(self.hist_file, "w+")
-            contents = '\n'.join(entries)
-            contents += '\n'
-            f.write(contents)
-            f.close()
-        except IOError:
-            print('Error: Cannot open history file "%s"' % self.hist_file)
-
     def print_status(self):
         """Display information about connected clients."""
 
@@ -761,12 +754,6 @@ class Shell(cmd.Cmd, object):
         'bye', 'exit', 'history', 'redo'
         """
 
-        # flush all of history to the hist_file.
-        readline.write_history_file(self.hist_file)
-
-        # remove commands defined in `self.HIST_EXCEPT`
-        self.clean_hist_file()
-
         try:
             f = open(self.hist_file)
 
-- 
2.7.4

[spp] [PATCH 4/5] controller: fix bug of completion of pri

[ogawa.yasufumi]

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

`pri` command is completed without `;` after `pri`. This update is to
fix it and to not show candidates if `;` does not exist.

  spp > pri; laun  # show candidates

  spp > pri lau  # do not show without `pri;`

Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
 src/controller/commands/pri.py | 88 ++++++++++++++++++++++--------------------
 src/controller/shell.py        |  1 +
 2 files changed, 47 insertions(+), 42 deletions(-)

diff --git a/src/controller/commands/pri.py b/src/controller/commands/pri.py
index 1f60ece..bb89a5f 100644
--- a/src/controller/commands/pri.py
+++ b/src/controller/commands/pri.py
@@ -5,6 +5,8 @@ from __future__ import absolute_import
 
 from .. import spp_common
 from ..shell_lib import common
+from ..spp_common import logger
+#from .. import spp_common
 
 
 class SppPrimary(object):
@@ -151,48 +153,50 @@ class SppPrimary(object):
         base_core = 1  # shared among secondaries
         mytemplate = "-l {},{} -m 512 -- {} {} -s {}"
 
-        # Show sub commands
-        if len(tokens) == 2:
-            # Add sub commands
-            candidates = candidates + self.PRI_CMDS[:]
-
-        # Show args of `launch` sub command.
-        elif len(tokens) == 3 and tokens[1] == 'launch':
-            for pt in spp_common.SEC_TYPES:
-                candidates.append('{}'.format(pt))
-
-        elif len(tokens) == 4 and tokens[1] == 'launch':
-            if tokens[2] in spp_common.SEC_TYPES:
-                candidates = [
-                        str(i+1) for i in range(spp_common.MAX_SECONDARY)]
-
-        elif len(tokens) == 5 and tokens[1] == 'launch':
-            if (tokens[2] in spp_common.SEC_TYPES) and \
-                    (int(tokens[3])-1 in range(spp_common.MAX_SECONDARY)):
-                ptype = tokens[2]
-                sid = tokens[3]
-
-                if ptype == 'nfv':
-                    opt_sid = '-n'
-                else:
-                    opt_sid = '--client-id'
-
-                server_addr = common.current_server_addr()
-                server_addr = server_addr.replace('7777', '6666')
-
-                # Define rest of cores dynamically.
-                # TODO(yasufum) decide rest of cores considering used cores
-                if ptype == 'nfv':  # one core is enough
-                    rest_core = sid
-                elif ptype == 'vf':  # at least three cores
-                    rest_core = '{}-{}'.format(int(sid), int(sid)+2)
-                elif ptype == 'mirror':  # two cores
-                    rest_core = sid
-                elif ptype == 'pcap':  # at least two cores
-                    rest_core = '{}-{}'.format(int(sid), int(sid)+1)
-
-                candidates = [mytemplate.format(
-                    base_core, rest_core, opt_sid, sid, server_addr)]
+        if tokens[0].endswith(';'):
+
+            # Show sub commands
+            if len(tokens) == 2:
+                # Add sub commands
+                candidates = candidates + self.PRI_CMDS[:]
+
+            # Show args of `launch` sub command.
+            elif len(tokens) == 3 and tokens[1] == 'launch':
+                for pt in spp_common.SEC_TYPES:
+                    candidates.append('{}'.format(pt))
+
+            elif len(tokens) == 4 and tokens[1] == 'launch':
+                if tokens[2] in spp_common.SEC_TYPES:
+                    candidates = [
+                            str(i+1) for i in range(spp_common.MAX_SECONDARY)]
+
+            elif len(tokens) == 5 and tokens[1] == 'launch':
+                if (tokens[2] in spp_common.SEC_TYPES) and \
+                        (int(tokens[3])-1 in range(spp_common.MAX_SECONDARY)):
+                    ptype = tokens[2]
+                    sid = tokens[3]
+
+                    if ptype == 'nfv':
+                        opt_sid = '-n'
+                    else:
+                        opt_sid = '--client-id'
+
+                    server_addr = common.current_server_addr()
+                    server_addr = server_addr.replace('7777', '6666')
+
+                    # Define rest of cores dynamically.
+                    # TODO(yasufum) decide rest of cores considering used cores
+                    if ptype == 'nfv':  # one core is enough
+                        rest_core = sid
+                    elif ptype == 'vf':  # at least three cores
+                        rest_core = '{}-{}'.format(int(sid), int(sid)+2)
+                    elif ptype == 'mirror':  # two cores
+                        rest_core = sid
+                    elif ptype == 'pcap':  # at least two cores
+                        rest_core = '{}-{}'.format(int(sid), int(sid)+1)
+
+                    candidates = [mytemplate.format(
+                        base_core, rest_core, opt_sid, sid, server_addr)]
 
         if not text:
             completions = candidates
diff --git a/src/controller/shell.py b/src/controller/shell.py
index c2a31c6..e68ac63 100644
--- a/src/controller/shell.py
+++ b/src/controller/shell.py
@@ -330,6 +330,7 @@ class Shell(cmd.Cmd, object):
     def complete_pri(self, text, line, begidx, endidx):
         """Completion for primary process commands."""
 
+        line = re.sub(r'\s+', " ", line)
         return self.primary.complete(text, line, begidx, endidx)
 
     def do_nfv(self, cmd):
-- 
2.7.4

[spp] [PATCH 5/5] controller: refactor configuration of logfile

[ogawa.yasufumi]

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

Make the name of logfile explicitly in `spp_common.py`.

Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
 src/controller/commands/pri.py | 2 ++
 src/controller/spp_common.py   | 4 +++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/controller/commands/pri.py b/src/controller/commands/pri.py
index bb89a5f..b9f1234 100644
--- a/src/controller/commands/pri.py
+++ b/src/controller/commands/pri.py
@@ -331,6 +331,8 @@ class SppPrimary(object):
             else:
                 opts['app']['--client-id'] = sec_id
 
+        logger.debug('launch, {}'.format(opts))
+
         # Send request for launch secondary.
         res = self.spp_ctl_cli.put('primary/launch', opts)
         if res is not None:
diff --git a/src/controller/spp_common.py b/src/controller/spp_common.py
index c94d175..6d0546e 100644
--- a/src/controller/spp_common.py
+++ b/src/controller/spp_common.py
@@ -9,6 +9,8 @@ PORT_TYPES = ['phy', 'ring', 'vhost', 'pcap', 'nullpmd']
 
 SEC_TYPES = ['nfv', 'vf', 'mirror', 'pcap']
 
+LOGFILE = 'spp_cli.log'  # name of logfile under `/src/controller/log/`
+
 cur_server_addr = None
 
 # Maximum num of sock queues for secondaries
@@ -19,7 +21,7 @@ logger = logging.getLogger(__name__)
 # handler = logging.StreamHandler()
 os.system("mkdir -p %s/log" % (os.path.dirname(__file__)))
 
-logfile = '%s/log/%s' % (os.path.dirname(__file__), 'spp.log')
+logfile = '%s/log/%s' % (os.path.dirname(__file__), LOGFILE)
 handler = logging.FileHandler(logfile)
 handler.setLevel(logging.DEBUG)
 formatter = logging.Formatter(
-- 
2.7.4