DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH v2 0/5] ixgbe: logging patches
@ 2015-05-15 17:08 Stephen Hemminger
  2015-05-15 17:08 ` [dpdk-dev] [PATCH 1/5] ixgbe: remove unnecessary casts Stephen Hemminger
                   ` (4 more replies)
  0 siblings, 5 replies; 14+ messages in thread
From: Stephen Hemminger @ 2015-05-15 17:08 UTC (permalink / raw)
  To: helin.zhang; +Cc: dev

This set of patches is breakout of earlier set of patches to
get rid of the log spam from ixgbe driver.

Stephen Hemminger (5):
  ixgbe: remove unnecessary casts
  ixgbe: don't print PCI address on link change
  ixgbe: raise priority of significant events
  ixgbe: use RTE_LOG not rte_log
  ixgbe: silence noisy log messages

 lib/librte_pmd_ixgbe/ixgbe_ethdev.c | 54 +++++++++++++++++--------------------
 lib/librte_pmd_ixgbe/ixgbe_logs.h   |  3 +--
 lib/librte_pmd_ixgbe/ixgbe_rxtx.c   | 20 +++++++-------
 3 files changed, 35 insertions(+), 42 deletions(-)

-- 
2.1.4

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [dpdk-dev] [PATCH 1/5] ixgbe: remove unnecessary casts
  2015-05-15 17:08 [dpdk-dev] [PATCH v2 0/5] ixgbe: logging patches Stephen Hemminger
@ 2015-05-15 17:08 ` Stephen Hemminger
  2015-05-18  9:24   ` Bruce Richardson
  2015-05-15 17:08 ` [dpdk-dev] [PATCH 2/5] ixgbe: don't print PCI address on link change Stephen Hemminger
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 14+ messages in thread
From: Stephen Hemminger @ 2015-05-15 17:08 UTC (permalink / raw)
  To: helin.zhang; +Cc: dev

Don't do unnecessary casts when logging messages. Better to use
the correct printf format code.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_pmd_ixgbe/ixgbe_ethdev.c | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
index 5f9a1cf..a585151 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
@@ -568,8 +568,8 @@ ixgbe_dev_queue_stats_mapping_set(struct rte_eth_dev *eth_dev,
 		(hw->mac.type != ixgbe_mac_X550EM_x))
 		return -ENOSYS;
 
-	PMD_INIT_LOG(INFO, "Setting port %d, %s queue_id %d to stat index %d",
-		     (int)(eth_dev->data->port_id), is_rx ? "RX" : "TX",
+	PMD_INIT_LOG(INFO, "Setting port %u, %s queue_id %d to stat index %d",
+		     eth_dev->data->port_id, is_rx ? "RX" : "TX",
 		     queue_id, stat_idx);
 
 	n = (uint8_t)(queue_id / NB_QMAP_FIELDS_PER_QSM_REG);
@@ -594,8 +594,8 @@ ixgbe_dev_queue_stats_mapping_set(struct rte_eth_dev *eth_dev,
 	else
 		stat_mappings->rqsmr[n] |= qsmr_mask;
 
-	PMD_INIT_LOG(INFO, "Set port %d, %s queue_id %d to stat index %d",
-		     (int)(eth_dev->data->port_id), is_rx ? "RX" : "TX",
+	PMD_INIT_LOG(INFO, "Set port %u, %s queue_id %d to stat index %d",
+		     eth_dev->data->port_id, is_rx ? "RX" : "TX",
 		     queue_id, stat_idx);
 	PMD_INIT_LOG(INFO, "%s[%d] = 0x%08x", is_rx ? "RQSMR" : "TQSM", n,
 		     is_rx ? stat_mappings->rqsmr[n] : stat_mappings->tqsm[n]);
@@ -889,11 +889,11 @@ eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev)
 
 	if (ixgbe_is_sfp(hw) && hw->phy.sfp_type != ixgbe_sfp_type_not_present)
 		PMD_INIT_LOG(DEBUG, "MAC: %d, PHY: %d, SFP+: %d",
-			     (int) hw->mac.type, (int) hw->phy.type,
-			     (int) hw->phy.sfp_type);
+			     hw->mac.type, hw->phy.type,
+			     hw->phy.sfp_type);
 	else
 		PMD_INIT_LOG(DEBUG, "MAC: %d, PHY: %d",
-			     (int) hw->mac.type, (int) hw->phy.type);
+			     hw->mac.type, hw->phy.type);
 
 	PMD_INIT_LOG(DEBUG, "port %d vendorID=0x%x deviceID=0x%x",
 			eth_dev->data->port_id, pci_dev->id.vendor_id,
@@ -2307,14 +2307,13 @@ ixgbe_dev_link_status_print(struct rte_eth_dev *dev)
 	memset(&link, 0, sizeof(link));
 	rte_ixgbe_dev_atomic_read_link_status(dev, &link);
 	if (link.link_status) {
-		PMD_INIT_LOG(INFO, "Port %d: Link Up - speed %u Mbps - %s",
-					(int)(dev->data->port_id),
-					(unsigned)link.link_speed,
-			link.link_duplex == ETH_LINK_FULL_DUPLEX ?
+		PMD_INIT_LOG(INFO, "Port %u: Link Up - speed %u Mbps - %s",
+			     dev->data->port_id, link.link_speed,
+			     link.link_duplex == ETH_LINK_FULL_DUPLEX ?
 					"full-duplex" : "half-duplex");
 	} else {
-		PMD_INIT_LOG(INFO, " Port %d: Link Down",
-				(int)(dev->data->port_id));
+		PMD_INIT_LOG(INFO, "Port %u: Link Down",
+				dev->data->port_id);
 	}
 	PMD_INIT_LOG(INFO, "PCI Address: %04d:%02d:%02d:%d",
 				dev->pci_dev->addr.domain,
-- 
2.1.4

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [dpdk-dev] [PATCH 2/5] ixgbe: don't print PCI address on link change
  2015-05-15 17:08 [dpdk-dev] [PATCH v2 0/5] ixgbe: logging patches Stephen Hemminger
  2015-05-15 17:08 ` [dpdk-dev] [PATCH 1/5] ixgbe: remove unnecessary casts Stephen Hemminger
