[PATCH 0/4] testpmd forwarding

[PATCH 0/4] testpmd forwarding

[Bernard Iremonger]

Modify testpmd to allow stop, close, detach and attach
of a port without stopping forwarding.

Bernard Iremonger (4):
  testpmd: add function port_is_forwarding
  testpmd: don't update fwding config when attaching/detaching a port
  testpmd: check port is not forwarding in stop_port and close_port
  testpmd: reconfigure forwarding after changing portlist

 app/test-pmd/config.c  | 26 ++++++++++++++++++++++---
 app/test-pmd/testpmd.c | 53 ++++++++++++--------------------------------------
 app/test-pmd/testpmd.h |  3 ++-
 3 files changed, 37 insertions(+), 45 deletions(-)

-- 
2.6.3

[PATCH 1/4] testpmd: add function port_is_forwarding

[Bernard Iremonger]

Add function port_is_forwarding to check whether
a port is forwarding or not.

Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 app/test-pmd/config.c  | 18 +++++++++++++++++-
 app/test-pmd/testpmd.h |  3 ++-
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index b1bbec6..db6b52b 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1,7 +1,7 @@
 /*-
  *   BSD LICENSE
  *
- *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
+ *   Copyright(c) 2010-2016 Intel Corporation. All rights reserved.
  *   All rights reserved.
  *
  *   Redistribution and use in source and binary forms, with or without
@@ -1565,6 +1565,22 @@ set_fwd_ports_number(uint16_t nb_pt)
 	       (unsigned int) nb_fwd_ports);
 }
 
+int
+port_is_forwarding(portid_t port_id)
+{
+	unsigned int i;
+
+	if (port_id_is_invalid(port_id, ENABLED_WARN))
+		return -1;
+
+	for (i = 0; i < nb_fwd_ports; i++) {
+		if (fwd_ports_ids[i] == port_id)
+			return 1;
+	}
+
+	return 0;
+}
+
 void
 set_nb_pkt_per_burst(uint16_t nb)
 {
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 0f72ca1..aa4bdac 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -1,7 +1,7 @@
 /*-
  *   BSD LICENSE
  *
- *   Copyright(c) 2010-2015 Intel Corporation. All rights reserved.
+ *   Copyright(c) 2010-2016 Intel Corporation. All rights reserved.
  *   All rights reserved.
  *
  *   Redistribution and use in source and binary forms, with or without
@@ -500,6 +500,7 @@ void set_fwd_lcores_number(uint16_t nb_lc);
 void set_fwd_ports_list(unsigned int *portlist, unsigned int nb_pt);
 void set_fwd_ports_mask(uint64_t portmask);
 void set_fwd_ports_number(uint16_t nb_pt);
+int port_is_forwarding(portid_t port_id);
 
 void rx_vlan_strip_set(portid_t port_id, int on);
 void rx_vlan_strip_set_on_queue(portid_t port_id, uint16_t queue_id, int on);
-- 
2.6.3

[PATCH 2/4] testpmd: don't update fwding config when attaching/detaching a port

[Bernard Iremonger]

Remove checks on test_done variable.
Remove code to update forwarding configuration.

Fixes: edab33b1c01d ("app/testpmd: support port hotplug")

Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 app/test-pmd/testpmd.c | 28 +---------------------------
 1 file changed, 1 insertion(+), 27 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 26a174c..5dc7bba 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -1506,7 +1506,7 @@ close_port(portid_t pid)
 void
 attach_port(char *identifier)
 {
-	portid_t i, j, pi = 0;
+	portid_t pi = 0;
 
 	printf("Attaching a new port...\n");
 
@@ -1515,11 +1515,6 @@ attach_port(char *identifier)
 		return;
 	}
 
-	if (test_done == 0) {
-		printf("Please stop forwarding first\n");
-		return;
-	}
-
 	if (rte_eth_dev_attach(identifier, &pi))
 		return;
 
@@ -1529,16 +1524,6 @@ attach_port(char *identifier)
 
 	nb_ports = rte_eth_dev_count();
 
-	/* set_default_fwd_ports_config(); */
-	memset(fwd_ports_ids, 0, sizeof(fwd_ports_ids));
-	i = 0;
-	FOREACH_PORT(j, ports) {
-		fwd_ports_ids[i] = j;
-		i++;
-	}
-	nb_cfg_ports = nb_ports;
-	nb_fwd_ports++;
-
 	ports[pi].port_status = RTE_PORT_STOPPED;
 
 	printf("Port %d is attached. Now total ports is %d\n", pi, nb_ports);
