DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH v4 0/4] Memory corruption due to HW rings allocation
@ 2020-06-22  9:54 Renata Saiakhova
  2020-06-22  9:54 ` [dpdk-dev] [PATCH v4 1/4] librte_ethdev: Introduce a function to release HW rings Renata Saiakhova
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Renata Saiakhova @ 2020-06-22  9:54 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.

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):
  librte_ethdev: Introduce a 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    | 20 ++++++++++++++++
 lib/librte_ethdev/rte_ethdev_version.map |  1 +
 9 files changed, 62 insertions(+), 2 deletions(-)

-- 
2.17.2


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

* [dpdk-dev] [PATCH v4 1/4] librte_ethdev: Introduce a function to release HW rings
  2020-06-22  9:54 [dpdk-dev] [PATCH v4 0/4] Memory corruption due to HW rings allocation Renata Saiakhova
@ 2020-06-22  9:54 ` Renata Saiakhova
  2020-06-22 17:09   ` Ferruh Yigit
  2020-06-22 17:13   ` Ferruh Yigit
  2020-06-22  9:54 ` [dpdk-dev] [PATCH v4 2/4] drivers/net: Fix in igb and ixgbe HW rings memory Renata Saiakhova
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 11+ messages in thread
From: Renata Saiakhova @ 2020-06-22  9:54 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    | 20 ++++++++++++++++
 lib/librte_ethdev/rte_ethdev_version.map |  1 +
 3 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 72aed59a5..ec1da2006 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -4181,6 +4181,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,
@@ -4190,8 +4194,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;
@@ -4206,6 +4209,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..462e765d1 100644
--- a/lib/librte_ethdev/rte_ethdev_driver.h
+++ b/lib/librte_ethdev/rte_ethdev_driver.h
@@ -180,6 +180,26 @@ 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);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * 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_experimental
+int
+rte_eth_dma_zone_free(const struct rte_eth_dev *dev, const char *ring_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..139a81302 100644
--- a/lib/librte_ethdev/rte_ethdev_version.map
+++ b/lib/librte_ethdev/rte_ethdev_version.map
@@ -241,4 +241,5 @@ EXPERIMENTAL {
 	__rte_ethdev_trace_rx_burst;
 	__rte_ethdev_trace_tx_burst;
 	rte_flow_get_aged_flows;
+	rte_eth_dma_zone_free;
 };
-- 
2.17.2


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

* [dpdk-dev] [PATCH v4 2/4] drivers/net: Fix in igb and ixgbe HW rings memory
  2020-06-22  9:54 [dpdk-dev] [PATCH v4 0/4] Memory corruption due to HW rings allocation Renata Saiakhova
  2020-06-22  9:54 ` [dpdk-dev] [PATCH v4 1/4] librte_ethdev: Introduce a function to release HW rings Renata Saiakhova
@ 2020-06-22  9:54 ` Renata Saiakhova
  2020-06-22  9:54 ` [dpdk-dev] [PATCH v4 3/4] drivers/net: Fix in i40e HW rings memory overlap Renata Saiakhova
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Renata Saiakhova @ 2020-06-22  9:54 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] 11+ messages in thread

* [dpdk-dev] [PATCH v4 3/4] drivers/net: Fix in i40e HW rings memory overlap
  2020-06-22  9:54 [dpdk-dev] [PATCH v4 0/4] Memory corruption due to HW rings allocation Renata Saiakhova
  2020-06-22  9:54 ` [dpdk-dev] [PATCH v4 1/4] librte_ethdev: Introduce a function to release HW rings Renata Saiakhova
  2020-06-22  9:54 ` [dpdk-dev] [PATCH v4 2/4] drivers/net: Fix in igb and ixgbe HW rings memory Renata Saiakhova
