diff mbox

[dpdk-dev] testpmd: Fix wrong message in testpmd

Message ID 1435132585-10192-1-git-send-email-michael.qiu@intel.com (mailing list archive)
State Accepted, archived
Headers

Commit Message

Michael Qiu June 24, 2015, 7:56 a.m. UTC 
  When close one port twice, testpmd will give out wrong messagse.

testpmd> port stop  0
Stopping ports...
Checking link statuses...
Port 0 Link Up - speed 0 Mbps - full-duplex
Port 1 Link Up - speed 0 Mbps - full-duplex
Done
testpmd> port close 0
Closing ports...
Done
testpmd> port close 0
Closing ports...
Port 0 is now not stopped
Done
testpmd> 


Signed-off-by: Michael Qiu <michael.qiu@intel.com>
---
 app/test-pmd/testpmd.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Michael Qiu July 8, 2015, 7:16 a.m. UTC | #1 
Any comments? This is a bug fix, not a feature.

Thanks,
Michael

On 6/24/2015 3:56 PM, Qiu, Michael wrote:
> When close one port twice, testpmd will give out wrong messagse.
>
> testpmd> port stop  0
> Stopping ports...
> Checking link statuses...
> Port 0 Link Up - speed 0 Mbps - full-duplex
> Port 1 Link Up - speed 0 Mbps - full-duplex
> Done
> testpmd> port close 0
> Closing ports...
> Done
> testpmd> port close 0
> Closing ports...
> Port 0 is now not stopped
> Done
> testpmd> 
>
>
> Signed-off-by: Michael Qiu <michael.qiu@intel.com>
> ---
>  app/test-pmd/testpmd.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index 3057791..907cda3 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -1534,6 +1534,12 @@ close_port(portid_t pid)
>  
>  		port = &ports[pi];
>  		if (rte_atomic16_cmpset(&(port->port_status),
> +			RTE_PORT_CLOSED, RTE_PORT_CLOSED) == 1) {
> +			printf("Port %d is already closed\n", pi);
> +			continue;
> +		}
> +
> +		if (rte_atomic16_cmpset(&(port->port_status),
>  			RTE_PORT_STOPPED, RTE_PORT_HANDLING) == 0) {
>  			printf("Port %d is now not stopped\n", pi);
>  			continue;
Bruce Richardson July 8, 2015, 9:04 a.m. UTC | #2 
On Wed, Jul 08, 2015 at 07:16:21AM +0000, Qiu, Michael wrote:
> Any comments? This is a bug fix, not a feature.
> 
> Thanks,
> Michael
> 
> On 6/24/2015 3:56 PM, Qiu, Michael wrote:
> > When close one port twice, testpmd will give out wrong messagse.
> >
> > testpmd> port stop  0
> > Stopping ports...
> > Checking link statuses...
> > Port 0 Link Up - speed 0 Mbps - full-duplex
> > Port 1 Link Up - speed 0 Mbps - full-duplex
> > Done
> > testpmd> port close 0
> > Closing ports...
> > Done
> > testpmd> port close 0
> > Closing ports...
> > Port 0 is now not stopped
> > Done
> > testpmd> 
> >
> >
> > Signed-off-by: Michael Qiu <michael.qiu@intel.com>
> > ---
> >  app/test-pmd/testpmd.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> > index 3057791..907cda3 100644
> > --- a/app/test-pmd/testpmd.c
> > +++ b/app/test-pmd/testpmd.c
> > @@ -1534,6 +1534,12 @@ close_port(portid_t pid)
> >  
> >  		port = &ports[pi];
> >  		if (rte_atomic16_cmpset(&(port->port_status),
> > +			RTE_PORT_CLOSED, RTE_PORT_CLOSED) == 1) {
> > +			printf("Port %d is already closed\n", pi);
> > +			continue;
> > +		}
> > +
> > +		if (rte_atomic16_cmpset(&(port->port_status),
> >  			RTE_PORT_STOPPED, RTE_PORT_HANDLING) == 0) {
> >  			printf("Port %d is now not stopped\n", pi);
> >  			continue;

I know it's not part of your change, but "Now not stopped" doesn't really seem
right to me. What is the message actually trying to report?

/Bruce
>
Michael Qiu July 28, 2015, 6:44 p.m. UTC | #3 
On 2015/7/8 2:04, Richardson, Bruce wrote:
> On Wed, Jul 08, 2015 at 07:16:21AM +0000, Qiu, Michael wrote:

[.../...]

>>>  		port = &ports[pi];
>>>  		if (rte_atomic16_cmpset(&(port->port_status),
>>> +			RTE_PORT_CLOSED, RTE_PORT_CLOSED) == 1) {
>>> +			printf("Port %d is already closed\n", pi);
>>> +			continue;
>>> +		}
>>> +
>>> +		if (rte_atomic16_cmpset(&(port->port_status),
>>>  			RTE_PORT_STOPPED, RTE_PORT_HANDLING) == 0) {
>>>  			printf("Port %d is now not stopped\n", pi);
>>>  			continue;
> I know it's not part of your change, but "Now not stopped" doesn't really seem
> right to me. What is the message actually trying to report?

It is just make sure the port is in stopped state. So it will check if
it is not in RTE_PORT_STOPPED stat or fail to set to RTE_PORT_HANDLING,
it will report as "now not stopped"


Thanks,
Michael
>
> /Bruce
Thomas Monjalon July 29, 2015, 10:20 p.m. UTC | #4 
2015-06-24 15:56, Michael Qiu:
> When close one port twice, testpmd will give out wrong messagse.
> 
> testpmd> port stop  0
> Stopping ports...
> Checking link statuses...
> Port 0 Link Up - speed 0 Mbps - full-duplex
> Port 1 Link Up - speed 0 Mbps - full-duplex
> Done
> testpmd> port close 0
> Closing ports...
> Done
> testpmd> port close 0
> Closing ports...
> Port 0 is now not stopped
> Done
> testpmd> 
> 
> 
> Signed-off-by: Michael Qiu <michael.qiu@intel.com>

Applied, thanks
diff mbox

Patch

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 3057791..907cda3 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -1534,6 +1534,12 @@  close_port(portid_t pid)
 
 		port = &ports[pi];
 		if (rte_atomic16_cmpset(&(port->port_status),
+			RTE_PORT_CLOSED, RTE_PORT_CLOSED) == 1) {
+			printf("Port %d is already closed\n", pi);
+			continue;
+		}
+
+		if (rte_atomic16_cmpset(&(port->port_status),
 			RTE_PORT_STOPPED, RTE_PORT_HANDLING) == 0) {
 			printf("Port %d is now not stopped\n", pi);
 			continue;