[dpdk-dev] [PATCH] ring: Fix memory leakage in rte_pmd_ring_devuninit()

[dpdk-dev] [PATCH] ring: Fix memory leakage in rte_pmd_ring_devuninit()

[Mauricio Vasquez B]

When freeing the device, it is also necessary to free rx_queues and tx_queues

Signed-off-by: Mauricio Vasquez B <mauricio.vasquezbernal@studenti.polito.it>
---
 drivers/net/ring/rte_eth_ring.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ring/rte_eth_ring.c b/drivers/net/ring/rte_eth_ring.c
index 9a31bce..e091e4f 100644
--- a/drivers/net/ring/rte_eth_ring.c
+++ b/drivers/net/ring/rte_eth_ring.c
@@ -582,6 +582,9 @@ rte_pmd_ring_devuninit(const char *name)
 		return -ENODEV;
 
 	eth_dev_stop(eth_dev);
+
+	rte_free(eth_dev->data->rx_queues);
+	rte_free(eth_dev->data->tx_queues);
 	rte_free(eth_dev->data->dev_private);
 	rte_free(eth_dev->data);
 
-- 
1.9.1

Re: [dpdk-dev] [PATCH] ring: Fix memory leakage in rte_pmd_ring_devuninit()

[Iremonger, Bernard]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Mauricio Vasquez B
> Sent: Wednesday, November 18, 2015 10:29 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH] ring: Fix memory leakage in
> rte_pmd_ring_devuninit()
> 
> When freeing the device, it is also necessary to free rx_queues and
> tx_queues
> 
> Signed-off-by: Mauricio Vasquez B
> <mauricio.vasquezbernal@studenti.polito.it>

Acked-by: Bernard Iremonger <bernard.iremonger@intel.com>

Re: [dpdk-dev] [PATCH] ring: Fix memory leakage in rte_pmd_ring_devuninit()

[Mcnamara, John]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Mauricio Vasquez B
> Sent: Wednesday, November 18, 2015 10:29 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH] ring: Fix memory leakage in
> rte_pmd_ring_devuninit()
> 
> When freeing the device, it is also necessary to free rx_queues and
> tx_queues
> 
> Signed-off-by: Mauricio Vasquez B
> <mauricio.vasquezbernal@studenti.polito.it>
> ---
>  drivers/net/ring/rte_eth_ring.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/net/ring/rte_eth_ring.c
> b/drivers/net/ring/rte_eth_ring.c index 9a31bce..e091e4f 100644
> --- a/drivers/net/ring/rte_eth_ring.c
> +++ b/drivers/net/ring/rte_eth_ring.c
> @@ -582,6 +582,9 @@ rte_pmd_ring_devuninit(const char *name)
>  		return -ENODEV;
> 
>  	eth_dev_stop(eth_dev);
> +
> +	rte_free(eth_dev->data->rx_queues);
> +	rte_free(eth_dev->data->tx_queues);
>  	rte_free(eth_dev->data->dev_private);
>  	rte_free(eth_dev->data);

Hi,

This should test for eth_dev->data before freeing the members. Like:

    if (eth_dev->data) {
        rte_free(eth_dev->data->rx_queues);
        rte_free(eth_dev->data->tx_queues);
        rte_free(eth_dev->data->dev_private);
    }

Thanks,

John

Re: [dpdk-dev] [PATCH] ring: Fix memory leakage in rte_pmd_ring_devuninit()

[Richardson, Bruce]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Mcnamara, John
> Sent: Friday, November 20, 2015 12:32 PM
> To: Mauricio Vasquez B <mauricio.vasquezbernal@studenti.polito.it>;
> dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] ring: Fix memory leakage in
> rte_pmd_ring_devuninit()
> 
> 
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Mauricio Vasquez
> > B
> > Sent: Wednesday, November 18, 2015 10:29 PM
> > To: dev@dpdk.org
> > Subject: [dpdk-dev] [PATCH] ring: Fix memory leakage in
> > rte_pmd_ring_devuninit()
> >
> > When freeing the device, it is also necessary to free rx_queues and
> > tx_queues
> >
> > Signed-off-by: Mauricio Vasquez B
> > <mauricio.vasquezbernal@studenti.polito.it>
> > ---
> >  drivers/net/ring/rte_eth_ring.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/net/ring/rte_eth_ring.c
> > b/drivers/net/ring/rte_eth_ring.c index 9a31bce..e091e4f 100644
> > --- a/drivers/net/ring/rte_eth_ring.c
> > +++ b/drivers/net/ring/rte_eth_ring.c
> > @@ -582,6 +582,9 @@ rte_pmd_ring_devuninit(const char *name)
> >  		return -ENODEV;
> >
> >  	eth_dev_stop(eth_dev);
> > +
> > +	rte_free(eth_dev->data->rx_queues);
> > +	rte_free(eth_dev->data->tx_queues);
> >  	rte_free(eth_dev->data->dev_private);
> >  	rte_free(eth_dev->data);
> 
> Hi,
> 
> This should test for eth_dev->data before freeing the members. Like:
> 
>     if (eth_dev->data) {
>         rte_free(eth_dev->data->rx_queues);
>         rte_free(eth_dev->data->tx_queues);
>         rte_free(eth_dev->data->dev_private);
>     }
> 
> Thanks,
> 
> John