@ 2020-06-22  9:54 ` Renata Saiakhova
  2020-06-22  9:54 ` [dpdk-dev] [PATCH v4 4/4] drivers/net: Fix in em and ice " Renata Saiakhova
  2020-07-10 14:36 ` [dpdk-dev] [PATCH v5 1/2] ethdev: add function to release HW rings Ferruh Yigit
  4 siblings, 0 replies; 11+ messages in thread
From: Renata Saiakhova @ 2020-06-22  9:54 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 5e7c86ed8..99cec9b99 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -2900,6 +2900,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++) {
@@ -2907,6 +2908,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] 11+ messages in thread

* [dpdk-dev] [PATCH v4 4/4] drivers/net: Fix in em and ice HW rings memory overlap
  2020-06-22  9:54 [dpdk-dev] [PATCH v4 0/4] Memory corruption due to HW rings allocation Renata Saiakhova
                   ` (2 preceding siblings ...)
  2020-06-22  9:54 ` [dpdk-dev] [PATCH v4 3/4] drivers/net: Fix in i40e HW rings memory overlap Renata Saiakhova
@ 2020-06-22  9:54 ` Renata Saiakhova
  2020-07-10 14:36 ` [dpdk-dev] [PATCH v5 1/2] ethdev: add function to release HW rings Ferruh Yigit
  4 siblings, 0 replies; 11+ messages in thread
From: Renata Saiakhova @ 2020-06-22  9:54 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 1c9f31efd..7c2dad9d6 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] 11+ messages in thread

* Re: [dpdk-dev] [PATCH v4 1/4] librte_ethdev: Introduce a function to release HW rings
  2020-06-22  9:54 ` [dpdk-dev] [PATCH v4 1/4] librte_ethdev: Introduce a function to release HW rings Renata Saiakhova
@ 2020-06-22 17:09   ` Ferruh Yigit
  2020-06-23  9:11     ` Renata Saiakhova
  2020-06-22 17:13   ` Ferruh Yigit
  1 sibling, 1 reply; 11+ messages in thread
From: Ferruh Yigit @ 2020-06-22 17:09 UTC (permalink / raw)
  To: Renata Saiakhova, dev

On 6/22/2020 10:54 AM, Renata Saiakhova wrote:
> 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    | 20 ++++++++++++++++
>  lib/librte_ethdev/rte_ethdev_version.map |  1 +
>  3 files changed, 49 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
> index 72aed59a5..ec1da2006 100644
> --- a/lib/librte_ethdev/rte_ethdev.c
> +++ b/lib/librte_ethdev/rte_ethdev.c
> @@ -4181,6 +4181,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,
> @@ -4190,8 +4194,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;
> @@ -4206,6 +4209,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..462e765d1 100644
> --- a/lib/librte_ethdev/rte_ethdev_driver.h
> +++ b/lib/librte_ethdev/rte_ethdev_driver.h
> @@ -180,6 +180,26 @@ 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);
>  
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice.
> + *
> + * 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_experimental
> +int
> +rte_eth_dma_zone_free(const struct rte_eth_dev *dev, const char *ring_name,
> +		 uint16_t queue_id);
> +

Hi Renata,

This API is not for applications, only for drivers, right?
We recently (last release ?) introduced 'internal' (__rte_internal) attribute,
we can use '__rte_internal' instead of '__rte_experimental' if the API is not
for applications. This requires adding 'INTERNAL' block in .map file (sample
'lib/librte_eal/rte_eal_version.map')


Also there are a few contribution conventions related issues which can be fixed
while merging but please check them if you will be doing new version:
 subsystem is only libname without prefix: 'ethdev'
 Commit title description starts with lowercase: 'introduce'.
sample => "ethdev: add function to release HW rings"


Thanks,
ferruh

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

* Re: [dpdk-dev] [PATCH v4 1/4] librte_ethdev: Introduce a function to release HW rings
  2020-06-22  9:54 ` [dpdk-dev] [PATCH v4 1/4] librte_ethdev: Introduce a function to release HW rings Renata Saiakhova
  2020-06-22 17:09   ` Ferruh Yigit
@ 2020-06-22 17:13   ` Ferruh Yigit
  1 sibling, 0 replies; 11+ messages in thread
