Re: [dpdk-dev] [PATCH v2] testpmd: call cleanup on exit

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 1/9/2020 4:13 PM, Stephen Hemminger wrote:
> On Thu, 9 Jan 2020 11:31:30 +0000
> Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> 
>> On 1/9/2020 11:16 AM, Ferruh Yigit wrote:
>>> On 1/8/2020 3:28 PM, Stephen Hemminger wrote:  
>>>> On Wed, 8 Jan 2020 09:45:54 +0000
>>>> "Iremonger, Bernard" <bernard.iremonger@intel.com> wrote:
>>>>  
>>>>> Hi Stephen,
>>>>>  
>>>>>> -----Original Message-----
>>>>>> From: dev <dev-bounces@dpdk.org> On Behalf Of Stephen Hemminger
>>>>>> Sent: Tuesday, January 7, 2020 7:00 PM
>>>>>> To: dev@dpdk.org
>>>>>> Cc: Stephen Hemminger <stephen@networkplumber.org>
>>>>>> Subject: [dpdk-dev] [PATCH v2] testpmd: call cleanup on exit
>>>>>>
>>>>>> The rte_eal_cleanup code is not exercised by testpmd which is the most
>>>>>> used DPDK test tool. Add a call at end of program.
>>>>>>
>>>>>> This helps exercise free and close paths which can be checked with tools like
>>>>>> valgrind.
>>>>>>
>>>>>> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
>>>>>> ---
>>>>>> v2 - report errors
>>>>>>
>>>>>>  app/test-pmd/testpmd.c | 7 ++++++-
>>>>>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
>>>>>> b3746822366f..2eec8afda1ec 100644
>>>>>> --- a/app/test-pmd/testpmd.c
>>>>>> +++ b/app/test-pmd/testpmd.c
>>>>>> @@ -3570,5 +3570,10 @@ main(int argc, char** argv)
>>>>>>  			return 1;
>>>>>>  	}
>>>>>>
>>>>>> -	return 0;
>>>>>> +	ret = rte_eal_cleanup();
>>>>>> +	if (ret != 0)
>>>>>> +		rte_exit(EXIT_FAILURE,
>>>>>> +			 "EAL cleanup failed: %s\n", strerror(-ret));
>>>>>> +
>>>>>> +	return EXIT_SUCCESS;
>>>>>>  }
>>>>>> --
>>>>>> 2.20.1    
>>>>>
>>>>> This looks like a fix to me and is probably worth backporting, it should probably have a fixes line.
>>>>>
>>>>> Regards,
>>>>>
>>>>> Bernard.
>>>>>  
>>>>
>>>> Not sure if it needs to be backported. It depends on the definition
>>>> of a fix.  The original definition of stable was fixes only.
>>>> Linux has moved on to fixes and backports of missing features
>>>> like this; but DPDK has mostly stuck to the original definition
>>>> of stable.
>>>>  
>>>
>>> I also tend to think this a fix more then a feature, and although it is a minor
>>> fix it is good to get it to reduce the change of the conflict on other stuff in
>>> LTS. I will add fix/stable tag while merging.
>>>   
>>
>>     Fixes: af75078fece3 ("first public release")
>>     Cc: stable@dpdk.org
>>
>>     Acked-by: Bernard Iremonger <bernard.iremonger@intel.com>
>>
>> Applied to dpdk-next-net/master, thanks.
> 
> Thanks,  my concern about adding it to stable is that it might expose
> bugs in other places (like drivers).  That is good for current release
> but not something stable users want to see.
> 

Overall I agree, but the 'rte_eal_cleanup()' is pretty simple as of now, only
doing a few simple cleanups not full cleanup, that is why I believe it can be OK
to add it.