Re: [dpdk-dev] [PATCH v4 1/7] net/ark: PMD for Atomic Rules Arkville driver stub

[Ferruh Yigit <ferruh.yigit@intel.com>]

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