From: Stephen Hemminger <stephen@networkplumber.org>
To: Ferruh Yigit <ferruh.yigit@amd.com>
Cc: Chaoyong He <chaoyong.he@corigine.com>,
"dev@dpdk.org" <dev@dpdk.org>,
oss-drivers <oss-drivers@corigine.com>,
Niklas Soderlund <niklas.soderlund@corigine.com>
Subject: Re: [PATCH 2/3] net/nfp: get rid of the usage of RTE log level type
Date: Mon, 20 Feb 2023 08:16:41 -0800 [thread overview]
Message-ID: <20230220081641.623a2a97@hermes.local> (raw)
In-Reply-To: <357ee243-d0d1-37fb-f7f3-ba4d99a001ed@amd.com>
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;
}
next prev parent reply other threads:[~2023-02-20 16:16 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-17 2:45 [PATCH 0/3] refactor the nfp log subsystem Chaoyong He
2023-02-17 2:45 ` [PATCH 1/3] net/nfp: add the log source file Chaoyong He
2023-02-17 2:45 ` [PATCH 2/3] net/nfp: get rid of the usage of RTE log level type Chaoyong He
2023-02-17 13:59 ` Ferruh Yigit
2023-02-20 1:36 ` Chaoyong He
2023-02-20 10:09 ` Ferruh Yigit
2023-02-20 16:16 ` Stephen Hemminger [this message]
2023-02-20 16:32 ` Ferruh Yigit
2023-02-17 2:45 ` [PATCH 3/3] net/nfp: get rid of the usage of RTE log macro Chaoyong He
2023-02-20 12:03 ` [PATCH 0/3] refactor the nfp log subsystem Ferruh Yigit
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230220081641.623a2a97@hermes.local \
--to=stephen@networkplumber.org \
--cc=chaoyong.he@corigine.com \
--cc=dev@dpdk.org \
--cc=ferruh.yigit@amd.com \
--cc=niklas.soderlund@corigine.com \
--cc=oss-drivers@corigine.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).