Re: [PATCH 2/3] net/nfp: get rid of the usage of RTE log level type

[Stephen Hemminger <stephen@networkplumber.org>]

On Mon, 20 Feb 2023 10:09:51 +0000
Ferruh Yigit <ferruh.yigit@amd.com> wrote:

> On 2/20/2023 1:36 AM, Chaoyong He wrote:
> >> On 2/17/2023 2:45 AM, Chaoyong He wrote:  
> >>> Register the own RX/TX debug log level type, and get rid of the usage
> >>> of RTE_LOGTYPE_*. Then we can control the log by a independent switch.
> >>>
> >>> Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
> >>> Reviewed-by: Niklas Söderlund <niklas.soderlund@corigine.com>
> >>> ---
> >>>  drivers/net/nfp/nfp_logs.c | 10 ++++++++++
> >>> drivers/net/nfp/nfp_logs.h |  8 ++++++--
> >>>  2 files changed, 16 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/net/nfp/nfp_logs.c b/drivers/net/nfp/nfp_logs.c
> >>> index 48c42fe53f..cd58bcee43 100644
> >>> --- a/drivers/net/nfp/nfp_logs.c
> >>> +++ b/drivers/net/nfp/nfp_logs.c
> >>> @@ -5,6 +5,16 @@
> >>>
> >>>  #include "nfp_logs.h"
> >>>
> >>> +#include <rte_ethdev.h>
> >>> +
> >>>  RTE_LOG_REGISTER_SUFFIX(nfp_logtype_init, init, NOTICE);
> >>> RTE_LOG_REGISTER_SUFFIX(nfp_logtype_driver, driver, NOTICE);
> >>> RTE_LOG_REGISTER_SUFFIX(nfp_logtype_cpp, cpp, NOTICE);
> >>> +
> >>> +#ifdef RTE_ETHDEV_DEBUG_RX
> >>> +RTE_LOG_REGISTER_SUFFIX(nfp_logtype_rx, rx, DEBUG) #endif
> >>> +
> >>> +#ifdef RTE_ETHDEV_DEBUG_TX
> >>> +RTE_LOG_REGISTER_SUFFIX(nfp_logtype_tx, tx, DEBUG) #endif
> >>> diff --git a/drivers/net/nfp/nfp_logs.h b/drivers/net/nfp/nfp_logs.h
> >>> index b7632ee72c..315a57811c 100644
> >>> --- a/drivers/net/nfp/nfp_logs.h
> >>> +++ b/drivers/net/nfp/nfp_logs.h
> >>> @@ -15,15 +15,19 @@ extern int nfp_logtype_init;  #define
> >>> PMD_INIT_FUNC_TRACE() PMD_INIT_LOG(DEBUG, " >>")
> >>>
> >>>  #ifdef RTE_ETHDEV_DEBUG_RX
> >>> +extern int nfp_logtype_rx;
> >>>  #define PMD_RX_LOG(level, fmt, args...) \
> >>> -	RTE_LOG(level, PMD, "%s() rx: " fmt "\n", __func__, ## args)
> >>> +	rte_log(RTE_LOG_ ## level, nfp_logtype_rx, \
> >>> +		"%s(): " fmt "\n", __func__, ## args)
> >>>  #else
> >>>  #define PMD_RX_LOG(level, fmt, args...) do { } while (0)  #endif
> >>>
> >>>  #ifdef RTE_ETHDEV_DEBUG_TX
> >>> +extern int nfp_logtype_tx;
> >>>  #define PMD_TX_LOG(level, fmt, args...) \
> >>> -	RTE_LOG(level, PMD, "%s() tx: " fmt "\n", __func__, ## args)
> >>> +	rte_log(RTE_LOG_ ## level, nfp_logtype_tx, \
> >>> +		"%s(): " fmt "\n", __func__, ## args)
> >>>  #else
> >>>  #define PMD_TX_LOG(level, fmt, args...) do { } while (0)  #endif  
> >>
> >> Intention is to replace 'RTE_LOG_DP' with 'PMD_RX_LOG'/'PMD_TX_LOG',
> >> but these are not exactly same (although difference is minor).
> >>
> >> When 'RTE_ETHDEV_DEBUG_RX' is set, ethdev layer also adds some
> >> additional load, although I believe that will small comparing to logging in
> >> driver.
> >> If 'RTE_LOG_DP' used, the ethdev layer cost can be removed.
> >>
> >> With 'RTE_LOG_DP', log level more verbose than requested won't cause any
> >> performance impact. Like if ERR level requested, INFO, DEBUG etc logs will
> >> be compiled out and won't cause any performance impact.
> >> But with 'RTE_ETHDEV_DEBUG_RX', even log level only request ERR, all
> >> logging will add cost of at least an if branch (checking log level).
> >>
> >>
> >> For many cases I am not sure these differences matters, and already many
> >> drivers directly uses 'RTE_ETHDEV_DEBUG_RX' as done here. So you may
> >> prefer to keep as it is.
> >>
> >> But if there is a desire for this fine grain approach, it is possible to add a
> >> version of 'RTE_LOG_DP' macro that accepts dynamic log type (instead of
> >> static RTE_LOGTYPE_# type), what do you think?
> >>  
> > 
> > Thanks for the suggestion.
> > For now, we prefer to keep as it is.
> > If we does need the more refined design in the future, we would follow your advice here, thanks again.  
> 
> ack, I just wanted to double check. I will proceed as it is.

As part of my patch series (work in progress) to get rid of RTE_LOGTYPE_PMD
needed to add a helper now for RTE_DP_LOG like this.

diff --git a/lib/eal/include/rte_log.h b/lib/eal/include/rte_log.h
index 6d2b0856a565..f377bc6db79b 100644
--- a/lib/eal/include/rte_log.h
+++ b/lib/eal/include/rte_log.h
@@ -336,6 +336,37 @@ int rte_vlog(uint32_t level, uint32_t logtype, const char *format, va_list ap)
 	 rte_log(RTE_LOG_ ## l,					\
 		 RTE_LOGTYPE_ ## t, # t ": " __VA_ARGS__)
 
+/**
+ * Generates a log message for data path.
+ *
+ * Similar to rte_log(), except that it is an inline function that
+ * can be eliminated at compilation time if RTE_LOG_DP_LEVEL configuration
+ * option is lower than the log level argument.
+ *
+ * @param level
+ *   Log level. A value between RTE_LOG_EMERG (1) and RTE_LOG_DEBUG (8).
+ * @param logtype
+ *   The log type, for example, RTE_LOGTYPE_EAL.
+ * @param format
+ *   The format string, as in printf(3), followed by the variable arguments
+ *   required by the format.
+ * @return
+ *   - 0: Success.
+ *   - Negative on error.
+ */
+static inline __rte_format_printf(3, 4)
+void rte_log_dp(uint32_t level, uint32_t logtype, const char *format, ...)
+
+{
+	va_list ap;
+
+	if (level <= RTE_LOG_DP_LEVEL) {
+		va_start(ap, format);
+		rte_vlog(level, logtype, format, ap);
+		va_end(ap);
+	}
+}
+
 /**
  * Generates a log message for data path.
  *
@@ -357,10 +388,8 @@ int rte_vlog(uint32_t level, uint32_t logtype, const char *format, va_list ap)
  *   - Negative on error.
  */
 #define RTE_LOG_DP(l, t, ...)					\
-	(void)((RTE_LOG_ ## l <= RTE_LOG_DP_LEVEL) ?		\
-	 rte_log(RTE_LOG_ ## l,					\
-		 RTE_LOGTYPE_ ## t, # t ": " __VA_ARGS__) :	\
-	 0)
+	rte_log_dp(RTE_LOG_ ## l,				\
+		 RTE_LOGTYPE_ ## t, # t ": " __VA_ARGS__)
 
 #define RTE_LOG_REGISTER_IMPL(type, name, level)			    \
 int type;								    \

The NFP part is:
diff --git a/drivers/net/nfp/flower/nfp_flower.c b/drivers/net/nfp/flower/nfp_flower.c
index f1424a010dde..15401c0d5ba6 100644
--- a/drivers/net/nfp/flower/nfp_flower.c
+++ b/drivers/net/nfp/flower/nfp_flower.c
@@ -330,7 +330,7 @@ nfp_flower_pf_recv_pkts(void *rx_queue,
 		 * DPDK just checks the queue is lower than max queues
 		 * enabled. But the queue needs to be configured
 		 */
-		RTE_LOG_DP(ERR, PMD, "RX Bad queue\n");
+		PMD_DP_LOG(ERR, "RX Bad queue");
 		return 0;
 	}
 
@@ -343,7 +343,7 @@ nfp_flower_pf_recv_pkts(void *rx_queue,
 	while (avail + avail_multiplexed < nb_pkts) {
 		rxb = &rxq->rxbufs[rxq->rd_p];
 		if (unlikely(rxb == NULL)) {
-			RTE_LOG_DP(ERR, PMD, "rxb does not exist!\n");
+			PMD_DP_LOG(ERR, "rxb does not exist!");
 			break;
 		}
 
@@ -363,9 +363,9 @@ nfp_flower_pf_recv_pkts(void *rx_queue,
 		 */
 		new_mb = rte_pktmbuf_alloc(rxq->mem_pool);
 		if (unlikely(new_mb == NULL)) {
-			RTE_LOG_DP(DEBUG, PMD,
-			"RX mbuf alloc failed port_id=%u queue_id=%d\n",
-				rxq->port_id, rxq->qidx);
+			PMD_DP_LOG(DEBUG,
+				   "RX mbuf alloc failed port_id=%u queue_id=%d\n",
+				   rxq->port_id, rxq->qidx);
 			nfp_net_mbuf_alloc_failed(rxq);
 			break;
 		}
@@ -378,7 +378,7 @@ nfp_flower_pf_recv_pkts(void *rx_queue,
 		rxb->mbuf = new_mb;
 
 		PMD_RX_LOG(DEBUG, "Packet len: %u, mbuf_size: %u",
-				rxds->rxd.data_len, rxq->mbuf_size);
+			   rxds->rxd.data_len, rxq->mbuf_size);
 
 		/* Size of this segment */
 		mb->data_len = rxds->rxd.data_len - NFP_DESC_META_LEN(rxds);
@@ -391,15 +391,15 @@ nfp_flower_pf_recv_pkts(void *rx_queue,
 			 * responsibility of avoiding it. But we have
 			 * to give some info about the error
 			 */
-			RTE_LOG_DP(ERR, PMD,
-				"mbuf overflow likely due to the RX offset.\n"
-				"\t\tYour mbuf size should have extra space for"
-				" RX offset=%u bytes.\n"
-				"\t\tCurrently you just have %u bytes available"
-				" but the received packet is %u bytes long",
-				hw->rx_offset,
-				rxq->mbuf_size - hw->rx_offset,
-				mb->data_len);
+			PMD_DP_LOG(ERR,
+				   "mbuf overflow likely due to the RX offset.\n"
+				   "\t\tYour mbuf size should have extra space for"
+				   " RX offset=%u bytes.\n"
+				   "\t\tCurrently you just have %u bytes available"
+				   " but the received packet is %u bytes long",
+				   hw->rx_offset,
+				   rxq->mbuf_size - hw->rx_offset,
+				   mb->data_len);
 			rte_pktmbuf_free(mb);
 			break;
 		}
@@ -420,14 +420,14 @@ nfp_flower_pf_recv_pkts(void *rx_queue,
 		/* Checking the RSS flag */
 		nfp_flower_parse_metadata(rxq, rxds, mb, &meta_portid);
 		PMD_RX_LOG(DEBUG, "Received from port %u type %u",
-				NFP_FLOWER_CMSG_PORT_VNIC(meta_portid),
-				NFP_FLOWER_CMSG_PORT_VNIC_TYPE(meta_portid));
+			   NFP_FLOWER_CMSG_PORT_VNIC(meta_portid),
+			   NFP_FLOWER_CMSG_PORT_VNIC_TYPE(meta_portid));
 
 		/* Checking the checksum flag */
 		nfp_net_rx_cksum(rxq, rxds, mb);
 
 		if ((rxds->rxd.flags & PCIE_DESC_RX_VLAN) &&
-				(hw->ctrl & NFP_NET_CFG_CTRL_RXVLAN)) {
+		    (hw->ctrl & NFP_NET_CFG_CTRL_RXVLAN)) {
 			mb->vlan_tci = rte_cpu_to_le_32(rxds->rxd.vlan);
 			mb->ol_flags |= RTE_MBUF_F_RX_VLAN | RTE_MBUF_F_RX_VLAN_STRIPPED;
 		}
@@ -439,7 +439,7 @@ nfp_flower_pf_recv_pkts(void *rx_queue,
 			avail_multiplexed++;
 		} else if (repr != NULL) {
 			PMD_RX_LOG(ERR, "[%u] No ring available for repr_port %s\n",
-					hw->idx, repr->name);
+				   hw->idx, repr->name);
 			PMD_RX_LOG(DEBUG, "Adding the mbuf to the mbuf array passed by the app");
 			rx_pkts[avail++] = mb;
 		} else {
@@ -465,7 +465,7 @@ nfp_flower_pf_recv_pkts(void *rx_queue,
 		return nb_hold;
 
 	PMD_RX_LOG(DEBUG, "RX port_id=%u queue_id=%d, %d packets received",
-			rxq->port_id, rxq->qidx, nb_hold);
+		   rxq->port_id, rxq->qidx, nb_hold);
 
 	nb_hold += rxq->nb_rx_hold;
 
@@ -476,7 +476,7 @@ nfp_flower_pf_recv_pkts(void *rx_queue,
 	rte_wmb();
 	if (nb_hold > rxq->rx_free_thresh) {
 		PMD_RX_LOG(DEBUG, "port=%u queue=%d nb_hold=%u avail=%d",
-				rxq->port_id, rxq->qidx, nb_hold, avail);
+			   rxq->port_id, rxq->qidx, nb_hold, avail);
 		nfp_qcp_ptr_add(rxq->qcp_fl, NFP_QCP_WRITE_PTR, nb_hold);
 		nb_hold = 0;
 	}
diff --git a/drivers/net/nfp/flower/nfp_flower_ctrl.c b/drivers/net/nfp/flower/nfp_flower_ctrl.c
index 03a2e2e6222f..3de3bf1ca2e9 100644
--- a/drivers/net/nfp/flower/nfp_flower_ctrl.c
+++ b/drivers/net/nfp/flower/nfp_flower_ctrl.c
@@ -91,15 +91,15 @@ nfp_flower_ctrl_vnic_recv(void *rx_queue,
 			 * responsibility of avoiding it. But we have
 			 * to give some info about the error
 			 */
-			RTE_LOG_DP(ERR, PMD,
-				"mbuf overflow likely due to the RX offset.\n"
-				"\t\tYour mbuf size should have extra space for"
-				" RX offset=%u bytes.\n"
-				"\t\tCurrently you just have %u bytes available"
-				" but the received packet is %u bytes long",
-				hw->rx_offset,
-				rxq->mbuf_size - hw->rx_offset,
-				mb->data_len);
+			PMD_DP_LOG(ERR,
+				   "mbuf overflow likely due to the RX offset.\n"
+				   "\t\tYour mbuf size should have extra space for"
+				   " RX offset=%u bytes.\n"
+				   "\t\tCurrently you just have %u bytes available"
+				   " but the received packet is %u bytes long",
+				   hw->rx_offset,
+				   rxq->mbuf_size - hw->rx_offset,
+				   mb->data_len);
 			rte_pktmbuf_free(mb);
 			break;
 		}
diff --git a/drivers/net/nfp/nfp_cpp_bridge.c b/drivers/net/nfp/nfp_cpp_bridge.c
index 4aa36eb5814f..3c67a8fd419c 100644
--- a/drivers/net/nfp/nfp_cpp_bridge.c
+++ b/drivers/net/nfp/nfp_cpp_bridge.c
@@ -126,7 +126,7 @@ nfp_cpp_bridge_serve_write(int sockfd, struct nfp_cpp *cpp)
 	size_t count, curlen;
 	int err = 0;
 
-	PMD_CPP_LOG(DEBUG, "%s: offset size %zu, count_size: %zu\n", __func__,
+	PMD_CPP_LOG(DEBUG, "offset size %zu, count_size: %zu",
 		sizeof(off_t), sizeof(size_t));
 
 	/* Reading the count param */
@@ -145,10 +145,9 @@ nfp_cpp_bridge_serve_write(int sockfd, struct nfp_cpp *cpp)
 	cpp_id = (offset >> 40) << 8;
 	nfp_offset = offset & ((1ull << 40) - 1);
 
-	PMD_CPP_LOG(DEBUG, "%s: count %zu and offset %jd\n", __func__, count,
-		offset);
-	PMD_CPP_LOG(DEBUG, "%s: cpp_id %08x and nfp_offset %jd\n", __func__,
-		cpp_id, nfp_offset);
+	PMD_CPP_LOG(DEBUG, "count %zu and offset %jd", count, offset);
+	PMD_CPP_LOG(DEBUG, "cpp_id %08x and nfp_offset %jd",
+		    cpp_id, nfp_offset);
 
 	/* Adjust length if not aligned */
 	if (((nfp_offset + (off_t)count - 1) & ~(NFP_CPP_MEMIO_BOUNDARY - 1)) !=
@@ -162,14 +161,14 @@ nfp_cpp_bridge_serve_write(int sockfd, struct nfp_cpp *cpp)
 		area = nfp_cpp_area_alloc_with_name(cpp, cpp_id, "nfp.cdev",
 						    nfp_offset, curlen);
 		if (area == NULL) {
-			RTE_LOG(ERR, PMD, "%s: area alloc fail\n", __func__);
+			PMD_DRV_LOG(ERR, "area alloc fail");
 			return -EIO;
 		}
 
 		/* mapping the target */
 		err = nfp_cpp_area_acquire(area);
 		if (err < 0) {
-			RTE_LOG(ERR, PMD, "area acquire failed\n");
+			PMD_DRV_LOG(ERR, "area acquire failed");
 			nfp_cpp_area_free(area);
 			return -EIO;
 		}
@@ -179,20 +178,18 @@ nfp_cpp_bridge_serve_write(int sockfd, struct nfp_cpp *cpp)
 			if (len > sizeof(tmpbuf))
 				len = sizeof(tmpbuf);
 
-			PMD_CPP_LOG(DEBUG, "%s: Receive %u of %zu\n", __func__,
-					   len, count);
+			PMD_CPP_LOG(DEBUG, "Receive %u of %zu", len, count);
 			err = recv(sockfd, tmpbuf, len, MSG_WAITALL);
 			if (err != (int)len) {
-				RTE_LOG(ERR, PMD,
-					"%s: error when receiving, %d of %zu\n",
-					__func__, err, count);
+				PMD_DRV_LOG(ERR, "error when receiving, %d of %zu",
+					    err, count);
 				nfp_cpp_area_release(area);
 				nfp_cpp_area_free(area);
 				return -EIO;
 			}
 			err = nfp_cpp_area_write(area, pos, tmpbuf, len);
 			if (err < 0) {
-				RTE_LOG(ERR, PMD, "nfp_cpp_area_write error\n");
+				PMD_DRV_LOG(ERR, "nfp_cpp_area_write error");
 				nfp_cpp_area_release(area);
 				nfp_cpp_area_free(area);
 				return -EIO;
@@ -227,7 +224,7 @@ nfp_cpp_bridge_serve_read(int sockfd, struct nfp_cpp *cpp)
 	size_t count, curlen;
 	int err = 0;
 
-	PMD_CPP_LOG(DEBUG, "%s: offset size %zu, count_size: %zu\n", __func__,
+	PMD_CPP_LOG(DEBUG, "offset size %zu, count_size: %zu",
 		sizeof(off_t), sizeof(size_t));
 
 	/* Reading the count param */
@@ -246,9 +243,8 @@ nfp_cpp_bridge_serve_read(int sockfd, struct nfp_cpp *cpp)
 	cpp_id = (offset >> 40) << 8;
 	nfp_offset = offset & ((1ull << 40) - 1);
 
-	PMD_CPP_LOG(DEBUG, "%s: count %zu and offset %jd\n", __func__, count,
-			   offset);
-	PMD_CPP_LOG(DEBUG, "%s: cpp_id %08x and nfp_offset %jd\n", __func__,
+	PMD_CPP_LOG(DEBUG, "count %zu and offset %jd", count, offset);
+	PMD_CPP_LOG(DEBUG, "cpp_id %08x and nfp_offset %jd",
 			   cpp_id, nfp_offset);
 
 	/* Adjust length if not aligned */
@@ -262,13 +258,13 @@ nfp_cpp_bridge_serve_read(int sockfd, struct nfp_cpp *cpp)
 		area = nfp_cpp_area_alloc_with_name(cpp, cpp_id, "nfp.cdev",
 						    nfp_offset, curlen);
 		if (area == NULL) {
-			RTE_LOG(ERR, PMD, "%s: area alloc failed\n", __func__);
+			PMD_DRV_LOG(ERR, "area alloc failed");
 			return -EIO;
 		}
 
 		err = nfp_cpp_area_acquire(area);
 		if (err < 0) {
-			RTE_LOG(ERR, PMD, "area acquire failed\n");
+			PMD_DRV_LOG(ERR, "area acquire failed");
 			nfp_cpp_area_free(area);
 			return -EIO;
 		}
@@ -280,19 +276,19 @@ nfp_cpp_bridge_serve_read(int sockfd, struct nfp_cpp *cpp)
 
 			err = nfp_cpp_area_read(area, pos, tmpbuf, len);
 			if (err < 0) {
-				RTE_LOG(ERR, PMD, "nfp_cpp_area_read error\n");
+				PMD_DRV_LOG(ERR, "nfp_cpp_area_read error");
 				nfp_cpp_area_release(area);
 				nfp_cpp_area_free(area);
 				return -EIO;
 			}
-			PMD_CPP_LOG(DEBUG, "%s: sending %u of %zu\n", __func__,
+			PMD_CPP_LOG(DEBUG, "sending %u of %zu",
 					   len, count);
 
 			err = send(sockfd, tmpbuf, len, 0);
 			if (err != (int)len) {
-				RTE_LOG(ERR, PMD,
-					"%s: error when sending: %d of %zu\n",
-					__func__, err, count);
+				PMD_DRV_LOG(ERR,
+					    "error when sending: %d of %zu",
+					    err, count);
 				nfp_cpp_area_release(area);
 				nfp_cpp_area_free(area);
 				return -EIO;
@@ -325,39 +321,39 @@ nfp_cpp_bridge_serve_ioctl(int sockfd, struct nfp_cpp *cpp)
 	/* Reading now the IOCTL command */
 	err = recv(sockfd, &cmd, 4, 0);
 	if (err != 4) {
-		RTE_LOG(ERR, PMD, "%s: read error from socket\n", __func__);
+		PMD_DRV_LOG(ERR, "read error from socket");
 		return -EIO;
 	}
 
 	/* Only supporting NFP_IOCTL_CPP_IDENTIFICATION */
 	if (cmd != NFP_IOCTL_CPP_IDENTIFICATION) {
-		RTE_LOG(ERR, PMD, "%s: unknown cmd %d\n", __func__, cmd);
+		PMD_DRV_LOG(ERR, "unknown cmd %d", cmd);
 		return -EINVAL;
 	}
 
 	err = recv(sockfd, &ident_size, 4, 0);
 	if (err != 4) {
-		RTE_LOG(ERR, PMD, "%s: read error from socket\n", __func__);
+		PMD_DRV_LOG(ERR, "read error from socket");
 		return -EIO;
 	}
 
 	tmp = nfp_cpp_model(cpp);
 
-	PMD_CPP_LOG(DEBUG, "%s: sending NFP model %08x\n", __func__, tmp);
+	PMD_CPP_LOG(DEBUG, "sending NFP model %08x", tmp);
 
 	err = send(sockfd, &tmp, 4, 0);
 	if (err != 4) {
-		RTE_LOG(ERR, PMD, "%s: error writing to socket\n", __func__);
+		PMD_DRV_LOG(ERR, "error writing to socket");
 		return -EIO;
 	}
 
 	tmp = cpp->interface;
 
-	PMD_CPP_LOG(DEBUG, "%s: sending NFP interface %08x\n", __func__, tmp);
+	PMD_CPP_LOG(DEBUG, "sending NFP interface %08x", tmp);
 
 	err = send(sockfd, &tmp, 4, 0);
 	if (err != 4) {
-		RTE_LOG(ERR, PMD, "%s: error writing to socket\n", __func__);
+		PMD_DRV_LOG(ERR, "error writing to socket");
 		return -EIO;
 	}
 
@@ -384,8 +380,7 @@ nfp_cpp_bridge_service_func(void *args)
 	unlink("/tmp/nfp_cpp");
 	sockfd = socket(AF_UNIX, SOCK_STREAM, 0);
 	if (sockfd < 0) {
-		RTE_LOG(ERR, PMD, "%s: socket creation error. Service failed\n",
-			__func__);
+		PMD_DRV_LOG(ERR, "socket creation error. Service failed");
 		return -EIO;
 	}
 
@@ -399,16 +394,14 @@ nfp_cpp_bridge_service_func(void *args)
 	ret = bind(sockfd, (const struct sockaddr *)&address,
 		   sizeof(struct sockaddr));
 	if (ret < 0) {
-		RTE_LOG(ERR, PMD, "%s: bind error (%d). Service failed\n",
-				  __func__, errno);
+		PMD_DRV_LOG(ERR, "bind error (%d). Service failed", errno);
 		close(sockfd);
 		return ret;
 	}
 
 	ret = listen(sockfd, 20);
 	if (ret < 0) {
-		RTE_LOG(ERR, PMD, "%s: listen error(%d). Service failed\n",
-				  __func__, errno);
+		PMD_DRV_LOG(ERR, "listen error(%d). Service failed", errno);
 		close(sockfd);
 		return ret;
 	}
@@ -421,9 +414,7 @@ nfp_cpp_bridge_service_func(void *args)
 			if (errno == EAGAIN || errno == EWOULDBLOCK)
 				continue;
 
-			RTE_LOG(ERR, PMD, "%s: accept call error (%d)\n",
-					  __func__, errno);
-			RTE_LOG(ERR, PMD, "%s: service failed\n", __func__);
+			PMD_DRV_LOG(ERR, "accept call error (%d)", errno);
 			close(sockfd);
 			return -EIO;
 		}
@@ -431,12 +422,11 @@ nfp_cpp_bridge_service_func(void *args)
 		while (1) {
 			ret = recv(datafd, &op, 4, 0);
 			if (ret <= 0) {
-				PMD_CPP_LOG(DEBUG, "%s: socket close\n",
-						   __func__);
+				PMD_CPP_LOG(DEBUG, "socket close");
 				break;
 			}
 
-			PMD_CPP_LOG(DEBUG, "%s: getting op %u\n", __func__, op);
+			PMD_CPP_LOG(DEBUG, "getting op %u", op);
 
 			if (op == NFP_BRIDGE_OP_READ)
 				nfp_cpp_bridge_serve_read(datafd, cpp);
diff --git a/drivers/net/nfp/nfp_ethdev.c b/drivers/net/nfp/nfp_ethdev.c
index 290e2fcb41a6..3ea07aa923dd 100644
--- a/drivers/net/nfp/nfp_ethdev.c
+++ b/drivers/net/nfp/nfp_ethdev.c
@@ -521,9 +521,9 @@ nfp_net_init(struct rte_eth_dev *eth_dev)
 
 	/* NFP can not handle DMA addresses requiring more than 40 bits */
 	if (rte_mem_check_dma_mask(40)) {
-		RTE_LOG(ERR, PMD,
-			"device %s can not be used: restricted dma mask to 40 bits!\n",
-			pci_dev->device.name);
+		PMD_DRV_LOG(ERR,
+			    "device %s can not be used: restricted dma mask to 40 bits!",
+			    pci_dev->device.name);
 		return -ENODEV;
 	}
 
diff --git a/drivers/net/nfp/nfp_ethdev_vf.c b/drivers/net/nfp/nfp_ethdev_vf.c
index 07a2e17ef8e7..6272b4c29466 100644
--- a/drivers/net/nfp/nfp_ethdev_vf.c
+++ b/drivers/net/nfp/nfp_ethdev_vf.c
@@ -293,9 +293,9 @@ nfp_netvf_init(struct rte_eth_dev *eth_dev)
 
 	/* NFP can not handle DMA addresses requiring more than 40 bits */
 	if (rte_mem_check_dma_mask(40)) {
-		RTE_LOG(ERR, PMD,
-			"device %s can not be used: restricted dma mask to 40 bits!\n",
-			pci_dev->device.name);
+		PMD_DRV_LOG(ERR,
+			    "device %s can not be used: restricted dma mask to 40 bits!\n",
+			    pci_dev->device.name);
 		return -ENODEV;
 	}
 
diff --git a/drivers/net/nfp/nfp_logs.h b/drivers/net/nfp/nfp_logs.h
index b7632ee72ca1..58dd1da12143 100644
--- a/drivers/net/nfp/nfp_logs.h
+++ b/drivers/net/nfp/nfp_logs.h
@@ -14,16 +14,23 @@ extern int nfp_logtype_init;
 		"%s(): " fmt "\n", __func__, ## args)
 #define PMD_INIT_FUNC_TRACE() PMD_INIT_LOG(DEBUG, " >>")
 
+extern int nfp_logtype_driver;
+#define PMD_DRV_LOG(level, fmt, args...) \
+	rte_log(RTE_LOG_ ## level, nfp_logtype_driver, \
+		"%s(): " fmt "\n", __func__, ## args)
+
+#define PMD_DP_LOG(level, fmt, args...) \
+	rte_log_dp(RTE_LOG_ ## level, nfp_logtype_driver, \
+		   "%s(): " fmt "\n", __func__, ## args)
+
 #ifdef RTE_ETHDEV_DEBUG_RX
-#define PMD_RX_LOG(level, fmt, args...) \
-	RTE_LOG(level, PMD, "%s() rx: " fmt "\n", __func__, ## args)
+#define PMD_RX_LOG(level, fmt, args...) PMD_DP_LOG(level, "rx: " fmt, ## args)
 #else
 #define PMD_RX_LOG(level, fmt, args...) do { } while (0)
 #endif
 
 #ifdef RTE_ETHDEV_DEBUG_TX
-#define PMD_TX_LOG(level, fmt, args...) \
-	RTE_LOG(level, PMD, "%s() tx: " fmt "\n", __func__, ## args)
+#define PMD_TX_LOG(level, fmt, args...) PMD_DP_LOG(level, "tx: " fmt, ## args)
 #else
 #define PMD_TX_LOG(level, fmt, args...) do { } while (0)
 #endif
@@ -33,9 +40,4 @@ extern int nfp_logtype_cpp;
 	rte_log(RTE_LOG_ ## level, nfp_logtype_cpp, \
 		"%s(): " fmt "\n", __func__, ## args)
 
-extern int nfp_logtype_driver;
-#define PMD_DRV_LOG(level, fmt, args...) \
-	rte_log(RTE_LOG_ ## level, nfp_logtype_driver, \
-		"%s(): " fmt "\n", __func__, ## args)
-
 #endif /* _NFP_LOGS_H_ */
diff --git a/drivers/net/nfp/nfp_rxtx.c b/drivers/net/nfp/nfp_rxtx.c
index cfc1a784b185..e5a3dc57ac38 100644
--- a/drivers/net/nfp/nfp_rxtx.c
+++ b/drivers/net/nfp/nfp_rxtx.c
@@ -353,7 +353,7 @@ nfp_net_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 		 * DPDK just checks the queue is lower than max queues
 		 * enabled. But the queue needs to be configured
 		 */
-		RTE_LOG_DP(ERR, PMD, "RX Bad queue\n");
+		PMD_DP_LOG(ERR, "RX Bad queue");
 		return avail;
 	}
 
@@ -363,7 +363,7 @@ nfp_net_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 	while (avail < nb_pkts) {
 		rxb = &rxq->rxbufs[rxq->rd_p];
 		if (unlikely(rxb == NULL)) {
-			RTE_LOG_DP(ERR, PMD, "rxb does not exist!\n");
+			PMD_DP_LOG(ERR, "rxb does not exist!");
 			break;
 		}
 
@@ -383,9 +383,9 @@ nfp_net_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 		 */
 		new_mb = rte_pktmbuf_alloc(rxq->mem_pool);
 		if (unlikely(new_mb == NULL)) {
-			RTE_LOG_DP(DEBUG, PMD,
-			"RX mbuf alloc failed port_id=%u queue_id=%u\n",
-				rxq->port_id, (unsigned int)rxq->qidx);
+			PMD_DP_LOG(DEBUG,
+				   "RX mbuf alloc failed port_id=%u queue_id=%u\n",
+				   rxq->port_id, (unsigned int)rxq->qidx);
 			nfp_net_mbuf_alloc_failed(rxq);
 			break;
 		}
@@ -412,15 +412,15 @@ nfp_net_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 			 * responsibility of avoiding it. But we have
 			 * to give some info about the error
 			 */
-			RTE_LOG_DP(ERR, PMD,
-				"mbuf overflow likely due to the RX offset.\n"
-				"\t\tYour mbuf size should have extra space for"
-				" RX offset=%u bytes.\n"
-				"\t\tCurrently you just have %u bytes available"
-				" but the received packet is %u bytes long",
-				hw->rx_offset,
-				rxq->mbuf_size - hw->rx_offset,
-				mb->data_len);
+			PMD_DP_LOG(ERR,
+				   "mbuf overflow likely due to the RX offset.\n"
+				   "\t\tYour mbuf size should have extra space for"
+				   " RX offset=%u bytes.\n"
+				   "\t\tCurrently you just have %u bytes available"
+				   " but the received packet is %u bytes long",
+				   hw->rx_offset,
+				   rxq->mbuf_size - hw->rx_offset,
+				   mb->data_len);
 			rte_pktmbuf_free(mb);
 			break;
 		}