From: Stephen Hemminger <shemming@brocade.com> These patches were sent earlier, updated to current tree. They make Intel drivers not spam the log with information messages that cause questions in production. Unfortunately, developers seem to get attached to log messages which are not appropriate in a production product Stephen Hemminger (6): ixgbe: convert debug messages to DEBUG level ixgbe: raise priority of significant log events ixgbe: allow pruning log during build e1000: allow pruning log during build e1000: change log level of debug messages e1000: raise log level of signifcant events drivers/net/e1000/e1000_logs.h | 3 +-- drivers/net/e1000/em_ethdev.c | 5 +++-- drivers/net/e1000/igb_ethdev.c | 9 +++++---- drivers/net/ixgbe/ixgbe_ethdev.c | 26 +++++++++++++------------- drivers/net/ixgbe/ixgbe_fdir.c | 2 +- drivers/net/ixgbe/ixgbe_logs.h | 3 +-- drivers/net/ixgbe/ixgbe_rxtx.c | 22 +++++++++++----------- 7 files changed, 35 insertions(+), 35 deletions(-) -- 2.1.4
From: Stephen Hemminger <shemming@brocade.com> All the debug chatter messages in the system log causes complaints from users. Change the INFO messages to DEBUG for normal startup kind of stuff. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- drivers/net/ixgbe/ixgbe_ethdev.c | 14 +++++++------- drivers/net/ixgbe/ixgbe_rxtx.c | 22 +++++++++++----------- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c index b7a60b1..0ff9583 100644 --- a/drivers/net/ixgbe/ixgbe_ethdev.c +++ b/drivers/net/ixgbe/ixgbe_ethdev.c @@ -576,7 +576,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 %d, %s queue_id %d to stat index %d", + PMD_INIT_LOG(DEBUG, "Setting port %d, %s queue_id %d to stat index %d", (int)(eth_dev->data->port_id), is_rx ? "RX" : "TX", queue_id, stat_idx); @@ -602,20 +602,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", + PMD_INIT_LOG(DEBUG, "Set port %d, %s queue_id %d to stat index %d", (int)(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]); } @@ -2297,7 +2297,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) { @@ -2338,7 +2338,7 @@ ixgbe_dev_link_status_print(struct rte_eth_dev *dev) PMD_INIT_LOG(INFO, " Port %d: Link Down", (int)(dev->data->port_id)); } - PMD_INIT_LOG(INFO, "PCI Address: %04d:%02d:%02d:%d", + PMD_INIT_LOG(DEBUG, "PCI Address: %04d:%02d:%02d:%d", dev->pci_dev->addr.domain, dev->pci_dev->addr.bus, dev->pci_dev->addr.devid, diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c index b1db57f..ecd2184 100644 --- a/drivers/net/ixgbe/ixgbe_rxtx.c +++ b/drivers/net/ixgbe/ixgbe_rxtx.c @@ -1,4 +1,4 @@ -/*- + /*- * BSD LICENSE * * Copyright(c) 2010-2015 Intel Corporation. All rights reserved. @@ -1874,23 +1874,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); @@ -3766,11 +3766,11 @@ 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; } @@ -3786,7 +3786,7 @@ 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; @@ -3808,7 +3808,7 @@ 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; @@ -3966,7 +3966,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
From: Stephen Hemminger <shemming@brocade.com> Customers often screen off info level messages, so raise log level of significant events Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- drivers/net/ixgbe/ixgbe_ethdev.c | 12 ++++++------ drivers/net/ixgbe/ixgbe_fdir.c | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c index 0ff9583..ba98dd7 100644 --- a/drivers/net/ixgbe/ixgbe_ethdev.c +++ b/drivers/net/ixgbe/ixgbe_ethdev.c @@ -759,8 +759,8 @@ 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. " - "Using default TX function."); + PMD_INIT_LOG(NOTICE, "No TX queues configured yet. " + "Using default TX function."); } ixgbe_set_rx_function(eth_dev); @@ -1256,7 +1256,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 { @@ -1280,7 +1280,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 { @@ -2981,12 +2981,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/drivers/net/ixgbe/ixgbe_fdir.c b/drivers/net/ixgbe/ixgbe_fdir.c index d294f85..5c8b833 100644 --- a/drivers/net/ixgbe/ixgbe_fdir.c +++ b/drivers/net/ixgbe/ixgbe_fdir.c @@ -377,7 +377,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; } -- 2.1.4
From: Stephen Hemminger <shemming@brocade.com> The ixgbe driver was not following DPDK convention and was leaving loggin always in even if LOG_LEVEL was configured to disable debug logs. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- drivers/net/ixgbe/ixgbe_logs.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/net/ixgbe/ixgbe_logs.h b/drivers/net/ixgbe/ixgbe_logs.h index 572e030..53ba42d 100644 --- a/drivers/net/ixgbe/ixgbe_logs.h +++ b/drivers/net/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
From: Stephen Hemminger <shemming@brocade.com> Similar to ixgbe, allow log messages to be removed by using RTE_LOG_LEVEL during the build process. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- drivers/net/e1000/e1000_logs.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/net/e1000/e1000_logs.h b/drivers/net/e1000/e1000_logs.h index 4a92804..81e7bf5 100644 --- a/drivers/net/e1000/e1000_logs.h +++ b/drivers/net/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, " >>") -- 2.1.4
From: Stephen Hemminger <shemming@brocade.com> Any debug messages about hardware should be under debug (or removed) and reduce customer visible log spam. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- drivers/net/e1000/em_ethdev.c | 5 +++-- drivers/net/e1000/igb_ethdev.c | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c index a306c55..29053a1 100644 --- a/drivers/net/e1000/em_ethdev.c +++ b/drivers/net/e1000/em_ethdev.c @@ -275,7 +275,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); @@ -1305,9 +1305,10 @@ eth_em_interrupt_action(struct rte_eth_dev *dev) } else { PMD_INIT_LOG(INFO, " Port %d: Link Down", dev->data->port_id); } - PMD_INIT_LOG(INFO, "PCI Address: %04d:%02d:%02d:%d", + PMD_INIT_LOG(DEBUG, "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/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c index 11e8ab8..cc8bfae 100644 --- a/drivers/net/e1000/igb_ethdev.c +++ b/drivers/net/e1000/igb_ethdev.c @@ -588,7 +588,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); @@ -1925,7 +1925,8 @@ eth_igb_interrupt_action(struct rte_eth_dev *dev) PMD_INIT_LOG(INFO, " Port %d: Link Down", dev->data->port_id); } - PMD_INIT_LOG(INFO, "PCI Address: %04d:%02d:%02d:%d", + + PMD_INIT_LOG(DEBUG, "PCI Address: %04d:%02d:%02d:%d", dev->pci_dev->addr.domain, dev->pci_dev->addr.bus, dev->pci_dev->addr.devid, -- 2.1.4
From: Stephen Hemminger <shemming@brocade.com> Any message about incorrect API usage should be at NOTICE level or above. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- drivers/net/e1000/igb_ethdev.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c index cc8bfae..472a1a4 100644 --- a/drivers/net/e1000/igb_ethdev.c +++ b/drivers/net/e1000/igb_ethdev.c @@ -2214,12 +2214,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
2015-07-09 16:01, Stephen Hemminger: > From: Stephen Hemminger <shemming@brocade.com> > > Customers often screen off info level messages, so raise log > level of significant events > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> It was acked by Bruce with a (not addressed) comment: http://dpdk.org/ml/archives/dev/2015-May/017785.html
2015-07-09 16:01, Stephen Hemminger: > From: Stephen Hemminger <shemming@brocade.com> > > The ixgbe driver was not following DPDK convention and > was leaving loggin always in even if LOG_LEVEL was configured > to disable debug logs. > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> It was acked by Bruce: http://dpdk.org/ml/archives/dev/2015-May/017786.html
On Fri, 10 Jul 2015 01:27:25 +0200 Thomas Monjalon <thomas.monjalon@6wind.com> wrote: > 2015-07-09 16:01, Stephen Hemminger: > > From: Stephen Hemminger <shemming@brocade.com> > > > > Customers often screen off info level messages, so raise log > > level of significant events > > > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> > > It was acked by Bruce with a (not addressed) comment: > http://dpdk.org/ml/archives/dev/2015-May/017785.html > His comment was: > 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). I prefer not to change the text of the messages in the same patch set.
2015-07-09 16:01, Stephen Hemminger:
> From: Stephen Hemminger <shemming@brocade.com>
>
> The ixgbe driver was not following DPDK convention and
> was leaving loggin always in even if LOG_LEVEL was configured
> to disable debug logs.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
This series is fixing e1000 and ixgbe.
There is the same issue with i40e, fm10k and bnx2x.
I will fix them in the same way.
For consistency, examples/l3fwd-power and eal_common_tailqs.c should
use RTE_LOG instead of rte_log.
2015-07-09 16:01, Stephen Hemminger:
> From: Stephen Hemminger <shemming@brocade.com>
>
> These patches were sent earlier, updated to current tree.
>
> They make Intel drivers not spam the log with information
> messages that cause questions in production.
>
> Unfortunately, developers seem to get attached to log messages
> which are not appropriate in a production product
>
> Stephen Hemminger (6):
> ixgbe: convert debug messages to DEBUG level
> ixgbe: raise priority of significant log events
> ixgbe: allow pruning log during build
> e1000: allow pruning log during build
> e1000: change log level of debug messages
> e1000: raise log level of signifcant events
Applied, thanks