[PATCH 04/13] net/ionic: fix missing volatile type for cqe pointers

[PATCH 04/13] net/ionic: fix missing volatile type for cqe pointers

[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

Re: [PATCH 04/13] net/ionic: fix missing volatile type for cqe pointers

[Stephen Hemminger]

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

Re: [PATCH 04/13] net/ionic: fix missing volatile type for cqe pointers

[Boyer, Andrew]

> 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

[PATCH 07/13] net/ionic: fix RSS query routine

[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

[PATCH 10/13] net/ionic: fix device close sequence to avoid crash

[Andrew Boyer]

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

[PATCH v2 04/13] net/ionic: fix missing volatile type for cqe pointers

[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

[PATCH v2 07/13] net/ionic: fix RSS query routine

[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

[PATCH v2 10/13] net/ionic: fix device close sequence to avoid crash

[Andrew Boyer]

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

[PATCH v3 04/13] net/ionic: fix missing volatile type for cqe pointers

[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

[PATCH v3 07/13] net/ionic: fix RSS query routine

[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

[PATCH v3 10/13] net/ionic: fix device close sequence to avoid crash

[Andrew Boyer]

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