DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH 0/3] remove usage of rte_panic() and ASSERT() from PMD
@ 2023-02-03  7:57 Chaoyong He
  2023-02-03  7:57 ` [PATCH 1/3] net/nfp: remove panic usage during reconfig Chaoyong He
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Chaoyong He @ 2023-02-03  7:57 UTC (permalink / raw)
  To: dev; +Cc: oss-drivers, niklas.soderlund, Chaoyong He

This patch series is aimed to allow the DPDK app to deal with
the error as necessary, by:
* Remove the usage of rte_panic()
* Remove the usage of ASSERT()

James Hershaw (3):
  net/nfp: remove panic usage during reconfig
  net/nfp: remove panic usage for multisegment Tx
  net/nfp: remove ASSERT() macro

 drivers/net/nfp/flower/nfp_flower.c | 11 ++++-------
 drivers/net/nfp/nfp_common.c        | 24 ++++++++++++------------
 drivers/net/nfp/nfp_logs.h          |  2 --
 drivers/net/nfp/nfp_rxtx.c          | 14 +++++---------
 4 files changed, 21 insertions(+), 30 deletions(-)

-- 
2.29.3


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

* [PATCH 1/3] net/nfp: remove panic usage during reconfig
  2023-02-03  7:57 [PATCH 0/3] remove usage of rte_panic() and ASSERT() from PMD Chaoyong He
@ 2023-02-03  7:57 ` Chaoyong He
  2023-02-03  7:57 ` [PATCH 2/3] net/nfp: remove panic usage for multisegment Tx Chaoyong He
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Chaoyong He @ 2023-02-03  7:57 UTC (permalink / raw)
  To: dev; +Cc: oss-drivers, niklas.soderlund, James Hershaw, Chaoyong He

From: James Hershaw <james.hershaw@corigine.com>

Remove usage of the rte_panic() function from the nfp PMD. It is better
to return error values and allow the application to handle the error.

Signed-off-by: James Hershaw <james.hershaw@corigine.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@corigine.com>
Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
---
 drivers/net/nfp/nfp_common.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/net/nfp/nfp_common.c b/drivers/net/nfp/nfp_common.c
index 4f21d9978d..7a60add3a6 100644
--- a/drivers/net/nfp/nfp_common.c
+++ b/drivers/net/nfp/nfp_common.c
@@ -63,8 +63,10 @@ __nfp_net_reconfig(struct nfp_net_hw *hw, uint32_t update)
 	PMD_DRV_LOG(DEBUG, "Writing to the configuration queue (%p)...",
 		    hw->qcp_cfg);
 
-	if (hw->qcp_cfg == NULL)
-		rte_panic("Bad configuration queue pointer\n");
+	if (hw->qcp_cfg == NULL) {
+		PMD_INIT_LOG(ERR, "Bad configuration queue pointer");
+		return -ENXIO;
+	}
 
 	nfp_qcp_ptr_add(hw->qcp_cfg, NFP_QCP_WRITE_PTR, 1);
 
@@ -85,7 +87,7 @@ __nfp_net_reconfig(struct nfp_net_hw *hw, uint32_t update)
 		if (cnt >= NFP_NET_POLL_TIMEOUT) {
 			PMD_INIT_LOG(ERR, "Reconfig timeout for 0x%08x after"
 					  " %dms", update, cnt);
-			rte_panic("Exiting\n");
+			return -EIO;
 		}
 		nanosleep(&wait, 0); /* waiting for a 1ms */
 	}
@@ -121,16 +123,14 @@ nfp_net_reconfig(struct nfp_net_hw *hw, uint32_t ctrl, uint32_t update)
 
 	rte_spinlock_unlock(&hw->reconfig_lock);
 
-	if (!err)
-		return 0;
+	if (err != 0) {
+		PMD_INIT_LOG(ERR, "Error nfp_net reconfig for ctrl: %x update: %x",
+			     ctrl, update);
+		return -EIO;
+	}
+
+	return 0;
 
-	/*
-	 * Reconfig errors imply situations where they can be handled.
-	 * Otherwise, rte_panic is called inside __nfp_net_reconfig
-	 */
-	PMD_INIT_LOG(ERR, "Error nfp_net reconfig for ctrl: %x update: %x",
-		     ctrl, update);
-	return -EIO;
 }
 
 /*
-- 
2.29.3


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

* [PATCH 2/3] net/nfp: remove panic usage for multisegment Tx
  2023-02-03  7:57 [PATCH 0/3] remove usage of rte_panic() and ASSERT() from PMD Chaoyong He
  2023-02-03  7:57 ` [PATCH 1/3] net/nfp: remove panic usage during reconfig Chaoyong He
