* [dpdk-dev] [PATCH 1/3] net/e1000: optimize Rx/Tx log message level
2016-12-03 10:43 [dpdk-dev] [PATCH 0/3] net: optimize Rx/Tx log message level Qiming Yang
@ 2016-12-03 10:43 ` Qiming Yang
2016-12-06 22:41 ` Stephen Hemminger
2016-12-03 10:43 ` [dpdk-dev] [PATCH 2/3] net/i40e: " Qiming Yang
` (2 subsequent siblings)
3 siblings, 1 reply; 6+ messages in thread
From: Qiming Yang @ 2016-12-03 10:43 UTC (permalink / raw)
To: dev; +Cc: Qiming Yang
Signed-off-by: Qiming Yang <qiming.yang@intel.com>
---
config/common_base | 1 +
drivers/net/e1000/e1000_logs.h | 7 +++++++
drivers/net/e1000/em_rxtx.c | 10 +++++-----
drivers/net/e1000/igb_rxtx.c | 10 +++++-----
4 files changed, 18 insertions(+), 10 deletions(-)
diff --git a/config/common_base b/config/common_base
index 4bff83a..46e9797 100644
--- a/config/common_base
+++ b/config/common_base
@@ -143,6 +143,7 @@ CONFIG_RTE_LIBRTE_EM_PMD=y
CONFIG_RTE_LIBRTE_IGB_PMD=y
CONFIG_RTE_LIBRTE_E1000_DEBUG_INIT=n
CONFIG_RTE_LIBRTE_E1000_DEBUG_RX=n
+CONFIG_RTE_LIBRTE_E1000_DEBUG_RX_FREE=n
CONFIG_RTE_LIBRTE_E1000_DEBUG_TX=n
CONFIG_RTE_LIBRTE_E1000_DEBUG_TX_FREE=n
CONFIG_RTE_LIBRTE_E1000_DEBUG_DRIVER=n
diff --git a/drivers/net/e1000/e1000_logs.h b/drivers/net/e1000/e1000_logs.h
index 81e7bf5..407c245 100644
--- a/drivers/net/e1000/e1000_logs.h
+++ b/drivers/net/e1000/e1000_logs.h
@@ -50,6 +50,13 @@
#define PMD_RX_LOG(level, fmt, args...) do { } while(0)
#endif
+#ifdef RTE_LIBRTE_E1000_DEBUG_RX_FREE
+#define PMD_RX_FREE_LOG(level, fmt, args...) \
+ RTE_LOG(level, PMD, "%s(): " fmt "\n", __func__, ## args)
+#else
+#define PMD_RX_FREE_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)
diff --git a/drivers/net/e1000/em_rxtx.c b/drivers/net/e1000/em_rxtx.c
index 41f51c0..673af85 100644
--- a/drivers/net/e1000/em_rxtx.c
+++ b/drivers/net/e1000/em_rxtx.c
@@ -721,7 +721,7 @@ eth_em_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
nmb = rte_mbuf_raw_alloc(rxq->mb_pool);
if (nmb == NULL) {
- PMD_RX_LOG(DEBUG, "RX mbuf alloc failed port_id=%u "
+ PMD_RX_LOG(ERR, "RX mbuf alloc failed port_id=%u "
"queue_id=%u",
(unsigned) rxq->port_id,
(unsigned) rxq->queue_id);
@@ -806,7 +806,7 @@ eth_em_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
*/
nb_hold = (uint16_t) (nb_hold + rxq->nb_rx_hold);
if (nb_hold > rxq->rx_free_thresh) {
- PMD_RX_LOG(DEBUG, "port_id=%u queue_id=%u rx_tail=%u "
+ PMD_RX_FREE_LOG(DEBUG, "port_id=%u queue_id=%u rx_tail=%u "
"nb_hold=%u nb_rx=%u",
(unsigned) rxq->port_id, (unsigned) rxq->queue_id,
(unsigned) rx_id, (unsigned) nb_hold,
@@ -901,7 +901,7 @@ eth_em_recv_scattered_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
nmb = rte_mbuf_raw_alloc(rxq->mb_pool);
if (nmb == NULL) {
- PMD_RX_LOG(DEBUG, "RX mbuf alloc failed port_id=%u "
+ PMD_RX_LOG(ERR, "RX mbuf alloc failed port_id=%u "
"queue_id=%u", (unsigned) rxq->port_id,
(unsigned) rxq->queue_id);
rte_eth_devices[rxq->port_id].data->rx_mbuf_alloc_failed++;
@@ -1051,7 +1051,7 @@ eth_em_recv_scattered_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
*/
nb_hold = (uint16_t) (nb_hold + rxq->nb_rx_hold);
if (nb_hold > rxq->rx_free_thresh) {
- PMD_RX_LOG(DEBUG, "port_id=%u queue_id=%u rx_tail=%u "
+ PMD_RX_FREE_LOG(DEBUG, "port_id=%u queue_id=%u rx_tail=%u "
"nb_hold=%u nb_rx=%u",
(unsigned) rxq->port_id, (unsigned) rxq->queue_id,
(unsigned) rx_id, (unsigned) nb_hold,
@@ -1391,7 +1391,7 @@ eth_em_rx_queue_count(struct rte_eth_dev *dev, uint16_t rx_queue_id)
uint32_t desc = 0;
if (rx_queue_id >= dev->data->nb_rx_queues) {
- PMD_RX_LOG(DEBUG, "Invalid RX queue_id=%d", rx_queue_id);
+ PMD_RX_LOG(ERR, "Invalid RX queue_id=%d", rx_queue_id);
return 0;
}
diff --git a/drivers/net/e1000/igb_rxtx.c b/drivers/net/e1000/igb_rxtx.c
index dbd37ac..b8e67c7 100644
--- a/drivers/net/e1000/igb_rxtx.c
+++ b/drivers/net/e1000/igb_rxtx.c
@@ -832,7 +832,7 @@ eth_igb_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
nmb = rte_mbuf_raw_alloc(rxq->mb_pool);
if (nmb == NULL) {
- PMD_RX_LOG(DEBUG, "RX mbuf alloc failed port_id=%u "
+ PMD_RX_LOG(ERR, "RX mbuf alloc failed port_id=%u "
"queue_id=%u", (unsigned) rxq->port_id,
(unsigned) rxq->queue_id);
rte_eth_devices[rxq->port_id].data->rx_mbuf_alloc_failed++;
@@ -919,7 +919,7 @@ eth_igb_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
*/
nb_hold = (uint16_t) (nb_hold + rxq->nb_rx_hold);
if (nb_hold > rxq->rx_free_thresh) {
- PMD_RX_LOG(DEBUG, "port_id=%u queue_id=%u rx_tail=%u "
+ PMD_RX_FREE_LOG(DEBUG, "port_id=%u queue_id=%u rx_tail=%u "
"nb_hold=%u nb_rx=%u",
(unsigned) rxq->port_id, (unsigned) rxq->queue_id,
(unsigned) rx_id, (unsigned) nb_hold,
@@ -1015,7 +1015,7 @@ eth_igb_recv_scattered_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
nmb = rte_mbuf_raw_alloc(rxq->mb_pool);
if (nmb == NULL) {
- PMD_RX_LOG(DEBUG, "RX mbuf alloc failed port_id=%u "
+ PMD_RX_LOG(ERR, "RX mbuf alloc failed port_id=%u "
"queue_id=%u", (unsigned) rxq->port_id,
(unsigned) rxq->queue_id);
rte_eth_devices[rxq->port_id].data->rx_mbuf_alloc_failed++;
@@ -1174,7 +1174,7 @@ eth_igb_recv_scattered_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
*/
nb_hold = (uint16_t) (nb_hold + rxq->nb_rx_hold);
if (nb_hold > rxq->rx_free_thresh) {
- PMD_RX_LOG(DEBUG, "port_id=%u queue_id=%u rx_tail=%u "
+ PMD_RX_FREE_LOG(DEBUG, "port_id=%u queue_id=%u rx_tail=%u "
"nb_hold=%u nb_rx=%u",
(unsigned) rxq->port_id, (unsigned) rxq->queue_id,
(unsigned) rx_id, (unsigned) nb_hold,
@@ -1830,7 +1830,7 @@ igb_is_vmdq_supported(const struct rte_eth_dev *dev)
case e1000_i210:
case e1000_i211:
default:
- PMD_INIT_LOG(ERR, "Cannot support VMDq feature");
+ PMD_INIT_LOG(WARNING, "Cannot support VMDq feature");
return 0;
}
}
--
2.7.4
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [dpdk-dev] [PATCH 1/3] net/e1000: optimize Rx/Tx log message level
2016-12-03 10:43 ` [dpdk-dev] [PATCH 1/3] net/e1000: " Qiming Yang
@ 2016-12-06 22:41 ` Stephen Hemminger
0 siblings, 0 replies; 6+ messages in thread
From: Stephen Hemminger @ 2016-12-06 22:41 UTC (permalink / raw)
To: Qiming Yang; +Cc: dev
On Sat, 3 Dec 2016 18:43:01 +0800
Qiming Yang <qiming.yang@intel.com> wrote:
>
> +#ifdef RTE_LIBRTE_E1000_DEBUG_RX_FREE
> +#define PMD_RX_FREE_LOG(level, fmt, args...) \
> + RTE_LOG(level, PMD, "%s(): " fmt "\n", __func__, ## args)
> +#else
> +#define PMD_RX_FREE_LOG(level, fmt, args...) do { } while (0)
> +#endif
If you have to copy/paste same code in more than one driver, that
looks like a missing interface. I.e Do not Repeat Yourself.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [dpdk-dev] [PATCH 2/3] net/i40e: optimize Rx/Tx log message level
2016-12-03 10:43 [dpdk-dev] [PATCH 0/3] net: optimize Rx/Tx log message level Qiming Yang
2016-12-03 10:43 ` [dpdk-dev] [PATCH 1/3] net/e1000: " Qiming Yang
@ 2016-12-03 10:43 ` Qiming Yang
2016-12-03 10:43 ` [dpdk-dev] [PATCH 3/3] net/ixgbe: " Qiming Yang
2016-12-06 10:51 ` [dpdk-dev] [PATCH 0/3] net: " Ferruh Yigit
3 siblings, 0 replies; 6+ messages in thread
From: Qiming Yang @ 2016-12-03 10:43 UTC (permalink / raw)
To: dev; +Cc: Qiming Yang
Signed-off-by: Qiming Yang <qiming.yang@intel.com>
---
drivers/net/i40e/i40e_logs.h | 7 +++++++
drivers/net/i40e/i40e_rxtx.c | 2 +-
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/net/i40e/i40e_logs.h b/drivers/net/i40e/i40e_logs.h
index e042e24..5c25dbf 100644
--- a/drivers/net/i40e/i40e_logs.h
+++ b/drivers/net/i40e/i40e_logs.h
@@ -50,6 +50,13 @@
#define PMD_RX_LOG(level, fmt, args...) do { } while(0)
#endif
+#ifdef RTE_LIBRTE_I40E_DEBUG_RX_FREE
+#define PMD_RX_FREE_LOG(level, fmt, args...) \
+ RTE_LOG(level, PMD, "%s(): " fmt "\n", __func__, ## args)
+#else
+#define PMD_RX_FREE_LOG(level, fmt, args...) do { } while (0)
+#endif
+
#ifdef RTE_LIBRTE_I40E_DEBUG_TX
#define PMD_TX_LOG(level, fmt, args...) \
RTE_LOG(level, PMD, "%s(): " fmt "\n", __func__, ## args)
diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
index 7ae7d9f..ef25fb1 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -612,7 +612,7 @@ rx_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
if (i40e_rx_alloc_bufs(rxq) != 0) {
uint16_t i, j;
- PMD_RX_LOG(DEBUG, "Rx mbuf alloc failed for "
+ PMD_RX_LOG(ERR, "Rx mbuf alloc failed for "
"port_id=%u, queue_id=%u",
rxq->port_id, rxq->queue_id);
rxq->rx_nb_avail = 0;
--
2.7.4
^ permalink raw reply [flat|nested] 6+ messages in thread* [dpdk-dev] [PATCH 3/3] net/ixgbe: optimize Rx/Tx log message level
2016-12-03 10:43 [dpdk-dev] [PATCH 0/3] net: optimize Rx/Tx log message level Qiming Yang
2016-12-03 10:43 ` [dpdk-dev] [PATCH 1/3] net/e1000: " Qiming Yang
2016-12-03 10:43 ` [dpdk-dev] [PATCH 2/3] net/i40e: " Qiming Yang
@ 2016-12-03 10:43 ` Qiming Yang
2016-12-06 10:51 ` [dpdk-dev] [PATCH 0/3] net: " Ferruh Yigit
3 siblings, 0 replies; 6+ messages in thread
From: Qiming Yang @ 2016-12-03 10:43 UTC (permalink / raw)
To: dev; +Cc: Qiming Yang
Signed-off-by: Qiming Yang <qiming.yang@intel.com>
---
drivers/net/ixgbe/ixgbe_logs.h | 7 +++++++
drivers/net/ixgbe/ixgbe_rxtx.c | 14 +++++++-------
2 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ixgbe/ixgbe_logs.h b/drivers/net/ixgbe/ixgbe_logs.h
index 53ba42d..68e47fd 100644
--- a/drivers/net/ixgbe/ixgbe_logs.h
+++ b/drivers/net/ixgbe/ixgbe_logs.h
@@ -50,6 +50,13 @@
#define PMD_RX_LOG(level, fmt, args...) do { } while(0)
#endif
+#ifdef RTE_LIBRTE_IXGBE_DEBUG_RX_FREE
+#define PMD_RX_FREE_LOG(level, fmt, args...) \
+ RTE_LOG(level, PMD, "%s(): " fmt "\n", __func__, ## args)
+#else
+#define PMD_RX_FREE_LOG(level, fmt, args...) do { } while (0)
+#endif
+
#ifdef RTE_LIBRTE_IXGBE_DEBUG_TX
#define PMD_TX_LOG(level, fmt, args...) \
RTE_LOG(level, PMD, "%s(): " fmt "\n", __func__, ## args)
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
index b2d9f45..b52c793 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx.c
@@ -1560,7 +1560,7 @@ rx_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
if (ixgbe_rx_alloc_bufs(rxq, true) != 0) {
int i, j;
- PMD_RX_LOG(DEBUG, "RX mbuf alloc failed port_id=%u "
+ PMD_RX_LOG(ERR, "RX mbuf alloc failed port_id=%u "
"queue_id=%u", (unsigned) rxq->port_id,
(unsigned) rxq->queue_id);
@@ -1701,7 +1701,7 @@ ixgbe_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
nmb = rte_mbuf_raw_alloc(rxq->mb_pool);
if (nmb == NULL) {
- PMD_RX_LOG(DEBUG, "RX mbuf alloc failed port_id=%u "
+ PMD_RX_LOG(ERR, "RX mbuf alloc failed port_id=%u "
"queue_id=%u", (unsigned) rxq->port_id,
(unsigned) rxq->queue_id);
rte_eth_devices[rxq->port_id].data->rx_mbuf_alloc_failed++;
@@ -1799,7 +1799,7 @@ ixgbe_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
*/
nb_hold = (uint16_t) (nb_hold + rxq->nb_rx_hold);
if (nb_hold > rxq->rx_free_thresh) {
- PMD_RX_LOG(DEBUG, "port_id=%u queue_id=%u rx_tail=%u "
+ PMD_RX_FREE_LOG(DEBUG, "port_id=%u queue_id=%u rx_tail=%u "
"nb_hold=%u nb_rx=%u",
(unsigned) rxq->port_id, (unsigned) rxq->queue_id,
(unsigned) rx_id, (unsigned) nb_hold,
@@ -1972,7 +1972,7 @@ ixgbe_recv_pkts_lro(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts,
if (!bulk_alloc) {
nmb = rte_mbuf_raw_alloc(rxq->mb_pool);
if (nmb == NULL) {
- PMD_RX_LOG(DEBUG, "RX mbuf alloc failed "
+ PMD_RX_LOG(ERR, "RX mbuf alloc failed "
"port_id=%u queue_id=%u",
rxq->port_id, rxq->queue_id);
@@ -1989,7 +1989,7 @@ ixgbe_recv_pkts_lro(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts,
next_rdt);
nb_hold -= rxq->rx_free_thresh;
} else {
- PMD_RX_LOG(DEBUG, "RX bulk alloc failed "
+ PMD_RX_FREE_LOG(DEBUG, "RX bulk alloc failed "
"port_id=%u queue_id=%u",
rxq->port_id, rxq->queue_id);
@@ -2152,7 +2152,7 @@ ixgbe_recv_pkts_lro(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts,
* hardware point of view...
*/
if (!bulk_alloc && nb_hold > rxq->rx_free_thresh) {
- PMD_RX_LOG(DEBUG, "port_id=%u queue_id=%u rx_tail=%u "
+ PMD_RX_FREE_LOG(DEBUG, "port_id=%u queue_id=%u rx_tail=%u "
"nb_hold=%u nb_rx=%u",
rxq->port_id, rxq->queue_id, rx_id, nb_hold, nb_rx);
@@ -4763,7 +4763,7 @@ ixgbe_setup_loopback_link_82599(struct ixgbe_hw *hw)
if (ixgbe_verify_lesm_fw_enabled_82599(hw)) {
if (hw->mac.ops.acquire_swfw_sync(hw, IXGBE_GSSR_MAC_CSR_SM) !=
IXGBE_SUCCESS) {
- PMD_INIT_LOG(ERR, "Could not enable loopback mode");
+ PMD_INIT_LOG(WARNING, "Could not enable loopback mode");
/* ignore error */
return;
}
--
2.7.4
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [dpdk-dev] [PATCH 0/3] net: optimize Rx/Tx log message level
2016-12-03 10:43 [dpdk-dev] [PATCH 0/3] net: optimize Rx/Tx log message level Qiming Yang
` (2 preceding siblings ...)
2016-12-03 10:43 ` [dpdk-dev] [PATCH 3/3] net/ixgbe: " Qiming Yang
@ 2016-12-06 10:51 ` Ferruh Yigit
3 siblings, 0 replies; 6+ messages in thread
From: Ferruh Yigit @ 2016-12-06 10:51 UTC (permalink / raw)
To: Qiming Yang, dev; +Cc: Wenzhuo Lu, Helin Zhang, Konstantin Ananyev, Jingjing Wu
Hi Qiming,
On 12/3/2016 10:43 AM, Qiming Yang wrote:
> These three patches optimized the level of Rx and Tx log
> messages. Add a new log control function PMD_RX_FREE_LOG
> to control the Rx message which is not printed in packet
> receive processing. This function switched by macro
> RTE_LIBRTE_<PMD>_DEBUG_RX_FREE.
Please CC the maintainers,
CC: Wenzhuo Lu <wenzhuo.lu@intel.com>
CC: Konstantin Ananyev <konstantin.ananyev@intel.com>, Helin Zhang
<helin.zhang@intel.com>
CC: Jingjing Wu <jingjing.wu@intel.com>
1- Do we really need a new config option just for this log function, we
already may have more config options?
2- Do we really need this new logging function at all? This is to report
when receive descriptor tail updated, as far as I can see. What is the
use case that makes this case so special?
3- Log level of some messages are increased, like mbuf allocation
failure message level increased to ERR, what is the expectation here? In
high throughput traffic this may cause lots of noise, and what user can
do with this message? If user can't do anything, why we bloating user
with error messages? There is already a dynamic counter, I believe which
is good thing to the for this error, and debug level logging.
Overall I am for rejecting this patchset, unless there is a good
justification for new debug log macro.
Thanks,
ferruh
^ permalink raw reply [flat|nested] 6+ messages in thread