[dpdk-dev] [PATCH] app/testpmd: change port detach interface

[dpdk-dev] [PATCH] app/testpmd: change port detach interface

[Nithin Dabilpuram]

With the latest published interface of
rte_eal_hotplug_[add,remove](), and rte_eth_dev_close(),
rte_eth_dev_close() would cleanup all the data structures of
port's eth dev leaving the device common resource intact
if RTE_ETH_DEV_CLOSE_REMOVE is set in dev flags.
So "port detach" (~hotplug remove) should be able to work,
with device identifier like "port attach" as eth_dev could have
been closed already and rte_eth_devices[port_id] reused.

This change alters "port detach" cmdline interface to
work with device identifier like "port attach".

Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
---
 app/test-pmd/cmdline.c | 15 ++++++++-------
 app/test-pmd/testpmd.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
 app/test-pmd/testpmd.h |  1 +
 3 files changed, 54 insertions(+), 7 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index c1042dd..c11b9d2 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -1456,7 +1456,7 @@ cmdline_parse_inst_t cmd_operate_attach_port = {
 struct cmd_operate_detach_port_result {
 	cmdline_fixed_string_t port;
 	cmdline_fixed_string_t keyword;
-	portid_t port_id;
+	cmdline_fixed_string_t identifier;
 };
 
 static void cmd_operate_detach_port_parsed(void *parsed_result,
@@ -1466,7 +1466,7 @@ static void cmd_operate_detach_port_parsed(void *parsed_result,
 	struct cmd_operate_detach_port_result *res = parsed_result;
 
 	if (!strcmp(res->keyword, "detach"))
-		detach_port_device(res->port_id);
+		detach_port(res->identifier);
 	else
 		printf("Unknown parameter\n");
 }
@@ -1477,18 +1477,19 @@ cmdline_parse_token_string_t cmd_operate_detach_port_port =
 cmdline_parse_token_string_t cmd_operate_detach_port_keyword =
 	TOKEN_STRING_INITIALIZER(struct cmd_operate_detach_port_result,
 			keyword, "detach");
-cmdline_parse_token_num_t cmd_operate_detach_port_port_id =
-	TOKEN_NUM_INITIALIZER(struct cmd_operate_detach_port_result,
-			port_id, UINT16);
+cmdline_parse_token_string_t cmd_operate_detach_port_identifier =
+	TOKEN_STRING_INITIALIZER(struct cmd_operate_detach_port_result,
+			identifier, NULL);
 
 cmdline_parse_inst_t cmd_operate_detach_port = {
 	.f = cmd_operate_detach_port_parsed,
 	.data = NULL,
-	.help_str = "port detach <port_id>",
+	.help_str = "port detach <identifier>:"
+		"(identifier: pci address or virtual dev name)",
 	.tokens = {
 		(void *)&cmd_operate_detach_port_port,
 		(void *)&cmd_operate_detach_port_keyword,
-		(void *)&cmd_operate_detach_port_port_id,
+		(void *)&cmd_operate_detach_port_identifier,
 		NULL,
 	},
 };
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 6fbfd29..f4ddd94 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -2453,6 +2453,51 @@ detach_port_device(portid_t port_id)
 }
 
 void
+detach_port(char *identifier)
+{
+	struct rte_dev_iterator iterator;
+	struct rte_devargs da;
+	portid_t port_id;
+
+	printf("Removing a device...\n");
+
+	memset(&da, 0, sizeof(da));
+	if (rte_devargs_parsef(&da, "%s", identifier)) {
+		printf("cannot parse identifier\n");
+		if (da.args)
+			free(da.args);
+		return;
+	}
+
+	RTE_ETH_FOREACH_MATCHING_DEV(port_id, identifier, &iterator) {
+		if (ports[port_id].port_status != RTE_PORT_CLOSED) {
+			if (ports[port_id].port_status != RTE_PORT_STOPPED) {
+				printf("Port %u not stopped\n", port_id);
+				return;
+			}
+
+			/* sibling ports are forced to be closed */
+			if (ports[port_id].flow_list)
+				port_flow_flush(port_id);
+			ports[port_id].port_status = RTE_PORT_CLOSED;
+			printf("Port %u is now closed\n", port_id);
+		}
+	}
+
+	if (rte_eal_hotplug_remove(da.bus->name, da.name) != 0) {
+		TESTPMD_LOG(ERR, "Failed to detach device %s(%s)\n",
+			    da.name, da.bus->name);
+		return;
+	}
+
+	remove_invalid_ports();
+
+	printf("Device %s is detached\n", identifier);
+	printf("Now total ports is %d\n", nb_ports);
+	printf("Done\n");
+}
+
+void
 pmd_test_exit(void)
 {
 	struct rte_device *device;
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 1d9b7a2..3f2e06d 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -788,6 +788,7 @@ void stop_port(portid_t pid);
 void close_port(portid_t pid);
 void reset_port(portid_t pid);
 void attach_port(char *identifier);
+void detach_port(char *identifier);
 void detach_port_device(portid_t port_id);
 int all_ports_stopped(void);
 int port_is_stopped(portid_t port_id);
-- 
2.8.4

[dpdk-dev] [PATCH] app/testpmd: change port detach interface

[Nithin Dabilpuram]

With the latest published interface of
rte_eal_hotplug_[add,remove](), and rte_eth_dev_close(),
rte_eth_dev_close() would cleanup all the data structures of
port's eth dev leaving the device common resource intact
if RTE_ETH_DEV_CLOSE_REMOVE is set in dev flags.
So "port detach" (~hotplug remove) should be able to work,
with device identifier like "port attach" as eth_dev could have
been closed already and rte_eth_devices[port_id] reused.

This change alters "port detach" cmdline interface to
work with device identifier like "port attach".

Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
---
 app/test-pmd/cmdline.c | 15 ++++++++-------
 app/test-pmd/testpmd.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
 app/test-pmd/testpmd.h |  1 +
 3 files changed, 54 insertions(+), 7 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index c1042dd..c11b9d2 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -1456,7 +1456,7 @@ cmdline_parse_inst_t cmd_operate_attach_port = {
 struct cmd_operate_detach_port_result {
 	cmdline_fixed_string_t port;
 	cmdline_fixed_string_t keyword;
-	portid_t port_id;
+	cmdline_fixed_string_t identifier;
 };
 
 static void cmd_operate_detach_port_parsed(void *parsed_result,
@@ -1466,7 +1466,7 @@ static void cmd_operate_detach_port_parsed(void *parsed_result,
 	struct cmd_operate_detach_port_result *res = parsed_result;
 
 	if (!strcmp(res->keyword, "detach"))
-		detach_port_device(res->port_id);
+		detach_port(res->identifier);
 	else
 		printf("Unknown parameter\n");
 }
@@ -1477,18 +1477,19 @@ cmdline_parse_token_string_t cmd_operate_detach_port_port =
 cmdline_parse_token_string_t cmd_operate_detach_port_keyword =
 	TOKEN_STRING_INITIALIZER(struct cmd_operate_detach_port_result,
 			keyword, "detach");
-cmdline_parse_token_num_t cmd_operate_detach_port_port_id =
-	TOKEN_NUM_INITIALIZER(struct cmd_operate_detach_port_result,
-			port_id, UINT16);
+cmdline_parse_token_string_t cmd_operate_detach_port_identifier =
+	TOKEN_STRING_INITIALIZER(struct cmd_operate_detach_port_result,
+			identifier, NULL);
 
 cmdline_parse_inst_t cmd_operate_detach_port = {
 	.f = cmd_operate_detach_port_parsed,
 	.data = NULL,
-	.help_str = "port detach <port_id>",
+	.help_str = "port detach <identifier>:"
+		"(identifier: pci address or virtual dev name)",
 	.tokens = {
 		(void *)&cmd_operate_detach_port_port,
 		(void *)&cmd_operate_detach_port_keyword,
-		(void *)&cmd_operate_detach_port_port_id,
+		(void *)&cmd_operate_detach_port_identifier,
 		NULL,
 	},
 };
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 6fbfd29..f4ddd94 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -2453,6 +2453,51 @@ detach_port_device(portid_t port_id)
 }
 
 void
+detach_port(char *identifier)
+{
+	struct rte_dev_iterator iterator;
+	struct rte_devargs da;
+	portid_t port_id;
+
+	printf("Removing a device...\n");
+
+	memset(&da, 0, sizeof(da));
+	if (rte_devargs_parsef(&da, "%s", identifier)) {
+		printf("cannot parse identifier\n");
+		if (da.args)
+			free(da.args);
+		return;
+	}
+
+	RTE_ETH_FOREACH_MATCHING_DEV(port_id, identifier, &iterator) {
+		if (ports[port_id].port_status != RTE_PORT_CLOSED) {
+			if (ports[port_id].port_status != RTE_PORT_STOPPED) {
+				printf("Port %u not stopped\n", port_id);
+				return;
+			}
+
+			/* sibling ports are forced to be closed */
+			if (ports[port_id].flow_list)
+				port_flow_flush(port_id);
+			ports[port_id].port_status = RTE_PORT_CLOSED;
+			printf("Port %u is now closed\n", port_id);
+		}
+	}
+
+	if (rte_eal_hotplug_remove(da.bus->name, da.name) != 0) {
+		TESTPMD_LOG(ERR, "Failed to detach device %s(%s)\n",
+			    da.name, da.bus->name);
+		return;
+	}
+
+	remove_invalid_ports();
+
+	printf("Device %s is detached\n", identifier);
+	printf("Now total ports is %d\n", nb_ports);
+	printf("Done\n");
+}
+
+void
 pmd_test_exit(void)
 {
 	struct rte_device *device;
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 1d9b7a2..3f2e06d 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -788,6 +788,7 @@ void stop_port(portid_t pid);
 void close_port(portid_t pid);
 void reset_port(portid_t pid);
 void attach_port(char *identifier);
+void detach_port(char *identifier);
 void detach_port_device(portid_t port_id);
 int all_ports_stopped(void);
 int port_is_stopped(portid_t port_id);
-- 
2.8.4

Re: [dpdk-dev] [PATCH] app/testpmd: change port detach interface

[Thomas Monjalon]

Hi,

13/05/2019 13:21, Nithin Dabilpuram:
> With the latest published interface of
> rte_eal_hotplug_[add,remove](), and rte_eth_dev_close(),
> rte_eth_dev_close() would cleanup all the data structures of
> port's eth dev leaving the device common resource intact
> if RTE_ETH_DEV_CLOSE_REMOVE is set in dev flags.
> So "port detach" (~hotplug remove) should be able to work,
> with device identifier like "port attach" as eth_dev could have
> been closed already and rte_eth_devices[port_id] reused.

"port attach" uses devargs as identifier because there
is no port id before creating it. But "detach port" uses
logically the port id to close.

> This change alters "port detach" cmdline interface to
> work with device identifier like "port attach".

The word "port" means an ethdev port, so it should be
referenced with a port id.
If you want to close an EAL rte_device, then you should
rename the command.
But testpmd purpose should be to work with ethdev ports only.

PS: Please remind that a device can have multiple ports.

Re: [dpdk-dev] [PATCH] app/testpmd: change port detach interface

[Thomas Monjalon]

Hi,

13/05/2019 13:21, Nithin Dabilpuram:
> With the latest published interface of
> rte_eal_hotplug_[add,remove](), and rte_eth_dev_close(),
> rte_eth_dev_close() would cleanup all the data structures of
> port's eth dev leaving the device common resource intact
> if RTE_ETH_DEV_CLOSE_REMOVE is set in dev flags.
> So "port detach" (~hotplug remove) should be able to work,
> with device identifier like "port attach" as eth_dev could have
> been closed already and rte_eth_devices[port_id] reused.

"port attach" uses devargs as identifier because there
is no port id before creating it. But "detach port" uses
logically the port id to close.

> This change alters "port detach" cmdline interface to
> work with device identifier like "port attach".

The word "port" means an ethdev port, so it should be
referenced with a port id.
If you want to close an EAL rte_device, then you should
rename the command.
But testpmd purpose should be to work with ethdev ports only.

PS: Please remind that a device can have multiple ports.

Re: [dpdk-dev] [PATCH] app/testpmd: change port detach interface

[Nithin Dabilpuram]

Hi Thomas,
On Tue, May 14, 2019 at 05:39:30PM +0200, Thomas Monjalon wrote:
> Hi,
> 
> 13/05/2019 13:21, Nithin Dabilpuram:
> > With the latest published interface of
> > rte_eal_hotplug_[add,remove](), and rte_eth_dev_close(),
> > rte_eth_dev_close() would cleanup all the data structures of
> > port's eth dev leaving the device common resource intact
> > if RTE_ETH_DEV_CLOSE_REMOVE is set in dev flags.
> > So "port detach" (~hotplug remove) should be able to work,
> > with device identifier like "port attach" as eth_dev could have
> > been closed already and rte_eth_devices[port_id] reused.
> 
> "port attach" uses devargs as identifier because there
> is no port id before creating it. But "detach port" uses
> logically the port id to close.
But if "port close" was already called on that port,
eth_dev->state would be set as RTE_ETH_DEV_UNUSED and
that port id could be reused.
So after "port close" if we call "port detach", isn't it
incorrect to use the same port id ?
> 
> > This change alters "port detach" cmdline interface to
> > work with device identifier like "port attach".
> 
> The word "port" means an ethdev port, so it should be
> referenced with a port id.
> If you want to close an EAL rte_device, then you should
> rename the command.
> But testpmd purpose should be to work with ethdev ports only.
Renaming the command to "detach <identifier>" ?
> 
> PS: Please remind that a device can have multiple ports.
> 
> 

Re: [dpdk-dev] [PATCH] app/testpmd: change port detach interface

[Nithin Dabilpuram]

Hi Thomas,
On Tue, May 14, 2019 at 05:39:30PM +0200, Thomas Monjalon wrote:
> Hi,
> 
> 13/05/2019 13:21, Nithin Dabilpuram:
> > With the latest published interface of
> > rte_eal_hotplug_[add,remove](), and rte_eth_dev_close(),
> > rte_eth_dev_close() would cleanup all the data structures of
> > port's eth dev leaving the device common resource intact
> > if RTE_ETH_DEV_CLOSE_REMOVE is set in dev flags.
> > So "port detach" (~hotplug remove) should be able to work,
> > with device identifier like "port attach" as eth_dev could have
> > been closed already and rte_eth_devices[port_id] reused.
> 
> "port attach" uses devargs as identifier because there
> is no port id before creating it. But "detach port" uses
> logically the port id to close.
But if "port close" was already called on that port,
eth_dev->state would be set as RTE_ETH_DEV_UNUSED and
that port id could be reused.
So after "port close" if we call "port detach", isn't it
incorrect to use the same port id ?
> 
> > This change alters "port detach" cmdline interface to
> > work with device identifier like "port attach".
> 
> The word "port" means an ethdev port, so it should be
> referenced with a port id.
> If you want to close an EAL rte_device, then you should
> rename the command.
> But testpmd purpose should be to work with ethdev ports only.
Renaming the command to "detach <identifier>" ?
> 
> PS: Please remind that a device can have multiple ports.
> 
> 

Re: [dpdk-dev] [PATCH] app/testpmd: change port detach interface

[Thomas Monjalon]

15/05/2019 08:52, Nithin Dabilpuram:
> Hi Thomas,
> On Tue, May 14, 2019 at 05:39:30PM +0200, Thomas Monjalon wrote:
> > Hi,
> > 
> > 13/05/2019 13:21, Nithin Dabilpuram:
> > > With the latest published interface of
> > > rte_eal_hotplug_[add,remove](), and rte_eth_dev_close(),
> > > rte_eth_dev_close() would cleanup all the data structures of
> > > port's eth dev leaving the device common resource intact
> > > if RTE_ETH_DEV_CLOSE_REMOVE is set in dev flags.
> > > So "port detach" (~hotplug remove) should be able to work,
> > > with device identifier like "port attach" as eth_dev could have
> > > been closed already and rte_eth_devices[port_id] reused.
> > 
> > "port attach" uses devargs as identifier because there
> > is no port id before creating it. But "detach port" uses
> > logically the port id to close.
> 
> But if "port close" was already called on that port,
> eth_dev->state would be set as RTE_ETH_DEV_UNUSED and
> that port id could be reused.
> So after "port close" if we call "port detach", isn't it
> incorrect to use the same port id ?

Yes it is incorrect to close a port which is already closed :)

> > > This change alters "port detach" cmdline interface to
> > > work with device identifier like "port attach".
> > 
> > The word "port" means an ethdev port, so it should be
> > referenced with a port id.
> > If you want to close an EAL rte_device, then you should
> > rename the command.
> > But testpmd purpose should be to work with ethdev ports only.
> 
> Renaming the command to "detach <identifier>" ?

Yes something like that.
But why do you want to manage rte_device in testpmd?
Being able to close ports in not enough?
Please describe a scenario.

Re: [dpdk-dev] [PATCH] app/testpmd: change port detach interface

[Thomas Monjalon]

15/05/2019 08:52, Nithin Dabilpuram:
> Hi Thomas,
> On Tue, May 14, 2019 at 05:39:30PM +0200, Thomas Monjalon wrote:
> > Hi,
> > 
> > 13/05/2019 13:21, Nithin Dabilpuram:
> > > With the latest published interface of
> > > rte_eal_hotplug_[add,remove](), and rte_eth_dev_close(),
> > > rte_eth_dev_close() would cleanup all the data structures of
> > > port's eth dev leaving the device common resource intact
> > > if RTE_ETH_DEV_CLOSE_REMOVE is set in dev flags.
> > > So "port detach" (~hotplug remove) should be able to work,
> > > with device identifier like "port attach" as eth_dev could have
> > > been closed already and rte_eth_devices[port_id] reused.
> > 
> > "port attach" uses devargs as identifier because there
> > is no port id before creating it. But "detach port" uses
> > logically the port id to close.
> 
> But if "port close" was already called on that port,
> eth_dev->state would be set as RTE_ETH_DEV_UNUSED and
> that port id could be reused.
> So after "port close" if we call "port detach", isn't it
> incorrect to use the same port id ?

Yes it is incorrect to close a port which is already closed :)

> > > This change alters "port detach" cmdline interface to
> > > work with device identifier like "port attach".
> > 
> > The word "port" means an ethdev port, so it should be
> > referenced with a port id.
> > If you want to close an EAL rte_device, then you should
> > rename the command.
> > But testpmd purpose should be to work with ethdev ports only.
> 
> Renaming the command to "detach <identifier>" ?

Yes something like that.
But why do you want to manage rte_device in testpmd?
Being able to close ports in not enough?
Please describe a scenario.

Re: [dpdk-dev] [PATCH] app/testpmd: change port detach interface

[Nithin Dabilpuram]

On Wed, May 15, 2019 at 09:27:22AM +0200, Thomas Monjalon wrote:
> 15/05/2019 08:52, Nithin Dabilpuram:
> > Hi Thomas,
> > On Tue, May 14, 2019 at 05:39:30PM +0200, Thomas Monjalon wrote:
> > > Hi,
> > > 
> > > 13/05/2019 13:21, Nithin Dabilpuram:
> > > > With the latest published interface of
> > > > rte_eal_hotplug_[add,remove](), and rte_eth_dev_close(),
> > > > rte_eth_dev_close() would cleanup all the data structures of
> > > > port's eth dev leaving the device common resource intact
> > > > if RTE_ETH_DEV_CLOSE_REMOVE is set in dev flags.
> > > > So "port detach" (~hotplug remove) should be able to work,
> > > > with device identifier like "port attach" as eth_dev could have
> > > > been closed already and rte_eth_devices[port_id] reused.
> > > 
> > > "port attach" uses devargs as identifier because there
> > > is no port id before creating it. But "detach port" uses
> > > logically the port id to close.
> > 
> > But if "port close" was already called on that port,
> > eth_dev->state would be set as RTE_ETH_DEV_UNUSED and
> > that port id could be reused.
> > So after "port close" if we call "port detach", isn't it
> > incorrect to use the same port id ?
> 
> Yes it is incorrect to close a port which is already closed :)
> 
> > > > This change alters "port detach" cmdline interface to
> > > > work with device identifier like "port attach".
> > > 
> > > The word "port" means an ethdev port, so it should be
> > > referenced with a port id.
> > > If you want to close an EAL rte_device, then you should
> > > rename the command.
> > > But testpmd purpose should be to work with ethdev ports only.
> > 
> > Renaming the command to "detach <identifier>" ?
> 
> Yes something like that.
> But why do you want to manage rte_device in testpmd?
> Being able to close ports in not enough?
> Please describe a scenario.
>

We just want to support testing hotplug detach along with
hotplug attach from testpmd. Currently there is no way to detach
if we close the port first.

Another reason is that in our new PMD, for detaching one specific port,
we need more than one try as the PMD might return -EAGAIN.
So with the current "port detach" implementation, after closing the port,
if PMD returns -EAGAIN for rte_dev_remove() call, there is no way to
try it again.
> 

Re: [dpdk-dev] [PATCH] app/testpmd: change port detach interface

[Thomas Monjalon]

17/05/2019 10:55, Nithin Dabilpuram:
> On Wed, May 15, 2019 at 09:27:22AM +0200, Thomas Monjalon wrote:
> > 15/05/2019 08:52, Nithin Dabilpuram:
> > > Hi Thomas,
> > > On Tue, May 14, 2019 at 05:39:30PM +0200, Thomas Monjalon wrote:
> > > > Hi,
> > > > 
> > > > 13/05/2019 13:21, Nithin Dabilpuram:
> > > > > With the latest published interface of
> > > > > rte_eal_hotplug_[add,remove](), and rte_eth_dev_close(),
> > > > > rte_eth_dev_close() would cleanup all the data structures of
> > > > > port's eth dev leaving the device common resource intact
> > > > > if RTE_ETH_DEV_CLOSE_REMOVE is set in dev flags.
> > > > > So "port detach" (~hotplug remove) should be able to work,
> > > > > with device identifier like "port attach" as eth_dev could have
> > > > > been closed already and rte_eth_devices[port_id] reused.
> > > > 
> > > > "port attach" uses devargs as identifier because there
> > > > is no port id before creating it. But "detach port" uses
> > > > logically the port id to close.
> > > 
> > > But if "port close" was already called on that port,
> > > eth_dev->state would be set as RTE_ETH_DEV_UNUSED and
> > > that port id could be reused.
> > > So after "port close" if we call "port detach", isn't it
> > > incorrect to use the same port id ?
> > 
> > Yes it is incorrect to close a port which is already closed :)
> > 
> > > > > This change alters "port detach" cmdline interface to
> > > > > work with device identifier like "port attach".
> > > > 
> > > > The word "port" means an ethdev port, so it should be
> > > > referenced with a port id.
> > > > If you want to close an EAL rte_device, then you should
> > > > rename the command.
> > > > But testpmd purpose should be to work with ethdev ports only.
> > > 
> > > Renaming the command to "detach <identifier>" ?
> > 
> > Yes something like that.
> > But why do you want to manage rte_device in testpmd?
> > Being able to close ports in not enough?
> > Please describe a scenario.
> >
> 
> We just want to support testing hotplug detach along with
> hotplug attach from testpmd. Currently there is no way to detach
> if we close the port first.