@ 2023-02-03  7:57 ` Chaoyong He
  2023-02-03  7:57 ` [PATCH 3/3] net/nfp: remove ASSERT() macro Chaoyong He
  2023-02-03 18:57 ` [PATCH 0/3] remove usage of rte_panic() and ASSERT() from PMD Ferruh Yigit
  3 siblings, 0 replies; 5+ messages in thread
From: Chaoyong He @ 2023-02-03  7:57 UTC (permalink / raw)
  To: dev; +Cc: oss-drivers, niklas.soderlund, James Hershaw, Chaoyong He

From: James Hershaw <james.hershaw@corigine.com>

Remove usage of the rte_panic() function from the nfp PMD.

Update the nfp_net_nfd3_xmit_pkts() function to match the functionality
of the nfp_net_nfdk_xmit_pkts(), rather than use rte_panic()

Update the logging within both the nfp_net_nfd3_xmit_pkts() and
nfp_net_nfdk_xmit_pkts() functions to only make use of a single
log when encountering a transmission with multiple segments when
NFP_NET_CFG_CTRL_GATHER is not set, and to change the log-level to
ERR.

Signed-off-by: James Hershaw <james.hershaw@corigine.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@corigine.com>
Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
---
 drivers/net/nfp/flower/nfp_flower.c | 3 +--
 drivers/net/nfp/nfp_rxtx.c          | 7 +++----
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/net/nfp/flower/nfp_flower.c b/drivers/net/nfp/flower/nfp_flower.c
index 7b46dc0f6a..e0e94e0a36 100644
--- a/drivers/net/nfp/flower/nfp_flower.c
+++ b/drivers/net/nfp/flower/nfp_flower.c
@@ -532,8 +532,7 @@ nfp_flower_pf_xmit_pkts(void *tx_queue,
 
 		if (unlikely(pkt->nb_segs > 1 &&
 				!(hw->cap & NFP_NET_CFG_CTRL_GATHER))) {
-			PMD_INIT_LOG(INFO, "NFP_NET_CFG_CTRL_GATHER not set");
-			PMD_INIT_LOG(INFO, "Multisegment packet unsupported");
+			PMD_INIT_LOG(ERR, "Multisegment packet not supported");
 			goto xmit_end;
 		}
 
diff --git a/drivers/net/nfp/nfp_rxtx.c b/drivers/net/nfp/nfp_rxtx.c
index 4a7574fd65..5f23128ac9 100644
--- a/drivers/net/nfp/nfp_rxtx.c
+++ b/drivers/net/nfp/nfp_rxtx.c
@@ -986,8 +986,8 @@ nfp_net_nfd3_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pk
 
 		if (unlikely(pkt->nb_segs > 1 &&
 			     !(hw->cap & NFP_NET_CFG_CTRL_GATHER))) {
-			PMD_INIT_LOG(INFO, "NFP_NET_CFG_CTRL_GATHER not set");
-			rte_panic("Multisegment packet unsupported\n");
+			PMD_INIT_LOG(ERR, "Multisegment packet not supported");
+			goto xmit_end;
 		}
 
 		/* Checking if we have enough descriptors */
@@ -1466,8 +1466,7 @@ nfp_net_nfdk_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pk
 
 		if (unlikely(pkt->nb_segs > 1 &&
 				!(hw->cap & NFP_NET_CFG_CTRL_GATHER))) {
-			PMD_INIT_LOG(INFO, "NFP_NET_CFG_CTRL_GATHER not set");
-			PMD_INIT_LOG(INFO, "Multisegment packet unsupported");
+			PMD_INIT_LOG(ERR, "Multisegment packet not supported");
 			goto xmit_end;
 		}
 
-- 
2.29.3


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

* [PATCH 3/3] net/nfp: remove ASSERT() macro
  2023-02-03  7:57 [PATCH 0/3] remove usage of rte_panic() and ASSERT() from PMD Chaoyong He
  2023-02-03  7:57 ` [PATCH 1/3] net/nfp: remove panic usage during reconfig Chaoyong He
  2023-02-03  7:57 ` [PATCH 2/3] net/nfp: remove panic usage for multisegment Tx Chaoyong He
@ 2023-02-03  7:57 ` Chaoyong He
  2023-02-03 18:57 ` [PATCH 0/3] remove usage of rte_panic() and ASSERT() from PMD Ferruh Yigit
  3 siblings, 0 replies; 5+ messages in thread
From: Chaoyong He @ 2023-02-03  7:57 UTC (permalink / raw)
  To: dev; +Cc: oss-drivers, niklas.soderlund, James Hershaw, Chaoyong He

From: James Hershaw <james.hershaw@corigine.com>

Remove usage of the rte_panic() function from the nfp PMD.

As the ASSERT(x) macro was only used twice in the PMD, it is better
remove it altogether and rework the code where it was used as it
is a very small change to make.

Signed-off-by: James Hershaw <james.hershaw@corigine.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@corigine.com>
Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
---
 drivers/net/nfp/flower/nfp_flower.c | 8 +++-----
 drivers/net/nfp/nfp_logs.h          | 2 --
 drivers/net/nfp/nfp_rxtx.c          | 7 ++-----
 3 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/drivers/net/nfp/flower/nfp_flower.c b/drivers/net/nfp/flower/nfp_flower.c
index e0e94e0a36..f1424a010d 100644
--- a/drivers/net/nfp/flower/nfp_flower.c
+++ b/drivers/net/nfp/flower/nfp_flower.c
@@ -491,7 +491,7 @@ nfp_flower_pf_xmit_pkts(void *tx_queue,
 		struct rte_mbuf **tx_pkts,
 		uint16_t nb_pkts)
 {
-	int i = 0;
+	int i;
 	int pkt_size;
 	int dma_size;
 	uint64_t dma_addr;
@@ -522,7 +522,7 @@ nfp_flower_pf_xmit_pkts(void *tx_queue,
 	issued_descs = 0;
 
 	/* Sending packets */
-	while ((i < nb_pkts) && free_descs) {
+	for (i = 0; i < nb_pkts && free_descs > 0; i++) {
 		/* Grabbing the mbuf linked to the current descriptor */
 		lmbuf = &txq->txbufs[txq->wr_p].mbuf;
 		/* Warming the cache for releasing the mbuf later on */
@@ -561,7 +561,7 @@ nfp_flower_pf_xmit_pkts(void *tx_queue,
 		 */
 		pkt_size = pkt->pkt_len;
 
-		while (pkt != NULL) {
+		while (pkt != NULL && free_descs > 0) {
 			/* Copying TSO, VLAN and cksum info */
 			*txds = txd;
 
@@ -583,7 +583,6 @@ nfp_flower_pf_xmit_pkts(void *tx_queue,
 			txds->data_len = txd.data_len;
 			txds->dma_addr_hi = (dma_addr >> 32) & 0xff;
 			txds->dma_addr_lo = (dma_addr & 0xffffffff);
-			ASSERT(free_descs > 0);
 			free_descs--;
 
 			txq->wr_p++;
@@ -607,7 +606,6 @@ nfp_flower_pf_xmit_pkts(void *tx_queue,
 			lmbuf = &txq->txbufs[txq->wr_p].mbuf;
 			issued_descs++;
 		}
-		i++;
 	}
 
 xmit_end:
diff --git a/drivers/net/nfp/nfp_logs.h b/drivers/net/nfp/nfp_logs.h
index 5da384e53f..b7632ee72c 100644
--- a/drivers/net/nfp/nfp_logs.h
+++ b/drivers/net/nfp/nfp_logs.h
@@ -24,10 +24,8 @@ extern int nfp_logtype_init;
 #ifdef RTE_ETHDEV_DEBUG_TX
 #define PMD_TX_LOG(level, fmt, 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)
-#define ASSERT(x) do { } while (0)
 #endif
 
 extern int nfp_logtype_cpp;
diff --git a/drivers/net/nfp/nfp_rxtx.c b/drivers/net/nfp/nfp_rxtx.c
index 5f23128ac9..a5907cc428 100644
--- a/drivers/net/nfp/nfp_rxtx.c
+++ b/drivers/net/nfp/nfp_rxtx.c
@@ -968,12 +968,11 @@ nfp_net_nfd3_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pk
 
 	pkt = *tx_pkts;
 
-	i = 0;
 	issued_descs = 0;
 	PMD_TX_LOG(DEBUG, "queue: %u. Sending %u packets",
 		   txq->qidx, nb_pkts);
 	/* Sending packets */
-	while ((i < nb_pkts) && free_descs) {
+	for (i = 0; i < nb_pkts && free_descs > 0; i++) {
 		memset(&meta_data, 0, sizeof(meta_data));
 		/* Grabbing the mbuf linked to the current descriptor */
 		lmbuf = &txq->txbufs[txq->wr_p].mbuf;
@@ -1010,7 +1009,7 @@ nfp_net_nfd3_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pk
 		 */
 		pkt_size = pkt->pkt_len;
 
-		while (pkt) {
+		while (pkt != NULL && free_descs > 0) {
 			/* Copying TSO, VLAN and cksum info */
 			*txds = txd;
 
@@ -1034,7 +1033,6 @@ nfp_net_nfd3_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pk
 			txds->data_len = txd.data_len;
 			txds->dma_addr_hi = (dma_addr >> 32) & 0xff;
 			txds->dma_addr_lo = (dma_addr & 0xffffffff);
-			ASSERT(free_descs > 0);
 			free_descs--;
 
 			txq->wr_p++;
@@ -1061,7 +1059,6 @@ nfp_net_nfd3_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pk
 			lmbuf = &txq->txbufs[txq->wr_p].mbuf;
 			issued_descs++;
 		}
-		i++;
 	}
 
 xmit_end:
-- 
2.29.3


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

* Re: [PATCH 0/3] remove usage of rte_panic() and ASSERT() from PMD
  2023-02-03  7:57 [PATCH 0/3] remove usage of rte_panic() and ASSERT() from PMD Chaoyong He
                   ` (2 preceding siblings ...)
  2023-02-03  7:57 ` [PATCH 3/3] net/nfp: remove ASSERT() macro Chaoyong He
@ 2023-02-03 18:57 ` Ferruh Yigit
  3 siblings, 0 replies; 5+ messages in thread
