On Tue, Mar 28, 2017 at 8:58 AM, Ferruh Yigit <ferruh.yi...@intel.com>
wrote:
> 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.
>
I verified that the if (0) is required.
>
>
> >> 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.
>
Flag has been added.
>
>
> >> 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
>
> Thank you. I refactored the messaging and logging code to a set of
macros named PMD_<XX>_LOG, where XX is one of {DEBUG, STATS, RX, TX,DRV}.
This follows the code pattern from the Intel PMDs. Much of the logging has
been changed throughout the other patches as well.
