[1/6] net/e1000: use dynamic log type for tx/rx debug
Checks
Commit Message
The generic RTE_LOGTYPE_PMD is a historical relic and should
not be used. Every driver should register the logtypes
for itself.
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
drivers/net/e1000/e1000_logs.c | 48 +++++++++++++++++++++++++++-------
drivers/net/e1000/e1000_logs.h | 25 +++++++++++-------
2 files changed, 55 insertions(+), 18 deletions(-)
Comments
On 7/16/2019 4:40 PM, Stephen Hemminger wrote:
> The generic RTE_LOGTYPE_PMD is a historical relic and should
> not be used. Every driver should register the logtypes
> for itself.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
<...>
> +#ifdef RTE_LIBRTE_E1000_DEBUG_RX
> + e1000_logtype_rx = rte_log_register("pmd.net.e1000.rx");
> + if (e1000_logtype_rx >= 0)
> + rte_log_set_level(e1000_logtype_rx, RTE_LOG_NOTICE);
What do you think setting default level for data path log level to
'RTE_LOG_DEBUG' since they are already controlled by a config option, and to
keep the behavior consistent with previous usage, because almost all macros are
called with DEBUG level. Same for all drivers in this patchset.
On 08/27, Ferruh Yigit wrote:
>On 7/16/2019 4:40 PM, Stephen Hemminger wrote:
>> The generic RTE_LOGTYPE_PMD is a historical relic and should
>> not be used. Every driver should register the logtypes
>> for itself.
>>
>> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
>
><...>
>
>> +#ifdef RTE_LIBRTE_E1000_DEBUG_RX
>> + e1000_logtype_rx = rte_log_register("pmd.net.e1000.rx");
>> + if (e1000_logtype_rx >= 0)
>> + rte_log_set_level(e1000_logtype_rx, RTE_LOG_NOTICE);
>
>What do you think setting default level for data path log level to
>'RTE_LOG_DEBUG' since they are already controlled by a config option, and to
>keep the behavior consistent with previous usage, because almost all macros are
>called with DEBUG level. Same for all drivers in this patchset.
+1
Thanks,
Xiaolong
On Mon, 30 Sep 2019 23:28:38 +0800
Ye Xiaolong <xiaolong.ye@intel.com> wrote:
> On 08/27, Ferruh Yigit wrote:
> >On 7/16/2019 4:40 PM, Stephen Hemminger wrote:
> >> The generic RTE_LOGTYPE_PMD is a historical relic and should
> >> not be used. Every driver should register the logtypes
> >> for itself.
> >>
> >> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> >
> ><...>
> >
> >> +#ifdef RTE_LIBRTE_E1000_DEBUG_RX
> >> + e1000_logtype_rx = rte_log_register("pmd.net.e1000.rx");
> >> + if (e1000_logtype_rx >= 0)
> >> + rte_log_set_level(e1000_logtype_rx, RTE_LOG_NOTICE);
> >
> >What do you think setting default level for data path log level to
> >'RTE_LOG_DEBUG' since they are already controlled by a config option, and to
> >keep the behavior consistent with previous usage, because almost all macros are
> >called with DEBUG level. Same for all drivers in this patchset.
>
> +1
>
> Thanks,
> Xiaolong
Or drop DEBUG_RX and DEBUG_TX completely since they are not useful anymore.
The code works, and distributions can not enable this.
On 9/30/2019 4:50 PM, Stephen Hemminger wrote:
> On Mon, 30 Sep 2019 23:28:38 +0800
> Ye Xiaolong <xiaolong.ye@intel.com> wrote:
>
>> On 08/27, Ferruh Yigit wrote:
>>> On 7/16/2019 4:40 PM, Stephen Hemminger wrote:
>>>> The generic RTE_LOGTYPE_PMD is a historical relic and should
>>>> not be used. Every driver should register the logtypes
>>>> for itself.
>>>>
>>>> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
>>>
>>> <...>
>>>
>>>> +#ifdef RTE_LIBRTE_E1000_DEBUG_RX
>>>> + e1000_logtype_rx = rte_log_register("pmd.net.e1000.rx");
>>>> + if (e1000_logtype_rx >= 0)
>>>> + rte_log_set_level(e1000_logtype_rx, RTE_LOG_NOTICE);
>>>
>>> What do you think setting default level for data path log level to
>>> 'RTE_LOG_DEBUG' since they are already controlled by a config option, and to
>>> keep the behavior consistent with previous usage, because almost all macros are
>>> called with DEBUG level. Same for all drivers in this patchset.
>>
>> +1
>>
>> Thanks,
>> Xiaolong
>
> Or drop DEBUG_RX and DEBUG_TX completely since they are not useful anymore.
> The code works, and distributions can not enable this.
>
Distributions can't enable this, but still can be useful for the debug. I am
planning to update the default log level to DEBUG and merge it, any objection?
On 8/27/2019 9:21 AM, Ferruh Yigit wrote:
> On 7/16/2019 4:40 PM, Stephen Hemminger wrote:
>> The generic RTE_LOGTYPE_PMD is a historical relic and should
>> not be used. Every driver should register the logtypes
>> for itself.
>>
>> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
>
> <...>
>
>> +#ifdef RTE_LIBRTE_E1000_DEBUG_RX
>> + e1000_logtype_rx = rte_log_register("pmd.net.e1000.rx");
>> + if (e1000_logtype_rx >= 0)
>> + rte_log_set_level(e1000_logtype_rx, RTE_LOG_NOTICE);
>
> What do you think setting default level for data path log level to
> 'RTE_LOG_DEBUG' since they are already controlled by a config option, and to
> keep the behavior consistent with previous usage, because almost all macros are
> called with DEBUG level. Same for all drivers in this patchset.
>
For series,
Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
Series applied to dpdk-next-net/master, thanks.
(Default log level for datapath change to RTE_LOG_DEBUG while merging.)
@@ -8,19 +8,49 @@
int e1000_logtype_init;
int e1000_logtype_driver;
+#ifdef RTE_LIBRTE_E1000_DEBUG_RX
+int e1000_logtype_rx;
+#endif
+#ifdef RTE_LIBRTE_E1000_DEBUG_TX
+int e1000_logtype_tx;
+#endif
+#ifdef RTE_LIBRTE_E1000_DEBUG_TX_FREE
+int e1000_logtype_tx_free;
+#endif
+
/* avoids double registering of logs if EM and IGB drivers are in use */
static int e1000_log_initialized;
void
e1000_igb_init_log(void)
{
- if (!e1000_log_initialized) {
- e1000_logtype_init = rte_log_register("pmd.net.e1000.init");
- if (e1000_logtype_init >= 0)
- rte_log_set_level(e1000_logtype_init, RTE_LOG_NOTICE);
- e1000_logtype_driver = rte_log_register("pmd.net.e1000.driver");
- if (e1000_logtype_driver >= 0)
- rte_log_set_level(e1000_logtype_driver, RTE_LOG_NOTICE);
- e1000_log_initialized = 1;
- }
+ if (e1000_log_initialized)
+ return;
+
+ e1000_logtype_init = rte_log_register("pmd.net.e1000.init");
+ if (e1000_logtype_init >= 0)
+ rte_log_set_level(e1000_logtype_init, RTE_LOG_NOTICE);
+ e1000_logtype_driver = rte_log_register("pmd.net.e1000.driver");
+ if (e1000_logtype_driver >= 0)
+ rte_log_set_level(e1000_logtype_driver, RTE_LOG_NOTICE);
+
+#ifdef RTE_LIBRTE_E1000_DEBUG_RX
+ e1000_logtype_rx = rte_log_register("pmd.net.e1000.rx");
+ if (e1000_logtype_rx >= 0)
+ rte_log_set_level(e1000_logtype_rx, RTE_LOG_NOTICE);
+#endif
+
+#ifdef RTE_LIBRTE_E1000_DEBUG_TX
+ e1000_logtype_tx = rte_log_register("pmd.net.e1000.tx");
+ if (e1000_logtype_tx >= 0)
+ rte_log_set_level(e1000_logtype_tx, RTE_LOG_NOTICE);
+#endif
+
+#ifdef RTE_LIBRTE_E1000_DEBUG_TX_FREE
+ e1000_logtype_tx_free = rte_log_register("pmd.net.e1000.tx_free");
+ if (e1000_logtype_tx_free >= 0)
+ rte_log_set_level(e1000_logtype_tx_free, RTE_LOG_NOTICE);
+#endif
+
+ e1000_log_initialized = 1;
}
@@ -8,6 +8,7 @@
#include <rte_log.h>
extern int e1000_logtype_init;
+
#define PMD_INIT_LOG(level, fmt, args...) \
rte_log(RTE_LOG_ ## level, e1000_logtype_init, \
"%s(): " fmt "\n", __func__, ##args)
@@ -15,24 +16,30 @@ extern int e1000_logtype_init;
#define PMD_INIT_FUNC_TRACE() PMD_INIT_LOG(DEBUG, " >>")
#ifdef RTE_LIBRTE_E1000_DEBUG_RX
-#define PMD_RX_LOG(level, fmt, args...) \
- RTE_LOG(level, PMD, "%s(): " fmt "\n", __func__, ## args)
+extern int e1000_logtype_rx;
+#define PMD_RX_LOG(level, fmt, args...) \
+ rte_log(RTE_LOG_ ## level, e1000_logtype_rx, \
+ "%s(): " fmt "\n", __func__, ## args)
#else
-#define PMD_RX_LOG(level, fmt, args...) do { } while(0)
+#define PMD_RX_LOG(level, fmt, args...) do { } while (0)
#endif
#ifdef RTE_LIBRTE_E1000_DEBUG_TX
-#define PMD_TX_LOG(level, fmt, args...) \
- RTE_LOG(level, PMD, "%s(): " fmt "\n", __func__, ## args)
+extern int e1000_logtype_tx;
+#define PMD_TX_LOG(level, fmt, args...) \
+ rte_log(RTE_LOG_ ## level, e1000_logtype_tx, \
+ "%s(): " fmt "\n", __func__, ## args)
#else
-#define PMD_TX_LOG(level, fmt, args...) do { } while(0)
+#define PMD_TX_LOG(level, fmt, args...) do { } while (0)
#endif
#ifdef RTE_LIBRTE_E1000_DEBUG_TX_FREE
-#define PMD_TX_FREE_LOG(level, fmt, args...) \
- RTE_LOG(level, PMD, "%s(): " fmt "\n", __func__, ## args)
+extern int e1000_logtype_tx_free;
+#define PMD_TX_FREE_LOG(level, fmt, args...) \
+ rte_log(RTE_LOG_ ## level, e1000_logtype_tx_free, \
+ "%s(): " fmt "\n", __func__, ## args)
#else
-#define PMD_TX_FREE_LOG(level, fmt, args...) do { } while(0)
+#define PMD_TX_FREE_LOG(level, fmt, args...) do { } while (0)
#endif
extern int e1000_logtype_driver;