[dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d

[dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d

[Jasvinder Singh]

Fix testpmd app exit by pressing ctrl+d, End-of-Transmission character (EOT)
on the empty command line.
 
Fixes: 0ad778b398c6 ("app/testpmd: rework softnic forward mode")

Reported-by: Mordechay Haimovsky <motih@mellanox.com>
Signed-off-by: Jasvinder Singh <jasvinder.singh@intel.com>
---
 app/test-pmd/cmdline.c       | 10 ++++++----
 lib/librte_cmdline/cmdline.c |  2 +-
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 5885289..edaf0e6 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -17581,12 +17581,14 @@ prompt(void)
 
 	for (;;) {
 		status = cmdline_poll(testpmd_cl);
-		if (status < 0)
-			rte_panic("CLI poll error (%" PRId32 ")\n", status);
-		else if (status == RDLINE_EXITED) {
+		if (status == RDLINE_EXITED || status == RDLINE_RES_EOF) {
+			if (status == RDLINE_RES_EOF)
+				pmd_test_exit();
+
 			cmdline_stdin_exit(testpmd_cl);
 			rte_exit(0, "\n");
-		}
+		} else if (status < 0)
+			rte_panic("CLI poll error (%" PRId32 ")\n", status);
 
 #if defined RTE_LIBRTE_PMD_SOFTNIC
 
diff --git a/lib/librte_cmdline/cmdline.c b/lib/librte_cmdline/cmdline.c
index 591b78b..47b0f6a 100644
--- a/lib/librte_cmdline/cmdline.c
+++ b/lib/librte_cmdline/cmdline.c
@@ -187,7 +187,7 @@ cmdline_in(struct cmdline *cl, const char *buf, int size)
 			rdline_newline(&cl->rdl, cl->prompt);
 		}
 		else if (ret == RDLINE_RES_EOF)
-			return -1;
+			return RDLINE_RES_EOF;
 		else if (ret == RDLINE_RES_EXITED)
 			return -1;
 	}
-- 
2.9.3

Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d

[Iremonger, Bernard]

Hi Jasvider,

> -----Original Message-----
> From: Singh, Jasvinder
> Sent: Monday, July 23, 2018 11:44 AM
> To: dev@dpdk.org
> Cc: Iremonger, Bernard <bernard.iremonger@intel.com>; Pattan, Reshma
> <reshma.pattan@intel.com>
> Subject: [PATCH] app/testpmd: fix testpmd exit using ctrl+d
> 
> Fix testpmd app exit by pressing ctrl+d, End-of-Transmission character (EOT)
> on the empty command line.
> 
> Fixes: 0ad778b398c6 ("app/testpmd: rework softnic forward mode")
> 
> Reported-by: Mordechay Haimovsky <motih@mellanox.com>
> Signed-off-by: Jasvinder Singh <jasvinder.singh@intel.com>
> ---
>  app/test-pmd/cmdline.c       | 10 ++++++----
>  lib/librte_cmdline/cmdline.c |  2 +-
>  2 files changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
> 5885289..edaf0e6 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -17581,12 +17581,14 @@ prompt(void)
> 
>  	for (;;) {
>  		status = cmdline_poll(testpmd_cl);
> -		if (status < 0)
> -			rte_panic("CLI poll error (%" PRId32 ")\n", status);
> -		else if (status == RDLINE_EXITED) {
> +		if (status == RDLINE_EXITED || status == RDLINE_RES_EOF) {
> +			if (status == RDLINE_RES_EOF)
> +				pmd_test_exit();
> +
>  			cmdline_stdin_exit(testpmd_cl);
>  			rte_exit(0, "\n");
> -		}
> +		} else if (status < 0)
> +			rte_panic("CLI poll error (%" PRId32 ")\n", status);
> 
>  #if defined RTE_LIBRTE_PMD_SOFTNIC
> 
> diff --git a/lib/librte_cmdline/cmdline.c b/lib/librte_cmdline/cmdline.c index
> 591b78b..47b0f6a 100644
> --- a/lib/librte_cmdline/cmdline.c
> +++ b/lib/librte_cmdline/cmdline.c
> @@ -187,7 +187,7 @@ cmdline_in(struct cmdline *cl, const char *buf, int
> size)
>  			rdline_newline(&cl->rdl, cl->prompt);
>  		}
>  		else if (ret == RDLINE_RES_EOF)
> -			return -1;
> +			return RDLINE_RES_EOF;
>  		else if (ret == RDLINE_RES_EXITED)
>  			return -1;
>  	}
> --
> 2.9.3
The check-git-log script is showing the following error:

./devtools/check-git-log.sh -1
Line too long:
        Fix testpmd app exit by pressing ctrl+d, End-of-Transmission character (EOT)

Regards,

Bernard.

Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d

[Thomas Monjalon]

23/07/2018 12:44, Jasvinder Singh:
> Fix testpmd app exit by pressing ctrl+d, End-of-Transmission character (EOT)
> on the empty command line.

Please describe what is the issue and what is the cause.

> Fixes: 0ad778b398c6 ("app/testpmd: rework softnic forward mode")
> 
> Reported-by: Mordechay Haimovsky <motih@mellanox.com>
> Signed-off-by: Jasvinder Singh <jasvinder.singh@intel.com>
> ---
>  app/test-pmd/cmdline.c       | 10 ++++++----
>  lib/librte_cmdline/cmdline.c |  2 +-

It is very suspicious to change the cmdline library.
If there is a bug in this library, it deserves a separate fix.

Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d

[Thomas Monjalon]

Important note:
	testpmd is currently really broken.
	We cannot have a RC2 until it is fixed.


24/07/2018 13:25, Thomas Monjalon:
> 23/07/2018 12:44, Jasvinder Singh:
> > Fix testpmd app exit by pressing ctrl+d, End-of-Transmission character (EOT)
> > on the empty command line.
> 
> Please describe what is the issue and what is the cause.
> 
> > Fixes: 0ad778b398c6 ("app/testpmd: rework softnic forward mode")
> > 
> > Reported-by: Mordechay Haimovsky <motih@mellanox.com>
> > Signed-off-by: Jasvinder Singh <jasvinder.singh@intel.com>
> > ---
> >  app/test-pmd/cmdline.c       | 10 ++++++----
> >  lib/librte_cmdline/cmdline.c |  2 +-
> 
> It is very suspicious to change the cmdline library.
> If there is a bug in this library, it deserves a separate fix.

Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d

[Pattan, Reshma]

Hi,

Jasvinder is OOO, let me have a look and send the next version of patch.

Thanks,
Reshma

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Tuesday, July 24, 2018 1:34 PM
> To: Singh, Jasvinder <jasvinder.singh@intel.com>
> Cc: dev@dpdk.org; Iremonger, Bernard <bernard.iremonger@intel.com>;
> Pattan, Reshma <reshma.pattan@intel.com>; motih@mellanox.com;
> olivier.matz@6wind.com; Dumitrescu, Cristian
> <cristian.dumitrescu@intel.com>
> Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d
> 
> Important note:
> 	testpmd is currently really broken.
> 	We cannot have a RC2 until it is fixed.
> 
> 
> 24/07/2018 13:25, Thomas Monjalon:
> > 23/07/2018 12:44, Jasvinder Singh:
> > > Fix testpmd app exit by pressing ctrl+d, End-of-Transmission
> > > character (EOT) on the empty command line.
> >
> > Please describe what is the issue and what is the cause.
> >
> > > Fixes: 0ad778b398c6 ("app/testpmd: rework softnic forward mode")
> > >
> > > Reported-by: Mordechay Haimovsky <motih@mellanox.com>
> > > Signed-off-by: Jasvinder Singh <jasvinder.singh@intel.com>
> > > ---
> > >  app/test-pmd/cmdline.c       | 10 ++++++----
> > >  lib/librte_cmdline/cmdline.c |  2 +-
> >
> > It is very suspicious to change the cmdline library.
> > If there is a bug in this library, it deserves a separate fix.
> 
> 

Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d

[Iremonger, Bernard]

Hi Thomas,

+John

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Tuesday, July 24, 2018 1:34 PM
> To: Singh, Jasvinder <jasvinder.singh@intel.com>
> Cc: dev@dpdk.org; Iremonger, Bernard <bernard.iremonger@intel.com>;
> Pattan, Reshma <reshma.pattan@intel.com>; motih@mellanox.com;
> olivier.matz@6wind.com; Dumitrescu, Cristian
> <cristian.dumitrescu@intel.com>
> Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d
> 
> Important note:
> 	testpmd is currently really broken.
> 	We cannot have a RC2 until it is fixed.
> 
> 
> 24/07/2018 13:25, Thomas Monjalon:
> > 23/07/2018 12:44, Jasvinder Singh:
> > > Fix testpmd app exit by pressing ctrl+d, End-of-Transmission
> > > character (EOT) on the empty command line.
> >
> > Please describe what is the issue and what is the cause.
> >
> > > Fixes: 0ad778b398c6 ("app/testpmd: rework softnic forward mode")
> > >
> > > Reported-by: Mordechay Haimovsky <motih@mellanox.com>
> > > Signed-off-by: Jasvinder Singh <jasvinder.singh@intel.com>
> > > ---
> > >  app/test-pmd/cmdline.c       | 10 ++++++----
> > >  lib/librte_cmdline/cmdline.c |  2 +-
> >
> > It is very suspicious to change the cmdline library.
> > If there is a bug in this library, it deserves a separate fix.
> 
> 

Do you know what commit has broken testpmd?

Regards,

Bernard

Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d

[Thomas Monjalon]

24/07/2018 14:41, Iremonger, Bernard:
> Hi Thomas,
> 
> +John
> 
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > 
> > Important note:
> > 	testpmd is currently really broken.
> > 	We cannot have a RC2 until it is fixed.
> > 
> > 
> > 24/07/2018 13:25, Thomas Monjalon:
> > > 23/07/2018 12:44, Jasvinder Singh:
> > > > Fix testpmd app exit by pressing ctrl+d, End-of-Transmission
> > > > character (EOT) on the empty command line.
> > >
> > > Please describe what is the issue and what is the cause.
> > >
> > > > Fixes: 0ad778b398c6 ("app/testpmd: rework softnic forward mode")
> > > >
> > > > Reported-by: Mordechay Haimovsky <motih@mellanox.com>
> > > > Signed-off-by: Jasvinder Singh <jasvinder.singh@intel.com>
> > > > ---
> > > >  app/test-pmd/cmdline.c       | 10 ++++++----
> > > >  lib/librte_cmdline/cmdline.c |  2 +-
> > >
> > > It is very suspicious to change the cmdline library.
> > > If there is a bug in this library, it deserves a separate fix.
> > 
> > 
> 
> Do you know what commit has broken testpmd?

I think the "Fixes" line above is right:
	0ad778b398c6 ("app/testpmd: rework softnic forward mode")

[dpdk-dev] [PATCH v2] app/testpmd: fix testpmd exit for ctrl+d

[Reshma Pattan]

Testpmd should be existed gracefully when ctrl+d is typed.
This behaviour is not handled properly, fix this by calling
pmd_test_exit() instead of rte_panic.

Fixes: 0ad778b398c6 ("app/testpmd: rework softnic forward mode")