From: Ferruh Yigit @ 2023-02-03 18:57 UTC (permalink / raw)
  To: Chaoyong He, dev; +Cc: oss-drivers, niklas.soderlund

On 2/3/2023 7:57 AM, Chaoyong He wrote:
> This patch series is aimed to allow the DPDK app to deal with
> the error as necessary, by:
> * Remove the usage of rte_panic()
> * Remove the usage of ASSERT()
> 
> James Hershaw (3):
>   net/nfp: remove panic usage during reconfig
>   net/nfp: remove panic usage for multisegment Tx
>   net/nfp: remove ASSERT() macro


Thanks for cleanup!

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

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

end of thread, other threads:[~2023-02-03 18:57 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-03  7:57 [PATCH 0/3] remove usage of rte_panic() and ASSERT() from PMD Chaoyong He
2023-02-03  7:57 ` [PATCH 1/3] net/nfp: remove panic usage during reconfig Chaoyong He
2023-02-03  7:57 ` [PATCH 2/3] net/nfp: remove panic usage for multisegment Tx Chaoyong He
2023-02-03  7:57 ` [PATCH 3/3] net/nfp: remove ASSERT() macro Chaoyong He
2023-02-03 18:57 ` [PATCH 0/3] remove usage of rte_panic() and ASSERT() from PMD 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).