DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH v5 0/4] Memory corruption due to HW rings allocation
@ 2020-06-23 13:42 Renata Saiakhova
  2020-06-23 13:42 ` [dpdk-dev] [PATCH v5 1/4] ethdev: add function to release HW rings Renata Saiakhova
                   ` (5 more replies)
  0 siblings, 6 replies; 16+ messages in thread
From: Renata Saiakhova @ 2020-06-23 13:42 UTC (permalink / raw)
  To: dev; +Cc: Renata Saiakhova

igb and ixgbe and some other drivers allocate HW rings using
rte_eth_dma_zone_reserve(),
which checks first if the memzone exists for a given name, consisting of port
id, queue_id, rx/tx direction, but not for the size, alignment, and socket_id.
If the memzone with a given name exists it is returned, otherwise it is
allocated.
Disconnecting dpdk port from one type of interface (igb) and connecting it
to another type of interface (ixgbe) for the same port id, potentially creates
memory overlap and corruption, because it may require memzone of bigger size.
That's what is happening from switching from igb to ixgbe having the same port
id.

v4->v5: rte_eth_dma_zone_free() marked as __rte_internal instead of
        __rte_experimental
v3->v4: Improve return code in rte_eth_dma_zone_free() in case mz is not found;
        Release explicitly i40e Flow Director rx/tx rings by calling
        rte_eth_dma_zone_free().
v2->v3: Remove #undef ETH_DMA_MZONE_NAME and minor changes in code standard.
v1->v2: Minor changes on code standard and additional fixes in i40e em and ice
        drivers.

Renata Saiakhova (4):
  ethdev: add function to release HW rings
  drivers/net: fix in igb and ixgbe HW rings memory
  drivers/net: fix in i40e HW rings memory overlap
  drivers/net: fix in em and ice HW rings memory overlap

 drivers/net/e1000/em_rxtx.c              |  2 ++
 drivers/net/e1000/igb_rxtx.c             |  2 ++
 drivers/net/i40e/i40e_fdir.c             |  3 +++
 drivers/net/i40e/i40e_rxtx.c             |  2 ++
 drivers/net/ice/ice_rxtx.c               |  2 ++
 drivers/net/ixgbe/ixgbe_rxtx.c           |  2 ++
 lib/librte_ethdev/rte_ethdev.c           | 30 ++++++++++++++++++++++--
 lib/librte_ethdev/rte_ethdev_driver.h    | 17 ++++++++++++++
 lib/librte_ethdev/rte_ethdev_version.map |  7 ++++++
 9 files changed, 65 insertions(+), 2 deletions(-)

-- 
2.17.2


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

* [dpdk-dev] [PATCH v5 1/4] ethdev: add function to release HW rings
  2020-06-23 13:42 [dpdk-dev] [PATCH v5 0/4] Memory corruption due to HW rings allocation Renata Saiakhova
@ 2020-06-23 13:42 ` Renata Saiakhova
  2020-06-23 13:52   ` Ferruh Yigit
  2020-06-23 15:49   ` Andrew Rybchenko
  2020-06-23 13:42 ` [dpdk-dev] [PATCH v5 2/4] drivers/net: fix in igb and ixgbe HW rings memory Renata Saiakhova
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 16+ messages in thread
From: Renata Saiakhova @ 2020-06-23 13:42 UTC (permalink / raw)
  To: dev; +Cc: Renata Saiakhova

Free previously allocated memzone for HW rings

Signed-off-by: Renata Saiakhova <Renata.Saiakhova@ekinops.com>
---
 lib/librte_ethdev/rte_ethdev.c           | 30 ++++++++++++++++++++++--
 lib/librte_ethdev/rte_ethdev_driver.h    | 17 ++++++++++++++
 lib/librte_ethdev/rte_ethdev_version.map |  7 ++++++
 3 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 8e10a6fc3..e378489a6 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -4184,6 +4184,10 @@ rte_eth_dev_rx_intr_ctl_q_get_fd(uint16_t port_id, uint16_t queue_id)
 	return fd;
 }
 