@@ -1548,7 +1533,6 @@ attach_port(char *identifier)
 void
 detach_port(uint8_t port_id)
 {
-	portid_t i, pi = 0;
 	char name[RTE_ETH_NAME_MAX_LEN];
 
 	printf("Detaching a port...\n");
@@ -1564,16 +1548,6 @@ detach_port(uint8_t port_id)
 	ports[port_id].enabled = 0;
 	nb_ports = rte_eth_dev_count();
 
-	/* set_default_fwd_ports_config(); */
-	memset(fwd_ports_ids, 0, sizeof(fwd_ports_ids));
-	i = 0;
-	FOREACH_PORT(pi, ports) {
-		fwd_ports_ids[i] = pi;
-		i++;
-	}
-	nb_cfg_ports = nb_ports;
-	nb_fwd_ports--;
-
 	printf("Port '%s' is detached. Now total ports is %d\n",
 			name, nb_ports);
 	printf("Done\n");
-- 
2.6.3

[PATCH 3/4] testpmd: check port is not forwarding in stop_port and close_port

[Bernard Iremonger]

Add calls to port_is_forwarding function.
Remove checks on test_done variable.

Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")

Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 app/test-pmd/testpmd.c | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 5dc7bba..7ccafd3 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -1268,11 +1268,6 @@ start_port(portid_t pid)
 	struct rte_port *port;
 	struct ether_addr mac_addr;
 
-	if (test_done == 0) {
-		printf("Please stop forwarding first\n");
-		return -1;
-	}
-
 	if (port_id_is_invalid(pid, ENABLED_WARN))
 		return 0;
 
@@ -1424,10 +1419,6 @@ stop_port(portid_t pid)
 	struct rte_port *port;
 	int need_check_link_status = 0;
 
-	if (test_done == 0) {
-		printf("Please stop forwarding first\n");
-		return;
-	}
 	if (dcb_test) {
 		dcb_test = 0;
 		dcb_config = 0;
@@ -1442,6 +1433,11 @@ stop_port(portid_t pid)
 		if (pid != pi && pid != (portid_t)RTE_PORT_ALL)
 			continue;
 
+		if (port_is_forwarding(pi) != 0) {
+			printf("Please remove port %d from forwarding configuration.\n", pi);
+			continue;
+		}
+
 		port = &ports[pi];
 		if (rte_atomic16_cmpset(&(port->port_status), RTE_PORT_STARTED,
 						RTE_PORT_HANDLING) == 0)
@@ -1466,11 +1462,6 @@ close_port(portid_t pid)
 	portid_t pi;
 	struct rte_port *port;
 
-	if (test_done == 0) {
-		printf("Please stop forwarding first\n");
-		return;
-	}
-
 	if (port_id_is_invalid(pid, ENABLED_WARN))
 		return;
 
@@ -1480,6 +1471,11 @@ close_port(portid_t pid)
 		if (pid != pi && pid != (portid_t)RTE_PORT_ALL)
 			continue;
 
+		if (port_is_forwarding(pi) != 0) {
+			printf("Please remove port %d from forwarding configuration.\n", pi);
+			continue;
+		}
+
 		port = &ports[pi];
 		if (rte_atomic16_cmpset(&(port->port_status),
 			RTE_PORT_CLOSED, RTE_PORT_CLOSED) == 1) {
-- 
2.6.3

[PATCH 4/4] testpmd: reconfigure forwarding after changing portlist

[Bernard Iremonger]

Set nb_fwd_ports to zero on quit.
Check portlist has been set before displaying forwarding configuration.

Fixes: d3a274ce9dee ("app/testpmd: handle SIGINT and SIGTERM")
Fixes: af75078fece3 ("first public release")

Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 app/test-pmd/config.c  | 8 ++++++--
 app/test-pmd/testpmd.c | 1 +
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index db6b52b..ff040d1 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1424,8 +1424,10 @@ pkt_fwd_config_display(struct fwd_config *cfg)
 void
 fwd_config_display(void)
 {
-	fwd_config_setup();
-	pkt_fwd_config_display(&cur_fwd_config);
+	if (cur_fwd_config.nb_fwd_ports)
+		pkt_fwd_config_display(&cur_fwd_config);
+	else
+		printf("Please set portlist first\n");
 }
 
 int
@@ -1529,6 +1531,8 @@ set_fwd_ports_list(unsigned int *portlist, unsigned int nb_pt)
 		       (unsigned int) nb_fwd_ports, nb_pt);
 		nb_fwd_ports = (portid_t) nb_pt;
 	}
+
+	fwd_config_setup();
 }
 
 void
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 7ccafd3..11e7fe0 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -1560,6 +1560,7 @@ pmd_test_exit(void)
 
 	if (ports != NULL) {
 		no_link_check = 1;
+		nb_fwd_ports = 0;
 		FOREACH_PORT(pt_id, ports) {
 			printf("\nShutting down port %d...\n", pt_id);
 			fflush(stdout);
-- 
2.6.3

[PATCH v2 0/8] app/testpmd: forwarding

[Bernard Iremonger]

Modify testpmd to allow stop, close, detach and attach
of a port without stopping forwarding.

This patchset should not be applied unless the following bonding patchset is applied:

0001-bonding-replace-spinlock-with-read-write-lock.patch
0002-bonding-add-read-write-lock-to-rx-tx-burst-functions.patch
0003-bonding-remove-memcopy-of-slaves-from-rx-tx-burst-fu.patch
0004-bonding-add-read-write-lock-to-stop-function.patch
0005-bonding-add-read-write-lock-to-the-link_update-funct.patch

Changes in v2:
Added 4 more patches
  app/testpmd: add function port_is_bonding_slave
  app/testpmd: move call to init_fwd_streams
  app/testpmd: check for valid socket id when attaching port
  app/testpmd: check for valid mbuf pool

Bernard Iremonger (8):
  app/testpmd: add function port_is_forwarding
  app/testpmd: don't update fwding config when attaching/detaching a
    port
  app/testpmd: check port is not forwarding in stop_port and close_port
  app/testpmd: reconfigure forwarding after changing portlist
  app/testpmd: add function port_is_bonding_slave
  app/testpmd: move call to init_fwd_streams
  app/testpmd: check for valid socket id when attaching port
  app/testpmd: check for valid mbuf pool

 app/test-pmd/config.c  |  26 ++++++++++--
 app/test-pmd/testpmd.c | 109 +++++++++++++++++++++++++------------------------
 app/test-pmd/testpmd.h |   5 ++-
 3 files changed, 82 insertions(+), 58 deletions(-)

-- 
2.6.3

[PATCH v2 1/8] app/testpmd: add function port_is_forwarding

[Bernard Iremonger]

Add function port_is_forwarding to check whether
a port is forwarding or not.

Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 app/test-pmd/config.c  | 18 +++++++++++++++++-
 app/test-pmd/testpmd.h |  3 ++-
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 1c552e4..f434999 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1,7 +1,7 @@
 /*-
  *   BSD LICENSE
  *
- *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
+ *   Copyright(c) 2010-2016 Intel Corporation. All rights reserved.
  *   All rights reserved.
  *
  *   Redistribution and use in source and binary forms, with or without
@@ -1565,6 +1565,22 @@ set_fwd_ports_number(uint16_t nb_pt)
 	       (unsigned int) nb_fwd_ports);
 }
 
+int
+port_is_forwarding(portid_t port_id)
+{
+	unsigned int i;
+
+	if (port_id_is_invalid(port_id, ENABLED_WARN))
+		return -1;
+
+	for (i = 0; i < nb_fwd_ports; i++) {
+		if (fwd_ports_ids[i] == port_id)
+			return 1;
+	}
+
+	return 0;
+}
+
 void
 set_nb_pkt_per_burst(uint16_t nb)
 {
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 0f72ca1..aa4bdac 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -1,7 +1,7 @@
 /*-
  *   BSD LICENSE
  *
- *   Copyright(c) 2010-2015 Intel Corporation. All rights reserved.
+ *   Copyright(c) 2010-2016 Intel Corporation. All rights reserved.
  *   All rights reserved.
  *
  *   Redistribution and use in source and binary forms, with or without
@@ -500,6 +500,7 @@ void set_fwd_lcores_number(uint16_t nb_lc);
 void set_fwd_ports_list(unsigned int *portlist, unsigned int nb_pt);
 void set_fwd_ports_mask(uint64_t portmask);
 void set_fwd_ports_number(uint16_t nb_pt);
+int port_is_forwarding(portid_t port_id);
 
 void rx_vlan_strip_set(portid_t port_id, int on);
 void rx_vlan_strip_set_on_queue(portid_t port_id, uint16_t queue_id, int on);
-- 
2.6.3

[PATCH v2 2/8] app/testpmd: don't update fwding config when attaching/detaching a port

[Bernard Iremonger]

Remove checks on test_done variable.
Remove code to update forwarding configuration.

Fixes: edab33b1c01d ("app/testpmd: support port hotplug")

Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 app/test-pmd/testpmd.c | 28 +---------------------------
 1 file changed, 1 insertion(+), 27 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 26a174c..5dc7bba 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -1506,7 +1506,7 @@ close_port(portid_t pid)
 void
 attach_port(char *identifier)
 {
-	portid_t i, j, pi = 0;
+	portid_t pi = 0;
 
 	printf("Attaching a new port...\n");
 
@@ -1515,11 +1515,6 @@ attach_port(char *identifier)
 		return;
 	}
 
-	if (test_done == 0) {
-		printf("Please stop forwarding first\n");
-		return;
-	}
-
 	if (rte_eth_dev_attach(identifier, &pi))
 		return;
 
@@ -1529,16 +1524,6 @@ attach_port(char *identifier)
 
 	nb_ports = rte_eth_dev_count();
 
-	/* set_default_fwd_ports_config(); */
-	memset(fwd_ports_ids, 0, sizeof(fwd_ports_ids));
-	i = 0;
-	FOREACH_PORT(j, ports) {
-		fwd_ports_ids[i] = j;
-		i++;
-	}
-	nb_cfg_ports = nb_ports;
-	nb_fwd_ports++;
-
 	ports[pi].port_status = RTE_PORT_STOPPED;
 
 	printf("Port %d is attached. Now total ports is %d\n", pi, nb_ports);
@@ -1548,7 +1533,6 @@ attach_port(char *identifier)
 void
 detach_port(uint8_t port_id)
 {
-	portid_t i, pi = 0;
 	char name[RTE_ETH_NAME_MAX_LEN];
 
 	printf("Detaching a port...\n");
@@ -1564,16 +1548,6 @@ detach_port(uint8_t port_id)
 	ports[port_id].enabled = 0;
 	nb_ports = rte_eth_dev_count();
 
-	/* set_default_fwd_ports_config(); */
-	memset(fwd_ports_ids, 0, sizeof(fwd_ports_ids));
-	i = 0;
-	FOREACH_PORT(pi, ports) {
-		fwd_ports_ids[i] = pi;
-		i++;
-	}
-	nb_cfg_ports = nb_ports;
-	nb_fwd_ports--;
-
 	printf("Port '%s' is detached. Now total ports is %d\n",
 			name, nb_ports);
 	printf("Done\n");
-- 
2.6.3

[PATCH v2 3/8] app/testpmd: check port is not forwarding in stop_port and close_port

[Bernard Iremonger]

Add calls to port_is_forwarding function.

Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 app/test-pmd/testpmd.c | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 5dc7bba..11b4cf7 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -1268,11 +1268,6 @@ start_port(portid_t pid)
 	struct rte_port *port;
 	struct ether_addr mac_addr;
 
-	if (test_done == 0) {
-		printf("Please stop forwarding first\n");
-		return -1;
-	}
-
 	if (port_id_is_invalid(pid, ENABLED_WARN))
 		return 0;
 
@@ -1424,10 +1419,6 @@ stop_port(portid_t pid)
 	struct rte_port *port;
 	int need_check_link_status = 0;
 
-	if (test_done == 0) {
-		printf("Please stop forwarding first\n");
-		return;
-	}
 	if (dcb_test) {
 		dcb_test = 0;
 		dcb_config = 0;
@@ -1442,6 +1433,11 @@ stop_port(portid_t pid)
 		if (pid != pi && pid != (portid_t)RTE_PORT_ALL)
 			continue;
 
+		if (port_is_forwarding(pi) != 0 && test_done == 0) {
+			printf("Please remove port %d from forwarding configuration.\n", pi);
+			continue;
+		}
+
 		port = &ports[pi];
 		if (rte_atomic16_cmpset(&(port->port_status), RTE_PORT_STARTED,
 						RTE_PORT_HANDLING) == 0)
@@ -1466,11 +1462,6 @@ close_port(portid_t pid)
 	portid_t pi;
 	struct rte_port *port;
 
-	if (test_done == 0) {
-		printf("Please stop forwarding first\n");
-		return;
-	}
-
 	if (port_id_is_invalid(pid, ENABLED_WARN))
 		return;
 
@@ -1480,6 +1471,11 @@ close_port(portid_t pid)
 		if (pid != pi && pid != (portid_t)RTE_PORT_ALL)
 			continue;
 
+		if (port_is_forwarding(pi) != 0 && test_done == 0) {
+			printf("Please remove port %d from forwarding configuration.\n", pi);
+			continue;
+		}
+
 		port = &ports[pi];
 		if (rte_atomic16_cmpset(&(port->port_status),
 			RTE_PORT_CLOSED, RTE_PORT_CLOSED) == 1) {
-- 
2.6.3

[PATCH v2 4/8] app/testpmd: reconfigure forwarding after changing portlist

[Bernard Iremonger]

Set nb_fwd_ports to zero on quit.
Check portlist has been set before displaying forwarding configuration.

Fixes: d3a274ce9dee ("app/testpmd: handle SIGINT and SIGTERM")
Fixes: af75078fece3 ("first public release")

Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 app/test-pmd/config.c  | 8 ++++++--
 app/test-pmd/testpmd.c | 1 +
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index f434999..10ac768 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1424,8 +1424,10 @@ pkt_fwd_config_display(struct fwd_config *cfg)
 void
 fwd_config_display(void)
 {
-	fwd_config_setup();
-	pkt_fwd_config_display(&cur_fwd_config);
+	if (cur_fwd_config.nb_fwd_ports)
+		pkt_fwd_config_display(&cur_fwd_config);
+	else
+		printf("Please set portlist first\n");
 }
 
 int
@@ -1529,6 +1531,8 @@ set_fwd_ports_list(unsigned int *portlist, unsigned int nb_pt)
 		       (unsigned int) nb_fwd_ports, nb_pt);
 		nb_fwd_ports = (portid_t) nb_pt;
 	}
+
+	fwd_config_setup();
 }
 
 void
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 11b4cf7..2c58075 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -1560,6 +1560,7 @@ pmd_test_exit(void)
 
 	if (ports != NULL) {
 		no_link_check = 1;
+		nb_fwd_ports = 0;
 		FOREACH_PORT(pt_id, ports) {
 			printf("\nShutting down port %d...\n", pt_id);
 			fflush(stdout);
-- 
2.6.3

Re: [PATCH v2 4/8] app/testpmd: reconfigure forwarding after changing portlist

[De Lara Guarch, Pablo]

Hi Bernard,

> -----Original Message-----
> From: Iremonger, Bernard
> Sent: Thursday, May 05, 2016 4:40 PM
> To: dev@dpdk.org
> Cc: De Lara Guarch, Pablo; Iremonger, Bernard
> Subject: [PATCH v2 4/8] app/testpmd: reconfigure forwarding after changing
> portlist
> 
> Set nb_fwd_ports to zero on quit.
> Check portlist has been set before displaying forwarding configuration.
> 
> Fixes: d3a274ce9dee ("app/testpmd: handle SIGINT and SIGTERM")
> Fixes: af75078fece3 ("first public release")

This patch is not fixing any issue, right? You are trying to improve the behaviour when changing portlist.
Therefore, you don't need to use Fixes tag.

> 
> Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
> ---
>  app/test-pmd/config.c  | 8 ++++++--
>  app/test-pmd/testpmd.c | 1 +
>  2 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index f434999..10ac768 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -1424,8 +1424,10 @@ pkt_fwd_config_display(struct fwd_config *cfg)
>  void
>  fwd_config_display(void)
>  {
> -	fwd_config_setup();
> -	pkt_fwd_config_display(&cur_fwd_config);
> +	if (cur_fwd_config.nb_fwd_ports)
> +		pkt_fwd_config_display(&cur_fwd_config);
> +	else
> +		printf("Please set portlist first\n");
>  }

The problem of doing this is that if user starts testpmd, it is not possible to show
the configuration of the ports directly, since fwd_config_setup() has not being called
(because set_fwd_ports_list() has not being called), so it looks like portlist must be set,
but if user starts forwarding directly, then it is not necessary.
What I mean, is that by default, portlist should be all the ports.
Maybe we need to call fwd_config_setup after all the testpmd initialization.

> 
>  int
> @@ -1529,6 +1531,8 @@ set_fwd_ports_list(unsigned int *portlist, unsigned
> int nb_pt)
>  		       (unsigned int) nb_fwd_ports, nb_pt);
>  		nb_fwd_ports = (portid_t) nb_pt;
>  	}
> +
> +	fwd_config_setup();
>  }

I understand what you are doing here, but there is a problem. If you use --portmask parameter,
this function gets called when the arguments are parsed, but at that point, the ports are not configured yet,
and you get the following:

Fail: nb_rxq(1) is greater than max_rx_queues(0)
Program received signal SIGSEGV, Segmentation fault.
0x00000000004835c9 in setup_fwd_config_of_each_lcore (cfg=0xca4160 <cur_fwd_config>) at /tmp/dpdk-latest/app/test-pmd/config.c:1073

Anyway, I like the idea of moving fwd_config_setup out of fwd_config_display().
The problem is that there are other functions that should call this, such as set_fwd_lcores_list
(so, with this patch, if coremask is changed and then we call "show config fwd", we will not see any change).
Basically, all that affects the forwarding configuration should reconfigure it.
That's why I think it was decided to reconfigure the configuration when starting the forwarding or when showing the configuration.

So, we have two options:
1 - We add fwd_config_setup() in all the functions that are changing the configurations.
2 - We leave it as it was, especially with this patch, it makes more sense: http://dpdk.org/dev/patchwork/patch/13132/


> 
>  void
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index 11b4cf7..2c58075 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -1560,6 +1560,7 @@ pmd_test_exit(void)
> 
>  	if (ports != NULL) {
>  		no_link_check = 1;
> +		nb_fwd_ports = 0;

Is this really necessary? I have removed it and I can quit testpmd with no problem.

>  		FOREACH_PORT(pt_id, ports) {
>  			printf("\nShutting down port %d...\n", pt_id);
>  			fflush(stdout);
> --
> 2.6.3

Re: [PATCH v2 4/8] app/testpmd: reconfigure forwarding after changing portlist

[Iremonger, Bernard]

Hi Pablo,

<snip>

> > Subject: [PATCH v2 4/8] app/testpmd: reconfigure forwarding after
> > changing portlist
> >
> > Set nb_fwd_ports to zero on quit.
> > Check portlist has been set before displaying forwarding configuration.
> >
> > Fixes: d3a274ce9dee ("app/testpmd: handle SIGINT and SIGTERM")
> > Fixes: af75078fece3 ("first public release")
> 
> This patch is not fixing any issue, right? You are trying to improve the
> behaviour when changing portlist.
> Therefore, you don't need to use Fixes tag.

Ok, fixes tag is not necessary here.

> > Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
> > ---
> >  app/test-pmd/config.c  | 8 ++++++--
> >  app/test-pmd/testpmd.c | 1 +
> >  2 files changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index
> > f434999..10ac768 100644
> > --- a/app/test-pmd/config.c
> > +++ b/app/test-pmd/config.c
> > @@ -1424,8 +1424,10 @@ pkt_fwd_config_display(struct fwd_config *cfg)
> > void
> >  fwd_config_display(void)
> >  {
> > -	fwd_config_setup();
> > -	pkt_fwd_config_display(&cur_fwd_config);
> > +	if (cur_fwd_config.nb_fwd_ports)
> > +		pkt_fwd_config_display(&cur_fwd_config);
> > +	else
> > +		printf("Please set portlist first\n");
> >  }
> 
> The problem of doing this is that if user starts testpmd, it is not possible to
> show the configuration of the ports directly, since fwd_config_setup() has
> not being called (because set_fwd_ports_list() has not being called), so it
> looks like portlist must be set, but if user starts forwarding directly, then it is
> not necessary.
> What I mean, is that by default, portlist should be all the ports.
> Maybe we need to call fwd_config_setup after all the testpmd initialization.
> 
> >
> >  int
> > @@ -1529,6 +1531,8 @@ set_fwd_ports_list(unsigned int *portlist,
> > unsigned int nb_pt)
> >  		       (unsigned int) nb_fwd_ports, nb_pt);
> >  		nb_fwd_ports = (portid_t) nb_pt;
> >  	}
> > +
> > +	fwd_config_setup();
> >  }
> 
> I understand what you are doing here, but there is a problem. If you use --
> portmask parameter, this function gets called when the arguments are
> parsed, but at that point, the ports are not configured yet, and you get the
> following:
> 
> Fail: nb_rxq(1) is greater than max_rx_queues(0) Program received signal
> SIGSEGV, Segmentation fault.
> 0x00000000004835c9 in setup_fwd_config_of_each_lcore (cfg=0xca4160
> <cur_fwd_config>) at /tmp/dpdk-latest/app/test-pmd/config.c:1073
> 
> Anyway, I like the idea of moving fwd_config_setup out of
> fwd_config_display().
> The problem is that there are other functions that should call this, such as
> set_fwd_lcores_list (so, with this patch, if coremask is changed and then we
> call "show config fwd", we will not see any change).
> Basically, all that affects the forwarding configuration should reconfigure it.
> That's why I think it was decided to reconfigure the configuration when
> starting the forwarding or when showing the configuration.
> 
> So, we have two options:
> 1 - We add fwd_config_setup() in all the functions that are changing the
> configurations.
> 2 - We leave it as it was, especially with this patch, it makes more sense:
> http://dpdk.org/dev/patchwork/patch/13132/

