DPDK patches and discussions
 help / color / mirror / Atom feed
* [RFT] net/nfb: use dynamic logtype
@ 2023-12-06 17:51 Stephen Hemminger
  2023-12-07 10:37 ` Martin Spinler
  2023-12-07 18:56 ` [PATCH 0/3] net/nfb: driver cleanups Stephen Hemminger
  0 siblings, 2 replies; 12+ messages in thread
From: Stephen Hemminger @ 2023-12-06 17:51 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, Martin Spinler, Rastislav Cernay

All drivers should be using dynamic logtype.
This should have been caught during initial driver review.

Since this driver requires non-standard external library this
patch can not be tested by me.

Fixes: 6435f9a0ac22 ("net/nfb: add new netcope driver")
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/nfb/nfb_ethdev.c | 22 ++++++++++------------
 drivers/net/nfb/nfb_log.h    | 13 +++++++++++++
 drivers/net/nfb/nfb_rx.c     |  9 +++++----
 drivers/net/nfb/nfb_rx.h     |  2 +-
 drivers/net/nfb/nfb_tx.c     |  8 ++++----
 drivers/net/nfb/nfb_tx.h     |  2 +-
 6 files changed, 34 insertions(+), 22 deletions(-)
 create mode 100644 drivers/net/nfb/nfb_log.h

diff --git a/drivers/net/nfb/nfb_ethdev.c b/drivers/net/nfb/nfb_ethdev.c
index defd118bd0ee..6c72ac101241 100644
--- a/drivers/net/nfb/nfb_ethdev.c
+++ b/drivers/net/nfb/nfb_ethdev.c
@@ -12,6 +12,7 @@
 #include <ethdev_pci.h>
 #include <rte_kvargs.h>
 
+#include "nfb_log.h"
 #include "nfb_stats.h"
 #include "nfb_rx.h"
 #include "nfb_tx.h"
@@ -192,8 +193,7 @@ nfb_eth_dev_configure(struct rte_eth_dev *dev __rte_unused)
 				(&nfb_timestamp_dynfield_offset,
 				&nfb_timestamp_rx_dynflag);
 		if (ret != 0) {
-			RTE_LOG(ERR, PMD, "Cannot register Rx timestamp"
-					" field/flag %d\n", ret);
+			NFB_LOG(ERR, "Cannot register Rx timestamp field/flag %d", ret);
 			nfb_close(internals->nfb);
 			return -rte_errno;
 		}
@@ -520,7 +520,7 @@ nfb_eth_dev_init(struct rte_eth_dev *dev)
 	struct rte_ether_addr eth_addr_init;
 	struct rte_kvargs *kvlist;
 
