[PATCH 0/2] Intel driver silence excess logging

[PATCH 0/2] Intel driver silence excess logging

[Stephen Hemminger]

The Intel drivers seem to be getting more verbose in the log with
each release. This is not a good thing.

Stephen Hemminger (2):
  ixgbe: silence noisy log messages
  e1000: silence noisy log messages

 lib/librte_pmd_e1000/e1000_logs.h   |  3 +--
 lib/librte_pmd_e1000/em_ethdev.c    | 10 ++++------
 lib/librte_pmd_e1000/igb_ethdev.c   | 12 ++++--------
 lib/librte_pmd_ixgbe/ixgbe_ethdev.c | 15 +++++----------
 lib/librte_pmd_ixgbe/ixgbe_logs.h   |  3 +--
 lib/librte_pmd_ixgbe/ixgbe_rxtx.c   | 12 ++++++------
 6 files changed, 21 insertions(+), 34 deletions(-)

-- 
2.1.4

[PATCH 1/2] ixgbe: silence noisy log messages

[Stephen Hemminger]

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.

Reduce the log spam by doing:
 * All the normal messages should be changed from INFO to DEBUG.
 * The log messages should be done with RTE_LOG so that they can be
   compiled out if RTE_LOG_LEVEL is set.
 * The link state print routine prints more than is necessary
   PCI information is already known (earlier in log) and has
   no purpose here.

Signed-off-by: Stephen Hemminger <stephen-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ@public.gmane.org>

--- a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
@@ -2278,20 +2278,15 @@
 	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),