Option 2 looks like the best choice here, to drop this patch in favour of patch
http://dpdk.org/dev/patchwork/patch/13132/
which is already acked.

> >  void
> > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> > 11b4cf7..2c58075 100644
> > --- a/app/test-pmd/testpmd.c
> > +++ b/app/test-pmd/testpmd.c
> > @@ -1560,6 +1560,7 @@ pmd_test_exit(void)
> >
> >  	if (ports != NULL) {
> >  		no_link_check = 1;
> > +		nb_fwd_ports = 0;
> 
> Is this really necessary? I have removed it and I can quit testpmd with no
> problem.

Ok, was just clearing this on exit as it had been set previously.

> 
> >  		FOREACH_PORT(pt_id, ports) {
> >  			printf("\nShutting down port %d...\n", pt_id);
> >  			fflush(stdout);
> > --
> > 2.6.3

Re: [PATCH v2 4/8] app/testpmd: reconfigure forwarding after changing portlist

[Iremonger, Bernard]

Hi Pablo,
<snip>

> > > Subject: [PATCH v2 4/8] app/testpmd: reconfigure forwarding after
> > > changing portlist
> > >
> > > Set nb_fwd_ports to zero on quit.
> > > Check portlist has been set before displaying forwarding configuration.
> > >
> > > Fixes: d3a274ce9dee ("app/testpmd: handle SIGINT and SIGTERM")
> > > Fixes: af75078fece3 ("first public release")
> >
> > This patch is not fixing any issue, right? You are trying to improve
> > the behaviour when changing portlist.
> > Therefore, you don't need to use Fixes tag.
> 
> Ok, fixes tag is not necessary here.
> 
> > > Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
> > > ---
> > >  app/test-pmd/config.c  | 8 ++++++--  app/test-pmd/testpmd.c | 1 +
> > >  2 files changed, 7 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index
> > > f434999..10ac768 100644
> > > --- a/app/test-pmd/config.c
> > > +++ b/app/test-pmd/config.c
> > > @@ -1424,8 +1424,10 @@ pkt_fwd_config_display(struct fwd_config
> > > *cfg) void
> > >  fwd_config_display(void)
> > >  {
> > > -	fwd_config_setup();
> > > -	pkt_fwd_config_display(&cur_fwd_config);
> > > +	if (cur_fwd_config.nb_fwd_ports)
> > > +		pkt_fwd_config_display(&cur_fwd_config);
> > > +	else
> > > +		printf("Please set portlist first\n");
> > >  }
> >
> > The problem of doing this is that if user starts testpmd, it is not
> > possible to show the configuration of the ports directly, since
> > fwd_config_setup() has not being called (because set_fwd_ports_list()
> > has not being called), so it looks like portlist must be set, but if
> > user starts forwarding directly, then it is not necessary.
> > What I mean, is that by default, portlist should be all the ports.
> > Maybe we need to call fwd_config_setup after all the testpmd
> initialization.

I will investigate this.

> > >  int
> > > @@ -1529,6 +1531,8 @@ set_fwd_ports_list(unsigned int *portlist,
> > > unsigned int nb_pt)
> > >  		       (unsigned int) nb_fwd_ports, nb_pt);
> > >  		nb_fwd_ports = (portid_t) nb_pt;
> > >  	}
> > > +
> > > +	fwd_config_setup();
> > >  }
> >
> > I understand what you are doing here, but there is a problem. If you
> > use -- portmask parameter, this function gets called when the
> > arguments are parsed, but at that point, the ports are not configured
> > yet, and you get the
> > following:
> >
> > Fail: nb_rxq(1) is greater than max_rx_queues(0) Program received
> > signal SIGSEGV, Segmentation fault.
> > 0x00000000004835c9 in setup_fwd_config_of_each_lcore (cfg=0xca4160
> > <cur_fwd_config>) at /tmp/dpdk-latest/app/test-pmd/config.c:1073
> >
> > Anyway, I like the idea of moving fwd_config_setup out of
> > fwd_config_display().

fwd_config_setup() should be moved out of  fwd_config_display() .
The display should not setup the config again. 

> > The problem is that there are other functions that should call this,
> > such as set_fwd_lcores_list (so, with this patch, if coremask is
> > changed and then we call "show config fwd", we will not see any change).
> > Basically, all that affects the forwarding configuration should reconfigure it.
> > That's why I think it was decided to reconfigure the configuration
> > when starting the forwarding or when showing the configuration.
> >
> > So, we have two options:
> > 1 - We add fwd_config_setup() in all the functions that are changing
> > the configurations.

This is probably the best way to go.

> > 2 - We leave it as it was, especially with this patch, it makes more sense:
> > http://dpdk.org/dev/patchwork/patch/13132/
> 
> Option 2 looks like the best choice here, to drop this patch in favour of patch
> http://dpdk.org/dev/patchwork/patch/13132/
> which is already acked.

I have changed my mind about option 2, this is doing a rename to clarify what is happening.
fwd_config_display() is renamed to fwd_config_setup_display(), it does not separate the setup from the display.
 
> > >  void
> > > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> > > 11b4cf7..2c58075 100644
> > > --- a/app/test-pmd/testpmd.c
> > > +++ b/app/test-pmd/testpmd.c
> > > @@ -1560,6 +1560,7 @@ pmd_test_exit(void)
> > >
> > >  	if (ports != NULL) {
> > >  		no_link_check = 1;
> > > +		nb_fwd_ports = 0;
> >
> > Is this really necessary? I have removed it and I can quit testpmd
> > with no problem.
> 
> Ok, was just clearing this on exit as it had been set previously.
> 
> >
> > >  		FOREACH_PORT(pt_id, ports) {
> > >  			printf("\nShutting down port %d...\n", pt_id);
> > >  			fflush(stdout);
> > > --
> > > 2.6.3

[PATCH v2 5/8] app/testpmd: add function port_is_bonding_slave

[Bernard Iremonger]

Use this function in stop_port and close_port functions.

Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 app/test-pmd/testpmd.c | 18 ++++++++++++++++++
 app/test-pmd/testpmd.h |  2 ++
 2 files changed, 20 insertions(+)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 2c58075..e5e50ca 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -1438,6 +1438,11 @@ stop_port(portid_t pid)
 			continue;
 		}
 
+		if (port_is_bonding_slave(pi)) {
+			printf("Please remove port %d from bonded device.\n", pi);
+			continue;
+		}
+
 		port = &ports[pi];
 		if (rte_atomic16_cmpset(&(port->port_status), RTE_PORT_STARTED,
 						RTE_PORT_HANDLING) == 0)
@@ -1476,6 +1481,11 @@ close_port(portid_t pid)
 			continue;
 		}
 
+		if (port_is_bonding_slave(pi)) {
+			printf("Please remove port %d from bonded device.\n", pi);
+			continue;
+		}
+
 		port = &ports[pi];
 		if (rte_atomic16_cmpset(&(port->port_status),
 			RTE_PORT_CLOSED, RTE_PORT_CLOSED) == 1) {
@@ -1814,6 +1824,14 @@ void clear_port_slave_flag(portid_t slave_pid)
 	port->slave_flag = 0;
 }
 
+uint8_t port_is_bonding_slave(portid_t slave_pid)
+{
+	struct rte_port *port;
+
+	port = &ports[slave_pid];
+	return port->slave_flag;
+}
+
 const uint16_t vlan_tags[] = {
 		0,  1,  2,  3,  4,  5,  6,  7,
 		8,  9, 10, 11,  12, 13, 14, 15,
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index aa4bdac..50f81d7 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -532,6 +532,8 @@ void dev_set_link_down(portid_t pid);
 void init_port_config(void);
 void set_port_slave_flag(portid_t slave_pid);
 void clear_port_slave_flag(portid_t slave_pid);
+uint8_t port_is_bonding_slave(portid_t slave_pid);
+
 int init_port_dcb_config(portid_t pid, enum dcb_mode_enable dcb_mode,
 		     enum rte_eth_nb_tcs num_tcs,
 		     uint8_t pfc_en);
-- 
2.6.3

[PATCH v2 6/8] app/testpmd: move call to init_fwd_streams

[Bernard Iremonger]

Move call to init_fwd_streams from start_port function
to start_packet_forwarding function.

Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 app/test-pmd/testpmd.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index e5e50ca..e632a5b 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -981,6 +981,12 @@ start_packet_forwarding(int with_tx_first)
 		printf("Packet forwarding already started\n");
 		return;
 	}