From: Ferruh Yigit @ 2020-06-22 17:13 UTC (permalink / raw)
  To: Renata Saiakhova, dev

On 6/22/2020 10:54 AM, Renata Saiakhova wrote:
> Free previously allocated memzone for HW rings
> 
> Signed-off-by: Renata Saiakhova <Renata.Saiakhova@ekinops.com>

<...>

> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice.
> + *
> + * 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

The param names for doxygen and actual param names are not same, causing doxygen
warning.
"@param eth_dev" -> 'dev'
"@param name" -> 'ring_name'

> + * @return
> + *   Negative errno value on error, 0 on success.
> + */
> +__rte_experimental
> +int
> +rte_eth_dma_zone_free(const struct rte_eth_dev *dev, const char *ring_name,
> +		 uint16_t queue_id);
> +




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

* Re: [dpdk-dev] [PATCH v4 1/4] librte_ethdev: Introduce a function to release HW rings
  2020-06-22 17:09   ` Ferruh Yigit
@ 2020-06-23  9:11     ` Renata Saiakhova
  2020-06-23 12:25       ` Ferruh Yigit
  0 siblings, 1 reply; 11+ messages in thread
From: Renata Saiakhova @ 2020-06-23  9:11 UTC (permalink / raw)
  To: Ferruh Yigit, dev

Hi Ferruh,

I added an INTERNAL block in .map file, that gives me an error:

Build targets in project: 806
Found ninja-1.8.2 at /usr/bin/ninja
[7/627] Linking target lib/librte_ethdev.so.20.0.3.
FAILED: lib/librte_ethdev.so.20.0.3
ccache cc  -o lib/librte_ethdev.so.20.0.3 'lib/lib@@rte_ethdev@sta/librte_ethdev_ethdev_private.c.o' 'lib/lib@@rte_ethdev@sta/librte_ethdev_ethdev_profile.c.o' 'lib/lib@@rte_ethdev@sta/librte_ethdev_ethdev_trace_points.c.o' 'lib/lib@@rte_ethdev@sta/librte_ethdev_rte_class_eth.c.o' 'lib/lib@@rte_ethdev@sta/librte_ethdev_rte_ethdev.c.o' 'lib/lib@@rte_ethdev@sta/librte_ethdev_rte_flow.c.o' 'lib/lib@@rte_ethdev@sta/librte_ethdev_rte_mtr.c.o' 'lib/lib@@rte_ethdev@sta/librte_ethdev_rte_tm.c.o' -Wl,--no-undefined -Wl,--as-needed -Wl,-O1 -shared -fPIC -Wl,--start-group -Wl,-soname,librte_ethdev.so.20.0 -Wl,--no-as-needed -pthread -lm -ldl -lnuma lib/librte_eal.so.20.0.3 lib/librte_kvargs.so.20.0.3 lib/librte_telemetry.so.0.200.3 lib/librte_net.so.20.0.3 lib/librte_mbuf.so.20.0.3 lib/librte_mempool.so.20.0.3 lib/librte_ring.so.20.0.3 lib/librte_meter.so.20.0.3 -Wl,--end-group -Wl,--version-script=/home/renata/work/dpdk/lib/librte_ethdev/rte_ethdev_version.map '-Wl,-rpath,$ORIGIN/' -Wl,-rpath-link,/home/renata/work/dpdk/build/lib
/usr/bin/ld:/home/renata/work/dpdk/lib/librte_ethdev/rte_ethdev_version.map:0: syntax error in VERSION script
collect2: error: ld returned 1 exit status

What could be an issue, is that I need to correct the version somewhere?

KR,
Renata

________________________________
From: Ferruh Yigit <ferruh.yigit@intel.com>
Sent: Monday, June 22, 2020 7:09 PM
To: Renata Saiakhova <renata.saiakhova@ekinops.com>; dev@dpdk.org <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH v4 1/4] librte_ethdev: Introduce a function to release HW rings

On 6/22/2020 10:54 AM, Renata Saiakhova wrote:
> 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    | 20 ++++++++++++++++
>  lib/librte_ethdev/rte_ethdev_version.map |  1 +
>  3 files changed, 49 insertions(+), 2 deletions(-)
>
> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
> index 72aed59a5..ec1da2006 100644
> --- a/lib/librte_ethdev/rte_ethdev.c
> +++ b/lib/librte_ethdev/rte_ethdev.c
> @@ -4181,6 +4181,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,
> @@ -4190,8 +4194,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;
> @@ -4206,6 +4209,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..462e765d1 100644
> --- a/lib/librte_ethdev/rte_ethdev_driver.h
> +++ b/lib/librte_ethdev/rte_ethdev_driver.h
> @@ -180,6 +180,26 @@ 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);
>
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice.
> + *
> + * 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_experimental
> +int
> +rte_eth_dma_zone_free(const struct rte_eth_dev *dev, const char *ring_name,
> +              uint16_t queue_id);
> +

Hi Renata,

This API is not for applications, only for drivers, right?
We recently (last release ?) introduced 'internal' (__rte_internal) attribute,
we can use '__rte_internal' instead of '__rte_experimental' if the API is not
for applications. This requires adding 'INTERNAL' block in .map file (sample
'lib/librte_eal/rte_eal_version.map')


Also there are a few contribution conventions related issues which can be fixed
while merging but please check them if you will be doing new version:
 subsystem is only libname without prefix: 'ethdev'
 Commit title description starts with lowercase: 'introduce'.
sample => "ethdev: add function to release HW rings"


Thanks,
ferruh

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

* Re: [dpdk-dev] [PATCH v4 1/4] librte_ethdev: Introduce a function to release HW rings
  2020-06-23  9:11     ` Renata Saiakhova
