net/nfp logging fixes
cover net/nfp logging fixes
These are several small changes to make the Netronome driver use logging macros in the same way as other drivers.
Compile tested only. I don't have Netronome hardware.
Stephen Hemminger (5):
net/nfp: use correct logtype for init messages
net/nfp: add implied new line to PMD_DRV_LOG
net/nfp: fix double space in init log
net/nfl: add newline in PMD_RX/TX_LOG macros
net/nfp: use dynamic logging everywhere
drivers/net/nfp/nfp_net.c | 186 ++++++++++++++++----------------- drivers/net/nfp/nfp_net_logs.h | 9 +- 2 files changed, 98 insertions(+), 97 deletions(-)
Signature · 1 line
2.17.0
PATCH 1/5 net/nfp: use correct logtype for init messages
The NFP driver init messages would come out under PMD not net.pmd.nfp.init.
--- drivers/net/nfp/nfp_net_logs.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/net/nfp/nfp_net_logs.h +2 -1
@@ -36,7 +36,8 @@extern int nfp_logtype_init; #define PMD_INIT_LOG(level, fmt, args...) \ - RTE_LOG(level, PMD, "%s(): " fmt "\n", __func__, ## args) + rte_log(RTE_LOG_ ## level, nfp_logtype_init, \ + "%s(): " fmt "\n", __func__, ## args)#define PMD_INIT_FUNC_TRACE () PMD_INIT_LOG(DEBUG, " >>")#ifdef RTE_LIBRTE_NFP_NET_DEBUG_RX
Signature · 1 line
2.17.0
PATCH 2/5 net/nfp: add implied new line to PMD_DRV_LOG
In reply to Stephen HemmingerThe PMD_INIT_LOG macro always adds a newline, and other drivers version of PMD_DRV_LOG always adds a newline. Therefore change nfp driver to be consitent with others.
--- drivers/net/nfp/nfp_net.c | 22 +++++++++++----------- drivers/net/nfp/nfp_net_logs.h | 2 +- 2 files changed, 12 insertions(+), 12 deletions(-)
drivers/net/nfp/nfp_net.c +11 -11
@@ -294,7 +294,7 @@ __nfp_net_reconfig(struct nfp_net_hw *hw, uint32_t update)uint32_t new;struct timespec wait; -PMD_DRV_LOG (DEBUG ,"Writing to the configuration queue (%p)...\n" , +PMD_DRV_LOG (DEBUG ,"Writing to the configuration queue (%p)..." , hw->qcp_cfg);if (hw->qcp_cfg ==NULL )
@@ -305,7 +305,7 @@ __nfp_net_reconfig(struct nfp_net_hw *hw, uint32_t update) wait.tv_sec =0 ; wait.tv_nsec =1000000 ; -PMD_DRV_LOG (DEBUG ,"Polling for update ack...\n" ); +PMD_DRV_LOG (DEBUG ,"Polling for update ack..." );/* Poll update field, waiting for NFP to ack the config */ for (cnt =0 ; ; cnt++) {
@@ -323,7 +323,7 @@ __nfp_net_reconfig(struct nfp_net_hw *hw, uint32_t update) }nanosleep (&wait,0 );/* waiting for a 1ms */ } -PMD_DRV_LOG (DEBUG ,"Ack DONE\n" ); +PMD_DRV_LOG (DEBUG ,"Ack DONE" );return 0 ; }
@@ -341,7 +341,7 @@ nfp_net_reconfig(struct nfp_net_hw *hw, uint32_t ctrl, uint32_t update) {uint32_t err; -PMD_DRV_LOG (DEBUG ,"nfp_net_reconfig: ctrl=%08x update=%08x\n" , +PMD_DRV_LOG (DEBUG ,"nfp_net_reconfig: ctrl=%08x update=%08x" , ctrl, update);rte_spinlock_lock (&hw->reconfig_lock);
@@ -983,7 +983,7 @@ nfp_net_promisc_enable(struct rte_eth_dev *dev)uint32_t new_ctrl, update =0 ;struct nfp_net_hw *hw; -PMD_DRV_LOG (DEBUG ,"Promiscuous mode enable\n" ); +PMD_DRV_LOG (DEBUG ,"Promiscuous mode enable" ); hw =NFP_NET_DEV_PRIVATE_TO_HW (dev->data->dev_private);
@@ -993,7 +993,7 @@ nfp_net_promisc_enable(struct rte_eth_dev *dev) }if (hw->ctrl &NFP_NET_CFG_CTRL_PROMISC ) { -PMD_DRV_LOG (INFO ,"Promiscuous mode already enabled\n" ); +PMD_DRV_LOG (INFO ,"Promiscuous mode already enabled" );return ; }
@@ -1019,7 +1019,7 @@ nfp_net_promisc_disable(struct rte_eth_dev *dev) hw =NFP_NET_DEV_PRIVATE_TO_HW (dev->data->dev_private);if ((hw->ctrl &NFP_NET_CFG_CTRL_PROMISC ) ==0 ) { -PMD_DRV_LOG (INFO ,"Promiscuous mode already disabled\n" ); +PMD_DRV_LOG (INFO ,"Promiscuous mode already disabled" );return ; }
@@ -1061,7 +1061,7 @@ nfp_net_link_update(struct rte_eth_dev *dev, __rte_unused int wait_to_complete) [NFP_NET_CFG_STS_LINK_RATE_100G ] =ETH_SPEED_NUM_100G , }; -PMD_DRV_LOG (DEBUG ,"Link update\n" ); +PMD_DRV_LOG (DEBUG ,"Link update" ); hw =NFP_NET_DEV_PRIVATE_TO_HW (dev->data->dev_private);
@@ -1085,9 +1085,9 @@ nfp_net_link_update(struct rte_eth_dev *dev, __rte_unused int wait_to_complete) ret =rte_eth_linkstatus_set (dev, &link);if (ret ==0 ) {if (link.link_status) -PMD_DRV_LOG (INFO ,"NIC Link is Up\n" ); +PMD_DRV_LOG (INFO ,"NIC Link is Up" );else -PMD_DRV_LOG (INFO ,"NIC Link is Down\n" ); +PMD_DRV_LOG (INFO ,"NIC Link is Down" ); }return ret; }
@@ -1472,7 +1472,7 @@ nfp_net_dev_interrupt_handler(void *param)struct rte_eth_link link;struct rte_eth_dev *dev = (struct rte_eth_dev *)param; -PMD_DRV_LOG (DEBUG ,"We got a LSC interrupt!!!\n" ); +PMD_DRV_LOG (DEBUG ,"We got a LSC interrupt!!!" );rte_eth_linkstatus_get (dev, &link);
drivers/net/nfp/nfp_net_logs.h +1 -1
@@ -59,6 +59,6 @@ extern int nfp_logtype_init;extern int nfp_logtype_driver; #define PMD_DRV_LOG(level, fmt, args...) \ rte_log(RTE_LOG_ ## level, nfp_logtype_driver, \ - "%s(): " fmt, __func__, ## args) + "%s(): " fmt "\n", __func__, ## args)#endif /* _NFP_NET_LOGS_H_ */
Signature · 1 line
2.17.0
PATCH 3/5 net/nfp: fix double space in init log
In reply to Stephen HemmingerShouldn't pass extra newline.
--- drivers/net/nfp/nfp_net.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/nfp/nfp_net.c +1 -1
@@ -718,7 +718,7 @@ nfp_configure_rx_interrupt(struct rte_eth_dev *dev,*/ nn_cfg_writeb (hw,NFP_NET_CFG_RXR_VEC (i), i +1 ); intr_handle->intr_vec[i] = i +1 ; -PMD_INIT_LOG (DEBUG ,"intr_vec[%d]= %d\n" , i, +PMD_INIT_LOG (DEBUG ,"intr_vec[%d]= %d" , i, intr_handle->intr_vec[i]); } }
Signature · 1 line
2.17.0
PATCH 4/5 net/nfl: add newline in PMD_RX/TX_LOG macros
In reply to Stephen HemmingerBe consistent with usage in other drivers.
No need for snowflake drivers.
--- drivers/net/nfp/nfp_net.c | 30 +++++++++++++++--------------- drivers/net/nfp/nfp_net_logs.h | 4 ++-- 2 files changed, 17 insertions(+), 17 deletions(-)
drivers/net/nfp/nfp_net.c +15 -15
@@ -1660,7 +1660,7 @@ nfp_net_rx_queue_setup(struct rte_eth_dev *dev,return -ENOMEM ; } -PMD_RX_LOG (DEBUG ,"rxbufs=%p hw_ring=%p dma_addr=0x%" PRIx64"\n" , +PMD_RX_LOG (DEBUG ,"rxbufs=%p hw_ring=%p dma_addr=0x%" PRIx64, rxq->rxbufs, rxq->rxds, (unsigned long int )rxq->dma);nfp_net_reset_rx_queue (rxq);
@@ -1685,7 +1685,7 @@ nfp_net_rx_fill_freelist(struct nfp_net_rxq *rxq)uint64_t dma_addr;unsigned i; -PMD_RX_LOG (DEBUG ,"nfp_net_rx_fill_freelist for %u descriptors\n" , +PMD_RX_LOG (DEBUG ,"nfp_net_rx_fill_freelist for %u descriptors" , rxq->rx_count);for (i =0 ; i < rxq->rx_count; i++) {
@@ -1705,14 +1705,14 @@ nfp_net_rx_fill_freelist(struct nfp_net_rxq *rxq) rxd->fld.dma_addr_hi = (dma_addr >>32 ) &0xff ; rxd->fld.dma_addr_lo = dma_addr &0xffffffff ; rxe[i].mbuf = mbuf; -PMD_RX_LOG (DEBUG ,"[%d]: %" PRIx64"\n" , i, dma_addr); +PMD_RX_LOG (DEBUG ,"[%d]: %" PRIx64, i, dma_addr); }/* Make sure all writes are flushed before telling the hardware */ rte_wmb ();/* Not advertising the whole ring as the firmware gets confused if so */ -PMD_RX_LOG (DEBUG ,"Increment FL write pointer in %u\n" , +PMD_RX_LOG (DEBUG ,"Increment FL write pointer in %u" , rxq->rx_count -1 );nfp_qcp_ptr_add (rxq->qcp_fl,NFP_QCP_WRITE_PTR , rxq->rx_count -1 );
@@ -1771,7 +1771,7 @@ nfp_net_tx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx,* calling nfp_net_stop */ if (dev->data->tx_queues[queue_idx]) { -PMD_TX_LOG (DEBUG ,"Freeing memory prior to re-allocation %d\n" , +PMD_TX_LOG (DEBUG ,"Freeing memory prior to re-allocation %d" , queue_idx);nfp_net_tx_queue_release (dev->data->tx_queues[queue_idx]); dev->data->tx_queues[queue_idx] =NULL ;
@@ -1825,7 +1825,7 @@ nfp_net_tx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx,nfp_net_tx_queue_release (txq);return -ENOMEM ; } -PMD_TX_LOG (DEBUG ,"txbufs=%p hw_ring=%p dma_addr=0x%" PRIx64"\n" , +PMD_TX_LOG (DEBUG ,"txbufs=%p hw_ring=%p dma_addr=0x%" PRIx64, txq->txbufs, txq->txds, (unsigned long int )txq->dma);nfp_net_reset_tx_queue (txq);
@@ -2120,7 +2120,7 @@ nfp_net_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts) mb = rxb->mbuf; rxb->mbuf = new_mb; -PMD_RX_LOG (DEBUG ,"Packet len: %u, mbuf_size: %u\n" , +PMD_RX_LOG (DEBUG ,"Packet len: %u, mbuf_size: %u" , rxds->rxd.data_len, rxq->mbuf_size);/* Size of this segment */
@@ -2191,7 +2191,7 @@ nfp_net_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)if (nb_hold ==0 )return nb_hold; -PMD_RX_LOG (DEBUG ,"RX port_id=%u queue_id=%u, %d packets received\n" , +PMD_RX_LOG (DEBUG ,"RX port_id=%u queue_id=%u, %d packets received" , rxq->port_id, (unsigned int )rxq->qidx, nb_hold); nb_hold += rxq->nb_rx_hold;
@@ -2202,7 +2202,7 @@ nfp_net_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)*/ rte_wmb ();if (nb_hold > rxq->rx_free_thresh) { -PMD_RX_LOG (DEBUG ,"port=%u queue=%u nb_hold=%u avail=%u\n" , +PMD_RX_LOG (DEBUG ,"port=%u queue=%u nb_hold=%u avail=%u" , rxq->port_id, (unsigned int )rxq->qidx, (unsigned )nb_hold, (unsigned )avail);nfp_qcp_ptr_add (rxq->qcp_fl,NFP_QCP_WRITE_PTR , nb_hold);
@@ -2226,14 +2226,14 @@ nfp_net_tx_free_bufs(struct nfp_net_txq *txq)int todo;PMD_TX_LOG (DEBUG ,"queue %u. Check for descriptor with a complete" -" status\n" , txq->qidx); +" status" , txq->qidx);/* Work out how many packets have been sent */ qcp_rd_p =nfp_qcp_read (txq->qcp_q,NFP_QCP_READ_PTR );if (qcp_rd_p == txq->rd_p) {PMD_TX_LOG (DEBUG ,"queue %u: It seems harrier is not sending " -"packets (%u, %u)\n" , txq->qidx, +"packets (%u, %u)" , txq->qidx, qcp_rd_p, txq->rd_p);return 0 ; }
@@ -2243,7 +2243,7 @@ nfp_net_tx_free_bufs(struct nfp_net_txq *txq)else todo = qcp_rd_p + txq->tx_count - txq->rd_p; -PMD_TX_LOG (DEBUG ,"qcp_rd_p %u, txq->rd_p: %u, qcp->rd_p: %u\n" , +PMD_TX_LOG (DEBUG ,"qcp_rd_p %u, txq->rd_p: %u, qcp->rd_p: %u" , qcp_rd_p, txq->rd_p, txq->rd_p);if (todo ==0 )
@@ -2297,7 +2297,7 @@ nfp_net_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts) hw = txq->hw; txds = &txq->txds[txq->wr_p]; -PMD_TX_LOG (DEBUG ,"working for queue %u at pos %d and %u packets\n" , +PMD_TX_LOG (DEBUG ,"working for queue %u at pos %d and %u packets" , txq->qidx, txq->wr_p, nb_pkts);if ((nfp_free_tx_desc (txq) < nb_pkts) || (nfp_net_txq_full (txq)))
@@ -2311,7 +2311,7 @@ nfp_net_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts) i =0 ; issued_descs =0 ; -PMD_TX_LOG (DEBUG ,"queue: %u. Sending %u packets\n" , +PMD_TX_LOG (DEBUG ,"queue: %u. Sending %u packets" , txq->qidx, nb_pkts);/* Sending packets */ while ((i < nb_pkts) && free_descs) {
@@ -2370,7 +2370,7 @@ nfp_net_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts) dma_size = pkt->data_len; dma_addr =rte_mbuf_data_iova (pkt);PMD_TX_LOG (DEBUG ,"Working with mbuf at dma address:" -"%" PRIx64"\n" , dma_addr); +"%" PRIx64"" , dma_addr);/* Filling descriptors fields */ txds->dma_len = dma_size;
drivers/net/nfp/nfp_net_logs.h +2 -2
@@ -42,14 +42,14 @@ extern int nfp_logtype_init;#ifdef RTE_LIBRTE_NFP_NET_DEBUG_RX #define PMD_RX_LOG(level, fmt, args...) \ - RTE_LOG(level, PMD, "%s() rx: " fmt, __func__, ## args) + RTE_LOG(level, PMD, "%s() rx: " fmt "\n", __func__, ## args)#else #define PMD_RX_LOG(level, fmt, args...) do { } while (0)#endif #ifdef RTE_LIBRTE_NFP_NET_DEBUG_TX #define PMD_TX_LOG(level, fmt, args...) \ - RTE_LOG(level, PMD, "%s() tx: " fmt, __func__, ## args) + RTE_LOG(level, PMD, "%s() tx: " fmt "\n", __func__, ## args)#define ASSERT (x) if (!(x)) rte_panic("NFP_NET: x")#else #define PMD_TX_LOG(level, fmt, args...) do { } while (0)
Signature · 1 line
2.17.0
PATCH 5/5 net/nfp: use dynamic logging everywhere
In reply to Stephen HemmingerDrivers should only log with their assigned logtype, not with the generic PMD log type.
--- drivers/net/nfp/nfp_net.c | 132 +++++++++++++++++++------------------- 1 file changed, 66 insertions(+), 66 deletions(-)
drivers/net/nfp/nfp_net.c +66 -66
@@ -1423,15 +1423,15 @@ nfp_net_dev_link_status_print(struct rte_eth_dev *dev)rte_eth_linkstatus_get (dev, &link);if (link.link_status) -RTE_LOG (INFO ,PMD ,"Port %d: Link Up - speed %u Mbps - %s\n" , - dev->data->port_id, link.link_speed, - link.link_duplex ==ETH_LINK_FULL_DUPLEX - ?"full-duplex" :"half-duplex" ); +PMD_DRV_LOG (INFO ,"Port %d: 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 -RTE_LOG (INFO ,PMD ," Port %d: Link Down\n" , - dev->data->port_id); +PMD_DRV_LOG (INFO ," Port %d: Link Down" , + dev->data->port_id); -RTE_LOG (INFO ,PMD ,"PCI Address: %04d:%02d:%02d:%d\n" , +PMD_DRV_LOG (INFO ,"PCI Address: %04d:%02d:%02d:%d" , pci_dev->addr.domain, pci_dev->addr.bus, pci_dev->addr.devid, pci_dev->addr.function); }
@@ -1491,7 +1491,7 @@ nfp_net_dev_interrupt_handler(void *param)if (rte_eal_alarm_set (timeout *1000 , nfp_net_dev_interrupt_delayed_handler, (void *)dev) <0 ) { -RTE_LOG (ERR ,PMD ,"Error setting alarm" ); +PMD_INIT_LOG (ERR ,"Error setting alarm" );/* Unmasking */ nfp_net_irq_unmask (dev); }
@@ -1578,7 +1578,7 @@ nfp_net_rx_queue_setup(struct rte_eth_dev *dev,if (((nb_desc *sizeof (struct nfp_net_rx_desc )) %128 ) !=0 || (nb_desc >NFP_NET_MAX_RX_DESC ) || (nb_desc <NFP_NET_MIN_RX_DESC )) { -RTE_LOG (ERR ,PMD ,"Wrong nb_desc value\n" ); +PMD_DRV_LOG (ERR ,"Wrong nb_desc value" );return -EINVAL ; }
@@ -1586,10 +1586,10 @@ nfp_net_rx_queue_setup(struct rte_eth_dev *dev, rxmode = &dev_conf->rxmode;if (rx_conf->offloads != rxmode->offloads) { -RTE_LOG (ERR ,PMD ,"queue %u rx offloads not as port offloads\n" , +PMD_DRV_LOG (ERR ,"queue %u rx offloads not as port offloads" , queue_idx); -RTE_LOG (ERR ,PMD ,"\tport: %" PRIx64"\n" , rxmode->offloads); -RTE_LOG (ERR ,PMD ,"\tqueue: %" PRIx64"\n" , rx_conf->offloads); +PMD_DRV_LOG (ERR ,"\tport: %" PRIx64"" , rxmode->offloads); +PMD_DRV_LOG (ERR ,"\tqueue: %" PRIx64"" , rx_conf->offloads);return -EINVAL ; }
@@ -1642,7 +1642,7 @@ nfp_net_rx_queue_setup(struct rte_eth_dev *dev, socket_id);if (tz ==NULL ) { -RTE_LOG (ERR ,PMD ,"Error allocatig rx dma\n" ); +PMD_DRV_LOG (ERR ,"Error allocatig rx dma" );nfp_net_rx_queue_release (rxq);return -ENOMEM ; }
@@ -1693,7 +1693,7 @@ nfp_net_rx_fill_freelist(struct nfp_net_rxq *rxq)struct rte_mbuf *mbuf =rte_pktmbuf_alloc (rxq->mem_pool);if (mbuf ==NULL ) { -RTE_LOG (ERR ,PMD ,"RX mbuf alloc failed queue_id=%u\n" , +PMD_DRV_LOG (ERR ,"RX mbuf alloc failed queue_id=%u" , (unsigned )rxq->qidx);return -ENOMEM ; }
@@ -1740,7 +1740,7 @@ nfp_net_tx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx,if (((nb_desc *sizeof (struct nfp_net_tx_desc )) %128 ) !=0 || (nb_desc >NFP_NET_MAX_TX_DESC ) || (nb_desc <NFP_NET_MIN_TX_DESC )) { -RTE_LOG (ERR ,PMD ,"Wrong nb_desc value\n" ); +PMD_DRV_LOG (ERR ,"Wrong nb_desc value" );return -EINVAL ; }
@@ -1748,7 +1748,7 @@ nfp_net_tx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx, txmode = &dev_conf->txmode;if (tx_conf->offloads != txmode->offloads) { -RTE_LOG (ERR ,PMD ,"queue %u tx offloads not as port offloads" , +PMD_DRV_LOG (ERR ,"queue %u tx offloads not as port offloads" , queue_idx);return -EINVAL ; }
@@ -1758,10 +1758,10 @@ nfp_net_tx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx, DEFAULT_TX_FREE_THRESH);if (tx_free_thresh > (nb_desc)) { -RTE_LOG (ERR ,PMD , +PMD_DRV_LOG (ERR ,"tx_free_thresh must be less than the number of TX " "descriptors. (tx_free_thresh=%u port=%d " -"queue=%d)\n" , (unsigned int )tx_free_thresh, +"queue=%d)" , (unsigned int )tx_free_thresh, dev->data->port_id, (int )queue_idx);return -(EINVAL ); }
@@ -1781,7 +1781,7 @@ nfp_net_tx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx, txq =rte_zmalloc_socket ("ethdev TX queue" ,sizeof (struct nfp_net_txq ),RTE_CACHE_LINE_SIZE , socket_id);if (txq ==NULL ) { -RTE_LOG (ERR ,PMD ,"Error allocating tx dma\n" ); +PMD_DRV_LOG (ERR ,"Error allocating tx dma" );return -ENOMEM ; }
@@ -1795,7 +1795,7 @@ nfp_net_tx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx,NFP_NET_MAX_TX_DESC ,NFP_MEMZONE_ALIGN , socket_id);if (tz ==NULL ) { -RTE_LOG (ERR ,PMD ,"Error allocating tx dma\n" ); +PMD_DRV_LOG (ERR ,"Error allocating tx dma" );nfp_net_tx_queue_release (txq);return -ENOMEM ; }
@@ -2420,7 +2420,7 @@ nfp_net_vlan_offload_set(struct rte_eth_dev *dev, int mask)if ((mask Ð_VLAN_FILTER_OFFLOAD ) || (mask Ð_VLAN_EXTEND_OFFLOAD )) -RTE_LOG (INFO ,PMD ,"No support for ETH_VLAN_FILTER_OFFLOAD or" +PMD_DRV_LOG (INFO ,"No support for ETH_VLAN_FILTER_OFFLOAD or" " ETH_VLAN_EXTEND_OFFLOAD" );/* Enable vlan strip if it is not configured yet */
@@ -2457,9 +2457,9 @@ nfp_net_rss_reta_write(struct rte_eth_dev *dev,NFP_NET_DEV_PRIVATE_TO_HW (dev->data->dev_private);if (reta_size !=NFP_NET_CFG_RSS_ITBL_SZ ) { -RTE_LOG (ERR ,PMD ,"The size of hash lookup table configured " +PMD_DRV_LOG (ERR ,"The size of hash lookup table configured " "(%d) doesn't match the number hardware can supported " -"(%d)\n" , reta_size,NFP_NET_CFG_RSS_ITBL_SZ ); +"(%d)" , reta_size,NFP_NET_CFG_RSS_ITBL_SZ );return -EINVAL ; }
@@ -2538,9 +2538,9 @@ nfp_net_reta_query(struct rte_eth_dev *dev,return -EINVAL ;if (reta_size !=NFP_NET_CFG_RSS_ITBL_SZ ) { -RTE_LOG (ERR ,PMD ,"The size of hash lookup table configured " +PMD_DRV_LOG (ERR ,"The size of hash lookup table configured " "(%d) doesn't match the number hardware can supported " -"(%d)\n" , reta_size,NFP_NET_CFG_RSS_ITBL_SZ ); +"(%d)" , reta_size,NFP_NET_CFG_RSS_ITBL_SZ );return -EINVAL ; }
@@ -2626,14 +2626,14 @@ nfp_net_rss_hash_update(struct rte_eth_dev *dev,/* Checking if RSS is enabled */ if (!(hw->ctrl &NFP_NET_CFG_CTRL_RSS )) {if (rss_hf !=0 ) {/* Enable RSS? */ -RTE_LOG (ERR ,PMD ,"RSS unsupported\n" ); +PMD_DRV_LOG (ERR ,"RSS unsupported" );return -EINVAL ; }return 0 ;/* Nothing to do */ }if (rss_conf->rss_key_len >NFP_NET_CFG_RSS_KEY_SZ ) { -RTE_LOG (ERR ,PMD ,"hash key too long\n" ); +PMD_DRV_LOG (ERR ,"hash key too long" );return -EINVAL ; }
@@ -2705,7 +2705,7 @@ nfp_net_rss_config_default(struct rte_eth_dev *dev)uint16_t queue;int i, j, ret; -RTE_LOG (INFO ,PMD ,"setting default RSS conf for %u queues\n" , +PMD_DRV_LOG (INFO ,"setting default RSS conf for %u queues" , rx_queues); nfp_reta_conf[0 ].mask = ~0x0 ;
@@ -2725,7 +2725,7 @@ nfp_net_rss_config_default(struct rte_eth_dev *dev) dev_conf = &dev->data->dev_conf;if (!dev_conf) { -RTE_LOG (INFO ,PMD ,"wrong rss conf" ); +PMD_DRV_LOG (INFO ,"wrong rss conf" );return -EINVAL ; } rss_conf = dev_conf->rx_adv_conf.rss_conf;
@@ -2816,11 +2816,11 @@ nfp_net_init(struct rte_eth_dev *eth_dev) (pci_dev->id.device_id == PCI_DEVICE_ID_NFP6000_PF_NIC)) { port =get_pf_port_number (eth_dev->data->name);if (port <0 || port >7 ) { -RTE_LOG (ERR ,PMD ,"Port value is wrong\n" ); +PMD_DRV_LOG (ERR ,"Port value is wrong" );return -ENODEV ; } -PMD_INIT_LOG (DEBUG ,"Working with PF port value %d\n" , port); +PMD_INIT_LOG (DEBUG ,"Working with PF port value %d" , port);/* This points to port 0 private data */ hwport0 =NFP_NET_DEV_PRIVATE_TO_HW (eth_dev->data->dev_private);
@@ -2854,8 +2854,8 @@ nfp_net_init(struct rte_eth_dev *eth_dev) hw->ctrl_bar = (uint8_t *)pci_dev->mem_resource[0 ].addr;if (hw->ctrl_bar ==NULL ) { -RTE_LOG (ERR ,PMD , -"hw->ctrl_bar is NULL. BAR0 not configured\n" ); +PMD_DRV_LOG (ERR , +"hw->ctrl_bar is NULL. BAR0 not configured" );return -ENODEV ; }
@@ -2864,11 +2864,11 @@ nfp_net_init(struct rte_eth_dev *eth_dev) hw->total_ports *32768 , &hw->ctrl_area);if (!hw->ctrl_bar) { -printf ("nfp_rtsym_map fails for _pf0_net_ctrl_bar\n" ); +printf ("nfp_rtsym_map fails for _pf0_net_ctrl_bar" );return -EIO ; } -PMD_INIT_LOG (DEBUG ,"ctrl bar: %p\n" , hw->ctrl_bar); +PMD_INIT_LOG (DEBUG ,"ctrl bar: %p" , hw->ctrl_bar); }if (port >0 ) {
@@ -2880,7 +2880,7 @@ nfp_net_init(struct rte_eth_dev *eth_dev) (port *NFP_PF_CSR_SLICE_SIZE ); } -PMD_INIT_LOG (DEBUG ,"ctrl bar: %p\n" , hw->ctrl_bar); +PMD_INIT_LOG (DEBUG ,"ctrl bar: %p" , hw->ctrl_bar); hw->max_rx_queues =nn_cfg_readl (hw,NFP_NET_CFG_MAX_RXRINGS ); hw->max_tx_queues =nn_cfg_readl (hw,NFP_NET_CFG_MAX_TXRINGS );
@@ -2896,13 +2896,13 @@ nfp_net_init(struct rte_eth_dev *eth_dev) rx_bar_off = start_q *NFP_QCP_QUEUE_ADDR_SZ ;break ;default : -RTE_LOG (ERR ,PMD ,"nfp_net: no device ID matching\n" ); +PMD_DRV_LOG (ERR ,"nfp_net: no device ID matching" ); err = -ENODEV ; goto dev_err_ctrl_map; } -PMD_INIT_LOG (DEBUG ,"tx_bar_off: 0x%" PRIx64"\n" , tx_bar_off); -PMD_INIT_LOG (DEBUG ,"rx_bar_off: 0x%" PRIx64"\n" , rx_bar_off); +PMD_INIT_LOG (DEBUG ,"tx_bar_off: 0x%" PRIx64"" , tx_bar_off); +PMD_INIT_LOG (DEBUG ,"rx_bar_off: 0x%" PRIx64"" , rx_bar_off);if (hw->is_pf && port ==0 ) {/* configure access to tx/rx vNIC BARs */
@@ -2912,12 +2912,12 @@ nfp_net_init(struct rte_eth_dev *eth_dev) &hw->hwqueues_area);if (!hwport0->hw_queues) { -printf ("nfp_rtsym_map fails for net.qc\n" ); +printf ("nfp_rtsym_map fails for net.qc" ); err = -EIO ; goto dev_err_ctrl_map; } -PMD_INIT_LOG (DEBUG ,"tx/rx bar address: 0x%p\n" , +PMD_INIT_LOG (DEBUG ,"tx/rx bar address: 0x%p" , hwport0->hw_queues); }
@@ -2998,7 +2998,7 @@ nfp_net_init(struct rte_eth_dev *eth_dev) }if (!is_valid_assigned_ether_addr ((struct ether_addr *)&hw->mac_addr)) { -PMD_INIT_LOG (INFO ,"Using random mac address for port %d\n" , +PMD_INIT_LOG (INFO ,"Using random mac address for port %d" , port);/* Using random mac addresses for VFs */ eth_random_addr (&hw->mac_addr[0 ]);
@@ -3124,7 +3124,7 @@ nfp_fw_upload(struct rte_pci_device *dev, struct nfp_nsp *nsp, char *card)sprintf (fw_name,"%s/%s.nffw" ,DEFAULT_FW_PATH , serial); -RTE_LOG (DEBUG ,PMD ,"Trying with fw file: %s\n" , fw_name); +PMD_DRV_LOG (DEBUG ,"Trying with fw file: %s" , fw_name); fw_f =open (fw_name,O_RDONLY );if (fw_f >0 ) goto read_fw;
@@ -3132,34 +3132,34 @@ nfp_fw_upload(struct rte_pci_device *dev, struct nfp_nsp *nsp, char *card)/* Then try the PCI name */ sprintf (fw_name,"%s/pci-%s.nffw" ,DEFAULT_FW_PATH , dev->device.name); -RTE_LOG (DEBUG ,PMD ,"Trying with fw file: %s\n" , fw_name); +PMD_DRV_LOG (DEBUG ,"Trying with fw file: %s" , fw_name); fw_f =open (fw_name,O_RDONLY );if (fw_f >0 ) goto read_fw;/* Finally try the card type and media */ sprintf (fw_name,"%s/%s" ,DEFAULT_FW_PATH , card); -RTE_LOG (DEBUG ,PMD ,"Trying with fw file: %s\n" , fw_name); +PMD_DRV_LOG (DEBUG ,"Trying with fw file: %s" , fw_name); fw_f =open (fw_name,O_RDONLY );if (fw_f <0 ) { -RTE_LOG (INFO ,PMD ,"Firmware file %s not found." , fw_name); +PMD_DRV_LOG (INFO ,"Firmware file %s not found." , fw_name);return -ENOENT ; } read_fw:if (fstat (fw_f, &file_stat) <0 ) { -RTE_LOG (INFO ,PMD ,"Firmware file %s size is unknown" , fw_name); +PMD_DRV_LOG (INFO ,"Firmware file %s size is unknown" , fw_name);close (fw_f);return -ENOENT ; } fsize = file_stat.st_size; -RTE_LOG (INFO ,PMD ,"Firmware file found at %s with size: %" PRIu64"\n" , +PMD_DRV_LOG (INFO ,"Firmware file found at %s with size: %" PRIu64"" , fw_name, (uint64_t )fsize); fw_buf =malloc ((size_t )fsize);if (!fw_buf) { -RTE_LOG (INFO ,PMD ,"malloc failed for fw buffer" ); +PMD_DRV_LOG (INFO ,"malloc failed for fw buffer" );close (fw_f);return -ENOMEM ; }
@@ -3167,7 +3167,7 @@ nfp_fw_upload(struct rte_pci_device *dev, struct nfp_nsp *nsp, char *card) bytes =read (fw_f, fw_buf, fsize);if (bytes != fsize) { -RTE_LOG (INFO ,PMD ,"Reading fw to buffer failed.\n" +PMD_DRV_LOG (INFO ,"Reading fw to buffer failed." "Just %" PRIu64" of %" PRIu64" bytes read" , (uint64_t )bytes, (uint64_t )fsize);free (fw_buf);
@@ -3175,9 +3175,9 @@ nfp_fw_upload(struct rte_pci_device *dev, struct nfp_nsp *nsp, char *card)return -EIO ; } -RTE_LOG (INFO ,PMD ,"Uploading the firmware ..." ); +PMD_DRV_LOG (INFO ,"Uploading the firmware ..." );nfp_nsp_load_fw (nsp, fw_buf, bytes); -RTE_LOG (INFO ,PMD ,"Done" ); +PMD_DRV_LOG (INFO ,"Done" );free (fw_buf);close (fw_f);
@@ -3197,29 +3197,29 @@ nfp_fw_setup(struct rte_pci_device *dev, struct nfp_cpp *cpp, nfp_fw_model =nfp_hwinfo_lookup (hwinfo,"assembly.partno" );if (nfp_fw_model) { -RTE_LOG (INFO ,PMD ,"firmware model found: %s\n" , nfp_fw_model); +PMD_DRV_LOG (INFO ,"firmware model found: %s" , nfp_fw_model); }else { -RTE_LOG (ERR ,PMD ,"firmware model NOT found\n" ); +PMD_DRV_LOG (ERR ,"firmware model NOT found" );return -EIO ; }if (nfp_eth_table->count ==0 || nfp_eth_table->count >8 ) { -RTE_LOG (ERR ,PMD ,"NFP ethernet table reports wrong ports: %u\n" , +PMD_DRV_LOG (ERR ,"NFP ethernet table reports wrong ports: %u" , nfp_eth_table->count);return -EIO ; } -RTE_LOG (INFO ,PMD ,"NFP ethernet port table reports %u ports\n" , +PMD_DRV_LOG (INFO ,"NFP ethernet port table reports %u ports" , nfp_eth_table->count); -RTE_LOG (INFO ,PMD ,"Port speed: %u\n" , nfp_eth_table->ports[0 ].speed); +PMD_DRV_LOG (INFO ,"Port speed: %u" , nfp_eth_table->ports[0 ].speed);sprintf (card_desc,"nic_%s_%dx%d.nffw" , nfp_fw_model, nfp_eth_table->count, nfp_eth_table->ports[0 ].speed /1000 ); nsp =nfp_nsp_open (cpp);if (!nsp) { -RTE_LOG (ERR ,PMD ,"NFP error when obtaining NSP handle\n" ); +PMD_DRV_LOG (ERR ,"NFP error when obtaining NSP handle" );return -EIO ; }
@@ -3248,25 +3248,25 @@ static int nfp_pf_pci_probe(struct rte_pci_driver *pci_drv __rte_unused, cpp =nfp_cpp_from_device_name (dev->device.name);if (!cpp) { -RTE_LOG (ERR ,PMD ,"A CPP handle can not be obtained" ); +PMD_DRV_LOG (ERR ,"A CPP handle can not be obtained" ); ret = -EIO ; goto error; } hwinfo =nfp_hwinfo_read (cpp);if (!hwinfo) { -RTE_LOG (ERR ,PMD ,"Error reading hwinfo table" ); +PMD_DRV_LOG (ERR ,"Error reading hwinfo table" );return -EIO ; } nfp_eth_table =nfp_eth_read_ports (cpp);if (!nfp_eth_table) { -RTE_LOG (ERR ,PMD ,"Error reading NFP ethernet table\n" ); +PMD_DRV_LOG (ERR ,"Error reading NFP ethernet table" );return -EIO ; }if (nfp_fw_setup (dev, cpp, nfp_eth_table, hwinfo)) { -RTE_LOG (INFO ,PMD ,"Error when uploading firmware\n" ); +PMD_DRV_LOG (INFO ,"Error when uploading firmware" ); ret = -EIO ; goto error; }
@@ -3274,7 +3274,7 @@ static int nfp_pf_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,/* Now the symbol table should be there */ sym_tbl =nfp_rtsym_table_read (cpp);if (!sym_tbl) { -RTE_LOG (ERR ,PMD ,"Something is wrong with the firmware" +PMD_DRV_LOG (ERR ,"Something is wrong with the firmware" " symbol table" ); ret = -EIO ; goto error;
@@ -3282,14 +3282,14 @@ static int nfp_pf_pci_probe(struct rte_pci_driver *pci_drv __rte_unused, total_ports =nfp_rtsym_read_le (sym_tbl,"nfd_cfg_pf0_num_ports" , &err);if (total_ports != (int )nfp_eth_table->count) { -RTE_LOG (ERR ,PMD ,"Inconsistent number of ports\n" ); +PMD_DRV_LOG (ERR ,"Inconsistent number of ports" ); ret = -EIO ; goto error; } -PMD_INIT_LOG (INFO ,"Total pf ports: %d\n" , total_ports); +PMD_INIT_LOG (INFO ,"Total pf ports: %d" , total_ports);if (total_ports <=0 || total_ports >8 ) { -RTE_LOG (ERR ,PMD ,"nfd_cfg_pf0_num_ports symbol with wrong value" ); +PMD_DRV_LOG (ERR ,"nfd_cfg_pf0_num_ports symbol with wrong value" ); ret = -ENODEV ; goto error; }
Signature · 1 line
2.17.0
Hi Stephen,
Thanks for this patch set.
I'm happy with it although I have some concerns regarding how the dynamic logs work, or maybe I have a wrong understanding about it. I have tried to read some doc about how it works, and I found the original patch from Olivier the best source, so maybe things have changed a bit and my concerns are unfounded.
I think it is OK to specifically add something like
--log-level='pmd\.i40e.*,8'
if you want to debug a PMD, but if you are an user and you just want to know why the app is not finding any port, finding out the right string is not trivial. For example, with an PF, the NFP PMD goes through a process where the NFP device (no the NIC) is accessed first through a complex interface, then firmware is uploaded, DPDK ports created (for multiport devices), etc. I think any error in that process should be output if the right loglevel is there and not just if the right log type was specifically enabled. Is this what would happen with your patchset?
I have suffered silent configuration problems, like the NFP card being in the wrong NUMA socket, and although I can solve that quickly because I have the knowledge, other people using NFP with DPDK require someone to help because they do not know what is going on. And this is usually bad because they have another NIC card in the same host (in the right NUMA socket) and the app just works smoothly then, leaving our NIC with a bad press. So I think, some errors should always appear with the right loglevel configured.
Quoted 20 lines · Stephen Hemminger
These are several small changes to make the Netronome driver use logging macros in the same way as other drivers.
Compile tested only. I don't have Netronome hardware.
Stephen Hemminger (5):
net/nfp: use correct logtype for init messages
net/nfp: add implied new line toPMD_DRV_LOG
net/nfp: fix double space in init log
net/nfl: add newline in PMD_RX/TX_LOG macros
net/nfp: use dynamic logging everywheredrivers/net/nfp/nfp_net.c | 186 ++++++++++++++++----------------- drivers/net/nfp/nfp_net_logs.h | 9 +- 2 files changed, 98 insertions(+), 97 deletions(-)--
2.17.0
Quoted 14 lines · On Thu, 26 Apr 2018 13:52:53 +0100 Alejandro Lucero…
Hi Stephen,
Thanks for this patch set.
I'm happy with it although I have some concerns regarding how the dynamic logs work, or maybe I have a wrong understanding about it. I have tried to read some doc about how it works, and I found the original patch from Olivier the best source, so maybe things have changed a bit and my concerns are unfounded.
I think it is OK to specifically add something like
--log-level='pmd\.i40e.*,8'
Quoted 8 lines
if you want to debug a PMD, but if you are an user and you just want to know why the app is not finding any port, finding out the right string is not trivial. For example, with an PF, the NFP PMD goes through a process where the NFP device (no the NIC) is accessed first through a complex interface, then firmware is uploaded, DPDK ports created (for multiport devices), etc. I think any error in that process should be output if the right loglevel is there and not just if the right log type was specifically enabled. Is this what would happen with your patchset?
Most drivers set default log level to NOTICE. Then if they see something obviously wrong it will show up if the right log level is used.
For the case of finding out why no drivers are found then doing something like
--log-level='pmd.*:info'
would be useful.
Latest version makes regex optional and allows symbolic levels.
Quoted 7 lines
I have suffered silent configuration problems, like the NFP card being in the wrong NUMA socket, and although I can solve that quickly because I have the knowledge, other people using NFP with DPDK require someone to help because they do not know what is going on. And this is usually bad because they have another NIC card in the same host (in the right NUMA socket) and the app just works smoothly then, leaving our NIC with a bad press. So I think, some errors should always appear with the right loglevel configured.
Driver should definitely use level > INFO for things that are wrong.
Quoted 46 lines · Stephen Hemminger
Quoted 6 lines · On Thu, 26 Apr 2018 13:52:53 +0100 Alejandro Lucero…
Hi Stephen,
Thanks for this patch set.
I'm happy with it although I have some concerns regarding how the dynamic logs work, or maybe I have a wrong understanding about it. I have tried
to
Quoted 2 lines
read some doc about how it works, and I found the original patch from Olivier the best source, so maybe things have changed a bit and my
concerns
Quoted 6 lines
are unfounded.
I think it is OK to specifically add something like
--log-level='pmd\.i40e.*,8'
Quoted 7 lines
if you want to debug a PMD, but if you are an user and you just want to know why the app is not finding any port, finding out the right string is not trivial. For example, with an PF, the NFP PMD goes through a process where the NFP device (no the NIC) is accessed first through a complex interface, then firmware is uploaded, DPDK ports created (for multiport devices), etc. I think any error in that process should be output if the right loglevel is there and not just if the right log type was
specifically
Quoted 1 line
enabled. Is this what would happen with your patchset?
Most drivers set default log level to NOTICE. Then if they see something obviously wrong it will show up if the right log level is used.
For the case of finding out why no drivers are found then doing something like
--log-level='pmd.*:info'would be useful.
Latest version makes regex optional and allows symbolic levels.
Quoted 2 lines
I have suffered silent configuration problems, like the NFP card being in the wrong NUMA socket, and although I can solve that quickly because I
have
Quoted 2 lines
the knowledge, other people using NFP with DPDK require someone to help because they do not know what is going on. And this is usually bad
Quoted 10 lines
because
Quoted 1 line
they have another NIC card in the same host (in the right NUMA socket)
and
Quoted 2 lines
the app just works smoothly then, leaving our NIC with a bad press. So I think, some errors should always appear with the right loglevel
configured.
Driver should definitely use level > INFO for things that are wrong.
Uhmm, yes. I think I need to submit some changes to the level of most of the PMD messages.
Thanks for the heads up.
I have reviewed and tested the patches and they all seem all right.
Quoted 62 lines · Alejandro Lucero
Quoted 46 lines · Stephen Hemminger
Quoted 6 lines · On Thu, 26 Apr 2018 13:52:53 +0100 Alejandro Lucero…
Hi Stephen,
Thanks for this patch set.
I'm happy with it although I have some concerns regarding how the dynamic logs work, or maybe I have a wrong understanding about it. I have tried
to
Quoted 2 lines
read some doc about how it works, and I found the original patch from Olivier the best source, so maybe things have changed a bit and my
concerns
Quoted 6 lines
are unfounded.
I think it is OK to specifically add something like
--log-level='pmd\.i40e.*,8'
Quoted 7 lines
if you want to debug a PMD, but if you are an user and you just want to know why the app is not finding any port, finding out the right string is not trivial. For example, with an PF, the NFP PMD goes through a process where the NFP device (no the NIC) is accessed first through a complex interface, then firmware is uploaded, DPDK ports created (for multiport devices), etc. I think any error in that process should be output if the right loglevel is there and not just if the right log type was
specifically
Quoted 1 line
enabled. Is this what would happen with your patchset?
Most drivers set default log level to NOTICE. Then if they see something obviously wrong it will show up if the right log level is used.
For the case of finding out why no drivers are found then doing something like
--log-level='pmd.*:info'would be useful.
Latest version makes regex optional and allows symbolic levels.
Quoted 2 lines
I have suffered silent configuration problems, like the NFP card being in the wrong NUMA socket, and although I can solve that quickly because I
have
Quoted 2 lines
the knowledge, other people using NFP with DPDK require someone to help because they do not know what is going on. And this is usually bad
Quoted 10 lines
because
Quoted 1 line
they have another NIC card in the same host (in the right NUMA socket)
and
Quoted 2 lines
the app just works smoothly then, leaving our NIC with a bad press. So I think, some errors should always appear with the right loglevel
configured.
Driver should definitely use level > INFO for things that are wrong.
Uhmm, yes. I think I need to submit some changes to the level of most of the PMD messages.
Quoted 6 lines
Thanks for the heads up.
I have reviewed and tested the patches and they all seem all right.
Acked-by Alejandro Lucero <[email protected]>Tested-by Alejandro Lucero <[email protected]>
Series applied to dpdk-next-net/master, thanks.