That was my thought initially too, but since this is an uninit routine, the data field must already have been set up correctly by the init/creation function. 
That being said, the check does no harm, so we might as well add it.

/Bruce

Re: [dpdk-dev] [PATCH] ring: Fix memory leakage in rte_pmd_ring_devuninit()

[Mauricio Vásquez]

On 20 November 2015 at 13:42, Richardson, Bruce <bruce.richardson@intel.com>
wrote:

>
>
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Mcnamara, John
> > Sent: Friday, November 20, 2015 12:32 PM
> > To: Mauricio Vasquez B <mauricio.vasquezbernal@studenti.polito.it>;
> > dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH] ring: Fix memory leakage in
> > rte_pmd_ring_devuninit()
> >
> >
> >
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Mauricio Vasquez
> > > B
> > > Sent: Wednesday, November 18, 2015 10:29 PM
> > > To: dev@dpdk.org
> > > Subject: [dpdk-dev] [PATCH] ring: Fix memory leakage in
> > > rte_pmd_ring_devuninit()
> > >
> > > When freeing the device, it is also necessary to free rx_queues and
> > > tx_queues
> > >
> > > Signed-off-by: Mauricio Vasquez B
> > > <mauricio.vasquezbernal@studenti.polito.it>
> > > ---
> > >  drivers/net/ring/rte_eth_ring.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/drivers/net/ring/rte_eth_ring.c
> > > b/drivers/net/ring/rte_eth_ring.c index 9a31bce..e091e4f 100644
> > > --- a/drivers/net/ring/rte_eth_ring.c
> > > +++ b/drivers/net/ring/rte_eth_ring.c
> > > @@ -582,6 +582,9 @@ rte_pmd_ring_devuninit(const char *name)
> > >             return -ENODEV;
> > >
> > >     eth_dev_stop(eth_dev);
> > > +
> > > +   rte_free(eth_dev->data->rx_queues);
> > > +   rte_free(eth_dev->data->tx_queues);
> > >     rte_free(eth_dev->data->dev_private);
> > >     rte_free(eth_dev->data);
> >
> > Hi,
> >
> > This should test for eth_dev->data before freeing the members. Like:
> >
> >     if (eth_dev->data) {
> >         rte_free(eth_dev->data->rx_queues);
> >         rte_free(eth_dev->data->tx_queues);
> >         rte_free(eth_dev->data->dev_private);
> >     }
> >
> > Thanks,
> >
> > John
>
> That was my thought initially too, but since this is an uninit routine,
> the data field must already have been set up correctly by the init/creation
> function.
>
That was my reasoning too.

> That being said, the check does no harm, so we might as well add it.
>
I agree, I will send a new patch version.

>
> /Bruce
>

Mauricio V,

[dpdk-dev] [PATCH] ring: Fix memory leakage in rte_pmd_ring_devuninit

[Mauricio Vasquez B]

When freeing the device it is also neccesary to free
rx_queues and tx_queues

Signed-off-by: Mauricio Vasquez B <mauricio.vasquezbernal@studenti.polito.it>
---
v2:
Added extra control before freeing members of eth_dev->data

 drivers/net/ring/rte_eth_ring.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ring/rte_eth_ring.c b/drivers/net/ring/rte_eth_ring.c
index 9a31bce..4ce0256 100644
--- a/drivers/net/ring/rte_eth_ring.c
+++ b/drivers/net/ring/rte_eth_ring.c
@@ -582,7 +582,12 @@ rte_pmd_ring_devuninit(const char *name)
 		return -ENODEV;
 
 	eth_dev_stop(eth_dev);
-	rte_free(eth_dev->data->dev_private);
+
+	if(eth_dev->data) {
+		rte_free(eth_dev->data->rx_queues);
+		rte_free(eth_dev->data->tx_queues);
+		rte_free(eth_dev->data->dev_private);
+	}
 	rte_free(eth_dev->data);
 
 	rte_eth_dev_release_port(eth_dev);
-- 
1.9.1

Re: [dpdk-dev] [PATCH] ring: Fix memory leakage in rte_pmd_ring_devuninit

[Thomas Monjalon]

2015-11-20 19:24, Mauricio Vasquez B:
> When freeing the device it is also neccesary to free
> rx_queues and tx_queues
> 
> Signed-off-by: Mauricio Vasquez B <mauricio.vasquezbernal@studenti.polito.it>

Applied with spacing fix, thanks