Reported-by: Mordechay Haimovsky <motih@mellanox.com>
Signed-off-by: Jasvinder Singh <jasvinder.singh@intel.com>
Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
---
v2: removed changes done to lib/librte_cmdline/cmdline.c
reworded commit message and heading.
---
 app/test-pmd/cmdline.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 588528928..406008d73 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -17581,9 +17581,9 @@ prompt(void)
 
 	for (;;) {
 		status = cmdline_poll(testpmd_cl);
-		if (status < 0)
-			rte_panic("CLI poll error (%" PRId32 ")\n", status);
-		else if (status == RDLINE_EXITED) {
+		if (status == RDLINE_EXITED || status == -1) {
+			if (status == -1)
+				pmd_test_exit();
 			cmdline_stdin_exit(testpmd_cl);
 			rte_exit(0, "\n");
 		}
-- 
2.14.4

Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d

[Mordechay Haimovsky]

Even after this fix we still have setups that use netvsc  for example,  on which testpmd exits with rte_panic right after loading it even without touching the KBD.

I recommend returning the previous prompt routine in test-pmd/cmdline.c
and rework the SOFTNIC section there, preferably moving its poll section to use rte_service in a separate file cleaning the CLI files from PMD-specific implementation.

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Tuesday, July 24, 2018 3:34 PM
> To: Jasvinder Singh <jasvinder.singh@intel.com>
> Cc: dev@dpdk.org; bernard.iremonger@intel.com;
> reshma.pattan@intel.com; Mordechay Haimovsky <motih@mellanox.com>;
> olivier.matz@6wind.com; cristian.dumitrescu@intel.com
> Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d
> 
> Important note:
> 	testpmd is currently really broken.
> 	We cannot have a RC2 until it is fixed.
> 
> 
> 24/07/2018 13:25, Thomas Monjalon:
> > 23/07/2018 12:44, Jasvinder Singh:
> > > Fix testpmd app exit by pressing ctrl+d, End-of-Transmission
> > > character (EOT) on the empty command line.
> >
> > Please describe what is the issue and what is the cause.
> >
> > > Fixes: 0ad778b398c6 ("app/testpmd: rework softnic forward mode")
> > >
> > > Reported-by: Mordechay Haimovsky <motih@mellanox.com>
> > > Signed-off-by: Jasvinder Singh <jasvinder.singh@intel.com>
> > > ---
> > >  app/test-pmd/cmdline.c       | 10 ++++++----
> > >  lib/librte_cmdline/cmdline.c |  2 +-
> >
> > It is very suspicious to change the cmdline library.
> > If there is a bug in this library, it deserves a separate fix.
> 
> 

Re: [dpdk-dev] [PATCH v2] app/testpmd: fix testpmd exit for ctrl+d

[Pattan, Reshma]

+ CC: Olivier and Mordechay Haimovsky. 

> -----Original Message-----
> From: Pattan, Reshma
> Sent: Tuesday, July 24, 2018 3:17 PM
> To: dev@dpdk.org
> Cc: Iremonger, Bernard <bernard.iremonger@intel.com>;
> thomas@monjalon.net; Singh, Jasvinder <jasvinder.singh@intel.com>;
> Pattan, Reshma <reshma.pattan@intel.com>
> Subject: [PATCH v2] app/testpmd: fix testpmd exit for ctrl+d
> 
> Testpmd should be existed gracefully when ctrl+d is typed.
> This behaviour is not handled properly, fix this by calling
> pmd_test_exit() instead of rte_panic.
> 
> Fixes: 0ad778b398c6 ("app/testpmd: rework softnic forward mode")
> 
> Reported-by: Mordechay Haimovsky <motih@mellanox.com>
> Signed-off-by: Jasvinder Singh <jasvinder.singh@intel.com>
> Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
> ---
> v2: removed changes done to lib/librte_cmdline/cmdline.c reworded commit
> message and heading.
> ---
>  app/test-pmd/cmdline.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
> 588528928..406008d73 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -17581,9 +17581,9 @@ prompt(void)
> 
>  	for (;;) {
>  		status = cmdline_poll(testpmd_cl);
> -		if (status < 0)
> -			rte_panic("CLI poll error (%" PRId32 ")\n", status);
> -		else if (status == RDLINE_EXITED) {
> +		if (status == RDLINE_EXITED || status == -1) {
> +			if (status == -1)
> +				pmd_test_exit();
>  			cmdline_stdin_exit(testpmd_cl);
>  			rte_exit(0, "\n");
>  		}
> --
> 2.14.4

Re: [dpdk-dev] [PATCH v2] app/testpmd: fix testpmd exit for ctrl+d

[Pattan, Reshma]

Hi ,

Can you check if v2 version is working ? And it would be great if you add Tested-by: if test passes.

Else, please let me know the steps to reproduce the issue.

Thanks,
Reshma

Re: [dpdk-dev] [PATCH v2] app/testpmd: fix testpmd exit for ctrl+d

[Iremonger, Bernard]

Hi Reshma,

> -----Original Message-----
> From: Pattan, Reshma
> Sent: Tuesday, July 24, 2018 3:47 PM
> To: dev@dpdk.org
> Cc: Iremonger, Bernard <bernard.iremonger@intel.com>;
> thomas@monjalon.net; Singh, Jasvinder <jasvinder.singh@intel.com>;
> olivier.matz@6wind.com; Mordechay Haimovsky <motih@mellanox.com>
> Subject: RE: [PATCH v2] app/testpmd: fix testpmd exit for ctrl+d
> 
> + CC: Olivier and Mordechay Haimovsky.
> 
> > -----Original Message-----
> > From: Pattan, Reshma
> > Sent: Tuesday, July 24, 2018 3:17 PM
> > To: dev@dpdk.org
> > Cc: Iremonger, Bernard <bernard.iremonger@intel.com>;
> > thomas@monjalon.net; Singh, Jasvinder <jasvinder.singh@intel.com>;
> > Pattan, Reshma <reshma.pattan@intel.com>
> > Subject: [PATCH v2] app/testpmd: fix testpmd exit for ctrl+d
> >
> > Testpmd should be existed gracefully when ctrl+d is typed.

Typo in commit message:
"should be existed" should be "should exit"

> > This behaviour is not handled properly, fix this by calling
> > pmd_test_exit() instead of rte_panic.
> >
> > Fixes: 0ad778b398c6 ("app/testpmd: rework softnic forward mode")
> >
> > Reported-by: Mordechay Haimovsky <motih@mellanox.com>
> > Signed-off-by: Jasvinder Singh <jasvinder.singh@intel.com>
> > Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
> > ---
> > v2: removed changes done to lib/librte_cmdline/cmdline.c reworded
> > commit message and heading.
> > ---
> >  app/test-pmd/cmdline.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
> > 588528928..406008d73 100644
> > --- a/app/test-pmd/cmdline.c
> > +++ b/app/test-pmd/cmdline.c
> > @@ -17581,9 +17581,9 @@ prompt(void)
> >
> >  	for (;;) {
> >  		status = cmdline_poll(testpmd_cl);
> > -		if (status < 0)
> > -			rte_panic("CLI poll error (%" PRId32 ")\n", status);
> > -		else if (status == RDLINE_EXITED) {
> > +		if (status == RDLINE_EXITED || status == -1) {
> > +			if (status == -1)
> > +				pmd_test_exit();
> >  			cmdline_stdin_exit(testpmd_cl);
> >  			rte_exit(0, "\n");
> >  		}
> > --
> > 2.14.4

Regards,

Bernard.

Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d

[Dumitrescu, Cristian]

> -----Original Message-----
> From: Mordechay Haimovsky [mailto:motih@mellanox.com]
> Sent: Tuesday, July 24, 2018 3:37 PM
> To: Thomas Monjalon <thomas@monjalon.net>; Singh, Jasvinder
> <jasvinder.singh@intel.com>
> Cc: dev@dpdk.org; Iremonger, Bernard <bernard.iremonger@intel.com>;
> Pattan, Reshma <reshma.pattan@intel.com>; olivier.matz@6wind.com;
> Dumitrescu, Cristian <cristian.dumitrescu@intel.com>
> Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d
> 
> Even after this fix we still have setups that use netvsc  for example,  on
> which testpmd exits with rte_panic right after loading it even without
> touching the KBD.
> 
> I recommend returning the previous prompt routine in test-pmd/cmdline.c
> and rework the SOFTNIC section there, preferably moving its poll section to
> use rte_service in a separate file cleaning the CLI files from PMD-specific
> implementation.
> 
> > -----Original Message-----
> > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > Sent: Tuesday, July 24, 2018 3:34 PM
> > To: Jasvinder Singh <jasvinder.singh@intel.com>
> > Cc: dev@dpdk.org; bernard.iremonger@intel.com;
> > reshma.pattan@intel.com; Mordechay Haimovsky
> <motih@mellanox.com>;
> > olivier.matz@6wind.com; cristian.dumitrescu@intel.com
> > Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d
> >
> > Important note:
> > 	testpmd is currently really broken.
> > 	We cannot have a RC2 until it is fixed.
> >
> >
> > 24/07/2018 13:25, Thomas Monjalon:
> > > 23/07/2018 12:44, Jasvinder Singh:
> > > > Fix testpmd app exit by pressing ctrl+d, End-of-Transmission
> > > > character (EOT) on the empty command line.
> > >
> > > Please describe what is the issue and what is the cause.
> > >
> > > > Fixes: 0ad778b398c6 ("app/testpmd: rework softnic forward mode")
> > > >
> > > > Reported-by: Mordechay Haimovsky <motih@mellanox.com>
> > > > Signed-off-by: Jasvinder Singh <jasvinder.singh@intel.com>
> > > > ---
> > > >  app/test-pmd/cmdline.c       | 10 ++++++----
> > > >  lib/librte_cmdline/cmdline.c |  2 +-
> > >
> > > It is very suspicious to change the cmdline library.
> > > If there is a bug in this library, it deserves a separate fix.
> >
> >


First, testpmd is not really broken, as only thing that changed is the Ctrl + D behavior. I agree this is an issue that we need to fix, as it looks that it is breaking some automation scripts for some people.

The change in behavior for Ctrl + D exit is caused by replacing the call to cmdline_interact() with calling cmdline_poll() in a loop. These two approaches should be identical in behavior, but it looks like they are not due to some issue in the cmdline library. Jasvinder proposed a quick patch, but it looks like something else needs to be fixed in cmdline library in order to bring cmdline_poll() on parity with cmdline_interact(). Any advice from Olivier would be very much appreciated!

It is really a bad practice to use cmdline_interact() in testpmd, as it is a blocking call that prohibits doing other things on the same thread that runs the CLI. Sometimes we need to run other things on the same core, e.g. the slow softnic_manage() function.

Worst case scenario: We can revert the cmdline_poll() back to cmdline_interact(), this is a small change, but not the proper way of doing things, as this is simply hiding the issue in cmdline library. It would also prevent us from testing some Soft NIC functionality.

Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d

[Thomas Monjalon]

24/07/2018 18:59, Dumitrescu, Cristian:
> From: Mordechay Haimovsky [mailto:motih@mellanox.com]
> > 
> > Even after this fix we still have setups that use netvsc  for example,  on
> > which testpmd exits with rte_panic right after loading it even without
> > touching the KBD.
> > 
> > I recommend returning the previous prompt routine in test-pmd/cmdline.c
> > and rework the SOFTNIC section there, preferably moving its poll section to
> > use rte_service in a separate file cleaning the CLI files from PMD-specific
> > implementation.
> > 
> > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > >
> > > Important note:
> > > 	testpmd is currently really broken.
> > > 	We cannot have a RC2 until it is fixed.
> > >
> 
> First, testpmd is not really broken, as only thing that changed is the Ctrl + D behavior. I agree this is an issue that we need to fix, as it looks that it is breaking some automation scripts for some people.
> 
> The change in behavior for Ctrl + D exit is caused by replacing the call to cmdline_interact() with calling cmdline_poll() in a loop. These two approaches should be identical in behavior, but it looks like they are not due to some issue in the cmdline library. Jasvinder proposed a quick patch, but it looks like something else needs to be fixed in cmdline library in order to bring cmdline_poll() on parity with cmdline_interact(). Any advice from Olivier would be very much appreciated!
> 
> It is really a bad practice to use cmdline_interact() in testpmd, as it is a blocking call that prohibits doing other things on the same thread that runs the CLI. Sometimes we need to run other things on the same core, e.g. the slow softnic_manage() function.
> 
> Worst case scenario: We can revert the cmdline_poll() back to cmdline_interact(), this is a small change, but not the proper way of doing things, as this is simply hiding the issue in cmdline library. It would also prevent us from testing some Soft NIC functionality.

There are some crashes, even without touching the keyboard.
So yes, we should revert.

Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d

[Singh, Jasvinder]

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Wednesday, July 25, 2018 9:19 AM
> To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>
> Cc: dev@dpdk.org; Mordechay Haimovsky <motih@mellanox.com>; Singh,
> Jasvinder <jasvinder.singh@intel.com>; Iremonger, Bernard
> <bernard.iremonger@intel.com>; Pattan, Reshma
> <reshma.pattan@intel.com>; olivier.matz@6wind.com
> Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d
> 
> 24/07/2018 18:59, Dumitrescu, Cristian:
> > From: Mordechay Haimovsky [mailto:motih@mellanox.com]
> > >
> > > Even after this fix we still have setups that use netvsc  for
> > > example,  on which testpmd exits with rte_panic right after loading
> > > it even without touching the KBD.
> > >
> > > I recommend returning the previous prompt routine in
> > > test-pmd/cmdline.c and rework the SOFTNIC section there, preferably
> > > moving its poll section to use rte_service in a separate file
> > > cleaning the CLI files from PMD-specific implementation.
> > >
> > > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > > >
> > > > Important note:
> > > > 	testpmd is currently really broken.
> > > > 	We cannot have a RC2 until it is fixed.
> > > >
> >
> > First, testpmd is not really broken, as only thing that changed is the Ctrl + D
> behavior. I agree this is an issue that we need to fix, as it looks that it is
> breaking some automation scripts for some people.
> >
> > The change in behavior for Ctrl + D exit is caused by replacing the call to
> cmdline_interact() with calling cmdline_poll() in a loop. These two approaches
> should be identical in behavior, but it looks like they are not due to some issue
> in the cmdline library. Jasvinder proposed a quick patch, but it looks like
> something else needs to be fixed in cmdline library in order to bring
> cmdline_poll() on parity with cmdline_interact(). Any advice from Olivier would
> be very much appreciated!
> >
> > It is really a bad practice to use cmdline_interact() in testpmd, as it is a
> blocking call that prohibits doing other things on the same thread that runs the
> CLI. Sometimes we need to run other things on the same core, e.g. the slow
> softnic_manage() function.
> >
> > Worst case scenario: We can revert the cmdline_poll() back to
> cmdline_interact(), this is a small change, but not the proper way of doing
> things, as this is simply hiding the issue in cmdline library. It would also prevent
> us from testing some Soft NIC functionality.
> 
> There are some crashes, even without touching the keyboard.

Thomas, could you be specific here so that we can reproduce the issues you are mentioning? Thanks.

> So yes, we should revert.
> 

Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d

[Dumitrescu, Cristian]

> -----Original Message-----
> From: Singh, Jasvinder
> Sent: Wednesday, July 25, 2018 9:31 AM
> To: Thomas Monjalon <thomas@monjalon.net>; Dumitrescu, Cristian
> <cristian.dumitrescu@intel.com>
> Cc: dev@dpdk.org; Mordechay Haimovsky <motih@mellanox.com>;
> Iremonger, Bernard <bernard.iremonger@intel.com>; Pattan, Reshma
> <reshma.pattan@intel.com>; olivier.matz@6wind.com
> Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d
> 
> 
> 
> > -----Original Message-----
> > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > Sent: Wednesday, July 25, 2018 9:19 AM
> > To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>
> > Cc: dev@dpdk.org; Mordechay Haimovsky <motih@mellanox.com>; Singh,
> > Jasvinder <jasvinder.singh@intel.com>; Iremonger, Bernard
> > <bernard.iremonger@intel.com>; Pattan, Reshma
> > <reshma.pattan@intel.com>; olivier.matz@6wind.com
> > Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d
> >
> > 24/07/2018 18:59, Dumitrescu, Cristian:
> > > From: Mordechay Haimovsky [mailto:motih@mellanox.com]
> > > >
> > > > Even after this fix we still have setups that use netvsc  for
> > > > example,  on which testpmd exits with rte_panic right after loading
> > > > it even without touching the KBD.
> > > >
> > > > I recommend returning the previous prompt routine in
> > > > test-pmd/cmdline.c and rework the SOFTNIC section there, preferably
> > > > moving its poll section to use rte_service in a separate file
> > > > cleaning the CLI files from PMD-specific implementation.
> > > >
> > > > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > > > >
> > > > > Important note:
> > > > > 	testpmd is currently really broken.
> > > > > 	We cannot have a RC2 until it is fixed.
> > > > >
> > >
> > > First, testpmd is not really broken, as only thing that changed is the Ctrl +
> D
> > behavior. I agree this is an issue that we need to fix, as it looks that it is
> > breaking some automation scripts for some people.
> > >
> > > The change in behavior for Ctrl + D exit is caused by replacing the call to
> > cmdline_interact() with calling cmdline_poll() in a loop. These two
> approaches
> > should be identical in behavior, but it looks like they are not due to some
> issue
> > in the cmdline library. Jasvinder proposed a quick patch, but it looks like
> > something else needs to be fixed in cmdline library in order to bring
> > cmdline_poll() on parity with cmdline_interact(). Any advice from Olivier
> would
> > be very much appreciated!
> > >
> > > It is really a bad practice to use cmdline_interact() in testpmd, as it is a
> > blocking call that prohibits doing other things on the same thread that runs
> the
> > CLI. Sometimes we need to run other things on the same core, e.g. the
> slow
> > softnic_manage() function.
> > >
> > > Worst case scenario: We can revert the cmdline_poll() back to
> > cmdline_interact(), this is a small change, but not the proper way of doing
> > things, as this is simply hiding the issue in cmdline library. It would also
> prevent
> > us from testing some Soft NIC functionality.
> >
> > There are some crashes, even without touching the keyboard.
> 
> Thomas, could you be specific here so that we can reproduce the issues you
> are mentioning? Thanks.
> 

Yeah, I think the crash mention was about a custom automation script relying on Ctrl+D behavior, not about tespmd crash. Moti, please clarify.

> > So yes, we should revert.
> >

Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d

[Mordechay Haimovsky]

Hi,
This issue happens on some of our setups sometimes without even pressing any key.
I will try to create a minimalistic setup which will show the problem and will be easy to construct by you.
Will send you all the details later on today.

Moti

> -----Original Message-----
> From: Singh, Jasvinder [mailto:jasvinder.singh@intel.com]
> Sent: Wednesday, July 25, 2018 11:31 AM
> To: Thomas Monjalon <thomas@monjalon.net>; Dumitrescu, Cristian
> <cristian.dumitrescu@intel.com>
> Cc: dev@dpdk.org; Mordechay Haimovsky <motih@mellanox.com>;
> Iremonger, Bernard <bernard.iremonger@intel.com>; Pattan, Reshma
> <reshma.pattan@intel.com>; olivier.matz@6wind.com
> Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d
> 
> 
> 
> > -----Original Message-----
> > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > Sent: Wednesday, July 25, 2018 9:19 AM
> > To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>
> > Cc: dev@dpdk.org; Mordechay Haimovsky <motih@mellanox.com>; Singh,
> > Jasvinder <jasvinder.singh@intel.com>; Iremonger, Bernard
> > <bernard.iremonger@intel.com>; Pattan, Reshma
> > <reshma.pattan@intel.com>; olivier.matz@6wind.com
> > Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using
> > ctrl+d
> >
> > 24/07/2018 18:59, Dumitrescu, Cristian:
> > > From: Mordechay Haimovsky [mailto:motih@mellanox.com]
> > > >
> > > > Even after this fix we still have setups that use netvsc  for
> > > > example,  on which testpmd exits with rte_panic right after
> > > > loading it even without touching the KBD.
> > > >
> > > > I recommend returning the previous prompt routine in
> > > > test-pmd/cmdline.c and rework the SOFTNIC section there,
> > > > preferably moving its poll section to use rte_service in a
> > > > separate file cleaning the CLI files from PMD-specific implementation.
> > > >
> > > > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > > > >
> > > > > Important note:
> > > > > 	testpmd is currently really broken.
> > > > > 	We cannot have a RC2 until it is fixed.
> > > > >
> > >
> > > First, testpmd is not really broken, as only thing that changed is
> > > the Ctrl + D
> > behavior. I agree this is an issue that we need to fix, as it looks
> > that it is breaking some automation scripts for some people.
> > >
> > > The change in behavior for Ctrl + D exit is caused by replacing the
> > > call to
> > cmdline_interact() with calling cmdline_poll() in a loop. These two
> > approaches should be identical in behavior, but it looks like they are
> > not due to some issue in the cmdline library. Jasvinder proposed a
> > quick patch, but it looks like something else needs to be fixed in
> > cmdline library in order to bring
> > cmdline_poll() on parity with cmdline_interact(). Any advice from
> > Olivier would be very much appreciated!
> > >
> > > It is really a bad practice to use cmdline_interact() in testpmd, as
> > > it is a
> > blocking call that prohibits doing other things on the same thread
> > that runs the CLI. Sometimes we need to run other things on the same
> > core, e.g. the slow
> > softnic_manage() function.
> > >
> > > Worst case scenario: We can revert the cmdline_poll() back to
> > cmdline_interact(), this is a small change, but not the proper way of
> > doing things, as this is simply hiding the issue in cmdline library.
> > It would also prevent us from testing some Soft NIC functionality.
> >
> > There are some crashes, even without touching the keyboard.
> 
> Thomas, could you be specific here so that we can reproduce the issues you
> are mentioning? Thanks.
> 
> > So yes, we should revert.
> >

Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d

[Ananyev, Konstantin]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Dumitrescu, Cristian
> Sent: Tuesday, July 24, 2018 5:59 PM
> To: Mordechay Haimovsky <motih@mellanox.com>; Thomas Monjalon <thomas@monjalon.net>; Singh, Jasvinder
> <jasvinder.singh@intel.com>
> Cc: dev@dpdk.org; Iremonger, Bernard <bernard.iremonger@intel.com>; Pattan, Reshma <reshma.pattan@intel.com>;
> olivier.matz@6wind.com
> Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d
> 
> 
> 
> > -----Original Message-----
> > From: Mordechay Haimovsky [mailto:motih@mellanox.com]
> > Sent: Tuesday, July 24, 2018 3:37 PM
> > To: Thomas Monjalon <thomas@monjalon.net>; Singh, Jasvinder
> > <jasvinder.singh@intel.com>
> > Cc: dev@dpdk.org; Iremonger, Bernard <bernard.iremonger@intel.com>;
> > Pattan, Reshma <reshma.pattan@intel.com>; olivier.matz@6wind.com;
> > Dumitrescu, Cristian <cristian.dumitrescu@intel.com>
> > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d
> >
> > Even after this fix we still have setups that use netvsc  for example,  on
> > which testpmd exits with rte_panic right after loading it even without
> > touching the KBD.
> >
> > I recommend returning the previous prompt routine in test-pmd/cmdline.c
> > and rework the SOFTNIC section there, preferably moving its poll section to
> > use rte_service in a separate file cleaning the CLI files from PMD-specific
> > implementation.
> >
> > > -----Original Message-----
> > > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > > Sent: Tuesday, July 24, 2018 3:34 PM
> > > To: Jasvinder Singh <jasvinder.singh@intel.com>
> > > Cc: dev@dpdk.org; bernard.iremonger@intel.com;
> > > reshma.pattan@intel.com; Mordechay Haimovsky
> > <motih@mellanox.com>;
> > > olivier.matz@6wind.com; cristian.dumitrescu@intel.com
> > > Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d
> > >
> > > Important note:
> > > 	testpmd is currently really broken.
> > > 	We cannot have a RC2 until it is fixed.
> > >
> > >
> > > 24/07/2018 13:25, Thomas Monjalon:
> > > > 23/07/2018 12:44, Jasvinder Singh:
> > > > > Fix testpmd app exit by pressing ctrl+d, End-of-Transmission
> > > > > character (EOT) on the empty command line.
> > > >
> > > > Please describe what is the issue and what is the cause.
> > > >
> > > > > Fixes: 0ad778b398c6 ("app/testpmd: rework softnic forward mode")
> > > > >
> > > > > Reported-by: Mordechay Haimovsky <motih@mellanox.com>
> > > > > Signed-off-by: Jasvinder Singh <jasvinder.singh@intel.com>
> > > > > ---
> > > > >  app/test-pmd/cmdline.c       | 10 ++++++----
> > > > >  lib/librte_cmdline/cmdline.c |  2 +-
> > > >
> > > > It is very suspicious to change the cmdline library.
> > > > If there is a bug in this library, it deserves a separate fix.
> > >
> > >
> 
> 
> First, testpmd is not really broken, as only thing that changed is the Ctrl + D behavior. I agree this is an issue that we need to fix, as
> it looks that it is breaking some automation scripts for some people.
> 
> The change in behavior for Ctrl + D exit is caused by replacing the call to cmdline_interact() with calling cmdline_poll() in a loop.
> These two approaches should be identical in behavior, but it looks like they are not due to some issue in the cmdline library.
> Jasvinder proposed a quick patch, but it looks like something else needs to be fixed in cmdline library in order to bring
> cmdline_poll() on parity with cmdline_interact(). Any advice from Olivier would be very much appreciated!
> 
> It is really a bad practice to use cmdline_interact() in testpmd, as it is a blocking call that prohibits doing other things on the same
> thread that runs the CLI. Sometimes we need to run other things on the same core, e.g. the slow softnic_manage() function.

Curious why not use a service core for softnic background stuff, and leave CLI one for CLI? 
Konstantin

> 
> Worst case scenario: We can revert the cmdline_poll() back to cmdline_interact(), this is a small change, but not the proper way of
> doing things, as this is simply hiding the issue in cmdline library. It would also prevent us from testing some Soft NIC functionality.

Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d

[Dumitrescu, Cristian]

> -----Original Message-----
> From: Ananyev, Konstantin
> Sent: Wednesday, July 25, 2018 10:04 AM
> To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Mordechay
> Haimovsky <motih@mellanox.com>; Thomas Monjalon
> <thomas@monjalon.net>; Singh, Jasvinder <jasvinder.singh@intel.com>
> Cc: dev@dpdk.org; Iremonger, Bernard <bernard.iremonger@intel.com>;
> Pattan, Reshma <reshma.pattan@intel.com>; olivier.matz@6wind.com
> Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d
> 
> 
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Dumitrescu,
> Cristian
> > Sent: Tuesday, July 24, 2018 5:59 PM
> > To: Mordechay Haimovsky <motih@mellanox.com>; Thomas Monjalon
> <thomas@monjalon.net>; Singh, Jasvinder
> > <jasvinder.singh@intel.com>
> > Cc: dev@dpdk.org; Iremonger, Bernard <bernard.iremonger@intel.com>;
> Pattan, Reshma <reshma.pattan@intel.com>;
> > olivier.matz@6wind.com
> > Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d
> >
> >
> >
> > > -----Original Message-----
> > > From: Mordechay Haimovsky [mailto:motih@mellanox.com]
> > > Sent: Tuesday, July 24, 2018 3:37 PM
> > > To: Thomas Monjalon <thomas@monjalon.net>; Singh, Jasvinder
> > > <jasvinder.singh@intel.com>
> > > Cc: dev@dpdk.org; Iremonger, Bernard <bernard.iremonger@intel.com>;
> > > Pattan, Reshma <reshma.pattan@intel.com>; olivier.matz@6wind.com;
> > > Dumitrescu, Cristian <cristian.dumitrescu@intel.com>
> > > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using
> ctrl+d
> > >
> > > Even after this fix we still have setups that use netvsc  for example,  on
> > > which testpmd exits with rte_panic right after loading it even without
> > > touching the KBD.
> > >
> > > I recommend returning the previous prompt routine in test-
> pmd/cmdline.c
> > > and rework the SOFTNIC section there, preferably moving its poll section
> to
> > > use rte_service in a separate file cleaning the CLI files from PMD-specific
> > > implementation.
> > >
> > > > -----Original Message-----
> > > > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > > > Sent: Tuesday, July 24, 2018 3:34 PM
> > > > To: Jasvinder Singh <jasvinder.singh@intel.com>
> > > > Cc: dev@dpdk.org; bernard.iremonger@intel.com;
> > > > reshma.pattan@intel.com; Mordechay Haimovsky
> > > <motih@mellanox.com>;
> > > > olivier.matz@6wind.com; cristian.dumitrescu@intel.com
> > > > Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using
> ctrl+d
> > > >
> > > > Important note:
> > > > 	testpmd is currently really broken.
> > > > 	We cannot have a RC2 until it is fixed.
> > > >
> > > >
> > > > 24/07/2018 13:25, Thomas Monjalon:
> > > > > 23/07/2018 12:44, Jasvinder Singh:
> > > > > > Fix testpmd app exit by pressing ctrl+d, End-of-Transmission
> > > > > > character (EOT) on the empty command line.
> > > > >
> > > > > Please describe what is the issue and what is the cause.
> > > > >
> > > > > > Fixes: 0ad778b398c6 ("app/testpmd: rework softnic forward
> mode")
> > > > > >
> > > > > > Reported-by: Mordechay Haimovsky <motih@mellanox.com>
> > > > > > Signed-off-by: Jasvinder Singh <jasvinder.singh@intel.com>
> > > > > > ---
> > > > > >  app/test-pmd/cmdline.c       | 10 ++++++----
> > > > > >  lib/librte_cmdline/cmdline.c |  2 +-
> > > > >
> > > > > It is very suspicious to change the cmdline library.
> > > > > If there is a bug in this library, it deserves a separate fix.
> > > >
> > > >
> >
> >
> > First, testpmd is not really broken, as only thing that changed is the Ctrl +
> D behavior. I agree this is an issue that we need to fix, as
> > it looks that it is breaking some automation scripts for some people.
> >
> > The change in behavior for Ctrl + D exit is caused by replacing the call to
> cmdline_interact() with calling cmdline_poll() in a loop.
> > These two approaches should be identical in behavior, but it looks like they
> are not due to some issue in the cmdline library.
> > Jasvinder proposed a quick patch, but it looks like something else needs to
> be fixed in cmdline library in order to bring
> > cmdline_poll() on parity with cmdline_interact(). Any advice from Olivier
> would be very much appreciated!
> >
> > It is really a bad practice to use cmdline_interact() in testpmd, as it is a
> blocking call that prohibits doing other things on the same
> > thread that runs the CLI. Sometimes we need to run other things on the
> same core, e.g. the slow softnic_manage() function.
> 
> Curious why not use a service core for softnic background stuff, and leave CLI
> one for CLI?
> Konstantin

I guess for a test application you can do anything you want, but in real life CPU cores are really expensive and dedicating a CPU core just for CLI is a colossal waste.

We did use the non-blocking cmdline_poll() function instead of the blocking cmdline_interact() in the past without any issues. The issues reported by Moti come as a surprise. It is probably good to see what this is about and see if we can quickly fix the issue in cmdline library. Otherwise, we can revert the usage of cmdline_poll() with cmdline_interact().


> 
> >
> > Worst case scenario: We can revert the cmdline_poll() back to
> cmdline_interact(), this is a small change, but not the proper way of
> > doing things, as this is simply hiding the issue in cmdline library. It would
> also prevent us from testing some Soft NIC functionality.

Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d

[Ananyev, Konstantin]

> -----Original Message-----
> From: Dumitrescu, Cristian
> Sent: Wednesday, July 25, 2018 10:36 AM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Mordechay Haimovsky <motih@mellanox.com>; Thomas Monjalon
> <thomas@monjalon.net>; Singh, Jasvinder <jasvinder.singh@intel.com>
> Cc: dev@dpdk.org; Iremonger, Bernard <bernard.iremonger@intel.com>; Pattan, Reshma <reshma.pattan@intel.com>;
> olivier.matz@6wind.com
> Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d
> 
> 
> 
> > -----Original Message-----
> > From: Ananyev, Konstantin
> > Sent: Wednesday, July 25, 2018 10:04 AM
> > To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Mordechay
> > Haimovsky <motih@mellanox.com>; Thomas Monjalon
> > <thomas@monjalon.net>; Singh, Jasvinder <jasvinder.singh@intel.com>
> > Cc: dev@dpdk.org; Iremonger, Bernard <bernard.iremonger@intel.com>;
> > Pattan, Reshma <reshma.pattan@intel.com>; olivier.matz@6wind.com
> > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d
> >
> >
> >
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Dumitrescu,
> > Cristian
> > > Sent: Tuesday, July 24, 2018 5:59 PM
> > > To: Mordechay Haimovsky <motih@mellanox.com>; Thomas Monjalon
> > <thomas@monjalon.net>; Singh, Jasvinder
> > > <jasvinder.singh@intel.com>
> > > Cc: dev@dpdk.org; Iremonger, Bernard <bernard.iremonger@intel.com>;
> > Pattan, Reshma <reshma.pattan@intel.com>;
> > > olivier.matz@6wind.com
> > > Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Mordechay Haimovsky [mailto:motih@mellanox.com]
> > > > Sent: Tuesday, July 24, 2018 3:37 PM
> > > > To: Thomas Monjalon <thomas@monjalon.net>; Singh, Jasvinder
> > > > <jasvinder.singh@intel.com>
> > > > Cc: dev@dpdk.org; Iremonger, Bernard <bernard.iremonger@intel.com>;
> > > > Pattan, Reshma <reshma.pattan@intel.com>; olivier.matz@6wind.com;
> > > > Dumitrescu, Cristian <cristian.dumitrescu@intel.com>
> > > > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using
> > ctrl+d
> > > >
> > > > Even after this fix we still have setups that use netvsc  for example,  on
> > > > which testpmd exits with rte_panic right after loading it even without
> > > > touching the KBD.
> > > >
> > > > I recommend returning the previous prompt routine in test-
> > pmd/cmdline.c
> > > > and rework the SOFTNIC section there, preferably moving its poll section
> > to
> > > > use rte_service in a separate file cleaning the CLI files from PMD-specific
> > > > implementation.
> > > >
> > > > > -----Original Message-----
> > > > > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > > > > Sent: Tuesday, July 24, 2018 3:34 PM
> > > > > To: Jasvinder Singh <jasvinder.singh@intel.com>
> > > > > Cc: dev@dpdk.org; bernard.iremonger@intel.com;
> > > > > reshma.pattan@intel.com; Mordechay Haimovsky
> > > > <motih@mellanox.com>;
> > > > > olivier.matz@6wind.com; cristian.dumitrescu@intel.com
> > > > > Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using
> > ctrl+d
> > > > >
> > > > > Important note:
> > > > > 	testpmd is currently really broken.
> > > > > 	We cannot have a RC2 until it is fixed.
> > > > >
> > > > >
> > > > > 24/07/2018 13:25, Thomas Monjalon:
> > > > > > 23/07/2018 12:44, Jasvinder Singh:
> > > > > > > Fix testpmd app exit by pressing ctrl+d, End-of-Transmission
> > > > > > > character (EOT) on the empty command line.
> > > > > >
> > > > > > Please describe what is the issue and what is the cause.
> > > > > >
> > > > > > > Fixes: 0ad778b398c6 ("app/testpmd: rework softnic forward
> > mode")
> > > > > > >
> > > > > > > Reported-by: Mordechay Haimovsky <motih@mellanox.com>
> > > > > > > Signed-off-by: Jasvinder Singh <jasvinder.singh@intel.com>
> > > > > > > ---
> > > > > > >  app/test-pmd/cmdline.c       | 10 ++++++----
> > > > > > >  lib/librte_cmdline/cmdline.c |  2 +-
> > > > > >
> > > > > > It is very suspicious to change the cmdline library.
> > > > > > If there is a bug in this library, it deserves a separate fix.
> > > > >
> > > > >
> > >
> > >
> > > First, testpmd is not really broken, as only thing that changed is the Ctrl +
> > D behavior. I agree this is an issue that we need to fix, as
> > > it looks that it is breaking some automation scripts for some people.
> > >
> > > The change in behavior for Ctrl + D exit is caused by replacing the call to
> > cmdline_interact() with calling cmdline_poll() in a loop.
> > > These two approaches should be identical in behavior, but it looks like they
> > are not due to some issue in the cmdline library.
> > > Jasvinder proposed a quick patch, but it looks like something else needs to
> > be fixed in cmdline library in order to bring
> > > cmdline_poll() on parity with cmdline_interact(). Any advice from Olivier
> > would be very much appreciated!
> > >
> > > It is really a bad practice to use cmdline_interact() in testpmd, as it is a
> > blocking call that prohibits doing other things on the same
> > > thread that runs the CLI. Sometimes we need to run other things on the
> > same core, e.g. the slow softnic_manage() function.
> >
> > Curious why not use a service core for softnic background stuff, and leave CLI
> > one for CLI?
> > Konstantin
> 
> I guess for a test application you can do anything you want, but in real life CPU cores are really expensive and dedicating a CPU core
> just for CLI is a colossal waste.

Ok, but let's application developer to decide how to use (waste) the cores he owns :)
What I am saying: there is a special thing (developed by Harry) service cores.
As I understand why of it's the purpose - allow PMD(s) to allocate CPU resources for
there background tasks in a unified and transparent way.
>From the description above - that seems to fit your needs (softnic background processing), no?
Konstantin

> 
> We did use the non-blocking cmdline_poll() function instead of the blocking cmdline_interact() in the past without any issues. The
> issues reported by Moti come as a surprise. It is probably good to see what this is about and see if we can quickly fix the issue in
> cmdline library. Otherwise, we can revert the usage of cmdline_poll() with cmdline_interact().
> 
> 
> >
> > >
> > > Worst case scenario: We can revert the cmdline_poll() back to
> > cmdline_interact(), this is a small change, but not the proper way of
> > > doing things, as this is simply hiding the issue in cmdline library. It would
> > also prevent us from testing some Soft NIC functionality.

Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d

[Mordechay Haimovsky]

This is how we reproduce the issue :
app/testpmd -n 4   --vdev="net_tap0,iface=tap0,remote=eth1"  -- --burst=64 --mbcache=512 -i   --rxq=2 --txq=2 --txd=512 --rxd=512  --port-topology=chained --forward-mode=rxonly

> -----Original Message-----
> From: Ananyev, Konstantin [mailto:konstantin.ananyev@intel.com]
> Sent: Wednesday, July 25, 2018 2:39 PM
> To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Mordechay
> Haimovsky <motih@mellanox.com>; Thomas Monjalon
> <thomas@monjalon.net>; Singh, Jasvinder <jasvinder.singh@intel.com>
> Cc: dev@dpdk.org; Iremonger, Bernard <bernard.iremonger@intel.com>;
> Pattan, Reshma <reshma.pattan@intel.com>; olivier.matz@6wind.com; Van
> Haaren, Harry <harry.van.haaren@intel.com>
> Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d
> 
> 
> 
> > -----Original Message-----
> > From: Dumitrescu, Cristian
> > Sent: Wednesday, July 25, 2018 10:36 AM
> > To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Mordechay
> > Haimovsky <motih@mellanox.com>; Thomas Monjalon
> <thomas@monjalon.net>;
> > Singh, Jasvinder <jasvinder.singh@intel.com>
> > Cc: dev@dpdk.org; Iremonger, Bernard <bernard.iremonger@intel.com>;
> > Pattan, Reshma <reshma.pattan@intel.com>; olivier.matz@6wind.com
> > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using
> > ctrl+d
> >
> >
> >
> > > -----Original Message-----
> > > From: Ananyev, Konstantin
> > > Sent: Wednesday, July 25, 2018 10:04 AM
> > > To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Mordechay
> > > Haimovsky <motih@mellanox.com>; Thomas Monjalon
> > > <thomas@monjalon.net>; Singh, Jasvinder <jasvinder.singh@intel.com>
> > > Cc: dev@dpdk.org; Iremonger, Bernard
> <bernard.iremonger@intel.com>;
> > > Pattan, Reshma <reshma.pattan@intel.com>; olivier.matz@6wind.com
> > > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using
> > > ctrl+d
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Dumitrescu,
> > > Cristian
> > > > Sent: Tuesday, July 24, 2018 5:59 PM
> > > > To: Mordechay Haimovsky <motih@mellanox.com>; Thomas Monjalon
> > > <thomas@monjalon.net>; Singh, Jasvinder
> > > > <jasvinder.singh@intel.com>
> > > > Cc: dev@dpdk.org; Iremonger, Bernard
> > > > <bernard.iremonger@intel.com>;
> > > Pattan, Reshma <reshma.pattan@intel.com>;
> > > > olivier.matz@6wind.com
> > > > Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit
> > > > using ctrl+d
> > > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Mordechay Haimovsky [mailto:motih@mellanox.com]
> > > > > Sent: Tuesday, July 24, 2018 3:37 PM
> > > > > To: Thomas Monjalon <thomas@monjalon.net>; Singh, Jasvinder
> > > > > <jasvinder.singh@intel.com>
> > > > > Cc: dev@dpdk.org; Iremonger, Bernard
> > > > > <bernard.iremonger@intel.com>; Pattan, Reshma
> > > > > <reshma.pattan@intel.com>; olivier.matz@6wind.com; Dumitrescu,
> > > > > Cristian <cristian.dumitrescu@intel.com>
> > > > > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit
> > > > > using
> > > ctrl+d
> > > > >
> > > > > Even after this fix we still have setups that use netvsc  for
> > > > > example,  on which testpmd exits with rte_panic right after
> > > > > loading it even without touching the KBD.
> > > > >
> > > > > I recommend returning the previous prompt routine in test-
> > > pmd/cmdline.c
> > > > > and rework the SOFTNIC section there, preferably moving its poll
> > > > > section
> > > to
> > > > > use rte_service in a separate file cleaning the CLI files from
> > > > > PMD-specific implementation.
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > > > > > Sent: Tuesday, July 24, 2018 3:34 PM
> > > > > > To: Jasvinder Singh <jasvinder.singh@intel.com>
> > > > > > Cc: dev@dpdk.org; bernard.iremonger@intel.com;
> > > > > > reshma.pattan@intel.com; Mordechay Haimovsky
> > > > > <motih@mellanox.com>;
> > > > > > olivier.matz@6wind.com; cristian.dumitrescu@intel.com
> > > > > > Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit
> > > > > > using
> > > ctrl+d
> > > > > >
> > > > > > Important note:
> > > > > > 	testpmd is currently really broken.
> > > > > > 	We cannot have a RC2 until it is fixed.
> > > > > >
> > > > > >
> > > > > > 24/07/2018 13:25, Thomas Monjalon:
> > > > > > > 23/07/2018 12:44, Jasvinder Singh:
> > > > > > > > Fix testpmd app exit by pressing ctrl+d,
> > > > > > > > End-of-Transmission character (EOT) on the empty command
> line.
> > > > > > >
> > > > > > > Please describe what is the issue and what is the cause.
> > > > > > >
> > > > > > > > Fixes: 0ad778b398c6 ("app/testpmd: rework softnic forward
> > > mode")
> > > > > > > >
> > > > > > > > Reported-by: Mordechay Haimovsky <motih@mellanox.com>
> > > > > > > > Signed-off-by: Jasvinder Singh <jasvinder.singh@intel.com>
> > > > > > > > ---
> > > > > > > >  app/test-pmd/cmdline.c       | 10 ++++++----
> > > > > > > >  lib/librte_cmdline/cmdline.c |  2 +-
> > > > > > >
> > > > > > > It is very suspicious to change the cmdline library.
> > > > > > > If there is a bug in this library, it deserves a separate fix.
> > > > > >
> > > > > >
> > > >
> > > >
> > > > First, testpmd is not really broken, as only thing that changed is
> > > > the Ctrl +
> > > D behavior. I agree this is an issue that we need to fix, as
> > > > it looks that it is breaking some automation scripts for some people.
> > > >
> > > > The change in behavior for Ctrl + D exit is caused by replacing
> > > > the call to
> > > cmdline_interact() with calling cmdline_poll() in a loop.
> > > > These two approaches should be identical in behavior, but it looks
> > > > like they
> > > are not due to some issue in the cmdline library.
> > > > Jasvinder proposed a quick patch, but it looks like something else
> > > > needs to
> > > be fixed in cmdline library in order to bring
> > > > cmdline_poll() on parity with cmdline_interact(). Any advice from
> > > > Olivier
> > > would be very much appreciated!
> > > >
> > > > It is really a bad practice to use cmdline_interact() in testpmd,
> > > > as it is a
> > > blocking call that prohibits doing other things on the same
> > > > thread that runs the CLI. Sometimes we need to run other things on
> > > > the
> > > same core, e.g. the slow softnic_manage() function.
> > >
> > > Curious why not use a service core for softnic background stuff, and
> > > leave CLI one for CLI?
> > > Konstantin
> >
> > I guess for a test application you can do anything you want, but in
> > real life CPU cores are really expensive and dedicating a CPU core just for
> CLI is a colossal waste.
> 
> Ok, but let's application developer to decide how to use (waste) the cores he
> owns :) What I am saying: there is a special thing (developed by Harry)
> service cores.
> As I understand why of it's the purpose - allow PMD(s) to allocate CPU
> resources for there background tasks in a unified and transparent way.
> From the description above - that seems to fit your needs (softnic
> background processing), no?
> Konstantin
> 
> >
> > We did use the non-blocking cmdline_poll() function instead of the
> > blocking cmdline_interact() in the past without any issues. The issues
> > reported by Moti come as a surprise. It is probably good to see what this is
> about and see if we can quickly fix the issue in cmdline library. Otherwise, we
> can revert the usage of cmdline_poll() with cmdline_interact().
> >
> >
> > >
> > > >
> > > > Worst case scenario: We can revert the cmdline_poll() back to
> > > cmdline_interact(), this is a small change, but not the proper way
> > > of
> > > > doing things, as this is simply hiding the issue in cmdline
> > > > library. It would
> > > also prevent us from testing some Soft NIC functionality.

Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d

[Dumitrescu, Cristian]

> -----Original Message-----
> From: Ananyev, Konstantin
> Sent: Wednesday, July 25, 2018 12:39 PM
> To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Mordechay
> Haimovsky <motih@mellanox.com>; Thomas Monjalon
> <thomas@monjalon.net>; Singh, Jasvinder <jasvinder.singh@intel.com>
> Cc: dev@dpdk.org; Iremonger, Bernard <bernard.iremonger@intel.com>;
> Pattan, Reshma <reshma.pattan@intel.com>; olivier.matz@6wind.com; Van
> Haaren, Harry <harry.van.haaren@intel.com>
> Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d
> 
> 
> 
> > -----Original Message-----
> > From: Dumitrescu, Cristian
> > Sent: Wednesday, July 25, 2018 10:36 AM
> > To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Mordechay
> Haimovsky <motih@mellanox.com>; Thomas Monjalon
> > <thomas@monjalon.net>; Singh, Jasvinder <jasvinder.singh@intel.com>
> > Cc: dev@dpdk.org; Iremonger, Bernard <bernard.iremonger@intel.com>;
> Pattan, Reshma <reshma.pattan@intel.com>;
> > olivier.matz@6wind.com
> > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d
> >
> >
> >
> > > -----Original Message-----
> > > From: Ananyev, Konstantin
> > > Sent: Wednesday, July 25, 2018 10:04 AM
> > > To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Mordechay
> > > Haimovsky <motih@mellanox.com>; Thomas Monjalon
> > > <thomas@monjalon.net>; Singh, Jasvinder <jasvinder.singh@intel.com>
> > > Cc: dev@dpdk.org; Iremonger, Bernard <bernard.iremonger@intel.com>;
> > > Pattan, Reshma <reshma.pattan@intel.com>; olivier.matz@6wind.com
> > > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using
> ctrl+d
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Dumitrescu,
> > > Cristian
> > > > Sent: Tuesday, July 24, 2018 5:59 PM
> > > > To: Mordechay Haimovsky <motih@mellanox.com>; Thomas Monjalon
> > > <thomas@monjalon.net>; Singh, Jasvinder
> > > > <jasvinder.singh@intel.com>
> > > > Cc: dev@dpdk.org; Iremonger, Bernard
> <bernard.iremonger@intel.com>;
> > > Pattan, Reshma <reshma.pattan@intel.com>;
> > > > olivier.matz@6wind.com
> > > > Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using
> ctrl+d
> > > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Mordechay Haimovsky [mailto:motih@mellanox.com]
> > > > > Sent: Tuesday, July 24, 2018 3:37 PM
> > > > > To: Thomas Monjalon <thomas@monjalon.net>; Singh, Jasvinder
> > > > > <jasvinder.singh@intel.com>
> > > > > Cc: dev@dpdk.org; Iremonger, Bernard
> <bernard.iremonger@intel.com>;
> > > > > Pattan, Reshma <reshma.pattan@intel.com>;
> olivier.matz@6wind.com;
> > > > > Dumitrescu, Cristian <cristian.dumitrescu@intel.com>
> > > > > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using
> > > ctrl+d
> > > > >
> > > > > Even after this fix we still have setups that use netvsc  for example,
> on
> > > > > which testpmd exits with rte_panic right after loading it even
> without
> > > > > touching the KBD.
> > > > >
> > > > > I recommend returning the previous prompt routine in test-
> > > pmd/cmdline.c
> > > > > and rework the SOFTNIC section there, preferably moving its poll
> section
> > > to
> > > > > use rte_service in a separate file cleaning the CLI files from PMD-
> specific
> > > > > implementation.
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > > > > > Sent: Tuesday, July 24, 2018 3:34 PM
> > > > > > To: Jasvinder Singh <jasvinder.singh@intel.com>
> > > > > > Cc: dev@dpdk.org; bernard.iremonger@intel.com;
> > > > > > reshma.pattan@intel.com; Mordechay Haimovsky
> > > > > <motih@mellanox.com>;
> > > > > > olivier.matz@6wind.com; cristian.dumitrescu@intel.com
> > > > > > Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit
> using
> > > ctrl+d
> > > > > >
> > > > > > Important note:
> > > > > > 	testpmd is currently really broken.
> > > > > > 	We cannot have a RC2 until it is fixed.
> > > > > >
> > > > > >
> > > > > > 24/07/2018 13:25, Thomas Monjalon:
> > > > > > > 23/07/2018 12:44, Jasvinder Singh:
> > > > > > > > Fix testpmd app exit by pressing ctrl+d, End-of-Transmission
> > > > > > > > character (EOT) on the empty command line.
> > > > > > >
> > > > > > > Please describe what is the issue and what is the cause.
> > > > > > >
> > > > > > > > Fixes: 0ad778b398c6 ("app/testpmd: rework softnic forward
> > > mode")
> > > > > > > >
> > > > > > > > Reported-by: Mordechay Haimovsky <motih@mellanox.com>
> > > > > > > > Signed-off-by: Jasvinder Singh <jasvinder.singh@intel.com>
> > > > > > > > ---
> > > > > > > >  app/test-pmd/cmdline.c       | 10 ++++++----
> > > > > > > >  lib/librte_cmdline/cmdline.c |  2 +-
> > > > > > >
> > > > > > > It is very suspicious to change the cmdline library.
> > > > > > > If there is a bug in this library, it deserves a separate fix.
> > > > > >
> > > > > >
> > > >
> > > >
> > > > First, testpmd is not really broken, as only thing that changed is the Ctrl
> +
> > > D behavior. I agree this is an issue that we need to fix, as
> > > > it looks that it is breaking some automation scripts for some people.
> > > >
> > > > The change in behavior for Ctrl + D exit is caused by replacing the call
> to
> > > cmdline_interact() with calling cmdline_poll() in a loop.
> > > > These two approaches should be identical in behavior, but it looks like
> they
> > > are not due to some issue in the cmdline library.
> > > > Jasvinder proposed a quick patch, but it looks like something else needs
> to
> > > be fixed in cmdline library in order to bring
> > > > cmdline_poll() on parity with cmdline_interact(). Any advice from
> Olivier
> > > would be very much appreciated!
> > > >
> > > > It is really a bad practice to use cmdline_interact() in testpmd, as it is a
> > > blocking call that prohibits doing other things on the same
> > > > thread that runs the CLI. Sometimes we need to run other things on the
> > > same core, e.g. the slow softnic_manage() function.
> > >
> > > Curious why not use a service core for softnic background stuff, and leave
> CLI
> > > one for CLI?
> > > Konstantin
> >
> > I guess for a test application you can do anything you want, but in real life
> CPU cores are really expensive and dedicating a CPU core
> > just for CLI is a colossal waste.
> 
> Ok, but let's application developer to decide how to use (waste) the cores he
> owns :)
> What I am saying: there is a special thing (developed by Harry) service cores.
> As I understand why of it's the purpose - allow PMD(s) to allocate CPU
> resources for
> there background tasks in a unified and transparent way.
> From the description above - that seems to fit your needs (softnic
> background processing), no?
> Konstantin
> 

Then why not put the testpmd CLI itself on a service core? Are you volunteering for a patch on this? :)


> >
> > We did use the non-blocking cmdline_poll() function instead of the
> blocking cmdline_interact() in the past without any issues. The
> > issues reported by Moti come as a surprise. It is probably good to see what
> this is about and see if we can quickly fix the issue in
> > cmdline library. Otherwise, we can revert the usage of cmdline_poll() with
> cmdline_interact().
> >
> >
> > >
> > > >
> > > > Worst case scenario: We can revert the cmdline_poll() back to
> > > cmdline_interact(), this is a small change, but not the proper way of
> > > > doing things, as this is simply hiding the issue in cmdline library. It
> would
> > > also prevent us from testing some Soft NIC functionality.

Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d

[Ananyev, Konstantin]

> -----Original Message-----
> From: Dumitrescu, Cristian
> Sent: Wednesday, July 25, 2018 12:57 PM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Mordechay Haimovsky <motih@mellanox.com>; Thomas Monjalon
> <thomas@monjalon.net>; Singh, Jasvinder <jasvinder.singh@intel.com>
> Cc: dev@dpdk.org; Iremonger, Bernard <bernard.iremonger@intel.com>; Pattan, Reshma <reshma.pattan@intel.com>;
> olivier.matz@6wind.com; Van Haaren, Harry <harry.van.haaren@intel.com>
> Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d
> 
> 
> 
> > -----Original Message-----
> > From: Ananyev, Konstantin
> > Sent: Wednesday, July 25, 2018 12:39 PM
> > To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Mordechay
> > Haimovsky <motih@mellanox.com>; Thomas Monjalon
> > <thomas@monjalon.net>; Singh, Jasvinder <jasvinder.singh@intel.com>
> > Cc: dev@dpdk.org; Iremonger, Bernard <bernard.iremonger@intel.com>;
> > Pattan, Reshma <reshma.pattan@intel.com>; olivier.matz@6wind.com; Van
> > Haaren, Harry <harry.van.haaren@intel.com>
> > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d
> >
> >
> >
> > > -----Original Message-----
> > > From: Dumitrescu, Cristian
> > > Sent: Wednesday, July 25, 2018 10:36 AM
> > > To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Mordechay
> > Haimovsky <motih@mellanox.com>; Thomas Monjalon
> > > <thomas@monjalon.net>; Singh, Jasvinder <jasvinder.singh@intel.com>
> > > Cc: dev@dpdk.org; Iremonger, Bernard <bernard.iremonger@intel.com>;
> > Pattan, Reshma <reshma.pattan@intel.com>;
> > > olivier.matz@6wind.com
> > > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Ananyev, Konstantin
> > > > Sent: Wednesday, July 25, 2018 10:04 AM
> > > > To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Mordechay
> > > > Haimovsky <motih@mellanox.com>; Thomas Monjalon
> > > > <thomas@monjalon.net>; Singh, Jasvinder <jasvinder.singh@intel.com>
> > > > Cc: dev@dpdk.org; Iremonger, Bernard <bernard.iremonger@intel.com>;
> > > > Pattan, Reshma <reshma.pattan@intel.com>; olivier.matz@6wind.com
> > > > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using
> > ctrl+d
> > > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Dumitrescu,
> > > > Cristian
> > > > > Sent: Tuesday, July 24, 2018 5:59 PM
> > > > > To: Mordechay Haimovsky <motih@mellanox.com>; Thomas Monjalon
> > > > <thomas@monjalon.net>; Singh, Jasvinder
> > > > > <jasvinder.singh@intel.com>
> > > > > Cc: dev@dpdk.org; Iremonger, Bernard
> > <bernard.iremonger@intel.com>;
> > > > Pattan, Reshma <reshma.pattan@intel.com>;
> > > > > olivier.matz@6wind.com
> > > > > Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using
> > ctrl+d
> > > > >
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Mordechay Haimovsky [mailto:motih@mellanox.com]
> > > > > > Sent: Tuesday, July 24, 2018 3:37 PM
> > > > > > To: Thomas Monjalon <thomas@monjalon.net>; Singh, Jasvinder
> > > > > > <jasvinder.singh@intel.com>
> > > > > > Cc: dev@dpdk.org; Iremonger, Bernard
> > <bernard.iremonger@intel.com>;
> > > > > > Pattan, Reshma <reshma.pattan@intel.com>;
> > olivier.matz@6wind.com;
> > > > > > Dumitrescu, Cristian <cristian.dumitrescu@intel.com>
> > > > > > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using
> > > > ctrl+d
> > > > > >
> > > > > > Even after this fix we still have setups that use netvsc  for example,
> > on
> > > > > > which testpmd exits with rte_panic right after loading it even
> > without
> > > > > > touching the KBD.
> > > > > >
> > > > > > I recommend returning the previous prompt routine in test-
> > > > pmd/cmdline.c
> > > > > > and rework the SOFTNIC section there, preferably moving its poll
> > section
> > > > to
> > > > > > use rte_service in a separate file cleaning the CLI files from PMD-
> > specific
> > > > > > implementation.
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > > > > > > Sent: Tuesday, July 24, 2018 3:34 PM
> > > > > > > To: Jasvinder Singh <jasvinder.singh@intel.com>
> > > > > > > Cc: dev@dpdk.org; bernard.iremonger@intel.com;
> > > > > > > reshma.pattan@intel.com; Mordechay Haimovsky
> > > > > > <motih@mellanox.com>;
> > > > > > > olivier.matz@6wind.com; cristian.dumitrescu@intel.com
> > > > > > > Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit
> > using
> > > > ctrl+d
> > > > > > >
> > > > > > > Important note:
> > > > > > > 	testpmd is currently really broken.
> > > > > > > 	We cannot have a RC2 until it is fixed.
> > > > > > >
> > > > > > >
> > > > > > > 24/07/2018 13:25, Thomas Monjalon:
> > > > > > > > 23/07/2018 12:44, Jasvinder Singh:
> > > > > > > > > Fix testpmd app exit by pressing ctrl+d, End-of-Transmission
> > > > > > > > > character (EOT) on the empty command line.
> > > > > > > >
> > > > > > > > Please describe what is the issue and what is the cause.
> > > > > > > >
> > > > > > > > > Fixes: 0ad778b398c6 ("app/testpmd: rework softnic forward
> > > > mode")
> > > > > > > > >
> > > > > > > > > Reported-by: Mordechay Haimovsky <motih@mellanox.com>
> > > > > > > > > Signed-off-by: Jasvinder Singh <jasvinder.singh@intel.com>
> > > > > > > > > ---
> > > > > > > > >  app/test-pmd/cmdline.c       | 10 ++++++----
> > > > > > > > >  lib/librte_cmdline/cmdline.c |  2 +-
> > > > > > > >
> > > > > > > > It is very suspicious to change the cmdline library.
> > > > > > > > If there is a bug in this library, it deserves a separate fix.
> > > > > > >
> > > > > > >
> > > > >
> > > > >
> > > > > First, testpmd is not really broken, as only thing that changed is the Ctrl
> > +
> > > > D behavior. I agree this is an issue that we need to fix, as
> > > > > it looks that it is breaking some automation scripts for some people.
> > > > >
> > > > > The change in behavior for Ctrl + D exit is caused by replacing the call
> > to
> > > > cmdline_interact() with calling cmdline_poll() in a loop.
> > > > > These two approaches should be identical in behavior, but it looks like
> > they
> > > > are not due to some issue in the cmdline library.
> > > > > Jasvinder proposed a quick patch, but it looks like something else needs
> > to
> > > > be fixed in cmdline library in order to bring
> > > > > cmdline_poll() on parity with cmdline_interact(). Any advice from
> > Olivier
> > > > would be very much appreciated!
> > > > >
> > > > > It is really a bad practice to use cmdline_interact() in testpmd, as it is a
> > > > blocking call that prohibits doing other things on the same
> > > > > thread that runs the CLI. Sometimes we need to run other things on the
> > > > same core, e.g. the slow softnic_manage() function.
> > > >
> > > > Curious why not use a service core for softnic background stuff, and leave
> > CLI
> > > > one for CLI?
> > > > Konstantin
> > >
> > > I guess for a test application you can do anything you want, but in real life
> > CPU cores are really expensive and dedicating a CPU core
> > > just for CLI is a colossal waste.
> >
> > Ok, but let's application developer to decide how to use (waste) the cores he
> > owns :)
> > What I am saying: there is a special thing (developed by Harry) service cores.
> > As I understand why of it's the purpose - allow PMD(s) to allocate CPU
> > resources for
> > there background tasks in a unified and transparent way.
> > From the description above - that seems to fit your needs (softnic
> > background processing), no?
> > Konstantin
> >
> 
> Then why not put the testpmd CLI itself on a service core? Are you volunteering for a patch on this? :)

It was not me who broke testpmd at first place.
I'd better live without softnic support in testpmd :)
Though I still don't understand why do you feel that service cores are not
good enough for you?
>From my understanding they were introduced for similar purposes
(Harry please feel free to correct me here).
So if you think they not fit for your case -
at least would be good to understand why.
Konstantin

> 
> 
> > >
> > > We did use the non-blocking cmdline_poll() function instead of the
> > blocking cmdline_interact() in the past without any issues. The
> > > issues reported by Moti come as a surprise. It is probably good to see what
> > this is about and see if we can quickly fix the issue in
> > > cmdline library. Otherwise, we can revert the usage of cmdline_poll() with
> > cmdline_interact().
> > >
> > >
> > > >
> > > > >
> > > > > Worst case scenario: We can revert the cmdline_poll() back to
> > > > cmdline_interact(), this is a small change, but not the proper way of
> > > > > doing things, as this is simply hiding the issue in cmdline library. It
> > would
> > > > also prevent us from testing some Soft NIC functionality.

Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d

[Van Haaren, Harry]

> -----Original Message-----
> From: Ananyev, Konstantin
> Sent: Wednesday, July 25, 2018 1:18 PM
> To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Mordechay Haimovsky
> <motih@mellanox.com>; Thomas Monjalon <thomas@monjalon.net>; Singh, Jasvinder
> <jasvinder.singh@intel.com>
> Cc: dev@dpdk.org; Iremonger, Bernard <bernard.iremonger@intel.com>; Pattan,
> Reshma <reshma.pattan@intel.com>; olivier.matz@6wind.com; Van Haaren, Harry
> <harry.van.haaren@intel.com>
> Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d
> 
> 
> 
> > -----Original Message-----
> > From: Dumitrescu, Cristian
> > Sent: Wednesday, July 25, 2018 12:57 PM
> > To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Mordechay Haimovsky
> <motih@mellanox.com>; Thomas Monjalon
> > <thomas@monjalon.net>; Singh, Jasvinder <jasvinder.singh@intel.com>
> > Cc: dev@dpdk.org; Iremonger, Bernard <bernard.iremonger@intel.com>; Pattan,
> Reshma <reshma.pattan@intel.com>;
> > olivier.matz@6wind.com; Van Haaren, Harry <harry.van.haaren@intel.com>
> > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d
> >
> >
> >
> > > -----Original Message-----
> > > From: Ananyev, Konstantin
> > > Sent: Wednesday, July 25, 2018 12:39 PM
> > > To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Mordechay
> > > Haimovsky <motih@mellanox.com>; Thomas Monjalon
> > > <thomas@monjalon.net>; Singh, Jasvinder <jasvinder.singh@intel.com>
> > > Cc: dev@dpdk.org; Iremonger, Bernard <bernard.iremonger@intel.com>;
> > > Pattan, Reshma <reshma.pattan@intel.com>; olivier.matz@6wind.com; Van
> > > Haaren, Harry <harry.van.haaren@intel.com>
> > > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Dumitrescu, Cristian
> > > > Sent: Wednesday, July 25, 2018 10:36 AM
> > > > To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Mordechay
> > > Haimovsky <motih@mellanox.com>; Thomas Monjalon
> > > > <thomas@monjalon.net>; Singh, Jasvinder <jasvinder.singh@intel.com>
> > > > Cc: dev@dpdk.org; Iremonger, Bernard <bernard.iremonger@intel.com>;
> > > Pattan, Reshma <reshma.pattan@intel.com>;
> > > > olivier.matz@6wind.com
> > > > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using
> ctrl+d
> > > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Ananyev, Konstantin
> > > > > Sent: Wednesday, July 25, 2018 10:04 AM
> > > > > To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Mordechay
> > > > > Haimovsky <motih@mellanox.com>; Thomas Monjalon
> > > > > <thomas@monjalon.net>; Singh, Jasvinder <jasvinder.singh@intel.com>
> > > > > Cc: dev@dpdk.org; Iremonger, Bernard <bernard.iremonger@intel.com>;
> > > > > Pattan, Reshma <reshma.pattan@intel.com>; olivier.matz@6wind.com
> > > > > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using
> > > ctrl+d
> > > > >
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Dumitrescu,
> > > > > Cristian
> > > > > > Sent: Tuesday, July 24, 2018 5:59 PM
> > > > > > To: Mordechay Haimovsky <motih@mellanox.com>; Thomas Monjalon
> > > > > <thomas@monjalon.net>; Singh, Jasvinder
> > > > > > <jasvinder.singh@intel.com>
> > > > > > Cc: dev@dpdk.org; Iremonger, Bernard
> > > <bernard.iremonger@intel.com>;
> > > > > Pattan, Reshma <reshma.pattan@intel.com>;
> > > > > > olivier.matz@6wind.com
> > > > > > Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using
> > > ctrl+d
> > > > > >
> > > > > >
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Mordechay Haimovsky [mailto:motih@mellanox.com]
> > > > > > > Sent: Tuesday, July 24, 2018 3:37 PM
> > > > > > > To: Thomas Monjalon <thomas@monjalon.net>; Singh, Jasvinder
> > > > > > > <jasvinder.singh@intel.com>
> > > > > > > Cc: dev@dpdk.org; Iremonger, Bernard
> > > <bernard.iremonger@intel.com>;
> > > > > > > Pattan, Reshma <reshma.pattan@intel.com>;
> > > olivier.matz@6wind.com;
> > > > > > > Dumitrescu, Cristian <cristian.dumitrescu@intel.com>
> > > > > > > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit
> using
> > > > > ctrl+d
> > > > > > >
> > > > > > > Even after this fix we still have setups that use netvsc  for
> example,
> > > on
> > > > > > > which testpmd exits with rte_panic right after loading it even
> > > without
> > > > > > > touching the KBD.
> > > > > > >
> > > > > > > I recommend returning the previous prompt routine in test-
> > > > > pmd/cmdline.c
> > > > > > > and rework the SOFTNIC section there, preferably moving its poll
> > > section
> > > > > to
> > > > > > > use rte_service in a separate file cleaning the CLI files from
> PMD-
> > > specific
> > > > > > > implementation.
> > > > > > >
> > > > > > > > -----Original Message-----
> > > > > > > > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > > > > > > > Sent: Tuesday, July 24, 2018 3:34 PM
> > > > > > > > To: Jasvinder Singh <jasvinder.singh@intel.com>
> > > > > > > > Cc: dev@dpdk.org; bernard.iremonger@intel.com;
> > > > > > > > reshma.pattan@intel.com; Mordechay Haimovsky
> > > > > > > <motih@mellanox.com>;
> > > > > > > > olivier.matz@6wind.com; cristian.dumitrescu@intel.com
> > > > > > > > Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit
> > > using
> > > > > ctrl+d
> > > > > > > >
> > > > > > > > Important note:
> > > > > > > > 	testpmd is currently really broken.
> > > > > > > > 	We cannot have a RC2 until it is fixed.
> > > > > > > >
> > > > > > > >
> > > > > > > > 24/07/2018 13:25, Thomas Monjalon:
> > > > > > > > > 23/07/2018 12:44, Jasvinder Singh:
> > > > > > > > > > Fix testpmd app exit by pressing ctrl+d, End-of-Transmission
> > > > > > > > > > character (EOT) on the empty command line.
> > > > > > > > >
> > > > > > > > > Please describe what is the issue and what is the cause.
> > > > > > > > >
> > > > > > > > > > Fixes: 0ad778b398c6 ("app/testpmd: rework softnic forward
> > > > > mode")
> > > > > > > > > >
> > > > > > > > > > Reported-by: Mordechay Haimovsky <motih@mellanox.com>
> > > > > > > > > > Signed-off-by: Jasvinder Singh <jasvinder.singh@intel.com>
> > > > > > > > > > ---
> > > > > > > > > >  app/test-pmd/cmdline.c       | 10 ++++++----
> > > > > > > > > >  lib/librte_cmdline/cmdline.c |  2 +-
> > > > > > > > >
> > > > > > > > > It is very suspicious to change the cmdline library.
> > > > > > > > > If there is a bug in this library, it deserves a separate fix.
> > > > > > > >
> > > > > > > >
> > > > > >
> > > > > >
> > > > > > First, testpmd is not really broken, as only thing that changed is
> the Ctrl
> > > +
> > > > > D behavior. I agree this is an issue that we need to fix, as
> > > > > > it looks that it is breaking some automation scripts for some
> people.
> > > > > >
> > > > > > The change in behavior for Ctrl + D exit is caused by replacing the
> call
> > > to
> > > > > cmdline_interact() with calling cmdline_poll() in a loop.
> > > > > > These two approaches should be identical in behavior, but it looks
> like
> > > they
> > > > > are not due to some issue in the cmdline library.
> > > > > > Jasvinder proposed a quick patch, but it looks like something else
> needs
> > > to
> > > > > be fixed in cmdline library in order to bring
> > > > > > cmdline_poll() on parity with cmdline_interact(). Any advice from
> > > Olivier
> > > > > would be very much appreciated!
> > > > > >
> > > > > > It is really a bad practice to use cmdline_interact() in testpmd, as
> it is a
> > > > > blocking call that prohibits doing other things on the same
> > > > > > thread that runs the CLI. Sometimes we need to run other things on
> the
> > > > > same core, e.g. the slow softnic_manage() function.
> > > > >
> > > > > Curious why not use a service core for softnic background stuff, and
> leave
> > > CLI
> > > > > one for CLI?
> > > > > Konstantin
> > > >
> > > > I guess for a test application you can do anything you want, but in real
> life
> > > CPU cores are really expensive and dedicating a CPU core
> > > > just for CLI is a colossal waste.
> > >
> > > Ok, but let's application developer to decide how to use (waste) the cores
> he
> > > owns :)
> > > What I am saying: there is a special thing (developed by Harry) service
> cores.
> > > As I understand why of it's the purpose - allow PMD(s) to allocate CPU
> > > resources for
> > > there background tasks in a unified and transparent way.
> > > From the description above - that seems to fit your needs (softnic
> > > background processing), no?
> > > Konstantin
> > >
> >
> > Then why not put the testpmd CLI itself on a service core? Are you
> volunteering for a patch on this? :)
> 
> It was not me who broke testpmd at first place.
> I'd better live without softnic support in testpmd :)
> Though I still don't understand why do you feel that service cores are not
> good enough for you?
> From my understanding they were introduced for similar purposes
> (Harry please feel free to correct me here).
> So if you think they not fit for your case -
> at least would be good to understand why.
> Konstantin


Service-cores is a mechanism to abstract SW fallback CPU core requirements;
Eg: A nic provides RSS, 0 cores required
    A NIC does not provide RSS, use 1 service core to provide that RSS function

Result: the application can rely on RSS, as we have a transparent fallback:
no application controlled lcore is performing any different than before with HW provided functionality

Service cores is not intended to be used for "background" tasks, as
the service-core consumes an Lcore and polls 100% of the time, regardless
of what services it is performing.


If there are multiple things that we can run on service-cores, that would make
sense. Eg: timer_manage(), CLI (non-blocking..), SoftNIC background tasks...

This design makes sense as the application can also register its own "background"
services, and run them all on a single service-core.


> >
> >
> > > >
> > > > We did use the non-blocking cmdline_poll() function instead of the
> > > blocking cmdline_interact() in the past without any issues. The
> > > > issues reported by Moti come as a surprise. It is probably good to see
> what
> > > this is about and see if we can quickly fix the issue in
> > > > cmdline library. Otherwise, we can revert the usage of cmdline_poll()
> with
> > > cmdline_interact().
> > > >
> > > >
> > > > >
> > > > > >
> > > > > > Worst case scenario: We can revert the cmdline_poll() back to
> > > > > cmdline_interact(), this is a small change, but not the proper way of
> > > > > > doing things, as this is simply hiding the issue in cmdline library.
> It
> > > would
> > > > > also prevent us from testing some Soft NIC functionality.

Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d

[Dumitrescu, Cristian]

> -----Original Message-----
> From: Ananyev, Konstantin
> Sent: Wednesday, July 25, 2018 1:18 PM
> To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Mordechay
> Haimovsky <motih@mellanox.com>; Thomas Monjalon
> <thomas@monjalon.net>; Singh, Jasvinder <jasvinder.singh@intel.com>
> Cc: dev@dpdk.org; Iremonger, Bernard <bernard.iremonger@intel.com>;
> Pattan, Reshma <reshma.pattan@intel.com>; olivier.matz@6wind.com; Van
> Haaren, Harry <harry.van.haaren@intel.com>
> Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d
> 
> 
> 
> > -----Original Message-----
> > From: Dumitrescu, Cristian
> > Sent: Wednesday, July 25, 2018 12:57 PM
> > To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Mordechay
> Haimovsky <motih@mellanox.com>; Thomas Monjalon
> > <thomas@monjalon.net>; Singh, Jasvinder <jasvinder.singh@intel.com>
> > Cc: dev@dpdk.org; Iremonger, Bernard <bernard.iremonger@intel.com>;
> Pattan, Reshma <reshma.pattan@intel.com>;
> > olivier.matz@6wind.com; Van Haaren, Harry
> <harry.van.haaren@intel.com>
> > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d
> >
> >
> >
> > > -----Original Message-----
> > > From: Ananyev, Konstantin
> > > Sent: Wednesday, July 25, 2018 12:39 PM
> > > To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Mordechay
> > > Haimovsky <motih@mellanox.com>; Thomas Monjalon
> > > <thomas@monjalon.net>; Singh, Jasvinder <jasvinder.singh@intel.com>
> > > Cc: dev@dpdk.org; Iremonger, Bernard <bernard.iremonger@intel.com>;
> > > Pattan, Reshma <reshma.pattan@intel.com>; olivier.matz@6wind.com;
> Van
> > > Haaren, Harry <harry.van.haaren@intel.com>
> > > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using
> ctrl+d
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Dumitrescu, Cristian
> > > > Sent: Wednesday, July 25, 2018 10:36 AM
> > > > To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Mordechay
> > > Haimovsky <motih@mellanox.com>; Thomas Monjalon
> > > > <thomas@monjalon.net>; Singh, Jasvinder
> <jasvinder.singh@intel.com>
> > > > Cc: dev@dpdk.org; Iremonger, Bernard
> <bernard.iremonger@intel.com>;
> > > Pattan, Reshma <reshma.pattan@intel.com>;
> > > > olivier.matz@6wind.com
> > > > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using
> ctrl+d
> > > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Ananyev, Konstantin
> > > > > Sent: Wednesday, July 25, 2018 10:04 AM
> > > > > To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>;
> Mordechay
> > > > > Haimovsky <motih@mellanox.com>; Thomas Monjalon
> > > > > <thomas@monjalon.net>; Singh, Jasvinder
> <jasvinder.singh@intel.com>
> > > > > Cc: dev@dpdk.org; Iremonger, Bernard
> <bernard.iremonger@intel.com>;
> > > > > Pattan, Reshma <reshma.pattan@intel.com>;
> olivier.matz@6wind.com
> > > > > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using
> > > ctrl+d
> > > > >
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of
> Dumitrescu,
> > > > > Cristian
> > > > > > Sent: Tuesday, July 24, 2018 5:59 PM
> > > > > > To: Mordechay Haimovsky <motih@mellanox.com>; Thomas
> Monjalon
> > > > > <thomas@monjalon.net>; Singh, Jasvinder
> > > > > > <jasvinder.singh@intel.com>
> > > > > > Cc: dev@dpdk.org; Iremonger, Bernard
> > > <bernard.iremonger@intel.com>;
> > > > > Pattan, Reshma <reshma.pattan@intel.com>;
> > > > > > olivier.matz@6wind.com
> > > > > > Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit
> using
> > > ctrl+d
> > > > > >
> > > > > >
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Mordechay Haimovsky [mailto:motih@mellanox.com]
> > > > > > > Sent: Tuesday, July 24, 2018 3:37 PM
> > > > > > > To: Thomas Monjalon <thomas@monjalon.net>; Singh, Jasvinder
> > > > > > > <jasvinder.singh@intel.com>
> > > > > > > Cc: dev@dpdk.org; Iremonger, Bernard
> > > <bernard.iremonger@intel.com>;
> > > > > > > Pattan, Reshma <reshma.pattan@intel.com>;
> > > olivier.matz@6wind.com;
> > > > > > > Dumitrescu, Cristian <cristian.dumitrescu@intel.com>
> > > > > > > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit
> using
> > > > > ctrl+d
> > > > > > >
> > > > > > > Even after this fix we still have setups that use netvsc  for
> example,
> > > on
> > > > > > > which testpmd exits with rte_panic right after loading it even
> > > without
> > > > > > > touching the KBD.
> > > > > > >
> > > > > > > I recommend returning the previous prompt routine in test-
> > > > > pmd/cmdline.c
> > > > > > > and rework the SOFTNIC section there, preferably moving its poll
> > > section
> > > > > to
> > > > > > > use rte_service in a separate file cleaning the CLI files from PMD-
> > > specific
> > > > > > > implementation.
> > > > > > >
> > > > > > > > -----Original Message-----
> > > > > > > > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > > > > > > > Sent: Tuesday, July 24, 2018 3:34 PM
> > > > > > > > To: Jasvinder Singh <jasvinder.singh@intel.com>
> > > > > > > > Cc: dev@dpdk.org; bernard.iremonger@intel.com;
> > > > > > > > reshma.pattan@intel.com; Mordechay Haimovsky
> > > > > > > <motih@mellanox.com>;
> > > > > > > > olivier.matz@6wind.com; cristian.dumitrescu@intel.com
> > > > > > > > Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit
> > > using
> > > > > ctrl+d
> > > > > > > >
> > > > > > > > Important note:
> > > > > > > > 	testpmd is currently really broken.
> > > > > > > > 	We cannot have a RC2 until it is fixed.
> > > > > > > >
> > > > > > > >
> > > > > > > > 24/07/2018 13:25, Thomas Monjalon:
> > > > > > > > > 23/07/2018 12:44, Jasvinder Singh:
> > > > > > > > > > Fix testpmd app exit by pressing ctrl+d, End-of-Transmission
> > > > > > > > > > character (EOT) on the empty command line.
> > > > > > > > >
> > > > > > > > > Please describe what is the issue and what is the cause.
> > > > > > > > >
> > > > > > > > > > Fixes: 0ad778b398c6 ("app/testpmd: rework softnic forward
> > > > > mode")
> > > > > > > > > >
> > > > > > > > > > Reported-by: Mordechay Haimovsky
> <motih@mellanox.com>
> > > > > > > > > > Signed-off-by: Jasvinder Singh <jasvinder.singh@intel.com>
> > > > > > > > > > ---
> > > > > > > > > >  app/test-pmd/cmdline.c       | 10 ++++++----
> > > > > > > > > >  lib/librte_cmdline/cmdline.c |  2 +-
> > > > > > > > >
> > > > > > > > > It is very suspicious to change the cmdline library.
> > > > > > > > > If there is a bug in this library, it deserves a separate fix.
> > > > > > > >
> > > > > > > >
> > > > > >
> > > > > >
> > > > > > First, testpmd is not really broken, as only thing that changed is the
> Ctrl
> > > +
> > > > > D behavior. I agree this is an issue that we need to fix, as
> > > > > > it looks that it is breaking some automation scripts for some
> people.
> > > > > >
> > > > > > The change in behavior for Ctrl + D exit is caused by replacing the
> call
> > > to
> > > > > cmdline_interact() with calling cmdline_poll() in a loop.
> > > > > > These two approaches should be identical in behavior, but it looks
> like
> > > they
> > > > > are not due to some issue in the cmdline library.
> > > > > > Jasvinder proposed a quick patch, but it looks like something else
> needs
> > > to
> > > > > be fixed in cmdline library in order to bring
> > > > > > cmdline_poll() on parity with cmdline_interact(). Any advice from
> > > Olivier
> > > > > would be very much appreciated!
> > > > > >
> > > > > > It is really a bad practice to use cmdline_interact() in testpmd, as it
> is a
> > > > > blocking call that prohibits doing other things on the same
> > > > > > thread that runs the CLI. Sometimes we need to run other things on
> the
> > > > > same core, e.g. the slow softnic_manage() function.
> > > > >
> > > > > Curious why not use a service core for softnic background stuff, and
> leave
> > > CLI
> > > > > one for CLI?
> > > > > Konstantin
> > > >
> > > > I guess for a test application you can do anything you want, but in real
> life
> > > CPU cores are really expensive and dedicating a CPU core
> > > > just for CLI is a colossal waste.
> > >
> > > Ok, but let's application developer to decide how to use (waste) the
> cores he
> > > owns :)
> > > What I am saying: there is a special thing (developed by Harry) service
> cores.
> > > As I understand why of it's the purpose - allow PMD(s) to allocate CPU
> > > resources for
> > > there background tasks in a unified and transparent way.
> > > From the description above - that seems to fit your needs (softnic
> > > background processing), no?
> > > Konstantin
> > >
> >
> > Then why not put the testpmd CLI itself on a service core? Are you
> volunteering for a patch on this? :)
> 
> It was not me who broke testpmd at first place.

I don't think you understand the issue. The issue is related to using the cmdline library in non-blocking mode, it has nothing to do with Soft NIC.

> I'd better live without softnic support in testpmd :)

Thanks a lot for your help ;(

> Though I still don't understand why do you feel that service cores are not
> good enough for you?

You don't seem to realize there is an issue that we need to root cause and hopefully fix rather than hiding it. This has nothing to do with service core.

> From my understanding they were introduced for similar purposes
> (Harry please feel free to correct me here).
> So if you think they not fit for your case -
> at least would be good to understand why.

Service core could be used here, as long as a new core is not wasted. Running this on the same core as CLI is also a valid solution.

> Konstantin
> 
> >
> >
> > > >
> > > > We did use the non-blocking cmdline_poll() function instead of the
> > > blocking cmdline_interact() in the past without any issues. The
> > > > issues reported by Moti come as a surprise. It is probably good to see
> what
> > > this is about and see if we can quickly fix the issue in
> > > > cmdline library. Otherwise, we can revert the usage of cmdline_poll()
> with
> > > cmdline_interact().
> > > >
> > > >
> > > > >
> > > > > >
> > > > > > Worst case scenario: We can revert the cmdline_poll() back to
> > > > > cmdline_interact(), this is a small change, but not the proper way of
> > > > > > doing things, as this is simply hiding the issue in cmdline library. It
> > > would
> > > > > also prevent us from testing some Soft NIC functionality.

Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d

[Dumitrescu, Cristian]

> -----Original Message-----
> From: Van Haaren, Harry
> Sent: Wednesday, July 25, 2018 1:27 PM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Dumitrescu,
> Cristian <cristian.dumitrescu@intel.com>; Mordechay Haimovsky
> <motih@mellanox.com>; Thomas Monjalon <thomas@monjalon.net>;
> Singh, Jasvinder <jasvinder.singh@intel.com>
> Cc: dev@dpdk.org; Iremonger, Bernard <bernard.iremonger@intel.com>;
> Pattan, Reshma <reshma.pattan@intel.com>; olivier.matz@6wind.com
> Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d
> 
> > -----Original Message-----
> > From: Ananyev, Konstantin
> > Sent: Wednesday, July 25, 2018 1:18 PM
> > To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Mordechay
> Haimovsky
> > <motih@mellanox.com>; Thomas Monjalon <thomas@monjalon.net>;
> Singh, Jasvinder
> > <jasvinder.singh@intel.com>
> > Cc: dev@dpdk.org; Iremonger, Bernard <bernard.iremonger@intel.com>;
> Pattan,
> > Reshma <reshma.pattan@intel.com>; olivier.matz@6wind.com; Van
> Haaren, Harry
> > <harry.van.haaren@intel.com>
> > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d
> >
> >
> >
> > > -----Original Message-----
> > > From: Dumitrescu, Cristian
> > > Sent: Wednesday, July 25, 2018 12:57 PM
> > > To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Mordechay
> Haimovsky
> > <motih@mellanox.com>; Thomas Monjalon
> > > <thomas@monjalon.net>; Singh, Jasvinder <jasvinder.singh@intel.com>
> > > Cc: dev@dpdk.org; Iremonger, Bernard <bernard.iremonger@intel.com>;
> Pattan,
> > Reshma <reshma.pattan@intel.com>;
> > > olivier.matz@6wind.com; Van Haaren, Harry
> <harry.van.haaren@intel.com>
> > > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using
> ctrl+d
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Ananyev, Konstantin
> > > > Sent: Wednesday, July 25, 2018 12:39 PM
> > > > To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Mordechay
> > > > Haimovsky <motih@mellanox.com>; Thomas Monjalon
> > > > <thomas@monjalon.net>; Singh, Jasvinder
> <jasvinder.singh@intel.com>
> > > > Cc: dev@dpdk.org; Iremonger, Bernard
> <bernard.iremonger@intel.com>;
> > > > Pattan, Reshma <reshma.pattan@intel.com>;
> olivier.matz@6wind.com; Van
> > > > Haaren, Harry <harry.van.haaren@intel.com>
> > > > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using
> ctrl+d
> > > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Dumitrescu, Cristian
> > > > > Sent: Wednesday, July 25, 2018 10:36 AM
> > > > > To: Ananyev, Konstantin <konstantin.ananyev@intel.com>;
> Mordechay
> > > > Haimovsky <motih@mellanox.com>; Thomas Monjalon
> > > > > <thomas@monjalon.net>; Singh, Jasvinder
> <jasvinder.singh@intel.com>
> > > > > Cc: dev@dpdk.org; Iremonger, Bernard
> <bernard.iremonger@intel.com>;
> > > > Pattan, Reshma <reshma.pattan@intel.com>;
> > > > > olivier.matz@6wind.com
> > > > > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using
> > ctrl+d
> > > > >
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Ananyev, Konstantin
> > > > > > Sent: Wednesday, July 25, 2018 10:04 AM
> > > > > > To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>;
> Mordechay
> > > > > > Haimovsky <motih@mellanox.com>; Thomas Monjalon
> > > > > > <thomas@monjalon.net>; Singh, Jasvinder
> <jasvinder.singh@intel.com>
> > > > > > Cc: dev@dpdk.org; Iremonger, Bernard
> <bernard.iremonger@intel.com>;
> > > > > > Pattan, Reshma <reshma.pattan@intel.com>;
> olivier.matz@6wind.com
> > > > > > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit
> using
> > > > ctrl+d
> > > > > >
> > > > > >
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of
> Dumitrescu,
> > > > > > Cristian
> > > > > > > Sent: Tuesday, July 24, 2018 5:59 PM
> > > > > > > To: Mordechay Haimovsky <motih@mellanox.com>; Thomas
> Monjalon
> > > > > > <thomas@monjalon.net>; Singh, Jasvinder
> > > > > > > <jasvinder.singh@intel.com>
> > > > > > > Cc: dev@dpdk.org; Iremonger, Bernard
> > > > <bernard.iremonger@intel.com>;
> > > > > > Pattan, Reshma <reshma.pattan@intel.com>;
> > > > > > > olivier.matz@6wind.com
> > > > > > > Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit
> using
> > > > ctrl+d
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > > -----Original Message-----
> > > > > > > > From: Mordechay Haimovsky [mailto:motih@mellanox.com]
> > > > > > > > Sent: Tuesday, July 24, 2018 3:37 PM
> > > > > > > > To: Thomas Monjalon <thomas@monjalon.net>; Singh,
> Jasvinder
> > > > > > > > <jasvinder.singh@intel.com>
> > > > > > > > Cc: dev@dpdk.org; Iremonger, Bernard
> > > > <bernard.iremonger@intel.com>;
> > > > > > > > Pattan, Reshma <reshma.pattan@intel.com>;
> > > > olivier.matz@6wind.com;
> > > > > > > > Dumitrescu, Cristian <cristian.dumitrescu@intel.com>
> > > > > > > > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit
> > using
> > > > > > ctrl+d
> > > > > > > >
> > > > > > > > Even after this fix we still have setups that use netvsc  for
> > example,
> > > > on
> > > > > > > > which testpmd exits with rte_panic right after loading it even
> > > > without
> > > > > > > > touching the KBD.
> > > > > > > >
> > > > > > > > I recommend returning the previous prompt routine in test-
> > > > > > pmd/cmdline.c
> > > > > > > > and rework the SOFTNIC section there, preferably moving its
> poll
> > > > section
> > > > > > to
> > > > > > > > use rte_service in a separate file cleaning the CLI files from
> > PMD-
> > > > specific
> > > > > > > > implementation.
> > > > > > > >
> > > > > > > > > -----Original Message-----
> > > > > > > > > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > > > > > > > > Sent: Tuesday, July 24, 2018 3:34 PM
> > > > > > > > > To: Jasvinder Singh <jasvinder.singh@intel.com>
> > > > > > > > > Cc: dev@dpdk.org; bernard.iremonger@intel.com;
> > > > > > > > > reshma.pattan@intel.com; Mordechay Haimovsky
> > > > > > > > <motih@mellanox.com>;
> > > > > > > > > olivier.matz@6wind.com; cristian.dumitrescu@intel.com
> > > > > > > > > Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd
> exit
> > > > using
> > > > > > ctrl+d
> > > > > > > > >
> > > > > > > > > Important note:
> > > > > > > > > 	testpmd is currently really broken.
> > > > > > > > > 	We cannot have a RC2 until it is fixed.
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > 24/07/2018 13:25, Thomas Monjalon:
> > > > > > > > > > 23/07/2018 12:44, Jasvinder Singh:
> > > > > > > > > > > Fix testpmd app exit by pressing ctrl+d, End-of-
> Transmission
> > > > > > > > > > > character (EOT) on the empty command line.
> > > > > > > > > >
> > > > > > > > > > Please describe what is the issue and what is the cause.
> > > > > > > > > >
> > > > > > > > > > > Fixes: 0ad778b398c6 ("app/testpmd: rework softnic
> forward
> > > > > > mode")
> > > > > > > > > > >
> > > > > > > > > > > Reported-by: Mordechay Haimovsky
> <motih@mellanox.com>
> > > > > > > > > > > Signed-off-by: Jasvinder Singh
> <jasvinder.singh@intel.com>
> > > > > > > > > > > ---
> > > > > > > > > > >  app/test-pmd/cmdline.c       | 10 ++++++----
> > > > > > > > > > >  lib/librte_cmdline/cmdline.c |  2 +-
> > > > > > > > > >
> > > > > > > > > > It is very suspicious to change the cmdline library.
> > > > > > > > > > If there is a bug in this library, it deserves a separate fix.
> > > > > > > > >
> > > > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > First, testpmd is not really broken, as only thing that changed is
> > the Ctrl
> > > > +
> > > > > > D behavior. I agree this is an issue that we need to fix, as
> > > > > > > it looks that it is breaking some automation scripts for some
> > people.
> > > > > > >
> > > > > > > The change in behavior for Ctrl + D exit is caused by replacing the
> > call
> > > > to
> > > > > > cmdline_interact() with calling cmdline_poll() in a loop.
> > > > > > > These two approaches should be identical in behavior, but it looks
> > like
> > > > they
> > > > > > are not due to some issue in the cmdline library.
> > > > > > > Jasvinder proposed a quick patch, but it looks like something else
> > needs
> > > > to
> > > > > > be fixed in cmdline library in order to bring
> > > > > > > cmdline_poll() on parity with cmdline_interact(). Any advice from
> > > > Olivier
> > > > > > would be very much appreciated!
> > > > > > >
> > > > > > > It is really a bad practice to use cmdline_interact() in testpmd, as
> > it is a
> > > > > > blocking call that prohibits doing other things on the same
> > > > > > > thread that runs the CLI. Sometimes we need to run other things
> on
> > the
> > > > > > same core, e.g. the slow softnic_manage() function.
> > > > > >
> > > > > > Curious why not use a service core for softnic background stuff, and
> > leave
> > > > CLI
> > > > > > one for CLI?
> > > > > > Konstantin
> > > > >
> > > > > I guess for a test application you can do anything you want, but in
> real
> > life
> > > > CPU cores are really expensive and dedicating a CPU core
> > > > > just for CLI is a colossal waste.
> > > >
> > > > Ok, but let's application developer to decide how to use (waste) the
> cores
> > he
> > > > owns :)
> > > > What I am saying: there is a special thing (developed by Harry) service
> > cores.
> > > > As I understand why of it's the purpose - allow PMD(s) to allocate CPU
> > > > resources for
> > > > there background tasks in a unified and transparent way.
> > > > From the description above - that seems to fit your needs (softnic
> > > > background processing), no?
> > > > Konstantin
> > > >
> > >
> > > Then why not put the testpmd CLI itself on a service core? Are you
> > volunteering for a patch on this? :)
> >
> > It was not me who broke testpmd at first place.
> > I'd better live without softnic support in testpmd :)
> > Though I still don't understand why do you feel that service cores are not
> > good enough for you?
> > From my understanding they were introduced for similar purposes
> > (Harry please feel free to correct me here).
> > So if you think they not fit for your case -
> > at least would be good to understand why.
> > Konstantin
> 
> 
> Service-cores is a mechanism to abstract SW fallback CPU core
> requirements;
> Eg: A nic provides RSS, 0 cores required
>     A NIC does not provide RSS, use 1 service core to provide that RSS
> function
> 
> Result: the application can rely on RSS, as we have a transparent fallback:
> no application controlled lcore is performing any different than before with
> HW provided functionality
> 
> Service cores is not intended to be used for "background" tasks, as
> the service-core consumes an Lcore and polls 100% of the time, regardless
> of what services it is performing.
> 
> 
> If there are multiple things that we can run on service-cores, that would
> make
> sense. Eg: timer_manage(), CLI (non-blocking..), SoftNIC background tasks...
> 
> This design makes sense as the application can also register its own
> "background"
> services, and run them all on a single service-core.
> 
> 

Thanks, Harry, we might have to revert to service core if we are not able to reproduce & fix this issue quickly enough.

> > >
> > >
> > > > >
> > > > > We did use the non-blocking cmdline_poll() function instead of the
> > > > blocking cmdline_interact() in the past without any issues. The
> > > > > issues reported by Moti come as a surprise. It is probably good to see
> > what
> > > > this is about and see if we can quickly fix the issue in
> > > > > cmdline library. Otherwise, we can revert the usage of cmdline_poll()
> > with
> > > > cmdline_interact().
> > > > >
> > > > >
> > > > > >
> > > > > > >
> > > > > > > Worst case scenario: We can revert the cmdline_poll() back to
> > > > > > cmdline_interact(), this is a small change, but not the proper way
> of
> > > > > > > doing things, as this is simply hiding the issue in cmdline library.
> > It
> > > > would
> > > > > > also prevent us from testing some Soft NIC functionality.

Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d

[Ananyev, Konstantin]

> -----Original Message-----
> From: Dumitrescu, Cristian
> Sent: Wednesday, July 25, 2018 1:41 PM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Mordechay Haimovsky <motih@mellanox.com>; Thomas Monjalon
> <thomas@monjalon.net>; Singh, Jasvinder <jasvinder.singh@intel.com>
> Cc: dev@dpdk.org; Iremonger, Bernard <bernard.iremonger@intel.com>; Pattan, Reshma <reshma.pattan@intel.com>;
> olivier.matz@6wind.com; Van Haaren, Harry <harry.van.haaren@intel.com>
> Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d
> 
> 
> 
> > -----Original Message-----
> > From: Ananyev, Konstantin
> > Sent: Wednesday, July 25, 2018 1:18 PM
> > To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Mordechay
> > Haimovsky <motih@mellanox.com>; Thomas Monjalon
> > <thomas@monjalon.net>; Singh, Jasvinder <jasvinder.singh@intel.com>
> > Cc: dev@dpdk.org; Iremonger, Bernard <bernard.iremonger@intel.com>;
> > Pattan, Reshma <reshma.pattan@intel.com>; olivier.matz@6wind.com; Van
> > Haaren, Harry <harry.van.haaren@intel.com>
> > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d
> >
> >
> >
> > > -----Original Message-----
> > > From: Dumitrescu, Cristian
> > > Sent: Wednesday, July 25, 2018 12:57 PM
> > > To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Mordechay
> > Haimovsky <motih@mellanox.com>; Thomas Monjalon
> > > <thomas@monjalon.net>; Singh, Jasvinder <jasvinder.singh@intel.com>
> > > Cc: dev@dpdk.org; Iremonger, Bernard <bernard.iremonger@intel.com>;
> > Pattan, Reshma <reshma.pattan@intel.com>;
> > > olivier.matz@6wind.com; Van Haaren, Harry
> > <harry.van.haaren@intel.com>
> > > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Ananyev, Konstantin
> > > > Sent: Wednesday, July 25, 2018 12:39 PM
> > > > To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Mordechay
> > > > Haimovsky <motih@mellanox.com>; Thomas Monjalon
> > > > <thomas@monjalon.net>; Singh, Jasvinder <jasvinder.singh@intel.com>
> > > > Cc: dev@dpdk.org; Iremonger, Bernard <bernard.iremonger@intel.com>;
> > > > Pattan, Reshma <reshma.pattan@intel.com>; olivier.matz@6wind.com;
> > Van
> > > > Haaren, Harry <harry.van.haaren@intel.com>
> > > > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using
> > ctrl+d
> > > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Dumitrescu, Cristian
> > > > > Sent: Wednesday, July 25, 2018 10:36 AM
> > > > > To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Mordechay
> > > > Haimovsky <motih@mellanox.com>; Thomas Monjalon
> > > > > <thomas@monjalon.net>; Singh, Jasvinder
> > <jasvinder.singh@intel.com>
> > > > > Cc: dev@dpdk.org; Iremonger, Bernard
> > <bernard.iremonger@intel.com>;
> > > > Pattan, Reshma <reshma.pattan@intel.com>;
> > > > > olivier.matz@6wind.com
> > > > > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using
> > ctrl+d
> > > > >
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Ananyev, Konstantin
> > > > > > Sent: Wednesday, July 25, 2018 10:04 AM
> > > > > > To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>;
> > Mordechay
> > > > > > Haimovsky <motih@mellanox.com>; Thomas Monjalon
> > > > > > <thomas@monjalon.net>; Singh, Jasvinder
> > <jasvinder.singh@intel.com>
> > > > > > Cc: dev@dpdk.org; Iremonger, Bernard
> > <bernard.iremonger@intel.com>;
> > > > > > Pattan, Reshma <reshma.pattan@intel.com>;
> > olivier.matz@6wind.com
> > > > > > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using
> > > > ctrl+d
> > > > > >
> > > > > >
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of
> > Dumitrescu,
> > > > > > Cristian
> > > > > > > Sent: Tuesday, July 24, 2018 5:59 PM
> > > > > > > To: Mordechay Haimovsky <motih@mellanox.com>; Thomas
> > Monjalon
> > > > > > <thomas@monjalon.net>; Singh, Jasvinder
> > > > > > > <jasvinder.singh@intel.com>
> > > > > > > Cc: dev@dpdk.org; Iremonger, Bernard
> > > > <bernard.iremonger@intel.com>;
> > > > > > Pattan, Reshma <reshma.pattan@intel.com>;
> > > > > > > olivier.matz@6wind.com
> > > > > > > Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit
> > using
> > > > ctrl+d
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > > -----Original Message-----
> > > > > > > > From: Mordechay Haimovsky [mailto:motih@mellanox.com]
> > > > > > > > Sent: Tuesday, July 24, 2018 3:37 PM
> > > > > > > > To: Thomas Monjalon <thomas@monjalon.net>; Singh, Jasvinder
> > > > > > > > <jasvinder.singh@intel.com>
> > > > > > > > Cc: dev@dpdk.org; Iremonger, Bernard
> > > > <bernard.iremonger@intel.com>;
> > > > > > > > Pattan, Reshma <reshma.pattan@intel.com>;
> > > > olivier.matz@6wind.com;
> > > > > > > > Dumitrescu, Cristian <cristian.dumitrescu@intel.com>
> > > > > > > > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit
> > using
> > > > > > ctrl+d
> > > > > > > >
> > > > > > > > Even after this fix we still have setups that use netvsc  for
> > example,
> > > > on
> > > > > > > > which testpmd exits with rte_panic right after loading it even
> > > > without
> > > > > > > > touching the KBD.
> > > > > > > >
> > > > > > > > I recommend returning the previous prompt routine in test-
> > > > > > pmd/cmdline.c
> > > > > > > > and rework the SOFTNIC section there, preferably moving its poll
> > > > section
> > > > > > to
> > > > > > > > use rte_service in a separate file cleaning the CLI files from PMD-
> > > > specific
> > > > > > > > implementation.
> > > > > > > >
> > > > > > > > > -----Original Message-----
> > > > > > > > > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > > > > > > > > Sent: Tuesday, July 24, 2018 3:34 PM
> > > > > > > > > To: Jasvinder Singh <jasvinder.singh@intel.com>
> > > > > > > > > Cc: dev@dpdk.org; bernard.iremonger@intel.com;
> > > > > > > > > reshma.pattan@intel.com; Mordechay Haimovsky
> > > > > > > > <motih@mellanox.com>;
> > > > > > > > > olivier.matz@6wind.com; cristian.dumitrescu@intel.com
> > > > > > > > > Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit
> > > > using
> > > > > > ctrl+d
> > > > > > > > >
> > > > > > > > > Important note:
> > > > > > > > > 	testpmd is currently really broken.
> > > > > > > > > 	We cannot have a RC2 until it is fixed.
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > 24/07/2018 13:25, Thomas Monjalon:
> > > > > > > > > > 23/07/2018 12:44, Jasvinder Singh:
> > > > > > > > > > > Fix testpmd app exit by pressing ctrl+d, End-of-Transmission
> > > > > > > > > > > character (EOT) on the empty command line.
> > > > > > > > > >
> > > > > > > > > > Please describe what is the issue and what is the cause.
> > > > > > > > > >
> > > > > > > > > > > Fixes: 0ad778b398c6 ("app/testpmd: rework softnic forward
> > > > > > mode")
> > > > > > > > > > >
> > > > > > > > > > > Reported-by: Mordechay Haimovsky
> > <motih@mellanox.com>
> > > > > > > > > > > Signed-off-by: Jasvinder Singh <jasvinder.singh@intel.com>
> > > > > > > > > > > ---
> > > > > > > > > > >  app/test-pmd/cmdline.c       | 10 ++++++----
> > > > > > > > > > >  lib/librte_cmdline/cmdline.c |  2 +-
> > > > > > > > > >
> > > > > > > > > > It is very suspicious to change the cmdline library.
> > > > > > > > > > If there is a bug in this library, it deserves a separate fix.
> > > > > > > > >
> > > > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > First, testpmd is not really broken, as only thing that changed is the
> > Ctrl
> > > > +
> > > > > > D behavior. I agree this is an issue that we need to fix, as
> > > > > > > it looks that it is breaking some automation scripts for some
> > people.
> > > > > > >
> > > > > > > The change in behavior for Ctrl + D exit is caused by replacing the
> > call
> > > > to
> > > > > > cmdline_interact() with calling cmdline_poll() in a loop.
> > > > > > > These two approaches should be identical in behavior, but it looks
> > like
> > > > they
> > > > > > are not due to some issue in the cmdline library.
> > > > > > > Jasvinder proposed a quick patch, but it looks like something else
> > needs
> > > > to
> > > > > > be fixed in cmdline library in order to bring
> > > > > > > cmdline_poll() on parity with cmdline_interact(). Any advice from
> > > > Olivier
> > > > > > would be very much appreciated!
> > > > > > >
> > > > > > > It is really a bad practice to use cmdline_interact() in testpmd, as it
> > is a
> > > > > > blocking call that prohibits doing other things on the same
> > > > > > > thread that runs the CLI. Sometimes we need to run other things on
> > the
> > > > > > same core, e.g. the slow softnic_manage() function.
> > > > > >
> > > > > > Curious why not use a service core for softnic background stuff, and
> > leave
> > > > CLI
> > > > > > one for CLI?
> > > > > > Konstantin
> > > > >
> > > > > I guess for a test application you can do anything you want, but in real
> > life
> > > > CPU cores are really expensive and dedicating a CPU core
> > > > > just for CLI is a colossal waste.
> > > >
> > > > Ok, but let's application developer to decide how to use (waste) the
> > cores he
> > > > owns :)
> > > > What I am saying: there is a special thing (developed by Harry) service
> > cores.
> > > > As I understand why of it's the purpose - allow PMD(s) to allocate CPU
> > > > resources for
> > > > there background tasks in a unified and transparent way.
> > > > From the description above - that seems to fit your needs (softnic
> > > > background processing), no?
> > > > Konstantin
> > > >
> > >
> > > Then why not put the testpmd CLI itself on a service core? Are you
> > volunteering for a patch on this? :)
> >
> > It was not me who broke testpmd at first place.
> 
> I don't think you understand the issue. The issue is related to using the cmdline library in non-blocking mode, it has nothing to do
> with Soft NIC.

Yep, I understand that.
What I am trying to say: you introduce rte_pmd_softnic_manage(),
and, as I understand, it is a user responsibility to call it from time to time
to keep softnic working correctly.
That's ok, but it means that now any app that wants to use softnic
has to be modified, correct?
While if you put that into service core - only startup EAL cmdline has to be changed. 
Though yes there are drawbacks: softnic would rely on presence of service core,
and if there is no other services registered - it would probably be a waste of cycles.
But it probably could be made configurable
(does user want to call softnic_manage(), or is it ok to run it as a service). 
Another thought - could it be registered as alarm handler, so it could be called from
DPDK interrupt thread?
Konstantin

> 
> > I'd better live without softnic support in testpmd :)
> 
> Thanks a lot for your help ;(
> 
> > Though I still don't understand why do you feel that service cores are not
> > good enough for you?
> 
> You don't seem to realize there is an issue that we need to root cause and hopefully fix rather than hiding it. This has nothing to do
> with service core.
> 
> > From my understanding they were introduced for similar purposes
> > (Harry please feel free to correct me here).
> > So if you think they not fit for your case -
> > at least would be good to understand why.
> 
> Service core could be used here, as long as a new core is not wasted. Running this on the same core as CLI is also a valid solution.
> 
> > Konstantin
> >
> > >
> > >
> > > > >
> > > > > We did use the non-blocking cmdline_poll() function instead of the
> > > > blocking cmdline_interact() in the past without any issues. The
> > > > > issues reported by Moti come as a surprise. It is probably good to see
> > what
> > > > this is about and see if we can quickly fix the issue in
> > > > > cmdline library. Otherwise, we can revert the usage of cmdline_poll()
> > with
> > > > cmdline_interact().
> > > > >
> > > > >
> > > > > >
> > > > > > >
> > > > > > > Worst case scenario: We can revert the cmdline_poll() back to
> > > > > > cmdline_interact(), this is a small change, but not the proper way of
> > > > > > > doing things, as this is simply hiding the issue in cmdline library. It
> > > > would
> > > > > > also prevent us from testing some Soft NIC functionality.

Re: [dpdk-dev] [PATCH v2] app/testpmd: fix testpmd exit for ctrl+d

[Thomas Monjalon]

24/07/2018 16:46, Pattan, Reshma:
> + CC: Olivier and Mordechay Haimovsky. 
> 
> > Testpmd should be existed gracefully when ctrl+d is typed.
> > This behaviour is not handled properly, fix this by calling
> > pmd_test_exit() instead of rte_panic.
> > 
> > Fixes: 0ad778b398c6 ("app/testpmd: rework softnic forward mode")
> > 
> > Reported-by: Mordechay Haimovsky <motih@mellanox.com>
> > Signed-off-by: Jasvinder Singh <jasvinder.singh@intel.com>
> > Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
> > ---
> > v2: removed changes done to lib/librte_cmdline/cmdline.c reworded commit
> > message and heading.

The partial revert from Moti is preferred.