@ 2015-05-15 17:08 ` Stephen Hemminger
  2015-05-18  9:32   ` Bruce Richardson
  2015-05-15 17:08 ` [dpdk-dev] [PATCH 3/5] ixgbe: raise priority of significant events Stephen Hemminger
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 14+ messages in thread
From: Stephen Hemminger @ 2015-05-15 17:08 UTC (permalink / raw)
  To: helin.zhang; +Cc: dev

Printing PCI information on link state change is unnecessary since
the same information has already been displayed earlier in the log.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_pmd_ixgbe/ixgbe_ethdev.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
index a585151..fa335f4 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
@@ -2315,11 +2315,6 @@ ixgbe_dev_link_status_print(struct rte_eth_dev *dev)
 		PMD_INIT_LOG(INFO, "Port %u: Link Down",
 				dev->data->port_id);
 	}
-	PMD_INIT_LOG(INFO, "PCI Address: %04d:%02d:%02d:%d",
-				dev->pci_dev->addr.domain,
-				dev->pci_dev->addr.bus,
-				dev->pci_dev->addr.devid,
-				dev->pci_dev->addr.function);
 }
 
 /*
-- 
2.1.4

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [dpdk-dev] [PATCH 3/5] ixgbe: raise priority of significant events
  2015-05-15 17:08 [dpdk-dev] [PATCH v2 0/5] ixgbe: logging patches Stephen Hemminger
  2015-05-15 17:08 ` [dpdk-dev] [PATCH 1/5] ixgbe: remove unnecessary casts Stephen Hemminger
  2015-05-15 17:08 ` [dpdk-dev] [PATCH 2/5] ixgbe: don't print PCI address on link change Stephen Hemminger
@ 2015-05-15 17:08 ` Stephen Hemminger
  2015-05-18  9:27   ` Bruce Richardson
  2015-05-15 17:08 ` [dpdk-dev] [PATCH 4/5] ixgbe: use RTE_LOG not rte_log Stephen Hemminger
  2015-05-15 17:08 ` [dpdk-dev] [PATCH 5/5] ixgbe: silence noisy log messages Stephen Hemminger
  4 siblings, 1 reply; 14+ messages in thread
From: Stephen Hemminger @ 2015-05-15 17:08 UTC (permalink / raw)
  To: helin.zhang; +Cc: dev

The driver does lots of logging at INFO level, but some setup
events are significant and should be at NOTICE or ERR level
since they are problems that user should see.

Also never put tabs in log messages because they get mangled
by syslog processing.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_pmd_ixgbe/ixgbe_ethdev.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
index fa335f4..7e75382 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
@@ -1054,8 +1054,8 @@ eth_ixgbevf_dev_init(struct rte_eth_dev *eth_dev)
 			eth_dev->data->mac_addrs = NULL;
 			return diag;
 		}
-		PMD_INIT_LOG(INFO, "\tVF MAC address not assigned by Host PF");
-		PMD_INIT_LOG(INFO, "\tAssign randomly generated MAC address "
+		PMD_INIT_LOG(NOTICE, "VF MAC address not assigned by Host PF");
+		PMD_INIT_LOG(NOTICE, "Assign randomly generated MAC address "
 			     "%02x:%02x:%02x:%02x:%02x:%02x",
 			     perm_addr->addr_bytes[0],
 			     perm_addr->addr_bytes[1],
@@ -1248,7 +1248,7 @@ ixgbe_vlan_hw_strip_disable(struct rte_eth_dev *dev, uint16_t queue)
 
 	if (hw->mac.type == ixgbe_mac_82598EB) {
 		/* No queue level support */
-		PMD_INIT_LOG(INFO, "82598EB not support queue level hw strip");
+		PMD_INIT_LOG(ERR, "82598EB not support queue level hw strip");
 		return;
 	}
 	else {
@@ -1272,7 +1272,7 @@ ixgbe_vlan_hw_strip_enable(struct rte_eth_dev *dev, uint16_t queue)
 
 	if (hw->mac.type == ixgbe_mac_82598EB) {
 		/* No queue level supported */
-		PMD_INIT_LOG(INFO, "82598EB not support queue level hw strip");
+		PMD_INIT_LOG(ERR, "82598EB not support queue level hw strip");
 		return;
 	}
 	else {
@@ -2951,12 +2951,12 @@ ixgbevf_dev_configure(struct rte_eth_dev *dev)
 	 */
 #ifndef RTE_LIBRTE_IXGBE_PF_DISABLE_STRIP_CRC
 	if (!conf->rxmode.hw_strip_crc) {
-		PMD_INIT_LOG(INFO, "VF can't disable HW CRC Strip");
+		PMD_INIT_LOG(NOTICE, "VF can't disable HW CRC Strip");
 		conf->rxmode.hw_strip_crc = 1;
 	}
 #else
 	if (conf->rxmode.hw_strip_crc) {
-		PMD_INIT_LOG(INFO, "VF can't enable HW CRC Strip");
+		PMD_INIT_LOG(NOTICE, "VF can't enable HW CRC Strip");
 		conf->rxmode.hw_strip_crc = 0;
 	}
 #endif
-- 
2.1.4

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [dpdk-dev] [PATCH 4/5] ixgbe: use RTE_LOG not rte_log
  2015-05-15 17:08 [dpdk-dev] [PATCH v2 0/5] ixgbe: logging patches Stephen Hemminger
                   ` (2 preceding siblings ...)
  2015-05-15 17:08 ` [dpdk-dev] [PATCH 3/5] ixgbe: raise priority of significant events Stephen Hemminger
@ 2015-05-15 17:08 ` Stephen Hemminger
  2015-05-18  9:28   ` Bruce Richardson
  2015-05-15 17:08 ` [dpdk-dev] [PATCH 5/5] ixgbe: silence noisy log messages Stephen Hemminger
  4 siblings, 1 reply; 14+ messages in thread
From: Stephen Hemminger @ 2015-05-15 17:08 UTC (permalink / raw)
  To: helin.zhang; +Cc: dev

