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 E2FC4A00C2; Thu, 10 Feb 2022 09:04:07 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 9A7A041170; Thu, 10 Feb 2022 09:04:07 +0100 (CET) Received: from smtpbgeu2.qq.com (smtpbgeu2.qq.com [18.194.254.142]) by mails.dpdk.org (Postfix) with ESMTP id B6000410E5 for ; Thu, 10 Feb 2022 09:04:06 +0100 (CET) X-QQ-mid: bizesmtp5t1644480236tuv0e2nj6 Received: from jiawenwu (unknown [183.129.236.74]) by bizesmtp.qq.com (ESMTP) with id ; Thu, 10 Feb 2022 16:03:54 +0800 (CST) X-QQ-SSF: 01400000002000F0L000000A0000000 X-QQ-FEAT: ZHWZeLXy+8dufM/iqvUqd+tnMNvu31GDxkK2xKwHLCQLvDLw+UkfDkeWGqaNR l0ETRxlvQ5rRCoHyIMHJTbZb3gNB026H4lDa2Zly19qVGzKvET8igwarRlCxBsTiW6msRIm 1P9AB/fWJ5gZW3PKNxbMxWJ1AcWSxs/w6HPRUoK2wQiFfY48Vp7HuvMMQYelu+xb+5jSW7L DYkToioYZ7Zskj+g4AM2hPtguXWgZttGPRiL2BmvqyPX1CSX7Tlpp8JGKYPYYHm9UukEUmg yEIpydENsJrd6cVvvxfKNARbEkbkgCEpsScip8kcJkLzwKxEge6F0ieAuyfczC59Nuo1Fmh ATYagrNyD07OIvW8cLR3QOqwQbsO7irMJOkAhMg X-QQ-GoodBg: 2 From: "Jiawen Wu" To: "'Ferruh Yigit'" , Cc: References: <20220209104213.602728-1-jiawenwu@trustnetic.com> <20220209104213.602728-9-jiawenwu@trustnetic.com> In-Reply-To: Subject: RE: [PATCH v2 08/12] net/ngbe: fix debug log Date: Thu, 10 Feb 2022 16:03:54 +0800 Message-ID: <004201d81e54$c0401160$40c03420$@trustnetic.com>+F6269B1AD92BF436 MIME-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Mailer: Microsoft Outlook 16.0 Content-Language: zh-cn Thread-Index: AQGgZ+TcfTHBt4TpTBeLIOldsX01eQI3FN5CAsnA/CGs03I1IA== X-QQ-SENDSIZE: 520 Feedback-ID: bizesmtp:trustnetic.com:qybgforeign:qybgforeign7 X-QQ-Bgrelay: 1 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 On February 10, 2022 3:07 AM, Ferruh Yigit wrote: > On 2/9/2022 10:42 AM, Jiawen Wu wrote: > > Remove 'DEBUGFUNC' due to too many invalid debug log prints. And fix > > that double line was added by using 'DEBUGOUT'. > > > > Fixes: cc934df178ab ("net/ngbe: add log and error types") > > Cc: stable@dpdk.org > > > > Signed-off-by: Jiawen Wu > > --- > > drivers/net/ngbe/ngbe_logs.h | 11 +++++++---- > > 1 file changed, 7 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/net/ngbe/ngbe_logs.h > > b/drivers/net/ngbe/ngbe_logs.h index fd306419e6..b7d78fb400 100644 > > --- a/drivers/net/ngbe/ngbe_logs.h > > +++ b/drivers/net/ngbe/ngbe_logs.h > > @@ -15,9 +15,12 @@ extern int ngbe_logtype_init; > > "%s(): " fmt "\n", __func__, ##args) > > > > extern int ngbe_logtype_driver; > > -#define PMD_DRV_LOG(level, fmt, args...) \ > > +#define PMD_TLOG_DRIVER(level, fmt, args...) \ > > rte_log(RTE_LOG_ ## level, ngbe_logtype_driver, \ > > - "%s(): " fmt "\n", __func__, ##args) > > + "%s(): " fmt, __func__, ##args) > > + > > +#define PMD_DRV_LOG(level, fmt, args...) \ > > + PMD_TLOG_DRIVER(level, fmt "\n", ## args) > > >=20 > Both 'DEBUGOUT' and 'PMD_DRV_LOG(DEBUG, ..' are in use for same thing, > but one appends '\n' other doesn't, this is very error prune and there = are > already wrong usage in the driver. > I think no need to add complexity for something as simple as this, = what do you > think to unify the DEBUG level macros, at least unify the line ending = behavior? >=20 >=20 > > #ifdef RTE_ETHDEV_DEBUG_RX > > extern int ngbe_logtype_rx; > > @@ -37,10 +40,10 @@ extern int ngbe_logtype_tx; > > #define PMD_TX_LOG(level, fmt, args...) do { } while (0) > > #endif > > > > -#define TLOG_DEBUG(fmt, args...) PMD_DRV_LOG(DEBUG, fmt, ##args) > > +#define TLOG_DEBUG(fmt, args...) PMD_TLOG_DRIVER(DEBUG, fmt, > ##args) > > > > #define DEBUGOUT(fmt, args...) TLOG_DEBUG(fmt, ##args) > > #define PMD_INIT_FUNC_TRACE() TLOG_DEBUG(" >>") >=20 > What is difference between 'PMD_INIT_FUNC_TRACE' and 'DEBUGFUNC'? > As far as I can see they are for same reason, and both macros are = used. If they > are for same reason can you please unify the usage? >=20 > > -#define DEBUGFUNC(fmt) TLOG_DEBUG(fmt) > > +#define DEBUGFUNC(fmt) do { } while (0) >=20 > If 'DEBUGFUNC' can be removed, why not removing from the code, instead = of > above define? This is your driver, you have full control on it. Okay. I just realize that this is going to be a big change, so I want to = keep it to a minimum. Anyway, 'DEBUGFUNC' should be removed, because 'DEBUGOUT' already = contains the function name.