On 6/15/2021 9:14 AM, Andrew Rybchenko wrote: > On 6/15/21 10:59 AM, Ferruh Yigit wrote: >> On 6/14/2021 5:56 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.rybche...@oktetlabs.ru> >>>>>> Sent: Friday, May 28, 2021 00:25 >>>>>> To: Li, Xiaoyun <xiaoyun...@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.rybche...@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. >>> >> >> Why application should not take log configuration and log level settings into >> account? I think this is a feature we can benefit. > > For me it sounds like an extra way to shoot its own leg. >
Please explain what is the cons of it? >> And for not logging to syslog, I think it is DPDK wide concern, not specific >> to >> testpmd, we should have way to say don't log to syslog or only log error to >> syslog etc.. When it is done, using 'TESTPMD_LOG' enables benefiting from >> that. > > Logging configuration should be flexible to support various > logging backends. IMHO, we don't need the flexibility here. > testpmd is a command-line test application and errors should > simply go to stderr. That's it. Since the result is the > same in both ways, my opinion is not strong, I'm just trying > to explain why I slightly prefer suggested way. > Ability to make it less or more verbose seems good opinion to me, just printf/fprintf doesn't enable it. And testpmd sometimes used in non-interactive mode for functional testing, flexible logging can help there too, I think at least. > I can switch to TESTPMD_LOG() (or define TESTPMD_ERR() and use > it) easily. I just need maintainers decision on it. > >>>> 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...@intel.com> >>>>> >>> >
