Re: [dpdk-dev] [PATCH] app/test-pmd: stop all ports before any close

["Dumitrescu, Cristian" <cristian.dumitrescu@intel.com>]

Hi Bernard,

Thanks for reviewing this patch.

> -----Original Message-----
> From: Iremonger, Bernard
> Sent: Friday, January 4, 2019 10:16 AM
> To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; dev@dpdk.org
> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>
> Subject: RE: [PATCH] app/test-pmd: stop all ports before any close
> 
> Hi Cristian,
> 
> > -----Original Message-----
> > From: Dumitrescu, Cristian
> > Sent: Thursday, January 3, 2019 4:37 PM
> > To: dev@dpdk.org
> > Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing
> > <jingjing.wu@intel.com>; Iremonger, Bernard
> <bernard.iremonger@intel.com>
> > Subject: [PATCH] app/test-pmd: stop all ports before any close
> 
> ./devtools/check-git-log.sh -1
> Wrong headline label:
>         app/test-pmd: stop all ports before any close
> 
> Should be app/testpmd

Will fix.

> 
> The commit message should also have the "fix" keyword
> 

We can treat this as a fix or as an improvement, I was not sure about what people think. I don't mind either way. Will treat this as a fix then in v2.

> >
> > This patch proposes a slightly different test-pmd quit operation: stop all
> devices
> > before starting to close any device. Basically, stop all moving parts before
> > beginning to remove them. The current test-pmd quit is stoping and closing
> each
> > device before moving to the next device.
> >
> > If all devices in the system are independent of each other, this difference is
> > usually not important. In case of Soft NIC devices, any such virtual device
> > typically depends on one or more physical devices being alive, as it accesses
> > their queues, so this difference becomes important.
> >
> > Without this straightforward fix, all the Soft NIC devices need to be
> manually
> > stopped before the quit command is issued, otherwise the quit command
> can
> > sometimes crash the test-pmd application.
> 
> There should be a fixes line before the sign-off, and maybe a " Cc:
> stable@dpdk.org" line
> 
> > Signed-off-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
> > ---
> >  app/test-pmd/testpmd.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> > 8d584b008..15a948828 100644
> > --- a/app/test-pmd/testpmd.c
> > +++ b/app/test-pmd/testpmd.c
> > @@ -2391,9 +2391,13 @@ pmd_test_exit(void)
> >  	if (ports != NULL) {
> >  		no_link_check = 1;
> >  		RTE_ETH_FOREACH_DEV(pt_id) {
> > -			printf("\nShutting down port %d...\n", pt_id);
> > +			printf("\nStopping port %d...\n", pt_id);
> >  			fflush(stdout);
> >  			stop_port(pt_id);
> > +		}
> > +		RTE_ETH_FOREACH_DEV(pt_id) {
> > +			printf("\nShutting down port %d...\n", pt_id);
> > +			fflush(stdout);
> >  			close_port(pt_id);
> >
> >  			/*
> > --
> > 2.17.1
> 
> Regards,
> 
> Bernard.

Regards,
Cristian