+#define ETH_DMA_MZONE_NAME(_name, _port_id, _queue_id, _ring_name) \
+	snprintf(_name, sizeof(_name), "eth_p%d_q%d_%s", \
+			_port_id, _queue_id, _ring_name)
+
 const struct rte_memzone *
 rte_eth_dma_zone_reserve(const struct rte_eth_dev *dev, const char *ring_name,
 			 uint16_t queue_id, size_t size, unsigned align,
@@ -4193,8 +4197,7 @@ rte_eth_dma_zone_reserve(const struct rte_eth_dev *dev, const char *ring_name,
 	const struct rte_memzone *mz;
 	int rc;
 
-	rc = snprintf(z_name, sizeof(z_name), "eth_p%d_q%d_%s",
-		      dev->data->port_id, queue_id, ring_name);
+	rc = ETH_DMA_MZONE_NAME(z_name, dev->data->port_id, queue_id, ring_name);
 	if (rc >= RTE_MEMZONE_NAMESIZE) {
 		RTE_ETHDEV_LOG(ERR, "ring name too long\n");
 		rte_errno = ENAMETOOLONG;
@@ -4209,6 +4212,29 @@ rte_eth_dma_zone_reserve(const struct rte_eth_dev *dev, const char *ring_name,
 			RTE_MEMZONE_IOVA_CONTIG, align);
 }
 
+int
+rte_eth_dma_zone_free(const struct rte_eth_dev *dev, const char *ring_name,
+		uint16_t queue_id)
+{
+	char z_name[RTE_MEMZONE_NAMESIZE];
+	const struct rte_memzone *mz;
+	int rc = 0;
+
+	rc = ETH_DMA_MZONE_NAME(z_name, dev->data->port_id, queue_id, ring_name);
+	if (rc >= RTE_MEMZONE_NAMESIZE) {
+		RTE_ETHDEV_LOG(ERR, "ring name too long\n");
+		return -ENAMETOOLONG;
+	}
+
+	mz = rte_memzone_lookup(z_name);
+	if (mz)
+		rc = rte_memzone_free(mz);
+	else
+		rc = -EINVAL;
+
+	return rc;
+}
+
 int
 rte_eth_dev_create(struct rte_device *device, const char *name,
 	size_t priv_data_size,
diff --git a/lib/librte_ethdev/rte_ethdev_driver.h b/lib/librte_ethdev/rte_ethdev_driver.h
index 99d4cd6cd..13fd049c0 100644
--- a/lib/librte_ethdev/rte_ethdev_driver.h
+++ b/lib/librte_ethdev/rte_ethdev_driver.h
@@ -180,6 +180,23 @@ rte_eth_dma_zone_reserve(const struct rte_eth_dev *eth_dev, const char *name,
 			 uint16_t queue_id, size_t size,
 			 unsigned align, int socket_id);
 
+/**
+ * Free previously allocated memzone for HW rings.
+ *
+ * @param eth_dev
+ *   The *eth_dev* pointer is the address of the *rte_eth_dev* structure
+ * @param name
+ *   The name of the memory zone
+ * @param queue_id
+ *   The index of the queue to add to name
+ * @return
+ *   Negative errno value on error, 0 on success.
+ */
+__rte_internal
+int
+rte_eth_dma_zone_free(const struct rte_eth_dev *eth_dev, const char *name,
+		 uint16_t queue_id);
+
 /**
  * @internal
  * Atomically set the link status for the specific device.
diff --git a/lib/librte_ethdev/rte_ethdev_version.map b/lib/librte_ethdev/rte_ethdev_version.map
index 715505604..664d6b86a 100644
--- a/lib/librte_ethdev/rte_ethdev_version.map
+++ b/lib/librte_ethdev/rte_ethdev_version.map
@@ -242,3 +242,10 @@ EXPERIMENTAL {
 	__rte_ethdev_trace_tx_burst;
 	rte_flow_get_aged_flows;
 };
+
+INTERNAL {
+	global:
+
+	rte_eth_dma_zone_free;
+};
+
-- 
2.17.2


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

* [dpdk-dev] [PATCH v5 2/4] drivers/net: fix in igb and ixgbe HW rings memory
  2020-06-23 13:42 [dpdk-dev] [PATCH v5 0/4] Memory corruption due to HW rings allocation Renata Saiakhova
  2020-06-23 13:42 ` [dpdk-dev] [PATCH v5 1/4] ethdev: add function to release HW rings Renata Saiakhova
@ 2020-06-23 13:42 ` Renata Saiakhova
  2020-06-23 13:42 ` [dpdk-dev] [PATCH v5 3/4] drivers/net: fix in i40e HW rings memory overlap Renata Saiakhova
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 16+ messages in thread
From: Renata Saiakhova @ 2020-06-23 13:42 UTC (permalink / raw)
  To: dev; +Cc: Renata Saiakhova

Delete memzones for HW rings in igb and ixgbe while freeing queues

Signed-off-by: Renata Saiakhova <Renata.Saiakhova@ekinops.com>
---
 drivers/net/e1000/igb_rxtx.c   | 2 ++
 drivers/net/ixgbe/ixgbe_rxtx.c | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/drivers/net/e1000/igb_rxtx.c b/drivers/net/e1000/igb_rxtx.c
index 684fa4ad8..fe80c0f0d 100644
--- a/drivers/net/e1000/igb_rxtx.c
+++ b/drivers/net/e1000/igb_rxtx.c
@@ -1884,12 +1884,14 @@ igb_dev_free_queues(struct rte_eth_dev *dev)
 	for (i = 0; i < dev->data->nb_rx_queues; i++) {
 		eth_igb_rx_queue_release(dev->data->rx_queues[i]);
 		dev->data->rx_queues[i] = NULL;
+		rte_eth_dma_zone_free(dev, "rx_ring", i);
 	}
 	dev->data->nb_rx_queues = 0;
 
 	for (i = 0; i < dev->data->nb_tx_queues; i++) {
 		eth_igb_tx_queue_release(dev->data->tx_queues[i]);
 		dev->data->tx_queues[i] = NULL;
+		rte_eth_dma_zone_free(dev, "tx_ring", i);
 	}
 	dev->data->nb_tx_queues = 0;
 }
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
index 2e20e18c7..977ecf513 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx.c
@@ -3368,12 +3368,14 @@ ixgbe_dev_free_queues(struct rte_eth_dev *dev)
 	for (i = 0; i < dev->data->nb_rx_queues; i++) {
 		ixgbe_dev_rx_queue_release(dev->data->rx_queues[i]);
 		dev->data->rx_queues[i] = NULL;
+		rte_eth_dma_zone_free(dev, "rx_ring", i);
 	}
 	dev->data->nb_rx_queues = 0;
 
 	for (i = 0; i < dev->data->nb_tx_queues; i++) {
 		ixgbe_dev_tx_queue_release(dev->data->tx_queues[i]);
 		dev->data->tx_queues[i] = NULL;
+		rte_eth_dma_zone_free(dev, "tx_ring", i);
 	}
 	dev->data->nb_tx_queues = 0;
 }
-- 
2.17.2


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

* [dpdk-dev] [PATCH v5 3/4] drivers/net: fix in i40e HW rings memory overlap
  2020-06-23 13:42 [dpdk-dev] [PATCH v5 0/4] Memory corruption due to HW rings allocation Renata Saiakhova
  2020-06-23 13:42 ` [dpdk-dev] [PATCH v5 1/4] ethdev: add function to release HW rings Renata Saiakhova
  2020-06-23 13:42 ` [dpdk-dev] [PATCH v5 2/4] drivers/net: fix in igb and ixgbe HW rings memory Renata Saiakhova
@ 2020-06-23 13:42 ` Renata Saiakhova
  2020-06-23 13:42 ` [dpdk-dev] [PATCH v5 4/4] drivers/net: fix in em and ice " Renata Saiakhova
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 16+ messages in thread
From: Renata Saiakhova @ 2020-06-23 13:42 UTC (permalink / raw)
  To: dev; +Cc: Renata Saiakhova

Delete memzones for HW rings in i40e while freeing queues

Signed-off-by: Renata Saiakhova <Renata.Saiakhova@ekinops.com>
---
 drivers/net/i40e/i40e_fdir.c | 3 +++
 drivers/net/i40e/i40e_rxtx.c | 2 ++
 2 files changed, 5 insertions(+)

diff --git a/drivers/net/i40e/i40e_fdir.c b/drivers/net/i40e/i40e_fdir.c
index d59399afe..3a82cafac 100644
--- a/drivers/net/i40e/i40e_fdir.c
+++ b/drivers/net/i40e/i40e_fdir.c
@@ -269,6 +269,7 @@ i40e_fdir_teardown(struct i40e_pf *pf)
 {
 	struct i40e_hw *hw = I40E_PF_TO_HW(pf);
 	struct i40e_vsi *vsi;
+	struct rte_eth_dev *dev = pf->adapter->eth_dev;
 
 	vsi = pf->fdir.fdir_vsi;
 	if (!vsi)
@@ -280,8 +281,10 @@ i40e_fdir_teardown(struct i40e_pf *pf)
 	if (err)
 		PMD_DRV_LOG(DEBUG, "Failed to do FDIR RX switch off");
 	i40e_dev_rx_queue_release(pf->fdir.rxq);
+	rte_eth_dma_zone_free(dev, "fdir_rx_ring", pf->fdir.rxq->queue_id);
 	pf->fdir.rxq = NULL;
 	i40e_dev_tx_queue_release(pf->fdir.txq);
+	rte_eth_dma_zone_free(dev, "fdir_tx_ring", pf->fdir.txq->queue_id);
 	pf->fdir.txq = NULL;
 	i40e_vsi_release(vsi);
 	pf->fdir.fdir_vsi = NULL;
diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
index 840b6f387..2d2efb71a 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -2928,6 +2928,7 @@ i40e_dev_free_queues(struct rte_eth_dev *dev)
 			continue;
 		i40e_dev_rx_queue_release(dev->data->rx_queues[i]);
 		dev->data->rx_queues[i] = NULL;
+		rte_eth_dma_zone_free(dev, "rx_ring", i);
 	}
 
 	for (i = 0; i < dev->data->nb_tx_queues; i++) {
@@ -2935,6 +2936,7 @@ i40e_dev_free_queues(struct rte_eth_dev *dev)
 			continue;
 		i40e_dev_tx_queue_release(dev->data->tx_queues[i]);
 		dev->data->tx_queues[i] = NULL;
+		rte_eth_dma_zone_free(dev, "tx_ring", i);
 	}
 }
 
-- 
2.17.2


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

* [dpdk-dev] [PATCH v5 4/4] drivers/net: fix in em and ice HW rings memory overlap
  2020-06-23 13:42 [dpdk-dev] [PATCH v5 0/4] Memory corruption due to HW rings allocation Renata Saiakhova
                   ` (2 preceding siblings ...)
  2020-06-23 13:42 ` [dpdk-dev] [PATCH v5 3/4] drivers/net: fix in i40e HW rings memory overlap Renata Saiakhova
@ 2020-06-23 13:42 ` Renata Saiakhova
  2020-06-23 13:53 ` [dpdk-dev] [PATCH v5 0/4] Memory corruption due to HW rings allocation Ferruh Yigit
  2020-07-10 14:42 ` [dpdk-dev] [PATCH v6 1/2] ethdev: add function to release HW rings Ferruh Yigit
  5 siblings, 0 replies; 16+ messages in thread
From: Renata Saiakhova @ 2020-06-23 13:42 UTC (permalink / raw)
  To: dev; +Cc: Renata Saiakhova

Delete memzones for HW rings in em and ice while freeing queues

Signed-off-by: Renata Saiakhova <Renata.Saiakhova@ekinops.com>
---
 drivers/net/e1000/em_rxtx.c | 2 ++
 drivers/net/ice/ice_rxtx.c  | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/drivers/net/e1000/em_rxtx.c b/drivers/net/e1000/em_rxtx.c
index 49c53712a..67a271e8c 100644
--- a/drivers/net/e1000/em_rxtx.c
+++ b/drivers/net/e1000/em_rxtx.c
@@ -1611,12 +1611,14 @@ em_dev_free_queues(struct rte_eth_dev *dev)
 	for (i = 0; i < dev->data->nb_rx_queues; i++) {
 		eth_em_rx_queue_release(dev->data->rx_queues[i]);
 		dev->data->rx_queues[i] = NULL;
+		rte_eth_dma_zone_free(dev, "rx_ring", i);
 	}
 	dev->data->nb_rx_queues = 0;
 
 	for (i = 0; i < dev->data->nb_tx_queues; i++) {
 		eth_em_tx_queue_release(dev->data->tx_queues[i]);
 		dev->data->tx_queues[i] = NULL;
+		rte_eth_dma_zone_free(dev, "tx_ring", i);
 	}
 	dev->data->nb_tx_queues = 0;
 }
diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.c
index 5d6f693c5..cc3139042 100644
--- a/drivers/net/ice/ice_rxtx.c
+++ b/drivers/net/ice/ice_rxtx.c
@@ -1905,6 +1905,7 @@ ice_free_queues(struct rte_eth_dev *dev)
 			continue;
 		ice_rx_queue_release(dev->data->rx_queues[i]);
 		dev->data->rx_queues[i] = NULL;
+		rte_eth_dma_zone_free(dev, "rx_ring", i);
 	}
 	dev->data->nb_rx_queues = 0;
 
@@ -1913,6 +1914,7 @@ ice_free_queues(struct rte_eth_dev *dev)
 			continue;
 		ice_tx_queue_release(dev->data->tx_queues[i]);
 		dev->data->tx_queues[i] = NULL;
+		rte_eth_dma_zone_free(dev, "tx_ring", i);
 	}
 	dev->data->nb_tx_queues = 0;
 }
-- 
2.17.2


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

* Re: [dpdk-dev] [PATCH v5 1/4] ethdev: add function to release HW rings
  2020-06-23 13:42 ` [dpdk-dev] [PATCH v5 1/4] ethdev: add function to release HW rings Renata Saiakhova
@ 2020-06-23 13:52   ` Ferruh Yigit
  2020-06-23 15:49   ` Andrew Rybchenko
  1 sibling, 0 replies; 16+ messages in thread
From: Ferruh Yigit @ 2020-06-23 13:52 UTC (permalink / raw)
  To: Renata Saiakhova, dev

On 6/23/2020 2:42 PM, Renata Saiakhova wrote:
> Free previously allocated memzone for HW rings
> 
> Signed-off-by: Renata Saiakhova <Renata.Saiakhova@ekinops.com>

Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>


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

* Re: [dpdk-dev] [PATCH v5 0/4] Memory corruption due to HW rings allocation
  2020-06-23 13:42 [dpdk-dev] [PATCH v5 0/4] Memory corruption due to HW rings allocation Renata Saiakhova
                   ` (3 preceding siblings ...)
  2020-06-23 13:42 ` [dpdk-dev] [PATCH v5 4/4] drivers/net: fix in em and ice " Renata Saiakhova
@ 2020-06-23 13:53 ` Ferruh Yigit
  2020-07-10 14:42 ` [dpdk-dev] [PATCH v6 1/2] ethdev: add function to release HW rings Ferruh Yigit
  5 siblings, 0 replies; 16+ messages in thread
From: Ferruh Yigit @ 2020-06-23 13:53 UTC (permalink / raw)
  To: Renata Saiakhova, dev

On 6/23/2020 2:42 PM, Renata Saiakhova wrote:
> igb and ixgbe and some other drivers allocate HW rings using
> rte_eth_dma_zone_reserve(),
> which checks first if the memzone exists for a given name, consisting of port
> id, queue_id, rx/tx direction, but not for the size, alignment, and socket_id.
> If the memzone with a given name exists it is returned, otherwise it is
> allocated.
> Disconnecting dpdk port from one type of interface (igb) and connecting it
> to another type of interface (ixgbe) for the same port id, potentially creates
> memory overlap and corruption, because it may require memzone of bigger size.
> That's what is happening from switching from igb to ixgbe having the same port
> id.
> 
> v4->v5: rte_eth_dma_zone_free() marked as __rte_internal instead of
>         __rte_experimental
> v3->v4: Improve return code in rte_eth_dma_zone_free() in case mz is not found;
>         Release explicitly i40e Flow Director rx/tx rings by calling
>         rte_eth_dma_zone_free().
> v2->v3: Remove #undef ETH_DMA_MZONE_NAME and minor changes in code standard.
> v1->v2: Minor changes on code standard and additional fixes in i40e em and ice
>         drivers.
> 
> Renata Saiakhova (4):
>   ethdev: add function to release HW rings
>   drivers/net: fix in igb and ixgbe HW rings memory
>   drivers/net: fix in i40e HW rings memory overlap
>   drivers/net: fix in em and ice HW rings memory overlap

I think good to keep driver patches separate to help the review and acks, but we
can squash them into single patch while merging.

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

* Re: [dpdk-dev] [PATCH v5 1/4] ethdev: add function to release HW rings
  2020-06-23 13:42 ` [dpdk-dev] [PATCH v5 1/4] ethdev: add function to release HW rings Renata Saiakhova
  2020-06-23 13:52   ` Ferruh Yigit
@ 2020-06-23 15:49   ` Andrew Rybchenko
  1 sibling, 0 replies; 16+ messages in thread
From: Andrew Rybchenko @ 2020-06-23 15:49 UTC (permalink / raw)
  To: Renata Saiakhova, dev; +Cc: Thomas Monjalon, Ferruh Yigit

Hi Renata,

Looks good to me except to minor notes below.

It is better to use:
 --to-cmd ./devtools/get-maintainer.sh --cc dev@dpdk.org
to have required list of maintainers directly specified.

On 6/23/20 4:42 PM, Renata Saiakhova wrote:
> Free previously allocated memzone for HW rings
> 
> Signed-off-by: Renata Saiakhova <Renata.Saiakhova@ekinops.com>

with minor notes below fixed:
Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>

> ---
>  lib/librte_ethdev/rte_ethdev.c           | 30 ++++++++++++++++++++++--
>  lib/librte_ethdev/rte_ethdev_driver.h    | 17 ++++++++++++++
>  lib/librte_ethdev/rte_ethdev_version.map |  7 ++++++
>  3 files changed, 52 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
> index 8e10a6fc3..e378489a6 100644
> --- a/lib/librte_ethdev/rte_ethdev.c
> +++ b/lib/librte_ethdev/rte_ethdev.c
> @@ -4184,6 +4184,10 @@ rte_eth_dev_rx_intr_ctl_q_get_fd(uint16_t port_id, uint16_t queue_id)
>  	return fd;
>  }
>  
> +#define ETH_DMA_MZONE_NAME(_name, _port_id, _queue_id, _ring_name) \
> +	snprintf(_name, sizeof(_name), "eth_p%d_q%d_%s", \
> +			_port_id, _queue_id, _ring_name)
> +

I think that small static function would be better here.
The macro is used to hide "sizeof(_name)", but I don't
think it is good, since it is error-prone if _name is
a pointer (not an array).

>  const struct rte_memzone *
>  rte_eth_dma_zone_reserve(const struct rte_eth_dev *dev, const char *ring_name,
>  			 uint16_t queue_id, size_t size, unsigned align,
> @@ -4193,8 +4197,7 @@ rte_eth_dma_zone_reserve(const struct rte_eth_dev *dev, const char *ring_name,
>  	const struct rte_memzone *mz;
>  	int rc;
>  
> -	rc = snprintf(z_name, sizeof(z_name), "eth_p%d_q%d_%s",
> -		      dev->data->port_id, queue_id, ring_name);
> +	rc = ETH_DMA_MZONE_NAME(z_name, dev->data->port_id, queue_id, ring_name);
>  	if (rc >= RTE_MEMZONE_NAMESIZE) {
>  		RTE_ETHDEV_LOG(ERR, "ring name too long\n");
>  		rte_errno = ENAMETOOLONG;
> @@ -4209,6 +4212,29 @@ rte_eth_dma_zone_reserve(const struct rte_eth_dev *dev, const char *ring_name,
>  			RTE_MEMZONE_IOVA_CONTIG, align);
>  }
>  
> +int
> +rte_eth_dma_zone_free(const struct rte_eth_dev *dev, const char *ring_name,
> +		uint16_t queue_id)
> +{
> +	char z_name[RTE_MEMZONE_NAMESIZE];
> +	const struct rte_memzone *mz;
> +	int rc = 0;
> +
> +	rc = ETH_DMA_MZONE_NAME(z_name, dev->data->port_id, queue_id, ring_name);
> +	if (rc >= RTE_MEMZONE_NAMESIZE) {
> +		RTE_ETHDEV_LOG(ERR, "ring name too long\n");
> +		return -ENAMETOOLONG;
> +	}
> +
> +	mz = rte_memzone_lookup(z_name);
> +	if (mz)
> +		rc = rte_memzone_free(mz);
> +	else
> +		rc = -EINVAL;

May be -ENOENT is better here?

> +
> +	return rc;
> +}
> +
>  int
>  rte_eth_dev_create(struct rte_device *device, const char *name,
>  	size_t priv_data_size,
> diff --git a/lib/librte_ethdev/rte_ethdev_driver.h b/lib/librte_ethdev/rte_ethdev_driver.h
> index 99d4cd6cd..13fd049c0 100644
> --- a/lib/librte_ethdev/rte_ethdev_driver.h
> +++ b/lib/librte_ethdev/rte_ethdev_driver.h
> @@ -180,6 +180,23 @@ rte_eth_dma_zone_reserve(const struct rte_eth_dev *eth_dev, const char *name,
>  			 uint16_t queue_id, size_t size,
>  			 unsigned align, int socket_id);
>  
> +/**
> + * Free previously allocated memzone for HW rings.
> + *
> + * @param eth_dev
> + *   The *eth_dev* pointer is the address of the *rte_eth_dev* structure
> + * @param name
> + *   The name of the memory zone
> + * @param queue_id
> + *   The index of the queue to add to name
> + * @return
> + *   Negative errno value on error, 0 on success.
> + */
> +__rte_internal
> +int
> +rte_eth_dma_zone_free(const struct rte_eth_dev *eth_dev, const char *name,
> +		 uint16_t queue_id);
> +
>  /**
>   * @internal
>   * Atomically set the link status for the specific device.
> diff --git a/lib/librte_ethdev/rte_ethdev_version.map b/lib/librte_ethdev/rte_ethdev_version.map
> index 715505604..664d6b86a 100644
> --- a/lib/librte_ethdev/rte_ethdev_version.map
> +++ b/lib/librte_ethdev/rte_ethdev_version.map
> @@ -242,3 +242,10 @@ EXPERIMENTAL {
>  	__rte_ethdev_trace_tx_burst;
>  	rte_flow_get_aged_flows;
>  };
> +
> +INTERNAL {
> +	global:
> +
> +	rte_eth_dma_zone_free;
> +};
> +
> 


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

* [dpdk-dev] [PATCH v6 1/2] ethdev: add function to release HW rings
  2020-06-23 13:42 [dpdk-dev] [PATCH v5 0/4] Memory corruption due to HW rings allocation Renata Saiakhova
                   ` (4 preceding siblings ...)
  2020-06-23 13:53 ` [dpdk-dev] [PATCH v5 0/4] Memory corruption due to HW rings allocation Ferruh Yigit
@ 2020-07-10 14:42 ` Ferruh Yigit
  2020-07-10 14:42   ` [dpdk-dev] [PATCH v6 2/2] drivers/net: delete HW rings while freeing queues Ferruh Yigit
  2020-07-10 21:43   ` [dpdk-dev] [PATCH v7 1/3] drivers/net: fix build with internal API usage Ferruh Yigit
  5 siblings, 2 replies; 16+ messages in thread
From: Ferruh Yigit @ 2020-07-10 14:42 UTC (permalink / raw)
  To: Thomas Monjalon, Andrew Rybchenko, Ray Kinsella, Neil Horman
  Cc: dev, Ferruh Yigit, Renata Saiakhova

From: Renata Saiakhova <renata.saiakhova@ekinops.com>

Free previously allocated memzone for HW rings

Signed-off-by: Renata Saiakhova <renata.saiakhova@ekinops.com>
Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
v6:
* Converted macro to static inline function
* Updated API return error value
---
 lib/librte_ethdev/rte_ethdev.c           | 36 ++++++++++++++++++++++--
 lib/librte_ethdev/rte_ethdev_driver.h    | 17 +++++++++++
 lib/librte_ethdev/rte_ethdev_version.map |  7 +++++
 3 files changed, 58 insertions(+), 2 deletions(-)

diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 5ff52046dc..049d0a684c 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -4200,6 +4200,14 @@ rte_eth_dev_rx_intr_ctl_q_get_fd(uint16_t port_id, uint16_t queue_id)
 	return fd;
 }
 
+static inline int
+eth_dma_mzone_name(char *name, size_t len, uint16_t port_id, uint16_t queue_id,
+		const char *ring_name)
+{
+	return snprintf(name, len, "eth_p%d_q%d_%s",
+			port_id, queue_id, ring_name);
+}
+
 const struct rte_memzone *
 rte_eth_dma_zone_reserve(const struct rte_eth_dev *dev, const char *ring_name,
 			 uint16_t queue_id, size_t size, unsigned align,
@@ -4209,8 +4217,8 @@ rte_eth_dma_zone_reserve(const struct rte_eth_dev *dev, const char *ring_name,
 	const struct rte_memzone *mz;
 	int rc;
 
-	rc = snprintf(z_name, sizeof(z_name), "eth_p%d_q%d_%s",
-		      dev->data->port_id, queue_id, ring_name);
+	rc = eth_dma_mzone_name(z_name, sizeof(z_name), dev->data->port_id,
+			queue_id, ring_name);
 	if (rc >= RTE_MEMZONE_NAMESIZE) {
 		RTE_ETHDEV_LOG(ERR, "ring name too long\n");
 		rte_errno = ENAMETOOLONG;
@@ -4235,6 +4243,30 @@ rte_eth_dma_zone_reserve(const struct rte_eth_dev *dev, const char *ring_name,
 			RTE_MEMZONE_IOVA_CONTIG, align);
 }
 
+int
+rte_eth_dma_zone_free(const struct rte_eth_dev *dev, const char *ring_name,
+		uint16_t queue_id)
+{
+	char z_name[RTE_MEMZONE_NAMESIZE];
+	const struct rte_memzone *mz;
+	int rc = 0;
+
+	rc = eth_dma_mzone_name(z_name, sizeof(z_name), dev->data->port_id,
+			queue_id, ring_name);
+	if (rc >= RTE_MEMZONE_NAMESIZE) {
+		RTE_ETHDEV_LOG(ERR, "ring name too long\n");
+		return -ENAMETOOLONG;
+	}
+
+	mz = rte_memzone_lookup(z_name);
+	if (mz)
+		rc = rte_memzone_free(mz);
+	else
+		rc = -ENOENT;
+
+	return rc;
+}
+
 int
 rte_eth_dev_create(struct rte_device *device, const char *name,
 	size_t priv_data_size,
diff --git a/lib/librte_ethdev/rte_ethdev_driver.h b/lib/librte_ethdev/rte_ethdev_driver.h
index 99d4cd6cd0..13fd049c0d 100644
--- a/lib/librte_ethdev/rte_ethdev_driver.h
+++ b/lib/librte_ethdev/rte_ethdev_driver.h
@@ -180,6 +180,23 @@ rte_eth_dma_zone_reserve(const struct rte_eth_dev *eth_dev, const char *name,
 			 uint16_t queue_id, size_t size,
 			 unsigned align, int socket_id);
 
+/**
+ * Free previously allocated memzone for HW rings.
+ *
+ * @param eth_dev
+ *   The *eth_dev* pointer is the address of the *rte_eth_dev* structure
+ * @param name
+ *   The name of the memory zone
+ * @param queue_id
+ *   The index of the queue to add to name
+ * @return
+ *   Negative errno value on error, 0 on success.
+ */
+__rte_internal
+int
+rte_eth_dma_zone_free(const struct rte_eth_dev *eth_dev, const char *name,
+		 uint16_t queue_id);
+
 /**
  * @internal
  * Atomically set the link status for the specific device.
diff --git a/lib/librte_ethdev/rte_ethdev_version.map b/lib/librte_ethdev/rte_ethdev_version.map
index 7155056045..664d6b86a2 100644
--- a/lib/librte_ethdev/rte_ethdev_version.map
+++ b/lib/librte_ethdev/rte_ethdev_version.map
@@ -242,3 +242,10 @@ EXPERIMENTAL {
 	__rte_ethdev_trace_tx_burst;
 	rte_flow_get_aged_flows;
 };
+
+INTERNAL {
+	global:
+
+	rte_eth_dma_zone_free;
+};
+
-- 
2.25.4


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

* [dpdk-dev] [PATCH v6 2/2] drivers/net: delete HW rings while freeing queues
  2020-07-10 14:42 ` [dpdk-dev] [PATCH v6 1/2] ethdev: add function to release HW rings Ferruh Yigit
@ 2020-07-10 14:42   ` Ferruh Yigit
  2020-07-10 21:43   ` [dpdk-dev] [PATCH v7 1/3] drivers/net: fix build with internal API usage Ferruh Yigit
  1 sibling, 0 replies; 16+ messages in thread
From: Ferruh Yigit @ 2020-07-10 14:42 UTC (permalink / raw)
  To: Wei Zhao, Jeff Guo, Beilei Xing, Qiming Yang, Qi Zhang
  Cc: dev, Ferruh Yigit, Renata Saiakhova

From: Renata Saiakhova <renata.saiakhova@ekinops.com>

Delete memzones for HW rings in igb and ixgbe while freeing queues

Updated igb, ixgbe, i40e, ice & em drivers.

Signed-off-by: Renata Saiakhova <renata.saiakhova@ekinops.com>
Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
 drivers/net/e1000/em_rxtx.c    | 2 ++
 drivers/net/e1000/igb_rxtx.c   | 2 ++
 drivers/net/i40e/i40e_fdir.c   | 3 +++
 drivers/net/i40e/i40e_rxtx.c   | 2 ++
 drivers/net/ice/ice_rxtx.c     | 2 ++
 drivers/net/ixgbe/ixgbe_rxtx.c | 2 ++
 6 files changed, 13 insertions(+)

diff --git a/drivers/net/e1000/em_rxtx.c b/drivers/net/e1000/em_rxtx.c
index 49c53712a2..67a271e8ce 100644
--- a/drivers/net/e1000/em_rxtx.c
+++ b/drivers/net/e1000/em_rxtx.c
@@ -1611,12 +1611,14 @@ em_dev_free_queues(struct rte_eth_dev *dev)
 	for (i = 0; i < dev->data->nb_rx_queues; i++) {
 		eth_em_rx_queue_release(dev->data->rx_queues[i]);
 		dev->data->rx_queues[i] = NULL;
+		rte_eth_dma_zone_free(dev, "rx_ring", i);
 	}
 	dev->data->nb_rx_queues = 0;
 
 	for (i = 0; i < dev->data->nb_tx_queues; i++) {
 		eth_em_tx_queue_release(dev->data->tx_queues[i]);
 		dev->data->tx_queues[i] = NULL;
+		rte_eth_dma_zone_free(dev, "tx_ring", i);
 	}
 	dev->data->nb_tx_queues = 0;
 }
diff --git a/drivers/net/e1000/igb_rxtx.c b/drivers/net/e1000/igb_rxtx.c
index 6883202842..5717cdb706 100644
--- a/drivers/net/e1000/igb_rxtx.c
+++ b/drivers/net/e1000/igb_rxtx.c
@@ -1884,12 +1884,14 @@ igb_dev_free_queues(struct rte_eth_dev *dev)
 	for (i = 0; i < dev->data->nb_rx_queues; i++) {
 		eth_igb_rx_queue_release(dev->data->rx_queues[i]);
 		dev->data->rx_queues[i] = NULL;
+		rte_eth_dma_zone_free(dev, "rx_ring", i);
 	}
 	dev->data->nb_rx_queues = 0;
 
 	for (i = 0; i < dev->data->nb_tx_queues; i++) {
 		eth_igb_tx_queue_release(dev->data->tx_queues[i]);
 		dev->data->tx_queues[i] = NULL;
+		rte_eth_dma_zone_free(dev, "tx_ring", i);
 	}
 	dev->data->nb_tx_queues = 0;
 }
diff --git a/drivers/net/i40e/i40e_fdir.c b/drivers/net/i40e/i40e_fdir.c
index 4a778db4c6..71eb31fb8b 100644
--- a/drivers/net/i40e/i40e_fdir.c
+++ b/drivers/net/i40e/i40e_fdir.c
@@ -269,6 +269,7 @@ i40e_fdir_teardown(struct i40e_pf *pf)
 {
 	struct i40e_hw *hw = I40E_PF_TO_HW(pf);
 	struct i40e_vsi *vsi;
+	struct rte_eth_dev *dev = pf->adapter->eth_dev;
 
 	vsi = pf->fdir.fdir_vsi;
 	if (!vsi)
@@ -280,8 +281,10 @@ i40e_fdir_teardown(struct i40e_pf *pf)
 	if (err)
 		PMD_DRV_LOG(DEBUG, "Failed to do FDIR RX switch off");
 	i40e_dev_rx_queue_release(pf->fdir.rxq);
+	rte_eth_dma_zone_free(dev, "fdir_rx_ring", pf->fdir.rxq->queue_id);
 	pf->fdir.rxq = NULL;
 	i40e_dev_tx_queue_release(pf->fdir.txq);
+	rte_eth_dma_zone_free(dev, "fdir_tx_ring", pf->fdir.txq->queue_id);
 	pf->fdir.txq = NULL;
 	i40e_vsi_release(vsi);
 	pf->fdir.fdir_vsi = NULL;
diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
index 840b6f387f..2d2efb71a5 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -2928,6 +2928,7 @@ i40e_dev_free_queues(struct rte_eth_dev *dev)
 			continue;
 		i40e_dev_rx_queue_release(dev->data->rx_queues[i]);
 		dev->data->rx_queues[i] = NULL;
+		rte_eth_dma_zone_free(dev, "rx_ring", i);
 	}
 
 	for (i = 0; i < dev->data->nb_tx_queues; i++) {
@@ -2935,6 +2936,7 @@ i40e_dev_free_queues(struct rte_eth_dev *dev)
 			continue;
 		i40e_dev_tx_queue_release(dev->data->tx_queues[i]);
 		dev->data->tx_queues[i] = NULL;
+		rte_eth_dma_zone_free(dev, "tx_ring", i);
 	}
 }
 
diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.c
index 5d6f693c52..cc3139042e 100644
--- a/drivers/net/ice/ice_rxtx.c
+++ b/drivers/net/ice/ice_rxtx.c
@@ -1905,6 +1905,7 @@ ice_free_queues(struct rte_eth_dev *dev)
 			continue;
 		ice_rx_queue_release(dev->data->rx_queues[i]);
 		dev->data->rx_queues[i] = NULL;
+		rte_eth_dma_zone_free(dev, "rx_ring", i);
 	}
 	dev->data->nb_rx_queues = 0;
 
@@ -1913,6 +1914,7 @@ ice_free_queues(struct rte_eth_dev *dev)
 			continue;
 		ice_tx_queue_release(dev->data->tx_queues[i]);
 		dev->data->tx_queues[i] = NULL;
+		rte_eth_dma_zone_free(dev, "tx_ring", i);
 	}
 	dev->data->nb_tx_queues = 0;
 }
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
index 2e20e18c7a..977ecf5137 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx.c
@@ -3368,12 +3368,14 @@ ixgbe_dev_free_queues(struct rte_eth_dev *dev)
 	for (i = 0; i < dev->data->nb_rx_queues; i++) {
 		ixgbe_dev_rx_queue_release(dev->data->rx_queues[i]);
 		dev->data->rx_queues[i] = NULL;
+		rte_eth_dma_zone_free(dev, "rx_ring", i);
 	}
 	dev->data->nb_rx_queues = 0;
 
 	for (i = 0; i < dev->data->nb_tx_queues; i++) {
 		ixgbe_dev_tx_queue_release(dev->data->tx_queues[i]);
 		dev->data->tx_queues[i] = NULL;
+		rte_eth_dma_zone_free(dev, "tx_ring", i);
 	}
 	dev->data->nb_tx_queues = 0;
 }
-- 
2.25.4


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

* [dpdk-dev] [PATCH v7 1/3] drivers/net: fix build with internal API usage
  2020-07-10 14:42 ` [dpdk-dev] [PATCH v6 1/2] ethdev: add function to release HW rings Ferruh Yigit
  2020-07-10 14:42   ` [dpdk-dev] [PATCH v6 2/2] drivers/net: delete HW rings while freeing queues Ferruh Yigit
@ 2020-07-10 21:43   ` Ferruh Yigit
  2020-07-10 21:43     ` [dpdk-dev] [PATCH v7 2/3] ethdev: add function to release HW rings Ferruh Yigit
                       ` (2 more replies)
  1 sibling, 3 replies; 16+ messages in thread
From: Ferruh Yigit @ 2020-07-10 21:43 UTC (permalink / raw)
  To: Igor Russkikh, Pavel Belous, Ajit Khaparde, Somnath Kotur,
	Hemant Agrawal, Sachin Saxena, Beilei Xing, Jeff Guo, Wei Zhao,
	Andrew Rybchenko, Thomas Monjalon, Shreyansh Jain
  Cc: dev, Ferruh Yigit, stable, David Marchand

Using '__rte_internal' tag in 'rte_ethdev_driver.h' causing build error
for applications and examples. Because they don't define
'ALLOW_INTERNAL_API' flag and '__rte_internal' causes the error.
This patch is preparation for future '__rte_internal' usage.

At first place, applications/examples should not include
'rte_ethdev_driver.h', this is happening because of PMD public header
files include 'rte_ethdev_driver.h' by mistake.

Updated PMD public header files to not include internal header files.

But for unit test application, 'app/test', enable accessing internal
APIs, since some unit tests need them.

Fixes: ffc905f3b856 ("ethdev: separate driver APIs")
Fixes: ec0dec44ecb9 ("net/atlantic: enable MACsec configuration")
Cc: stable@dpdk.org

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
Cc: David Marchand <david.marchand@redhat.com>
---
 app/test/meson.build                    | 3 +++
 drivers/net/atlantic/rte_pmd_atlantic.h | 2 +-
 drivers/net/bnxt/rte_pmd_bnxt.h         | 3 ++-
 drivers/net/dpaa/rte_pmd_dpaa.h         | 2 --
 drivers/net/i40e/rte_pmd_i40e.h         | 4 +++-
 drivers/net/ixgbe/rte_pmd_ixgbe.h       | 4 +++-
 6 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/app/test/meson.build b/app/test/meson.build
index e0d33ea5ef..786a213972 100644
--- a/app/test/meson.build
+++ b/app/test/meson.build
@@ -396,6 +396,9 @@ cflags += '-D_GNU_SOURCE'
 # Strict-aliasing rules are violated by uint8_t[] to context size casts.
 cflags += '-fno-strict-aliasing'
 
+# Enable using internal APIs in unit tests
+cflags += ['-DALLOW_INTERNAL_API']
+
 test_dep_objs = []
 if dpdk_conf.has('RTE_LIBRTE_COMPRESSDEV')
 	compress_test_dep = dependency('zlib', required: false)
diff --git a/drivers/net/atlantic/rte_pmd_atlantic.h b/drivers/net/atlantic/rte_pmd_atlantic.h
index c0208569b6..0100fc16e5 100644
--- a/drivers/net/atlantic/rte_pmd_atlantic.h
+++ b/drivers/net/atlantic/rte_pmd_atlantic.h
@@ -11,7 +11,7 @@
 #ifndef _PMD_ATLANTIC_H_
 #define _PMD_ATLANTIC_H_
 
-#include <rte_ethdev_driver.h>
+#include <rte_compat.h>
 
 /**
  * @warning
diff --git a/drivers/net/bnxt/rte_pmd_bnxt.h b/drivers/net/bnxt/rte_pmd_bnxt.h
index 2e893cc7bf..81d0d0e032 100644
--- a/drivers/net/bnxt/rte_pmd_bnxt.h
+++ b/drivers/net/bnxt/rte_pmd_bnxt.h
@@ -6,7 +6,8 @@
 #ifndef _PMD_BNXT_H_
 #define _PMD_BNXT_H_
 
-#include <rte_ethdev_driver.h>
+#include <rte_ethdev.h>
+#include <rte_ether.h>
 
 /*
  * Response sent back to the caller after callback
diff --git a/drivers/net/dpaa/rte_pmd_dpaa.h b/drivers/net/dpaa/rte_pmd_dpaa.h
index 37eea9b032..8d244bb491 100644
--- a/drivers/net/dpaa/rte_pmd_dpaa.h
+++ b/drivers/net/dpaa/rte_pmd_dpaa.h
@@ -15,8 +15,6 @@
  *
  */
 
-#include <rte_ethdev_driver.h>
-
 /**
  * Enable/Disable TX loopback
  *
diff --git a/drivers/net/i40e/rte_pmd_i40e.h b/drivers/net/i40e/rte_pmd_i40e.h
index 0f6715efc8..fc3560c28c 100644
--- a/drivers/net/i40e/rte_pmd_i40e.h
+++ b/drivers/net/i40e/rte_pmd_i40e.h
@@ -14,7 +14,9 @@
  *
  */
 
-#include <rte_ethdev_driver.h>
+#include <rte_compat.h>
+#include <rte_ethdev.h>
+#include <rte_ether.h>
 
 /**
  * Response sent back to i40e driver from user app after callback
diff --git a/drivers/net/ixgbe/rte_pmd_ixgbe.h b/drivers/net/ixgbe/rte_pmd_ixgbe.h
index 8b6bb99a58..90fc8160b1 100644
--- a/drivers/net/ixgbe/rte_pmd_ixgbe.h
+++ b/drivers/net/ixgbe/rte_pmd_ixgbe.h
@@ -11,7 +11,9 @@
 #ifndef _PMD_IXGBE_H_
 #define _PMD_IXGBE_H_
 
-#include <rte_ethdev_driver.h>
+#include <rte_compat.h>
+#include <rte_ethdev.h>
+#include <rte_ether.h>
 
 /**
  * Notify VF when PF link status changes.
-- 
2.25.4


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

* [dpdk-dev] [PATCH v7 2/3] ethdev: add function to release HW rings
  2020-07-10 21:43   ` [dpdk-dev] [PATCH v7 1/3] drivers/net: fix build with internal API usage Ferruh Yigit
@ 2020-07-10 21:43     ` Ferruh Yigit
  2020-07-10 21:43     ` [dpdk-dev] [PATCH v7 3/3] drivers/net: delete HW rings while freeing queues Ferruh Yigit
  2020-07-10 22:07     ` [dpdk-dev] [PATCH v7 1/3] drivers/net: fix build with internal API usage Thomas Monjalon
  2 siblings, 0 replies; 16+ messages in thread
From: Ferruh Yigit @ 2020-07-10 21:43 UTC (permalink / raw)
  To: Thomas Monjalon, Andrew Rybchenko, Ray Kinsella, Neil Horman
  Cc: dev, Ferruh Yigit, Renata Saiakhova

From: Renata Saiakhova <renata.saiakhova@ekinops.com>

Free previously allocated memzone for HW rings

Signed-off-by: Renata Saiakhova <renata.saiakhova@ekinops.com>
Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
v6:
* Converted macro to static inline function
* Updated API return error value

v7:
* Application build fixed for __rte_internal usage, fix done in separate
  patch
---
 lib/librte_ethdev/rte_ethdev.c           | 36 ++++++++++++++++++++++--
 lib/librte_ethdev/rte_ethdev_driver.h    | 17 +++++++++++
 lib/librte_ethdev/rte_ethdev_version.map |  7 +++++
 3 files changed, 58 insertions(+), 2 deletions(-)

diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 598ff80a6e..7858ad5f11 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -4201,6 +4201,14 @@ rte_eth_dev_rx_intr_ctl_q_get_fd(uint16_t port_id, uint16_t queue_id)
 	return fd;
 }
 
+static inline int
+eth_dma_mzone_name(char *name, size_t len, uint16_t port_id, uint16_t queue_id,
+		const char *ring_name)
+{
+	return snprintf(name, len, "eth_p%d_q%d_%s",
+			port_id, queue_id, ring_name);
+}
+
 const struct rte_memzone *
 rte_eth_dma_zone_reserve(const struct rte_eth_dev *dev, const char *ring_name,
 			 uint16_t queue_id, size_t size, unsigned align,
@@ -4210,8 +4218,8 @@ rte_eth_dma_zone_reserve(const struct rte_eth_dev *dev, const char *ring_name,
 	const struct rte_memzone *mz;
 	int rc;
 
-	rc = snprintf(z_name, sizeof(z_name), "eth_p%d_q%d_%s",
-		      dev->data->port_id, queue_id, ring_name);
+	rc = eth_dma_mzone_name(z_name, sizeof(z_name), dev->data->port_id,
+			queue_id, ring_name);
 	if (rc >= RTE_MEMZONE_NAMESIZE) {
 		RTE_ETHDEV_LOG(ERR, "ring name too long\n");
 		rte_errno = ENAMETOOLONG;
@@ -4236,6 +4244,30 @@ rte_eth_dma_zone_reserve(const struct rte_eth_dev *dev, const char *ring_name,
 			RTE_MEMZONE_IOVA_CONTIG, align);
 }
 
+int
+rte_eth_dma_zone_free(const struct rte_eth_dev *dev, const char *ring_name,
+		uint16_t queue_id)
+{
+	char z_name[RTE_MEMZONE_NAMESIZE];
+	const struct rte_memzone *mz;
+	int rc = 0;
+
+	rc = eth_dma_mzone_name(z_name, sizeof(z_name), dev->data->port_id,
+			queue_id, ring_name);
+	if (rc >= RTE_MEMZONE_NAMESIZE) {
+		RTE_ETHDEV_LOG(ERR, "ring name too long\n");
+		return -ENAMETOOLONG;
+	}
+
+	mz = rte_memzone_lookup(z_name);
+	if (mz)
+		rc = rte_memzone_free(mz);
+	else
+		rc = -ENOENT;
+
+	return rc;
+}
+
 int
 rte_eth_dev_create(struct rte_device *device, const char *name,
 	size_t priv_data_size,
diff --git a/lib/librte_ethdev/rte_ethdev_driver.h b/lib/librte_ethdev/rte_ethdev_driver.h
index 99d4cd6cd0..13fd049c0d 100644
--- a/lib/librte_ethdev/rte_ethdev_driver.h
+++ b/lib/librte_ethdev/rte_ethdev_driver.h
@@ -180,6 +180,23 @@ rte_eth_dma_zone_reserve(const struct rte_eth_dev *eth_dev, const char *name,
 			 uint16_t queue_id, size_t size,
 			 unsigned align, int socket_id);
 
+/**
+ * Free previously allocated memzone for HW rings.
+ *
+ * @param eth_dev
+ *   The *eth_dev* pointer is the address of the *rte_eth_dev* structure
+ * @param name
+ *   The name of the memory zone
+ * @param queue_id
+ *   The index of the queue to add to name
+ * @return
+ *   Negative errno value on error, 0 on success.
+ */
+__rte_internal
+int
+rte_eth_dma_zone_free(const struct rte_eth_dev *eth_dev, const char *name,
+		 uint16_t queue_id);
+
 /**
  * @internal
  * Atomically set the link status for the specific device.
diff --git a/lib/librte_ethdev/rte_ethdev_version.map b/lib/librte_ethdev/rte_ethdev_version.map
index 7155056045..664d6b86a2 100644
--- a/lib/librte_ethdev/rte_ethdev_version.map
+++ b/lib/librte_ethdev/rte_ethdev_version.map
@@ -242,3 +242,10 @@ EXPERIMENTAL {
 	__rte_ethdev_trace_tx_burst;
 	rte_flow_get_aged_flows;
 };
+
+INTERNAL {
+	global:
+
+	rte_eth_dma_zone_free;
+};
+
-- 
2.25.4


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

* [dpdk-dev] [PATCH v7 3/3] drivers/net: delete HW rings while freeing queues
  2020-07-10 21:43   ` [dpdk-dev] [PATCH v7 1/3] drivers/net: fix build with internal API usage Ferruh Yigit
  2020-07-10 21:43     ` [dpdk-dev] [PATCH v7 2/3] ethdev: add function to release HW rings Ferruh Yigit
@ 2020-07-10 21:43     ` Ferruh Yigit
  2020-07-10 22:07     ` [dpdk-dev] [PATCH v7 1/3] drivers/net: fix build with internal API usage Thomas Monjalon
  2 siblings, 0 replies; 16+ messages in thread
From: Ferruh Yigit @ 2020-07-10 21:43 UTC (permalink / raw)
  To: Wei Zhao, Jeff Guo, Beilei Xing, Qiming Yang, Qi Zhang
  Cc: dev, Ferruh Yigit, Renata Saiakhova

From: Renata Saiakhova <renata.saiakhova@ekinops.com>

Delete memzones for HW rings in igb and ixgbe while freeing queues

Updated igb, ixgbe, i40e, ice & em drivers.

Signed-off-by: Renata Saiakhova <renata.saiakhova@ekinops.com>
Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
 drivers/net/e1000/em_rxtx.c    | 2 ++
 drivers/net/e1000/igb_rxtx.c   | 2 ++
 drivers/net/i40e/i40e_fdir.c   | 3 +++
 drivers/net/i40e/i40e_rxtx.c   | 2 ++
 drivers/net/ice/ice_rxtx.c     | 2 ++
 drivers/net/ixgbe/ixgbe_rxtx.c | 2 ++
 6 files changed, 13 insertions(+)

diff --git a/drivers/net/e1000/em_rxtx.c b/drivers/net/e1000/em_rxtx.c
index 49c53712a2..67a271e8ce 100644
--- a/drivers/net/e1000/em_rxtx.c
+++ b/drivers/net/e1000/em_rxtx.c
@@ -1611,12 +1611,14 @@ em_dev_free_queues(struct rte_eth_dev *dev)
 	for (i = 0; i < dev->data->nb_rx_queues; i++) {
 		eth_em_rx_queue_release(dev->data->rx_queues[i]);
 		dev->data->rx_queues[i] = NULL;
+		rte_eth_dma_zone_free(dev, "rx_ring", i);
 	}
 	dev->data->nb_rx_queues = 0;
 
 	for (i = 0; i < dev->data->nb_tx_queues; i++) {
 		eth_em_tx_queue_release(dev->data->tx_queues[i]);
 		dev->data->tx_queues[i] = NULL;
+		rte_eth_dma_zone_free(dev, "tx_ring", i);
 	}
 	dev->data->nb_tx_queues = 0;
 }
diff --git a/drivers/net/e1000/igb_rxtx.c b/drivers/net/e1000/igb_rxtx.c
index 6883202842..5717cdb706 100644
--- a/drivers/net/e1000/igb_rxtx.c
+++ b/drivers/net/e1000/igb_rxtx.c
@@ -1884,12 +1884,14 @@ igb_dev_free_queues(struct rte_eth_dev *dev)
 	for (i = 0; i < dev->data->nb_rx_queues; i++) {
 		eth_igb_rx_queue_release(dev->data->rx_queues[i]);
 		dev->data->rx_queues[i] = NULL;
+		rte_eth_dma_zone_free(dev, "rx_ring", i);
 	}
 	dev->data->nb_rx_queues = 0;
 
 	for (i = 0; i < dev->data->nb_tx_queues; i++) {
 		eth_igb_tx_queue_release(dev->data->tx_queues[i]);
 		dev->data->tx_queues[i] = NULL;
+		rte_eth_dma_zone_free(dev, "tx_ring", i);
 	}
 	dev->data->nb_tx_queues = 0;
 }
diff --git a/drivers/net/i40e/i40e_fdir.c b/drivers/net/i40e/i40e_fdir.c
index 4a778db4c6..71eb31fb8b 100644
--- a/drivers/net/i40e/i40e_fdir.c
+++ b/drivers/net/i40e/i40e_fdir.c
@@ -269,6 +269,7 @@ i40e_fdir_teardown(struct i40e_pf *pf)
 {
 	struct i40e_hw *hw = I40E_PF_TO_HW(pf);
 	struct i40e_vsi *vsi;
+	struct rte_eth_dev *dev = pf->adapter->eth_dev;
 
 	vsi = pf->fdir.fdir_vsi;
 	if (!vsi)
@@ -280,8 +281,10 @@ i40e_fdir_teardown(struct i40e_pf *pf)
 	if (err)
 		PMD_DRV_LOG(DEBUG, "Failed to do FDIR RX switch off");
 	i40e_dev_rx_queue_release(pf->fdir.rxq);
+	rte_eth_dma_zone_free(dev, "fdir_rx_ring", pf->fdir.rxq->queue_id);
 	pf->fdir.rxq = NULL;
 	i40e_dev_tx_queue_release(pf->fdir.txq);
+	rte_eth_dma_zone_free(dev, "fdir_tx_ring", pf->fdir.txq->queue_id);
 	pf->fdir.txq = NULL;
 	i40e_vsi_release(vsi);
 	pf->fdir.fdir_vsi = NULL;
diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
index 840b6f387f..2d2efb71a5 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -2928,6 +2928,7 @@ i40e_dev_free_queues(struct rte_eth_dev *dev)
 			continue;
 		i40e_dev_rx_queue_release(dev->data->rx_queues[i]);
 		dev->data->rx_queues[i] = NULL;
+		rte_eth_dma_zone_free(dev, "rx_ring", i);
 	}
 
 	for (i = 0; i < dev->data->nb_tx_queues; i++) {
@@ -2935,6 +2936,7 @@ i40e_dev_free_queues(struct rte_eth_dev *dev)
 			continue;
 		i40e_dev_tx_queue_release(dev->data->tx_queues[i]);
 		dev->data->tx_queues[i] = NULL;
+		rte_eth_dma_zone_free(dev, "tx_ring", i);
 	}
 }
 
diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.c
index 5d6f693c52..cc3139042e 100644
--- a/drivers/net/ice/ice_rxtx.c
+++ b/drivers/net/ice/ice_rxtx.c
@@ -1905,6 +1905,7 @@ ice_free_queues(struct rte_eth_dev *dev)
 			continue;
 		ice_rx_queue_release(dev->data->rx_queues[i]);
 		dev->data->rx_queues[i] = NULL;
+		rte_eth_dma_zone_free(dev, "rx_ring", i);
 	}
 	dev->data->nb_rx_queues = 0;
 
@@ -1913,6 +1914,7 @@ ice_free_queues(struct rte_eth_dev *dev)
 			continue;
 		ice_tx_queue_release(dev->data->tx_queues[i]);
 		dev->data->tx_queues[i] = NULL;
+		rte_eth_dma_zone_free(dev, "tx_ring", i);
 	}
 	dev->data->nb_tx_queues = 0;
 }
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
index 2e20e18c7a..977ecf5137 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx.c
@@ -3368,12 +3368,14 @@ ixgbe_dev_free_queues(struct rte_eth_dev *dev)
 	for (i = 0; i < dev->data->nb_rx_queues; i++) {
 		ixgbe_dev_rx_queue_release(dev->data->rx_queues[i]);
 		dev->data->rx_queues[i] = NULL;
+		rte_eth_dma_zone_free(dev, "rx_ring", i);
 	}
 	dev->data->nb_rx_queues = 0;
 
 	for (i = 0; i < dev->data->nb_tx_queues; i++) {
 		ixgbe_dev_tx_queue_release(dev->data->tx_queues[i]);
 		dev->data->tx_queues[i] = NULL;
+		rte_eth_dma_zone_free(dev, "tx_ring", i);
 	}
 	dev->data->nb_tx_queues = 0;
 }
-- 
2.25.4


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

* Re: [dpdk-dev] [PATCH v7 1/3] drivers/net: fix build with internal API usage
  2020-07-10 21:43   ` [dpdk-dev] [PATCH v7 1/3] drivers/net: fix build with internal API usage Ferruh Yigit
  2020-07-10 21:43     ` [dpdk-dev] [PATCH v7 2/3] ethdev: add function to release HW rings Ferruh Yigit
  2020-07-10 21:43     ` [dpdk-dev] [PATCH v7 3/3] drivers/net: delete HW rings while freeing queues Ferruh Yigit
@ 2020-07-10 22:07     ` Thomas Monjalon
  2020-07-10 23:17       ` Ferruh Yigit
  2 siblings, 1 reply; 16+ messages in thread
From: Thomas Monjalon @ 2020-07-10 22:07 UTC (permalink / raw)
  To: Ferruh Yigit
  Cc: Igor Russkikh, Pavel Belous, Ajit Khaparde, Somnath Kotur,
	Hemant Agrawal, Sachin Saxena, Beilei Xing, Jeff Guo, Wei Zhao,
	Andrew Rybchenko, Shreyansh Jain, dev, Ferruh Yigit, stable,
	David Marchand

10/07/2020 23:43, Ferruh Yigit:
> Using '__rte_internal' tag in 'rte_ethdev_driver.h' causing build error
> for applications and examples. Because they don't define
> 'ALLOW_INTERNAL_API' flag and '__rte_internal' causes the error.
> This patch is preparation for future '__rte_internal' usage.
> 
> At first place, applications/examples should not include
> 'rte_ethdev_driver.h', this is happening because of PMD public header
> files include 'rte_ethdev_driver.h' by mistake.
> 
> Updated PMD public header files to not include internal header files.
> 
> But for unit test application, 'app/test', enable accessing internal
> APIs, since some unit tests need them.
> 
> Fixes: ffc905f3b856 ("ethdev: separate driver APIs")
> Fixes: ec0dec44ecb9 ("net/atlantic: enable MACsec configuration")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>

Not sure to understand the title but I have no better proposal.
For the explanations and the code,
Acked-by: Thomas Monjalon <thomas@monjalon.net>



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

* Re: [dpdk-dev] [PATCH v7 1/3] drivers/net: fix build with internal API usage
  2020-07-10 22:07     ` [dpdk-dev] [PATCH v7 1/3] drivers/net: fix build with internal API usage Thomas Monjalon
@ 2020-07-10 23:17       ` Ferruh Yigit
  2020-07-11  0:19         ` Ferruh Yigit
  0 siblings, 1 reply; 16+ messages in thread
From: Ferruh Yigit @ 2020-07-10 23:17 UTC (permalink / raw)
  To: Thomas Monjalon
  Cc: Igor Russkikh, Pavel Belous, Ajit Khaparde, Somnath Kotur,
	Hemant Agrawal, Sachin Saxena, Beilei Xing, Jeff Guo, Wei Zhao,
	Andrew Rybchenko, Shreyansh Jain, dev, stable, David Marchand

On 7/10/2020 11:07 PM, Thomas Monjalon wrote:
> 10/07/2020 23:43, Ferruh Yigit:
>> Using '__rte_internal' tag in 'rte_ethdev_driver.h' causing build error
>> for applications and examples. Because they don't define
>> 'ALLOW_INTERNAL_API' flag and '__rte_internal' causes the error.
>> This patch is preparation for future '__rte_internal' usage.
>>
>> At first place, applications/examples should not include
>> 'rte_ethdev_driver.h', this is happening because of PMD public header
>> files include 'rte_ethdev_driver.h' by mistake.
>>
>> Updated PMD public header files to not include internal header files.
>>
>> But for unit test application, 'app/test', enable accessing internal
>> APIs, since some unit tests need them.
>>
>> Fixes: ffc905f3b856 ("ethdev: separate driver APIs")
>> Fixes: ec0dec44ecb9 ("net/atlantic: enable MACsec configuration")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> 
> Not sure to understand the title but I have no better proposal.

Agree, I will change to following:
drivers/net: fix exposing internal headers

> For the explanations and the code,
> Acked-by: Thomas Monjalon <thomas@monjalon.net>
> 
> 


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

* Re: [dpdk-dev] [PATCH v7 1/3] drivers/net: fix build with internal API usage
  2020-07-10 23:17       ` Ferruh Yigit
@ 2020-07-11  0:19         ` Ferruh Yigit
  0 siblings, 0 replies; 16+ messages in thread
From: Ferruh Yigit @ 2020-07-11  0:19 UTC (permalink / raw)
  To: Thomas Monjalon
  Cc: Igor Russkikh, Pavel Belous, Ajit Khaparde, Somnath Kotur,
	Hemant Agrawal, Sachin Saxena, Beilei Xing, Jeff Guo, Wei Zhao,
	Andrew Rybchenko, Shreyansh Jain, dev, stable, David Marchand

On 7/11/2020 12:17 AM, Ferruh Yigit wrote:
> On 7/10/2020 11:07 PM, Thomas Monjalon wrote:
>> 10/07/2020 23:43, Ferruh Yigit:
>>> Using '__rte_internal' tag in 'rte_ethdev_driver.h' causing build error
>>> for applications and examples. Because they don't define
>>> 'ALLOW_INTERNAL_API' flag and '__rte_internal' causes the error.
>>> This patch is preparation for future '__rte_internal' usage.
>>>
>>> At first place, applications/examples should not include
>>> 'rte_ethdev_driver.h', this is happening because of PMD public header
>>> files include 'rte_ethdev_driver.h' by mistake.
>>>
>>> Updated PMD public header files to not include internal header files.
>>>
>>> But for unit test application, 'app/test', enable accessing internal
>>> APIs, since some unit tests need them.
>>>
>>> Fixes: ffc905f3b856 ("ethdev: separate driver APIs")
>>> Fixes: ec0dec44ecb9 ("net/atlantic: enable MACsec configuration")
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>>
>> Not sure to understand the title but I have no better proposal.
> 
> Agree, I will change to following:
> drivers/net: fix exposing internal headers
> 
>> For the explanations and the code,
>> Acked-by: Thomas Monjalon <thomas@monjalon.net>
>>
> 

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

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

end of thread, other threads:[~2020-07-11  0:19 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-23 13:42 [dpdk-dev] [PATCH v5 0/4] Memory corruption due to HW rings allocation Renata Saiakhova
2020-06-23 13:42 ` [dpdk-dev] [PATCH v5 1/4] ethdev: add function to release HW rings Renata Saiakhova
2020-06-23 13:52   ` Ferruh Yigit
2020-06-23 15:49   ` Andrew Rybchenko
2020-06-23 13:42 ` [dpdk-dev] [PATCH v5 2/4] drivers/net: fix in igb and ixgbe HW rings memory Renata Saiakhova
2020-06-23 13:42 ` [dpdk-dev] [PATCH v5 3/4] drivers/net: fix in i40e HW rings memory overlap Renata Saiakhova
2020-06-23 13:42 ` [dpdk-dev] [PATCH v5 4/4] drivers/net: fix in em and ice " Renata Saiakhova
2020-06-23 13:53 ` [dpdk-dev] [PATCH v5 0/4] Memory corruption due to HW rings allocation Ferruh Yigit
2020-07-10 14:42 ` [dpdk-dev] [PATCH v6 1/2] ethdev: add function to release HW rings Ferruh Yigit
2020-07-10 14:42   ` [dpdk-dev] [PATCH v6 2/2] drivers/net: delete HW rings while freeing queues Ferruh Yigit
2020-07-10 21:43   ` [dpdk-dev] [PATCH v7 1/3] drivers/net: fix build with internal API usage Ferruh Yigit
2020-07-10 21:43     ` [dpdk-dev] [PATCH v7 2/3] ethdev: add function to release HW rings Ferruh Yigit
2020-07-10 21:43     ` [dpdk-dev] [PATCH v7 3/3] drivers/net: delete HW rings while freeing queues Ferruh Yigit
2020-07-10 22:07     ` [dpdk-dev] [PATCH v7 1/3] drivers/net: fix build with internal API usage Thomas Monjalon
2020-07-10 23:17       ` Ferruh Yigit
2020-07-11  0:19         ` 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).