diff mbox series

[v3,3/4] drivers/net: Fix in i40e HW rings memory overlap

Message ID 20200513131425.27817-4-Renata.Saiakhova@ekinops.com (mailing list archive)
State Changes Requested, archived
Delegated to: Ferruh Yigit
Headers
Series Memory corruption due to HW rings allocation |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Renata Saiakhova May 13, 2020, 1:14 p.m. UTC 
  Delete memzones for HW rings in i40e while freeing queues

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

Comments

Zhao1, Wei June 1, 2020, 7:58 a.m. UTC | #1 
Hi, Renata Saiakhova

   I think this patch is very important, It seems all kind of NIC has memory leak problem that used for store
Tx or rx descriptor. If that is true, memory point by rxq-> rx_ring/ txq-> tx_ring will never be freed even if dev_close? 
Is my understanding right or wrong?

If that is true, it seems you should also add in functioni40e_fdir_teardown(), because
Tx_ring allocated in i40e_fdir_setup_tx_resources() also need freed, and memzones need to be delete.
Is that so?

Thanks.



> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Renata Saiakhova
> Sent: Wednesday, May 13, 2020 9:14 PM
> To: dev@dpdk.org
> Cc: Renata Saiakhova <Renata.Saiakhova@ekinops.com>
> Subject: [dpdk-dev] [PATCH v3 3/4] drivers/net: Fix in i40e HW rings memory
> overlap
> 
> Delete memzones for HW rings in i40e while freeing queues
> 
> Signed-off-by: Renata Saiakhova <Renata.Saiakhova@ekinops.com>
> ---
>  drivers/net/i40e/i40e_rxtx.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> 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
Ferruh Yigit June 19, 2020, 4:56 p.m. UTC | #2 
On 6/1/2020 8:58 AM, Zhao1, Wei wrote:
> Hi, Renata Saiakhova
> 
>    I think this patch is very important, It seems all kind of NIC has memory leak problem that used for store
> Tx or rx descriptor. If that is true, memory point by rxq-> rx_ring/ txq-> tx_ring will never be freed even if dev_close? 
> Is my understanding right or wrong?

That is right as far as I understand, and yes we should free them in dev_close.
I think DPDK relies on that the memory will be freed on the process exit and
sometimes behaves lazy on the memory free as being this case.

> 
> If that is true, it seems you should also add in functioni40e_fdir_teardown(), because
> Tx_ring allocated in i40e_fdir_setup_tx_resources() also need freed, and memzones need to be delete.
> Is that so?
> 
> Thanks.
> 
> 
> 
>> -----Original Message-----
>> From: dev <dev-bounces@dpdk.org> On Behalf Of Renata Saiakhova
>> Sent: Wednesday, May 13, 2020 9:14 PM
>> To: dev@dpdk.org
>> Cc: Renata Saiakhova <Renata.Saiakhova@ekinops.com>
>> Subject: [dpdk-dev] [PATCH v3 3/4] drivers/net: Fix in i40e HW rings memory
>> overlap
>>
>> Delete memzones for HW rings in i40e while freeing queues
>>
>> Signed-off-by: Renata Saiakhova <Renata.Saiakhova@ekinops.com>
>> ---
>>  drivers/net/i40e/i40e_rxtx.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> 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
>
Zhao1, Wei June 20, 2020, 3:02 a.m. UTC | #3 
> -----Original Message-----
> From: Yigit, Ferruh <ferruh.yigit@intel.com>
> Sent: Saturday, June 20, 2020 12:56 AM
> To: Zhao1, Wei <wei.zhao1@intel.com>; Renata Saiakhova
> <Renata.Saiakhova@ekinops.com>; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v3 3/4] drivers/net: Fix in i40e HW rings
> memory overlap
> 
> On 6/1/2020 8:58 AM, Zhao1, Wei wrote:
> > Hi, Renata Saiakhova
> >
> >    I think this patch is very important, It seems all kind of NIC has
> > memory leak problem that used for store Tx or rx descriptor. If that is true,
> memory point by rxq-> rx_ring/ txq-> tx_ring will never be freed even if
> dev_close?
> > Is my understanding right or wrong?
> 
> That is right as far as I understand, and yes we should free them in dev_close.
> I think DPDK relies on that the memory will be freed on the process exit and
> sometimes behaves lazy on the memory free as being this case.
> 

Thanks! Hope for accept for this patch.

> >
> > If that is true, it seems you should also add in
> > functioni40e_fdir_teardown(), because Tx_ring allocated in
> i40e_fdir_setup_tx_resources() also need freed, and memzones need to be
> delete.
> > Is that so?
> >
> > Thanks.
> >
> >
> >
> >> -----Original Message-----
> >> From: dev <dev-bounces@dpdk.org> On Behalf Of Renata Saiakhova
> >> Sent: Wednesday, May 13, 2020 9:14 PM
> >> To: dev@dpdk.org
> >> Cc: Renata Saiakhova <Renata.Saiakhova@ekinops.com>
> >> Subject: [dpdk-dev] [PATCH v3 3/4] drivers/net: Fix in i40e HW rings
> >> memory overlap
> >>
> >> Delete memzones for HW rings in i40e while freeing queues
> >>
> >> Signed-off-by: Renata Saiakhova <Renata.Saiakhova@ekinops.com>
> >> ---
> >>  drivers/net/i40e/i40e_rxtx.c | 2 ++
> >>  1 file changed, 2 insertions(+)
> >>
> >> 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
> >
diff mbox series

Patch

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);
 	}
 }