OK

> Another reason is that in our new PMD, for detaching one specific port,
> we need more than one try as the PMD might return -EAGAIN.
> So with the current "port detach" implementation, after closing the port,
> if PMD returns -EAGAIN for rte_dev_remove() call, there is no way to
> try it again.

This is a bug.
Should we catch -EAGAIN somewhere?

Re: [dpdk-dev] [PATCH] app/testpmd: change port detach interface

[Nithin Dabilpuram]

On Fri, May 17, 2019 at 10:59:38AM +0200, Thomas Monjalon wrote:
> 17/05/2019 10:55, Nithin Dabilpuram:
> > On Wed, May 15, 2019 at 09:27:22AM +0200, Thomas Monjalon wrote:
> > > 15/05/2019 08:52, Nithin Dabilpuram:
> > > > Hi Thomas,
> > > > On Tue, May 14, 2019 at 05:39:30PM +0200, Thomas Monjalon wrote:
> > > > > Hi,
> > > > > 
> > > > > 13/05/2019 13:21, Nithin Dabilpuram:
> > > > > > With the latest published interface of
> > > > > > rte_eal_hotplug_[add,remove](), and rte_eth_dev_close(),
> > > > > > rte_eth_dev_close() would cleanup all the data structures of
> > > > > > port's eth dev leaving the device common resource intact
> > > > > > if RTE_ETH_DEV_CLOSE_REMOVE is set in dev flags.
> > > > > > So "port detach" (~hotplug remove) should be able to work,
> > > > > > with device identifier like "port attach" as eth_dev could have
> > > > > > been closed already and rte_eth_devices[port_id] reused.
> > > > > 
> > > > > "port attach" uses devargs as identifier because there
> > > > > is no port id before creating it. But "detach port" uses
> > > > > logically the port id to close.
> > > > 
> > > > But if "port close" was already called on that port,
> > > > eth_dev->state would be set as RTE_ETH_DEV_UNUSED and
> > > > that port id could be reused.
> > > > So after "port close" if we call "port detach", isn't it
> > > > incorrect to use the same port id ?
> > > 
> > > Yes it is incorrect to close a port which is already closed :)
> > > 
> > > > > > This change alters "port detach" cmdline interface to
> > > > > > work with device identifier like "port attach".
> > > > > 
> > > > > The word "port" means an ethdev port, so it should be
> > > > > referenced with a port id.
> > > > > If you want to close an EAL rte_device, then you should
> > > > > rename the command.
> > > > > But testpmd purpose should be to work with ethdev ports only.
> > > > 
> > > > Renaming the command to "detach <identifier>" ?
> > > 
> > > Yes something like that.
> > > But why do you want to manage rte_device in testpmd?
> > > Being able to close ports in not enough?
> > > Please describe a scenario.
> > >
> > 
> > We just want to support testing hotplug detach along with
> > hotplug attach from testpmd. Currently there is no way to detach
> > if we close the port first.
> 
> OK
So can I send next revision with command renamed to "detach <identifier>" ?
> 
> > Another reason is that in our new PMD, for detaching one specific port,
> > we need more than one try as the PMD might return -EAGAIN.
> > So with the current "port detach" implementation, after closing the port,
> > if PMD returns -EAGAIN for rte_dev_remove() call, there is no way to
> > try it again.
> 
> This is a bug.
> Should we catch -EAGAIN somewhere?