-	RTE_LOG(INFO, PMD, "Initializing NFB device (" PCI_PRI_FMT ")\n",
+	NFB_LOG(INFO, "Initializing NFB device (" PCI_PRI_FMT ")",
 		pci_addr->domain, pci_addr->bus, pci_addr->devid,
 		pci_addr->function);
 
@@ -536,7 +536,7 @@ nfb_eth_dev_init(struct rte_eth_dev *dev)
 		kvlist = rte_kvargs_parse(dev->device->devargs->args,
 						VALID_KEYS);
 		if (kvlist == NULL) {
-			RTE_LOG(ERR, PMD, "Failed to parse device arguments %s",
+			NFB_LOG(ERR, "Failed to parse device arguments %s",
 				dev->device->devargs->args);
 			rte_kvargs_free(kvlist);
 			return -EINVAL;
@@ -551,14 +551,13 @@ nfb_eth_dev_init(struct rte_eth_dev *dev)
 	 */
 	internals->nfb = nfb_open(internals->nfb_dev);
 	if (internals->nfb == NULL) {
-		RTE_LOG(ERR, PMD, "nfb_open(): failed to open %s",
-			internals->nfb_dev);
+		NFB_LOG(ERR, "nfb_open(): failed to open %s", internals->nfb_dev);
 		return -EINVAL;
 	}
 	data->nb_rx_queues = ndp_get_rx_queue_available_count(internals->nfb);
 	data->nb_tx_queues = ndp_get_tx_queue_available_count(internals->nfb);
 
-	RTE_LOG(INFO, PMD, "Available NDP queues RX: %u TX: %u\n",
+	NFB_LOG(INFO, "Available NDP queues RX: %u TX: %u",
 		data->nb_rx_queues, data->nb_tx_queues);
 
 	nfb_nc_rxmac_init(internals->nfb,
@@ -583,7 +582,7 @@ nfb_eth_dev_init(struct rte_eth_dev *dev)
 	data->mac_addrs = rte_zmalloc(data->name,
 		sizeof(struct rte_ether_addr) * mac_count, RTE_CACHE_LINE_SIZE);
 	if (data->mac_addrs == NULL) {
-		RTE_LOG(ERR, PMD, "Could not alloc space for MAC address!\n");
+		NFB_LOG(ERR, "Could not alloc space for MAC address");
 		nfb_close(internals->nfb);
 		return -EINVAL;
 	}
@@ -601,8 +600,7 @@ nfb_eth_dev_init(struct rte_eth_dev *dev)
 
 	dev->data->dev_flags |= RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS;
 
-	RTE_LOG(INFO, PMD, "NFB device ("
-		PCI_PRI_FMT ") successfully initialized\n",
+	NFB_LOG(INFO, "NFB device (" PCI_PRI_FMT ") successfully initialized",
 		pci_addr->domain, pci_addr->bus, pci_addr->devid,
 		pci_addr->function);
 
@@ -626,8 +624,7 @@ nfb_eth_dev_uninit(struct rte_eth_dev *dev)
 
 	nfb_eth_dev_close(dev);
 
-	RTE_LOG(INFO, PMD, "NFB device ("
-		PCI_PRI_FMT ") successfully uninitialized\n",
+	NFB_LOG(INFO,"NFB device (" PCI_PRI_FMT ") successfully uninitialized",
 		pci_addr->domain, pci_addr->bus, pci_addr->devid,
 		pci_addr->function);
 
@@ -690,3 +687,4 @@ static struct rte_pci_driver nfb_eth_driver = {
 RTE_PMD_REGISTER_PCI(RTE_NFB_DRIVER_NAME, nfb_eth_driver);
 RTE_PMD_REGISTER_PCI_TABLE(RTE_NFB_DRIVER_NAME, nfb_pci_id_table);
 RTE_PMD_REGISTER_KMOD_DEP(RTE_NFB_DRIVER_NAME, "* nfb");
+RTE_LOG_REGISTER_DEFAULT(nfb_logtype, NOTICE);
diff --git a/drivers/net/nfb/nfb_log.h b/drivers/net/nfb/nfb_log.h
new file mode 100644
index 000000000000..fac66a38d4b3
--- /dev/null
+++ b/drivers/net/nfb/nfb_log.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ */
+
+#ifndef _NFB_STATS_H_
+#define _NFB_STATS_H_
+
+extern int nfb_logtype;
+
+#define NFB_LOG(level, fmt, args...) \
+	rte_log(RTE_LOG_ ## level, nfb_logtype, "%s(): " fmt "\n", \
+		__func__, ## args)
+
+#endif /* _NFB_STATS_H_ */
diff --git a/drivers/net/nfb/nfb_rx.c b/drivers/net/nfb/nfb_rx.c
index 8a9b232305f2..e39592d04737 100644
--- a/drivers/net/nfb/nfb_rx.c
+++ b/drivers/net/nfb/nfb_rx.c
@@ -6,6 +6,7 @@
 
 #include <rte_kvargs.h>
 
+#include "nfb_log.h"
 #include "nfb_rx.h"
 #include "nfb.h"
 
@@ -19,7 +20,7 @@ nfb_eth_rx_queue_start(struct rte_eth_dev *dev, uint16_t rxq_id)
 	int ret;
 
 	if (rxq->queue == NULL) {
-		RTE_LOG(ERR, PMD, "RX NDP queue is NULL!\n");
+		NFP_LOG(ERR, "RX NDP queue is NULL");
 		return -EINVAL;
 	}
 
@@ -40,7 +41,7 @@ nfb_eth_rx_queue_stop(struct rte_eth_dev *dev, uint16_t rxq_id)
 	int ret;
 
 	if (rxq->queue == NULL) {
-		RTE_LOG(ERR, PMD, "RX NDP queue is NULL!\n");
+		NFB_LOG(ERR, "RX NDP queue is NULL");
 		return -EINVAL;
 	}
 
@@ -70,8 +71,8 @@ nfb_eth_rx_queue_setup(struct rte_eth_dev *dev,
 			RTE_CACHE_LINE_SIZE, socket_id);
 
 	if (rxq == NULL) {
-		RTE_LOG(ERR, PMD, "rte_zmalloc_socket() failed for rx queue id "
-				"%" PRIu16 "!\n", rx_queue_id);
+		NFB_LOG(ERR, "rte_zmalloc_socket() failed for rx queue id %" PRIu16,
+			rx_queue_id);
 		return -ENOMEM;
 	}
 
diff --git a/drivers/net/nfb/nfb_rx.h b/drivers/net/nfb/nfb_rx.h
index b618682e1393..2802f17091a0 100644
--- a/drivers/net/nfb/nfb_rx.h
+++ b/drivers/net/nfb/nfb_rx.h
@@ -156,7 +156,7 @@ nfb_eth_ndp_rx(void *queue,
 	struct rte_mbuf *mbufs[nb_pkts];
 
 	if (unlikely(ndp->queue == NULL || nb_pkts == 0)) {
-		RTE_LOG(ERR, PMD, "RX invalid arguments!\n");
+		NFB_LOG(ERR, "RX invalid arguments");
 		return 0;
 	}
 
diff --git a/drivers/net/nfb/nfb_tx.c b/drivers/net/nfb/nfb_tx.c
index d49fc324e76b..3fdfb66a969e 100644
--- a/drivers/net/nfb/nfb_tx.c
+++ b/drivers/net/nfb/nfb_tx.c
@@ -14,7 +14,7 @@ nfb_eth_tx_queue_start(struct rte_eth_dev *dev, uint16_t txq_id)
 	int ret;
 
 	if (txq->queue == NULL) {
-		RTE_LOG(ERR, PMD, "RX NDP queue is NULL!\n");
+		NFB_LOG(ERR, "RX NDP queue is NULL");
 		return -EINVAL;
 	}
 
@@ -35,7 +35,7 @@ nfb_eth_tx_queue_stop(struct rte_eth_dev *dev, uint16_t txq_id)
 	int ret;
 
 	if (txq->queue == NULL) {
-		RTE_LOG(ERR, PMD, "TX NDP queue is NULL!\n");
+		NFB_LOG(ERR, "TX NDP queue is NULL");
 		return -EINVAL;
 	}
 
@@ -62,8 +62,8 @@ nfb_eth_tx_queue_setup(struct rte_eth_dev *dev,
 		RTE_CACHE_LINE_SIZE, socket_id);
 
 	if (txq == NULL) {
-		RTE_LOG(ERR, PMD, "rte_zmalloc_socket() failed for tx queue id "
-			"%" PRIu16 "!\n", tx_queue_id);
+		NFB_LOG(ERR, "rte_zmalloc_socket() failed for tx queue id %" PRIu16,
+			tx_queue_id);
 		return -ENOMEM;
 	}
 
diff --git a/drivers/net/nfb/nfb_tx.h b/drivers/net/nfb/nfb_tx.h
index 910020e9e96f..f107cf914bbd 100644
--- a/drivers/net/nfb/nfb_tx.h
+++ b/drivers/net/nfb/nfb_tx.h
@@ -140,7 +140,7 @@ nfb_eth_ndp_tx(void *queue,
 		return 0;
 
 	if (unlikely(ndp->queue == NULL)) {
-		RTE_LOG(ERR, PMD, "TX invalid arguments!\n");
+		NFB_LOG(ERR, "TX invalid arguments");
 		return 0;
 	}
 
-- 
2.42.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFT] net/nfb: use dynamic logtype
  2023-12-06 17:51 [RFT] net/nfb: use dynamic logtype Stephen Hemminger
@ 2023-12-07 10:37 ` Martin Spinler
  2023-12-07 17:32   ` Stephen Hemminger
  2023-12-07 18:56 ` [PATCH 0/3] net/nfb: driver cleanups Stephen Hemminger
  1 sibling, 1 reply; 12+ messages in thread
From: Martin Spinler @ 2023-12-07 10:37 UTC (permalink / raw)
  To: Stephen Hemminger, dev; +Cc: Rastislav Cernay

Thanks for patch! There are some issues.

On Wed, 2023-12-06 at 09:51 -0800, Stephen Hemminger wrote:
> 
> diff --git a/drivers/net/nfb/nfb_log.h b/drivers/net/nfb/nfb_log.h
> new file mode 100644
> index 000000000000..fac66a38d4b3
> --- /dev/null
> +++ b/drivers/net/nfb/nfb_log.h
> @@ -0,0 +1,13 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + */
> +
> +#ifndef _NFB_STATS_H_
> +#define _NFB_STATS_H_

use the _NFB_LOG_H_ guards

> +
> +extern int nfb_logtype;
> +
> +#define NFB_LOG(level, fmt, args...) \
> +	rte_log(RTE_LOG_ ## level, nfb_logtype, "%s(): " fmt "\n", \
> +		__func__, ## args)
> +
> +#endif /* _NFB_STATS_H_ */

use the _NFB_LOG_H_ guard

> diff --git a/drivers/net/nfb/nfb_rx.c b/drivers/net/nfb/nfb_rx.c
> index 8a9b232305f2..e39592d04737 100644
> --- a/drivers/net/nfb/nfb_rx.c
> +++ b/drivers/net/nfb/nfb_rx.c
>  
> @@ -19,7 +20,7 @@ nfb_eth_rx_queue_start(struct rte_eth_dev *dev, uint16_t rxq_id)
>  	int ret;
>  
>  	if (rxq->queue == NULL) {
> -		RTE_LOG(ERR, PMD, "RX NDP queue is NULL!\n");
> +		NFP_LOG(ERR, "RX NDP queue is NULL");

typo, should be NFB_LOG instead of NFP_LOG

>  		return -EINVAL;
>  	}
>  
>  

Also, the nfb_rx.h and nfb_tx.h files use the macro NFB_LOG inside,
please add '#include "nfb_log.h"' into them (then the include in
nfb_rx.c will be duplicate). Otherwise, all .c sources, which include
main nfb.h, don't compile.

With these changes, the driver works.

Thank you!
Martin

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFT] net/nfb: use dynamic logtype
  2023-12-07 10:37 ` Martin Spinler
@ 2023-12-07 17:32   ` Stephen Hemminger
  0 siblings, 0 replies; 12+ messages in thread
From: Stephen Hemminger @ 2023-12-07 17:32 UTC (permalink / raw)
  To: Martin Spinler; +Cc: dev, Rastislav Cernay

On Thu, 07 Dec 2023 11:37:52 +0100
Martin Spinler <spinler@cesnet.cz> wrote:

> Also, the nfb_rx.h and nfb_tx.h files use the macro NFB_LOG inside,
> please add '#include "nfb_log.h"' into them (then the include in
> nfb_rx.c will be duplicate). Otherwise, all .c sources, which include
> main nfb.h, don't compile.
> 
> With these changes, the driver works.
> 
> Thank you!
> Martin

I found a few more leftover bits in nfb driver.
Will make a new patchset, and it looks like can build test in a
Redhat VM.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 0/3] net/nfb: driver cleanups
  2023-12-06 17:51 [RFT] net/nfb: use dynamic logtype Stephen Hemminger
  2023-12-07 10:37 ` Martin Spinler
@ 2023-12-07 18:56 ` Stephen Hemminger
  2023-12-07 18:56   ` [PATCH 1/3] net/nfb: remove unused device args Stephen Hemminger
                     ` (4 more replies)
  1 sibling, 5 replies; 12+ messages in thread
From: Stephen Hemminger @ 2023-12-07 18:56 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

Replace static logtype with dynamic logtype and
remove dead code. Compile tested on Fedora.

Stephen Hemminger (3):
  net/nfb: remove unused device args
  net/nfb: make device path local to init function
  net/nfb: use dynamic logtype

 drivers/net/nfb/nfb.h        | 10 ++++----
 drivers/net/nfb/nfb_ethdev.c | 44 ++++++++++--------------------------
 drivers/net/nfb/nfb_rx.c     |  9 ++++----
 drivers/net/nfb/nfb_rx.h     |  2 +-
 drivers/net/nfb/nfb_tx.c     |  9 ++++----
 drivers/net/nfb/nfb_tx.h     |  2 +-
 6 files changed, 27 insertions(+), 49 deletions(-)

-- 
2.42.0

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/3] net/nfb: remove unused device args
  2023-12-07 18:56 ` [PATCH 0/3] net/nfb: driver cleanups Stephen Hemminger
@ 2023-12-07 18:56   ` Stephen Hemminger
  2023-12-07 18:56   ` [PATCH 2/3] net/nfb: make device path local to init function Stephen Hemminger
                     ` (3 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Stephen Hemminger @ 2023-12-07 18:56 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, Martin Spinler

The driver has no entries in VALID_KEYS array so there are
no device args. And after parsing it just frees the result.
Looks like it was copy/pasted from some other driver.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/nfb/nfb.h        |  2 --
 drivers/net/nfb/nfb_ethdev.c | 16 ----------------
 2 files changed, 18 deletions(-)

diff --git a/drivers/net/nfb/nfb.h b/drivers/net/nfb/nfb.h
index 7dc5bd29e44c..21cd8f78f641 100644
--- a/drivers/net/nfb/nfb.h
+++ b/drivers/net/nfb/nfb.h
@@ -36,8 +36,6 @@
 
 #define RTE_NFB_DRIVER_NAME net_nfb
 
-/* Device arguments */
-static const char * const VALID_KEYS[] = {NULL};
 
 struct pmd_internals {
 	uint16_t         max_rxmac;
diff --git a/drivers/net/nfb/nfb_ethdev.c b/drivers/net/nfb/nfb_ethdev.c
index defd118bd0ee..4c4e2e3273e6 100644
--- a/drivers/net/nfb/nfb_ethdev.c
+++ b/drivers/net/nfb/nfb_ethdev.c
@@ -518,7 +518,6 @@ nfb_eth_dev_init(struct rte_eth_dev *dev)
 	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
 	struct rte_pci_addr *pci_addr = &pci_dev->addr;
 	struct rte_ether_addr eth_addr_init;
-	struct rte_kvargs *kvlist;
 
 	RTE_LOG(INFO, PMD, "Initializing NFB device (" PCI_PRI_FMT ")\n",
 		pci_addr->domain, pci_addr->bus, pci_addr->devid,
@@ -529,21 +528,6 @@ nfb_eth_dev_init(struct rte_eth_dev *dev)
 		pci_addr->domain, pci_addr->bus, pci_addr->devid,
 		pci_addr->function);
 
-	/* Check validity of device args */
-	if (dev->device->devargs != NULL &&
-			dev->device->devargs->args != NULL &&
-			strlen(dev->device->devargs->args) > 0) {
-		kvlist = rte_kvargs_parse(dev->device->devargs->args,
-						VALID_KEYS);
-		if (kvlist == NULL) {
-			RTE_LOG(ERR, PMD, "Failed to parse device arguments %s",
-				dev->device->devargs->args);
-			rte_kvargs_free(kvlist);
-			return -EINVAL;
-		}
-		rte_kvargs_free(kvlist);
-	}
-
 	/*
 	 * Get number of available DMA RX and TX queues, which is maximum
 	 * number of queues that can be created and store it in private device
-- 
2.42.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 2/3] net/nfb: make device path local to init function
  2023-12-07 18:56 ` [PATCH 0/3] net/nfb: driver cleanups Stephen Hemminger
  2023-12-07 18:56   ` [PATCH 1/3] net/nfb: remove unused device args Stephen Hemminger
@ 2023-12-07 18:56   ` Stephen Hemminger
  2023-12-07 18:56   ` [PATCH 3/3] net/nfb: use dynamic logtype Stephen Hemminger
                     ` (2 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Stephen Hemminger @ 2023-12-07 18:56 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, Martin Spinler

The device path is only used to call nfb_open() it does
not have to be stored in internal structure.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/nfb/nfb.h        | 2 --
 drivers/net/nfb/nfb_ethdev.c | 8 ++++----
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/net/nfb/nfb.h b/drivers/net/nfb/nfb.h
index 21cd8f78f641..2707f3db6240 100644
--- a/drivers/net/nfb/nfb.h
+++ b/drivers/net/nfb/nfb.h
@@ -42,8 +42,6 @@ struct pmd_internals {
 	uint16_t         max_txmac;
 	struct nc_rxmac *rxmac[RTE_MAX_NC_RXMAC];
 	struct nc_txmac *txmac[RTE_MAX_NC_TXMAC];
-
-	char             nfb_dev[PATH_MAX];
 	struct nfb_device *nfb;
 };
 
diff --git a/drivers/net/nfb/nfb_ethdev.c b/drivers/net/nfb/nfb_ethdev.c
index 4c4e2e3273e6..892abe81f3d6 100644
--- a/drivers/net/nfb/nfb_ethdev.c
+++ b/drivers/net/nfb/nfb_ethdev.c
@@ -518,12 +518,13 @@ nfb_eth_dev_init(struct rte_eth_dev *dev)
 	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
 	struct rte_pci_addr *pci_addr = &pci_dev->addr;
 	struct rte_ether_addr eth_addr_init;
+	char nfb_dev[PATH_MAX];
 
 	RTE_LOG(INFO, PMD, "Initializing NFB device (" PCI_PRI_FMT ")\n",
 		pci_addr->domain, pci_addr->bus, pci_addr->devid,
 		pci_addr->function);
 
-	snprintf(internals->nfb_dev, PATH_MAX,
+	snprintf(nfb_dev, sizeof(nfb_dev),
 		"/dev/nfb/by-pci-slot/" PCI_PRI_FMT,
 		pci_addr->domain, pci_addr->bus, pci_addr->devid,
 		pci_addr->function);
@@ -533,10 +534,9 @@ nfb_eth_dev_init(struct rte_eth_dev *dev)
 	 * number of queues that can be created and store it in private device
 	 * data structure.
 	 */
-	internals->nfb = nfb_open(internals->nfb_dev);
+	internals->nfb = nfb_open(nfb_dev);
 	if (internals->nfb == NULL) {
-		RTE_LOG(ERR, PMD, "nfb_open(): failed to open %s",
-			internals->nfb_dev);
+		RTE_LOG(ERR, PMD, "nfb_open(): failed to open %s", nfb_dev);
 		return -EINVAL;
 	}
 	data->nb_rx_queues = ndp_get_rx_queue_available_count(internals->nfb);
-- 
2.42.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 3/3] net/nfb: use dynamic logtype
  2023-12-07 18:56 ` [PATCH 0/3] net/nfb: driver cleanups Stephen Hemminger
  2023-12-07 18:56   ` [PATCH 1/3] net/nfb: remove unused device args Stephen Hemminger
  2023-12-07 18:56   ` [PATCH 2/3] net/nfb: make device path local to init function Stephen Hemminger
@ 2023-12-07 18:56   ` Stephen Hemminger
  2023-12-12  8:49     ` Martin Spinler
  2024-01-12 12:16   ` [PATCH 0/3] net/nfb: driver cleanups Ferruh Yigit
  2024-02-08 12:01   ` Ferruh Yigit
  4 siblings, 1 reply; 12+ messages in thread
From: Stephen Hemminger @ 2023-12-07 18:56 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, Martin Spinler, Rastislav Cernay

All drivers should be using dynamic logtype.

Fixes: 6435f9a0ac22 ("net/nfb: add new netcope driver")
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/nfb/nfb.h        |  6 +++++-
 drivers/net/nfb/nfb_ethdev.c | 22 +++++++++-------------
 drivers/net/nfb/nfb_rx.c     |  9 ++++-----
 drivers/net/nfb/nfb_rx.h     |  2 +-
 drivers/net/nfb/nfb_tx.c     |  9 ++++-----
 drivers/net/nfb/nfb_tx.h     |  2 +-
 6 files changed, 24 insertions(+), 26 deletions(-)

diff --git a/drivers/net/nfb/nfb.h b/drivers/net/nfb/nfb.h
index 2707f3db6240..3a27457d6e33 100644
--- a/drivers/net/nfb/nfb.h
+++ b/drivers/net/nfb/nfb.h
@@ -12,6 +12,11 @@
 #include <netcope/rxmac.h>
 #include <netcope/txmac.h>
 
+extern int nfb_logtype;
+#define NFB_LOG(level, fmt, args...) \
+	rte_log(RTE_LOG_ ## level, nfb_logtype, "%s(): " fmt "\n", \
+		__func__, ## args)
+
 #include "nfb_rx.h"
 #include "nfb_tx.h"
 
@@ -36,7 +41,6 @@
 
 #define RTE_NFB_DRIVER_NAME net_nfb
 
-
 struct pmd_internals {
 	uint16_t         max_rxmac;
 	uint16_t         max_txmac;
diff --git a/drivers/net/nfb/nfb_ethdev.c b/drivers/net/nfb/nfb_ethdev.c
index 892abe81f3d6..fc510db7390c 100644
--- a/drivers/net/nfb/nfb_ethdev.c
+++ b/drivers/net/nfb/nfb_ethdev.c
@@ -12,11 +12,9 @@
 #include <ethdev_pci.h>
 #include <rte_kvargs.h>
 
+#include "nfb.h"
 #include "nfb_stats.h"
-#include "nfb_rx.h"
-#include "nfb_tx.h"
 #include "nfb_rxmode.h"
-#include "nfb.h"
 
 /**
  * Default MAC addr
@@ -192,8 +190,7 @@ nfb_eth_dev_configure(struct rte_eth_dev *dev __rte_unused)
 				(&nfb_timestamp_dynfield_offset,
 				&nfb_timestamp_rx_dynflag);
 		if (ret != 0) {
-			RTE_LOG(ERR, PMD, "Cannot register Rx timestamp"
-					" field/flag %d\n", ret);
+			NFB_LOG(ERR, "Cannot register Rx timestamp field/flag %d", ret);
 			nfb_close(internals->nfb);
 			return -rte_errno;
 		}
@@ -520,7 +517,7 @@ nfb_eth_dev_init(struct rte_eth_dev *dev)
 	struct rte_ether_addr eth_addr_init;
 	char nfb_dev[PATH_MAX];
 
-	RTE_LOG(INFO, PMD, "Initializing NFB device (" PCI_PRI_FMT ")\n",
+	NFB_LOG(INFO, "Initializing NFB device (" PCI_PRI_FMT ")",
 		pci_addr->domain, pci_addr->bus, pci_addr->devid,
 		pci_addr->function);
 
@@ -536,13 +533,13 @@ nfb_eth_dev_init(struct rte_eth_dev *dev)
 	 */
 	internals->nfb = nfb_open(nfb_dev);
 	if (internals->nfb == NULL) {
-		RTE_LOG(ERR, PMD, "nfb_open(): failed to open %s", nfb_dev);
+		NFB_LOG(ERR, "nfb_open(): failed to open %s", nfb_dev);
 		return -EINVAL;
 	}
 	data->nb_rx_queues = ndp_get_rx_queue_available_count(internals->nfb);
 	data->nb_tx_queues = ndp_get_tx_queue_available_count(internals->nfb);
 
-	RTE_LOG(INFO, PMD, "Available NDP queues RX: %u TX: %u\n",
+	NFB_LOG(INFO, "Available NDP queues RX: %u TX: %u\n",
 		data->nb_rx_queues, data->nb_tx_queues);
 
 	nfb_nc_rxmac_init(internals->nfb,
@@ -567,7 +564,7 @@ nfb_eth_dev_init(struct rte_eth_dev *dev)
 	data->mac_addrs = rte_zmalloc(data->name,
 		sizeof(struct rte_ether_addr) * mac_count, RTE_CACHE_LINE_SIZE);
 	if (data->mac_addrs == NULL) {
-		RTE_LOG(ERR, PMD, "Could not alloc space for MAC address!\n");
+		NFB_LOG(ERR, "Could not alloc space for MAC address");
 		nfb_close(internals->nfb);
 		return -EINVAL;
 	}
@@ -585,8 +582,7 @@ nfb_eth_dev_init(struct rte_eth_dev *dev)
 
 	dev->data->dev_flags |= RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS;
 
-	RTE_LOG(INFO, PMD, "NFB device ("
-		PCI_PRI_FMT ") successfully initialized\n",
+	NFB_LOG(INFO, "NFB device (" PCI_PRI_FMT ") successfully initialized",
 		pci_addr->domain, pci_addr->bus, pci_addr->devid,
 		pci_addr->function);
 
@@ -610,8 +606,7 @@ nfb_eth_dev_uninit(struct rte_eth_dev *dev)
 
 	nfb_eth_dev_close(dev);
 
-	RTE_LOG(INFO, PMD, "NFB device ("
-		PCI_PRI_FMT ") successfully uninitialized\n",
+	NFB_LOG(INFO, "NFB device (" PCI_PRI_FMT ") successfully uninitialized",
 		pci_addr->domain, pci_addr->bus, pci_addr->devid,
 		pci_addr->function);
 
@@ -674,3 +669,4 @@ static struct rte_pci_driver nfb_eth_driver = {
 RTE_PMD_REGISTER_PCI(RTE_NFB_DRIVER_NAME, nfb_eth_driver);
 RTE_PMD_REGISTER_PCI_TABLE(RTE_NFB_DRIVER_NAME, nfb_pci_id_table);
 RTE_PMD_REGISTER_KMOD_DEP(RTE_NFB_DRIVER_NAME, "* nfb");
+RTE_LOG_REGISTER_DEFAULT(nfb_logtype, NOTICE);
diff --git a/drivers/net/nfb/nfb_rx.c b/drivers/net/nfb/nfb_rx.c
index 8a9b232305f2..a20f7b9b4b41 100644
--- a/drivers/net/nfb/nfb_rx.c
+++ b/drivers/net/nfb/nfb_rx.c
@@ -6,7 +6,6 @@
 
 #include <rte_kvargs.h>
 
-#include "nfb_rx.h"
 #include "nfb.h"
 
 uint64_t nfb_timestamp_rx_dynflag;
@@ -19,7 +18,7 @@ nfb_eth_rx_queue_start(struct rte_eth_dev *dev, uint16_t rxq_id)
 	int ret;
 
 	if (rxq->queue == NULL) {
-		RTE_LOG(ERR, PMD, "RX NDP queue is NULL!\n");
+		NFB_LOG(ERR, "RX NDP queue is NULL");
 		return -EINVAL;
 	}
 
@@ -40,7 +39,7 @@ nfb_eth_rx_queue_stop(struct rte_eth_dev *dev, uint16_t rxq_id)
 	int ret;
 
 	if (rxq->queue == NULL) {
-		RTE_LOG(ERR, PMD, "RX NDP queue is NULL!\n");
+		NFB_LOG(ERR, "RX NDP queue is NULL");
 		return -EINVAL;
 	}
 
@@ -70,8 +69,8 @@ nfb_eth_rx_queue_setup(struct rte_eth_dev *dev,
 			RTE_CACHE_LINE_SIZE, socket_id);
 
 	if (rxq == NULL) {
-		RTE_LOG(ERR, PMD, "rte_zmalloc_socket() failed for rx queue id "
-				"%" PRIu16 "!\n", rx_queue_id);
+		NFB_LOG(ERR, "rte_zmalloc_socket() failed for rx queue id %" PRIu16,
+			rx_queue_id);
 		return -ENOMEM;
 	}
 
diff --git a/drivers/net/nfb/nfb_rx.h b/drivers/net/nfb/nfb_rx.h
index b618682e1393..2802f17091a0 100644
--- a/drivers/net/nfb/nfb_rx.h
+++ b/drivers/net/nfb/nfb_rx.h
@@ -156,7 +156,7 @@ nfb_eth_ndp_rx(void *queue,
 	struct rte_mbuf *mbufs[nb_pkts];
 
 	if (unlikely(ndp->queue == NULL || nb_pkts == 0)) {
-		RTE_LOG(ERR, PMD, "RX invalid arguments!\n");
+		NFB_LOG(ERR, "RX invalid arguments");
 		return 0;
 	}
 
diff --git a/drivers/net/nfb/nfb_tx.c b/drivers/net/nfb/nfb_tx.c
index d49fc324e76b..0cc2f596301a 100644
--- a/drivers/net/nfb/nfb_tx.c
+++ b/drivers/net/nfb/nfb_tx.c
@@ -4,7 +4,6 @@
  * All rights reserved.
  */
 
-#include "nfb_tx.h"
 #include "nfb.h"
 
 int
@@ -14,7 +13,7 @@ nfb_eth_tx_queue_start(struct rte_eth_dev *dev, uint16_t txq_id)
 	int ret;
 
 	if (txq->queue == NULL) {
-		RTE_LOG(ERR, PMD, "RX NDP queue is NULL!\n");
+		NFB_LOG(ERR, "RX NDP queue is NULL");
 		return -EINVAL;
 	}
 
@@ -35,7 +34,7 @@ nfb_eth_tx_queue_stop(struct rte_eth_dev *dev, uint16_t txq_id)
 	int ret;
 
 	if (txq->queue == NULL) {
-		RTE_LOG(ERR, PMD, "TX NDP queue is NULL!\n");
+		NFB_LOG(ERR, "TX NDP queue is NULL");
 		return -EINVAL;
 	}
 
@@ -62,8 +61,8 @@ nfb_eth_tx_queue_setup(struct rte_eth_dev *dev,
 		RTE_CACHE_LINE_SIZE, socket_id);
 
 	if (txq == NULL) {
-		RTE_LOG(ERR, PMD, "rte_zmalloc_socket() failed for tx queue id "
-			"%" PRIu16 "!\n", tx_queue_id);
+		NFB_LOG(ERR, "rte_zmalloc_socket() failed for tx queue id %" PRIu16,
+			tx_queue_id);
 		return -ENOMEM;
 	}
 
diff --git a/drivers/net/nfb/nfb_tx.h b/drivers/net/nfb/nfb_tx.h
index 910020e9e96f..f107cf914bbd 100644
--- a/drivers/net/nfb/nfb_tx.h
+++ b/drivers/net/nfb/nfb_tx.h
@@ -140,7 +140,7 @@ nfb_eth_ndp_tx(void *queue,
 		return 0;
 
 	if (unlikely(ndp->queue == NULL)) {
-		RTE_LOG(ERR, PMD, "TX invalid arguments!\n");
+		NFB_LOG(ERR, "TX invalid arguments");
 		return 0;
 	}
 
-- 
2.42.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3] net/nfb: use dynamic logtype
  2023-12-07 18:56   ` [PATCH 3/3] net/nfb: use dynamic logtype Stephen Hemminger
@ 2023-12-12  8:49     ` Martin Spinler
  0 siblings, 0 replies; 12+ messages in thread
From: Martin Spinler @ 2023-12-12  8:49 UTC (permalink / raw)
  To: Stephen Hemminger, dev

On Thu, 2023-12-07 at 10:56 -0800, Stephen Hemminger wrote:
> All drivers should be using dynamic logtype.
> 
> Fixes: 6435f9a0ac22 ("net/nfb: add new netcope driver")
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

Tested-by: Martin Spinler <spinler@cesnet.cz>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3] net/nfb: driver cleanups
  2023-12-07 18:56 ` [PATCH 0/3] net/nfb: driver cleanups Stephen Hemminger
                     ` (2 preceding siblings ...)
  2023-12-07 18:56   ` [PATCH 3/3] net/nfb: use dynamic logtype Stephen Hemminger
@ 2024-01-12 12:16   ` Ferruh Yigit
  2024-01-12 13:50     ` Martin Spinler
  2024-02-08 12:01   ` Ferruh Yigit
  4 siblings, 1 reply; 12+ messages in thread
From: Ferruh Yigit @ 2024-01-12 12:16 UTC (permalink / raw)
  To: Stephen Hemminger, Martin Spinler; +Cc: dev

On 12/7/2023 6:56 PM, Stephen Hemminger wrote:
> Replace static logtype with dynamic logtype and
> remove dead code. Compile tested on Fedora.
> 
> Stephen Hemminger (3):
>   net/nfb: remove unused device args
>   net/nfb: make device path local to init function
>   net/nfb: use dynamic logtype
> 
>  

Hi Martin,

Can you please review the set?

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3] net/nfb: driver cleanups
  2024-01-12 12:16   ` [PATCH 0/3] net/nfb: driver cleanups Ferruh Yigit
@ 2024-01-12 13:50     ` Martin Spinler
  2024-02-08  1:02       ` Ferruh Yigit
  0 siblings, 1 reply; 12+ messages in thread
From: Martin Spinler @ 2024-01-12 13:50 UTC (permalink / raw)
  To: Ferruh Yigit, Stephen Hemminger; +Cc: dev

Tested-by: Martin Spinler <spinler@cesnet.cz>
Acked-by: Martin Spinler <spinler@cesnet.cz>

---

Hi! Thanks for the cleanup. I've tested that patchset and works fine.

I'm just not sure, if the "net/nfb: use dynamic logtype" patch merges
with the "Remove uses of PMD logtype" series as they slightly differs
(both links below).
Stephen, would it make sense to remove the last patch from this series?

https://patchwork.dpdk.org/project/dpdk/patch/20231207185720.19913-4-stephen@networkplumber.org/
https://patchwork.dpdk.org/project/dpdk/patch/20231222171820.8778-9-stephen@networkplumber.org/


On Fri, 2024-01-12 at 12:16 +0000, Ferruh Yigit wrote:
> On 12/7/2023 6:56 PM, Stephen Hemminger wrote:
> > Replace static logtype with dynamic logtype and
> > remove dead code. Compile tested on Fedora.
> > 
> > Stephen Hemminger (3):
> >   net/nfb: remove unused device args
> >   net/nfb: make device path local to init function
> >   net/nfb: use dynamic logtype
> > 
> >  
> 
> Hi Martin,
> 
> Can you please review the set?


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3] net/nfb: driver cleanups
  2024-01-12 13:50     ` Martin Spinler
@ 2024-02-08  1:02       ` Ferruh Yigit
  0 siblings, 0 replies; 12+ messages in thread
From: Ferruh Yigit @ 2024-02-08  1:02 UTC (permalink / raw)
  To: Martin Spinler, Stephen Hemminger; +Cc: dev, Thomas Monjalon

On 1/12/2024 1:50 PM, Martin Spinler wrote:
> Tested-by: Martin Spinler <spinler@cesnet.cz>
> Acked-by: Martin Spinler <spinler@cesnet.cz>
> 
> ---
> 
> Hi! Thanks for the cleanup. I've tested that patchset and works fine.
> 
> I'm just not sure, if the "net/nfb: use dynamic logtype" patch merges
> with the "Remove uses of PMD logtype" series as they slightly differs
> (both links below).
> Stephen, would it make sense to remove the last patch from this series?
> 
> https://patchwork.dpdk.org/project/dpdk/patch/20231207185720.19913-4-stephen@networkplumber.org/
> https://patchwork.dpdk.org/project/dpdk/patch/20231222171820.8778-9-stephen@networkplumber.org/
> 

Second one is larger set with multiple components involved, this one is
more specific, I will proceed with this one.

@Thomas may drop the nfp patch in that series.

> 
> On Fri, 2024-01-12 at 12:16 +0000, Ferruh Yigit wrote:
>> On 12/7/2023 6:56 PM, Stephen Hemminger wrote:
>>> Replace static logtype with dynamic logtype and
>>> remove dead code. Compile tested on Fedora.
>>>
>>> Stephen Hemminger (3):
>>>   net/nfb: remove unused device args
>>>   net/nfb: make device path local to init function
>>>   net/nfb: use dynamic logtype
>>>
>>>  
>>
>> Hi Martin,
>>
>> Can you please review the set?
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3] net/nfb: driver cleanups
  2023-12-07 18:56 ` [PATCH 0/3] net/nfb: driver cleanups Stephen Hemminger
                     ` (3 preceding siblings ...)
  2024-01-12 12:16   ` [PATCH 0/3] net/nfb: driver cleanups Ferruh Yigit
@ 2024-02-08 12:01   ` Ferruh Yigit
  4 siblings, 0 replies; 12+ messages in thread
From: Ferruh Yigit @ 2024-02-08 12:01 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev

On 12/7/2023 6:56 PM, Stephen Hemminger wrote:
> Replace static logtype with dynamic logtype and
> remove dead code. Compile tested on Fedora.
> 
> Stephen Hemminger (3):
>   net/nfb: remove unused device args
>   net/nfb: make device path local to init function
>   net/nfb: use dynamic logtype
> 

Moving ack from other thread:
Tested-by: Martin Spinler <spinler@cesnet.cz>
Acked-by: Martin Spinler <spinler@cesnet.cz>


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


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2024-02-08 12:01 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-06 17:51 [RFT] net/nfb: use dynamic logtype Stephen Hemminger
2023-12-07 10:37 ` Martin Spinler
2023-12-07 17:32   ` Stephen Hemminger
2023-12-07 18:56 ` [PATCH 0/3] net/nfb: driver cleanups Stephen Hemminger
2023-12-07 18:56   ` [PATCH 1/3] net/nfb: remove unused device args Stephen Hemminger
2023-12-07 18:56   ` [PATCH 2/3] net/nfb: make device path local to init function Stephen Hemminger
2023-12-07 18:56   ` [PATCH 3/3] net/nfb: use dynamic logtype Stephen Hemminger
2023-12-12  8:49     ` Martin Spinler
2024-01-12 12:16   ` [PATCH 0/3] net/nfb: driver cleanups Ferruh Yigit
2024-01-12 13:50     ` Martin Spinler
2024-02-08  1:02       ` Ferruh Yigit
2024-02-08 12:01   ` Ferruh Yigit

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