From mboxrd@z Thu Jan  1 00:00:00 1970
From: Shreyansh Jain <shreyansh.jain@nxp.com>
Subject: Re: [PATCH 3/7] net/dpaa2: change into dynamic logging
Date: Mon, 12 Mar 2018 17:31:46 +0530
Message-ID: <013ee451-9e4e-4d88-ae77-bc6a316575f0@nxp.com>
References: <20180312092547.18472-1-shreyansh.jain@nxp.com>
 <20180312092547.18472-4-shreyansh.jain@nxp.com>
 <f5fe140a-c3a8-c299-73c8-dabe1e9c6fb8@intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Cc: dev@dpdk.org, hemant.agrawal@nxp.com, akhil.goyal@nxp.com,
 nipun.gupta@nxp.com
To: Ferruh Yigit <ferruh.yigit@intel.com>
Return-path: <dev-bounces@dpdk.org>
Received: from EUR01-HE1-obe.outbound.protection.outlook.com
 (mail-he1eur01on0076.outbound.protection.outlook.com [104.47.0.76])
 by dpdk.org (Postfix) with ESMTP id 3E1295F20
 for <dev@dpdk.org>; Mon, 12 Mar 2018 13:02:07 +0100 (CET)
In-Reply-To: <f5fe140a-c3a8-c299-73c8-dabe1e9c6fb8@intel.com>
Content-Language: en-US
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

On 3/12/2018 4:34 PM, Ferruh Yigit wrote:
> On 3/12/2018 9:25 AM, Shreyansh Jain wrote:
>> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
> 
> <...>
> 
>> @@ -188,11 +188,6 @@ CONFIG_RTE_LIBRTE_DPAA2_USE_PHYS_IOVA=y
>>   # Compile burst-oriented NXP DPAA2 PMD driver
>>   #
>>   CONFIG_RTE_LIBRTE_DPAA2_PMD=n
>> -CONFIG_RTE_LIBRTE_DPAA2_DEBUG_INIT=n
>> -CONFIG_RTE_LIBRTE_DPAA2_DEBUG_DRIVER=n
>> -CONFIG_RTE_LIBRTE_DPAA2_DEBUG_RX=n
>> -CONFIG_RTE_LIBRTE_DPAA2_DEBUG_TX=n
>> -CONFIG_RTE_LIBRTE_DPAA2_DEBUG_TX_FREE=n
> 
> Just to double check, are you sure to remove data path logging config options
> too (RX, TX, TX_FREE)?

Yes, I have removed all conditional compilations.
Some more context below.

> 
> <...>
> 
>> @@ -557,6 +536,27 @@ for details.
>>         Done
>>         testpmd>
>>   
>> +Enabling logs
>> +-------------
>> +
>> +For enabling logging for DPAA2 PMD, following log-level prefix can be used:
>> +
>> + .. code-block:: console
>> +
>> +    <dpdk app> <EAL args> --log-level=bus.fslmc,<level> -- ...
>> +
>> +Using ``bus.fslmc`` as log matching criteria, all FSLMC bus logs can be enabled
>> +which are lower than logging ``level``.
>> +
>> + Or
>> +
>> + .. code-block:: console
>> +
>> +    <dpdk app> <EAL args> --log-level=pmd.dpaa2,<level> -- ...
> 
> Reminder, this will be also effected from naming change (pmd.net.dpaa2)

Ah, I think I completely forgot about that naming convention proposal.

> 
> <...>
> 
>> @@ -2045,3 +2046,12 @@ static struct rte_dpaa2_driver rte_dpaa2_pmd = {
>>   };
>>   
>>   RTE_PMD_REGISTER_DPAA2(net_dpaa2, rte_dpaa2_pmd);
>> +
>> +RTE_INIT(dpaa2_pmd_init_log);
>> +static void
>> +dpaa2_pmd_init_log(void)
>> +{
>> +	dpaa2_logtype_pmd = rte_log_register("pmd.dpaa2");
> 
> After commit [1] naming changed to "pmd.net.dpaa2"
> 
> [1]
> Commit: 7db274b9ada2 ("doc: describe dynamic logging format")

I will replace the net/eventdev/crypto registration strings

> 
> <...>
> 
>> +/* DP Logs, toggled out at compile time if level lower than current level */
>> +#define DPAA2_PMD_DP_LOG(level, fmt, args...) \
>> +	RTE_LOG_DP(level, PMD, fmt, ## args)
>> +
>> +#define DPAA2_PMD_DP_DEBUG(fmt, args...) \
>> +	DPAA2_PMD_DP_LOG(DEBUG, fmt, ## args)
>> +#define DPAA2_PMD_DP_INFO(fmt, args...) \
>> +	DPAA2_PMD_DP_LOG(INFO, fmt, ## args)
>> +#define DPAA2_PMD_DP_WARN(fmt, args...) \
>> +	DPAA2_PMD_DP_LOG(WARNING, fmt, ## args)
> 
> Just a reminder about using RTE_LOG_DP without config wrapper to disable them,
> not all code will be removed in compilation time, only ones with log_level >
> RTE_LOG_DP_LEVEL, so with default config DPAA2_PMD_DP_WARN() ones will not be
> removed.
> This can be OK or not based on your usage but this may effect your datapath.

I understand your point.
Only some selected locations have WARN in dpaa2 - and those cases are 
non-ideal datapath.
Nevertheless, thanks for highlighting. While creating a v2 for above 
registration string, I will re-consider this as well.

> 
> <...>
> 

Thanks for review.