It is already caught in local_dev_remove() and
rte_dev_remove() fails. Only problem as I said below is
in testpmd if first call to detach_port_device() i.e handler of "port detach", 
rte_dev_remove() returns -EAGAIN and PMD cleaned up the resources partially like eth_dev
resources, the second time call cannot work port_id will not be valid anymore.

> 
> 

Re: [dpdk-dev] [PATCH] app/testpmd: change port detach interface

[Nithin Dabilpuram]

Hi Thomas,

On Mon, May 20, 2019 at 06:20:53PM +0530, Nithin Dabilpuram wrote:
> On Fri, May 17, 2019 at 10:59:38AM +0200, Thomas Monjalon wrote:
> > 17/05/2019 10:55, Nithin Dabilpuram:
> > > On Wed, May 15, 2019 at 09:27:22AM +0200, Thomas Monjalon wrote:
> > > > 15/05/2019 08:52, Nithin Dabilpuram:
> > > > > Hi Thomas,
> > > > > On Tue, May 14, 2019 at 05:39:30PM +0200, Thomas Monjalon wrote:
> > > > > > Hi,
> > > > > > 
> > > > > > 13/05/2019 13:21, Nithin Dabilpuram:
> > > > > > > With the latest published interface of
> > > > > > > rte_eal_hotplug_[add,remove](), and rte_eth_dev_close(),
> > > > > > > rte_eth_dev_close() would cleanup all the data structures of
> > > > > > > port's eth dev leaving the device common resource intact
> > > > > > > if RTE_ETH_DEV_CLOSE_REMOVE is set in dev flags.
> > > > > > > So "port detach" (~hotplug remove) should be able to work,
> > > > > > > with device identifier like "port attach" as eth_dev could have
> > > > > > > been closed already and rte_eth_devices[port_id] reused.
> > > > > > 
> > > > > > "port attach" uses devargs as identifier because there
> > > > > > is no port id before creating it. But "detach port" uses
> > > > > > logically the port id to close.
> > > > > 
> > > > > But if "port close" was already called on that port,
> > > > > eth_dev->state would be set as RTE_ETH_DEV_UNUSED and
> > > > > that port id could be reused.
> > > > > So after "port close" if we call "port detach", isn't it
> > > > > incorrect to use the same port id ?
> > > > 
> > > > Yes it is incorrect to close a port which is already closed :)
> > > > 
> > > > > > > This change alters "port detach" cmdline interface to
> > > > > > > work with device identifier like "port attach".
> > > > > > 
> > > > > > The word "port" means an ethdev port, so it should be
> > > > > > referenced with a port id.
> > > > > > If you want to close an EAL rte_device, then you should
> > > > > > rename the command.
> > > > > > But testpmd purpose should be to work with ethdev ports only.
> > > > > 
> > > > > Renaming the command to "detach <identifier>" ?
> > > > 
> > > > Yes something like that.
> > > > But why do you want to manage rte_device in testpmd?
> > > > Being able to close ports in not enough?
> > > > Please describe a scenario.
> > > >
> > > 
> > > We just want to support testing hotplug detach along with
> > > hotplug attach from testpmd. Currently there is no way to detach
> > > if we close the port first.
> > 
> > OK
> So can I send next revision with command renamed to "detach <identifier>" ?

Any info on this ? I can even add it as another cmd without disturbing existing
command if needed.

> > 
> > > Another reason is that in our new PMD, for detaching one specific port,
> > > we need more than one try as the PMD might return -EAGAIN.
> > > So with the current "port detach" implementation, after closing the port,
> > > if PMD returns -EAGAIN for rte_dev_remove() call, there is no way to
> > > try it again.
> > 
> > This is a bug.
> > Should we catch -EAGAIN somewhere?
> 
> It is already caught in local_dev_remove() and
> rte_dev_remove() fails. Only problem as I said below is
> in testpmd if first call to detach_port_device() i.e handler of "port detach", 
> rte_dev_remove() returns -EAGAIN and PMD cleaned up the resources partially like eth_dev
> resources, the second time call cannot work port_id will not be valid anymore.
> 
> > 
> > 

Re: [dpdk-dev] [PATCH] app/testpmd: change port detach interface

[Nithin Dabilpuram]

Hi Thomas,

A reminder about this patch.

