[dpdk-dev] [PATCH 0/5] net/nfp logging fixes

[dpdk-dev] [PATCH 0/5] net/nfp logging fixes

[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 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(-)

-- 
2.17.0

[dpdk-dev] [PATCH 1/5] net/nfp: use correct logtype for init messages

[Stephen Hemminger]

The NFP driver init messages would come out under PMD not net.pmd.nfp.init.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/nfp/nfp_net_logs.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/nfp/nfp_net_logs.h b/drivers/net/nfp/nfp_net_logs.h
index 3fe24e96b347..2917da56ced3 100644
--- a/drivers/net/nfp/nfp_net_logs.h
+++ b/drivers/net/nfp/nfp_net_logs.h
@@ -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
-- 
2.17.0

[dpdk-dev] [PATCH 2/5] net/nfp: add implied new line to PMD_DRV_LOG

[Stephen Hemminger]

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

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/nfp/nfp_net.c      | 22 +++++++++++-----------
 drivers/net/nfp/nfp_net_logs.h |  2 +-
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
index bedd4b668711..bf9d821c8c21 100644
--- a/drivers/net/nfp/nfp_net.c
+++ b/drivers/net/nfp/nfp_net.c
@@ -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);
 
diff --git a/drivers/net/nfp/nfp_net_logs.h b/drivers/net/nfp/nfp_net_logs.h
index 2917da56ced3..1641de2ba863 100644
--- a/drivers/net/nfp/nfp_net_logs.h
+++ b/drivers/net/nfp/nfp_net_logs.h
@@ -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_ */
-- 
2.17.0

[dpdk-dev] [PATCH 3/5] net/nfp: fix double space in init log

[Stephen Hemminger]

Shouldn't pass extra newline.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/nfp/nfp_net.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
index bf9d821c8c21..f32104ae7327 100644
--- a/drivers/net/nfp/nfp_net.c
+++ b/drivers/net/nfp/nfp_net.c
@@ -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]);
 		}
 	}
-- 
2.17.0

[dpdk-dev] [PATCH 4/5] net/nfl: add newline in PMD_RX/TX_LOG macros

[Stephen Hemminger]

Be consistent with usage in other drivers.
No need for snowflake drivers.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/nfp/nfp_net.c      | 30 +++++++++++++++---------------
 drivers/net/nfp/nfp_net_logs.h |  4 ++--
 2 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
index f32104ae7327..c8b0fff9df14 100644
--- a/drivers/net/nfp/nfp_net.c
+++ b/drivers/net/nfp/nfp_net.c
@@ -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;
diff --git a/drivers/net/nfp/nfp_net_logs.h b/drivers/net/nfp/nfp_net_logs.h
index 1641de2ba863..9952881cd52c 100644
--- a/drivers/net/nfp/nfp_net_logs.h
+++ b/drivers/net/nfp/nfp_net_logs.h
@@ -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)
-- 
2.17.0

[dpdk-dev] [PATCH 5/5] net/nfp: use dynamic logging everywhere

[Stephen Hemminger]

Drivers should only log with their assigned logtype, not with the
generic PMD log type.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/nfp/nfp_net.c | 132 +++++++++++++++++++-------------------
 1 file changed, 66 insertions(+), 66 deletions(-)

diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
index c8b0fff9df14..e092ab20296f 100644
--- a/drivers/net/nfp/nfp_net.c
+++ b/drivers/net/nfp/nfp_net.c
@@ -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 & ETH_VLAN_FILTER_OFFLOAD) ||
 	    (mask & ETH_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;
 	}
-- 
2.17.0

Re: [dpdk-dev] [PATCH 0/5] net/nfp logging fixes

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

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.




On Wed, Apr 25, 2018 at 4:45 PM, Stephen Hemminger <
stephen@networkplumber.org> wrote:

> 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(-)
>
> --
> 2.17.0
>
>

Re: [dpdk-dev] [PATCH 0/5] net/nfp logging fixes

[Stephen Hemminger]

On Thu, 26 Apr 2018 13:52:53 +0100
Alejandro Lucero <alejandro.lucero@netronome.com> wrote:

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

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.


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

Re: [dpdk-dev] [PATCH 0/5] net/nfp logging fixes

[Alejandro Lucero]

On Thu, Apr 26, 2018 at 4:42 PM, Stephen Hemminger <
stephen@networkplumber.org> wrote:

> On Thu, 26 Apr 2018 13:52:53 +0100
> Alejandro Lucero <alejandro.lucero@netronome.com> wrote:
>
> > 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?
>
> 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.
>
>
> > 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.
>
>
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.

Acked-by: Alejandro Lucero <alejandro.lucero@netronome.com>
Tested-by: Alejandro Lucero <alejandro.lucero@netronome.com>

Re: [dpdk-dev] [PATCH 0/5] net/nfp logging fixes

[Ferruh Yigit]

On 4/26/2018 7:14 PM, Alejandro Lucero wrote:
> On Thu, Apr 26, 2018 at 4:42 PM, Stephen Hemminger <
> stephen@networkplumber.org> wrote:
> 
>> On Thu, 26 Apr 2018 13:52:53 +0100
>> Alejandro Lucero <alejandro.lucero@netronome.com> wrote:
>>
>>> 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?
>>
>> 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.
>>
>>
>>> 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.
>>
>>
> 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.
> 
> Acked-by: Alejandro Lucero <alejandro.lucero@netronome.com>
> Tested-by: Alejandro Lucero <alejandro.lucero@netronome.com>

Series applied to dpdk-next-net/master, thanks.