DPDK patches and discussions
 help / color / mirror / Atom feed
From: Ferruh Yigit <ferruh.yigit@intel.com>
To: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>,
	"Li, Xiaoyun" <xiaoyun.li@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH] app/testpmd: send failure logs to stderr
Date: Fri, 11 Jun 2021 11:35:59 +0100	[thread overview]
Message-ID: <87dd3fd0-9dab-8079-d135-50d966d6a5cd@intel.com> (raw)
In-Reply-To: <1f419fbb-b951-1a84-3329-97701c32c956@oktetlabs.ru>

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.

And because of same reason perhaps a release note can be added.

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.
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>
> 


  reply	other threads:[~2021-06-11 10:36 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-27 16:24 Andrew Rybchenko
2021-06-11  2:06 ` Li, Xiaoyun
2021-06-11  9:19   ` Andrew Rybchenko
2021-06-11 10:35     ` Ferruh Yigit [this message]
2021-06-11 13:21       ` Bruce Richardson
2021-06-14 16:47         ` Andrew Rybchenko
2021-06-14 16:56       ` Andrew Rybchenko
2021-06-14 17:49         ` Singh, Aman Deep
2021-06-15  7:59         ` Ferruh Yigit
2021-06-15  8:14           ` Andrew Rybchenko
2021-06-15  8:52             ` Ferruh Yigit
2021-06-15  9:00               ` Andrew Rybchenko
2021-06-15  9:53                 ` Ferruh Yigit
2021-06-16 16:32 ` [dpdk-dev] [PATCH v2] " Andrew Rybchenko
2021-06-17  3:29   ` Li, Xiaoyun
2021-06-17 14:21     ` Andrew Rybchenko
2021-06-17 14:20 ` [dpdk-dev] [PATCH v3] " Andrew Rybchenko
2021-06-18  8:32   ` Li, Xiaoyun
2021-06-28 11:05     ` Andrew Rybchenko
2021-07-24 13:14     ` Thomas Monjalon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87dd3fd0-9dab-8079-d135-50d966d6a5cd@intel.com \
    --to=ferruh.yigit@intel.com \
    --cc=andrew.rybchenko@oktetlabs.ru \
    --cc=dev@dpdk.org \
    --cc=xiaoyun.li@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).