On Wed, May 29, 2019 at 01:46:20PM +0530, Nithin Dabilpuram wrote:
> Hi Thomas,
> 
> On Mon, May 20, 2019 at 06:20:53PM +0530, Nithin Dabilpuram wrote:
> > On Fri, May 17, 2019 at 10:59:38AM +0200, Thomas Monjalon wrote:
> > > 17/05/2019 10:55, Nithin Dabilpuram:
> > > > On Wed, May 15, 2019 at 09:27:22AM +0200, Thomas Monjalon wrote:
> > > > > 15/05/2019 08:52, Nithin Dabilpuram:
> > > > > > Hi Thomas,
> > > > > > On Tue, May 14, 2019 at 05:39:30PM +0200, Thomas Monjalon wrote:
> > > > > > > Hi,
> > > > > > > 
> > > > > > > 13/05/2019 13:21, Nithin Dabilpuram:
> > > > > > > > With the latest published interface of
> > > > > > > > rte_eal_hotplug_[add,remove](), and rte_eth_dev_close(),
> > > > > > > > rte_eth_dev_close() would cleanup all the data structures of
> > > > > > > > port's eth dev leaving the device common resource intact
> > > > > > > > if RTE_ETH_DEV_CLOSE_REMOVE is set in dev flags.
> > > > > > > > So "port detach" (~hotplug remove) should be able to work,
> > > > > > > > with device identifier like "port attach" as eth_dev could have
> > > > > > > > been closed already and rte_eth_devices[port_id] reused.
> > > > > > > 
> > > > > > > "port attach" uses devargs as identifier because there
> > > > > > > is no port id before creating it. But "detach port" uses
> > > > > > > logically the port id to close.
> > > > > > 
> > > > > > But if "port close" was already called on that port,
> > > > > > eth_dev->state would be set as RTE_ETH_DEV_UNUSED and
> > > > > > that port id could be reused.
> > > > > > So after "port close" if we call "port detach", isn't it
> > > > > > incorrect to use the same port id ?
> > > > > 
> > > > > Yes it is incorrect to close a port which is already closed :)
> > > > > 
> > > > > > > > This change alters "port detach" cmdline interface to
> > > > > > > > work with device identifier like "port attach".
> > > > > > > 
> > > > > > > The word "port" means an ethdev port, so it should be
> > > > > > > referenced with a port id.
> > > > > > > If you want to close an EAL rte_device, then you should
> > > > > > > rename the command.
> > > > > > > But testpmd purpose should be to work with ethdev ports only.
> > > > > > 
> > > > > > Renaming the command to "detach <identifier>" ?
> > > > > 
> > > > > Yes something like that.
> > > > > But why do you want to manage rte_device in testpmd?
> > > > > Being able to close ports in not enough?
> > > > > Please describe a scenario.
> > > > >
> > > > 
> > > > We just want to support testing hotplug detach along with
> > > > hotplug attach from testpmd. Currently there is no way to detach
> > > > if we close the port first.
> > > 
> > > OK
> > So can I send next revision with command renamed to "detach <identifier>" ?
> 
> Any info on this ? I can even add it as another cmd without disturbing existing
> command if needed.
> 
> > > 
> > > > Another reason is that in our new PMD, for detaching one specific port,
> > > > we need more than one try as the PMD might return -EAGAIN.
> > > > So with the current "port detach" implementation, after closing the port,
> > > > if PMD returns -EAGAIN for rte_dev_remove() call, there is no way to
> > > > try it again.
> > > 
> > > This is a bug.
> > > Should we catch -EAGAIN somewhere?
> > 
> > It is already caught in local_dev_remove() and
> > rte_dev_remove() fails. Only problem as I said below is
> > in testpmd if first call to detach_port_device() i.e handler of "port detach", 
> > rte_dev_remove() returns -EAGAIN and PMD cleaned up the resources partially like eth_dev
> > resources, the second time call cannot work port_id will not be valid anymore.
> > 
> > > 
> > > 

Re: [dpdk-dev] [PATCH] app/testpmd: change port detach interface

[Yigit, Ferruh]

On 5/29/2019 9:16 AM, Nithin Dabilpuram wrote:
> Hi Thomas,
> 
> On Mon, May 20, 2019 at 06:20:53PM +0530, Nithin Dabilpuram wrote:
>> On Fri, May 17, 2019 at 10:59:38AM +0200, Thomas Monjalon wrote:
>>> 17/05/2019 10:55, Nithin Dabilpuram:
>>>> On Wed, May 15, 2019 at 09:27:22AM +0200, Thomas Monjalon wrote:
>>>>> 15/05/2019 08:52, Nithin Dabilpuram:
>>>>>> Hi Thomas,
>>>>>> On Tue, May 14, 2019 at 05:39:30PM +0200, Thomas Monjalon wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> 13/05/2019 13:21, Nithin Dabilpuram:
>>>>>>>> With the latest published interface of
>>>>>>>> rte_eal_hotplug_[add,remove](), and rte_eth_dev_close(),
>>>>>>>> rte_eth_dev_close() would cleanup all the data structures of
>>>>>>>> port's eth dev leaving the device common resource intact
>>>>>>>> if RTE_ETH_DEV_CLOSE_REMOVE is set in dev flags.
>>>>>>>> So "port detach" (~hotplug remove) should be able to work,
>>>>>>>> with device identifier like "port attach" as eth_dev could have
>>>>>>>> been closed already and rte_eth_devices[port_id] reused.
>>>>>>>
>>>>>>> "port attach" uses devargs as identifier because there
>>>>>>> is no port id before creating it. But "detach port" uses
>>>>>>> logically the port id to close.
>>>>>>
>>>>>> But if "port close" was already called on that port,
>>>>>> eth_dev->state would be set as RTE_ETH_DEV_UNUSED and
>>>>>> that port id could be reused.
>>>>>> So after "port close" if we call "port detach", isn't it
>>>>>> incorrect to use the same port id ?
>>>>>
>>>>> Yes it is incorrect to close a port which is already closed :)
>>>>>
>>>>>>>> This change alters "port detach" cmdline interface to
>>>>>>>> work with device identifier like "port attach".
>>>>>>>
>>>>>>> The word "port" means an ethdev port, so it should be
>>>>>>> referenced with a port id.
>>>>>>> If you want to close an EAL rte_device, then you should
>>>>>>> rename the command.
>>>>>>> But testpmd purpose should be to work with ethdev ports only.
>>>>>>
>>>>>> Renaming the command to "detach <identifier>" ?
>>>>>
>>>>> Yes something like that.
>>>>> But why do you want to manage rte_device in testpmd?
>>>>> Being able to close ports in not enough?
>>>>> Please describe a scenario.
>>>>>
>>>>
>>>> We just want to support testing hotplug detach along with
>>>> hotplug attach from testpmd. Currently there is no way to detach
>>>> if we close the port first.
>>>
>>> OK
>> So can I send next revision with command renamed to "detach <identifier>" ?
> 
> Any info on this ? I can even add it as another cmd without disturbing existing
> command if needed.

This sounds better option to me. I see the need to remove device via
'identifier' but also still it is easier to use 'port_id' for removal when
applicable, so I am for keeping it.

What do you think adding a new command:
'device detach'

Also testpmd doesn't dead with 'device' much, it mainly works in port level,
because of this does it make sense to add another command something like:
"show device info all"

> 
>>>
>>>> Another reason is that in our new PMD, for detaching one specific port,
>>>> we need more than one try as the PMD might return -EAGAIN.
>>>> So with the current "port detach" implementation, after closing the port,
>>>> if PMD returns -EAGAIN for rte_dev_remove() call, there is no way to
>>>> try it again.
>>>
>>> This is a bug.
>>> Should we catch -EAGAIN somewhere?
>>
>> It is already caught in local_dev_remove() and
>> rte_dev_remove() fails. Only problem as I said below is
>> in testpmd if first call to detach_port_device() i.e handler of "port detach", 
>> rte_dev_remove() returns -EAGAIN and PMD cleaned up the resources partially like eth_dev
>> resources, the second time call cannot work port_id will not be valid anymore.
>>
>>>
>>>

Re: [dpdk-dev] [PATCH] app/testpmd: change port detach interface

[Nithin Dabilpuram]

On Tue, Jul 02, 2019 at 04:58:17PM +0100, Yigit, Ferruh wrote:
> On 5/29/2019 9:16 AM, Nithin Dabilpuram wrote:
> > Hi Thomas,
> > 
> > On Mon, May 20, 2019 at 06:20:53PM +0530, Nithin Dabilpuram wrote:
> >> On Fri, May 17, 2019 at 10:59:38AM +0200, Thomas Monjalon wrote:
> >>> 17/05/2019 10:55, Nithin Dabilpuram:
> >>>> On Wed, May 15, 2019 at 09:27:22AM +0200, Thomas Monjalon wrote:
> >>>>> 15/05/2019 08:52, Nithin Dabilpuram:
> >>>>>> Hi Thomas,
> >>>>>> On Tue, May 14, 2019 at 05:39:30PM +0200, Thomas Monjalon wrote:
> >>>>>>> Hi,
> >>>>>>>
> >>>>>>> 13/05/2019 13:21, Nithin Dabilpuram:
> >>>>>>>> With the latest published interface of
> >>>>>>>> rte_eal_hotplug_[add,remove](), and rte_eth_dev_close(),
> >>>>>>>> rte_eth_dev_close() would cleanup all the data structures of
> >>>>>>>> port's eth dev leaving the device common resource intact
> >>>>>>>> if RTE_ETH_DEV_CLOSE_REMOVE is set in dev flags.
> >>>>>>>> So "port detach" (~hotplug remove) should be able to work,
> >>>>>>>> with device identifier like "port attach" as eth_dev could have
> >>>>>>>> been closed already and rte_eth_devices[port_id] reused.
> >>>>>>>
> >>>>>>> "port attach" uses devargs as identifier because there
> >>>>>>> is no port id before creating it. But "detach port" uses
> >>>>>>> logically the port id to close.
> >>>>>>
> >>>>>> But if "port close" was already called on that port,
> >>>>>> eth_dev->state would be set as RTE_ETH_DEV_UNUSED and
> >>>>>> that port id could be reused.
> >>>>>> So after "port close" if we call "port detach", isn't it
> >>>>>> incorrect to use the same port id ?
> >>>>>
> >>>>> Yes it is incorrect to close a port which is already closed :)
> >>>>>
> >>>>>>>> This change alters "port detach" cmdline interface to
> >>>>>>>> work with device identifier like "port attach".
> >>>>>>>
> >>>>>>> The word "port" means an ethdev port, so it should be
> >>>>>>> referenced with a port id.
> >>>>>>> If you want to close an EAL rte_device, then you should
> >>>>>>> rename the command.
> >>>>>>> But testpmd purpose should be to work with ethdev ports only.
> >>>>>>
> >>>>>> Renaming the command to "detach <identifier>" ?
> >>>>>
> >>>>> Yes something like that.
> >>>>> But why do you want to manage rte_device in testpmd?
> >>>>> Being able to close ports in not enough?
> >>>>> Please describe a scenario.
> >>>>>
> >>>>
> >>>> We just want to support testing hotplug detach along with
> >>>> hotplug attach from testpmd. Currently there is no way to detach
> >>>> if we close the port first.
> >>>
> >>> OK
> >> So can I send next revision with command renamed to "detach <identifier>" ?
> > 
> > Any info on this ? I can even add it as another cmd without disturbing existing
> > command if needed.
> 
> This sounds better option to me. I see the need to remove device via
> 'identifier' but also still it is easier to use 'port_id' for removal when
> applicable, so I am for keeping it.
Thanks. Will send out a patch for the same.
> 
> What do you think adding a new command:
> 'device detach'
> 
> Also testpmd doesn't dead with 'device' much, it mainly works in port level,
> because of this does it make sense to add another command something like:
> "show device info all"
Sure. Will add it.
> 
> > 
> >>>
> >>>> Another reason is that in our new PMD, for detaching one specific port,
> >>>> we need more than one try as the PMD might return -EAGAIN.
> >>>> So with the current "port detach" implementation, after closing the port,
> >>>> if PMD returns -EAGAIN for rte_dev_remove() call, there is no way to
> >>>> try it again.
> >>>
> >>> This is a bug.
> >>> Should we catch -EAGAIN somewhere?
> >>
> >> It is already caught in local_dev_remove() and
> >> rte_dev_remove() fails. Only problem as I said below is
> >> in testpmd if first call to detach_port_device() i.e handler of "port detach", 
> >> rte_dev_remove() returns -EAGAIN and PMD cleaned up the resources partially like eth_dev
> >> resources, the second time call cannot work port_id will not be valid anymore.
> >>
> >>>
> >>>
> 

