[spp] Port ID incrementation is changed from DPDK18.02

[spp] Port ID incrementation is changed from DPDK18.02

[Yasufumi Ogawa]

Hi all,

Before DPDK 18.02 is fixed, behaviour of rte_eth_dev_get_port_by_name() 
in lib/librte_ether/rte_ethdev.c is changed by this commit.
 > 5b7ba3114 ethdev: add port ownership

This update is a desirable change to fix a problem for synchronization 
of port management. However, this function assigns unexpected port_id 
for SPP.

Previsouly, add command of secondary expected to be assigned port_id 
incrementally and independent among each of other secondary processes. 
For instance, adding ring 0 for sec 1 and 2 results like as

spp > sec 1;status
recv:8:{Client ID 1 Idling
clinet_id:1
port_id:0,on,PHY,outport:none
port_id:1,on,PHY,outport:none
port_id:2,on,RING(0),outport:none
}
spp > sec 2;status
recv:9:{Client ID 2 Idling
clinet_id:2
port_id:0,on,PHY,outport:none
port_id:1,on,PHY,outport:none
port_id:2,on,RING(0),outport:none
}

Both of secondary has port_id:2 for ring 0. However, it is different by 
using DPDK 18.02.

spp > sec 1;status
recv:8:{Client ID 1 Idling
clinet_id:1
port_id:0,on,PHY,outport:none
port_id:1,on,PHY,outport:none
port_id:4,on,RING(0),outport:none
}
spp > sec 2;status
recv:9:{Client ID 2 Idling
clinet_id:2
port_id:0,on,PHY,outport:none
port_id:1,on,PHY,outport:none
port_id:5,on,RING(0),outport:none
}

port_id are different, 4 and 5. It is not a serious problem, but would 
be annoying for users. I would like to fix it.

Luckly, we can patch ports without not port IDs and instead of resource 
IDs by recent update. So, I will change port assignment to not to use 
port ID by removing port_id from status.

Here is an updated message example.

spp > sec 1;status
recv:8:{Client ID 1 Idling
clinet_id:1
PHY(0),outport:none
PHY(1),outport:none
RING(0),outport:none
}

Patching ports with resource IDs is done by

spp > patch phy:0 ring:0

Thanks,
Yasufumi

-- 
Yasufumi Ogawa
NTT Network Service Systems Labs

[spp] [PATCH 0/9] Update port management

[ogawa.yasufumi]

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

To fix the problem of unexpected port ID, change assigning ports with
from port ID to resource ID as described below.

  spp > sec 1;patch phy:0 ring:0

Secondary's status message is changed from
    
  spp > sec 1;status
  port_id:0,on,PHY,outport:none
  port_id:1,on,PHY,outport:none
  port_id:2,on,RING(0),outport:none

to be more simplified YAML like style.

  spp > sec 1;status
  status: idling
  ports:
    - 'phy:0 -> ring:0'
    - 'phy:1'
    - 'ring:0 -> phy:1'

topo and topo_subgraph commands are also updated for this changes.


  controller: change displaying status
  shared: update print_active_ports
  spp_nfv: change format displaying status
  spp_vm: change format displaying status
  controller: add delimiter for topo command
  controller: update Shell for topo_subgraph
  controller: update generating graph
  controller: update topo command
  shared: fix bug for print port status

 src/controller/conn_thread.py |   4 +-
 src/controller/shell.py       |  61 +++++++++--
 src/controller/spp_common.py  |   3 +
 src/controller/topo.py        | 246 ++++++++++++++++++++++--------------------
 src/nfv/nfv.c                 |   6 +-
 src/shared/common.c           |  84 +++++++++++----
 src/shared/common.h           |   2 +-
 src/vm/main.c                 |   7 +-
 8 files changed, 260 insertions(+), 153 deletions(-)

-- 
2.7.4

[spp] [PATCH 1/9] controller: change displaying status

[ogawa.yasufumi]

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

This update is for removing port_id from the result of status
command because bahaviour of port_id assignment is changed from
DPDK 18.02 and it possibly confuses users.

Secondary's status message is changed from

  spp > sec 1;status
  port_id:0,on,PHY,outport:none
  port_id:1,on,PHY,outport:none
  port_id:2,on,RING(0),outport:none

to be more simplified YAML like style.

  spp > sec 1;status
  status: idling
  ports:
    - 'phy:0 -> ring:0'
    - 'phy:1'
    - 'ring:0 -> phy:1'

SPP controller expects message format from secondary is as following to
decode it to show as above example.

  status: running\nports: phy:0-vhost:0,vhost:0-null

This update is only for controller. Updates for secondary processes are
included in next patches.

Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
 src/controller/conn_thread.py |  4 ++--
 src/controller/shell.py       | 22 +++++++++++++++++++++-
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/src/controller/conn_thread.py b/src/controller/conn_thread.py
index 1620046..039a3ad 100644
--- a/src/controller/conn_thread.py
+++ b/src/controller/conn_thread.py
@@ -49,8 +49,8 @@ class ConnectionThread(threading.Thread):
                 # 1024 stands for bytes of data to be received
                 data = self.conn.recv(1024)
                 if data:
-                    spp_common.SEC2MAIN[self.client_id].put(
-                        "recv:%s:{%s}" % (str(self.conn.fileno()), data))
+                    msg = "%s" % data
+                    spp_common.SEC2MAIN[self.client_id].put(msg)
                 else:
                     spp_common.SEC2MAIN[self.client_id].put(
                         "closing:" + str(self.conn))
diff --git a/src/controller/shell.py b/src/controller/shell.py
index 91b78f1..4bca3e5 100644
--- a/src/controller/shell.py
+++ b/src/controller/shell.py
@@ -88,6 +88,23 @@ class Shell(cmd.Cmd, object):
         for i in spp_common.SECONDARY_LIST:
             print ("Connected secondary id: %d" % i)
 
+    def print_sec_status(self, msg):
+        msg = msg.replace("\x00", "")  # remove null chars in msg from sec
+        status, ports = msg.split("\n")
+        res = "%s\nports:\n" % status
+        port_list = ports.split(' ')[1].split(',')
+
+        tmp = []
+        for p in port_list:
+            p1, p2 = p.split('-')
+            if p2 == 'null':
+                tmp.append("  - '%s'" % p1)
+            else:
+                tmp.append("  - '%s -> %s'" % (p1, p2))
+        tmp.sort()
+        res += "\n".join(tmp)
+        print(res)
+
     def command_primary(self, command):
         """Send command to primary process"""
 
@@ -107,7 +124,10 @@ class Shell(cmd.Cmd, object):
         if sec_id in spp_common.SECONDARY_LIST:
             spp_common.MAIN2SEC[sec_id].put(command)
             recv = spp_common.SEC2MAIN[sec_id].get(True)
-            print (recv)
+            if command == 'status':
+                self.print_sec_status(recv)
+            else:
+                print(recv)
             return self.CMD_OK, recv
         else:
             message = "secondary id %d not exist" % sec_id
-- 
2.7.4

[spp] [PATCH 2/9] shared: update print_active_ports

[ogawa.yasufumi]

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

To comply with change displaying status, update the message format
defined in print_active_ports() in secondary process.

Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
 src/shared/common.c | 71 ++++++++++++++++++++++++++++++++++++++++-------------
 src/shared/common.h |  2 +-
 2 files changed, 55 insertions(+), 18 deletions(-)

diff --git a/src/shared/common.c b/src/shared/common.c
index 4176b6d..1ef63de 100644
--- a/src/shared/common.c
+++ b/src/shared/common.c
@@ -258,15 +258,14 @@ spp_atoi(const char *str, int *val)
  * "port_id:[PORT_ID],[IN_PORT_STAT],[TYPE],output:[OUTPORT_STAT]"
  */
 void