+
+	if (init_fwd_streams() < 0) {
+		printf("Fail from init_fwd_streams()\n");
+		return;
+	}
+
 	if(dcb_test) {
 		for (i = 0; i < nb_fwd_ports; i++) {
 			pt_id = fwd_ports_ids[i];
@@ -1271,11 +1277,6 @@ start_port(portid_t pid)
 	if (port_id_is_invalid(pid, ENABLED_WARN))
 		return 0;
 
-	if (init_fwd_streams() < 0) {
-		printf("Fail from init_fwd_streams()\n");
-		return -1;
-	}
-
 	if(dcb_config)
 		dcb_test = 1;
 	FOREACH_PORT(pi, ports) {
-- 
2.6.3

[PATCH v2 7/8] app/testpmd: check for valid socket id when attaching port

[Bernard Iremonger]

Fixes: edab33b1c01d ("app/testpmd: support port hotplug")
Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 app/test-pmd/testpmd.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index e632a5b..f044a91 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -1514,6 +1514,7 @@ void
 attach_port(char *identifier)
 {
 	portid_t pi = 0;
+	unsigned int socket_id;
 
 	printf("Attaching a new port...\n");
 
@@ -1526,7 +1527,11 @@ attach_port(char *identifier)
 		return;
 
 	ports[pi].enabled = 1;
-	reconfig(pi, rte_eth_dev_socket_id(pi));
+	socket_id = (unsigned)rte_eth_dev_socket_id(pi);
+	/* if socket_id is invalid, set to 0 */
+	if (check_socket_id(socket_id) < 0)
+		socket_id = 0;
+	reconfig(pi, socket_id);
 	rte_eth_promiscuous_enable(pi);
 
 	nb_ports = rte_eth_dev_count();
-- 
2.6.3

[PATCH v2 8/8] app/testpmd: check for valid mbuf pool

[Bernard Iremonger]

Fixes: b6ea6408fbc7 ("ethdev: store numa_node per device")
Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 app/test-pmd/testpmd.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index f044a91..dad616d 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -1347,7 +1347,7 @@ start_port(portid_t pid)
 					if (mp == NULL) {
 						printf("Failed to setup RX queue:"
 							"No mempool allocation"
-							"on the socket %d\n",
+							" on the socket %d\n",
 							rxring_numa[pi]);
 						return -1;
 					}
@@ -1355,17 +1355,23 @@ start_port(portid_t pid)
 					diag = rte_eth_rx_queue_setup(pi, qi,
 					     nb_rxd,rxring_numa[pi],
 					     &(port->rx_conf),mp);
-				}
-				else
+				} else {
+					struct rte_mempool *mp =
+						mbuf_pool_find(port->socket_id);
+					if (mp == NULL) {
+						printf("Failed to setup RX queue:"
+							"No mempool allocation"
+							" on the socket %d\n",
+							port->socket_id);
+						return -1;
+					}
 					diag = rte_eth_rx_queue_setup(pi, qi,
 					     nb_rxd,port->socket_id,
-					     &(port->rx_conf),
-				             mbuf_pool_find(port->socket_id));
-
+					     &(port->rx_conf), mp);
+				}
 				if (diag == 0)
 					continue;
 
-
 				/* Fail to setup rx queue, return */
 				if (rte_atomic16_cmpset(&(port->port_status),
 							RTE_PORT_HANDLING,
-- 
2.6.3

[PATCH v3 0/9] app/testpmd: forwarding

[Bernard Iremonger]

Modify testpmd to allow stop, close, detach and attach
of a port without stopping forwarding.

Changes in v3:
rebase to latest master.
added patch 9
  app/testpmd: stop forwarding on exit
reworked and renamed patch 4
  app/testpmd: remove fwd_config_setup from fwd_config_display

Changes in v2:
Added 4 more patches
  app/testpmd: add function port_is_bonding_slave
  app/testpmd: move call to init_fwd_streams
  app/testpmd: check for valid socket id when attaching port
  app/testpmd: check for valid mbuf pool

Bernard Iremonger (9):
  app/testpmd: add function port_is_forwarding
  app/testpmd: don't update fwding config when attaching/detaching a
    port
  app/testpmd: check port is not forwarding in stop_port and close_port
  app/testpmd: remove fwd_config_setup from fwd_config_display
  app/testpmd: add function port_is_bonding_slave
  app/testpmd: move call to init_fwd_streams
  app/testpmd: check for valid socket id when attaching port
  app/testpmd: check for valid mbuf pool
  app/testpmd: stop forwarding on exit

 app/test-pmd/cmdline.c |  23 ++++++----
 app/test-pmd/config.c  |  24 +++++++++--
 app/test-pmd/testpmd.c | 112 ++++++++++++++++++++++++-------------------------
 app/test-pmd/testpmd.h |   5 ++-
 4 files changed, 95 insertions(+), 69 deletions(-)

-- 
2.6.3

[PATCH v3 1/9] app/testpmd: add function port_is_forwarding

[Bernard Iremonger]

Add function port_is_forwarding to check whether
a port is forwarding or not.

Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 app/test-pmd/config.c  | 18 +++++++++++++++++-
 app/test-pmd/testpmd.h |  3 ++-
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 1c552e4..f434999 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1,7 +1,7 @@
 /*-
  *   BSD LICENSE
  *
- *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
+ *   Copyright(c) 2010-2016 Intel Corporation. All rights reserved.
  *   All rights reserved.
  *
  *   Redistribution and use in source and binary forms, with or without
@@ -1565,6 +1565,22 @@ set_fwd_ports_number(uint16_t nb_pt)
 	       (unsigned int) nb_fwd_ports);
 }
 
+int
+port_is_forwarding(portid_t port_id)
+{
+	unsigned int i;
+
+	if (port_id_is_invalid(port_id, ENABLED_WARN))
+		return -1;
+
+	for (i = 0; i < nb_fwd_ports; i++) {
+		if (fwd_ports_ids[i] == port_id)
+			return 1;
+	}
+
+	return 0;
+}
+
 void
 set_nb_pkt_per_burst(uint16_t nb)
 {
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 0f72ca1..aa4bdac 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -1,7 +1,7 @@
 /*-
  *   BSD LICENSE
  *
- *   Copyright(c) 2010-2015 Intel Corporation. All rights reserved.
+ *   Copyright(c) 2010-2016 Intel Corporation. All rights reserved.
  *   All rights reserved.
  *
  *   Redistribution and use in source and binary forms, with or without
@@ -500,6 +500,7 @@ void set_fwd_lcores_number(uint16_t nb_lc);
 void set_fwd_ports_list(unsigned int *portlist, unsigned int nb_pt);
 void set_fwd_ports_mask(uint64_t portmask);
 void set_fwd_ports_number(uint16_t nb_pt);
+int port_is_forwarding(portid_t port_id);
 
 void rx_vlan_strip_set(portid_t port_id, int on);
 void rx_vlan_strip_set_on_queue(portid_t port_id, uint16_t queue_id, int on);
-- 
2.6.3

[PATCH v3 2/9] app/testpmd: don't update fwding config when attaching/detaching a port

[Bernard Iremonger]

Remove checks on test_done variable.
Remove code to update forwarding configuration.

Fixes: edab33b1c01d ("app/testpmd: support port hotplug")

Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 app/test-pmd/testpmd.c | 28 +---------------------------
 1 file changed, 1 insertion(+), 27 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index dd6b046..7a1e470 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -1517,7 +1517,7 @@ close_port(portid_t pid)
 void
 attach_port(char *identifier)
 {
-	portid_t i, j, pi = 0;
+	portid_t pi = 0;
 
 	printf("Attaching a new port...\n");
 
@@ -1526,11 +1526,6 @@ attach_port(char *identifier)
 		return;
 	}
 
-	if (test_done == 0) {
-		printf("Please stop forwarding first\n");
-		return;
-	}
-
 	if (rte_eth_dev_attach(identifier, &pi))
 		return;
 
@@ -1540,16 +1535,6 @@ attach_port(char *identifier)
 
 	nb_ports = rte_eth_dev_count();
 
-	/* set_default_fwd_ports_config(); */
-	memset(fwd_ports_ids, 0, sizeof(fwd_ports_ids));
-	i = 0;
-	FOREACH_PORT(j, ports) {
-		fwd_ports_ids[i] = j;
-		i++;
-	}
-	nb_cfg_ports = nb_ports;
-	nb_fwd_ports++;
-
 	ports[pi].port_status = RTE_PORT_STOPPED;
 
 	printf("Port %d is attached. Now total ports is %d\n", pi, nb_ports);
@@ -1559,7 +1544,6 @@ attach_port(char *identifier)
 void
 detach_port(uint8_t port_id)
 {
-	portid_t i, pi = 0;
 	char name[RTE_ETH_NAME_MAX_LEN];
 
 	printf("Detaching a port...\n");
@@ -1575,16 +1559,6 @@ detach_port(uint8_t port_id)
 	ports[port_id].enabled = 0;
 	nb_ports = rte_eth_dev_count();
 
-	/* set_default_fwd_ports_config(); */
-	memset(fwd_ports_ids, 0, sizeof(fwd_ports_ids));
-	i = 0;
-	FOREACH_PORT(pi, ports) {
-		fwd_ports_ids[i] = pi;
-		i++;
-	}
-	nb_cfg_ports = nb_ports;
-	nb_fwd_ports--;
-
 	printf("Port '%s' is detached. Now total ports is %d\n",
 			name, nb_ports);
 	printf("Done\n");
-- 
2.6.3

[PATCH v3 3/9] app/testpmd: check port is not forwarding in stop_port and close_port

[Bernard Iremonger]

Add calls to port_is_forwarding function.

Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 app/test-pmd/testpmd.c | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 7a1e470..f22d1b6 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -1279,11 +1279,6 @@ start_port(portid_t pid)
 	struct rte_port *port;
 	struct ether_addr mac_addr;
 
-	if (test_done == 0) {
-		printf("Please stop forwarding first\n");
-		return -1;
-	}
-
 	if (port_id_is_invalid(pid, ENABLED_WARN))
 		return 0;
 
@@ -1435,10 +1430,6 @@ stop_port(portid_t pid)
 	struct rte_port *port;
 	int need_check_link_status = 0;
 
-	if (test_done == 0) {
-		printf("Please stop forwarding first\n");
-		return;
-	}
 	if (dcb_test) {
 		dcb_test = 0;
 		dcb_config = 0;
@@ -1453,6 +1444,11 @@ stop_port(portid_t pid)
 		if (pid != pi && pid != (portid_t)RTE_PORT_ALL)
 			continue;
 
+		if (port_is_forwarding(pi) != 0 && test_done == 0) {
+			printf("Please remove port %d from forwarding configuration.\n", pi);
+			continue;
+		}
+
 		port = &ports[pi];
 		if (rte_atomic16_cmpset(&(port->port_status), RTE_PORT_STARTED,
 						RTE_PORT_HANDLING) == 0)
@@ -1477,11 +1473,6 @@ close_port(portid_t pid)
 	portid_t pi;
 	struct rte_port *port;
 
-	if (test_done == 0) {
-		printf("Please stop forwarding first\n");
-		return;
-	}
-
 	if (port_id_is_invalid(pid, ENABLED_WARN))
 		return;
 
@@ -1491,6 +1482,11 @@ close_port(portid_t pid)
 		if (pid != pi && pid != (portid_t)RTE_PORT_ALL)
 			continue;
 
+		if (port_is_forwarding(pi) != 0 && test_done == 0) {
+			printf("Please remove port %d from forwarding configuration.\n", pi);
+			continue;
+		}
+
 		port = &ports[pi];
 		if (rte_atomic16_cmpset(&(port->port_status),
 			RTE_PORT_CLOSED, RTE_PORT_CLOSED) == 1) {
-- 
2.6.3

[PATCH v3 4/9] app/testpmd: remove fwd_config_setup from fwd_config_display

[Bernard Iremonger]

Remove fwd_config_setup from fwd_config_display and check that
forwarding has been setup before displaying forwarding configuration.
Add call to fwd_config_setup for corelist, coremask, nbcore setup.
Add call to fwd_config_setup for portlist, portmask, nbport setup.

Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 app/test-pmd/cmdline.c | 23 ++++++++++++++++-------
 app/test-pmd/config.c  |  6 ++++--
 2 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index fd389ac..2c2a0c7 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -2520,16 +2520,20 @@ static void cmd_set_list_parsed(void *parsed_result,
 		nb_item = parse_item_list(res->list_of_items, "core",
 					  RTE_MAX_LCORE,
 					  parsed_items.lcorelist, 1);
-		if (nb_item > 0)
+		if (nb_item > 0) {
 			set_fwd_lcores_list(parsed_items.lcorelist, nb_item);
+			fwd_config_setup();
+		}
 		return;
 	}
 	if (!strcmp(res->list_name, "portlist")) {
 		nb_item = parse_item_list(res->list_of_items, "port",
 					  RTE_MAX_ETHPORTS,
 					  parsed_items.portlist, 1);
-		if (nb_item > 0)
+		if (nb_item > 0) {
 			set_fwd_ports_list(parsed_items.portlist, nb_item);
+			fwd_config_setup();
+		}
 	}
 }
 
@@ -2573,10 +2577,13 @@ static void cmd_set_mask_parsed(void *parsed_result,
 		printf("Please stop forwarding first\n");
 		return;
 	}
-	if (!strcmp(res->mask, "coremask"))
+	if (!strcmp(res->mask, "coremask")) {
 		set_fwd_lcores_mask(res->hexavalue);
-	else if (!strcmp(res->mask, "portmask"))
+		fwd_config_setup();
+	} else if (!strcmp(res->mask, "portmask")) {
 		set_fwd_ports_mask(res->hexavalue);
+		fwd_config_setup();
+	}
 }
 
 cmdline_parse_token_string_t cmd_setmask_set =
@@ -2613,11 +2620,13 @@ static void cmd_set_parsed(void *parsed_result,
 			   __attribute__((unused)) void *data)
 {
 	struct cmd_set_result *res = parsed_result;
-	if (!strcmp(res->what, "nbport"))
+	if (!strcmp(res->what, "nbport")) {
 		set_fwd_ports_number(res->value);
-	else if (!strcmp(res->what, "nbcore"))
+		fwd_config_setup();
+	} else if (!strcmp(res->what, "nbcore")) {
 		set_fwd_lcores_number(res->value);
-	else if (!strcmp(res->what, "burst"))
+		fwd_config_setup();
+	} else if (!strcmp(res->what, "burst"))
 		set_nb_pkt_per_burst(res->value);
 	else if (!strcmp(res->what, "verbose"))
 		set_verbose_level(res->value);
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index f434999..8ef9c85 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1424,8 +1424,10 @@ pkt_fwd_config_display(struct fwd_config *cfg)
 void
 fwd_config_display(void)
 {
-	fwd_config_setup();
-	pkt_fwd_config_display(&cur_fwd_config);
+	if (cur_fwd_config.nb_fwd_ports)
+		pkt_fwd_config_display(&cur_fwd_config);
+	else
+		printf("Please set portlist first\n");
 }
 
 int
-- 
2.6.3

Re: [PATCH v3 4/9] app/testpmd: remove fwd_config_setup from fwd_config_display

[De Lara Guarch, Pablo]

Hi Bernard,

> -----Original Message-----
> From: Iremonger, Bernard
> Sent: Sunday, June 12, 2016 4:23 PM
> To: dev@dpdk.org
> Cc: De Lara Guarch, Pablo; Iremonger, Bernard
> Subject: [PATCH v3 4/9] app/testpmd: remove fwd_config_setup from
> fwd_config_display
> 
> Remove fwd_config_setup from fwd_config_display and check that
> forwarding has been setup before displaying forwarding configuration.
> Add call to fwd_config_setup for corelist, coremask, nbcore setup.
> Add call to fwd_config_setup for portlist, portmask, nbport setup.

Probably we need to add that call to the commands that change number of RX/TX queues, as well.

> 
> Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
> ---
>  app/test-pmd/cmdline.c | 23 ++++++++++++++++-------
>  app/test-pmd/config.c  |  6 ++++--
>  2 files changed, 20 insertions(+), 9 deletions(-)
> 

 [...]

> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -1424,8 +1424,10 @@ pkt_fwd_config_display(struct fwd_config *cfg)
>  void
>  fwd_config_display(void)
>  {
> -	fwd_config_setup();
> -	pkt_fwd_config_display(&cur_fwd_config);
> +	if (cur_fwd_config.nb_fwd_ports)
> +		pkt_fwd_config_display(&cur_fwd_config);
> +	else
> +		printf("Please set portlist first\n");

I think this is not necessary. I would call fwd_config_setup() at startup,
so everything is setup when this is called (even if it is the first command when the prompt comes up).
I think at the end of init_config() is a good place.


>  }
> 
>  int
> --
> 2.6.3

Re: [PATCH v3 4/9] app/testpmd: remove fwd_config_setup from fwd_config_display

[Iremonger, Bernard]

Hi Pablo,

<snip>

> > Subject: [PATCH v3 4/9] app/testpmd: remove fwd_config_setup from
> > fwd_config_display
> >
> > Remove fwd_config_setup from fwd_config_display and check that
> > forwarding has been setup before displaying forwarding configuration.
> > Add call to fwd_config_setup for corelist, coremask, nbcore setup.
> > Add call to fwd_config_setup for portlist, portmask, nbport setup.
> 
> Probably we need to add that call to the commands that change number of
> RX/TX queues, as well.

I will check this.

> > Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
> > ---
> >  app/test-pmd/cmdline.c | 23 ++++++++++++++++-------
> > app/test-pmd/config.c  |  6 ++++--
> >  2 files changed, 20 insertions(+), 9 deletions(-)
> >
> 
>  [...]
> 
> > --- a/app/test-pmd/config.c
> > +++ b/app/test-pmd/config.c
> > @@ -1424,8 +1424,10 @@ pkt_fwd_config_display(struct fwd_config *cfg)
> > void
> >  fwd_config_display(void)
> >  {
> > -	fwd_config_setup();
> > -	pkt_fwd_config_display(&cur_fwd_config);
> > +	if (cur_fwd_config.nb_fwd_ports)
> > +		pkt_fwd_config_display(&cur_fwd_config);
> > +	else
> > +		printf("Please set portlist first\n");
> 
> I think this is not necessary. I would call fwd_config_setup() at startup, so
> everything is setup when this is called (even if it is the first command when
> the prompt comes up).
> I think at the end of init_config() is a good place.

Ok, I will try this. 

<snip>

Regards,

Bernard.

Re: [PATCH v3 4/9] app/testpmd: remove fwd_config_setup from fwd_config_display

[Iremonger, Bernard]

Hi Pablo,

 <snip>
> > > Subject: [PATCH v3 4/9] app/testpmd: remove fwd_config_setup from
> > > fwd_config_display
> > >
> > > Remove fwd_config_setup from fwd_config_display and check that
> > > forwarding has been setup before displaying forwarding configuration.
> > > Add call to fwd_config_setup for corelist, coremask, nbcore setup.
> > > Add call to fwd_config_setup for portlist, portmask, nbport setup.
> >
> > Probably we need to add that call to the commands that change number
> > of RX/TX queues, as well.
> 
> I will check this.

Should fwd_config_setup  be added for the  --rxd and --txd  options as well?
 
> > > Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
> > > ---
> > >  app/test-pmd/cmdline.c | 23 ++++++++++++++++-------
> > > app/test-pmd/config.c  |  6 ++++--
> > >  2 files changed, 20 insertions(+), 9 deletions(-)
> > >
> >
> >  [...]
> >
> > > --- a/app/test-pmd/config.c
> > > +++ b/app/test-pmd/config.c
> > > @@ -1424,8 +1424,10 @@ pkt_fwd_config_display(struct fwd_config
> > > *cfg) void
> > >  fwd_config_display(void)
> > >  {
> > > -	fwd_config_setup();
> > > -	pkt_fwd_config_display(&cur_fwd_config);
> > > +	if (cur_fwd_config.nb_fwd_ports)
> > > +		pkt_fwd_config_display(&cur_fwd_config);
> > > +	else
> > > +		printf("Please set portlist first\n");
> >
> > I think this is not necessary. I would call fwd_config_setup() at
> > startup, so everything is setup when this is called (even if it is the
> > first command when the prompt comes up).
> > I think at the end of init_config() is a good place.
> 
> Ok, I will try this.

Yes, this works.

<snip>

Regards,

Bernard.

[PATCH v3 5/9] app/testpmd: add function port_is_bonding_slave

[Bernard Iremonger]

Use this function in stop_port and close_port functions.

Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 app/test-pmd/testpmd.c | 18 ++++++++++++++++++
 app/test-pmd/testpmd.h |  2 ++
 2 files changed, 20 insertions(+)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index f22d1b6..6f68a18 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -1449,6 +1449,11 @@ stop_port(portid_t pid)
 			continue;
 		}
 
+		if (port_is_bonding_slave(pi)) {
+			printf("Please remove port %d from bonded device.\n", pi);
+			continue;
+		}
+
 		port = &ports[pi];
 		if (rte_atomic16_cmpset(&(port->port_status), RTE_PORT_STARTED,
 						RTE_PORT_HANDLING) == 0)
@@ -1487,6 +1492,11 @@ close_port(portid_t pid)
 			continue;
 		}
 
+		if (port_is_bonding_slave(pi)) {
+			printf("Please remove port %d from bonded device.\n", pi);
+			continue;
+		}
+
 		port = &ports[pi];
 		if (rte_atomic16_cmpset(&(port->port_status),
 			RTE_PORT_CLOSED, RTE_PORT_CLOSED) == 1) {
@@ -1824,6 +1834,14 @@ void clear_port_slave_flag(portid_t slave_pid)
 	port->slave_flag = 0;
 }
 
+uint8_t port_is_bonding_slave(portid_t slave_pid)
+{
+	struct rte_port *port;
+
+	port = &ports[slave_pid];
+	return port->slave_flag;
+}
+
 const uint16_t vlan_tags[] = {
 		0,  1,  2,  3,  4,  5,  6,  7,
 		8,  9, 10, 11,  12, 13, 14, 15,
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index aa4bdac..50f81d7 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -532,6 +532,8 @@ void dev_set_link_down(portid_t pid);
 void init_port_config(void);
 void set_port_slave_flag(portid_t slave_pid);
 void clear_port_slave_flag(portid_t slave_pid);
+uint8_t port_is_bonding_slave(portid_t slave_pid);
+
 int init_port_dcb_config(portid_t pid, enum dcb_mode_enable dcb_mode,
 		     enum rte_eth_nb_tcs num_tcs,
 		     uint8_t pfc_en);
-- 
2.6.3

[PATCH v3 6/9] app/testpmd: move call to init_fwd_streams

[Bernard Iremonger]

Move call to init_fwd_streams from start_port function
to start_packet_forwarding function.

Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 app/test-pmd/testpmd.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 6f68a18..c7ab8a0 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -992,6 +992,12 @@ start_packet_forwarding(int with_tx_first)
 		printf("Packet forwarding already started\n");
 		return;
 	}
+
+	if (init_fwd_streams() < 0) {
+		printf("Fail from init_fwd_streams()\n");
+		return;
+	}
+
 	if(dcb_test) {
 		for (i = 0; i < nb_fwd_ports; i++) {
 			pt_id = fwd_ports_ids[i];
@@ -1282,11 +1288,6 @@ start_port(portid_t pid)
 	if (port_id_is_invalid(pid, ENABLED_WARN))
 		return 0;
 
-	if (init_fwd_streams() < 0) {
-		printf("Fail from init_fwd_streams()\n");
-		return -1;
-	}
-
 	if(dcb_config)
 		dcb_test = 1;
 	FOREACH_PORT(pi, ports) {
-- 
2.6.3

[PATCH v3 7/9] app/testpmd: check for valid socket id when attaching port

[Bernard Iremonger]

Fixes: edab33b1c01d ("app/testpmd: support port hotplug")
Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 app/test-pmd/testpmd.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index c7ab8a0..991457d 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -1525,6 +1525,7 @@ void
 attach_port(char *identifier)
 {
 	portid_t pi = 0;
+	unsigned int socket_id;
 
 	printf("Attaching a new port...\n");
 
@@ -1537,7 +1538,11 @@ attach_port(char *identifier)
 		return;
 
 	ports[pi].enabled = 1;
-	reconfig(pi, rte_eth_dev_socket_id(pi));
+	socket_id = (unsigned)rte_eth_dev_socket_id(pi);
+	/* if socket_id is invalid, set to 0 */
+	if (check_socket_id(socket_id) < 0)
+		socket_id = 0;
+	reconfig(pi, socket_id);
 	rte_eth_promiscuous_enable(pi);
 
 	nb_ports = rte_eth_dev_count();
-- 
2.6.3

[PATCH v3 8/9] app/testpmd: check for valid mbuf pool

[Bernard Iremonger]

Fixes: b6ea6408fbc7 ("ethdev: store numa_node per device")
Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 app/test-pmd/testpmd.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 991457d..1e13c36 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -1358,7 +1358,7 @@ start_port(portid_t pid)
 					if (mp == NULL) {
 						printf("Failed to setup RX queue:"
 							"No mempool allocation"
-							"on the socket %d\n",
+							" on the socket %d\n",
 							rxring_numa[pi]);
 						return -1;
 					}
@@ -1366,17 +1366,23 @@ start_port(portid_t pid)
 					diag = rte_eth_rx_queue_setup(pi, qi,
 					     nb_rxd,rxring_numa[pi],
 					     &(port->rx_conf),mp);
-				}
-				else
+				} else {
+					struct rte_mempool *mp =
+						mbuf_pool_find(port->socket_id);
+					if (mp == NULL) {
+						printf("Failed to setup RX queue:"
+							"No mempool allocation"
+							" on the socket %d\n",
+							port->socket_id);
+						return -1;
+					}
 					diag = rte_eth_rx_queue_setup(pi, qi,
 					     nb_rxd,port->socket_id,
-					     &(port->rx_conf),
-				             mbuf_pool_find(port->socket_id));
-
+					     &(port->rx_conf), mp);
+				}
 				if (diag == 0)
 					continue;
 
-
 				/* Fail to setup rx queue, return */
 				if (rte_atomic16_cmpset(&(port->port_status),
 							RTE_PORT_HANDLING,
-- 
2.6.3

[PATCH v3 9/9] app/testpmd: stop forwarding on exit

[Bernard Iremonger]

Stop forwarding on exit whether all ports are started or not.

Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 app/test-pmd/testpmd.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 1e13c36..2fc17ef 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -1087,10 +1087,6 @@ stop_packet_forwarding(void)
 #endif
 	static const char *acc_stats_border = "+++++++++++++++";
 
-	if (all_ports_started() == 0) {
-		printf("Not all ports were started\n");
-		return;
-	}
 	if (test_done) {
 		printf("Packet forwarding not started\n");
 		return;
-- 
2.6.3

[PATCH v4 0/9] app/testpmd: forwarding

[Bernard Iremonger]

Modify testpmd to allow stop, close, detach and attach
of a port without stopping forwarding.

Changes in v4:
rebased to latest master
reworked patch 4

Changes in v3:
rebased to latest master.
added patch 9
  app/testpmd: stop forwarding on exit
reworked and renamed patch 4
  app/testpmd: remove fwd_config_setup from fwd_config_display

Changes in v2:
Added 4 more patches
  app/testpmd: add function port_is_bonding_slave
  app/testpmd: move call to init_fwd_streams
  app/testpmd: check for valid socket id when attaching port
  app/testpmd: check for valid mbuf pool

Bernard Iremonger (9):
  app/testpmd: add function port_is_forwarding
  app/testpmd: don't update fwding config when attaching/detaching a
    port
  app/testpmd: check port is not forwarding in stop_port and close_port
  app/testpmd: remove fwd_config_setup from fwd_config_display
  app/testpmd: add function port_is_bonding_slave
  app/testpmd: move call to init_fwd_streams
  app/testpmd: check for valid socket id when attaching port
  app/testpmd: check for valid mbuf pool
  app/testpmd: stop forwarding on exit

 app/test-pmd/cmdline.c |  25 ++++++++---
 app/test-pmd/config.c  |  19 ++++++++-
 app/test-pmd/testpmd.c | 114 ++++++++++++++++++++++++-------------------------
 app/test-pmd/testpmd.h |   5 ++-
 4 files changed, 95 insertions(+), 68 deletions(-)

-- 
2.6.3

[PATCH v4 1/9] app/testpmd: add function port_is_forwarding

[Bernard Iremonger]

Add function port_is_forwarding to check whether
a port is forwarding or not.

Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 app/test-pmd/config.c  | 18 +++++++++++++++++-
 app/test-pmd/testpmd.h |  3 ++-
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 1c552e4..f434999 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1,7 +1,7 @@
 /*-
  *   BSD LICENSE
  *
- *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
+ *   Copyright(c) 2010-2016 Intel Corporation. All rights reserved.
  *   All rights reserved.
  *
  *   Redistribution and use in source and binary forms, with or without
@@ -1565,6 +1565,22 @@ set_fwd_ports_number(uint16_t nb_pt)
 	       (unsigned int) nb_fwd_ports);
 }
 
+int
+port_is_forwarding(portid_t port_id)
+{
+	unsigned int i;
+
+	if (port_id_is_invalid(port_id, ENABLED_WARN))
+		return -1;
+
+	for (i = 0; i < nb_fwd_ports; i++) {
+		if (fwd_ports_ids[i] == port_id)
+			return 1;
+	}
+
+	return 0;
+}
+
 void
 set_nb_pkt_per_burst(uint16_t nb)
 {
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 0f72ca1..aa4bdac 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -1,7 +1,7 @@
 /*-
  *   BSD LICENSE
  *
- *   Copyright(c) 2010-2015 Intel Corporation. All rights reserved.
+ *   Copyright(c) 2010-2016 Intel Corporation. All rights reserved.
  *   All rights reserved.
  *
  *   Redistribution and use in source and binary forms, with or without
@@ -500,6 +500,7 @@ void set_fwd_lcores_number(uint16_t nb_lc);
 void set_fwd_ports_list(unsigned int *portlist, unsigned int nb_pt);
 void set_fwd_ports_mask(uint64_t portmask);
 void set_fwd_ports_number(uint16_t nb_pt);
+int port_is_forwarding(portid_t port_id);
 
 void rx_vlan_strip_set(portid_t port_id, int on);
 void rx_vlan_strip_set_on_queue(portid_t port_id, uint16_t queue_id, int on);
-- 
2.6.3

[PATCH v4 2/9] app/testpmd: don't update fwding config when attaching/detaching a port

[Bernard Iremonger]

Remove checks on test_done variable.
Remove code to update forwarding configuration.

Fixes: edab33b1c01d ("app/testpmd: support port hotplug")

Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 app/test-pmd/testpmd.c | 28 +---------------------------
 1 file changed, 1 insertion(+), 27 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index dd6b046..7a1e470 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -1517,7 +1517,7 @@ close_port(portid_t pid)
 void
 attach_port(char *identifier)
 {
-	portid_t i, j, pi = 0;
+	portid_t pi = 0;
 
 	printf("Attaching a new port...\n");
 
@@ -1526,11 +1526,6 @@ attach_port(char *identifier)
 		return;
 	}
 
-	if (test_done == 0) {
-		printf("Please stop forwarding first\n");
-		return;
-	}
-
 	if (rte_eth_dev_attach(identifier, &pi))
 		return;
 
@@ -1540,16 +1535,6 @@ attach_port(char *identifier)
 
 	nb_ports = rte_eth_dev_count();
 
-	/* set_default_fwd_ports_config(); */
-	memset(fwd_ports_ids, 0, sizeof(fwd_ports_ids));
-	i = 0;
-	FOREACH_PORT(j, ports) {
-		fwd_ports_ids[i] = j;
-		i++;
-	}
-	nb_cfg_ports = nb_ports;
-	nb_fwd_ports++;
-
 	ports[pi].port_status = RTE_PORT_STOPPED;
 
 	printf("Port %d is attached. Now total ports is %d\n", pi, nb_ports);
@@ -1559,7 +1544,6 @@ attach_port(char *identifier)
 void
 detach_port(uint8_t port_id)
 {
-	portid_t i, pi = 0;
 	char name[RTE_ETH_NAME_MAX_LEN];
 
 	printf("Detaching a port...\n");
@@ -1575,16 +1559,6 @@ detach_port(uint8_t port_id)
 	ports[port_id].enabled = 0;
 	nb_ports = rte_eth_dev_count();
 
-	/* set_default_fwd_ports_config(); */
-	memset(fwd_ports_ids, 0, sizeof(fwd_ports_ids));
-	i = 0;
-	FOREACH_PORT(pi, ports) {
-		fwd_ports_ids[i] = pi;
-		i++;
-	}
-	nb_cfg_ports = nb_ports;
-	nb_fwd_ports--;
-
 	printf("Port '%s' is detached. Now total ports is %d\n",
 			name, nb_ports);
 	printf("Done\n");
-- 
2.6.3

[PATCH v4 3/9] app/testpmd: check port is not forwarding in stop_port and close_port

[Bernard Iremonger]

Add calls to port_is_forwarding function.

Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 app/test-pmd/testpmd.c | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 7a1e470..f22d1b6 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -1279,11 +1279,6 @@ start_port(portid_t pid)
 	struct rte_port *port;
 	struct ether_addr mac_addr;
 
-	if (test_done == 0) {
-		printf("Please stop forwarding first\n");
-		return -1;
-	}
-
 	if (port_id_is_invalid(pid, ENABLED_WARN))
 		return 0;
 
@@ -1435,10 +1430,6 @@ stop_port(portid_t pid)
 	struct rte_port *port;
 	int need_check_link_status = 0;
 
-	if (test_done == 0) {
-		printf("Please stop forwarding first\n");
-		return;
-	}
 	if (dcb_test) {
 		dcb_test = 0;
 		dcb_config = 0;
@@ -1453,6 +1444,11 @@ stop_port(portid_t pid)
 		if (pid != pi && pid != (portid_t)RTE_PORT_ALL)
 			continue;
 
+		if (port_is_forwarding(pi) != 0 && test_done == 0) {
+			printf("Please remove port %d from forwarding configuration.\n", pi);
+			continue;
+		}
+
 		port = &ports[pi];
 		if (rte_atomic16_cmpset(&(port->port_status), RTE_PORT_STARTED,
 						RTE_PORT_HANDLING) == 0)
@@ -1477,11 +1473,6 @@ close_port(portid_t pid)
 	portid_t pi;
 	struct rte_port *port;
 
-	if (test_done == 0) {
-		printf("Please stop forwarding first\n");
-		return;
-	}
-
 	if (port_id_is_invalid(pid, ENABLED_WARN))
 		return;
 
@@ -1491,6 +1482,11 @@ close_port(portid_t pid)
 		if (pid != pi && pid != (portid_t)RTE_PORT_ALL)
 			continue;
 
+		if (port_is_forwarding(pi) != 0 && test_done == 0) {
+			printf("Please remove port %d from forwarding configuration.\n", pi);
+			continue;
+		}
+
 		port = &ports[pi];
 		if (rte_atomic16_cmpset(&(port->port_status),
 			RTE_PORT_CLOSED, RTE_PORT_CLOSED) == 1) {
-- 
2.6.3

[PATCH v4 4/9] app/testpmd: remove fwd_config_setup from fwd_config_display

[Bernard Iremonger]

Add call to fwd_config_setup to init_config.
Remove fwd_config_setup from fwd_config_display.
Add call to fwd_config_setup for corelist, coremask and nbcore setup.
Add call to fwd_config_setup for portlist, portmask and nbport setup.
Add call to fwd_config_setup for rxq, txq, rxd and txd setup.

Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 app/test-pmd/cmdline.c | 25 ++++++++++++++++++-------
 app/test-pmd/config.c  |  1 -
 app/test-pmd/testpmd.c |  2 ++
 3 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index fd389ac..44a56f0 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -1223,6 +1223,8 @@ cmd_config_rx_tx_parsed(void *parsed_result,
 		return;
 	}
 
+	fwd_config_setup();
+
 	init_port_config();
 
 	cmd_reconfig_device_queue(RTE_PORT_ALL, 1, 1);
@@ -2520,16 +2522,20 @@ static void cmd_set_list_parsed(void *parsed_result,
 		nb_item = parse_item_list(res->list_of_items, "core",
 					  RTE_MAX_LCORE,
 					  parsed_items.lcorelist, 1);
-		if (nb_item > 0)
+		if (nb_item > 0) {
 			set_fwd_lcores_list(parsed_items.lcorelist, nb_item);
+			fwd_config_setup();
+		}
 		return;
 	}
 	if (!strcmp(res->list_name, "portlist")) {
 		nb_item = parse_item_list(res->list_of_items, "port",
 					  RTE_MAX_ETHPORTS,
 					  parsed_items.portlist, 1);
-		if (nb_item > 0)
+		if (nb_item > 0) {
 			set_fwd_ports_list(parsed_items.portlist, nb_item);
+			fwd_config_setup();
+		}
 	}
 }
 
@@ -2573,10 +2579,13 @@ static void cmd_set_mask_parsed(void *parsed_result,
 		printf("Please stop forwarding first\n");
 		return;
 	}
-	if (!strcmp(res->mask, "coremask"))
+	if (!strcmp(res->mask, "coremask")) {
 		set_fwd_lcores_mask(res->hexavalue);
-	else if (!strcmp(res->mask, "portmask"))
+		fwd_config_setup();
+	} else if (!strcmp(res->mask, "portmask")) {
 		set_fwd_ports_mask(res->hexavalue);
+		fwd_config_setup();
+	}
 }
 
 cmdline_parse_token_string_t cmd_setmask_set =
@@ -2613,11 +2622,13 @@ static void cmd_set_parsed(void *parsed_result,
 			   __attribute__((unused)) void *data)
 {
 	struct cmd_set_result *res = parsed_result;
-	if (!strcmp(res->what, "nbport"))
+	if (!strcmp(res->what, "nbport")) {
 		set_fwd_ports_number(res->value);
-	else if (!strcmp(res->what, "nbcore"))
+		fwd_config_setup();
+	} else if (!strcmp(res->what, "nbcore")) {
 		set_fwd_lcores_number(res->value);
-	else if (!strcmp(res->what, "burst"))
+		fwd_config_setup();
+	} else if (!strcmp(res->what, "burst"))
 		set_nb_pkt_per_burst(res->value);
 	else if (!strcmp(res->what, "verbose"))
 		set_verbose_level(res->value);
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index f434999..47acbf6 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1424,7 +1424,6 @@ pkt_fwd_config_display(struct fwd_config *cfg)
 void
 fwd_config_display(void)
 {
-	fwd_config_setup();
 	pkt_fwd_config_display(&cur_fwd_config);
 }
 
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index f22d1b6..e8698cf 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -591,6 +591,8 @@ init_config(void)
 	/* Configuration of packet forwarding streams. */
 	if (init_fwd_streams() < 0)
 		rte_exit(EXIT_FAILURE, "FAIL from init_fwd_streams()\n");
+
+	fwd_config_setup();
 }
 
 
-- 
2.6.3

Re: [PATCH v4 4/9] app/testpmd: remove fwd_config_setup from fwd_config_display

[De Lara Guarch, Pablo]

Hi Bernard,

> -----Original Message-----
> From: Iremonger, Bernard
> Sent: Tuesday, June 14, 2016 1:28 PM
> To: dev@dpdk.org
> Cc: De Lara Guarch, Pablo; Iremonger, Bernard
> Subject: [PATCH v4 4/9] app/testpmd: remove fwd_config_setup from
> fwd_config_display
> 
> Add call to fwd_config_setup to init_config.
> Remove fwd_config_setup from fwd_config_display.
> Add call to fwd_config_setup for corelist, coremask and nbcore setup.
> Add call to fwd_config_setup for portlist, portmask and nbport setup.
> Add call to fwd_config_setup for rxq, txq, rxd and txd setup.
> 
> Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>

Apologies for misleading here, but I think we should drop this patch for the moment.
It is conflicting with another patch and it is not really necessary for your changes (am I right here?),
so I would say best thing to do is to drop it and discuss if we want this change separately.

Re: [PATCH v4 4/9] app/testpmd: remove fwd_config_setup from fwd_config_display

[Iremonger, Bernard]

Hi Pablo,

<snip>

> > Subject: [PATCH v4 4/9] app/testpmd: remove fwd_config_setup from
> > fwd_config_display
> >
> > Add call to fwd_config_setup to init_config.
> > Remove fwd_config_setup from fwd_config_display.
> > Add call to fwd_config_setup for corelist, coremask and nbcore setup.
> > Add call to fwd_config_setup for portlist, portmask and nbport setup.
> > Add call to fwd_config_setup for rxq, txq, rxd and txd setup.
> >
> > Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
> 
> Apologies for misleading here, but I think we should drop this patch for the
> moment.
> It is conflicting with another patch and it is not really necessary for your
> changes (am I right here?), so I would say best thing to do is to drop it and
> discuss if we want this change separately.

This patch is needed for the patchset and should not be dropped.

Is it conflicting with patch the following patch?

http://dpdk.org/dev/patchwork/patch/13132/

This patch is renaming fwd_config_display to fwd_config_setup_display.
It clarifies what the function is doing, but does not address the issue of separating the setup from the display.

Separating the setup from the display is resolved in my patch.

Regards,

Bernard.

Re: [PATCH v4 4/9] app/testpmd: remove fwd_config_setup from fwd_config_display

[De Lara Guarch, Pablo]

> -----Original Message-----
> From: Iremonger, Bernard
> Sent: Tuesday, June 14, 2016 3:14 PM
> To: De Lara Guarch, Pablo; dev@dpdk.org
> Subject: RE: [PATCH v4 4/9] app/testpmd: remove fwd_config_setup from
> fwd_config_display
> 
> Hi Pablo,
> 
> <snip>
> 
> > > Subject: [PATCH v4 4/9] app/testpmd: remove fwd_config_setup from
> > > fwd_config_display
> > >
> > > Add call to fwd_config_setup to init_config.
> > > Remove fwd_config_setup from fwd_config_display.
> > > Add call to fwd_config_setup for corelist, coremask and nbcore setup.
> > > Add call to fwd_config_setup for portlist, portmask and nbport setup.
> > > Add call to fwd_config_setup for rxq, txq, rxd and txd setup.
> > >
> > > Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
> >
> > Apologies for misleading here, but I think we should drop this patch for the
> > moment.
> > It is conflicting with another patch and it is not really necessary for your
> > changes (am I right here?), so I would say best thing to do is to drop it and
> > discuss if we want this change separately.
> 
> This patch is needed for the patchset and should not be dropped.
> 
> Is it conflicting with patch the following patch?
> 
> http://dpdk.org/dev/patchwork/patch/13132/
> 
> This patch is renaming fwd_config_display to fwd_config_setup_display.
> It clarifies what the function is doing, but does not address the issue of
> separating the setup from the display.
> 
> Separating the setup from the display is resolved in my patch.

Right, I thought this was only refactoring. Sorry about that!
Anyway, since fwd_config_display() is going to call only pkt_fwd_config_display() now,
it makes sense to move the code from that function to fwd_config_display().
You can send a separate patch for that, as it is only refactoring.

Thanks,
Pablo

> 
> Regards,
> 
> Bernard.
> 
> 

[PATCH v4 5/9] app/testpmd: add function port_is_bonding_slave

[Bernard Iremonger]

Use this function in stop_port and close_port functions.

Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 app/test-pmd/testpmd.c | 18 ++++++++++++++++++
 app/test-pmd/testpmd.h |  2 ++
 2 files changed, 20 insertions(+)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index e8698cf..5afce27 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -1451,6 +1451,11 @@ stop_port(portid_t pid)
 			continue;
 		}
 
+		if (port_is_bonding_slave(pi)) {
+			printf("Please remove port %d from bonded device.\n", pi);
+			continue;
+		}
+
 		port = &ports[pi];
 		if (rte_atomic16_cmpset(&(port->port_status), RTE_PORT_STARTED,
 						RTE_PORT_HANDLING) == 0)
@@ -1489,6 +1494,11 @@ close_port(portid_t pid)
 			continue;
 		}
 
+		if (port_is_bonding_slave(pi)) {
+			printf("Please remove port %d from bonded device.\n", pi);
+			continue;
+		}
+
 		port = &ports[pi];
 		if (rte_atomic16_cmpset(&(port->port_status),
 			RTE_PORT_CLOSED, RTE_PORT_CLOSED) == 1) {
@@ -1826,6 +1836,14 @@ void clear_port_slave_flag(portid_t slave_pid)
 	port->slave_flag = 0;
 }
 
+uint8_t port_is_bonding_slave(portid_t slave_pid)
+{
+	struct rte_port *port;
+
+	port = &ports[slave_pid];
+	return port->slave_flag;
+}
+
 const uint16_t vlan_tags[] = {
 		0,  1,  2,  3,  4,  5,  6,  7,
 		8,  9, 10, 11,  12, 13, 14, 15,
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index aa4bdac..50f81d7 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -532,6 +532,8 @@ void dev_set_link_down(portid_t pid);
 void init_port_config(void);
 void set_port_slave_flag(portid_t slave_pid);
 void clear_port_slave_flag(portid_t slave_pid);
+uint8_t port_is_bonding_slave(portid_t slave_pid);
+
 int init_port_dcb_config(portid_t pid, enum dcb_mode_enable dcb_mode,
 		     enum rte_eth_nb_tcs num_tcs,
 		     uint8_t pfc_en);
-- 
2.6.3

[PATCH v4 6/9] app/testpmd: move call to init_fwd_streams

[Bernard Iremonger]

Move call to init_fwd_streams from start_port function
to start_packet_forwarding function.

Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 app/test-pmd/testpmd.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 5afce27..38cddca 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -994,6 +994,12 @@ start_packet_forwarding(int with_tx_first)
 		printf("Packet forwarding already started\n");
 		return;
 	}
+
+	if (init_fwd_streams() < 0) {
+		printf("Fail from init_fwd_streams()\n");
+		return;
+	}
+
 	if(dcb_test) {
 		for (i = 0; i < nb_fwd_ports; i++) {
 			pt_id = fwd_ports_ids[i];
@@ -1284,11 +1290,6 @@ start_port(portid_t pid)
 	if (port_id_is_invalid(pid, ENABLED_WARN))
 		return 0;
 
-	if (init_fwd_streams() < 0) {
-		printf("Fail from init_fwd_streams()\n");
-		return -1;
-	}
-
 	if(dcb_config)
 		dcb_test = 1;
 	FOREACH_PORT(pi, ports) {
-- 
2.6.3

[PATCH v4 7/9] app/testpmd: check for valid socket id when attaching port

[Bernard Iremonger]

Fixes: edab33b1c01d ("app/testpmd: support port hotplug")
Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 app/test-pmd/testpmd.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 38cddca..fa6434c 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -1527,6 +1527,7 @@ void
 attach_port(char *identifier)
 {
 	portid_t pi = 0;
+	unsigned int socket_id;
 
 	printf("Attaching a new port...\n");
 
@@ -1539,7 +1540,11 @@ attach_port(char *identifier)
 		return;
 
 	ports[pi].enabled = 1;
-	reconfig(pi, rte_eth_dev_socket_id(pi));
+	socket_id = (unsigned)rte_eth_dev_socket_id(pi);
+	/* if socket_id is invalid, set to 0 */
+	if (check_socket_id(socket_id) < 0)
+		socket_id = 0;
+	reconfig(pi, socket_id);
 	rte_eth_promiscuous_enable(pi);
 
 	nb_ports = rte_eth_dev_count();
-- 
2.6.3

[PATCH v4 8/9] app/testpmd: check for valid mbuf pool

[Bernard Iremonger]

Fixes: b6ea6408fbc7 ("ethdev: store numa_node per device")
Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 app/test-pmd/testpmd.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index fa6434c..29947fd 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -1360,7 +1360,7 @@ start_port(portid_t pid)
 					if (mp == NULL) {
 						printf("Failed to setup RX queue:"
 							"No mempool allocation"
-							"on the socket %d\n",
+							" on the socket %d\n",
 							rxring_numa[pi]);
 						return -1;
 					}
@@ -1368,17 +1368,23 @@ start_port(portid_t pid)
 					diag = rte_eth_rx_queue_setup(pi, qi,
 					     nb_rxd,rxring_numa[pi],
 					     &(port->rx_conf),mp);
-				}
-				else
+				} else {
+					struct rte_mempool *mp =
+						mbuf_pool_find(port->socket_id);
+					if (mp == NULL) {
+						printf("Failed to setup RX queue:"
+							"No mempool allocation"
+							" on the socket %d\n",
+							port->socket_id);
+						return -1;
+					}
 					diag = rte_eth_rx_queue_setup(pi, qi,
 					     nb_rxd,port->socket_id,
-					     &(port->rx_conf),
-				             mbuf_pool_find(port->socket_id));
-
+					     &(port->rx_conf), mp);
+				}
 				if (diag == 0)
 					continue;
 
-
 				/* Fail to setup rx queue, return */
 				if (rte_atomic16_cmpset(&(port->port_status),
 							RTE_PORT_HANDLING,
-- 
2.6.3

[PATCH v4 9/9] app/testpmd: stop forwarding on exit

[Bernard Iremonger]

Stop forwarding on exit whether all ports are started or not.

Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 app/test-pmd/testpmd.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 29947fd..60be416 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -1089,10 +1089,6 @@ stop_packet_forwarding(void)
 #endif
 	static const char *acc_stats_border = "+++++++++++++++";
 
-	if (all_ports_started() == 0) {
-		printf("Not all ports were started\n");
-		return;
-	}
 	if (test_done) {
 		printf("Packet forwarding not started\n");
 		return;
-- 
2.6.3

[PATCH v5 0/9] app/testpmd: forwarding

[Bernard Iremonger]

Modify testpmd to allow stop, close, detach and attach
of a port without stopping forwarding.

Changes in v5:
rebased to latest master
reworded commit messages, resulting in changes to patch names.

Changes in v4:
rebased to latest master
reworked patch 4

Changes in v3:
rebased to latest master.
added patch 9
  app/testpmd: stop forwarding on exit
reworked and renamed patch 4
  app/testpmd: remove fwd_config_setup from fwd_config_display

Changes in v2:
Added 4 more patches
  app/testpmd: add function port_is_bonding_slave
  app/testpmd: move call to init_fwd_streams
  app/testpmd: check for valid socket id when attaching port
  app/testpmd: check for valid mbuf pool

Bernard Iremonger (9):
  app/testpmd: add new function
  app/testpmd: do not update forwarding config
  app/testpmd: check port is not forwarding
  app/testpmd: separate fwd config setup from display
  app/testpmd: add another new function
  app/testpmd: move fwd streams initialisation
  app/testpmd: check for valid socket id when attaching port
  app/testpmd: check for valid mbuf pool
  app/testpmd: stop forwarding on exit

 app/test-pmd/cmdline.c |  25 ++++++++---
 app/test-pmd/config.c  |  19 ++++++++-
 app/test-pmd/testpmd.c | 114 ++++++++++++++++++++++++-------------------------
 app/test-pmd/testpmd.h |   5 ++-
 4 files changed, 95 insertions(+), 68 deletions(-)

-- 
2.6.3

[PATCH v5 1/9] app/testpmd: add new function

[Bernard Iremonger]

Add function port_is_forwarding to check whether
a port is forwarding or not.

Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 app/test-pmd/config.c  | 18 +++++++++++++++++-
 app/test-pmd/testpmd.h |  3 ++-
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 1c552e4..f434999 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1,7 +1,7 @@
 /*-
  *   BSD LICENSE
  *
- *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
+ *   Copyright(c) 2010-2016 Intel Corporation. All rights reserved.
  *   All rights reserved.
  *
  *   Redistribution and use in source and binary forms, with or without
@@ -1565,6 +1565,22 @@ set_fwd_ports_number(uint16_t nb_pt)
 	       (unsigned int) nb_fwd_ports);
 }
 
+int
+port_is_forwarding(portid_t port_id)
+{
+	unsigned int i;
+
+	if (port_id_is_invalid(port_id, ENABLED_WARN))
+		return -1;
+
+	for (i = 0; i < nb_fwd_ports; i++) {
+		if (fwd_ports_ids[i] == port_id)
+			return 1;
+	}
+
+	return 0;
+}
+
 void
 set_nb_pkt_per_burst(uint16_t nb)
 {
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 0f72ca1..aa4bdac 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -1,7 +1,7 @@
 /*-
  *   BSD LICENSE
  *
- *   Copyright(c) 2010-2015 Intel Corporation. All rights reserved.
+ *   Copyright(c) 2010-2016 Intel Corporation. All rights reserved.
  *   All rights reserved.
  *
  *   Redistribution and use in source and binary forms, with or without
@@ -500,6 +500,7 @@ void set_fwd_lcores_number(uint16_t nb_lc);
 void set_fwd_ports_list(unsigned int *portlist, unsigned int nb_pt);
 void set_fwd_ports_mask(uint64_t portmask);
 void set_fwd_ports_number(uint16_t nb_pt);
+int port_is_forwarding(portid_t port_id);
 
 void rx_vlan_strip_set(portid_t port_id, int on);
 void rx_vlan_strip_set_on_queue(portid_t port_id, uint16_t queue_id, int on);
-- 
2.6.3

[PATCH v5 2/9] app/testpmd: do not update forwarding config

[Bernard Iremonger]

Do not update forwarding configuration when attaching or detaching a port
Remove checks on test_done variable.
Remove code to update forwarding configuration.

Fixes: edab33b1c01d ("app/testpmd: support port hotplug")

Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 app/test-pmd/testpmd.c | 28 +---------------------------
 1 file changed, 1 insertion(+), 27 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index dd6b046..7a1e470 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -1517,7 +1517,7 @@ close_port(portid_t pid)
 void
 attach_port(char *identifier)
 {
-	portid_t i, j, pi = 0;
+	portid_t pi = 0;
 
 	printf("Attaching a new port...\n");
 
@@ -1526,11 +1526,6 @@ attach_port(char *identifier)
 		return;
 	}
 
-	if (test_done == 0) {
-		printf("Please stop forwarding first\n");
-		return;
-	}
-
 	if (rte_eth_dev_attach(identifier, &pi))
 		return;
 
@@ -1540,16 +1535,6 @@ attach_port(char *identifier)
 
 	nb_ports = rte_eth_dev_count();
 
-	/* set_default_fwd_ports_config(); */
-	memset(fwd_ports_ids, 0, sizeof(fwd_ports_ids));
-	i = 0;
-	FOREACH_PORT(j, ports) {
-		fwd_ports_ids[i] = j;
-		i++;
-	}
-	nb_cfg_ports = nb_ports;
-	nb_fwd_ports++;
-
 	ports[pi].port_status = RTE_PORT_STOPPED;
 
 	printf("Port %d is attached. Now total ports is %d\n", pi, nb_ports);
@@ -1559,7 +1544,6 @@ attach_port(char *identifier)
 void
 detach_port(uint8_t port_id)
 {
-	portid_t i, pi = 0;
 	char name[RTE_ETH_NAME_MAX_LEN];
 
 	printf("Detaching a port...\n");
@@ -1575,16 +1559,6 @@ detach_port(uint8_t port_id)
 	ports[port_id].enabled = 0;
 	nb_ports = rte_eth_dev_count();
 
-	/* set_default_fwd_ports_config(); */
-	memset(fwd_ports_ids, 0, sizeof(fwd_ports_ids));
-	i = 0;
-	FOREACH_PORT(pi, ports) {
-		fwd_ports_ids[i] = pi;
-		i++;
-	}
-	nb_cfg_ports = nb_ports;
-	nb_fwd_ports--;
-
 	printf("Port '%s' is detached. Now total ports is %d\n",
 			name, nb_ports);
 	printf("Done\n");
-- 
2.6.3

[PATCH v5 3/9] app/testpmd: check port is not forwarding

[Bernard Iremonger]

Add calls to port_is_forwarding function in stop_port and
close_port functions to check that port is not forwarding.

Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 app/test-pmd/testpmd.c | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 7a1e470..f22d1b6 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -1279,11 +1279,6 @@ start_port(portid_t pid)
 	struct rte_port *port;
 	struct ether_addr mac_addr;
 
-	if (test_done == 0) {
-		printf("Please stop forwarding first\n");
-		return -1;
-	}
-
 	if (port_id_is_invalid(pid, ENABLED_WARN))
 		return 0;
 
@@ -1435,10 +1430,6 @@ stop_port(portid_t pid)
 	struct rte_port *port;
 	int need_check_link_status = 0;
 
-	if (test_done == 0) {
-		printf("Please stop forwarding first\n");
-		return;
-	}
 	if (dcb_test) {
 		dcb_test = 0;
 		dcb_config = 0;
@@ -1453,6 +1444,11 @@ stop_port(portid_t pid)
 		if (pid != pi && pid != (portid_t)RTE_PORT_ALL)
 			continue;
 
+		if (port_is_forwarding(pi) != 0 && test_done == 0) {
+			printf("Please remove port %d from forwarding configuration.\n", pi);
+			continue;
+		}
+
 		port = &ports[pi];
 		if (rte_atomic16_cmpset(&(port->port_status), RTE_PORT_STARTED,
 						RTE_PORT_HANDLING) == 0)
@@ -1477,11 +1473,6 @@ close_port(portid_t pid)
 	portid_t pi;
 	struct rte_port *port;
 
-	if (test_done == 0) {
-		printf("Please stop forwarding first\n");
-		return;
-	}
-
 	if (port_id_is_invalid(pid, ENABLED_WARN))
 		return;
 
@@ -1491,6 +1482,11 @@ close_port(portid_t pid)
 		if (pid != pi && pid != (portid_t)RTE_PORT_ALL)
 			continue;
 
+		if (port_is_forwarding(pi) != 0 && test_done == 0) {
+			printf("Please remove port %d from forwarding configuration.\n", pi);
+			continue;
+		}
+
 		port = &ports[pi];
 		if (rte_atomic16_cmpset(&(port->port_status),
 			RTE_PORT_CLOSED, RTE_PORT_CLOSED) == 1) {
-- 
2.6.3

[PATCH v5 4/9] app/testpmd: separate fwd config setup from display

[Bernard Iremonger]

Add call to fwd_config_setup to init_config.
Remove fwd_config_setup from fwd_config_display.
Add call to fwd_config_setup for corelist, coremask and nbcore setup.
Add call to fwd_config_setup for portlist, portmask and nbport setup.
Add call to fwd_config_setup for rxq, txq, rxd and txd setup.

Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 app/test-pmd/cmdline.c | 25 ++++++++++++++++++-------
 app/test-pmd/config.c  |  1 -
 app/test-pmd/testpmd.c |  2 ++
 3 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index fd389ac..44a56f0 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -1223,6 +1223,8 @@ cmd_config_rx_tx_parsed(void *parsed_result,
 		return;
 	}
 
+	fwd_config_setup();
+
 	init_port_config();
 
 	cmd_reconfig_device_queue(RTE_PORT_ALL, 1, 1);
@@ -2520,16 +2522,20 @@ static void cmd_set_list_parsed(void *parsed_result,
 		nb_item = parse_item_list(res->list_of_items, "core",
 					  RTE_MAX_LCORE,
 					  parsed_items.lcorelist, 1);
-		if (nb_item > 0)
+		if (nb_item > 0) {
 			set_fwd_lcores_list(parsed_items.lcorelist, nb_item);
+			fwd_config_setup();
+		}
 		return;
 	}
 	if (!strcmp(res->list_name, "portlist")) {
 		nb_item = parse_item_list(res->list_of_items, "port",
 					  RTE_MAX_ETHPORTS,
 					  parsed_items.portlist, 1);
-		if (nb_item > 0)
+		if (nb_item > 0) {
 			set_fwd_ports_list(parsed_items.portlist, nb_item);
+			fwd_config_setup();
+		}
 	}
 }
 
@@ -2573,10 +2579,13 @@ static void cmd_set_mask_parsed(void *parsed_result,
 		printf("Please stop forwarding first\n");
 		return;
 	}
-	if (!strcmp(res->mask, "coremask"))
+	if (!strcmp(res->mask, "coremask")) {
 		set_fwd_lcores_mask(res->hexavalue);
-	else if (!strcmp(res->mask, "portmask"))
+		fwd_config_setup();
+	} else if (!strcmp(res->mask, "portmask")) {
 		set_fwd_ports_mask(res->hexavalue);
+		fwd_config_setup();
+	}
 }
 
 cmdline_parse_token_string_t cmd_setmask_set =
@@ -2613,11 +2622,13 @@ static void cmd_set_parsed(void *parsed_result,
 			   __attribute__((unused)) void *data)
 {
 	struct cmd_set_result *res = parsed_result;
-	if (!strcmp(res->what, "nbport"))
+	if (!strcmp(res->what, "nbport")) {
 		set_fwd_ports_number(res->value);
-	else if (!strcmp(res->what, "nbcore"))
+		fwd_config_setup();
+	} else if (!strcmp(res->what, "nbcore")) {
 		set_fwd_lcores_number(res->value);
-	else if (!strcmp(res->what, "burst"))
+		fwd_config_setup();
+	} else if (!strcmp(res->what, "burst"))
 		set_nb_pkt_per_burst(res->value);
 	else if (!strcmp(res->what, "verbose"))
 		set_verbose_level(res->value);
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index f434999..47acbf6 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1424,7 +1424,6 @@ pkt_fwd_config_display(struct fwd_config *cfg)
 void
 fwd_config_display(void)
 {
-	fwd_config_setup();
 	pkt_fwd_config_display(&cur_fwd_config);
 }
 
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index f22d1b6..e8698cf 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -591,6 +591,8 @@ init_config(void)
 	/* Configuration of packet forwarding streams. */
 	if (init_fwd_streams() < 0)
 		rte_exit(EXIT_FAILURE, "FAIL from init_fwd_streams()\n");
+
+	fwd_config_setup();
 }
 
 
-- 
2.6.3

[PATCH v5 5/9] app/testpmd: add another new function

[Bernard Iremonger]

Add new function port_is_bonding_slave
Use this function in stop_port and close_port functions.

Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 app/test-pmd/testpmd.c | 18 ++++++++++++++++++
 app/test-pmd/testpmd.h |  2 ++
 2 files changed, 20 insertions(+)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index e8698cf..5afce27 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -1451,6 +1451,11 @@ stop_port(portid_t pid)
 			continue;
 		}
 
+		if (port_is_bonding_slave(pi)) {
+			printf("Please remove port %d from bonded device.\n", pi);
+			continue;
+		}
+
 		port = &ports[pi];
 		if (rte_atomic16_cmpset(&(port->port_status), RTE_PORT_STARTED,
 						RTE_PORT_HANDLING) == 0)
@@ -1489,6 +1494,11 @@ close_port(portid_t pid)
 			continue;
 		}
 
+		if (port_is_bonding_slave(pi)) {
+			printf("Please remove port %d from bonded device.\n", pi);
+			continue;
+		}
+
 		port = &ports[pi];
 		if (rte_atomic16_cmpset(&(port->port_status),
 			RTE_PORT_CLOSED, RTE_PORT_CLOSED) == 1) {
@@ -1826,6 +1836,14 @@ void clear_port_slave_flag(portid_t slave_pid)
 	port->slave_flag = 0;
 }
 
+uint8_t port_is_bonding_slave(portid_t slave_pid)
+{
+	struct rte_port *port;
+
+	port = &ports[slave_pid];
+	return port->slave_flag;
+}
+
 const uint16_t vlan_tags[] = {
 		0,  1,  2,  3,  4,  5,  6,  7,
 		8,  9, 10, 11,  12, 13, 14, 15,
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index aa4bdac..50f81d7 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -532,6 +532,8 @@ void dev_set_link_down(portid_t pid);
 void init_port_config(void);
 void set_port_slave_flag(portid_t slave_pid);
 void clear_port_slave_flag(portid_t slave_pid);
+uint8_t port_is_bonding_slave(portid_t slave_pid);
+
 int init_port_dcb_config(portid_t pid, enum dcb_mode_enable dcb_mode,
 		     enum rte_eth_nb_tcs num_tcs,
 		     uint8_t pfc_en);
-- 
2.6.3

[PATCH v5 6/9] app/testpmd: move fwd streams initialisation

[Bernard Iremonger]

Move call to init_fwd_streams from start_port function
to start_packet_forwarding function.

Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 app/test-pmd/testpmd.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 5afce27..38cddca 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -994,6 +994,12 @@ start_packet_forwarding(int with_tx_first)
 		printf("Packet forwarding already started\n");
 		return;
 	}
+
+	if (init_fwd_streams() < 0) {
+		printf("Fail from init_fwd_streams()\n");
+		return;
+	}
+
 	if(dcb_test) {
 		for (i = 0; i < nb_fwd_ports; i++) {
 			pt_id = fwd_ports_ids[i];
@@ -1284,11 +1290,6 @@ start_port(portid_t pid)
 	if (port_id_is_invalid(pid, ENABLED_WARN))
 		return 0;
 
-	if (init_fwd_streams() < 0) {
-		printf("Fail from init_fwd_streams()\n");
-		return -1;
-	}
-
 	if(dcb_config)
 		dcb_test = 1;
 	FOREACH_PORT(pi, ports) {
-- 
2.6.3

[PATCH v5 7/9] app/testpmd: check for valid socket id when attaching port

[Bernard Iremonger]

Fixes: edab33b1c01d ("app/testpmd: support port hotplug")
Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 app/test-pmd/testpmd.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 38cddca..fa6434c 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -1527,6 +1527,7 @@ void
 attach_port(char *identifier)
 {
 	portid_t pi = 0;
+	unsigned int socket_id;
 
 	printf("Attaching a new port...\n");
 
@@ -1539,7 +1540,11 @@ attach_port(char *identifier)
 		return;
 
 	ports[pi].enabled = 1;
-	reconfig(pi, rte_eth_dev_socket_id(pi));
+	socket_id = (unsigned)rte_eth_dev_socket_id(pi);
+	/* if socket_id is invalid, set to 0 */
+	if (check_socket_id(socket_id) < 0)
+		socket_id = 0;
+	reconfig(pi, socket_id);
 	rte_eth_promiscuous_enable(pi);
 
 	nb_ports = rte_eth_dev_count();
-- 
2.6.3

[PATCH v5 8/9] app/testpmd: check for valid mbuf pool

[Bernard Iremonger]

Fixes: b6ea6408fbc7 ("ethdev: store numa_node per device")
Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 app/test-pmd/testpmd.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index fa6434c..29947fd 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -1360,7 +1360,7 @@ start_port(portid_t pid)
 					if (mp == NULL) {
 						printf("Failed to setup RX queue:"
 							"No mempool allocation"
-							"on the socket %d\n",
+							" on the socket %d\n",
 							rxring_numa[pi]);
 						return -1;
 					}
@@ -1368,17 +1368,23 @@ start_port(portid_t pid)
 					diag = rte_eth_rx_queue_setup(pi, qi,
 					     nb_rxd,rxring_numa[pi],
 					     &(port->rx_conf),mp);
-				}
-				else
+				} else {
+					struct rte_mempool *mp =
+						mbuf_pool_find(port->socket_id);
+					if (mp == NULL) {
+						printf("Failed to setup RX queue:"
+							"No mempool allocation"
+							" on the socket %d\n",
+							port->socket_id);
+						return -1;
+					}
 					diag = rte_eth_rx_queue_setup(pi, qi,
 					     nb_rxd,port->socket_id,
-					     &(port->rx_conf),
-				             mbuf_pool_find(port->socket_id));
-
+					     &(port->rx_conf), mp);
+				}
 				if (diag == 0)
 					continue;
 
-
 				/* Fail to setup rx queue, return */
 				if (rte_atomic16_cmpset(&(port->port_status),
 							RTE_PORT_HANDLING,
-- 
2.6.3

[PATCH v5 9/9] app/testpmd: stop forwarding on exit

[Bernard Iremonger]

Stop forwarding on exit whether all ports are started or not.

Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 app/test-pmd/testpmd.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 29947fd..60be416 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -1089,10 +1089,6 @@ stop_packet_forwarding(void)
 #endif
 	static const char *acc_stats_border = "+++++++++++++++";
 
-	if (all_ports_started() == 0) {
-		printf("Not all ports were started\n");
-		return;
-	}
 	if (test_done) {
 		printf("Packet forwarding not started\n");
 		return;
-- 
2.6.3

Re: [PATCH v5 0/9] app/testpmd: forwarding

[De Lara Guarch, Pablo]

> -----Original Message-----
> From: Iremonger, Bernard
> Sent: Tuesday, June 14, 2016 4:36 PM
> To: dev@dpdk.org
> Cc: De Lara Guarch, Pablo; Iremonger, Bernard
> Subject: [PATCH v5 0/9] app/testpmd: forwarding
> 
> Modify testpmd to allow stop, close, detach and attach
> of a port without stopping forwarding.
> 
> Changes in v5:
> rebased to latest master
> reworded commit messages, resulting in changes to patch names.
> 
> Changes in v4:
> rebased to latest master
> reworked patch 4
> 
> Changes in v3:
> rebased to latest master.
> added patch 9
>   app/testpmd: stop forwarding on exit
> reworked and renamed patch 4
>   app/testpmd: remove fwd_config_setup from fwd_config_display
> 
> Changes in v2:
> Added 4 more patches
>   app/testpmd: add function port_is_bonding_slave
>   app/testpmd: move call to init_fwd_streams
>   app/testpmd: check for valid socket id when attaching port
>   app/testpmd: check for valid mbuf pool
> 
> Bernard Iremonger (9):
>   app/testpmd: add new function
>   app/testpmd: do not update forwarding config
>   app/testpmd: check port is not forwarding
>   app/testpmd: separate fwd config setup from display
>   app/testpmd: add another new function
>   app/testpmd: move fwd streams initialisation
>   app/testpmd: check for valid socket id when attaching port
>   app/testpmd: check for valid mbuf pool
>   app/testpmd: stop forwarding on exit
> 
>  app/test-pmd/cmdline.c |  25 ++++++++---
>  app/test-pmd/config.c  |  19 ++++++++-
>  app/test-pmd/testpmd.c | 114 ++++++++++++++++++++++++---------------------
> ----
>  app/test-pmd/testpmd.h |   5 ++-
>  4 files changed, 95 insertions(+), 68 deletions(-)
> 
> --
> 2.6.3

Series-acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>

Thanks for your work!

Re: [PATCH v5 0/9] app/testpmd: forwarding

[Thomas Monjalon]

> > Bernard Iremonger (9):
> >   app/testpmd: add new function
> >   app/testpmd: do not update forwarding config
> >   app/testpmd: check port is not forwarding
> >   app/testpmd: separate fwd config setup from display
> >   app/testpmd: add another new function
> >   app/testpmd: move fwd streams initialisation
> >   app/testpmd: check for valid socket id when attaching port
> >   app/testpmd: check for valid mbuf pool
> >   app/testpmd: stop forwarding on exit
> 
> Series-acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> 
> Thanks for your work!

Applied with a bit of squashing, thanks