@ 2020-06-23 12:25       ` Ferruh Yigit
  0 siblings, 0 replies; 11+ messages in thread
From: Ferruh Yigit @ 2020-06-23 12:25 UTC (permalink / raw)
  To: Renata Saiakhova, dev

On 6/23/2020 10:11 AM, Renata Saiakhova wrote:
> Hi Ferruh,
> 
> I added an INTERNAL block in .map file, that gives me an error:
> 
> Build targets in project: 806
> Found ninja-1.8.2 at /usr/bin/ninja
> [7/627] Linking target lib/librte_ethdev.so.20.0.3.
> FAILED: lib/librte_ethdev.so.20.0.3
> ccache cc  -o lib/librte_ethdev.so.20.0.3
> 'lib/lib@@rte_ethdev@sta/librte_ethdev_ethdev_private.c.o'
> 'lib/lib@@rte_ethdev@sta/librte_ethdev_ethdev_profile.c.o'
> 'lib/lib@@rte_ethdev@sta/librte_ethdev_ethdev_trace_points.c.o'
> 'lib/lib@@rte_ethdev@sta/librte_ethdev_rte_class_eth.c.o'
> 'lib/lib@@rte_ethdev@sta/librte_ethdev_rte_ethdev.c.o'
> 'lib/lib@@rte_ethdev@sta/librte_ethdev_rte_flow.c.o'
> 'lib/lib@@rte_ethdev@sta/librte_ethdev_rte_mtr.c.o'
> 'lib/lib@@rte_ethdev@sta/librte_ethdev_rte_tm.c.o' -Wl,--no-undefined
> -Wl,--as-needed -Wl,-O1 -shared -fPIC -Wl,--start-group
> -Wl,-soname,librte_ethdev.so.20.0 -Wl,--no-as-needed -pthread -lm -ldl -lnuma
> lib/librte_eal.so.20.0.3 lib/librte_kvargs.so.20.0.3
> lib/librte_telemetry.so.0.200.3 lib/librte_net.so.20.0.3
> lib/librte_mbuf.so.20.0.3 lib/librte_mempool.so.20.0.3 lib/librte_ring.so.20.0.3
> lib/librte_meter.so.20.0.3 -Wl,--end-group
> -Wl,--version-script=/home/renata/work/dpdk/lib/librte_ethdev/rte_ethdev_version.map
> '-Wl,-rpath,$ORIGIN/' -Wl,-rpath-link,/home/renata/work/dpdk/build/lib  
> /usr/bin/ld:/home/renata/work/dpdk/lib/librte_ethdev/rte_ethdev_version.map:0:
> syntax error in VERSION script
> collect2: error: ld returned 1 exit status
> 
> What could be an issue, is that I need to correct the version somewhere?

Hi Renata,

The log says "syntax error in VERSION script", can be related to it. I did
following update on top of your patches and it seems worked for me:

 diff --git a/lib/librte_ethdev/rte_ethdev_driver.h b/lib/librte_ethdev
/rte_ethdev_driver.h
 index 462e765d1..ca16f1bcc 100644
 --- a/lib/librte_ethdev/rte_ethdev_driver.h
 +++ b/lib/librte_ethdev/rte_ethdev_driver.h
 @@ -181,9 +181,6 @@ rte_eth_dma_zone_reserve(const struct rte_eth_dev *eth_dev,
 const char *name,
                          unsigned align, int socket_id);

  /**
 - * @warning
 - * @b EXPERIMENTAL: this API may change without prior notice.
 - *
   * Free previously allocated memzone for HW rings.
   *
   * @param eth_dev
 @@ -195,7 +192,7 @@ rte_eth_dma_zone_reserve(const struct rte_eth_dev *eth_dev,
const char *name,
   * @return
   *   Negative errno value on error, 0 on success.
   */
 -__rte_experimental
 +__rte_internal
  int
  rte_eth_dma_zone_free(const struct rte_eth_dev *dev, const char *ring_name,
                  uint16_t queue_id);
 diff --git a/lib/librte_ethdev/rte_ethdev_version.map b/lib/librte_ethdev
/rte_ethdev_version.map
 index 139a81302..1212a17d3 100644
 --- a/lib/librte_ethdev/rte_ethdev_version.map
 +++ b/lib/librte_ethdev/rte_ethdev_version.map
 @@ -241,5 +241,10 @@ EXPERIMENTAL {
         __rte_ethdev_trace_rx_burst;
         __rte_ethdev_trace_tx_burst;
         rte_flow_get_aged_flows;
 +};
 +
 +INTERNAL {
 +       global:
 +
         rte_eth_dma_zone_free;
  };


> 
> --------------------------------------------------------------------------------
> *From:* Ferruh Yigit <ferruh.yigit@intel.com>
> *Sent:* Monday, June 22, 2020 7:09 PM
> *To:* Renata Saiakhova <renata.saiakhova@ekinops.com>; dev@dpdk.org <dev@dpdk.org>
> *Subject:* Re: [dpdk-dev] [PATCH v4 1/4] librte_ethdev: Introduce a function to
> release HW rings
>  
> On 6/22/2020 10:54 AM, Renata Saiakhova wrote:
>> 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    | 20 ++++++++++++++++
>>  lib/librte_ethdev/rte_ethdev_version.map |  1 +
>>  3 files changed, 49 insertions(+), 2 deletions(-)
>> 
>> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
>> index 72aed59a5..ec1da2006 100644
>> --- a/lib/librte_ethdev/rte_ethdev.c
>> +++ b/lib/librte_ethdev/rte_ethdev.c
>> @@ -4181,6 +4181,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,
>> @@ -4190,8 +4194,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;
>> @@ -4206,6 +4209,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..462e765d1 100644
>> --- a/lib/librte_ethdev/rte_ethdev_driver.h
>> +++ b/lib/librte_ethdev/rte_ethdev_driver.h
>> @@ -180,6 +180,26 @@ 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);
>>  
>> +/**
>> + * @warning
>> + * @b EXPERIMENTAL: this API may change without prior notice.
>> + *
>> + * 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_experimental
>> +int
>> +rte_eth_dma_zone_free(const struct rte_eth_dev *dev, const char *ring_name,
>> +              uint16_t queue_id);
>> +
> 
> Hi Renata,
> 
> This API is not for applications, only for drivers, right?
> We recently (last release ?) introduced 'internal' (__rte_internal) attribute,
> we can use '__rte_internal' instead of '__rte_experimental' if the API is not
> for applications. This requires adding 'INTERNAL' block in .map file (sample
> 'lib/librte_eal/rte_eal_version.map')
> 
> 
> Also there are a few contribution conventions related issues which can be fixed
> while merging but please check them if you will be doing new version:
>  subsystem is only libname without prefix: 'ethdev'
>  Commit title description starts with lowercase: 'introduce'.
> sample => "ethdev: add function to release HW rings"
> 
> 
> Thanks,
> ferruh


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

* [dpdk-dev] [PATCH v5 1/2] ethdev: add function to release HW rings
  2020-06-22  9:54 [dpdk-dev] [PATCH v4 0/4] Memory corruption due to HW rings allocation Renata Saiakhova
                   ` (3 preceding siblings ...)
  2020-06-22  9:54 ` [dpdk-dev] [PATCH v4 4/4] drivers/net: Fix in em and ice " Renata Saiakhova
@ 2020-07-10 14:36 ` Ferruh Yigit
  2020-07-10 14:36   ` [dpdk-dev] [PATCH v5 2/2] drivers/net: delete HW rings while freeing queues Ferruh Yigit
  4 siblings, 1 reply; 11+ messages in thread
From: Ferruh Yigit @ 2020-07-10 14:36 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>
---
v5:
* 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] 11+ messages in thread

* [dpdk-dev] [PATCH v5 2/2] drivers/net: delete HW rings while freeing queues
  2020-07-10 14:36 ` [dpdk-dev] [PATCH v5 1/2] ethdev: add function to release HW rings Ferruh Yigit
