[dpdk-dev] [PATCH v2 0/5] ixgbe: logging patches

[dpdk-dev] [PATCH v2 0/5] ixgbe: logging patches

[Stephen Hemminger]

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

[dpdk-dev] [PATCH 1/5] ixgbe: remove unnecessary casts

[Stephen Hemminger]

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

Re: [dpdk-dev] [PATCH 1/5] ixgbe: remove unnecessary casts

[Bruce Richardson]

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
> 

[dpdk-dev] [PATCH 2/5] ixgbe: don't print PCI address on link change

[Stephen Hemminger]

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

Re: [dpdk-dev] [PATCH 2/5] ixgbe: don't print PCI address on link change

[Bruce Richardson]

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
> 

[dpdk-dev] [PATCH 3/5] ixgbe: raise priority of significant events

[Stephen Hemminger]

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

Re: [dpdk-dev] [PATCH 3/5] ixgbe: raise priority of significant events

[Bruce Richardson]

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
> 

[dpdk-dev] [PATCH 4/5] ixgbe: use RTE_LOG not rte_log

[Stephen Hemminger]

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

Re: [dpdk-dev] [PATCH 4/5] ixgbe: use RTE_LOG not rte_log

[Bruce Richardson]

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
> 

[dpdk-dev] [PATCH 5/5] 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. 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

Re: [dpdk-dev] [PATCH 5/5] ixgbe: silence noisy log messages

[Bruce Richardson]

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
> 

Re: [dpdk-dev] [PATCH 5/5] ixgbe: silence noisy log messages

[Stephen Hemminger]

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)

Re: [dpdk-dev] [PATCH 5/5] ixgbe: silence noisy log messages

[David Marchand]

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

Re: [dpdk-dev] [PATCH 5/5] ixgbe: silence noisy log messages

[Stephen Hemminger]

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
>
>