[dpdk-dev] [PATCH v2] app/testpmd: add device related cmds

[Nithin Dabilpuram]

With the latest published interface of
rte_eal_hotplug_[add,remove](), and rte_eth_dev_close(),
rte_eth_dev_close() would cleanup all the data structures of
port's eth dev leaving the device common resource intact
if RTE_ETH_DEV_CLOSE_REMOVE is set in dev flags.

So a new command "detach device" (~hotplug remove) to work,
with device identifier like "port attach" is added
to be able to detach closed devices.

Also to display currently probed devices, another command
"show device info <identifier>|all" is also added as a
part of this change.

Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
---
 app/test-pmd/cmdline.c                      | 88 +++++++++++++++++++++++++++++
 app/test-pmd/config.c                       | 73 ++++++++++++++++++++++++
 app/test-pmd/testpmd.c                      | 45 +++++++++++++++
 app/test-pmd/testpmd.h                      |  2 +
 doc/guides/testpmd_app_ug/testpmd_funcs.rst | 64 +++++++++++++++++++++
 5 files changed, 272 insertions(+)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 01dd45f..e10e82a 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -1493,6 +1493,47 @@ cmdline_parse_inst_t cmd_operate_detach_port = {
 	},
 };
 
+/* *** detach device by identifier *** */
+struct cmd_operate_detach_device_result {
+	cmdline_fixed_string_t device;
+	cmdline_fixed_string_t keyword;
+	cmdline_fixed_string_t identifier;
+};
+
+static void cmd_operate_detach_device_parsed(void *parsed_result,
+				__attribute__((unused)) struct cmdline *cl,
+				__attribute__((unused)) void *data)
+{
+	struct cmd_operate_detach_device_result *res = parsed_result;
+
+	if (!strcmp(res->keyword, "detach"))
+		detach_device(res->identifier);
+	else
+		printf("Unknown parameter\n");
+}
+
+cmdline_parse_token_string_t cmd_operate_detach_device_device =
+	TOKEN_STRING_INITIALIZER(struct cmd_operate_detach_device_result,
+			device, "device");
+cmdline_parse_token_string_t cmd_operate_detach_device_keyword =
+	TOKEN_STRING_INITIALIZER(struct cmd_operate_detach_device_result,
+			keyword, "detach");
+cmdline_parse_token_string_t cmd_operate_detach_device_identifier =
+	TOKEN_STRING_INITIALIZER(struct cmd_operate_detach_device_result,
+			identifier, NULL);
+
+cmdline_parse_inst_t cmd_operate_detach_device = {
+	.f = cmd_operate_detach_device_parsed,
+	.data = NULL,
+	.help_str = "device detach <identifier>:"
+		"(identifier: pci address or virtual dev name)",
+	.tokens = {
+		(void *)&cmd_operate_detach_device_device,
+		(void *)&cmd_operate_detach_device_keyword,
+		(void *)&cmd_operate_detach_device_identifier,
+		NULL,
+	},
+};
 /* *** configure speed for all ports *** */
 struct cmd_config_speed_all {
 	cmdline_fixed_string_t port;
@@ -7456,6 +7497,51 @@ cmdline_parse_inst_t cmd_showport = {
 	},
 };
 
