[dpdk-dev] [PATCH] app/testpmd: add port check before manual detach

[dpdk-dev] [PATCH] app/testpmd: add port check before manual detach

[Thomas Monjalon]

User may try to run "port detach <port_id>"
for an already detached device.
It has been decided to protect from such usage in testpmd,
so a check was added to detach_port_device() in DPDK 19.11.
This check might be removed to allow hotplug path detaching
the device of a closed port.
Whatever will be decided in future, this check is also added
before the call to detach_port_device().

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 app/test-pmd/cmdline.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index de7a695d74..99e4168103 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -1497,10 +1497,12 @@ static void cmd_operate_detach_port_parsed(void *parsed_result,
 {
 	struct cmd_operate_detach_port_result *res = parsed_result;
 
-	if (!strcmp(res->keyword, "detach"))
+	if (!strcmp(res->keyword, "detach")) {
+		RTE_ETH_VALID_PORTID_OR_RET(res->port_id);
 		detach_port_device(res->port_id);
-	else
+	} else {
 		printf("Unknown parameter\n");
+	}
 }
 
 cmdline_parse_token_string_t cmd_operate_detach_port_port =
-- 
2.25.0

Re: [dpdk-dev] [PATCH] app/testpmd: add port check before manual detach

[Ferruh Yigit]

On 2/13/2020 2:50 PM, Thomas Monjalon wrote:
> User may try to run "port detach <port_id>"
> for an already detached device.
> It has been decided to protect from such usage in testpmd,
> so a check was added to detach_port_device() in DPDK 19.11.
> This check might be removed to allow hotplug path detaching
> the device of a closed port.
> Whatever will be decided in future, this check is also added
> before the call to detach_port_device().
> 
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
>  app/test-pmd/cmdline.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index de7a695d74..99e4168103 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -1497,10 +1497,12 @@ static void cmd_operate_detach_port_parsed(void *parsed_result,
>  {
>  	struct cmd_operate_detach_port_result *res = parsed_result;
>  
> -	if (!strcmp(res->keyword, "detach"))
> +	if (!strcmp(res->keyword, "detach")) {
> +		RTE_ETH_VALID_PORTID_OR_RET(res->port_id);

What do you think adding a comment to say this is a workaround for:
a) closed() ports can be re-used
b) fix problem in double detach when pointers are not cleared properly in the
first pass
and this workaround should be removed when above fixed?

To give some hint to the future us why we are preventing closed ports to be
detached.

>  		detach_port_device(res->port_id);
> -	else
> +	} else {
>  		printf("Unknown parameter\n");
> +	}
>  }
>  
>  cmdline_parse_token_string_t cmd_operate_detach_port_port =
> 

Re: [dpdk-dev] [PATCH] app/testpmd: add port check before manual detach

[Ferruh Yigit]

On 2/13/2020 2:50 PM, Thomas Monjalon wrote:
> User may try to run "port detach <port_id>"
> for an already detached device.
> It has been decided to protect from such usage in testpmd,
> so a check was added to detach_port_device() in DPDK 19.11.
> This check might be removed to allow hotplug path detaching
> the device of a closed port.
> Whatever will be decided in future, this check is also added
> before the call to detach_port_device().
> 
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>

Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
Applied to dpdk-next-net/master, thanks.