diff mbox series

app/testpmd: add port check before manual detach

Message ID 20200213145024.1022480-1-thomas@monjalon.net (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers
Series app/testpmd: add port check before manual detach |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-testing success Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/travis-robot success Travis build: passed
ci/Intel-compilation fail apply issues

Commit Message

Thomas Monjalon Feb. 13, 2020, 2:50 p.m. UTC 
  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(-)

Comments

Ferruh Yigit Feb. 13, 2020, 3:15 p.m. UTC | #1 
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 =
>
Ferruh Yigit Feb. 13, 2020, 6:25 p.m. UTC | #2 
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.
diff mbox series

Patch

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 =