28/08/2017 12:30, Ferruh Yigit:

On 8/28/2017 10:55 AM, Gaëtan Rivet wrote:

quoted

Hi Raslan,

Redoing the review with the remarks from Thomas in mind.

On Wed, Aug 23, 2017 at 05:37:04PM +0300, Raslan Darawsheh wrote:

quoted

Added hotplug in testpmd, to be able to test hotplug function
in the PMD's.

Signed-off-by: Raslan Darawsheh <redacted>
---
 app/test-pmd/cmdline.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
 app/test-pmd/testpmd.c | 18 ++++++++++++++++++
 app/test-pmd/testpmd.h |  1 +
 3 files changed, 63 insertions(+)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index cd8c358..b32a368 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c

@@ -716,6 +716,9 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"port config (port_id|all) l2-tunnel E-tag"
 			" (enable|disable)\n"
 			"    Enable/disable the E-tag support.\n\n"
+
+			" device remove (device)\n"

I think (device) might be hard to understand for a user.
Maybe (device name)?

I am suspicious on adding new root level command to testpmd, it is
getting bigger, each command looks OK on its own context, but as a whole
app getting more confusing [1].

We can keep the root level "port" if we make clear that the impact
is beyond the port.

Since dealing with device is kind of new, it can be OK to create new
command tree, but there are already hotplug commands per port:
"port attach #PCI|#VDEV_NAME"
"port detach #P"

perhaps it can be good to keep "attach", "detach" keywords for device to
be consistent?

Not sure.
Anyway ethdev attach and detach should be deprecated.
We are moving to a more correct design where hotplug is done at EAL level.

"device attach #name"
"device detach #name"

Also a show equivalent can be added to work in device level:
"show device info"


[1]
http://dpdk.org/ml/archives/dev/2017-August/072764.html