patches for DPDK stable branches
 help / color / mirror / Atom feed
* [PATCH 04/13] net/ionic: fix missing volatile type for cqe pointers
       [not found] <20240202193238.62669-1-andrew.boyer@amd.com>
@ 2024-02-02 19:32 ` Andrew Boyer
  2024-02-03  4:26   ` Stephen Hemminger
  2024-02-02 19:32 ` [PATCH 07/13] net/ionic: fix RSS query routine Andrew Boyer
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 11+ messages in thread
From: Andrew Boyer @ 2024-02-02 19:32 UTC (permalink / raw)
  To: dev; +Cc: Neel Patel, stable, Andrew Boyer

From: Neel Patel <neel.patel@amd.com>

This memory may be changed by the hardware, so the volatile
keyword is required for correctness.

Fixes: e86a6fcc7cf3 ("net/ionic: add optimized non-scattered Rx/Tx")
cc: stable@dpdk.org

Signed-off-by: Andrew Boyer <andrew.boyer@amd.com>
Signed-off-by: Neel Patel <neel.patel@amd.com>
---
 drivers/net/ionic/ionic_rxtx.c        | 4 ++--
 drivers/net/ionic/ionic_rxtx_sg.c     | 8 +++++---
 drivers/net/ionic/ionic_rxtx_simple.c | 8 +++++---
 3 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ionic/ionic_rxtx.c b/drivers/net/ionic/ionic_rxtx.c
index d92b231f8f..d92fa1cca7 100644
--- a/drivers/net/ionic/ionic_rxtx.c
+++ b/drivers/net/ionic/ionic_rxtx.c
@@ -755,7 +755,7 @@ ionic_dev_rx_descriptor_status(void *rx_queue, uint16_t offset)
 {
 	struct ionic_rx_qcq *rxq = rx_queue;
 	struct ionic_qcq *qcq = &rxq->qcq;
-	struct ionic_rxq_comp *cq_desc;
+	volatile struct ionic_rxq_comp *cq_desc;
 	uint16_t mask, head, tail, pos;
 	bool done_color;
 
@@ -794,7 +794,7 @@ ionic_dev_tx_descriptor_status(void *tx_queue, uint16_t offset)
 {
 	struct ionic_tx_qcq *txq = tx_queue;
 	struct ionic_qcq *qcq = &txq->qcq;
-	struct ionic_txq_comp *cq_desc;
+	volatile struct ionic_txq_comp *cq_desc;
 	uint16_t mask, head, tail, pos, cq_pos;
 	bool done_color;
 
diff --git a/drivers/net/ionic/ionic_rxtx_sg.c b/drivers/net/ionic/ionic_rxtx_sg.c
index 6c028a698c..1392342463 100644
--- a/drivers/net/ionic/ionic_rxtx_sg.c
+++ b/drivers/net/ionic/ionic_rxtx_sg.c
@@ -28,7 +28,8 @@ ionic_tx_flush_sg(struct ionic_tx_qcq *txq)
 	struct ionic_queue *q = &txq->qcq.q;
 	struct ionic_tx_stats *stats = &txq->stats;
 	struct rte_mbuf *txm;
-	struct ionic_txq_comp *cq_desc, *cq_desc_base = cq->base;
+	struct ionic_txq_comp *cq_desc_base = cq->base;
+	volatile struct ionic_txq_comp *cq_desc;
 	void **info;
 	uint32_t i;
 
@@ -254,7 +255,7 @@ ionic_xmit_pkts_sg(void *tx_queue, struct rte_mbuf **tx_pkts,
  */
 static __rte_always_inline void
 ionic_rx_clean_one_sg(struct ionic_rx_qcq *rxq,
-		struct ionic_rxq_comp *cq_desc,
+		volatile struct ionic_rxq_comp *cq_desc,
 		struct ionic_rx_service *rx_svc)
 {
 	struct ionic_queue *q = &rxq->qcq.q;
@@ -440,7 +441,8 @@ ionic_rxq_service_sg(struct ionic_rx_qcq *rxq, uint32_t work_to_do,
 	struct ionic_cq *cq = &rxq->qcq.cq;
 	struct ionic_queue *q = &rxq->qcq.q;
 	struct ionic_rxq_desc *q_desc_base = q->base;
-	struct ionic_rxq_comp *cq_desc, *cq_desc_base = cq->base;
+	struct ionic_rxq_comp *cq_desc_base = cq->base;
+	volatile struct ionic_rxq_comp *cq_desc;
 	uint32_t work_done = 0;
 	uint64_t then, now, hz, delta;
 
diff --git a/drivers/net/ionic/ionic_rxtx_simple.c b/drivers/net/ionic/ionic_rxtx_simple.c
index 5969287b66..00152c885a 100644
--- a/drivers/net/ionic/ionic_rxtx_simple.c
+++ b/drivers/net/ionic/ionic_rxtx_simple.c
@@ -28,7 +28,8 @@ ionic_tx_flush(struct ionic_tx_qcq *txq)
 	struct ionic_queue *q = &txq->qcq.q;
 	struct ionic_tx_stats *stats = &txq->stats;
 	struct rte_mbuf *txm;
-	struct ionic_txq_comp *cq_desc, *cq_desc_base = cq->base;
+	struct ionic_txq_comp *cq_desc_base = cq->base;
+	volatile struct ionic_txq_comp *cq_desc;
 	void **info;
 
 	cq_desc = &cq_desc_base[cq->tail_idx];
@@ -227,7 +228,7 @@ ionic_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
  */
 static __rte_always_inline void
 ionic_rx_clean_one(struct ionic_rx_qcq *rxq,
-		struct ionic_rxq_comp *cq_desc,
+		volatile struct ionic_rxq_comp *cq_desc,
 		struct ionic_rx_service *rx_svc)
 {
 	struct ionic_queue *q = &rxq->qcq.q;
@@ -361,7 +362,8 @@ ionic_rxq_service(struct ionic_rx_qcq *rxq, uint32_t work_to_do,
 	struct ionic_cq *cq = &rxq->qcq.cq;
 	struct ionic_queue *q = &rxq->qcq.q;
 	struct ionic_rxq_desc *q_desc_base = q->base;
-	struct ionic_rxq_comp *cq_desc, *cq_desc_base = cq->base;
+	struct ionic_rxq_comp *cq_desc_base = cq->base;
+	volatile struct ionic_rxq_comp *cq_desc;
 	uint32_t work_done = 0;
 	uint64_t then, now, hz, delta;
 
-- 
2.17.1


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

* [PATCH 07/13] net/ionic: fix RSS query routine
       [not found] <20240202193238.62669-1-andrew.boyer@amd.com>
  2024-02-02 19:32 ` [PATCH 04/13] net/ionic: fix missing volatile type for cqe pointers Andrew Boyer
