[1/6] net/e1000: use dynamic log type for tx/rx debug

Message ID 20190716154013.6974-2-stephen@networkplumber.org
State New
Delegated to: Ferruh Yigit
Headers show
Series
  • replace usage of LOGTYPE_PMD in Intel drivers
Related show

Checks

Context Check Description
ci/Intel-compilation success Compilation OK
ci/intel-Performance-Testing success Performance Testing PASS
ci/mellanox-Performance-Testing success Performance Testing PASS
ci/checkpatch success coding style OK

Commit Message

Stephen Hemminger July 16, 2019, 3:40 p.m.
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

Ferruh Yigit Aug. 27, 2019, 8:21 a.m. | #1
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.

Patch

diff --git a/drivers/net/e1000/e1000_logs.c b/drivers/net/e1000/e1000_logs.c
index 22173939f5e5..05f883dd201b 100644
--- a/drivers/net/e1000/e1000_logs.c
+++ b/drivers/net/e1000/e1000_logs.c
@@ -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;
 }
diff --git a/drivers/net/e1000/e1000_logs.h b/drivers/net/e1000/e1000_logs.h
index 69d3d31186e0..2612134f38a0 100644
--- a/drivers/net/e1000/e1000_logs.h
+++ b/drivers/net/e1000/e1000_logs.h
@@ -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;