This driver should follow standard DPDK practice and use
RTE_LOG macro which allows setting config option to remove
the debug log messages.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_pmd_ixgbe/ixgbe_logs.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lib/librte_pmd_ixgbe/ixgbe_logs.h b/lib/librte_pmd_ixgbe/ixgbe_logs.h
index 572e030..53ba42d 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_logs.h
+++ b/lib/librte_pmd_ixgbe/ixgbe_logs.h
@@ -35,8 +35,7 @@
 #define _IXGBE_LOGS_H_
 
 #define PMD_INIT_LOG(level, fmt, args...) \
-	rte_log(RTE_LOG_ ## level, RTE_LOGTYPE_PMD, \
-		"PMD: %s(): " fmt "\n", __func__, ##args)
+	RTE_LOG(level, PMD, "%s(): " fmt "\n", __func__, ##args)
 
 #ifdef RTE_LIBRTE_IXGBE_DEBUG_INIT
 #define PMD_INIT_FUNC_TRACE() PMD_INIT_LOG(DEBUG, " >>")
-- 
2.1.4

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [dpdk-dev] [PATCH 5/5] ixgbe: silence noisy log messages
  2015-05-15 17:08 [dpdk-dev] [PATCH v2 0/5] ixgbe: logging patches Stephen Hemminger
                   ` (3 preceding siblings ...)
  2015-05-15 17:08 ` [dpdk-dev] [PATCH 4/5] ixgbe: use RTE_LOG not rte_log Stephen Hemminger
@ 2015-05-15 17:08 ` Stephen Hemminger
  2015-05-18  9:32   ` Bruce Richardson
  4 siblings, 1 reply; 14+ messages in thread
From: Stephen Hemminger @ 2015-05-15 17:08 UTC (permalink / raw)
  To: helin.zhang; +Cc: dev

The ixgbe driver likes to be far to chatty in the system log
which is good for the original developer but not good for a production
product. All the normal messages should be changed from INFO to DEBUG.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_pmd_ixgbe/ixgbe_ethdev.c | 14 +++++++-------
 lib/librte_pmd_ixgbe/ixgbe_rxtx.c   | 20 ++++++++++----------
 2 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
index 7e75382..bb24a17 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
@@ -568,7 +568,7 @@ ixgbe_dev_queue_stats_mapping_set(struct rte_eth_dev *eth_dev,
 		(hw->mac.type != ixgbe_mac_X550EM_x))
 		return -ENOSYS;
 
-	PMD_INIT_LOG(INFO, "Setting port %u, %s queue_id %d to stat index %d",
+	PMD_INIT_LOG(DEBUG, "Setting port %u, %s queue_id %d to stat index %d",
 		     eth_dev->data->port_id, is_rx ? "RX" : "TX",
 		     queue_id, stat_idx);
 
@@ -594,20 +594,20 @@ ixgbe_dev_queue_stats_mapping_set(struct rte_eth_dev *eth_dev,
 	else
 		stat_mappings->rqsmr[n] |= qsmr_mask;
 
-	PMD_INIT_LOG(INFO, "Set port %u, %s queue_id %d to stat index %d",
+	PMD_INIT_LOG(DEBUG, "Set port %u, %s queue_id %d to stat index %d",
 		     eth_dev->data->port_id, is_rx ? "RX" : "TX",
 		     queue_id, stat_idx);
-	PMD_INIT_LOG(INFO, "%s[%d] = 0x%08x", is_rx ? "RQSMR" : "TQSM", n,
+	PMD_INIT_LOG(DEBUG, "%s[%d] = 0x%08x", is_rx ? "RQSMR" : "TQSM", n,
 		     is_rx ? stat_mappings->rqsmr[n] : stat_mappings->tqsm[n]);
 
 	/* Now write the mapping in the appropriate register */
 	if (is_rx) {
-		PMD_INIT_LOG(INFO, "Write 0x%x to RX IXGBE stat mapping reg:%d",
+		PMD_INIT_LOG(DEBUG, "Write 0x%x to RX IXGBE stat mapping reg:%d",
 			     stat_mappings->rqsmr[n], n);
 		IXGBE_WRITE_REG(hw, IXGBE_RQSMR(n), stat_mappings->rqsmr[n]);
 	}
 	else {
-		PMD_INIT_LOG(INFO, "Write 0x%x to TX IXGBE stat mapping reg:%d",
+		PMD_INIT_LOG(DEBUG, "Write 0x%x to TX IXGBE stat mapping reg:%d",
 			     stat_mappings->tqsm[n], n);
 		IXGBE_WRITE_REG(hw, IXGBE_TQSM(n), stat_mappings->tqsm[n]);
 	}
@@ -751,7 +751,7 @@ eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev)
 			ixgbe_set_tx_function(eth_dev, txq);
 		} else {
 			/* Use default TX function if we get here */
-			PMD_INIT_LOG(INFO, "No TX queues configured yet. "
+			PMD_INIT_LOG(DEBUG, "No TX queues configured yet. "
 			                   "Using default TX function.");
 		}
 
@@ -2275,7 +2275,7 @@ ixgbe_dev_interrupt_get_status(struct rte_eth_dev *dev)
 
 	/* read-on-clear nic registers here */
 	eicr = IXGBE_READ_REG(hw, IXGBE_EICR);
-	PMD_DRV_LOG(INFO, "eicr %x", eicr);
+	PMD_DRV_LOG(DEBUG, "eicr %x", eicr);
 
 	intr->flags = 0;
 	if (eicr & IXGBE_EICR_LSC) {
diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
index 57c9430..08830bf 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
@@ -1871,23 +1871,23 @@ ixgbe_set_tx_function(struct rte_eth_dev *dev, struct ixgbe_tx_queue *txq)
 	/* Use a simple Tx queue (no offloads, no multi segs) if possible */
 	if (((txq->txq_flags & IXGBE_SIMPLE_FLAGS) == IXGBE_SIMPLE_FLAGS)
 			&& (txq->tx_rs_thresh >= RTE_PMD_IXGBE_TX_MAX_BURST)) {
-		PMD_INIT_LOG(INFO, "Using simple tx code path");
+		PMD_INIT_LOG(DEBUG, "Using simple tx code path");
 #ifdef RTE_IXGBE_INC_VECTOR
 		if (txq->tx_rs_thresh <= RTE_IXGBE_TX_MAX_FREE_BUF_SZ &&
 				(rte_eal_process_type() != RTE_PROC_PRIMARY ||
 					ixgbe_txq_vec_setup(txq) == 0)) {
-			PMD_INIT_LOG(INFO, "Vector tx enabled.");
+			PMD_INIT_LOG(DEBUG, "Vector tx enabled.");
 			dev->tx_pkt_burst = ixgbe_xmit_pkts_vec;
 		} else
 #endif
 		dev->tx_pkt_burst = ixgbe_xmit_pkts_simple;
 	} else {
-		PMD_INIT_LOG(INFO, "Using full-featured tx code path");
-		PMD_INIT_LOG(INFO,
+		PMD_INIT_LOG(DEBUG, "Using full-featured tx code path");
+		PMD_INIT_LOG(DEBUG,
 				" - txq_flags = %lx " "[IXGBE_SIMPLE_FLAGS=%lx]",
 				(unsigned long)txq->txq_flags,
 				(unsigned long)IXGBE_SIMPLE_FLAGS);
-		PMD_INIT_LOG(INFO,
+		PMD_INIT_LOG(DEBUG,
 				" - tx_rs_thresh = %lu " "[RTE_PMD_IXGBE_TX_MAX_BURST=%lu]",
 				(unsigned long)txq->tx_rs_thresh,
 				(unsigned long)RTE_PMD_IXGBE_TX_MAX_BURST);
@@ -3761,11 +3761,11 @@ void ixgbe_set_rx_function(struct rte_eth_dev *dev)
 	 */
 	if (dev->data->lro) {
 		if (adapter->rx_bulk_alloc_allowed) {
-			PMD_INIT_LOG(INFO, "LRO is requested. Using a bulk "
+			PMD_INIT_LOG(DEBUG, "LRO is requested. Using a bulk "
 					   "allocation version");
 			dev->rx_pkt_burst = ixgbe_recv_pkts_lro_bulk_alloc;
 		} else {
-			PMD_INIT_LOG(INFO, "LRO is requested. Using a single "
+			PMD_INIT_LOG(DEBUG, "LRO is requested. Using a single "
 					   "allocation version");
 			dev->rx_pkt_burst = ixgbe_recv_pkts_lro_single_alloc;
 		}
@@ -3781,7 +3781,7 @@ void ixgbe_set_rx_function(struct rte_eth_dev *dev)
 
 			dev->rx_pkt_burst = ixgbe_recv_scattered_pkts_vec;
 		} else if (adapter->rx_bulk_alloc_allowed) {
-			PMD_INIT_LOG(INFO, "Using a Scattered with bulk "
+			PMD_INIT_LOG(DEBUG, "Using a Scattered with bulk "
 					   "allocation callback (port=%d).",
 				     dev->data->port_id);
 			dev->rx_pkt_burst = ixgbe_recv_pkts_lro_bulk_alloc;
@@ -3803,7 +3803,7 @@ void ixgbe_set_rx_function(struct rte_eth_dev *dev)
 	 *    - Single buffer allocation (the simplest one)
 	 */
 	} else if (adapter->rx_vec_allowed) {
-		PMD_INIT_LOG(INFO, "Vector rx enabled, please make sure RX "
+		PMD_INIT_LOG(DEBUG, "Vector rx enabled, please make sure RX "
 				   "burst size no less than 32.");
 
 		dev->rx_pkt_burst = ixgbe_recv_pkts_vec;
@@ -3961,7 +3961,7 @@ ixgbe_set_rsc(struct rte_eth_dev *dev)
 
 	dev->data->lro = 1;
 
-	PMD_INIT_LOG(INFO, "enabling LRO mode");
+	PMD_INIT_LOG(DEBUG, "enabling LRO mode");
 
 	return 0;
 }
-- 
2.1.4

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [PATCH 1/5] ixgbe: remove unnecessary casts
  2015-05-15 17:08 ` [dpdk-dev] [PATCH 1/5] ixgbe: remove unnecessary casts Stephen Hemminger
@ 2015-05-18  9:24   ` Bruce Richardson
  0 siblings, 0 replies; 14+ messages in thread
From: Bruce Richardson @ 2015-05-18  9:24 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev

On Fri, May 15, 2015 at 10:08:23AM -0700, Stephen Hemminger wrote:
> Don't do unnecessary casts when logging messages. Better to use
> the correct printf format code.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

+1 

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

> ---
>  lib/librte_pmd_ixgbe/ixgbe_ethdev.c | 25 ++++++++++++-------------
>  1 file changed, 12 insertions(+), 13 deletions(-)
> 
> diff --git a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
> index 5f9a1cf..a585151 100644
> --- a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
> +++ b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
> @@ -568,8 +568,8 @@ ixgbe_dev_queue_stats_mapping_set(struct rte_eth_dev *eth_dev,
>  		(hw->mac.type != ixgbe_mac_X550EM_x))
>  		return -ENOSYS;
>  
> -	PMD_INIT_LOG(INFO, "Setting port %d, %s queue_id %d to stat index %d",
> -		     (int)(eth_dev->data->port_id), is_rx ? "RX" : "TX",
> +	PMD_INIT_LOG(INFO, "Setting port %u, %s queue_id %d to stat index %d",
> +		     eth_dev->data->port_id, is_rx ? "RX" : "TX",
>  		     queue_id, stat_idx);
>  
>  	n = (uint8_t)(queue_id / NB_QMAP_FIELDS_PER_QSM_REG);
> @@ -594,8 +594,8 @@ ixgbe_dev_queue_stats_mapping_set(struct rte_eth_dev *eth_dev,
>  	else
>  		stat_mappings->rqsmr[n] |= qsmr_mask;
>  
> -	PMD_INIT_LOG(INFO, "Set port %d, %s queue_id %d to stat index %d",
> -		     (int)(eth_dev->data->port_id), is_rx ? "RX" : "TX",
> +	PMD_INIT_LOG(INFO, "Set port %u, %s queue_id %d to stat index %d",
> +		     eth_dev->data->port_id, is_rx ? "RX" : "TX",
>  		     queue_id, stat_idx);
>  	PMD_INIT_LOG(INFO, "%s[%d] = 0x%08x", is_rx ? "RQSMR" : "TQSM", n,
>  		     is_rx ? stat_mappings->rqsmr[n] : stat_mappings->tqsm[n]);
> @@ -889,11 +889,11 @@ eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev)
>  
>  	if (ixgbe_is_sfp(hw) && hw->phy.sfp_type != ixgbe_sfp_type_not_present)
>  		PMD_INIT_LOG(DEBUG, "MAC: %d, PHY: %d, SFP+: %d",
> -			     (int) hw->mac.type, (int) hw->phy.type,
> -			     (int) hw->phy.sfp_type);
> +			     hw->mac.type, hw->phy.type,
> +			     hw->phy.sfp_type);
>  	else
>  		PMD_INIT_LOG(DEBUG, "MAC: %d, PHY: %d",
> -			     (int) hw->mac.type, (int) hw->phy.type);
> +			     hw->mac.type, hw->phy.type);
>  
>  	PMD_INIT_LOG(DEBUG, "port %d vendorID=0x%x deviceID=0x%x",
>  			eth_dev->data->port_id, pci_dev->id.vendor_id,
> @@ -2307,14 +2307,13 @@ ixgbe_dev_link_status_print(struct rte_eth_dev *dev)
>  	memset(&link, 0, sizeof(link));
>  	rte_ixgbe_dev_atomic_read_link_status(dev, &link);
>  	if (link.link_status) {
> -		PMD_INIT_LOG(INFO, "Port %d: Link Up - speed %u Mbps - %s",
> -					(int)(dev->data->port_id),
> -					(unsigned)link.link_speed,
> -			link.link_duplex == ETH_LINK_FULL_DUPLEX ?
> +		PMD_INIT_LOG(INFO, "Port %u: Link Up - speed %u Mbps - %s",
> +			     dev->data->port_id, link.link_speed,
> +			     link.link_duplex == ETH_LINK_FULL_DUPLEX ?
>  					"full-duplex" : "half-duplex");
>  	} else {
> -		PMD_INIT_LOG(INFO, " Port %d: Link Down",
> -				(int)(dev->data->port_id));
> +		PMD_INIT_LOG(INFO, "Port %u: Link Down",
> +				dev->data->port_id);
>  	}
>  	PMD_INIT_LOG(INFO, "PCI Address: %04d:%02d:%02d:%d",
>  				dev->pci_dev->addr.domain,
> -- 
> 2.1.4
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [PATCH 3/5] ixgbe: raise priority of significant events
  2015-05-15 17:08 ` [dpdk-dev] [PATCH 3/5] ixgbe: raise priority of significant events Stephen Hemminger
@ 2015-05-18  9:27   ` Bruce Richardson
  0 siblings, 0 replies; 14+ messages in thread
From: Bruce Richardson @ 2015-05-18  9:27 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev

On Fri, May 15, 2015 at 10:08:25AM -0700, Stephen Hemminger wrote:
> The driver does lots of logging at INFO level, but some setup
> events are significant and should be at NOTICE or ERR level
> since they are problems that user should see.
> 
> Also never put tabs in log messages because they get mangled
> by syslog processing.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

One small nit below.

> ---
>  lib/librte_pmd_ixgbe/ixgbe_ethdev.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
> index fa335f4..7e75382 100644
> --- a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
> +++ b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
> @@ -1054,8 +1054,8 @@ eth_ixgbevf_dev_init(struct rte_eth_dev *eth_dev)
>  			eth_dev->data->mac_addrs = NULL;
>  			return diag;
>  		}
> -		PMD_INIT_LOG(INFO, "\tVF MAC address not assigned by Host PF");
> -		PMD_INIT_LOG(INFO, "\tAssign randomly generated MAC address "
> +		PMD_INIT_LOG(NOTICE, "VF MAC address not assigned by Host PF");
> +		PMD_INIT_LOG(NOTICE, "Assign randomly generated MAC address "
>  			     "%02x:%02x:%02x:%02x:%02x:%02x",
>  			     perm_addr->addr_bytes[0],
>  			     perm_addr->addr_bytes[1],
> @@ -1248,7 +1248,7 @@ ixgbe_vlan_hw_strip_disable(struct rte_eth_dev *dev, uint16_t queue)
>  
>  	if (hw->mac.type == ixgbe_mac_82598EB) {
>  		/* No queue level support */
> -		PMD_INIT_LOG(INFO, "82598EB not support queue level hw strip");
> +		PMD_INIT_LOG(ERR, "82598EB not support queue level hw strip");

Should we not fix the text here to be "82599EB does not support ..." while we
are making this change? (Same with next chunk below too).

>  		return;
>  	}
>  	else {
> @@ -1272,7 +1272,7 @@ ixgbe_vlan_hw_strip_enable(struct rte_eth_dev *dev, uint16_t queue)
>  
>  	if (hw->mac.type == ixgbe_mac_82598EB) {
>  		/* No queue level supported */
> -		PMD_INIT_LOG(INFO, "82598EB not support queue level hw strip");
> +		PMD_INIT_LOG(ERR, "82598EB not support queue level hw strip");
>  		return;
>  	}
>  	else {
> @@ -2951,12 +2951,12 @@ ixgbevf_dev_configure(struct rte_eth_dev *dev)
>  	 */
>  #ifndef RTE_LIBRTE_IXGBE_PF_DISABLE_STRIP_CRC
>  	if (!conf->rxmode.hw_strip_crc) {
> -		PMD_INIT_LOG(INFO, "VF can't disable HW CRC Strip");
> +		PMD_INIT_LOG(NOTICE, "VF can't disable HW CRC Strip");
>  		conf->rxmode.hw_strip_crc = 1;
>  	}
>  #else
>  	if (conf->rxmode.hw_strip_crc) {
> -		PMD_INIT_LOG(INFO, "VF can't enable HW CRC Strip");
> +		PMD_INIT_LOG(NOTICE, "VF can't enable HW CRC Strip");
>  		conf->rxmode.hw_strip_crc = 0;
>  	}
>  #endif
> -- 
> 2.1.4
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [PATCH 4/5] ixgbe: use RTE_LOG not rte_log
  2015-05-15 17:08 ` [dpdk-dev] [PATCH 4/5] ixgbe: use RTE_LOG not rte_log Stephen Hemminger
@ 2015-05-18  9:28   ` Bruce Richardson
  0 siblings, 0 replies; 14+ messages in thread
From: Bruce Richardson @ 2015-05-18  9:28 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev

On Fri, May 15, 2015 at 10:08:26AM -0700, Stephen Hemminger wrote:
> This driver should follow standard DPDK practice and use
> RTE_LOG macro which allows setting config option to remove
> the debug log messages.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

> ---
>  lib/librte_pmd_ixgbe/ixgbe_logs.h | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/lib/librte_pmd_ixgbe/ixgbe_logs.h b/lib/librte_pmd_ixgbe/ixgbe_logs.h
> index 572e030..53ba42d 100644
> --- a/lib/librte_pmd_ixgbe/ixgbe_logs.h
> +++ b/lib/librte_pmd_ixgbe/ixgbe_logs.h
> @@ -35,8 +35,7 @@
>  #define _IXGBE_LOGS_H_
>  
>  #define PMD_INIT_LOG(level, fmt, args...) \
> -	rte_log(RTE_LOG_ ## level, RTE_LOGTYPE_PMD, \
> -		"PMD: %s(): " fmt "\n", __func__, ##args)
> +	RTE_LOG(level, PMD, "%s(): " fmt "\n", __func__, ##args)
>  
>  #ifdef RTE_LIBRTE_IXGBE_DEBUG_INIT
>  #define PMD_INIT_FUNC_TRACE() PMD_INIT_LOG(DEBUG, " >>")
> -- 
> 2.1.4
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [PATCH 5/5] ixgbe: silence noisy log messages
  2015-05-15 17:08 ` [dpdk-dev] [PATCH 5/5] ixgbe: silence noisy log messages Stephen Hemminger
@ 2015-05-18  9:32   ` Bruce Richardson
  2015-05-18 16:39     ` Stephen Hemminger
  0 siblings, 1 reply; 14+ messages in thread
From: Bruce Richardson @ 2015-05-18  9:32 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev

On Fri, May 15, 2015 at 10:08:27AM -0700, Stephen Hemminger wrote:
> The ixgbe driver likes to be far to chatty in the system log
> which is good for the original developer but not good for a production
> product. All the normal messages should be changed from INFO to DEBUG.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

For the most part, this looks fine. However, I'm unsure about changing the log
level of the messages stating what the RX and TX burst functions in use are. I
would view this as important information that should generally be displayed as
the performance impacts of using a sub-optimal RX/TX code path are large.

/Bruce

> ---
>  lib/librte_pmd_ixgbe/ixgbe_ethdev.c | 14 +++++++-------
>  lib/librte_pmd_ixgbe/ixgbe_rxtx.c   | 20 ++++++++++----------
>  2 files changed, 17 insertions(+), 17 deletions(-)
> 
> diff --git a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
> index 7e75382..bb24a17 100644
> --- a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
> +++ b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
> @@ -568,7 +568,7 @@ ixgbe_dev_queue_stats_mapping_set(struct rte_eth_dev *eth_dev,
>  		(hw->mac.type != ixgbe_mac_X550EM_x))
>  		return -ENOSYS;
>  
> -	PMD_INIT_LOG(INFO, "Setting port %u, %s queue_id %d to stat index %d",
> +	PMD_INIT_LOG(DEBUG, "Setting port %u, %s queue_id %d to stat index %d",
>  		     eth_dev->data->port_id, is_rx ? "RX" : "TX",
>  		     queue_id, stat_idx);
>  
> @@ -594,20 +594,20 @@ ixgbe_dev_queue_stats_mapping_set(struct rte_eth_dev *eth_dev,
>  	else
>  		stat_mappings->rqsmr[n] |= qsmr_mask;
>  
> -	PMD_INIT_LOG(INFO, "Set port %u, %s queue_id %d to stat index %d",
> +	PMD_INIT_LOG(DEBUG, "Set port %u, %s queue_id %d to stat index %d",
>  		     eth_dev->data->port_id, is_rx ? "RX" : "TX",
>  		     queue_id, stat_idx);
> -	PMD_INIT_LOG(INFO, "%s[%d] = 0x%08x", is_rx ? "RQSMR" : "TQSM", n,
> +	PMD_INIT_LOG(DEBUG, "%s[%d] = 0x%08x", is_rx ? "RQSMR" : "TQSM", n,
>  		     is_rx ? stat_mappings->rqsmr[n] : stat_mappings->tqsm[n]);
>  
>  	/* Now write the mapping in the appropriate register */
>  	if (is_rx) {
> -		PMD_INIT_LOG(INFO, "Write 0x%x to RX IXGBE stat mapping reg:%d",
> +		PMD_INIT_LOG(DEBUG, "Write 0x%x to RX IXGBE stat mapping reg:%d",
>  			     stat_mappings->rqsmr[n], n);
>  		IXGBE_WRITE_REG(hw, IXGBE_RQSMR(n), stat_mappings->rqsmr[n]);
>  	}
>  	else {
> -		PMD_INIT_LOG(INFO, "Write 0x%x to TX IXGBE stat mapping reg:%d",
> +		PMD_INIT_LOG(DEBUG, "Write 0x%x to TX IXGBE stat mapping reg:%d",
>  			     stat_mappings->tqsm[n], n);
>  		IXGBE_WRITE_REG(hw, IXGBE_TQSM(n), stat_mappings->tqsm[n]);
>  	}
> @@ -751,7 +751,7 @@ eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev)
>  			ixgbe_set_tx_function(eth_dev, txq);
>  		} else {
>  			/* Use default TX function if we get here */
> -			PMD_INIT_LOG(INFO, "No TX queues configured yet. "
> +			PMD_INIT_LOG(DEBUG, "No TX queues configured yet. "
>  			                   "Using default TX function.");
>  		}
>  
> @@ -2275,7 +2275,7 @@ ixgbe_dev_interrupt_get_status(struct rte_eth_dev *dev)
>  
>  	/* read-on-clear nic registers here */
>  	eicr = IXGBE_READ_REG(hw, IXGBE_EICR);
> -	PMD_DRV_LOG(INFO, "eicr %x", eicr);
> +	PMD_DRV_LOG(DEBUG, "eicr %x", eicr);
>  
>  	intr->flags = 0;
>  	if (eicr & IXGBE_EICR_LSC) {
> diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> index 57c9430..08830bf 100644
> --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> @@ -1871,23 +1871,23 @@ ixgbe_set_tx_function(struct rte_eth_dev *dev, struct ixgbe_tx_queue *txq)
>  	/* Use a simple Tx queue (no offloads, no multi segs) if possible */
>  	if (((txq->txq_flags & IXGBE_SIMPLE_FLAGS) == IXGBE_SIMPLE_FLAGS)
>  			&& (txq->tx_rs_thresh >= RTE_PMD_IXGBE_TX_MAX_BURST)) {
> -		PMD_INIT_LOG(INFO, "Using simple tx code path");
> +		PMD_INIT_LOG(DEBUG, "Using simple tx code path");
>  #ifdef RTE_IXGBE_INC_VECTOR
>  		if (txq->tx_rs_thresh <= RTE_IXGBE_TX_MAX_FREE_BUF_SZ &&
>  				(rte_eal_process_type() != RTE_PROC_PRIMARY ||
>  					ixgbe_txq_vec_setup(txq) == 0)) {
> -			PMD_INIT_LOG(INFO, "Vector tx enabled.");
> +			PMD_INIT_LOG(DEBUG, "Vector tx enabled.");
>  			dev->tx_pkt_burst = ixgbe_xmit_pkts_vec;
>  		} else
>  #endif
>  		dev->tx_pkt_burst = ixgbe_xmit_pkts_simple;
>  	} else {
> -		PMD_INIT_LOG(INFO, "Using full-featured tx code path");
> -		PMD_INIT_LOG(INFO,
> +		PMD_INIT_LOG(DEBUG, "Using full-featured tx code path");
> +		PMD_INIT_LOG(DEBUG,
>  				" - txq_flags = %lx " "[IXGBE_SIMPLE_FLAGS=%lx]",
>  				(unsigned long)txq->txq_flags,
>  				(unsigned long)IXGBE_SIMPLE_FLAGS);
> -		PMD_INIT_LOG(INFO,
> +		PMD_INIT_LOG(DEBUG,
>  				" - tx_rs_thresh = %lu " "[RTE_PMD_IXGBE_TX_MAX_BURST=%lu]",
>  				(unsigned long)txq->tx_rs_thresh,
>  				(unsigned long)RTE_PMD_IXGBE_TX_MAX_BURST);
> @@ -3761,11 +3761,11 @@ void ixgbe_set_rx_function(struct rte_eth_dev *dev)
>  	 */
>  	if (dev->data->lro) {
>  		if (adapter->rx_bulk_alloc_allowed) {
> -			PMD_INIT_LOG(INFO, "LRO is requested. Using a bulk "
> +			PMD_INIT_LOG(DEBUG, "LRO is requested. Using a bulk "
>  					   "allocation version");
>  			dev->rx_pkt_burst = ixgbe_recv_pkts_lro_bulk_alloc;
>  		} else {
> -			PMD_INIT_LOG(INFO, "LRO is requested. Using a single "
> +			PMD_INIT_LOG(DEBUG, "LRO is requested. Using a single "
>  					   "allocation version");
>  			dev->rx_pkt_burst = ixgbe_recv_pkts_lro_single_alloc;
>  		}
> @@ -3781,7 +3781,7 @@ void ixgbe_set_rx_function(struct rte_eth_dev *dev)
>  
>  			dev->rx_pkt_burst = ixgbe_recv_scattered_pkts_vec;
>  		} else if (adapter->rx_bulk_alloc_allowed) {
> -			PMD_INIT_LOG(INFO, "Using a Scattered with bulk "
> +			PMD_INIT_LOG(DEBUG, "Using a Scattered with bulk "
>  					   "allocation callback (port=%d).",
>  				     dev->data->port_id);
>  			dev->rx_pkt_burst = ixgbe_recv_pkts_lro_bulk_alloc;
> @@ -3803,7 +3803,7 @@ void ixgbe_set_rx_function(struct rte_eth_dev *dev)
>  	 *    - Single buffer allocation (the simplest one)
>  	 */
>  	} else if (adapter->rx_vec_allowed) {
> -		PMD_INIT_LOG(INFO, "Vector rx enabled, please make sure RX "
> +		PMD_INIT_LOG(DEBUG, "Vector rx enabled, please make sure RX "
>  				   "burst size no less than 32.");
>  
>  		dev->rx_pkt_burst = ixgbe_recv_pkts_vec;
> @@ -3961,7 +3961,7 @@ ixgbe_set_rsc(struct rte_eth_dev *dev)
>  
>  	dev->data->lro = 1;
>  
> -	PMD_INIT_LOG(INFO, "enabling LRO mode");
> +	PMD_INIT_LOG(DEBUG, "enabling LRO mode");
>  
>  	return 0;
>  }
> -- 
> 2.1.4
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [PATCH 2/5] ixgbe: don't print PCI address on link change
  2015-05-15 17:08 ` [dpdk-dev] [PATCH 2/5] ixgbe: don't print PCI address on link change Stephen Hemminger
@ 2015-05-18  9:32   ` Bruce Richardson
  0 siblings, 0 replies; 14+ messages in thread
From: Bruce Richardson @ 2015-05-18  9:32 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev

On Fri, May 15, 2015 at 10:08:24AM -0700, Stephen Hemminger wrote:
> Printing PCI information on link state change is unnecessary since
> the same information has already been displayed earlier in the log.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

> ---
>  lib/librte_pmd_ixgbe/ixgbe_ethdev.c | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
> index a585151..fa335f4 100644
> --- a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
> +++ b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
> @@ -2315,11 +2315,6 @@ ixgbe_dev_link_status_print(struct rte_eth_dev *dev)
>  		PMD_INIT_LOG(INFO, "Port %u: Link Down",
>  				dev->data->port_id);
>  	}
> -	PMD_INIT_LOG(INFO, "PCI Address: %04d:%02d:%02d:%d",
> -				dev->pci_dev->addr.domain,
> -				dev->pci_dev->addr.bus,
> -				dev->pci_dev->addr.devid,
> -				dev->pci_dev->addr.function);
>  }
>  
>  /*
> -- 
> 2.1.4
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [PATCH 5/5] ixgbe: silence noisy log messages
  2015-05-18  9:32   ` Bruce Richardson
@ 2015-05-18 16:39     ` Stephen Hemminger
  2015-05-19  7:46       ` David Marchand
  0 siblings, 1 reply; 14+ messages in thread
From: Stephen Hemminger @ 2015-05-18 16:39 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: dev

On Mon, 18 May 2015 10:32:01 +0100
Bruce Richardson <bruce.richardson@intel.com> wrote:

> For the most part, this looks fine. However, I'm unsure about changing the log
> level of the messages stating what the RX and TX burst functions in use are. I
> would view this as important information that should generally be displayed as
> the performance impacts of using a sub-optimal RX/TX code path are large.
> 
> /Bruce


At INFO level it shows up in log files that customers read.
This is an issue where DPDK has to grow up and be ready for real world
use, rather than being developer friendly.

Our customers ask about every log message (believe me). So if there
is no problem the drivers must be absolutely silent (STFU)

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [PATCH 5/5] ixgbe: silence noisy log messages
  2015-05-18 16:39     ` Stephen Hemminger
@ 2015-05-19  7:46       ` David Marchand
  2015-05-19 14:40         ` Stephen Hemminger
  0 siblings, 1 reply; 14+ messages in thread
From: David Marchand @ 2015-05-19  7:46 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev

On Mon, May 18, 2015 at 6:39 PM, Stephen Hemminger <
stephen@networkplumber.org> wrote:

> On Mon, 18 May 2015 10:32:01 +0100
> Bruce Richardson <bruce.richardson@intel.com> wrote:
>
> > For the most part, this looks fine. However, I'm unsure about changing
> the log
> > level of the messages stating what the RX and TX burst functions in use
> are. I
> > would view this as important information that should generally be
> displayed as
> > the performance impacts of using a sub-optimal RX/TX code path are large.
> >
> > /Bruce
>
>
> At INFO level it shows up in log files that customers read.
> This is an issue where DPDK has to grow up and be ready for real world
> use, rather than being developer friendly.
>
> Our customers ask about every log message (believe me). So if there
> is no problem the drivers must be absolutely silent (STFU)
>

Are there messages at INFO level you want to keep ?
Or can't you just set the log level (the runtime option not the build one)
to something like WARNING or ERR ?


-- 
David Marchand

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [PATCH 5/5] ixgbe: silence noisy log messages
  2015-05-19  7:46       ` David Marchand
@ 2015-05-19 14:40         ` Stephen Hemminger
  0 siblings, 0 replies; 14+ messages in thread
From: Stephen Hemminger @ 2015-05-19 14:40 UTC (permalink / raw)
  To: David Marchand; +Cc: dev

We need info messages about topology etc from other components.

On Tue, May 19, 2015 at 12:46 AM, David Marchand <david.marchand@6wind.com>
wrote:

> On Mon, May 18, 2015 at 6:39 PM, Stephen Hemminger <
> stephen@networkplumber.org> wrote:
>
>> On Mon, 18 May 2015 10:32:01 +0100
>> Bruce Richardson <bruce.richardson@intel.com> wrote:
>>
>> > For the most part, this looks fine. However, I'm unsure about changing
>> the log
>> > level of the messages stating what the RX and TX burst functions in use
>> are. I
>> > would view this as important information that should generally be
>> displayed as
>> > the performance impacts of using a sub-optimal RX/TX code path are
>> large.
>> >
>> > /Bruce
>>
>>
>> At INFO level it shows up in log files that customers read.
>> This is an issue where DPDK has to grow up and be ready for real world
>> use, rather than being developer friendly.
>>
>> Our customers ask about every log message (believe me). So if there
>> is no problem the drivers must be absolutely silent (STFU)
>>
>
> Are there messages at INFO level you want to keep ?
> Or can't you just set the log level (the runtime option not the build one)
> to something like WARNING or ERR ?
>
>
> --
> David Marchand
>
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2015-05-19 14:40 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-15 17:08 [dpdk-dev] [PATCH v2 0/5] ixgbe: logging patches Stephen Hemminger
2015-05-15 17:08 ` [dpdk-dev] [PATCH 1/5] ixgbe: remove unnecessary casts Stephen Hemminger
2015-05-18  9:24   ` Bruce Richardson
2015-05-15 17:08 ` [dpdk-dev] [PATCH 2/5] ixgbe: don't print PCI address on link change Stephen Hemminger
2015-05-18  9:32   ` Bruce Richardson
2015-05-15 17:08 ` [dpdk-dev] [PATCH 3/5] ixgbe: raise priority of significant events Stephen Hemminger
2015-05-18  9:27   ` Bruce Richardson
2015-05-15 17:08 ` [dpdk-dev] [PATCH 4/5] ixgbe: use RTE_LOG not rte_log Stephen Hemminger
2015-05-18  9:28   ` Bruce Richardson
2015-05-15 17:08 ` [dpdk-dev] [PATCH 5/5] ixgbe: silence noisy log messages Stephen Hemminger
2015-05-18  9:32   ` Bruce Richardson
2015-05-18 16:39     ` Stephen Hemminger
2015-05-19  7:46       ` David Marchand
2015-05-19 14:40         ` Stephen Hemminger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).