-print_active_ports(char *str, uint16_t client_id,
+print_active_ports(char *str,
 		struct port *ports_fwd_array,
 		struct port_map *port_map)
 {
 	unsigned int i;
 
-	sprintf(str, "clinet_id:%d\n", client_id);
-
 	/* Every elements value */
+	sprintf(str, "ports: ");
 	for (i = 0; i < RTE_MAX_ETHPORTS; i++) {
 		if (ports_fwd_array[i].in_port_id == PORT_RESET)
 			continue;
@@ -275,51 +274,89 @@ print_active_ports(char *str, uint16_t client_id,
 		RTE_LOG(INFO, APP, "Status %d\n",
 			ports_fwd_array[i].in_port_id);
 
-		sprintf(str + strlen(str), "port_id:%d,", i);
 		/* in_port_id is same value as port_id */
-		if (ports_fwd_array[i].in_port_id != PORT_RESET)
-			sprintf(str + strlen(str), "on,");
-		else
-			sprintf(str + strlen(str), "off,");
+		/**
+		 * NOTE(yasuufm)
+		 * in_port_id cannot be PORT_RESET currently and it is
+		 * meaningless, but not remove for future possible change
+		 */
+		// if (ports_fwd_array[i].in_port_id != PORT_RESET)
+		// 	sprintf(str + strlen(str), "on,");
+		// else
+		// 	sprintf(str + strlen(str), "off,");
 
 		switch (port_map[i].port_type) {
 		case PHY:
 			RTE_LOG(INFO, APP, "Type: PHY\n");
-			sprintf(str + strlen(str), "PHY,");
+			sprintf(str + strlen(str), "phy:%u-",
+					port_map[i].id);
 			break;
 		case RING:
 			RTE_LOG(INFO, APP, "Type: RING\n");
-			sprintf(str + strlen(str), "RING(%u),",
+			sprintf(str + strlen(str), "ring:%u-",
 				port_map[i].id);
 			break;
 		case VHOST:
 			RTE_LOG(INFO, APP, "Type: VHOST\n");
-			sprintf(str + strlen(str), "VHOST(%u),",
+			sprintf(str + strlen(str), "vhost:%u-",
 				port_map[i].id);
 			break;
 		case PCAP:
 			RTE_LOG(INFO, APP, "Type: PCAP\n");
-			sprintf(str + strlen(str), "PCAP(%u),",
+			sprintf(str + strlen(str), "pcap:%u-",
 					port_map[i].id);
 			break;
 		case NULLPMD:
 			RTE_LOG(INFO, APP, "Type: NULLPMD\n");
-			sprintf(str + strlen(str), "NULLPMD(%u),",
+			sprintf(str + strlen(str), "nullpmd:%u-",
 					port_map[i].id);
 			break;
 		case UNDEF:
 			RTE_LOG(INFO, APP, "Type: UDF\n");
-			sprintf(str + strlen(str), "UDF,");
+			/* TODO(yasufum) remove print for undefined ? */
+			sprintf(str + strlen(str), "udf-");
 			break;
 		}
 
 		RTE_LOG(INFO, APP, "Out Port ID %d\n",
 				ports_fwd_array[i].out_port_id);
 		if (ports_fwd_array[i].out_port_id == PORT_RESET) {
-			sprintf(str + strlen(str), "outport:%s\n", "none");
+			sprintf(str + strlen(str), "%s", "null,");
 		} else {
-			sprintf(str + strlen(str), "outport:%d\n",
-					ports_fwd_array[i].out_port_id);
+			unsigned int j = ports_fwd_array[i].out_port_id;
+			switch (port_map[j].port_type) {
+			case PHY:
+				RTE_LOG(INFO, APP, "Type: PHY\n");
+				sprintf(str + strlen(str), "phy:%u,",
+						port_map[j].id);
+				break;
+			case RING:
+				RTE_LOG(INFO, APP, "Type: RING\n");
+				sprintf(str + strlen(str), "ring:%u,",
+					port_map[j].id);
+				break;
+			case VHOST:
+				RTE_LOG(INFO, APP, "Type: VHOST\n");
+				sprintf(str + strlen(str), "vhost:%u,",
+						port_map[j].id);
+				break;
+			case PCAP:
+				RTE_LOG(INFO, APP, "Type: PCAP\n");
+				sprintf(str + strlen(str), "pcap:%u,",
+						port_map[j].id);
+				break;
+			case NULLPMD:
+				RTE_LOG(INFO, APP, "Type: NULLPMD\n");
+				sprintf(str + strlen(str), "nullpmd:%u,",
+						port_map[j].id);
+				break;
+			case UNDEF:
+				RTE_LOG(INFO, APP, "Type: UDF\n");
+				/* TODO(yasufum) remove print for undefined ? */
+				sprintf(str + strlen(str), "udf,");
+				break;
+			}
 		}
 	}
+	sprintf(str + strlen(str) - 1, "%c", '\0');
 }
diff --git a/src/shared/common.h b/src/shared/common.h
index f0d68e5..2dc31c9 100644
--- a/src/shared/common.h
+++ b/src/shared/common.h
@@ -216,7 +216,7 @@ int parse_portmask(struct port_info *ports, uint16_t max_ports,
 int parse_num_clients(uint16_t *num_clients, const char *clients);
 int parse_server(char **server_ip, int *server_port, char *server_addr);
 
-void print_active_ports(char *str, uint16_t client_id,
+void print_active_ports(char *str,
 		struct port *ports_fwd_array,
 		struct port_map *port_map);
 
-- 
2.7.4

[spp] [PATCH 3/9] spp_nfv: change format displaying status

[ogawa.yasufumi]

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

Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
 src/nfv/nfv.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/nfv/nfv.c b/src/nfv/nfv.c
index 1dc996a..f677691 100644
--- a/src/nfv/nfv.c
+++ b/src/nfv/nfv.c
@@ -713,10 +713,10 @@ parse_command(char *str)
 		RTE_LOG(DEBUG, APP, "status\n");
 		memset(str, '\0', MSG_SIZE);
 		if (cmd == FORWARD)
-			i = sprintf(str, "Client ID %d Running\n", client_id);
+			i = sprintf(str, "status: running\n");
 		else
-			i = sprintf(str, "Client ID %d Idling\n", client_id);
-		print_active_ports(str + i, client_id, ports_fwd_array, port_map);
+			i = sprintf(str, "status: idling\n");
+		print_active_ports(str + i, ports_fwd_array, port_map);
 
 	} else if (!strcmp(token_list[0], "_get_client_id")) {
 		memset(str, '\0', MSG_SIZE);
-- 
2.7.4

[spp] [PATCH 4/9] spp_vm: change format displaying status

[ogawa.yasufumi]

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

Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
 src/vm/main.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/src/vm/main.c b/src/vm/main.c
index 52f381f..94ba8d1 100644
--- a/src/vm/main.c
+++ b/src/vm/main.c
@@ -444,11 +444,10 @@ parse_command(char *str)
 		RTE_LOG(DEBUG, APP, "status\n");
 		memset(str, '\0', MSG_SIZE);
 		if (cmd == FORWARD)
-			i = sprintf(str, "Client ID %d Running\n", client_id);
+			i = sprintf(str, "status: running\n");
 		else
-			i = sprintf(str, "Client ID %d Idling\n", client_id);
-		print_active_ports(
-				str+i, client_id, ports_fwd_array, port_map);
+			i = sprintf(str, "status: idling\n");
+		print_active_ports(str + i, ports_fwd_array, port_map);
 
 	} else if (!strcmp(token_list[0], "_get_client_id")) {
 		memset(str, '\0', MSG_SIZE);
-- 
2.7.4

[spp] [PATCH 5/9] controller: add delimiter for topo command

[ogawa.yasufumi]

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

Resource ID consists of resource type and ID separated with
delimiter ':', for instance, 'phy:0'. This delimiter causes a parse
error in topo command because graphviz does not support ':' as a valid
character for node ID. If ':' is included in node ID, graphviz outputs
an imcomplete graph.

To avoid this problem, replace ':' with other valid character for node
IDs while labels displayed in the graph are not replaced.

This update add definitions for charactors of delimiter and label in
'spp_common.py'. Delimiter ':' is replaced with label charactor '_'
while graphviz script is generated.

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

diff --git a/src/controller/spp_common.py b/src/controller/spp_common.py
index 59ba504..b1ab60f 100644
--- a/src/controller/spp_common.py
+++ b/src/controller/spp_common.py
@@ -33,6 +33,9 @@ REMOTE_COMMAND = "RCMD"
 RCMD_EXECUTE_QUEUE = Queue()
 RCMD_RESULT_QUEUE = Queue()
 
+delim_node = '_'
+delim_label = ':'
+
 
 class GrowingList(list):
     """Growing List
-- 
2.7.4

[spp] [PATCH 6/9] controller: update Shell for topo_subgraph

[ogawa.yasufumi]

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

Update do_topo_subgraph() for parsing delimiter. It also update help
message.

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

diff --git a/src/controller/shell.py b/src/controller/shell.py
index 4bca3e5..c379c2a 100644
--- a/src/controller/shell.py
+++ b/src/controller/shell.py
@@ -550,17 +550,15 @@ class Shell(cmd.Cmd, object):
         For topo command, it is used for grouping resources of each
         of VM or container to topology be more understandable.
 
-        Add subgraph labeled 'vm1'. Resource name is capitalized and
-        both of them is OK.
-        spp > topo_subgraph add vm1 VHOST1;VHOST2  # upper case
-        spp > topo_subgraph add vm1 vhost1;vhost2  # lower case
+        Add subgraph labeled 'vm1'.
+        spp > topo_subgraph add vm1 vhost:1;vhost:2
 
         Delete subgraph 'vm1'.
         spp > topo_subgraph del vm1
 
         To show subgraphs, run topo_subgraph without args.
         spp > topo_subgraph
-        {'vm1', 'VHOST1;VHOST2'}
+        label: vm1	subgraph: "vhost:1;vhost:2"
         """
 
         args_cleaned = re.sub(r"\s+", ' ', args).strip()
@@ -577,8 +575,11 @@ class Shell(cmd.Cmd, object):
             if tokens[0] == 'add':
                 if len(tokens) == 3:
                     label = tokens[1]
-                    subg = tokens[2].upper()
+                    subg = tokens[2]
                     if ',' in subg:
+                        subg = re.sub(
+                            r'%s' % spp_common.delim_node,
+                            spp_common.delim_label, subg)
                         subg = re.sub(r",", ";", subg)
 
                     # TODO(yasufum) add validation for subgraph
-- 
2.7.4

[spp] [PATCH 7/9] controller: update generating graph

[ogawa.yasufumi]

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

Update parsing network configuration and generating graph in Topo class
since resource information held in secondary is changed from 18.02.

Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
 src/controller/topo.py | 237 +++++++++++++++++++++++++------------------------
 1 file changed, 119 insertions(+), 118 deletions(-)

diff --git a/src/controller/topo.py b/src/controller/topo.py
index 92d1634..0fc529b 100644
--- a/src/controller/topo.py
+++ b/src/controller/topo.py
@@ -3,11 +3,13 @@
 
 import os
 import re
+import spp_common
 from spp_common import logger
 import subprocess
 import traceback
 import uuid
 import websocket
+import yaml
 
 
 class Topo(object):
@@ -31,7 +33,8 @@ class Topo(object):
         res_ary = []
         for sec_id in self.sec_ids:
             self.m2s_queues[sec_id].put("status")
-            res = self.format_sec_status(self.s2m_queues[sec_id].get(True))
+            res = self.format_sec_status(
+                sec_id, self.s2m_queues[sec_id].get(True))
             res_ary.append(res)
         if dtype == "http":
             self.to_http(res_ary)
@@ -45,7 +48,8 @@ class Topo(object):
         res_ary = []
         for sec_id in self.sec_ids:
             self.m2s_queues[sec_id].put("status")
-            res = self.format_sec_status(self.s2m_queues[sec_id].get(True))
+            res = self.format_sec_status(
+                sec_id, self.s2m_queues[sec_id].get(True))
             res_ary.append(res)
 
         if ftype == "dot":
@@ -64,58 +68,63 @@ class Topo(object):
 
     def to_dot(self, sec_list, output_fname):
         # Label given if outport is "none"
-        NO_PORT = "none"
+        NO_PORT = None
 
         # Graphviz params
+        # TODO(yasufum) consider to move gviz params to config file.
         SEC_COLORS = [
             "blue", "green", "orange", "chocolate", "black",
             "cyan3", "green3", "indianred", "lawngreen", "limegreen"]
         PORT_COLORS = {
-            "PHY": "white",
-            "RING": "yellow",
-            "VHOST": "limegreen"}
+            "phy": "white",
+            "ring": "yellow",
+            "vhost": "limegreen"}
         LINE_STYLE = {
-            "RUNNING": "solid",
-            "IDLING": "dashed"}
+            "running": "solid",
+            "idling": "dashed"}
         GRAPH_TYPE = "digraph"
         LINK_TYPE = "->"
 
         node_attrs = 'node[shape="rectangle", style="filled"];'
 
+        node_template = '%s' + spp_common.delim_node + '%s'
+
         phys = []
         rings = []
         vhosts = []
         links = []
 
+        # parse status message from sec.
         for sec in sec_list:
             for port in sec["ports"]:
-                if port["iface"]["type"] == "PHY":
+                if port["iface"]["type"] == "phy":
                     phys.append(port)
-                elif port["iface"]["type"] == "RING":
+                elif port["iface"]["type"] == "ring":
                     rings.append(port)
-                elif port["iface"]["type"] == "VHOST":
+                elif port["iface"]["type"] == "vhost":
                     vhosts.append(port)
                 else:
                     raise ValueError(
                         "Invaid interface type: %s" % port["iface"]["type"])
 
-                if port["out"] != NO_PORT:
-                    out_id = int(port["out"])
-                    if sec["forward"] is True:
-                        l_style = LINE_STYLE["RUNNING"]
+                if port['out'] != NO_PORT:
+                    out_type, out_id = port['out'].split(':')
+                    if sec['status'] == 'running':
+                        l_style = LINE_STYLE["running"]
                     else:
-                        l_style = LINE_STYLE["IDLING"]
+                        l_style = LINE_STYLE["idling"]
                     attrs = '[label="%s", color="%s", style="%s"]' % (
-                        "sec" + sec["sec_id"],
-                        SEC_COLORS[int(sec["sec_id"])],
+                        "sec%d" % sec["sec_id"],
+                        SEC_COLORS[sec["sec_id"]],
                         l_style
                     )
-                    tmp = "%s%s %s %s%s%s;" % (
+                    link_style = node_template + ' %s ' + node_template + '%s;'
+                    tmp = link_style % (
                         port["iface"]["type"],
                         port["iface"]["id"],
                         LINK_TYPE,
-                        sec["ports"][out_id]["iface"]["type"],
-                        sec["ports"][out_id]["iface"]["id"],
+                        out_type,
+                        out_id,
                         attrs
                     )
                     links.append(tmp)
@@ -124,51 +133,61 @@ class Topo(object):
         output.append("newrank=true;")
         output.append(node_attrs)
 
-        phy_labels = []
-        for p in phys:
-            phy_labels.append(p["iface"]["type"] + p["iface"]["id"])
-        phy_labels = list(set(phy_labels))
-        for l in phy_labels:
+        phy_nodes = []
+        for node in phys:
+            phy_nodes.append(
+                node_template % (node['iface']['type'], node['iface']['id']))
+        phy_nodes = list(set(phy_nodes))
+        for node in phy_nodes:
+            label = re.sub(
+                r'%s' % spp_common.delim_node, spp_common.delim_label, node)
             output.append(
-                    '%s[label="%s", fillcolor="%s"];' % (
-                        l, l, PORT_COLORS["PHY"]))
+                '%s[label="%s", fillcolor="%s"];' % (
+                    node, label, PORT_COLORS["phy"]))
 
-        ring_labels = []
+        ring_nodes = []
         for p in rings:
-            ring_labels.append(p["iface"]["type"] + p["iface"]["id"])
-        ring_labels = list(set(ring_labels))
-        for l in ring_labels:
+            ring_nodes.append(
+                node_template % (p['iface']['type'], p['iface']['id']))
+        ring_nodes = list(set(ring_nodes))
+        for node in ring_nodes:
+            label = re.sub(
+                r'%s' % spp_common.delim_node, spp_common.delim_label, node)
             output.append(
                 '%s[label="%s", fillcolor="%s"];' % (
-                    l, l, PORT_COLORS["RING"]))
+                    node, label, PORT_COLORS["ring"]))
 
-        vhost_labels = []
+        vhost_nodes = []
         for p in vhosts:
-            vhost_labels.append(p["iface"]["type"] + p["iface"]["id"])
-        vhost_labels = list(set(vhost_labels))
-        for l in vhost_labels:
+            vhost_nodes.append(
+                node_template % (p["iface"]["type"], p["iface"]["id"]))
+        vhost_nodes = list(set(vhost_nodes))
+        for node in vhost_nodes:
+            label = re.sub(
+                r'%s' % spp_common.delim_node, spp_common.delim_label, node)
             output.append(
                 '%s[label="%s", fillcolor="%s"];' % (
-                    l, l, PORT_COLORS["VHOST"]))
+                    node, label, PORT_COLORS["vhost"]))
 
         # rank
         output.append(
-            '{rank=same; %s}' % ("; ".join(ring_labels)))
+            '{rank=same; %s}' % ("; ".join(ring_nodes)))
         output.append(
-            '{rank=same; %s}' % ("; ".join(vhost_labels)))
+            '{rank=same; %s}' % ("; ".join(vhost_nodes)))
 
