DPDK patches and discussions
 help / color / Atom feed
* [dpdk-dev] [PATCH] app/testpmd: add port check before manual detach
@ 2020-02-13 14:50 Thomas Monjalon
  2020-02-13 15:15 ` Ferruh Yigit
  2020-02-13 18:25 ` Ferruh Yigit
  0 siblings, 2 replies; 3+ messages in thread
From: Thomas Monjalon @ 2020-02-13 14:50 UTC (permalink / raw)
  To: dev; +Cc: matan, ferruh.yigit, Wenzhuo Lu, Jingjing Wu, Bernard Iremonger

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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [dpdk-dev] [PATCH] app/testpmd: add port check before manual detach
  2020-02-13 14:50 [dpdk-dev] [PATCH] app/testpmd: add port check before manual detach Thomas Monjalon
@ 2020-02-13 15:15 ` Ferruh Yigit
  2020-02-13 18:25 ` Ferruh Yigit
  1 sibling, 0 replies; 3+ messages in thread
From: Ferruh Yigit @ 2020-02-13 15:15 UTC (permalink / raw)
  To: Thomas Monjalon, dev; +Cc: matan, Wenzhuo Lu, Jingjing Wu, Bernard Iremonger

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 =
> 


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [dpdk-dev] [PATCH] app/testpmd: add port check before manual detach
  2020-02-13 14:50 [dpdk-dev] [PATCH] app/testpmd: add port check before manual detach Thomas Monjalon
  2020-02-13 15:15 ` Ferruh Yigit
@ 2020-02-13 18:25 ` Ferruh Yigit
  1 sibling, 0 replies; 3+ messages in thread
From: Ferruh Yigit @ 2020-02-13 18:25 UTC (permalink / raw)
  To: Thomas Monjalon, dev; +Cc: matan, Wenzhuo Lu, Jingjing Wu, Bernard Iremonger

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.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-13 14:50 [dpdk-dev] [PATCH] app/testpmd: add port check before manual detach Thomas Monjalon
2020-02-13 15:15 ` Ferruh Yigit
2020-02-13 18:25 ` Ferruh Yigit

DPDK patches and discussions

Archives are clonable:
	git clone --mirror http://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ http://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev


Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/ public-inbox