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

Zhang, Helin helin.zhang at intel.com
Tue Apr 14 10:44:20 CEST 2015



> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen at networkplumber.org]
> Sent: Friday, April 10, 2015 11:18 PM
> To: Zhang, Helin; Ananyev, Konstantin
> Cc: dev at dpdk.org; Stephen Hemminger
> Subject: [PATCH 1/2] ixgbe: silence noisy log messages
> 
> 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 at networkplumber.org>
> ---
>  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 ++++++-------
>  4 files changed, 27 insertions(+), 33 deletions(-)
> 
> diff --git a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
> b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
> index 5caee22..adc0fb9 100644
> --- a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
> +++ b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
> @@ -569,8 +569,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(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);
> 
>  	n = (uint8_t)(queue_id / NB_QMAP_FIELDS_PER_QSM_REG); @@ -595,20
> +595,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 %d, %s queue_id %d to stat index %d",
> -		     (int)(eth_dev->data->port_id), is_rx ? "RX" : "TX",
> +	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]);
>  	}
> @@ -752,7 +752,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.");
>  		}
> 
> @@ -1249,7 +1249,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(NOTICE, "82598EB not support queue level hw
> strip");
>  		return;
>  	}
>  	else {
> @@ -1273,7 +1273,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(NOTICE, "82598EB not support queue level hw
> strip");
>  		return;
>  	}
>  	else {
> @@ -2265,7 +2265,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) {
> @@ -2297,20 +2297,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,
Forced conversion of (int) here was to fix a compile error on some specific gcc/icc version, if I am not wrong.

>  					(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);
> +	}
>  }
As suggested by Thomas before, I think you need to put the rework of logs into another
separate patch, but not combined together with the replacing INFO with DEBUG.
This is common for all of this patch set.

> 
>  /*
> @@ -2947,12 +2942,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
> diff --git a/lib/librte_pmd_ixgbe/ixgbe_fdir.c b/lib/librte_pmd_ixgbe/ixgbe_fdir.c
> index afc53cb..df6aaee 100644
> --- a/lib/librte_pmd_ixgbe/ixgbe_fdir.c
> +++ b/lib/librte_pmd_ixgbe/ixgbe_fdir.c
> @@ -384,7 +384,7 @@ ixgbe_set_fdir_flex_conf(struct rte_eth_dev *dev,
>  	fdirm = IXGBE_READ_REG(hw, IXGBE_FDIRM);
> 
>  	if (conf == NULL) {
> -		PMD_DRV_LOG(INFO, "NULL pointer.");
> +		PMD_DRV_LOG(ERR, "NULL pointer.");
>  		return -EINVAL;
>  	}
> 
> 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 9da2c7e..34e3b9a 100644
> --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> @@ -1781,23 +1781,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); @@
> -3549,8 +3549,8 @@ void ixgbe_set_rx_function(struct rte_eth_dev *dev)
>  	 *    - Single buffer allocation (the simplest one)
>  	 */
>  	} else if (hw->rx_vec_allowed) {
> -		PMD_INIT_LOG(INFO, "Vector rx enabled, please make sure RX "
> -				   "burst size no less than 32.");
> +		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;
>  	} else if (hw->rx_bulk_alloc_allowed) {
> --
> 2.1.4

Regards,
Helin


More information about the dev mailing list