+        rank_style = '{rank=max; %s}' % node_template
         if len(phys) > 0:
             output.append(
-                '{rank=max; %s}' % (
-                    phys[0]["iface"]["type"] + phys[0]["iface"]["id"]))
+                rank_style % (
+                    phys[0]["iface"]["type"], phys[0]["iface"]["id"]))
         elif len(vhosts) > 0:
             output.append(
-                '{rank=max; %s}' % (
-                    vhosts[0]["iface"]["type"] + vhosts[0]["iface"]["id"]))
+                rank_style % (
+                    vhosts[0]["iface"]["type"], vhosts[0]["iface"]["id"]))
 
-        if len(phy_labels) > 0:
+        if len(phy_nodes) > 0:
             output.append(
-                '{rank=same; %s}' % ("; ".join(phy_labels)))
+                '{rank=same; %s}' % ("; ".join(phy_nodes)))
 
         # Add subgraph
         ssgs = []
@@ -177,24 +196,26 @@ class Topo(object):
             for label, val in self.sub_graphs.items():
                 cluster_id = "cluster%d" % cnt
                 ssg_label = label
-                ssg_ports = val
+                ssg_ports = re.sub(
+                    r'%s' % spp_common.delim_label,
+                    spp_common.delim_node, val)
                 ssg = 'subgraph %s {label="%s" %s}' % (
-                        cluster_id, ssg_label, ssg_ports)
+                    cluster_id, ssg_label, ssg_ports)
                 ssgs.append(ssg)
                 cnt += 1
 
         cluster_id = "cluster0"
         sg_label = "Host"