+		PMD_INIT_LOG(DEBUG, "Port %u: Link Up - speed %u Mbps - %s",
+					dev->data->port_id,
 					(unsigned)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(DEBUG, "Port %d: 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);
 }

 /*
--- 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, " >>")
--- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
@@ -1752,23 +1752,23 @@
 	/* 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);
@@ -2245,7 +2245,7 @@
 		dev->rx_pkt_burst = ixgbe_recv_pkts_bulk_alloc;
 #ifdef RTE_IXGBE_INC_VECTOR
 		if (!ixgbe_rx_vec_condition_check(dev)) {
-			PMD_INIT_LOG(INFO, "Vector rx enabled, please make "
+			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;
 		}
---
 lib/librte_pmd_ixgbe/ixgbe_ethdev.c | 15 +++++----------
 lib/librte_pmd_ixgbe/ixgbe_logs.h   |  3 +--
 lib/librte_pmd_ixgbe/ixgbe_rxtx.c   | 12 ++++++------
 3 files changed, 12 insertions(+), 18 deletions(-)

diff --git a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
index 9bdc046..171809f 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
@@ -2293,20 +2293,15 @@ 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),
+		PMD_INIT_LOG(DEBUG, "Port %u: Link Up - speed %u Mbps - %s",
+					dev->data->port_id,
 					(unsigned)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, "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);
+		PMD_INIT_LOG(DEBUG, "Port %d: Link Down",
+				dev->data->port_id);
+	}
 }
 
 /*
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, " >>")
diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
index 811493e..8881194 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
@@ -1770,23 +1770,23 @@ set_tx_function(struct rte_eth_dev *dev, struct igb_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);
@@ -2263,7 +2263,7 @@ ixgbe_dev_rx_queue_setup(struct rte_eth_dev *dev,
 #ifdef RTE_IXGBE_INC_VECTOR
 		if (!ixgbe_rx_vec_condition_check(dev) &&
 		    (rte_is_power_of_2(nb_desc))) {
-			PMD_INIT_LOG(INFO, "Vector rx enabled, please make "
+			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;
 		}
-- 
2.1.4

Re: [PATCH 1/2] ixgbe: silence noisy log messages

[David Marchand]

On 03/09/2015 08:18 PM, 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.
>
> Reduce the log spam by doing:
>   * All the normal messages should be changed from INFO to DEBUG.
>   * The log messages should be done with RTE_LOG so that they can be
>     compiled out if RTE_LOG_LEVEL is set.
>   * The link state print routine prints more than is necessary
>     PCI information is already known (earlier in log) and has
>     no purpose here.
>
> Signed-off-by: Stephen Hemminger <stephen-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ@public.gmane.org>

This patch is broken.

The changes are there twice.
I removed the first part of the mail.

> ---
>   lib/librte_pmd_ixgbe/ixgbe_ethdev.c | 15 +++++----------
>   lib/librte_pmd_ixgbe/ixgbe_logs.h   |  3 +--
>   lib/librte_pmd_ixgbe/ixgbe_rxtx.c   | 12 ++++++------
>   3 files changed, 12 insertions(+), 18 deletions(-)
>
> diff --git a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
> index 9bdc046..171809f 100644
> --- a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
> +++ b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
> @@ -2293,20 +2293,15 @@ 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),
> +		PMD_INIT_LOG(DEBUG, "Port %u: Link Up - speed %u Mbps - %s",
> +					dev->data->port_id,
>   					(unsigned)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, "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);
> +		PMD_INIT_LOG(DEBUG, "Port %d: Link Down",
> +				dev->data->port_id);
> +	}
>   }

Well, I would say that PMD_INIT_LOG should not be called from here.
PMD_DRV_LOG should be the preferred macro (pure debug stuff, out of init).


The rest looks fine to me.


-- 
David Marchand

[PATCH 2/2] e1000: silence noisy log messages

[Stephen Hemminger]

Same as ixgbe. Should not be as chatty in log and should be
possible to compile away all the debug messages.

Signed-off-by: Stephen Hemminger <stephen-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ@public.gmane.org>

--- a/lib/librte_pmd_e1000/e1000_logs.h
+++ b/lib/librte_pmd_e1000/e1000_logs.h
@@ -35,8 +35,7 @@
 #define _E1000_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_E1000_DEBUG_INIT
 #define PMD_INIT_FUNC_TRACE() PMD_INIT_LOG(DEBUG, " >>")
--- a/lib/librte_pmd_e1000/em_ethdev.c
+++ b/lib/librte_pmd_e1000/em_ethdev.c
@@ -272,7 +272,7 @@
 	/* initialize the vfta */
 	memset(shadow_vfta, 0, sizeof(*shadow_vfta));

-	PMD_INIT_LOG(INFO, "port_id %d vendorID=0x%x deviceID=0x%x",
+	PMD_INIT_LOG(DEBUG, "port_id %d vendorID=0x%x deviceID=0x%x",
 		     eth_dev->data->port_id, pci_dev->id.vendor_id,
 		     pci_dev->id.device_id);

@@ -1295,16 +1295,14 @@
 	memset(&link, 0, sizeof(link));
 	rte_em_dev_atomic_read_link_status(dev, &link);
 	if (link.link_status) {
-		PMD_INIT_LOG(INFO, " Port %d: Link Up - speed %u Mbps - %s",
+		PMD_INIT_LOG(DEBUG, " Port %d: Link Up - speed %u Mbps - %s",
 			     dev->data->port_id, (unsigned)link.link_speed,
 			     link.link_duplex == ETH_LINK_FULL_DUPLEX ?
 			     "full-duplex" : "half-duplex");
 	} else {
-		PMD_INIT_LOG(INFO, " Port %d: Link Down", dev->data->port_id);
+		PMD_INIT_LOG(DEBUG, " Port %d: 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);
+
 	tctl = E1000_READ_REG(hw, E1000_TCTL);
 	rctl = E1000_READ_REG(hw, E1000_RCTL);
 	if (link.link_status) {
--- a/lib/librte_pmd_e1000/igb_ethdev.c
+++ b/lib/librte_pmd_e1000/igb_ethdev.c
@@ -586,7 +586,7 @@
 	E1000_WRITE_REG(hw, E1000_CTRL_EXT, ctrl_ext);
 	E1000_WRITE_FLUSH(hw);

-	PMD_INIT_LOG(INFO, "port_id %d vendorID=0x%x deviceID=0x%x",
+	PMD_INIT_LOG(DEBUG, "port_id %d vendorID=0x%x deviceID=0x%x",
 		     eth_dev->data->port_id, pci_dev->id.vendor_id,
 		     pci_dev->id.device_id);

@@ -1873,21 +1873,17 @@
 		memset(&link, 0, sizeof(link));
 		rte_igb_dev_atomic_read_link_status(dev, &link);
 		if (link.link_status) {
-			PMD_INIT_LOG(INFO,
+			PMD_INIT_LOG(DEBUG,
 				     " Port %d: Link Up - speed %u Mbps - %s",
 				     dev->data->port_id,
 				     (unsigned)link.link_speed,
 				     link.link_duplex == ETH_LINK_FULL_DUPLEX ?
 				     "full-duplex" : "half-duplex");
 		} else {
-			PMD_INIT_LOG(INFO, " Port %d: Link Down",
+			PMD_INIT_LOG(DEBUG, " Port %d: 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);
+
 		tctl = E1000_READ_REG(hw, E1000_TCTL);
 		rctl = E1000_READ_REG(hw, E1000_RCTL);
 		if (link.link_status) {
---
 lib/librte_pmd_e1000/e1000_logs.h |  3 +--
 lib/librte_pmd_e1000/em_ethdev.c  | 10 ++++------
 lib/librte_pmd_e1000/igb_ethdev.c | 12 ++++--------
 3 files changed, 9 insertions(+), 16 deletions(-)

diff --git a/lib/librte_pmd_e1000/e1000_logs.h b/lib/librte_pmd_e1000/e1000_logs.h
index 4a92804..81e7bf5 100644
--- a/lib/librte_pmd_e1000/e1000_logs.h
+++ b/lib/librte_pmd_e1000/e1000_logs.h
@@ -35,8 +35,7 @@
 #define _E1000_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_E1000_DEBUG_INIT
 #define PMD_INIT_FUNC_TRACE() PMD_INIT_LOG(DEBUG, " >>")
diff --git a/lib/librte_pmd_e1000/em_ethdev.c b/lib/librte_pmd_e1000/em_ethdev.c
index 3f2897e..a865d0a 100644
--- a/lib/librte_pmd_e1000/em_ethdev.c
+++ b/lib/librte_pmd_e1000/em_ethdev.c
@@ -272,7 +272,7 @@ eth_em_dev_init(__attribute__((unused)) struct eth_driver *eth_drv,
 	/* initialize the vfta */
 	memset(shadow_vfta, 0, sizeof(*shadow_vfta));
 
-	PMD_INIT_LOG(INFO, "port_id %d vendorID=0x%x deviceID=0x%x",
+	PMD_INIT_LOG(DEBUG, "port_id %d vendorID=0x%x deviceID=0x%x",
 		     eth_dev->data->port_id, pci_dev->id.vendor_id,
 		     pci_dev->id.device_id);
 
@@ -1295,16 +1295,14 @@ eth_em_interrupt_action(struct rte_eth_dev *dev)
 	memset(&link, 0, sizeof(link));
 	rte_em_dev_atomic_read_link_status(dev, &link);
 	if (link.link_status) {
-		PMD_INIT_LOG(INFO, " Port %d: Link Up - speed %u Mbps - %s",
+		PMD_INIT_LOG(DEBUG, " Port %d: Link Up - speed %u Mbps - %s",
 			     dev->data->port_id, (unsigned)link.link_speed,
 			     link.link_duplex == ETH_LINK_FULL_DUPLEX ?
 			     "full-duplex" : "half-duplex");
 	} else {
-		PMD_INIT_LOG(INFO, " Port %d: Link Down", dev->data->port_id);
+		PMD_INIT_LOG(DEBUG, " Port %d: 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);
+
 	tctl = E1000_READ_REG(hw, E1000_TCTL);
 	rctl = E1000_READ_REG(hw, E1000_RCTL);
 	if (link.link_status) {
diff --git a/lib/librte_pmd_e1000/igb_ethdev.c b/lib/librte_pmd_e1000/igb_ethdev.c
index 504ae74..c0068e3 100644
--- a/lib/librte_pmd_e1000/igb_ethdev.c
+++ b/lib/librte_pmd_e1000/igb_ethdev.c
@@ -582,7 +582,7 @@ eth_igb_dev_init(__attribute__((unused)) struct eth_driver *eth_drv,
 	E1000_WRITE_REG(hw, E1000_CTRL_EXT, ctrl_ext);
 	E1000_WRITE_FLUSH(hw);
 
-	PMD_INIT_LOG(INFO, "port_id %d vendorID=0x%x deviceID=0x%x",
+	PMD_INIT_LOG(DEBUG, "port_id %d vendorID=0x%x deviceID=0x%x",
 		     eth_dev->data->port_id, pci_dev->id.vendor_id,
 		     pci_dev->id.device_id);
 
@@ -1906,21 +1906,17 @@ eth_igb_interrupt_action(struct rte_eth_dev *dev)
 		memset(&link, 0, sizeof(link));
 		rte_igb_dev_atomic_read_link_status(dev, &link);
 		if (link.link_status) {
-			PMD_INIT_LOG(INFO,
+			PMD_INIT_LOG(DEBUG,
 				     " Port %d: Link Up - speed %u Mbps - %s",
 				     dev->data->port_id,
 				     (unsigned)link.link_speed,
 				     link.link_duplex == ETH_LINK_FULL_DUPLEX ?
 				     "full-duplex" : "half-duplex");
 		} else {
-			PMD_INIT_LOG(INFO, " Port %d: Link Down",
+			PMD_INIT_LOG(DEBUG, " Port %d: 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);
+
 		tctl = E1000_READ_REG(hw, E1000_TCTL);
 		rctl = E1000_READ_REG(hw, E1000_RCTL);
 		if (link.link_status) {
-- 
2.1.4

Re: [PATCH 2/2] e1000: silence noisy log messages

[David Marchand]

On 03/09/2015 08:18 PM, Stephen Hemminger wrote:
> Same as ixgbe. Should not be as chatty in log and should be
> possible to compile away all the debug messages.
>
> Signed-off-by: Stephen Hemminger <stephen-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ@public.gmane.org>


And same as ixgbe patch.

Patch is broken.
Macro in interrupt handler can be changed to PMD_DRV_LOG.


-- 
David Marchand

Re: [PATCH 0/2] Intel driver silence excess logging

[David Marchand]

On 03/09/2015 08:18 PM, Stephen Hemminger wrote:
> The Intel drivers seem to be getting more verbose in the log with
> each release. This is not a good thing.
>
> Stephen Hemminger (2):
>    ixgbe: silence noisy log messages
>    e1000: silence noisy log messages
>
>   lib/librte_pmd_e1000/e1000_logs.h   |  3 +--
>   lib/librte_pmd_e1000/em_ethdev.c    | 10 ++++------
>   lib/librte_pmd_e1000/igb_ethdev.c   | 12 ++++--------
>   lib/librte_pmd_ixgbe/ixgbe_ethdev.c | 15 +++++----------
>   lib/librte_pmd_ixgbe/ixgbe_logs.h   |  3 +--
>   lib/librte_pmd_ixgbe/ixgbe_rxtx.c   | 12 ++++++------
>   6 files changed, 21 insertions(+), 34 deletions(-)
>

These changes could be applied to other pmds for consistency, not only 
e1000 and ixgbe.


-- 
David Marchand

[PATCH 0/2] fix logging in Intel PMD's

[Stephen Hemminger]

The Intel Poll Mode Drivers generate a lot of log messages and
they are mostly at the INFO logging level. This should be changed
to the common syslog conventions, ie. put unimportant stuff for
production users at DEBUG level and things of interest but not
errors at NOTICE level.

Also should use RTE_LOG not rte_log so that messages can be
compiled out.

Stephen Hemminger (2):
  ixgbe: silence noisy log messages
  e1000: silence noisy log messages

 lib/librte_pmd_e1000/e1000_logs.h   |  3 +--
 lib/librte_pmd_e1000/em_ethdev.c    | 10 ++++-----
 lib/librte_pmd_e1000/igb_ethdev.c   | 16 ++++++---------
 lib/librte_pmd_ixgbe/ixgbe_ethdev.c | 41 ++++++++++++++++---------------------
 lib/librte_pmd_ixgbe/ixgbe_fdir.c   |  2 +-
 lib/librte_pmd_ixgbe/ixgbe_logs.h   |  3 +--
 lib/librte_pmd_ixgbe/ixgbe_rxtx.c   | 14 ++++++-------
 7 files changed, 38 insertions(+), 51 deletions(-)

-- 
2.1.4

[PATCH 2/2] e1000: silence noisy log messages

[Stephen Hemminger]

Same as ixgbe. Should not be as chatty in log and should be
possible to compile away all the debug messages.

Signed-off-by: Stephen Hemminger <stephen-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ@public.gmane.org>
---
 lib/librte_pmd_e1000/e1000_logs.h |  3 +--
 lib/librte_pmd_e1000/em_ethdev.c  | 10 ++++------
 lib/librte_pmd_e1000/igb_ethdev.c | 16 ++++++----------
 3 files changed, 11 insertions(+), 18 deletions(-)

diff --git a/lib/librte_pmd_e1000/e1000_logs.h b/lib/librte_pmd_e1000/e1000_logs.h
index 4a92804..81e7bf5 100644
--- a/lib/librte_pmd_e1000/e1000_logs.h
+++ b/lib/librte_pmd_e1000/e1000_logs.h
@@ -35,8 +35,7 @@
 #define _E1000_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_E1000_DEBUG_INIT
 #define PMD_INIT_FUNC_TRACE() PMD_INIT_LOG(DEBUG, " >>")
diff --git a/lib/librte_pmd_e1000/em_ethdev.c b/lib/librte_pmd_e1000/em_ethdev.c
index 76f45c9..9189134 100644
--- a/lib/librte_pmd_e1000/em_ethdev.c
+++ b/lib/librte_pmd_e1000/em_ethdev.c
@@ -270,7 +270,7 @@ eth_em_dev_init(struct rte_eth_dev *eth_dev)
 	/* initialize the vfta */
 	memset(shadow_vfta, 0, sizeof(*shadow_vfta));
 
-	PMD_INIT_LOG(INFO, "port_id %d vendorID=0x%x deviceID=0x%x",
+	PMD_INIT_LOG(DEBUG, "port_id %d vendorID=0x%x deviceID=0x%x",
 		     eth_dev->data->port_id, pci_dev->id.vendor_id,
 		     pci_dev->id.device_id);
 
@@ -1293,16 +1293,14 @@ eth_em_interrupt_action(struct rte_eth_dev *dev)
 	memset(&link, 0, sizeof(link));
 	rte_em_dev_atomic_read_link_status(dev, &link);
 	if (link.link_status) {
-		PMD_INIT_LOG(INFO, " Port %d: Link Up - speed %u Mbps - %s",
+		PMD_INIT_LOG(DEBUG, " Port %d: Link Up - speed %u Mbps - %s",
 			     dev->data->port_id, (unsigned)link.link_speed,
 			     link.link_duplex == ETH_LINK_FULL_DUPLEX ?
 			     "full-duplex" : "half-duplex");
 	} else {
-		PMD_INIT_LOG(INFO, " Port %d: Link Down", dev->data->port_id);
+		PMD_INIT_LOG(DEBUG, " Port %d: 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);
+
 	tctl = E1000_READ_REG(hw, E1000_TCTL);
 	rctl = E1000_READ_REG(hw, E1000_RCTL);
 	if (link.link_status) {
diff --git a/lib/librte_pmd_e1000/igb_ethdev.c b/lib/librte_pmd_e1000/igb_ethdev.c
index b3892a5..7d00aed 100644
--- a/lib/librte_pmd_e1000/igb_ethdev.c
+++ b/lib/librte_pmd_e1000/igb_ethdev.c
@@ -580,7 +580,7 @@ eth_igb_dev_init(struct rte_eth_dev *eth_dev)
 	E1000_WRITE_REG(hw, E1000_CTRL_EXT, ctrl_ext);
 	E1000_WRITE_FLUSH(hw);
 
-	PMD_INIT_LOG(INFO, "port_id %d vendorID=0x%x deviceID=0x%x",
+	PMD_INIT_LOG(DEBUG, "port_id %d vendorID=0x%x deviceID=0x%x",
 		     eth_dev->data->port_id, pci_dev->id.vendor_id,
 		     pci_dev->id.device_id);
 
@@ -1906,21 +1906,17 @@ eth_igb_interrupt_action(struct rte_eth_dev *dev)
 		memset(&link, 0, sizeof(link));
 		rte_igb_dev_atomic_read_link_status(dev, &link);
 		if (link.link_status) {
-			PMD_INIT_LOG(INFO,
+			PMD_INIT_LOG(DEBUG,
 				     " Port %d: Link Up - speed %u Mbps - %s",
 				     dev->data->port_id,
 				     (unsigned)link.link_speed,
 				     link.link_duplex == ETH_LINK_FULL_DUPLEX ?
 				     "full-duplex" : "half-duplex");
 		} else {
-			PMD_INIT_LOG(INFO, " Port %d: Link Down",
+			PMD_INIT_LOG(DEBUG, " Port %d: 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);
+
 		tctl = E1000_READ_REG(hw, E1000_TCTL);
 		rctl = E1000_READ_REG(hw, E1000_RCTL);
 		if (link.link_status) {
@@ -2204,12 +2200,12 @@ igbvf_dev_configure(struct rte_eth_dev *dev)
 	 */
 #ifndef RTE_LIBRTE_E1000_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