Re: [dpdk-dev] [PATCH] app/testpmd: send failure logs to stderr

["Singh, Aman Deep" <aman.deep.singh@intel.com>]

On 6/14/2021 10:26 PM, Andrew Rybchenko wrote:
> On 6/11/21 1:35 PM, Ferruh Yigit wrote:
>> On 6/11/2021 10:19 AM, Andrew Rybchenko wrote:
>>> On 6/11/21 5:06 AM, Li, Xiaoyun wrote:
>>>> Hi
>>>> -----Original Message-----
>>>> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>>>> Sent: Friday, May 28, 2021 00:25
>>>> To: Li, Xiaoyun <xiaoyun.li@intel.com>
>>>> Cc: dev@dpdk.org
>>>> Subject: [PATCH] app/testpmd: send failure logs to stderr
>>>>
>>>> Running with stdout suppressed or redirected for further processing 
>>>> is very confusing in the case of errors.
>>>>
>>>> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>>>> ---
>>>>
>>>> This patch looks good to me.
>>>> But what do you think about make it as a fix and backport to stable 
>>>> branches?
>>>> Anyway works for me.
>>>
>>> I have no strong opinion on the topic.
>>>
>>> @Ferruh, what do you think?
>>>
>>
>> Same here, no strong opinion.
>> Sending errors to 'stderr' looks correct thing to do, but changing 
>> behavior in
>> the LTS may cause some unexpected side affect, if it is scripted and 
>> testpmd
>> output is parsed etc... For this possibility I would wait for the 
>> next LTS.
>
> So, I guess all agree that backporting to LTS is a bad idea because of
> behaviour change.
>
> As I said in a sub-thread I tend to apply in v21.08 since it is a right
> thing to do like a fix, but the fix is not that required to be
> backported to change behaviour of LTS releases.
>
>> And because of same reason perhaps a release note can be added.
>
> I'll make v2 with release notes added. Good point.
>
>> Also there is 'TESTPMD_LOG' macro for logs in testpmd, (as well as 
>> 'RTE_LOG'
>> macro), I don't know if we should switch all logs, including 
>> 'printf', to
>> 'TESTPMD_LOG' macro?
>> Later stdout/sderr can be managed in rte_log level, instead of any 
>> specific
>> logic for the testpmd.
>
> I think fprintf() is a better option for debug tool, since its
> messages should not go to syslog etc. It should go to stdout/stderr
> regardless of logging configuration and log level settings.

For loging, currently we are using multiple API's in testpmd 
(TESTPMD_LOG, RTE_LOG,  snprintf, printf). I agree with Ferruh, in long 
run we should use single loging mechanism where log_level should decide 
the output stream.

For now fprintf() seems OK, as we can pass the output stream compared to 
printf().

>
>> Even there was a defect for this in the rte_log level, that logs 
>> should go to
>> stderr: https://bugs.dpdk.org/show_bug.cgi?id=8
>>
>>
>>>> Acked-by: Xiaoyun Li <xiaoyun.li@intel.com>
>>>
>
Acked-by: Aman Deep Singh <aman.deep.singh@intel.com 
<mailto:aman.deep.singh@intel.com>