-        sg_ports = "; ".join(phy_labels + ring_labels)
+        sg_ports = "; ".join(phy_nodes + ring_nodes)
         if len(ssgs) == 0:
             output.append(
-                    'subgraph %s {label="%s" %s}' % (
-                        cluster_id, sg_label, sg_ports))
+                'subgraph %s {label="%s" %s}' % (
+                    cluster_id, sg_label, sg_ports))
         else:
             tmp = 'label="%s" %s' % (sg_label, sg_ports)
             contents = [tmp] + ssgs
             output.append(
-                    'subgraph %s {%s}' % (cluster_id, '; '.join(contents)))
+                'subgraph %s {%s}' % (cluster_id, '; '.join(contents)))
 
         # Add links
         for link in links:
@@ -255,85 +276,65 @@ class Topo(object):
         subprocess.call("%s %s" % (img_cmd, tmpfile), shell=True)
         subprocess.call(["rm", "-f", tmpfile])
 
-    def format_sec_status(self, stat):
+    def format_sec_status(self, sec_id, stat):
         """Return formatted secondary status as a hash
 
         By running status command on controller, status is sent from
         secondary process and receiving message is displayed.
 
         This is an example of receiving status message.
-            recv:8:{Client ID 1 Idling
-            client_id:1
-            port_id:0,on,PHY,outport:2
-            port_id:1,on,PHY,outport:none
-            port_id:2,on,RING(0),outport:3
-            port_id:3,on,VHOST(1),outport:none
-            }
-
-        This method returns as following.
-            {
-            'forward': False,
+
+        spp > sec 1;status
+        status: idling
+        ports:
+          - 'phy:0 -> vhost:1'
+          - 'phy:1'
+          - 'ring:0'
+          - 'vhost:1 -> ring:0'
+
+        This method returns a result as following.
+        {
+            'sec_id': '1',
+            'status': 'idling',
             'ports': [
-                {
-                    'out': 'none',
-                    'id': '0',
-                    'iface': {'type': 'PHY', 'id': '0'}
-                },
-                {
-                    'out': 'none',
-                    'id': '1',
-                    'iface': {'type': 'PHY', 'id': '1'}
-                }
+                 {
+                     'rid': 'phy:0',
+                     'out': 'vhost:0',
+                     'iface': {'type': 'phy', 'id': '0'}
+                 },
+                 {
+                     'rid': 'phy:1',
+                     'out': None,
+                     'iface': {'type': 'phy', 'id': '1'}
+                 }
             ],
-            'sec_id': '2'
-            }
+            'sec_id': '2',
+            ...
+        }
         """
 
