From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id 99896CFA4 for ; Tue, 28 Mar 2017 14:58:47 +0200 (CEST) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orsmga104.jf.intel.com with ESMTP; 28 Mar 2017 05:58:46 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.36,236,1486454400"; d="scan'208";a="1147902907" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.237.220.122]) ([10.237.220.122]) by fmsmga002.fm.intel.com with ESMTP; 28 Mar 2017 05:58:45 -0700 To: Ed Czeck References: <1490231015-31748-1-git-send-email-ed.czeck@atomicrules.com> <0d1c21b6-6163-12f0-cbe4-d23c06a809e9@intel.com> Cc: dev@dpdk.org From: Ferruh Yigit Message-ID: <8625c310-943c-14b5-8215-b1f66bb94bb2@intel.com> Date: Tue, 28 Mar 2017 13:58:44 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH v4 1/7] net/ark: PMD for Atomic Rules Arkville driver stub X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 28 Mar 2017 12:58:48 -0000 On 3/23/2017 7:46 PM, Ed Czeck wrote: >> > +#define ARK_TRACE_ON(fmt, ...) \ >> > + PMD_DRV_LOG(ERR, fmt, ##__VA_ARGS__) >> > + >> > +#define ARK_TRACE_OFF(fmt, ...) \ >> > + do {if (0) PMD_DRV_LOG(ERR, fmt, ##__VA_ARGS__); } while (0) >> why not just "do { } while(0)" ? > > A do while body always executes at least once. The if (0) is required. Are you sure about this? I believe "do { } while(0)" also removed completely during compile. >> dev->data->dev_flags |= RTE_ETH_DEV_DETACHABLE; > > I do not understand your comment. This flag also should be added, all eth devices by are detachable. >> The general usage of DEBUG_TRACE is providing backtrace log, function >> enterance / exit informations. I guess, that is why it has been >> controlled by different config option. >> Here what you need looks like regular debugging functions, PMD_DRV_LOG / >> RTE_LOG variant. > > I'm unclear on your request or comment. Are you suggesting that we use > the dpdk versions of debug? The advantage of a local version is that > they can be enabled without all the debug traces. In many drivers there are independent log macros, two of them are: 1) PMD_.._TRACE 2) PMD_.._LOG Those above controlled by different config option. As name suggest, first one lets you to get function trace logs, many PMDs don't use them, since you can get this information from debugger. Second one is PMD wise log macro, controlled by its specific config option. It is possible enable / disable tracing logs (1), without effecting general logging macros (2). In ARK PMD, both ARK_DEBUG_TRACE and PMD_DRV_LOG macros are used for regular logging. a) I believe it is wrong to use ARK_DEBUG_TRACE for regular logging, all should be PMD_DRV_LOG b) And PMD_DRV_LOG should be controlled by a config option. As far as I can see ARK_DEBUG_TRACE never used for tracing really, so you can rename all occurrences to PMD_DRV_LOG, and remove ARK_DEBUG_TRACE and its config option. And introduce a config option for PMD_DRV_LOG