@ 2024-02-02 19:32 ` Andrew Boyer
  2024-02-02 19:32 ` [PATCH 10/13] net/ionic: fix device close sequence to avoid crash Andrew Boyer
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Andrew Boyer @ 2024-02-02 19:32 UTC (permalink / raw)
  To: dev; +Cc: Akshay Dorwat, cardigliano, stable, Andrew Boyer

From: Akshay Dorwat <akshay.dorwat@amd.com>

The routine that copies out the RSS config can't use memcpy() because
'reta_conf->reta' is an array of uint16_t while 'lif->rss_ind_tbl' is
an array of uint8_t. Instead, copy the values individually.

Fixes: 22e7171bc63b ("net/ionic: support RSS")
Cc: cardigliano@ntop.org
Cc: stable@dpdk.org

Signed-off-by: Akshay Dorwat <akshay.dorwat@amd.com>
Signed-off-by: Andrew Boyer <andrew.boyer@amd.com>
---
 drivers/net/ionic/ionic_ethdev.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ionic/ionic_ethdev.c b/drivers/net/ionic/ionic_ethdev.c
index 340fd0cd59..008e50e0b9 100644
--- a/drivers/net/ionic/ionic_ethdev.c
+++ b/drivers/net/ionic/ionic_ethdev.c
@@ -561,7 +561,7 @@ ionic_dev_rss_reta_query(struct rte_eth_dev *eth_dev,
 	struct ionic_lif *lif = IONIC_ETH_DEV_TO_LIF(eth_dev);
 	struct ionic_adapter *adapter = lif->adapter;
 	struct ionic_identity *ident = &adapter->ident;
-	int i, num;
+	int i, j, num;
 	uint16_t tbl_sz = rte_le_to_cpu_16(ident->lif.eth.rss_ind_tbl_sz);
 
 	IONIC_PRINT_CALL();
@@ -582,9 +582,10 @@ ionic_dev_rss_reta_query(struct rte_eth_dev *eth_dev,
 	num = reta_size / RTE_ETH_RETA_GROUP_SIZE;
 
 	for (i = 0; i < num; i++) {
-		memcpy(reta_conf->reta,
-			&lif->rss_ind_tbl[i * RTE_ETH_RETA_GROUP_SIZE],
-			RTE_ETH_RETA_GROUP_SIZE);
+		for (j = 0; j < RTE_ETH_RETA_GROUP_SIZE; j++) {
+			reta_conf->reta[j] =
+				lif->rss_ind_tbl[(i * RTE_ETH_RETA_GROUP_SIZE) + j];
+		}
 		reta_conf++;
 	}
 
-- 
2.17.1


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

* [PATCH 10/13] net/ionic: fix device close sequence to avoid crash
       [not found] <20240202193238.62669-1-andrew.boyer@amd.com>
  2024-02-02 19:32 ` [PATCH 04/13] net/ionic: fix missing volatile type for cqe pointers Andrew Boyer
  2024-02-02 19:32 ` [PATCH 07/13] net/ionic: fix RSS query routine Andrew Boyer
@ 2024-02-02 19:32 ` Andrew Boyer
  2024-02-07  2:18 ` [PATCH v2 04/13] net/ionic: fix missing volatile type for cqe pointers Andrew Boyer
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Andrew Boyer @ 2024-02-02 19:32 UTC (permalink / raw)
  To: dev; +Cc: Andrew Boyer, stable

The close routine should release all resources, but not
call rte_eth_dev_destroy(). As written this code will call
rte_eth_dev_release_port() twice and segfault.

Instead, move rte_eth_dev_destroy() to the remove routine.
eth_ionic_dev_uninit() will call close if necessary.

Fixes: 175e4e7ed760 ("net/ionic: complete release on close")
Cc: stable@dpdk.org

Signed-off-by: Andrew Boyer <andrew.boyer@amd.com>
---
 drivers/net/ionic/ionic_ethdev.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ionic/ionic_ethdev.c b/drivers/net/ionic/ionic_ethdev.c
index 7c55a26956..bedcf958e2 100644
--- a/drivers/net/ionic/ionic_ethdev.c
+++ b/drivers/net/ionic/ionic_ethdev.c
@@ -1009,19 +1009,21 @@ ionic_dev_close(struct rte_eth_dev *eth_dev)
 
 	ionic_lif_stop(lif);
 
-	ionic_lif_free_queues(lif);
-
 	IONIC_PRINT(NOTICE, "Removing device %s", eth_dev->device->name);
 	if (adapter->intf->unconfigure_intr)
 		(*adapter->intf->unconfigure_intr)(adapter);
 
-	rte_eth_dev_destroy(eth_dev, eth_ionic_dev_uninit);
-
 	ionic_port_reset(adapter);
 	ionic_reset(adapter);
+
+	ionic_lif_free_queues(lif);
+	ionic_lif_deinit(lif);
+	ionic_lif_free(lif); /* Does not free LIF object */
+
 	if (adapter->intf->unmap_bars)
 		(*adapter->intf->unmap_bars)(adapter);
 
+	lif->adapter = NULL;
 	rte_free(adapter);
 
 	return 0;
@@ -1098,21 +1100,18 @@ eth_ionic_dev_init(struct rte_eth_dev *eth_dev, void *init_params)
 static int
 eth_ionic_dev_uninit(struct rte_eth_dev *eth_dev)
 {
-	struct ionic_lif *lif = IONIC_ETH_DEV_TO_LIF(eth_dev);
-	struct ionic_adapter *adapter = lif->adapter;
-
 	IONIC_PRINT_CALL();
 
 	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
 		return 0;
 
-	adapter->lif = NULL;
-
-	ionic_lif_deinit(lif);
-	ionic_lif_free(lif);
+	if (eth_dev->state != RTE_ETH_DEV_UNUSED)
+		ionic_dev_close(eth_dev);
 
-	if (!(lif->state & IONIC_LIF_F_FW_RESET))
-		ionic_lif_reset(lif);
+	eth_dev->dev_ops = NULL;
+	eth_dev->rx_pkt_burst = NULL;
+	eth_dev->tx_pkt_burst = NULL;
+	eth_dev->tx_pkt_prepare = NULL;
 
 	return 0;
 }
@@ -1267,17 +1266,18 @@ eth_ionic_dev_remove(struct rte_device *rte_dev)
 {
 	char name[RTE_ETH_NAME_MAX_LEN];
 	struct rte_eth_dev *eth_dev;
+	int ret = 0;
 
 	/* Adapter lookup is using the eth_dev name */
 	snprintf(name, sizeof(name), "%s_lif", rte_dev->name);
 
 	eth_dev = rte_eth_dev_allocated(name);
 	if (eth_dev)
-		ionic_dev_close(eth_dev);
+		ret = rte_eth_dev_destroy(eth_dev, eth_ionic_dev_uninit);
 	else
 		IONIC_PRINT(DEBUG, "Cannot find device %s", rte_dev->name);
 
-	return 0;
+	return ret;
 }
 
 RTE_LOG_REGISTER_DEFAULT(ionic_logtype, NOTICE);
-- 
2.17.1


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

* Re: [PATCH 04/13] net/ionic: fix missing volatile type for cqe pointers
  2024-02-02 19:32 ` [PATCH 04/13] net/ionic: fix missing volatile type for cqe pointers Andrew Boyer
@ 2024-02-03  4:26   ` Stephen Hemminger
  2024-02-05 13:33     ` Boyer, Andrew
  0 siblings, 1 reply; 11+ messages in thread
From: Stephen Hemminger @ 2024-02-03  4:26 UTC (permalink / raw)
  To: Andrew Boyer; +Cc: dev, Neel Patel, stable

On Fri, 2 Feb 2024 11:32:29 -0800
Andrew Boyer <andrew.boyer@amd.com> wrote:

> From: Neel Patel <neel.patel@amd.com>
> 
> This memory may be changed by the hardware, so the volatile
> keyword is required for correctness.
> 
> Fixes: e86a6fcc7cf3 ("net/ionic: add optimized non-scattered Rx/Tx")
> cc: stable@dpdk.org
> 
> Signed-off-by: Andrew Boyer <andrew.boyer@amd.com>
> Signed-off-by: Neel Patel <neel.patel@amd.com>

In general barriers are better than volatile.
Volatile is a compiler not hardware construct really.

https://www.kernel.org/doc/html/latest/process/volatile-considered-harmful.html

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

* Re: [PATCH 04/13] net/ionic: fix missing volatile type for cqe pointers
  2024-02-03  4:26   ` Stephen Hemminger
@ 2024-02-05 13:33     ` Boyer, Andrew
  0 siblings, 0 replies; 11+ messages in thread
From: Boyer, Andrew @ 2024-02-05 13:33 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Boyer, Andrew, dev, Patel, Neel, stable



> On Feb 2, 2024, at 11:26 PM, Stephen Hemminger <stephen@networkplumber.org> wrote:
> 
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> 
> 
> On Fri, 2 Feb 2024 11:32:29 -0800
> Andrew Boyer <andrew.boyer@amd.com> wrote:
> 
>> From: Neel Patel <neel.patel@amd.com>
>> 
>> This memory may be changed by the hardware, so the volatile
>> keyword is required for correctness.
>> 
>> Fixes: e86a6fcc7cf3 ("net/ionic: add optimized non-scattered Rx/Tx")
>> cc: stable@dpdk.org
>> 
>> Signed-off-by: Andrew Boyer <andrew.boyer@amd.com>
>> Signed-off-by: Neel Patel <neel.patel@amd.com>
> 
> In general barriers are better than volatile.
> Volatile is a compiler not hardware construct really.
> 
> https://www.kernel.org/doc/html/latest/process/volatile-considered-harmful.html

Thanks for looking. The goal here is to prevent inappropriate compiler optimization.

From the link you posted:

> Pointers to data structures in coherent memory which might be modified by I/O devices can, sometimes, legitimately be volatile. A ring buffer used by a network adapter, where that adapter changes pointers to indicate which descriptors have been processed, is an example of this type of situation.


Doesn't that sound like what we are doing?

Thanks,
Andrew


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

* [PATCH v2 04/13] net/ionic: fix missing volatile type for cqe pointers
       [not found] <20240202193238.62669-1-andrew.boyer@amd.com>
                   ` (2 preceding siblings ...)
  2024-02-02 19:32 ` [PATCH 10/13] net/ionic: fix device close sequence to avoid crash Andrew Boyer
@ 2024-02-07  2:18 ` Andrew Boyer
  2024-02-07  2:18 ` [PATCH v2 07/13] net/ionic: fix RSS query routine Andrew Boyer
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Andrew Boyer @ 2024-02-07  2:18 UTC (permalink / raw)
  To: dev; +Cc: Neel Patel, stable, Andrew Boyer

From: Neel Patel <neel.patel@amd.com>

This memory may be changed by the hardware, so the volatile
keyword is required for correctness.

Fixes: e86a6fcc7cf3 ("net/ionic: add optimized non-scattered Rx/Tx")
cc: stable@dpdk.org

Signed-off-by: Andrew Boyer <andrew.boyer@amd.com>
Signed-off-by: Neel Patel <neel.patel@amd.com>
---
 drivers/net/ionic/ionic_rxtx.c        | 4 ++--
 drivers/net/ionic/ionic_rxtx_sg.c     | 8 +++++---
 drivers/net/ionic/ionic_rxtx_simple.c | 8 +++++---
 3 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ionic/ionic_rxtx.c b/drivers/net/ionic/ionic_rxtx.c
index d92b231f8f..d92fa1cca7 100644
--- a/drivers/net/ionic/ionic_rxtx.c
+++ b/drivers/net/ionic/ionic_rxtx.c
@@ -755,7 +755,7 @@ ionic_dev_rx_descriptor_status(void *rx_queue, uint16_t offset)
 {
 	struct ionic_rx_qcq *rxq = rx_queue;
 	struct ionic_qcq *qcq = &rxq->qcq;
-	struct ionic_rxq_comp *cq_desc;
+	volatile struct ionic_rxq_comp *cq_desc;
 	uint16_t mask, head, tail, pos;
 	bool done_color;
 
@@ -794,7 +794,7 @@ ionic_dev_tx_descriptor_status(void *tx_queue, uint16_t offset)
 {
 	struct ionic_tx_qcq *txq = tx_queue;
 	struct ionic_qcq *qcq = &txq->qcq;
-	struct ionic_txq_comp *cq_desc;
+	volatile struct ionic_txq_comp *cq_desc;
 	uint16_t mask, head, tail, pos, cq_pos;
 	bool done_color;
 
diff --git a/drivers/net/ionic/ionic_rxtx_sg.c b/drivers/net/ionic/ionic_rxtx_sg.c
index 6c028a698c..1392342463 100644
--- a/drivers/net/ionic/ionic_rxtx_sg.c
+++ b/drivers/net/ionic/ionic_rxtx_sg.c
@@ -28,7 +28,8 @@ ionic_tx_flush_sg(struct ionic_tx_qcq *txq)
 	struct ionic_queue *q = &txq->qcq.q;
 	struct ionic_tx_stats *stats = &txq->stats;
 	struct rte_mbuf *txm;
-	struct ionic_txq_comp *cq_desc, *cq_desc_base = cq->base;
+	struct ionic_txq_comp *cq_desc_base = cq->base;
+	volatile struct ionic_txq_comp *cq_desc;
 	void **info;
 	uint32_t i;
 
@@ -254,7 +255,7 @@ ionic_xmit_pkts_sg(void *tx_queue, struct rte_mbuf **tx_pkts,
  */
 static __rte_always_inline void
 ionic_rx_clean_one_sg(struct ionic_rx_qcq *rxq,
-		struct ionic_rxq_comp *cq_desc,
+		volatile struct ionic_rxq_comp *cq_desc,
 		struct ionic_rx_service *rx_svc)
 {
 	struct ionic_queue *q = &rxq->qcq.q;
@@ -440,7 +441,8 @@ ionic_rxq_service_sg(struct ionic_rx_qcq *rxq, uint32_t work_to_do,
 	struct ionic_cq *cq = &rxq->qcq.cq;
 	struct ionic_queue *q = &rxq->qcq.q;
 	struct ionic_rxq_desc *q_desc_base = q->base;
-	struct ionic_rxq_comp *cq_desc, *cq_desc_base = cq->base;
+	struct ionic_rxq_comp *cq_desc_base = cq->base;
+	volatile struct ionic_rxq_comp *cq_desc;
 	uint32_t work_done = 0;
 	uint64_t then, now, hz, delta;
 
diff --git a/drivers/net/ionic/ionic_rxtx_simple.c b/drivers/net/ionic/ionic_rxtx_simple.c
index 5969287b66..00152c885a 100644
--- a/drivers/net/ionic/ionic_rxtx_simple.c
+++ b/drivers/net/ionic/ionic_rxtx_simple.c
@@ -28,7 +28,8 @@ ionic_tx_flush(struct ionic_tx_qcq *txq)
 	struct ionic_queue *q = &txq->qcq.q;
 	struct ionic_tx_stats *stats = &txq->stats;
 	struct rte_mbuf *txm;
-	struct ionic_txq_comp *cq_desc, *cq_desc_base = cq->base;
+	struct ionic_txq_comp *cq_desc_base = cq->base;
+	volatile struct ionic_txq_comp *cq_desc;
 	void **info;
 
 	cq_desc = &cq_desc_base[cq->tail_idx];
@@ -227,7 +228,7 @@ ionic_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
  */
 static __rte_always_inline void
 ionic_rx_clean_one(struct ionic_rx_qcq *rxq,
-		struct ionic_rxq_comp *cq_desc,
+		volatile struct ionic_rxq_comp *cq_desc,
 		struct ionic_rx_service *rx_svc)
 {
 	struct ionic_queue *q = &rxq->qcq.q;
@@ -361,7 +362,8 @@ ionic_rxq_service(struct ionic_rx_qcq *rxq, uint32_t work_to_do,
 	struct ionic_cq *cq = &rxq->qcq.cq;
 	struct ionic_queue *q = &rxq->qcq.q;
 	struct ionic_rxq_desc *q_desc_base = q->base;
-	struct ionic_rxq_comp *cq_desc, *cq_desc_base = cq->base;
+	struct ionic_rxq_comp *cq_desc_base = cq->base;
+	volatile struct ionic_rxq_comp *cq_desc;
 	uint32_t work_done = 0;
 	uint64_t then, now, hz, delta;
 
-- 
2.17.1


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

* [PATCH v2 07/13] net/ionic: fix RSS query routine
       [not found] <20240202193238.62669-1-andrew.boyer@amd.com>
                   ` (3 preceding siblings ...)
  2024-02-07  2:18 ` [PATCH v2 04/13] net/ionic: fix missing volatile type for cqe pointers Andrew Boyer
@ 2024-02-07  2:18 ` Andrew Boyer
  2024-02-07  2:18 ` [PATCH v2 10/13] net/ionic: fix device close sequence to avoid crash Andrew Boyer
       [not found] ` <20240207031317.32293-1-andrew.boyer@amd.com>
  6 siblings, 0 replies; 11+ messages in thread
From: Andrew Boyer @ 2024-02-07  2:18 UTC (permalink / raw)
  To: dev; +Cc: Akshay Dorwat, cardigliano, stable, Andrew Boyer

From: Akshay Dorwat <akshay.dorwat@amd.com>

The routine that copies out the RSS config can't use memcpy() because
'reta_conf->reta' is an array of uint16_t while 'lif->rss_ind_tbl' is
an array of uint8_t. Instead, copy the values individually.

Fixes: 22e7171bc63b ("net/ionic: support RSS")
Cc: cardigliano@ntop.org
Cc: stable@dpdk.org

Signed-off-by: Akshay Dorwat <akshay.dorwat@amd.com>
Signed-off-by: Andrew Boyer <andrew.boyer@amd.com>
---
 drivers/net/ionic/ionic_ethdev.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ionic/ionic_ethdev.c b/drivers/net/ionic/ionic_ethdev.c
index 340fd0cd59..008e50e0b9 100644
--- a/drivers/net/ionic/ionic_ethdev.c
+++ b/drivers/net/ionic/ionic_ethdev.c
@@ -561,7 +561,7 @@ ionic_dev_rss_reta_query(struct rte_eth_dev *eth_dev,
 	struct ionic_lif *lif = IONIC_ETH_DEV_TO_LIF(eth_dev);
 	struct ionic_adapter *adapter = lif->adapter;
 	struct ionic_identity *ident = &adapter->ident;
-	int i, num;
+	int i, j, num;
 	uint16_t tbl_sz = rte_le_to_cpu_16(ident->lif.eth.rss_ind_tbl_sz);
 
 	IONIC_PRINT_CALL();
@@ -582,9 +582,10 @@ ionic_dev_rss_reta_query(struct rte_eth_dev *eth_dev,
 	num = reta_size / RTE_ETH_RETA_GROUP_SIZE;
 
 	for (i = 0; i < num; i++) {
-		memcpy(reta_conf->reta,
-			&lif->rss_ind_tbl[i * RTE_ETH_RETA_GROUP_SIZE],
-			RTE_ETH_RETA_GROUP_SIZE);
+		for (j = 0; j < RTE_ETH_RETA_GROUP_SIZE; j++) {
+			reta_conf->reta[j] =
+				lif->rss_ind_tbl[(i * RTE_ETH_RETA_GROUP_SIZE) + j];
+		}
 		reta_conf++;
 	}
 
-- 
2.17.1


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

* [PATCH v2 10/13] net/ionic: fix device close sequence to avoid crash
       [not found] <20240202193238.62669-1-andrew.boyer@amd.com>
                   ` (4 preceding siblings ...)
  2024-02-07  2:18 ` [PATCH v2 07/13] net/ionic: fix RSS query routine Andrew Boyer
@ 2024-02-07  2:18 ` Andrew Boyer
       [not found] ` <20240207031317.32293-1-andrew.boyer@amd.com>
  6 siblings, 0 replies; 11+ messages in thread
From: Andrew Boyer @ 2024-02-07  2:18 UTC (permalink / raw)
  To: dev; +Cc: Andrew Boyer, stable

The close routine should release all resources, but not
call rte_eth_dev_destroy(). As written this code will call
rte_eth_dev_release_port() twice and segfault.

Instead, move rte_eth_dev_destroy() to the remove routine.
eth_ionic_dev_uninit() will call close if necessary.

Fixes: 175e4e7ed760 ("net/ionic: complete release on close")
Cc: stable@dpdk.org

Signed-off-by: Andrew Boyer <andrew.boyer@amd.com>
---
 drivers/net/ionic/ionic_ethdev.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ionic/ionic_ethdev.c b/drivers/net/ionic/ionic_ethdev.c
index 7c55a26956..bedcf958e2 100644
--- a/drivers/net/ionic/ionic_ethdev.c
+++ b/drivers/net/ionic/ionic_ethdev.c
@@ -1009,19 +1009,21 @@ ionic_dev_close(struct rte_eth_dev *eth_dev)
 
 	ionic_lif_stop(lif);
 
-	ionic_lif_free_queues(lif);
-
 	IONIC_PRINT(NOTICE, "Removing device %s", eth_dev->device->name);
 	if (adapter->intf->unconfigure_intr)
 		(*adapter->intf->unconfigure_intr)(adapter);
 
-	rte_eth_dev_destroy(eth_dev, eth_ionic_dev_uninit);
-
 	ionic_port_reset(adapter);
 	ionic_reset(adapter);
+
+	ionic_lif_free_queues(lif);
+	ionic_lif_deinit(lif);
+	ionic_lif_free(lif); /* Does not free LIF object */
+
 	if (adapter->intf->unmap_bars)
 		(*adapter->intf->unmap_bars)(adapter);
 
+	lif->adapter = NULL;
 	rte_free(adapter);
 
 	return 0;
@@ -1098,21 +1100,18 @@ eth_ionic_dev_init(struct rte_eth_dev *eth_dev, void *init_params)
 static int
 eth_ionic_dev_uninit(struct rte_eth_dev *eth_dev)
 {
-	struct ionic_lif *lif = IONIC_ETH_DEV_TO_LIF(eth_dev);
-	struct ionic_adapter *adapter = lif->adapter;
-
 	IONIC_PRINT_CALL();
 
 	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
 		return 0;
 
-	adapter->lif = NULL;
-
-	ionic_lif_deinit(lif);
-	ionic_lif_free(lif);
+	if (eth_dev->state != RTE_ETH_DEV_UNUSED)
+		ionic_dev_close(eth_dev);
 
-	if (!(lif->state & IONIC_LIF_F_FW_RESET))
-		ionic_lif_reset(lif);
+	eth_dev->dev_ops = NULL;
+	eth_dev->rx_pkt_burst = NULL;
+	eth_dev->tx_pkt_burst = NULL;
+	eth_dev->tx_pkt_prepare = NULL;
 
 	return 0;
 }
@@ -1267,17 +1266,18 @@ eth_ionic_dev_remove(struct rte_device *rte_dev)
 {
 	char name[RTE_ETH_NAME_MAX_LEN];
 	struct rte_eth_dev *eth_dev;
+	int ret = 0;
 
 	/* Adapter lookup is using the eth_dev name */
 	snprintf(name, sizeof(name), "%s_lif", rte_dev->name);
 
 	eth_dev = rte_eth_dev_allocated(name);
 	if (eth_dev)
-		ionic_dev_close(eth_dev);
+		ret = rte_eth_dev_destroy(eth_dev, eth_ionic_dev_uninit);
 	else
 		IONIC_PRINT(DEBUG, "Cannot find device %s", rte_dev->name);
 
-	return 0;
+	return ret;
 }
 
 RTE_LOG_REGISTER_DEFAULT(ionic_logtype, NOTICE);
-- 
2.17.1


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

* [PATCH v3 04/13] net/ionic: fix missing volatile type for cqe pointers
       [not found] ` <20240207031317.32293-1-andrew.boyer@amd.com>
@ 2024-02-07  3:13   ` Andrew Boyer
  2024-02-07  3:13   ` [PATCH v3 07/13] net/ionic: fix RSS query routine Andrew Boyer
  2024-02-07  3:13   ` [PATCH v3 10/13] net/ionic: fix device close sequence to avoid crash Andrew Boyer
  2 siblings, 0 replies; 11+ messages in thread
From: Andrew Boyer @ 2024-02-07  3:13 UTC (permalink / raw)
  To: dev; +Cc: Neel Patel, stable, Andrew Boyer

From: Neel Patel <neel.patel@amd.com>

This memory may be changed by the hardware, so the volatile
keyword is required for correctness.

Fixes: e86a6fcc7cf3 ("net/ionic: add optimized non-scattered Rx/Tx")
cc: stable@dpdk.org

Signed-off-by: Andrew Boyer <andrew.boyer@amd.com>
Signed-off-by: Neel Patel <neel.patel@amd.com>
---
 drivers/net/ionic/ionic_rxtx.c        | 4 ++--
 drivers/net/ionic/ionic_rxtx_sg.c     | 8 +++++---
 drivers/net/ionic/ionic_rxtx_simple.c | 8 +++++---
 3 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ionic/ionic_rxtx.c b/drivers/net/ionic/ionic_rxtx.c
index d92b231f8f..d92fa1cca7 100644
--- a/drivers/net/ionic/ionic_rxtx.c
+++ b/drivers/net/ionic/ionic_rxtx.c
@@ -755,7 +755,7 @@ ionic_dev_rx_descriptor_status(void *rx_queue, uint16_t offset)
 {
 	struct ionic_rx_qcq *rxq = rx_queue;
 	struct ionic_qcq *qcq = &rxq->qcq;
-	struct ionic_rxq_comp *cq_desc;
+	volatile struct ionic_rxq_comp *cq_desc;
 	uint16_t mask, head, tail, pos;
 	bool done_color;
 
@@ -794,7 +794,7 @@ ionic_dev_tx_descriptor_status(void *tx_queue, uint16_t offset)
 {
 	struct ionic_tx_qcq *txq = tx_queue;
 	struct ionic_qcq *qcq = &txq->qcq;
-	struct ionic_txq_comp *cq_desc;
+	volatile struct ionic_txq_comp *cq_desc;
 	uint16_t mask, head, tail, pos, cq_pos;
 	bool done_color;
 
diff --git a/drivers/net/ionic/ionic_rxtx_sg.c b/drivers/net/ionic/ionic_rxtx_sg.c
index 6c028a698c..1392342463 100644
--- a/drivers/net/ionic/ionic_rxtx_sg.c
+++ b/drivers/net/ionic/ionic_rxtx_sg.c
@@ -28,7 +28,8 @@ ionic_tx_flush_sg(struct ionic_tx_qcq *txq)
 	struct ionic_queue *q = &txq->qcq.q;
 	struct ionic_tx_stats *stats = &txq->stats;
 	struct rte_mbuf *txm;
-	struct ionic_txq_comp *cq_desc, *cq_desc_base = cq->base;
+	struct ionic_txq_comp *cq_desc_base = cq->base;
+	volatile struct ionic_txq_comp *cq_desc;
 	void **info;
 	uint32_t i;
 
@@ -254,7 +255,7 @@ ionic_xmit_pkts_sg(void *tx_queue, struct rte_mbuf **tx_pkts,
  */
 static __rte_always_inline void
 ionic_rx_clean_one_sg(struct ionic_rx_qcq *rxq,
-		struct ionic_rxq_comp *cq_desc,
+		volatile struct ionic_rxq_comp *cq_desc,
 		struct ionic_rx_service *rx_svc)
 {
 	struct ionic_queue *q = &rxq->qcq.q;
@@ -440,7 +441,8 @@ ionic_rxq_service_sg(struct ionic_rx_qcq *rxq, uint32_t work_to_do,
 	struct ionic_cq *cq = &rxq->qcq.cq;
 	struct ionic_queue *q = &rxq->qcq.q;
 	struct ionic_rxq_desc *q_desc_base = q->base;
-	struct ionic_rxq_comp *cq_desc, *cq_desc_base = cq->base;
+	struct ionic_rxq_comp *cq_desc_base = cq->base;
+	volatile struct ionic_rxq_comp *cq_desc;
 	uint32_t work_done = 0;
 	uint64_t then, now, hz, delta;
 
diff --git a/drivers/net/ionic/ionic_rxtx_simple.c b/drivers/net/ionic/ionic_rxtx_simple.c
index 5969287b66..00152c885a 100644
--- a/drivers/net/ionic/ionic_rxtx_simple.c
+++ b/drivers/net/ionic/ionic_rxtx_simple.c
@@ -28,7 +28,8 @@ ionic_tx_flush(struct ionic_tx_qcq *txq)
 	struct ionic_queue *q = &txq->qcq.q;
 	struct ionic_tx_stats *stats = &txq->stats;
 	struct rte_mbuf *txm;
-	struct ionic_txq_comp *cq_desc, *cq_desc_base = cq->base;
+	struct ionic_txq_comp *cq_desc_base = cq->base;
+	volatile struct ionic_txq_comp *cq_desc;
 	void **info;
 
 	cq_desc = &cq_desc_base[cq->tail_idx];
@@ -227,7 +228,7 @@ ionic_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
  */
 static __rte_always_inline void
 ionic_rx_clean_one(struct ionic_rx_qcq *rxq,
-		struct ionic_rxq_comp *cq_desc,
+		volatile struct ionic_rxq_comp *cq_desc,
 		struct ionic_rx_service *rx_svc)
 {
 	struct ionic_queue *q = &rxq->qcq.q;
@@ -361,7 +362,8 @@ ionic_rxq_service(struct ionic_rx_qcq *rxq, uint32_t work_to_do,
 	struct ionic_cq *cq = &rxq->qcq.cq;
 	struct ionic_queue *q = &rxq->qcq.q;
 	struct ionic_rxq_desc *q_desc_base = q->base;
-	struct ionic_rxq_comp *cq_desc, *cq_desc_base = cq->base;
+	struct ionic_rxq_comp *cq_desc_base = cq->base;
+	volatile struct ionic_rxq_comp *cq_desc;
 	uint32_t work_done = 0;
 	uint64_t then, now, hz, delta;
 
-- 
2.17.1


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

* [PATCH v3 07/13] net/ionic: fix RSS query routine
       [not found] ` <20240207031317.32293-1-andrew.boyer@amd.com>
  2024-02-07  3:13   ` [PATCH v3 04/13] net/ionic: fix missing volatile type for cqe pointers Andrew Boyer
@ 2024-02-07  3:13   ` Andrew Boyer
  2024-02-07  3:13   ` [PATCH v3 10/13] net/ionic: fix device close sequence to avoid crash Andrew Boyer
  2 siblings, 0 replies; 11+ messages in thread
From: Andrew Boyer @ 2024-02-07  3:13 UTC (permalink / raw)
  To: dev; +Cc: Akshay Dorwat, cardigliano, stable, Andrew Boyer

From: Akshay Dorwat <akshay.dorwat@amd.com>

The routine that copies out the RSS config can't use memcpy() because
'reta_conf->reta' is an array of uint16_t while 'lif->rss_ind_tbl' is
an array of uint8_t. Instead, copy the values individually.

Fixes: 22e7171bc63b ("net/ionic: support RSS")
Cc: cardigliano@ntop.org
Cc: stable@dpdk.org

Signed-off-by: Akshay Dorwat <akshay.dorwat@amd.com>
Signed-off-by: Andrew Boyer <andrew.boyer@amd.com>
---
 drivers/net/ionic/ionic_ethdev.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ionic/ionic_ethdev.c b/drivers/net/ionic/ionic_ethdev.c
index 340fd0cd59..008e50e0b9 100644
--- a/drivers/net/ionic/ionic_ethdev.c
+++ b/drivers/net/ionic/ionic_ethdev.c
@@ -561,7 +561,7 @@ ionic_dev_rss_reta_query(struct rte_eth_dev *eth_dev,
 	struct ionic_lif *lif = IONIC_ETH_DEV_TO_LIF(eth_dev);
 	struct ionic_adapter *adapter = lif->adapter;
 	struct ionic_identity *ident = &adapter->ident;
-	int i, num;
+	int i, j, num;
 	uint16_t tbl_sz = rte_le_to_cpu_16(ident->lif.eth.rss_ind_tbl_sz);
 
 	IONIC_PRINT_CALL();
@@ -582,9 +582,10 @@ ionic_dev_rss_reta_query(struct rte_eth_dev *eth_dev,
 	num = reta_size / RTE_ETH_RETA_GROUP_SIZE;
 
 	for (i = 0; i < num; i++) {
-		memcpy(reta_conf->reta,
-			&lif->rss_ind_tbl[i * RTE_ETH_RETA_GROUP_SIZE],
-			RTE_ETH_RETA_GROUP_SIZE);
+		for (j = 0; j < RTE_ETH_RETA_GROUP_SIZE; j++) {
+			reta_conf->reta[j] =
+				lif->rss_ind_tbl[(i * RTE_ETH_RETA_GROUP_SIZE) + j];
+		}
 		reta_conf++;
 	}
 
-- 
2.17.1


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

* [PATCH v3 10/13] net/ionic: fix device close sequence to avoid crash
       [not found] ` <20240207031317.32293-1-andrew.boyer@amd.com>
  2024-02-07  3:13   ` [PATCH v3 04/13] net/ionic: fix missing volatile type for cqe pointers Andrew Boyer
  2024-02-07  3:13   ` [PATCH v3 07/13] net/ionic: fix RSS query routine Andrew Boyer
@ 2024-02-07  3:13   ` Andrew Boyer
  2 siblings, 0 replies; 11+ messages in thread
From: Andrew Boyer @ 2024-02-07  3:13 UTC (permalink / raw)
  To: dev; +Cc: Andrew Boyer, stable

The close routine should release all resources, but not
call rte_eth_dev_destroy(). As written this code will call
rte_eth_dev_release_port() twice and segfault.

Instead, move rte_eth_dev_destroy() to the remove routine.
eth_ionic_dev_uninit() will call close if necessary.

Fixes: 175e4e7ed760 ("net/ionic: complete release on close")
Cc: stable@dpdk.org

Signed-off-by: Andrew Boyer <andrew.boyer@amd.com>
---
 drivers/net/ionic/ionic_ethdev.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ionic/ionic_ethdev.c b/drivers/net/ionic/ionic_ethdev.c
index 7c55a26956..bedcf958e2 100644
--- a/drivers/net/ionic/ionic_ethdev.c
+++ b/drivers/net/ionic/ionic_ethdev.c
@@ -1009,19 +1009,21 @@ ionic_dev_close(struct rte_eth_dev *eth_dev)
 
 	ionic_lif_stop(lif);
 
-	ionic_lif_free_queues(lif);
-
 	IONIC_PRINT(NOTICE, "Removing device %s", eth_dev->device->name);
 	if (adapter->intf->unconfigure_intr)
 		(*adapter->intf->unconfigure_intr)(adapter);
 
-	rte_eth_dev_destroy(eth_dev, eth_ionic_dev_uninit);
-
 	ionic_port_reset(adapter);
 	ionic_reset(adapter);
+
+	ionic_lif_free_queues(lif);
+	ionic_lif_deinit(lif);
+	ionic_lif_free(lif); /* Does not free LIF object */
+
 	if (adapter->intf->unmap_bars)
 		(*adapter->intf->unmap_bars)(adapter);
 
+	lif->adapter = NULL;
 	rte_free(adapter);
 
 	return 0;
@@ -1098,21 +1100,18 @@ eth_ionic_dev_init(struct rte_eth_dev *eth_dev, void *init_params)
 static int
 eth_ionic_dev_uninit(struct rte_eth_dev *eth_dev)
 {
-	struct ionic_lif *lif = IONIC_ETH_DEV_TO_LIF(eth_dev);
-	struct ionic_adapter *adapter = lif->adapter;
-
 	IONIC_PRINT_CALL();
 
 	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
 		return 0;
 
-	adapter->lif = NULL;
-
-	ionic_lif_deinit(lif);
-	ionic_lif_free(lif);
+	if (eth_dev->state != RTE_ETH_DEV_UNUSED)
+		ionic_dev_close(eth_dev);
 
-	if (!(lif->state & IONIC_LIF_F_FW_RESET))
-		ionic_lif_reset(lif);
+	eth_dev->dev_ops = NULL;
+	eth_dev->rx_pkt_burst = NULL;
+	eth_dev->tx_pkt_burst = NULL;
+	eth_dev->tx_pkt_prepare = NULL;
 
 	return 0;
 }
@@ -1267,17 +1266,18 @@ eth_ionic_dev_remove(struct rte_device *rte_dev)
 {
 	char name[RTE_ETH_NAME_MAX_LEN];
 	struct rte_eth_dev *eth_dev;
+	int ret = 0;
 
 	/* Adapter lookup is using the eth_dev name */
 	snprintf(name, sizeof(name), "%s_lif", rte_dev->name);
 
 	eth_dev = rte_eth_dev_allocated(name);
 	if (eth_dev)
-		ionic_dev_close(eth_dev);
+		ret = rte_eth_dev_destroy(eth_dev, eth_ionic_dev_uninit);
 	else
 		IONIC_PRINT(DEBUG, "Cannot find device %s", rte_dev->name);
 
-	return 0;
+	return ret;
 }
 
 RTE_LOG_REGISTER_DEFAULT(ionic_logtype, NOTICE);
-- 
2.17.1


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

end of thread, other threads:[~2024-02-07  3:14 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20240202193238.62669-1-andrew.boyer@amd.com>
2024-02-02 19:32 ` [PATCH 04/13] net/ionic: fix missing volatile type for cqe pointers Andrew Boyer
2024-02-03  4:26   ` Stephen Hemminger
2024-02-05 13:33     ` Boyer, Andrew
2024-02-02 19:32 ` [PATCH 07/13] net/ionic: fix RSS query routine Andrew Boyer
2024-02-02 19:32 ` [PATCH 10/13] net/ionic: fix device close sequence to avoid crash Andrew Boyer
2024-02-07  2:18 ` [PATCH v2 04/13] net/ionic: fix missing volatile type for cqe pointers Andrew Boyer
2024-02-07  2:18 ` [PATCH v2 07/13] net/ionic: fix RSS query routine Andrew Boyer
2024-02-07  2:18 ` [PATCH v2 10/13] net/ionic: fix device close sequence to avoid crash Andrew Boyer
     [not found] ` <20240207031317.32293-1-andrew.boyer@amd.com>
2024-02-07  3:13   ` [PATCH v3 04/13] net/ionic: fix missing volatile type for cqe pointers Andrew Boyer
2024-02-07  3:13   ` [PATCH v3 07/13] net/ionic: fix RSS query routine Andrew Boyer
2024-02-07  3:13   ` [PATCH v3 10/13] net/ionic: fix device close sequence to avoid crash Andrew Boyer

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