-        stat_ary = stat.split("\n")
+        stat_obj = yaml.load(stat)
         res = {}
+        res['sec_id'] = sec_id
+        res['status'] = stat_obj['status']
 
+        port_list = []
         try:
-            # Check running status
-            if "Idling" in stat_ary[0]:
-                res["forward"] = False
-            elif "Running" in stat_ary[0]:
-                res["forward"] = True
-            else:
-                print("Invalid forwarding status:", stat_ary[0])
-
-            ptn = re.compile(r"clinet_id:(\d+)")
-            m = ptn.match(stat_ary[1])
-            if m is not None:
-                res["sec_id"] = m.group(1)
-            else:
-                raise Exception("No client ID matched!")
-
-            ports = []
-            # match PHY, for exp. 'port_id:0,on,PHY,outport:none'
-            ptn_p = re.compile(r"port_id:(\d+),on,(\w+),outport:(\w+)")
-
-            # match RING for exp. 'port_id:2,on,RING(0),outport:3'
-            # or VHOST for exp. 'port_id:3,on,VHOST(1),outport:none'
-            ptn_v = re.compile(
-                r"port_id:(\d+),on,(\w+)\((\d+)\),outport:(\w+)")
-
-            for i in range(2, len(stat_ary)-1):
-                m = ptn_p.match(stat_ary[i])
-                if m is not None:
-                    ports.append({
-                        "id": m.group(1),
-                        "iface": {"type": m.group(2), "id": m.group(1)},
-                        "out": m.group(3)})
-                    continue
-
-                m = ptn_v.match(stat_ary[i])
-                if m is not None:
-                    ports.append({
-                        "id": m.group(1),
-                        "iface": {"type": m.group(2), "id": m.group(3)},
-                        "out": m.group(4)})
-
-            res["ports"] = ports
+            ports = stat_obj['ports'].split(',')
+            for port_info in ports:
+                rid, outport = port_info.split('-')
+                if outport == 'null':
+                    outport = None
+
+                itype, pid = rid.split(':')
+                iface = {'type': itype, 'id': pid}
+                port_list.append({
+                    'rid': rid,
+                    'out': outport,
+                    'iface': iface
+                })
+
+            res["ports"] = port_list
             return res
 
         except Exception:
