From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ferruh Yigit Subject: Re: [PATCH 1/7] net/qede: reduce noise in debug logs Date: Thu, 8 Dec 2016 16:44:15 +0000 Message-ID: <91ac9846-530c-2dd3-4bd6-2c3b9143405b@intel.com> References: <1480733039-13046-1-git-send-email-harish.patil@qlogic.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Cc: Dept-EngDPDKDev@qlogic.com, Rasesh Mody To: Harish Patil , dev@dpdk.org Return-path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id BCD042B93 for ; Thu, 8 Dec 2016 17:44:17 +0100 (CET) In-Reply-To: <1480733039-13046-1-git-send-email-harish.patil@qlogic.com> 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 12/3/2016 2:43 AM, Harish Patil wrote: > From: Rasesh Mody > > Replace CONFIG_RTE_LIBRTE_QEDE_DEBUG_DRIVER with > CONFIG_RTE_LIBRTE_QEDE_DEBUG_VAL which is a 32-bit bitmapped value > where each bit represent a particular submodule to debug. Also move > notice messages under CONFIG_RTE_LIBRTE_QEDE_DEBUG_INFO. > > Signed-off-by: Harish Patil > Signed-off-by: Rasesh Mody > --- > config/common_base | 2 +- > doc/guides/nics/qede.rst | 4 ++-- > drivers/net/qede/qede_ethdev.c | 4 ++-- > drivers/net/qede/qede_logs.h | 21 +++++---------------- > 4 files changed, 10 insertions(+), 21 deletions(-) > > diff --git a/config/common_base b/config/common_base > index 4bff83a..2ffd557 100644 > --- a/config/common_base > +++ b/config/common_base > @@ -320,7 +320,7 @@ CONFIG_RTE_LIBRTE_BOND_DEBUG_ALB_L1=n > CONFIG_RTE_LIBRTE_QEDE_PMD=y > CONFIG_RTE_LIBRTE_QEDE_DEBUG_INIT=n > CONFIG_RTE_LIBRTE_QEDE_DEBUG_INFO=n > -CONFIG_RTE_LIBRTE_QEDE_DEBUG_DRIVER=n > +CONFIG_RTE_LIBRTE_QEDE_DEBUG_VAL=0 > CONFIG_RTE_LIBRTE_QEDE_DEBUG_TX=n > CONFIG_RTE_LIBRTE_QEDE_DEBUG_RX=n > #Provides abs path/name of the firmware file. > diff --git a/doc/guides/nics/qede.rst b/doc/guides/nics/qede.rst > index d22ecdd..ddf4248 100644 > --- a/doc/guides/nics/qede.rst > +++ b/doc/guides/nics/qede.rst > @@ -103,9 +103,9 @@ enabling debugging options may affect system performance. > > Toggle display of generic debugging messages. > > -- ``CONFIG_RTE_LIBRTE_QEDE_DEBUG_DRIVER`` (default **n**) > +- ``CONFIG_RTE_LIBRTE_QEDE_DEBUG_VAL`` (default **0**) Does it make sense to document how DEBUG_VAL used? Also commit log says bitmapped value to enable/disable a particular submodule, you may want to document here which value enable/disable which submodule. > > - Toggle display of ecore related messages. > + Control driver debug verbosity using 32-bit bitmap flags. > > - ``CONFIG_RTE_LIBRTE_QEDE_DEBUG_TX`` (default **n**) > <...> > diff --git a/drivers/net/qede/qede_logs.h b/drivers/net/qede/qede_logs.h > index 45c4af0..08fdf04 100644 > --- a/drivers/net/qede/qede_logs.h > +++ b/drivers/net/qede/qede_logs.h > @@ -16,15 +16,18 @@ > (p_dev)->name ? (p_dev)->name : "", \ > ##__VA_ARGS__) > > +#ifdef RTE_LIBRTE_QEDE_DEBUG_INFO Is "_INFO" carries any meaning in this config option, why not just RTE_LIBRTE_QEDE_DEBUG? > #define DP_NOTICE(p_dev, is_assert, fmt, ...) \ > rte_log(RTE_LOG_NOTICE, RTE_LOGTYPE_PMD,\ > "[QEDE PMD: (%s)]%s:" fmt, \ > (p_dev)->name ? (p_dev)->name : "", \ > __func__, \ > ##__VA_ARGS__) > +#else > +#define DP_NOTICE(p_dev, fmt, ...) do { } while (0) > +#endif > > #ifdef RTE_LIBRTE_QEDE_DEBUG_INFO > - > #define DP_INFO(p_dev, fmt, ...) \ > rte_log(RTE_LOG_INFO, RTE_LOGTYPE_PMD, \ > "[%s:%d(%s)]" fmt, \ > @@ -33,10 +36,8 @@ > ##__VA_ARGS__) > #else > #define DP_INFO(p_dev, fmt, ...) do { } while (0) > - > #endif > > -#ifdef RTE_LIBRTE_QEDE_DEBUG_DRIVER Are you sure you want to enable DP_VERBOSE, I guess most verbose log macro, by default? Perhaps may want to control it via RTE_LIBRTE_QEDE_DEBUG_INFO? > #define DP_VERBOSE(p_dev, module, fmt, ...) \ > do { \ > if ((p_dev)->dp_module & module) \ > @@ -46,9 +47,7 @@ > (p_dev)->name ? (p_dev)->name : "", \ > ##__VA_ARGS__); \ > } while (0) > -#else > -#define DP_VERBOSE(p_dev, fmt, ...) do { } while (0) > -#endif > + > <...>