* [spp] [PATCH 1/6] spp_nfv: fix bug of do_del if port does not exist
2018-10-09 10:53 [spp] [PATCH 0/6] Fix bug of assigning ports ogawa.yasufumi
@ 2018-10-09 10:53 ` ogawa.yasufumi
2018-10-09 10:53 ` [spp] [PATCH 2/6] spp_nfv: refactor log of patch command ogawa.yasufumi
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: ogawa.yasufumi @ 2018-10-09 10:53 UTC (permalink / raw)
To: spp, ferruh.yigit, ogawa.yasufumi
From: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
Fix bug of do_del() causing segmentation fault if not existing port is
deleted. It is because determining the result of find_port_id() is
incorrect and tries to delete invalid port.
This update is to correct the result of find_port_id() and determining
port ID.
It is also including refactor of patch command in which find_port_id()
is used. An error message is shown in log without terminating the
process if patched port is not found.
Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
src/nfv/nfv.c | 80 +++++++++++++++++++++++++++++++++++++++++++----------------
1 file changed, 58 insertions(+), 22 deletions(-)
diff --git a/src/nfv/nfv.c b/src/nfv/nfv.c
index c261e8d..049b3cf 100644
--- a/src/nfv/nfv.c
+++ b/src/nfv/nfv.c
@@ -265,11 +265,15 @@ forward_array_remove(int port_id)
}
}
-static int
+/*
+ * Return actual port ID which is assigned by system internally, or PORT_RESET
+ * if port is not found.
+ */
+static uint16_t
find_port_id(int id, enum port_type type)
{
- int port_id = PORT_RESET;
- unsigned int i;
+ uint16_t port_id = PORT_RESET;
+ uint16_t i;
for (i = 0; i < RTE_MAX_ETHPORTS; i++) {
if (port_map[i].port_type != type)
@@ -287,18 +291,21 @@ find_port_id(int id, enum port_type type)
static int
do_del(char *res_uid)
{
- int port_id = PORT_RESET;
+ uint16_t port_id = PORT_RESET;
char *p_type;
int p_id;
int res;
res = parse_resource_uid(res_uid, &p_type, &p_id);
- if (res < 0)
+ if (res < 0) {
+ RTE_LOG(ERR, APP,
+ "Failed to parse resource UID\n");
return -1;
+ }
if (!strcmp(p_type, "vhost")) {
port_id = find_port_id(p_id, VHOST);
- if (port_id < 0)
+ if (port_id == PORT_RESET)
return -1;
} else if (!strcmp(p_type, "ring")) {
@@ -306,7 +313,7 @@ do_del(char *res_uid)
RTE_LOG(DEBUG, APP, "Del ring id %d\n", p_id);
port_id = find_port_id(p_id, RING);
- if (port_id < 0)
+ if (port_id == PORT_RESET)
return -1;
rte_eth_dev_detach(port_id, name);
@@ -315,7 +322,7 @@ do_del(char *res_uid)
char name[RTE_ETH_NAME_MAX_LEN];
port_id = find_port_id(p_id, PCAP);
- if (port_id < 0)
+ if (port_id == PORT_RESET)
return -1;
rte_eth_dev_detach(port_id, name);
@@ -324,7 +331,7 @@ do_del(char *res_uid)
char name[RTE_ETH_NAME_MAX_LEN];
port_id = find_port_id(p_id, NULLPMD);
- if (port_id < 0)
+ if (port_id == PORT_RESET)
return -1;
rte_eth_dev_detach(port_id, name);
@@ -694,7 +701,7 @@ do_add(char *res_uid)
else
port_id = (uint16_t) res;
- port_map[port_id].id = (uint16_t) p_id;
+ port_map[port_id].id = p_id;
port_map[port_id].port_type = type;
port_map[port_id].stats = &ports->client_stats[p_id];
@@ -775,23 +782,52 @@ parse_command(char *str)
/* reset forward array*/
forward_array_reset();
} else {
- int in_port;
- int out_port;
+ uint16_t in_port;
+ uint16_t out_port;
if (max_token <= 2)
return 0;
- char *p_type;
- int p_id;
-
- parse_resource_uid(token_list[1], &p_type, &p_id);
- in_port = find_port_id(p_id, get_port_type(p_type));
-
- parse_resource_uid(token_list[2], &p_type, &p_id);
- out_port = find_port_id(p_id, get_port_type(p_type));
-
- if (in_port < 0 || out_port < 0)
+ char *in_p_type;
+ char *out_p_type;
+ int in_p_id;
+ int out_p_id;
+
+ parse_resource_uid(token_list[1], &in_p_type, &in_p_id);
+ in_port = find_port_id(in_p_id,
+ get_port_type(in_p_type));
+
+ parse_resource_uid(token_list[2],
+ &out_p_type, &out_p_id);
+ out_port = find_port_id(out_p_id,
+ get_port_type(out_p_type));
+
+ if (in_port == PORT_RESET && out_port == PORT_RESET) {
+ char err_msg[128];
+ memset(err_msg, '\0', sizeof(err_msg));
+ sprintf(err_msg, "%s '%s:%d' and '%s:%d' %s",
+ "Failed to patch, both of",
+ in_p_type, in_p_id,
+ out_p_type, out_p_id,
+ "not found");
+ RTE_LOG(ERR, APP, "%s\n", err_msg);
+ return 0;
+ } else if (in_port == PORT_RESET) {
+ char err_msg[128];
+ memset(err_msg, '\0', sizeof(err_msg));
+ sprintf(err_msg, "%s '%s:%d' not found",
+ "Failed to patch, in_port",
+ in_p_type, in_p_id);
+ RTE_LOG(ERR, APP, "%s\n", err_msg);
return 0;
+ } else if (out_port == PORT_RESET) {
+ char err_msg[128];
+ memset(err_msg, '\0', sizeof(err_msg));
+ sprintf(err_msg, "%s '%s:%d' not found",
+ "Failed to patch, out_port",
+ out_p_type, out_p_id);
+ RTE_LOG(ERR, APP, "%s\n", err_msg);
+ }
add_patch(in_port, out_port);
}
--
2.7.4
^ permalink raw reply [flat|nested] 7+ messages in thread
* [spp] [PATCH 2/6] spp_nfv: refactor log of patch command
2018-10-09 10:53 [spp] [PATCH 0/6] Fix bug of assigning ports ogawa.yasufumi
2018-10-09 10:53 ` [spp] [PATCH 1/6] spp_nfv: fix bug of do_del if port does not exist ogawa.yasufumi
@ 2018-10-09 10:53 ` ogawa.yasufumi
2018-10-09 10:53 ` [spp] [PATCH 3/6] spp_vm: fix bug of do_del if port does not exist ogawa.yasufumi
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: ogawa.yasufumi @ 2018-10-09 10:53 UTC (permalink / raw)
To: spp, ferruh.yigit, ogawa.yasufumi
From: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
To describe the result of patch command correctly, refactor log message.
Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
src/nfv/nfv.c | 38 +++++++++++++++++++++++---------------
1 file changed, 23 insertions(+), 15 deletions(-)
diff --git a/src/nfv/nfv.c b/src/nfv/nfv.c
index 049b3cf..05290ed 100644
--- a/src/nfv/nfv.c
+++ b/src/nfv/nfv.c
@@ -344,17 +344,19 @@ do_del(char *res_uid)
return 0;
}
+/* Return 0 if invalid */
static int
-is_valid_port(int port_id)
+is_valid_port(uint16_t port_id)
{
- if (port_id < 0 || port_id > RTE_MAX_ETHPORTS)
+ if (port_id > RTE_MAX_ETHPORTS)
return 0;
return port_map[port_id].id != PORT_RESET;
}
+/* Return -1 as an error if given patch is invalid */
static int
-add_patch(int in_port, int out_port)
+add_patch(uint16_t in_port, uint16_t out_port)
{
if (!is_valid_port(in_port) || !is_valid_port(out_port))
return -1;
@@ -711,6 +713,7 @@ do_add(char *res_uid)
return 0;
}
+/* Return -1 if exit command is called to terminate the process */
static int
parse_command(char *str)
{
@@ -805,31 +808,36 @@ parse_command(char *str)
if (in_port == PORT_RESET && out_port == PORT_RESET) {
char err_msg[128];
memset(err_msg, '\0', sizeof(err_msg));
- sprintf(err_msg, "%s '%s:%d' and '%s:%d' %s",
- "Failed to patch, both of",
+ sprintf(err_msg, "%s '%s:%d' and '%s:%d'",
+ "Patch not found, both of",
in_p_type, in_p_id,
- out_p_type, out_p_id,
- "not found");
+ out_p_type, out_p_id);
RTE_LOG(ERR, APP, "%s\n", err_msg);
- return 0;
} else if (in_port == PORT_RESET) {
char err_msg[128];
memset(err_msg, '\0', sizeof(err_msg));
- sprintf(err_msg, "%s '%s:%d' not found",
- "Failed to patch, in_port",
+ sprintf(err_msg, "%s '%s:%d'",
+ "Patch not found, in_port",
in_p_type, in_p_id);
RTE_LOG(ERR, APP, "%s\n", err_msg);
- return 0;
} else if (out_port == PORT_RESET) {
char err_msg[128];
memset(err_msg, '\0', sizeof(err_msg));
- sprintf(err_msg, "%s '%s:%d' not found",
- "Failed to patch, out_port",
+ sprintf(err_msg, "%s '%s:%d'",
+ "Patch not found, out_port",
out_p_type, out_p_id);
RTE_LOG(ERR, APP, "%s\n", err_msg);
}
- add_patch(in_port, out_port);
+ if (add_patch(in_port, out_port) == 0)
+ RTE_LOG(INFO, APP,
+ "Patched '%s:%d' and '%s:%d'\n",
+ in_p_type, in_p_id,
+ out_p_type, out_p_id);
+
+ else
+ RTE_LOG(ERR, APP, "Failed to patch\n");
+ ret = 0;
}
} else if (!strcmp(token_list[0], "del")) {
@@ -993,7 +1001,7 @@ main(int argc, char *argv[])
RTE_LOG(DEBUG, APP, "Received string: %s\n", str);
ret = parse_command(str);
- if (ret < 0)
+ if (ret < 0) /* terminate process if exit is called */
break;
/*Send the message back to client*/
--
2.7.4
^ permalink raw reply [flat|nested] 7+ messages in thread
* [spp] [PATCH 3/6] spp_vm: fix bug of do_del if port does not exist
2018-10-09 10:53 [spp] [PATCH 0/6] Fix bug of assigning ports ogawa.yasufumi
2018-10-09 10:53 ` [spp] [PATCH 1/6] spp_nfv: fix bug of do_del if port does not exist ogawa.yasufumi
2018-10-09 10:53 ` [spp] [PATCH 2/6] spp_nfv: refactor log of patch command ogawa.yasufumi
@ 2018-10-09 10:53 ` ogawa.yasufumi
2018-10-09 10:53 ` [spp] [PATCH 4/6] spp_vm: fix to use uint16_t port_id in do_add ogawa.yasufumi
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: ogawa.yasufumi @ 2018-10-09 10:53 UTC (permalink / raw)
To: spp, ferruh.yigit, ogawa.yasufumi
From: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
Fix bug of do_del() causing segmentation fault if not existing port is
deleted. It is because determining the result of find_port_id() is
incorrect and tries to delete invalid port.
This update is to correct the result of find_port_id() and determining
port ID.
Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
src/vm/main.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/vm/main.c b/src/vm/main.c
index 63d5684..3807370 100644
--- a/src/vm/main.c
+++ b/src/vm/main.c
@@ -205,11 +205,15 @@ forward_array_remove(int port_id)
}
}
-static int
+/*
+ * Return actual port ID which is assigned by system internally, or PORT_RESET
+ * if port is not found.
+ */
+static uint16_t
find_port_id(int id, enum port_type type)
{
- int port_id = PORT_RESET;
- unsigned int i;
+ uint16_t port_id = PORT_RESET;
+ uint16_t i;
for (i = 0; i < RTE_MAX_ETHPORTS; i++) {
if (port_map[i].port_type != type)
@@ -227,21 +231,24 @@ find_port_id(int id, enum port_type type)
static int
do_del(char *res_uid)
{
- int port_id = PORT_RESET;
+ uint16_t port_id = PORT_RESET;
char *p_type;
int p_id;
int res;
res = parse_resource_uid(res_uid, &p_type, &p_id);
- if (res < 0)
+ if (res < 0) {
+ RTE_LOG(ERR, APP,
+ "Failed to parse resource UID\n");
return -1;
+ }
if (!strcmp(p_type, "ring")) {
char name[RTE_ETH_NAME_MAX_LEN];
RTE_LOG(DEBUG, APP, "Del ring id %d\n", p_id);
port_id = find_port_id(p_id, RING);
- if (port_id < 0)
+ if (port_id == PORT_RESET)
return -1;
rte_eth_dev_detach(port_id, name);
--
2.7.4
^ permalink raw reply [flat|nested] 7+ messages in thread
* [spp] [PATCH 4/6] spp_vm: fix to use uint16_t port_id in do_add
2018-10-09 10:53 [spp] [PATCH 0/6] Fix bug of assigning ports ogawa.yasufumi
` (2 preceding siblings ...)
2018-10-09 10:53 ` [spp] [PATCH 3/6] spp_vm: fix bug of do_del if port does not exist ogawa.yasufumi
@ 2018-10-09 10:53 ` ogawa.yasufumi
2018-10-09 10:53 ` [spp] [PATCH 5/6] spp_vm: correct error handling if patch cmd failed ogawa.yasufumi
2018-10-09 10:54 ` [spp] [PATCH 6/6] spp_vm: refactor log of patch command ogawa.yasufumi
5 siblings, 0 replies; 7+ messages in thread
From: ogawa.yasufumi @ 2018-10-09 10:53 UTC (permalink / raw)
To: spp, ferruh.yigit, ogawa.yasufumi
From: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
Modify to use uint16_t type for port_id.
Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
src/vm/main.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/vm/main.c b/src/vm/main.c
index 3807370..2813fca 100644
--- a/src/vm/main.c
+++ b/src/vm/main.c
@@ -373,7 +373,7 @@ static int
do_add(char *res_uid)
{
enum port_type type = UNDEF;
- int port_id = PORT_RESET;
+ uint16_t port_id = PORT_RESET;
char *p_type;
int p_id;
int res;
@@ -384,11 +384,13 @@ do_add(char *res_uid)
if (!strcmp(p_type, "ring")) {
type = RING;
- port_id = add_ring_pmd(p_id);
+ res = add_ring_pmd(p_id);
}
- if (port_id < 0)
+ if (res < 0)
return -1;
+ else
+ port_id = (uint16_t) res;
port_map[port_id].id = p_id;
port_map[port_id].port_type = type;
--
2.7.4
^ permalink raw reply [flat|nested] 7+ messages in thread
* [spp] [PATCH 5/6] spp_vm: correct error handling if patch cmd failed
2018-10-09 10:53 [spp] [PATCH 0/6] Fix bug of assigning ports ogawa.yasufumi
` (3 preceding siblings ...)
2018-10-09 10:53 ` [spp] [PATCH 4/6] spp_vm: fix to use uint16_t port_id in do_add ogawa.yasufumi
@ 2018-10-09 10:53 ` ogawa.yasufumi
2018-10-09 10:54 ` [spp] [PATCH 6/6] spp_vm: refactor log of patch command ogawa.yasufumi
5 siblings, 0 replies; 7+ messages in thread
From: ogawa.yasufumi @ 2018-10-09 10:53 UTC (permalink / raw)
To: spp, ferruh.yigit, ogawa.yasufumi
From: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
Refactor of patch command in which find_port_id() is used. This update
is to show an error message in log without terminating the process if
patched port is not found.
Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
src/vm/main.c | 54 ++++++++++++++++++++++++++++++++++++++++++------------
1 file changed, 42 insertions(+), 12 deletions(-)
diff --git a/src/vm/main.c b/src/vm/main.c
index 2813fca..997db5b 100644
--- a/src/vm/main.c
+++ b/src/vm/main.c
@@ -402,6 +402,7 @@ do_add(char *res_uid)
return 0;
}
+/* Return -1 if exit command is called to terminate the process */
static int
parse_command(char *str)
{
@@ -473,23 +474,52 @@ parse_command(char *str)
/* reset forward array*/
forward_array_reset();
} else {
- int in_port;
- int out_port;
+ uint16_t in_port;
+ uint16_t out_port;
if (max_token <= 2)
return 0;
- char *p_type;
- int p_id;
-
- parse_resource_uid(token_list[1], &p_type, &p_id);
- in_port = find_port_id(p_id, get_port_type(p_type));
-
- parse_resource_uid(token_list[2], &p_type, &p_id);
- out_port = find_port_id(p_id, get_port_type(p_type));
-
- if (in_port < 0 || out_port < 0)
+ char *in_p_type;
+ char *out_p_type;
+ int in_p_id;
+ int out_p_id;
+
+ parse_resource_uid(token_list[1], &in_p_type, &in_p_id);
+ in_port = find_port_id(in_p_id,
+ get_port_type(in_p_type));
+
+ parse_resource_uid(token_list[2],
+ &out_p_type, &out_p_id);
+ out_port = find_port_id(out_p_id,
+ get_port_type(out_p_type));
+
+ if (in_port == PORT_RESET && out_port == PORT_RESET) {
+ char err_msg[128];
+ memset(err_msg, '\0', sizeof(err_msg));
+ sprintf(err_msg, "%s '%s:%d' and '%s:%d' %s",
+ "Failed to patch, both of",
+ in_p_type, in_p_id,
+ out_p_type, out_p_id,
+ "not found");
+ RTE_LOG(ERR, APP, "%s\n", err_msg);
+ return 0;
+ } else if (in_port == PORT_RESET) {
+ char err_msg[128];
+ memset(err_msg, '\0', sizeof(err_msg));
+ sprintf(err_msg, "%s '%s:%d' not found",
+ "Failed to patch, in_port",
+ in_p_type, in_p_id);
+ RTE_LOG(ERR, APP, "%s\n", err_msg);
return 0;
+ } else if (out_port == PORT_RESET) {
+ char err_msg[128];
+ memset(err_msg, '\0', sizeof(err_msg));
+ sprintf(err_msg, "%s '%s:%d' not found",
+ "Failed to patch, out_port",
+ out_p_type, out_p_id);
+ RTE_LOG(ERR, APP, "%s\n", err_msg);
+ }
add_patch(in_port, out_port);
}
--
2.7.4
^ permalink raw reply [flat|nested] 7+ messages in thread
* [spp] [PATCH 6/6] spp_vm: refactor log of patch command
2018-10-09 10:53 [spp] [PATCH 0/6] Fix bug of assigning ports ogawa.yasufumi
` (4 preceding siblings ...)
2018-10-09 10:53 ` [spp] [PATCH 5/6] spp_vm: correct error handling if patch cmd failed ogawa.yasufumi
@ 2018-10-09 10:54 ` ogawa.yasufumi
5 siblings, 0 replies; 7+ messages in thread
From: ogawa.yasufumi @ 2018-10-09 10:54 UTC (permalink / raw)
To: spp, ferruh.yigit, ogawa.yasufumi
From: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
To describe the result of patch command correctly, refactor log message.
Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
src/vm/main.c | 36 +++++++++++++++++++++---------------
1 file changed, 21 insertions(+), 15 deletions(-)
diff --git a/src/vm/main.c b/src/vm/main.c
index 997db5b..1d7d83a 100644
--- a/src/vm/main.c
+++ b/src/vm/main.c
@@ -260,17 +260,19 @@ do_del(char *res_uid)
return 0;
}
+/* Return 0 if invalid */
static int
-is_valid_port(int port_id)
+is_valid_port(uint16_t port_id)
{
- if (port_id < 0 || port_id > RTE_MAX_ETHPORTS)
+ if (port_id > RTE_MAX_ETHPORTS)
return 0;
return port_map[port_id].id != PORT_RESET;
}
+/* Return -1 as an error if given patch is invalid */
static int
-add_patch(int in_port, int out_port)
+add_patch(uint16_t in_port, uint16_t out_port)
{
if (!is_valid_port(in_port) || !is_valid_port(out_port))
return -1;
@@ -497,31 +499,35 @@ parse_command(char *str)
if (in_port == PORT_RESET && out_port == PORT_RESET) {
char err_msg[128];
memset(err_msg, '\0', sizeof(err_msg));
- sprintf(err_msg, "%s '%s:%d' and '%s:%d' %s",
- "Failed to patch, both of",
+ sprintf(err_msg, "%s '%s:%d' and '%s:%d'",
+ "Patch not found, both of",
in_p_type, in_p_id,
- out_p_type, out_p_id,
- "not found");
+ out_p_type, out_p_id);
RTE_LOG(ERR, APP, "%s\n", err_msg);
- return 0;
} else if (in_port == PORT_RESET) {
char err_msg[128];
memset(err_msg, '\0', sizeof(err_msg));
- sprintf(err_msg, "%s '%s:%d' not found",
- "Failed to patch, in_port",
+ sprintf(err_msg, "%s '%s:%d'",
+ "Patch not found, in_port",
in_p_type, in_p_id);
RTE_LOG(ERR, APP, "%s\n", err_msg);
- return 0;
} else if (out_port == PORT_RESET) {
char err_msg[128];
memset(err_msg, '\0', sizeof(err_msg));
- sprintf(err_msg, "%s '%s:%d' not found",
- "Failed to patch, out_port",
+ sprintf(err_msg, "%s '%s:%d'",
+ "Patch not found, out_port",
out_p_type, out_p_id);
RTE_LOG(ERR, APP, "%s\n", err_msg);
}
- add_patch(in_port, out_port);
+ if (add_patch(in_port, out_port) == 0)
+ RTE_LOG(INFO, APP,
+ "Patched '%s:%d' and '%s:%d'\n",
+ in_p_type, in_p_id,
+ out_p_type, out_p_id);
+ else
+ RTE_LOG(ERR, APP, "Failed to patch\n");
+ ret = 0;
}
} else if (strncmp(str, "del", 3) == 0) {
RTE_LOG(DEBUG, APP, "Received del command\n");
@@ -646,7 +652,7 @@ main(int argc, char *argv[])
RTE_LOG(DEBUG, APP, "Received string: %s\n", str);
ret = parse_command(str);
- if (ret < 0)
+ if (ret < 0) /* terminate process if exit is called */
break;
/*Send the message back to client*/
--
2.7.4
^ permalink raw reply [flat|nested] 7+ messages in thread