-- 
2.7.4

[spp] [PATCH 8/9] controller: update topo command

[ogawa.yasufumi]

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

Update topo and topo_subgraph commands for parsing port status which
is changed for 18.02.

Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
 src/controller/shell.py | 50 +++++++++++++++++++++++++++++++++++++------------
 src/controller/topo.py  |  9 +++++++++
 2 files changed, 47 insertions(+), 12 deletions(-)

diff --git a/src/controller/shell.py b/src/controller/shell.py
index c379c2a..ea5e8a9 100644
--- a/src/controller/shell.py
+++ b/src/controller/shell.py
@@ -89,20 +89,46 @@ class Shell(cmd.Cmd, object):
             print ("Connected secondary id: %d" % i)
 
     def print_sec_status(self, msg):
-        msg = msg.replace("\x00", "")  # remove null chars in msg from sec
-        status, ports = msg.split("\n")
-        res = "%s\nports:\n" % status
+        """Parse and print message from SPP secondary
+
+        The format of sent message is expected as YAML like format as
+
+        status: idling\nports: 'phy:0-phy:1,phy:1-null'\x00\x00..
+
+        'ports' is a set of combinations of patches. The value is
+        encapsulated with "'" and ended series of null character "\x00".
+        If the destination is not defined, null is assigned.
+        """
+
+        msg = msg.replace("\x00", "").replace("'", "")  # clean sec's msg
+        sec_attr = msg.split("\n")
+
+        # Do nothing if returned msg is not valid format.
+        if len(sec_attr) < 2:
+            return None
+
+        status = sec_attr[0]
+        ports = sec_attr[1]
+
+        # Printed result to which port info is appended.
+        res = status
+
         port_list = ports.split(' ')[1].split(',')
+        if port_list[0] == '':  # port_list is [''] if there are no ports
+            res = '%s\nports: "no ports"' % res
+        else:
+            res = "%s\nports:\n" % res
+            tmp_list = []
+            for port_ent in port_list:
+                if '-' in port_ent:
+                    p1, p2 = port_ent.split('-')
+                    if p2 == 'null':
+                        tmp_list.append("  - '%s'" % p1)
+                    else:
+                        tmp_list.append("  - '%s -> %s'" % (p1, p2))
+            tmp_list.sort()
+            res += "\n".join(tmp_list)
 
