From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 3037EA0C49; Tue, 15 Jun 2021 10:52:22 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 174EE4067A; Tue, 15 Jun 2021 10:52:22 +0200 (CEST) Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by mails.dpdk.org (Postfix) with ESMTP id 3936E40140 for ; Tue, 15 Jun 2021 10:52:20 +0200 (CEST) IronPort-SDR: eyxAMYr8fhrOSPnbOZUI0jAmcQPIuM1QhaA45+aLIu3uXPES2DP1rCeIzI7blqpDKbJKJf1BE4 8kHrBDxi+twQ== X-IronPort-AV: E=McAfee;i="6200,9189,10015"; a="267098192" X-IronPort-AV: E=Sophos;i="5.83,275,1616482800"; d="scan'208";a="267098192" Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 15 Jun 2021 01:52:18 -0700 IronPort-SDR: x9VTLJZ+sPET+x51pCfcebWe4EgZ0Uvof71XDy/7PnYjr1VNg7QYJBjqY4JnLLSyW6a0eGVxFx dLKCi7u/oHBw== X-IronPort-AV: E=Sophos;i="5.83,275,1616482800"; d="scan'208";a="421055457" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.213.200.187]) ([10.213.200.187]) by orsmga002-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 15 Jun 2021 01:52:17 -0700 To: Andrew Rybchenko , "Li, Xiaoyun" Cc: "dev@dpdk.org" , Bruce Richardson References: <20210527162452.1568351-1-andrew.rybchenko@oktetlabs.ru> <1f419fbb-b951-1a84-3329-97701c32c956@oktetlabs.ru> <87dd3fd0-9dab-8079-d135-50d966d6a5cd@intel.com> <2876ce81-7dfc-5e02-2f4a-9861dab1f877@oktetlabs.ru> <11e9bea3-0d44-f55e-51e2-a6afc4b4c1a3@intel.com> <44643b14-0d89-1f8b-e60f-1bc13dfa3cd2@oktetlabs.ru> From: Ferruh Yigit X-User: ferruhy Message-ID: <2453ce9c-82c0-cb1c-9402-3e460d40c710@intel.com> Date: Tue, 15 Jun 2021 09:52:13 +0100 MIME-Version: 1.0 In-Reply-To: <44643b14-0d89-1f8b-e60f-1bc13dfa3cd2@oktetlabs.ru> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH] app/testpmd: send failure logs to stderr X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 >>>>>> Sent: Friday, May 28, 2021 00:25 >>>>>> To: Li, Xiaoyun >>>>>> 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 >>>>>> --- >>>>>> >>>>>> 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 >>>>> >>> >