@ 2020-07-10 14:36   ` Ferruh Yigit
  0 siblings, 0 replies; 11+ messages in thread
From: Ferruh Yigit @ 2020-07-10 14:36 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] 11+ messages in thread

end of thread, other threads:[~2020-07-10 14:37 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-22  9:54 [dpdk-dev] [PATCH v4 0/4] Memory corruption due to HW rings allocation Renata Saiakhova
2020-06-22  9:54 ` [dpdk-dev] [PATCH v4 1/4] librte_ethdev: Introduce a function to release HW rings Renata Saiakhova
2020-06-22 17:09   ` Ferruh Yigit
2020-06-23  9:11     ` Renata Saiakhova
2020-06-23 12:25       ` Ferruh Yigit
2020-06-22 17:13   ` Ferruh Yigit
2020-06-22  9:54 ` [dpdk-dev] [PATCH v4 2/4] drivers/net: Fix in igb and ixgbe HW rings memory Renata Saiakhova
2020-06-22  9:54 ` [dpdk-dev] [PATCH v4 3/4] drivers/net: Fix in i40e HW rings memory overlap Renata Saiakhova
2020-06-22  9:54 ` [dpdk-dev] [PATCH v4 4/4] drivers/net: Fix in em and ice " Renata Saiakhova
2020-07-10 14:36 ` [dpdk-dev] [PATCH v5 1/2] ethdev: add function to release HW rings Ferruh Yigit
2020-07-10 14:36   ` [dpdk-dev] [PATCH v5 2/2] drivers/net: delete HW rings while freeing queues Ferruh Yigit

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git