-        tmp = []
-        for p in port_list:
-            p1, p2 = p.split('-')
-            if p2 == 'null':
-                tmp.append("  - '%s'" % p1)
-            else:
-                tmp.append("  - '%s -> %s'" % (p1, p2))
-        tmp.sort()
-        res += "\n".join(tmp)
         print(res)
 
     def command_primary(self, command):
diff --git a/src/controller/topo.py b/src/controller/topo.py
index 0fc529b..76dd54f 100644
--- a/src/controller/topo.py
+++ b/src/controller/topo.py
@@ -96,6 +96,8 @@ class Topo(object):
 
         # parse status message from sec.
         for sec in sec_list:
+            if sec is None:
+                continue
             for port in sec["ports"]:
                 if port["iface"]["type"] == "phy":
                     phys.append(port)
@@ -313,6 +315,10 @@ class Topo(object):
         }
         """
 
+        # Clean sec stat message
+        stat = stat.replace("\x00", "")
+        stat = stat.replace("'", "")
+
         stat_obj = yaml.load(stat)
         res = {}
         res['sec_id'] = sec_id
@@ -320,6 +326,9 @@ class Topo(object):
 
         port_list = []
         try:
+            if stat_obj['ports'] is None:
+                return None
+
             ports = stat_obj['ports'].split(',')
             for port_info in ports:
                 rid, outport = port_info.split('-')
-- 
2.7.4

[spp] [PATCH 9/9] shared: fix bug for print port status

[ogawa.yasufumi]

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

Result of print_active_ports() is invalid format if sec has no ports.
This update is to fix for this case.

Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
 src/shared/common.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/src/shared/common.c b/src/shared/common.c
index 1ef63de..b11cb6d 100644
--- a/src/shared/common.c
+++ b/src/shared/common.c
@@ -32,7 +32,6 @@
  */
 
 #include <rte_cycles.h>
-
 #include "common.h"
 
 /* Check the link status of all ports in up to 9s, and print them finally */
@@ -263,9 +262,10 @@ print_active_ports(char *str,
 		struct port_map *port_map)
 {
 	unsigned int i;
+	const char *port_prefix = "ports: '";
 
 	/* Every elements value */
-	sprintf(str, "ports: ");
+	sprintf(str, "%s",  port_prefix);
 	for (i = 0; i < RTE_MAX_ETHPORTS; i++) {
 		if (ports_fwd_array[i].in_port_id == PORT_RESET)
 			continue;
@@ -313,7 +313,7 @@ print_active_ports(char *str,
 			break;
 		case UNDEF:
 			RTE_LOG(INFO, APP, "Type: UDF\n");
-			/* TODO(yasufum) remove print for undefined ? */
+			/* TODO(yasufum) Need to remove print for undefined ? */
 			sprintf(str + strlen(str), "udf-");
 			break;
 		}
@@ -352,11 +352,22 @@ print_active_ports(char *str,
 				break;
 			case UNDEF:
 				RTE_LOG(INFO, APP, "Type: UDF\n");
-				/* TODO(yasufum) remove print for undefined ? */
+				/**
+				 * TODO(yasufum) Need to remove print for
+				 * undefined ?
+				 */
 				sprintf(str + strlen(str), "udf,");
 				break;
 			}
 		}
 	}
-	sprintf(str + strlen(str) - 1, "%c", '\0');
+
+	// If there are no ports, it's formatted as "ports: ''"
+	if (strcmp(str, port_prefix) == 0) {
+		sprintf(str + strlen(str), "'");
+	} else {  // Remove last ','
+		sprintf(str + strlen(str) - 1, "'");
+	}
+	// make sure to be terminated with null character
+	sprintf(str + strlen(str), "%c", '\0');
 }
-- 
2.7.4

Re: [spp] [PATCH 0/9] Update port management

[Ferruh Yigit]

On 3/12/2018 5:35 AM, ogawa.yasufumi@lab.ntt.co.jp wrote:
> From: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
> 
> To fix the problem of unexpected port ID, change assigning ports with
> from port ID to resource ID as described below.
> 
>   spp > sec 1;patch phy:0 ring:0
> 
> Secondary's status message is changed from
>     
>   spp > sec 1;status
>   port_id:0,on,PHY,outport:none
>   port_id:1,on,PHY,outport:none
>   port_id:2,on,RING(0),outport:none
> 
> to be more simplified YAML like style.
> 
>   spp > sec 1;status
>   status: idling
>   ports:
>     - 'phy:0 -> ring:0'
>     - 'phy:1'
>     - 'ring:0 -> phy:1'
> 
> topo and topo_subgraph commands are also updated for this changes.
> 
> 
>   controller: change displaying status
>   shared: update print_active_ports
>   spp_nfv: change format displaying status
>   spp_vm: change format displaying status
>   controller: add delimiter for topo command
>   controller: update Shell for topo_subgraph
>   controller: update generating graph
>   controller: update topo command
>   shared: fix bug for print port status

Series applied thanks.