+/* *** SHOW DEVICE INFO *** */
+struct cmd_showdevice_result {
+	cmdline_fixed_string_t show;
+	cmdline_fixed_string_t device;
+	cmdline_fixed_string_t what;
+	cmdline_fixed_string_t identifier;
+};
+
+static void cmd_showdevice_parsed(void *parsed_result,
+				__attribute__((unused)) struct cmdline *cl,
+				__attribute__((unused)) void *data)
+{
+	struct cmd_showdevice_result *res = parsed_result;
+	if (!strcmp(res->what, "info")) {
+		if (!strcmp(res->identifier, "all"))
+			device_infos_display(NULL);
+		else
+			device_infos_display(res->identifier);
+	}
+}
+
+cmdline_parse_token_string_t cmd_showdevice_show =
+	TOKEN_STRING_INITIALIZER(struct cmd_showdevice_result, show,
+				 "show");
+cmdline_parse_token_string_t cmd_showdevice_device =
+	TOKEN_STRING_INITIALIZER(struct cmd_showdevice_result, device, "device");
+cmdline_parse_token_string_t cmd_showdevice_what =
+	TOKEN_STRING_INITIALIZER(struct cmd_showdevice_result, what,
+				 "info");
+cmdline_parse_token_string_t cmd_showdevice_identifier =
+	TOKEN_STRING_INITIALIZER(struct cmd_showdevice_result,
+			identifier, NULL);
+
+cmdline_parse_inst_t cmd_showdevice = {
+	.f = cmd_showdevice_parsed,
+	.data = NULL,
+	.help_str = "show device info <identifier>|all",
+	.tokens = {
+		(void *)&cmd_showdevice_show,
+		(void *)&cmd_showdevice_device,
+		(void *)&cmd_showdevice_what,
+		(void *)&cmd_showdevice_identifier,
+		NULL,
+	},
+};
 /* *** SHOW QUEUE INFO *** */
 struct cmd_showqueue_result {
 	cmdline_fixed_string_t show;
@@ -18723,6 +18809,7 @@ cmdline_parse_ctx_t main_ctx[] = {
 	(cmdline_parse_inst_t *)&cmd_showport,
 	(cmdline_parse_inst_t *)&cmd_showqueue,
 	(cmdline_parse_inst_t *)&cmd_showportall,
+	(cmdline_parse_inst_t *)&cmd_showdevice,
 	(cmdline_parse_inst_t *)&cmd_showcfg,
 	(cmdline_parse_inst_t *)&cmd_showfwdall,
 	(cmdline_parse_inst_t *)&cmd_start,
@@ -18811,6 +18898,7 @@ cmdline_parse_ctx_t main_ctx[] = {
 	(cmdline_parse_inst_t *)&cmd_operate_specific_port,
 	(cmdline_parse_inst_t *)&cmd_operate_attach_port,
 	(cmdline_parse_inst_t *)&cmd_operate_detach_port,
+	(cmdline_parse_inst_t *)&cmd_operate_detach_device,
 	(cmdline_parse_inst_t *)&cmd_set_port_setup_on,
 	(cmdline_parse_inst_t *)&cmd_config_speed_all,
 	(cmdline_parse_inst_t *)&cmd_config_speed_specific,
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 1d80470..537e73f 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -386,6 +386,79 @@ tx_queue_infos_display(portid_t port_id, uint16_t queue_id)
 	printf("\n");
 }
 
+static int bus_match_all(const struct rte_bus *bus, const void *data)
+{
+	RTE_SET_USED(bus);
+	RTE_SET_USED(data);
+	return 0;
+}
+
+void
+device_infos_display(const char *identifier)
+{
+	static const char *info_border = "*********************";
+	struct rte_bus *start = NULL, *next;
+	struct rte_dev_iterator dev_iter;
+	char name[RTE_ETH_NAME_MAX_LEN];
+	struct rte_ether_addr mac_addr;
+	struct rte_device *dev;
+	struct rte_devargs da;
+	portid_t port_id;
+	char devstr[128];
+
+	memset(&da, 0, sizeof(da));
+	if (!identifier)
+		goto skip_parse;
+
+	if (rte_devargs_parsef(&da, "%s", identifier)) {
+		printf("cannot parse identifier\n");
+		if (da.args)
+			free(da.args);
+		return;
+	}
+
+skip_parse:
+	while ((next = rte_bus_find(start, bus_match_all, NULL)) != NULL) {
+
+		start = next;
+		if (identifier && da.bus != next)
+			continue;
+
+		/* Skip buses that don't have iterate method */
+		if (!next->dev_iterate)
+			continue;
+
+		snprintf(devstr, sizeof(devstr), "bus=%s", next->name);
+		RTE_DEV_FOREACH(dev, devstr, &dev_iter) {
+
+			if (!dev->driver)
+				continue;
+			/* Check for matching device if identifier is present */
+			if (identifier &&
+			    strncmp(da.name, dev->name, strlen(dev->name)))
+				continue;
+			printf("\n%s Infos for device %s %s\n",
+			       info_border, dev->name, info_border);
+			printf("Bus name: %s", dev->bus->name);
+			printf("\nDriver name: %s", dev->driver->name);
+			printf("\nDevargs: %s",
+			       dev->devargs ? dev->devargs->args : "");
+			printf("\nConnect to socket: %d", dev->numa_node);
+			printf("\n");
+
+			/* List ports with matching device name */
+			RTE_ETH_FOREACH_DEV_OF(port_id, dev) {
+				rte_eth_macaddr_get(port_id, &mac_addr);
+				printf("\n\tPort id: %-2d", port_id);
+				print_ethaddr("\n\tMAC address: ", &mac_addr);
+				rte_eth_dev_get_name_by_port(port_id, name);
+				printf("\n\tDevice name: %s", name);
+				printf("\n");
+			}
+		}
+	};
+}
+
 void
 port_infos_display(portid_t port_id)
 {
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 3ed3523..a3b1542 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -2476,6 +2476,51 @@ detach_port_device(portid_t port_id)
 }
 
 void
+detach_device(char *identifier)
+{
+	struct rte_dev_iterator iterator;
+	struct rte_devargs da;
+	portid_t port_id;
+
+	printf("Removing a device...\n");
+
+	memset(&da, 0, sizeof(da));
+	if (rte_devargs_parsef(&da, "%s", identifier)) {
+		printf("cannot parse identifier\n");
+		if (da.args)
+			free(da.args);
+		return;
+	}
+
+	RTE_ETH_FOREACH_MATCHING_DEV(port_id, identifier, &iterator) {
+		if (ports[port_id].port_status != RTE_PORT_CLOSED) {
+			if (ports[port_id].port_status != RTE_PORT_STOPPED) {
+				printf("Port %u not stopped\n", port_id);
+				return;
+			}
+
+			/* sibling ports are forced to be closed */
+			if (ports[port_id].flow_list)
+				port_flow_flush(port_id);
+			ports[port_id].port_status = RTE_PORT_CLOSED;
+			printf("Port %u is now closed\n", port_id);
+		}
+	}
+
+	if (rte_eal_hotplug_remove(da.bus->name, da.name) != 0) {
+		TESTPMD_LOG(ERR, "Failed to detach device %s(%s)\n",
+			    da.name, da.bus->name);
+		return;
+	}
+
+	remove_invalid_ports();
+
+	printf("Device %s is detached\n", identifier);
+	printf("Now total ports is %d\n", nb_ports);
+	printf("Done\n");
+}
+
+void
 pmd_test_exit(void)
 {
 	portid_t pt_id;
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index e3a6f7c..93bed6d 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -689,6 +689,7 @@ void nic_stats_clear(portid_t port_id);
 void nic_xstats_display(portid_t port_id);
 void nic_xstats_clear(portid_t port_id);
 void nic_stats_mapping_display(portid_t port_id);
+void device_infos_display(const char *identifier);
 void port_infos_display(portid_t port_id);
 void port_summary_display(portid_t port_id);
 void port_summary_header_display(void);
@@ -788,6 +789,7 @@ void stop_port(portid_t pid);
 void close_port(portid_t pid);
 void reset_port(portid_t pid);
 void attach_port(char *identifier);
+void detach_device(char *identifier);
 void detach_port_device(portid_t port_id);
 int all_ports_stopped(void);
 int port_is_stopped(portid_t port_id);
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index ebab2f1..04b9a31 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -467,6 +467,29 @@ Show Tx metadata value set for a specific port::
 
    testpmd> show port (port_id) tx_metadata
 
+show device info
+~~~~~~~~~~~~~~~~
+
+Show general information about devices probed::
+
+   testpmd> show device info (<identifier>|all)
+
+For example:
+
+.. code-block:: console
+
+    testpmd> show device info net_pcap0
+
+    ********************* Infos for device net_pcap0 *********************
+    Bus name: vdev
+    Driver name: net_pcap
+    Devargs: iface=enP2p6s0,phy_mac=1
+    Connect to socket: -1
+
+            Port id: 2
+            MAC address: 1E:37:93:28:04:B8
+            Device name: net_pcap0
+
 dump physmem
 ~~~~~~~~~~~~
 
@@ -2289,6 +2312,47 @@ hash of input [IP] packets received on port::
                      ipv6-udp-ex <string of hex digits \
                      (variable length, NIC dependent)>)
 
+Device Functions
+----------------
+
+The following sections show functions for device operations.
+
+device detach
+~~~~~~~~~~~~~
+
+Detach a device specified by pci address or virtual device args::
+
+   testpmd> device detach (identifier)
+
+Before detaching a device associated with ports, the ports should be stopped and closed.
+
+For example, to detach a pci device whose address is 0002:03:00.0.
+
+.. code-block:: console
+
+    testpmd> device detach 0002:03:00.0
+    Removing a device...
+    Port 1 is now closed
+    EAL: Releasing pci mapped resource for 0002:03:00.0
+    EAL: Calling pci_unmap_resource for 0002:03:00.0 at 0x218a050000
+    EAL: Calling pci_unmap_resource for 0002:03:00.0 at 0x218c050000
+    Device 0002:03:00.0 is detached
+    Now total ports is 1
+
+For example, to detach a port created by pcap PMD.
+
+.. code-block:: console
+
+    testpmd> device detach net_pcap0
+    Removing a device...
+    Port 0 is now closed
+    Device net_pcap0 is detached
+    Now total ports is 0
+    Done
+
+In this case, identifier is ``net_pcap0``.
+This identifier format is the same as ``--vdev`` format of DPDK applications.
+
 Link Bonding Functions
 ----------------------
 
-- 
2.8.4

Re: [dpdk-dev] [PATCH v2] app/testpmd: add device related cmds

[Ferruh Yigit]

On 7/10/2019 2:07 PM, Nithin Dabilpuram wrote:
> With the latest published interface of
> rte_eal_hotplug_[add,remove](), and rte_eth_dev_close(),
> rte_eth_dev_close() would cleanup all the data structures of
> port's eth dev leaving the device common resource intact
> if RTE_ETH_DEV_CLOSE_REMOVE is set in dev flags.
> 
> So a new command "detach device" (~hotplug remove) to work,
> with device identifier like "port attach" is added
> to be able to detach closed devices.
> 
> Also to display currently probed devices, another command
> "show device info <identifier>|all" is also added as a
> part of this change.
> 
> Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
> ---
>  app/test-pmd/cmdline.c                      | 88 +++++++++++++++++++++++++++++
>  app/test-pmd/config.c                       | 73 ++++++++++++++++++++++++
>  app/test-pmd/testpmd.c                      | 45 +++++++++++++++
>  app/test-pmd/testpmd.h                      |  2 +
>  doc/guides/testpmd_app_ug/testpmd_funcs.rst | 64 +++++++++++++++++++++
>  5 files changed, 272 insertions(+)

Can you please add new commands to help string in 'cmd_help_long_parsed()' too?

<...>

Re: [dpdk-dev] [EXT] Re: [PATCH v2] app/testpmd: add device related cmds

[Nithin Kumar Dabilpuram]

On Tue, Jul 16, 2019 at 07:30:23PM +0100, Ferruh Yigit wrote:
> External Email
> 
> ----------------------------------------------------------------------
> On 7/10/2019 2:07 PM, Nithin Dabilpuram wrote:
> > With the latest published interface of
> > rte_eal_hotplug_[add,remove](), and rte_eth_dev_close(),
> > rte_eth_dev_close() would cleanup all the data structures of
> > port's eth dev leaving the device common resource intact
> > if RTE_ETH_DEV_CLOSE_REMOVE is set in dev flags.
> > 
> > So a new command "detach device" (~hotplug remove) to work,
> > with device identifier like "port attach" is added
> > to be able to detach closed devices.
> > 
> > Also to display currently probed devices, another command
> > "show device info <identifier>|all" is also added as a
> > part of this change.
> > 
> > Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
> > ---
> >  app/test-pmd/cmdline.c                      | 88 +++++++++++++++++++++++++++++
> >  app/test-pmd/config.c                       | 73 ++++++++++++++++++++++++
> >  app/test-pmd/testpmd.c                      | 45 +++++++++++++++
> >  app/test-pmd/testpmd.h                      |  2 +
> >  doc/guides/testpmd_app_ug/testpmd_funcs.rst | 64 +++++++++++++++++++++
> >  5 files changed, 272 insertions(+)
> 
> Can you please add new commands to help string in 'cmd_help_long_parsed()' too?
>

Sorry, I missed that, will add it in next version.

> <...>
> 
> 

[dpdk-dev] [PATCH v3] app/testpmd: add device related cmds

[Nithin Dabilpuram]

With the latest published interface of
rte_eal_hotplug_[add,remove](), and rte_eth_dev_close(),
rte_eth_dev_close() would cleanup all the data structures of
port's eth dev leaving the device common resource intact
if RTE_ETH_DEV_CLOSE_REMOVE is set in dev flags.

So a new command "detach device" (~hotplug remove) to work,
with device identifier like "port attach" is added
to be able to detach closed devices.

Also to display currently probed devices, another command
"show device info <identifier>|all" is also added as a
part of this change.

Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
---

v3:
* Add help command text for new device related commands
v2:
* Add new commands "device detach" and "show device info all"
  instead of change in existing command "port detach"

 app/test-pmd/cmdline.c                      | 107 +++++++++++++++++++++++++++-
 app/test-pmd/config.c                       |  73 +++++++++++++++++++
 app/test-pmd/testpmd.c                      |  45 ++++++++++++
 app/test-pmd/testpmd.h                      |   2 +
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  64 +++++++++++++++++
 5 files changed, 289 insertions(+), 2 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 01dd45f..6b8aeed 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -101,6 +101,7 @@ static void cmd_help_brief_parsed(__attribute__((unused)) void *parsed_result,
 		"    help registers                  : Reading and setting port registers.\n"
 		"    help filters                    : Filters configuration help.\n"
 		"    help traffic_management         : Traffic Management commmands.\n"
+		"    help devices                    : Device related cmds.\n"
 		"    help all                        : All of the above sections.\n\n"
 	);
 
@@ -236,6 +237,9 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"show port (port_id) tx_metadata\n"
 			"    Show Tx metadata value set"
 			" for a specific port\n\n"
+
+			"show device info (<identifier>|all)"
+			"       Show general information about devices probed.\n\n"
 		);
 	}
 
@@ -1240,6 +1244,17 @@ static void cmd_help_long_parsed(void *parsed_result,
 		);
 	}
 
+	if (show_all || !strcmp(res->section, "devices")) {
+		cmdline_printf(
+			cl,
+			"\n"
+			"Device Operations:\n"
+			"--------------\n"
+			"device detach (identifier)\n"
+			"       Detach device by identifier.\n\n"
+		);
+	}
+
 }
 
 cmdline_parse_token_string_t cmd_help_long_help =
@@ -1248,13 +1263,13 @@ cmdline_parse_token_string_t cmd_help_long_help =
 cmdline_parse_token_string_t cmd_help_long_section =
 	TOKEN_STRING_INITIALIZER(struct cmd_help_long_result, section,
 			"all#control#display#config#"
-			"ports#registers#filters#traffic_management");
+			"ports#registers#filters#traffic_management#devices");
 
 cmdline_parse_inst_t cmd_help_long = {
 	.f = cmd_help_long_parsed,
 	.data = NULL,
 	.help_str = "help all|control|display|config|ports|register|"
-		"filters|traffic_management: "
+		"filters|traffic_management|devices: "
 		"Show help",
 	.tokens = {
 		(void *)&cmd_help_long_help,
@@ -1493,6 +1508,47 @@ cmdline_parse_inst_t cmd_operate_detach_port = {
 	},
 };
 
+/* *** detach device by identifier *** */
+struct cmd_operate_detach_device_result {
+	cmdline_fixed_string_t device;
+	cmdline_fixed_string_t keyword;
+	cmdline_fixed_string_t identifier;
+};
+
+static void cmd_operate_detach_device_parsed(void *parsed_result,
+				__attribute__((unused)) struct cmdline *cl,
+				__attribute__((unused)) void *data)
+{
+	struct cmd_operate_detach_device_result *res = parsed_result;
+
+	if (!strcmp(res->keyword, "detach"))
+		detach_device(res->identifier);
+	else
+		printf("Unknown parameter\n");
+}
+
+cmdline_parse_token_string_t cmd_operate_detach_device_device =
+	TOKEN_STRING_INITIALIZER(struct cmd_operate_detach_device_result,
+			device, "device");
+cmdline_parse_token_string_t cmd_operate_detach_device_keyword =
+	TOKEN_STRING_INITIALIZER(struct cmd_operate_detach_device_result,
+			keyword, "detach");
+cmdline_parse_token_string_t cmd_operate_detach_device_identifier =
+	TOKEN_STRING_INITIALIZER(struct cmd_operate_detach_device_result,
+			identifier, NULL);
+
+cmdline_parse_inst_t cmd_operate_detach_device = {
+	.f = cmd_operate_detach_device_parsed,
+	.data = NULL,
+	.help_str = "device detach <identifier>:"
+		"(identifier: pci address or virtual dev name)",
+	.tokens = {
+		(void *)&cmd_operate_detach_device_device,
+		(void *)&cmd_operate_detach_device_keyword,
+		(void *)&cmd_operate_detach_device_identifier,
+		NULL,
+	},
+};
 /* *** configure speed for all ports *** */
 struct cmd_config_speed_all {
 	cmdline_fixed_string_t port;
@@ -7456,6 +7512,51 @@ cmdline_parse_inst_t cmd_showport = {
 	},
 };
 
+/* *** SHOW DEVICE INFO *** */
+struct cmd_showdevice_result {
+	cmdline_fixed_string_t show;
+	cmdline_fixed_string_t device;
+	cmdline_fixed_string_t what;
+	cmdline_fixed_string_t identifier;
+};
+
+static void cmd_showdevice_parsed(void *parsed_result,
+				__attribute__((unused)) struct cmdline *cl,
+				__attribute__((unused)) void *data)
+{
+	struct cmd_showdevice_result *res = parsed_result;
+	if (!strcmp(res->what, "info")) {
+		if (!strcmp(res->identifier, "all"))
+			device_infos_display(NULL);
+		else
+			device_infos_display(res->identifier);
+	}
+}
+
+cmdline_parse_token_string_t cmd_showdevice_show =
+	TOKEN_STRING_INITIALIZER(struct cmd_showdevice_result, show,
+				 "show");
+cmdline_parse_token_string_t cmd_showdevice_device =
+	TOKEN_STRING_INITIALIZER(struct cmd_showdevice_result, device, "device");
+cmdline_parse_token_string_t cmd_showdevice_what =
+	TOKEN_STRING_INITIALIZER(struct cmd_showdevice_result, what,
+				 "info");
+cmdline_parse_token_string_t cmd_showdevice_identifier =
+	TOKEN_STRING_INITIALIZER(struct cmd_showdevice_result,
+			identifier, NULL);
+
+cmdline_parse_inst_t cmd_showdevice = {
+	.f = cmd_showdevice_parsed,
+	.data = NULL,
+	.help_str = "show device info <identifier>|all",
+	.tokens = {
+		(void *)&cmd_showdevice_show,
+		(void *)&cmd_showdevice_device,
+		(void *)&cmd_showdevice_what,
+		(void *)&cmd_showdevice_identifier,
+		NULL,
+	},
+};
 /* *** SHOW QUEUE INFO *** */
 struct cmd_showqueue_result {
 	cmdline_fixed_string_t show;
@@ -18723,6 +18824,7 @@ cmdline_parse_ctx_t main_ctx[] = {
 	(cmdline_parse_inst_t *)&cmd_showport,
 	(cmdline_parse_inst_t *)&cmd_showqueue,
 	(cmdline_parse_inst_t *)&cmd_showportall,
+	(cmdline_parse_inst_t *)&cmd_showdevice,
 	(cmdline_parse_inst_t *)&cmd_showcfg,
 	(cmdline_parse_inst_t *)&cmd_showfwdall,
 	(cmdline_parse_inst_t *)&cmd_start,
@@ -18811,6 +18913,7 @@ cmdline_parse_ctx_t main_ctx[] = {
 	(cmdline_parse_inst_t *)&cmd_operate_specific_port,
 	(cmdline_parse_inst_t *)&cmd_operate_attach_port,
 	(cmdline_parse_inst_t *)&cmd_operate_detach_port,
+	(cmdline_parse_inst_t *)&cmd_operate_detach_device,
 	(cmdline_parse_inst_t *)&cmd_set_port_setup_on,
 	(cmdline_parse_inst_t *)&cmd_config_speed_all,
 	(cmdline_parse_inst_t *)&cmd_config_speed_specific,
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 1d80470..537e73f 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -386,6 +386,79 @@ tx_queue_infos_display(portid_t port_id, uint16_t queue_id)
 	printf("\n");
 }
 
+static int bus_match_all(const struct rte_bus *bus, const void *data)
+{
+	RTE_SET_USED(bus);
+	RTE_SET_USED(data);
+	return 0;
+}
+
+void
+device_infos_display(const char *identifier)
+{
+	static const char *info_border = "*********************";
+	struct rte_bus *start = NULL, *next;
+	struct rte_dev_iterator dev_iter;
+	char name[RTE_ETH_NAME_MAX_LEN];
+	struct rte_ether_addr mac_addr;
+	struct rte_device *dev;
+	struct rte_devargs da;
+	portid_t port_id;
+	char devstr[128];
+
+	memset(&da, 0, sizeof(da));
+	if (!identifier)
+		goto skip_parse;
+
+	if (rte_devargs_parsef(&da, "%s", identifier)) {
+		printf("cannot parse identifier\n");
+		if (da.args)
+			free(da.args);
+		return;
+	}
+
+skip_parse:
+	while ((next = rte_bus_find(start, bus_match_all, NULL)) != NULL) {
+
+		start = next;
+		if (identifier && da.bus != next)
+			continue;
+
+		/* Skip buses that don't have iterate method */
+		if (!next->dev_iterate)
+			continue;
+
+		snprintf(devstr, sizeof(devstr), "bus=%s", next->name);
+		RTE_DEV_FOREACH(dev, devstr, &dev_iter) {
+
+			if (!dev->driver)
+				continue;
+			/* Check for matching device if identifier is present */
+			if (identifier &&
+			    strncmp(da.name, dev->name, strlen(dev->name)))
+				continue;
+			printf("\n%s Infos for device %s %s\n",
+			       info_border, dev->name, info_border);
+			printf("Bus name: %s", dev->bus->name);
+			printf("\nDriver name: %s", dev->driver->name);
+			printf("\nDevargs: %s",
+			       dev->devargs ? dev->devargs->args : "");
+			printf("\nConnect to socket: %d", dev->numa_node);
+			printf("\n");
+
+			/* List ports with matching device name */
+			RTE_ETH_FOREACH_DEV_OF(port_id, dev) {
+				rte_eth_macaddr_get(port_id, &mac_addr);
+				printf("\n\tPort id: %-2d", port_id);
+				print_ethaddr("\n\tMAC address: ", &mac_addr);
+				rte_eth_dev_get_name_by_port(port_id, name);
+				printf("\n\tDevice name: %s", name);
+				printf("\n");
+			}
+		}
+	};
+}
+
 void
 port_infos_display(portid_t port_id)
 {
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 3ed3523..a3b1542 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -2476,6 +2476,51 @@ detach_port_device(portid_t port_id)
 }
 
 void
+detach_device(char *identifier)
+{
+	struct rte_dev_iterator iterator;
+	struct rte_devargs da;
+	portid_t port_id;
+
+	printf("Removing a device...\n");
+
+	memset(&da, 0, sizeof(da));
+	if (rte_devargs_parsef(&da, "%s", identifier)) {
+		printf("cannot parse identifier\n");
+		if (da.args)
+			free(da.args);
+		return;
+	}
+
+	RTE_ETH_FOREACH_MATCHING_DEV(port_id, identifier, &iterator) {
+		if (ports[port_id].port_status != RTE_PORT_CLOSED) {
+			if (ports[port_id].port_status != RTE_PORT_STOPPED) {
+				printf("Port %u not stopped\n", port_id);
+				return;
+			}
+
+			/* sibling ports are forced to be closed */
+			if (ports[port_id].flow_list)
+				port_flow_flush(port_id);
+			ports[port_id].port_status = RTE_PORT_CLOSED;
+			printf("Port %u is now closed\n", port_id);
+		}
+	}
+
+	if (rte_eal_hotplug_remove(da.bus->name, da.name) != 0) {
+		TESTPMD_LOG(ERR, "Failed to detach device %s(%s)\n",
+			    da.name, da.bus->name);
+		return;
+	}
+
+	remove_invalid_ports();
+
+	printf("Device %s is detached\n", identifier);
+	printf("Now total ports is %d\n", nb_ports);
+	printf("Done\n");
+}
+
+void
 pmd_test_exit(void)
 {
 	portid_t pt_id;
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index e3a6f7c..93bed6d 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -689,6 +689,7 @@ void nic_stats_clear(portid_t port_id);
 void nic_xstats_display(portid_t port_id);
 void nic_xstats_clear(portid_t port_id);
 void nic_stats_mapping_display(portid_t port_id);
+void device_infos_display(const char *identifier);
 void port_infos_display(portid_t port_id);
 void port_summary_display(portid_t port_id);
 void port_summary_header_display(void);
@@ -788,6 +789,7 @@ void stop_port(portid_t pid);
 void close_port(portid_t pid);
 void reset_port(portid_t pid);
 void attach_port(char *identifier);
+void detach_device(char *identifier);
 void detach_port_device(portid_t port_id);
 int all_ports_stopped(void);
 int port_is_stopped(portid_t port_id);
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index ebab2f1..04b9a31 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -467,6 +467,29 @@ Show Tx metadata value set for a specific port::
 
    testpmd> show port (port_id) tx_metadata
 
+show device info
+~~~~~~~~~~~~~~~~
+
+Show general information about devices probed::
+
+   testpmd> show device info (<identifier>|all)
+
+For example:
+
+.. code-block:: console
+
+    testpmd> show device info net_pcap0
+
+    ********************* Infos for device net_pcap0 *********************
+    Bus name: vdev
+    Driver name: net_pcap
+    Devargs: iface=enP2p6s0,phy_mac=1
+    Connect to socket: -1
+
+            Port id: 2
+            MAC address: 1E:37:93:28:04:B8
+            Device name: net_pcap0
+
 dump physmem
 ~~~~~~~~~~~~
 
@@ -2289,6 +2312,47 @@ hash of input [IP] packets received on port::
                      ipv6-udp-ex <string of hex digits \
                      (variable length, NIC dependent)>)
 
+Device Functions
+----------------
+
+The following sections show functions for device operations.
+
+device detach
+~~~~~~~~~~~~~
+
+Detach a device specified by pci address or virtual device args::
+
+   testpmd> device detach (identifier)
+
+Before detaching a device associated with ports, the ports should be stopped and closed.
+
+For example, to detach a pci device whose address is 0002:03:00.0.
+
+.. code-block:: console
+
+    testpmd> device detach 0002:03:00.0
+    Removing a device...
+    Port 1 is now closed
+    EAL: Releasing pci mapped resource for 0002:03:00.0
+    EAL: Calling pci_unmap_resource for 0002:03:00.0 at 0x218a050000
+    EAL: Calling pci_unmap_resource for 0002:03:00.0 at 0x218c050000
+    Device 0002:03:00.0 is detached
+    Now total ports is 1
+
+For example, to detach a port created by pcap PMD.
+
+.. code-block:: console
+
+    testpmd> device detach net_pcap0
+    Removing a device...
+    Port 0 is now closed
+    Device net_pcap0 is detached
+    Now total ports is 0
+    Done
+
+In this case, identifier is ``net_pcap0``.
+This identifier format is the same as ``--vdev`` format of DPDK applications.
+
 Link Bonding Functions
 ----------------------
 
-- 
2.8.4

Re: [dpdk-dev] [PATCH v3] app/testpmd: add device related cmds

[Ferruh Yigit]

On 7/17/2019 1:30 PM, Nithin Dabilpuram wrote:
> With the latest published interface of
> rte_eal_hotplug_[add,remove](), and rte_eth_dev_close(),
> rte_eth_dev_close() would cleanup all the data structures of
> port's eth dev leaving the device common resource intact
> if RTE_ETH_DEV_CLOSE_REMOVE is set in dev flags.
> 
> So a new command "detach device" (~hotplug remove) to work,
> with device identifier like "port attach" is added
> to be able to detach closed devices.
> 
> Also to display currently probed devices, another command
> "show device info <identifier>|all" is also added as a
> part of this change.

In "show device info all" output [1] there are 'dpaa' & 'fslmc' logs [2], do you
know why we are getting them, can we get rid of them?




[1]
"
********************* Infos for device net_null0 *********************
Bus name: vdev
Driver name: net_null
Devargs:
Connect to socket: 0

        Port id: 1
        MAC address: 8A:D0:A1:F3:B5:0C
        Device name: net_null0
dpaa: Invalid device string ()

fslmc: Invalid device string ()
"


[2]
"
dpaa: Invalid device string ()

fslmc: Invalid device string ()
"

Re: [dpdk-dev] [PATCH v3] app/testpmd: add device related cmds

[Ferruh Yigit]

On 7/17/2019 1:30 PM, Nithin Dabilpuram wrote:
> With the latest published interface of
> rte_eal_hotplug_[add,remove](), and rte_eth_dev_close(),
> rte_eth_dev_close() would cleanup all the data structures of
> port's eth dev leaving the device common resource intact
> if RTE_ETH_DEV_CLOSE_REMOVE is set in dev flags.
> 
> So a new command "detach device" (~hotplug remove) to work,
> with device identifier like "port attach" is added
> to be able to detach closed devices.
> 
> Also to display currently probed devices, another command
> "show device info <identifier>|all" is also added as a
> part of this change.
> 
> Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>

Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

Applied to dpdk-next-net/master, thanks.

Re: [dpdk-dev] [EXT] Re: [PATCH v3] app/testpmd: add device related cmds

[Nithin Kumar Dabilpuram]

Hi Ferruh,

On 7/17/2019 10:21 PM, Ferruh Yigit wrote:
> External Email
>
> ----------------------------------------------------------------------
> On 7/17/2019 1:30 PM, Nithin Dabilpuram wrote:
>> With the latest published interface of
>> rte_eal_hotplug_[add,remove](), and rte_eth_dev_close(),
>> rte_eth_dev_close() would cleanup all the data structures of
>> port's eth dev leaving the device common resource intact
>> if RTE_ETH_DEV_CLOSE_REMOVE is set in dev flags.
>>
>> So a new command "detach device" (~hotplug remove) to work,
>> with device identifier like "port attach" is added
>> to be able to detach closed devices.
>>
>> Also to display currently probed devices, another command
>> "show device info <identifier>|all" is also added as a
>> part of this change.
> In "show device info all" output [1] there are 'dpaa' & 'fslmc' logs [2], do you
> know why we are getting them, can we get rid of them?
>
Is this issue sill there ?  I'm not able to see it in my setup.

>
>
> [1]
> "
> ********************* Infos for device net_null0 *********************
> Bus name: vdev
> Driver name: net_null
> Devargs:
> Connect to socket: 0
>
>          Port id: 1
>          MAC address: 8A:D0:A1:F3:B5:0C
>          Device name: net_null0
> dpaa: Invalid device string ()
>
> fslmc: Invalid device string ()
> "
>
>
> [2]
> "
> dpaa: Invalid device string ()
>
> fslmc: Invalid device string ()
> "

Re: [dpdk-dev] [EXT] Re: [PATCH v3] app/testpmd: add device related cmds

[Ferruh Yigit]

On 7/18/2019 6:27 AM, Nithin Kumar Dabilpuram wrote:
> Hi Ferruh,
> 
> On 7/17/2019 10:21 PM, Ferruh Yigit wrote:
>> External Email
>>
>> ----------------------------------------------------------------------
>> On 7/17/2019 1:30 PM, Nithin Dabilpuram wrote:
>>> With the latest published interface of
>>> rte_eal_hotplug_[add,remove](), and rte_eth_dev_close(),
>>> rte_eth_dev_close() would cleanup all the data structures of
>>> port's eth dev leaving the device common resource intact
>>> if RTE_ETH_DEV_CLOSE_REMOVE is set in dev flags.
>>>
>>> So a new command "detach device" (~hotplug remove) to work,
>>> with device identifier like "port attach" is added
>>> to be able to detach closed devices.
>>>
>>> Also to display currently probed devices, another command
>>> "show device info <identifier>|all" is also added as a
>>> part of this change.
>> In "show device info all" output [1] there are 'dpaa' & 'fslmc' logs [2], do you
>> know why we are getting them, can we get rid of them?
>>
> Is this issue sill there ?  I'm not able to see it in my setup.

Yes, still there. I can see them with latest next-net.

> 
>>
>>
>> [1]
>> "
>> ********************* Infos for device net_null0 *********************
>> Bus name: vdev
>> Driver name: net_null
>> Devargs:
>> Connect to socket: 0
>>
>>          Port id: 1
>>          MAC address: 8A:D0:A1:F3:B5:0C
>>          Device name: net_null0
>> dpaa: Invalid device string ()
>>
>> fslmc: Invalid device string ()
>> "
>>
>>
>> [2]
>> "
>> dpaa: Invalid device string ()
>>
>> fslmc: Invalid device string ()
>> "

Re: [dpdk-dev] [EXT] Re: [PATCH v3] app/testpmd: add device related cmds

[Hemant Agrawal]

HI,
> On 7/18/2019 6:27 AM, Nithin Kumar Dabilpuram wrote:
> > Hi Ferruh,
> >
> > On 7/17/2019 10:21 PM, Ferruh Yigit wrote:
> >> External Email
> >>
> >> ---------------------------------------------------------------------
> >> - On 7/17/2019 1:30 PM, Nithin Dabilpuram wrote:
> >>> With the latest published interface of
> >>> rte_eal_hotplug_[add,remove](), and rte_eth_dev_close(),
> >>> rte_eth_dev_close() would cleanup all the data structures of port's
> >>> eth dev leaving the device common resource intact if
> >>> RTE_ETH_DEV_CLOSE_REMOVE is set in dev flags.
> >>>
> >>> So a new command "detach device" (~hotplug remove) to work, with
> >>> device identifier like "port attach" is added to be able to detach
> >>> closed devices.
> >>>
> >>> Also to display currently probed devices, another command "show
> >>> device info <identifier>|all" is also added as a part of this
> >>> change.
> >> In "show device info all" output [1] there are 'dpaa' & 'fslmc' logs
> >> [2], do you know why we are getting them, can we get rid of them?
> >>
> > Is this issue sill there ?  I'm not able to see it in my setup.
> 
> Yes, still there. I can see them with latest next-net.
> 
[Hemant] Patch is on the way. 

Re: [dpdk-dev] [EXT] Re: [PATCH v3] app/testpmd: add device related cmds

[Nithin Kumar Dabilpuram]

On 7/22/2019 11:31 AM, Hemant Agrawal wrote:
> HI,
>> On 7/18/2019 6:27 AM, Nithin Kumar Dabilpuram wrote:
>>> Hi Ferruh,
>>>
>>> On 7/17/2019 10:21 PM, Ferruh Yigit wrote:
>>>> External Email
>>>>
>>>> ---------------------------------------------------------------------
>>>> - On 7/17/2019 1:30 PM, Nithin Dabilpuram wrote:
>>>>> With the latest published interface of
>>>>> rte_eal_hotplug_[add,remove](), and rte_eth_dev_close(),
>>>>> rte_eth_dev_close() would cleanup all the data structures of port's
>>>>> eth dev leaving the device common resource intact if
>>>>> RTE_ETH_DEV_CLOSE_REMOVE is set in dev flags.
>>>>>
>>>>> So a new command "detach device" (~hotplug remove) to work, with
>>>>> device identifier like "port attach" is added to be able to detach
>>>>> closed devices.
>>>>>
>>>>> Also to display currently probed devices, another command "show
>>>>> device info <identifier>|all" is also added as a part of this
>>>>> change.
>>>> In "show device info all" output [1] there are 'dpaa' & 'fslmc' logs
>>>> [2], do you know why we are getting them, can we get rid of them?
>>>>
>>> Is this issue sill there ?  I'm not able to see it in my setup.
>> Yes, still there. I can see them with latest next-net.
>>
> [Hemant] Patch is on the way.
Thanks.

Re: [dpdk-dev] [EXT] Re: [PATCH v3] app/testpmd: add device related cmds

[Ferruh Yigit]

On 7/22/2019 7:15 AM, Nithin Kumar Dabilpuram wrote:
> 
> On 7/22/2019 11:31 AM, Hemant Agrawal wrote:
>> HI,
>>> On 7/18/2019 6:27 AM, Nithin Kumar Dabilpuram wrote:
>>>> Hi Ferruh,
>>>>
>>>> On 7/17/2019 10:21 PM, Ferruh Yigit wrote:
>>>>> External Email
>>>>>
>>>>> ---------------------------------------------------------------------
>>>>> - On 7/17/2019 1:30 PM, Nithin Dabilpuram wrote:
>>>>>> With the latest published interface of
>>>>>> rte_eal_hotplug_[add,remove](), and rte_eth_dev_close(),
>>>>>> rte_eth_dev_close() would cleanup all the data structures of port's
>>>>>> eth dev leaving the device common resource intact if
>>>>>> RTE_ETH_DEV_CLOSE_REMOVE is set in dev flags.
>>>>>>
>>>>>> So a new command "detach device" (~hotplug remove) to work, with
>>>>>> device identifier like "port attach" is added to be able to detach
>>>>>> closed devices.
>>>>>>
>>>>>> Also to display currently probed devices, another command "show
>>>>>> device info <identifier>|all" is also added as a part of this
>>>>>> change.
>>>>> In "show device info all" output [1] there are 'dpaa' & 'fslmc' logs
>>>>> [2], do you know why we are getting them, can we get rid of them?
>>>>>
>>>> Is this issue sill there ?  I'm not able to see it in my setup.
>>> Yes, still there. I can see them with latest next-net.
>>>
>> [Hemant] Patch is on the way.
> Thanks.
> 

Hi Hemant, I confirm logs are gone (unless debug level enabled), thanks.