[PATCH v1] net/iavf: fix resource leak issue

[PATCH v1] net/iavf: fix resource leak issue

[Wenjun Wu]

This patch fixes resource leak issue reported by coverity.

Coverity issue: 378017
Fixes: b14e8a57b9fe ("net/iavf: support quanta size configuration")

Signed-off-by: Wenjun Wu <wenjun1.wu@intel.com>
---
 drivers/net/iavf/iavf_ethdev.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c
index d1a2b53675..82672841f4 100644
--- a/drivers/net/iavf/iavf_ethdev.c
+++ b/drivers/net/iavf/iavf_ethdev.c
@@ -2188,7 +2188,8 @@ static int iavf_parse_devargs(struct rte_eth_dev *dev)
 	if (ad->devargs.quanta_size < 256 || ad->devargs.quanta_size > 4096 ||
 	    ad->devargs.quanta_size & 0x40) {
 		PMD_INIT_LOG(ERR, "invalid quanta size\n");
-		return -EINVAL;
+		ret = -EINVAL;
+		goto bail;
 	}
 
 bail:
-- 
2.25.1

RE: [PATCH v1] net/iavf: fix resource leak issue

[Zhang, Qi Z]

> -----Original Message-----
> From: Wu, Wenjun1 <wenjun1.wu@intel.com>
> Sent: Tuesday, May 10, 2022 8:54 AM
> To: dev@dpdk.org; Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei
> <beilei.xing@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: Wu, Wenjun1 <wenjun1.wu@intel.com>
> Subject: [PATCH v1] net/iavf: fix resource leak issue
> 
> This patch fixes resource leak issue reported by coverity.
> 
> Coverity issue: 378017
> Fixes: b14e8a57b9fe ("net/iavf: support quanta size configuration")
> 
> Signed-off-by: Wenjun Wu <wenjun1.wu@intel.com>

Acked-by: Qi Zhang <qi.z.zhang@intel.com>

Applied to dpdk-next-net-intel.

Thanks
Qi

Re: [PATCH v1] net/iavf: fix resource leak issue

[Thomas Monjalon]

11/05/2022 13:46, Zhang, Qi Z:
> From: Wu, Wenjun1 <wenjun1.wu@intel.com>
> > 
> > This patch fixes resource leak issue reported by coverity.
> > 
> > Coverity issue: 378017
> > Fixes: b14e8a57b9fe ("net/iavf: support quanta size configuration")
> > 
> > Signed-off-by: Wenjun Wu <wenjun1.wu@intel.com>
> 
> Acked-by: Qi Zhang <qi.z.zhang@intel.com>
> 
> Applied to dpdk-next-net-intel.

In the title, the word "issue" is useless.
It would be more interesting to precise where is the leak.

RE: [PATCH v1] net/iavf: fix resource leak issue

[Zhang, Qi Z]

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Wednesday, May 11, 2022 10:52 PM
> To: Wu, Wenjun1 <wenjun1.wu@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org; Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei
> <beilei.xing@intel.com>
> Subject: Re: [PATCH v1] net/iavf: fix resource leak issue
> 
> 11/05/2022 13:46, Zhang, Qi Z:
> > From: Wu, Wenjun1 <wenjun1.wu@intel.com>
> > >
> > > This patch fixes resource leak issue reported by coverity.
> > >
> > > Coverity issue: 378017
> > > Fixes: b14e8a57b9fe ("net/iavf: support quanta size configuration")
> > >
> > > Signed-off-by: Wenjun Wu <wenjun1.wu@intel.com>
> >
> > Acked-by: Qi Zhang <qi.z.zhang@intel.com>
> >
> > Applied to dpdk-next-net-intel.
> 
> In the title, the word "issue" is useless.
> It would be more interesting to precise where is the leak.
> 

Added below into commit log in dpdk-next-net-intel.

kvlist should be freed before return an error due to invalid quanta size. 

Re: [PATCH v1] net/iavf: fix resource leak issue

[Thomas Monjalon]

11/05/2022 17:31, Zhang, Qi Z:
> From: Thomas Monjalon <thomas@monjalon.net>
> > 11/05/2022 13:46, Zhang, Qi Z:
> > > From: Wu, Wenjun1 <wenjun1.wu@intel.com>
> > > >
> > > > This patch fixes resource leak issue reported by coverity.
> > > >
> > > > Coverity issue: 378017
> > > > Fixes: b14e8a57b9fe ("net/iavf: support quanta size configuration")
> > > >
> > > > Signed-off-by: Wenjun Wu <wenjun1.wu@intel.com>
> > >
> > > Acked-by: Qi Zhang <qi.z.zhang@intel.com>
> > >
> > > Applied to dpdk-next-net-intel.
> > 
> > In the title, the word "issue" is useless.
> > It would be more interesting to precise where is the leak.
> > 
> 
> Added below into commit log in dpdk-next-net-intel.
> 
> kvlist should be freed before return an error due to invalid quanta size. 

It does not explain what is the scenario. What is the impacted feature?

RE: [PATCH v1] net/iavf: fix resource leak issue

[Zhang, Qi Z]

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Thursday, May 12, 2022 5:17 AM
> To: Wu, Wenjun1 <wenjun1.wu@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org; Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei
> <beilei.xing@intel.com>
> Subject: Re: [PATCH v1] net/iavf: fix resource leak issue
> 
> 11/05/2022 17:31, Zhang, Qi Z:
> > From: Thomas Monjalon <thomas@monjalon.net>
> > > 11/05/2022 13:46, Zhang, Qi Z:
> > > > From: Wu, Wenjun1 <wenjun1.wu@intel.com>
> > > > >
> > > > > This patch fixes resource leak issue reported by coverity.
> > > > >
> > > > > Coverity issue: 378017
> > > > > Fixes: b14e8a57b9fe ("net/iavf: support quanta size configuration")
> > > > >
> > > > > Signed-off-by: Wenjun Wu <wenjun1.wu@intel.com>
> > > >
> > > > Acked-by: Qi Zhang <qi.z.zhang@intel.com>
> > > >
> > > > Applied to dpdk-next-net-intel.
> > >
> > > In the title, the word "issue" is useless.
> > > It would be more interesting to precise where is the leak.
> > >
> >
> > Added below into commit log in dpdk-next-net-intel.
> >
> > kvlist should be freed before return an error due to invalid quanta size.
> 
> It does not explain what is the scenario. What is the impacted feature?

I think the scenario has been explained as

"kvlist should be freed before return an error due to invalid quanta size"

the impact has been mentioned as "resource leak" I guess this is how Coverity describe the issue

i agree "memory leak" would be better.

> 

Re: [PATCH v1] net/iavf: fix resource leak issue

[Thomas Monjalon]

12/05/2022 02:20, Zhang, Qi Z:
> 
> > -----Original Message-----
> > From: Thomas Monjalon <thomas@monjalon.net>
> > Sent: Thursday, May 12, 2022 5:17 AM
> > To: Wu, Wenjun1 <wenjun1.wu@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
> > Cc: dev@dpdk.org; Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei
> > <beilei.xing@intel.com>
> > Subject: Re: [PATCH v1] net/iavf: fix resource leak issue
> > 
> > 11/05/2022 17:31, Zhang, Qi Z:
> > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > 11/05/2022 13:46, Zhang, Qi Z:
> > > > > From: Wu, Wenjun1 <wenjun1.wu@intel.com>
> > > > > >
> > > > > > This patch fixes resource leak issue reported by coverity.
> > > > > >
> > > > > > Coverity issue: 378017
> > > > > > Fixes: b14e8a57b9fe ("net/iavf: support quanta size configuration")
> > > > > >
> > > > > > Signed-off-by: Wenjun Wu <wenjun1.wu@intel.com>
> > > > >
> > > > > Acked-by: Qi Zhang <qi.z.zhang@intel.com>
> > > > >
> > > > > Applied to dpdk-next-net-intel.
> > > >
> > > > In the title, the word "issue" is useless.
> > > > It would be more interesting to precise where is the leak.
> > > >
> > >
> > > Added below into commit log in dpdk-next-net-intel.
> > >
> > > kvlist should be freed before return an error due to invalid quanta size.
> > 
> > It does not explain what is the scenario. What is the impacted feature?
> 
> I think the scenario has been explained as
> 
> "kvlist should be freed before return an error due to invalid quanta size"
> the impact has been mentioned as "resource leak" I guess this is how Coverity describe the issue

No, it does not say how to reproduce (the scenario).
We have no idea where it is in the code.

RE: [PATCH v1] net/iavf: fix resource leak issue

[Zhang, Qi Z]

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Thursday, May 12, 2022 3:07 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: Wu, Wenjun1 <wenjun1.wu@intel.com>; dev@dpdk.org; Wu, Jingjing
> <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>
> Subject: Re: [PATCH v1] net/iavf: fix resource leak issue
> 
> 12/05/2022 02:20, Zhang, Qi Z:
> >
> > > -----Original Message-----
> > > From: Thomas Monjalon <thomas@monjalon.net>
> > > Sent: Thursday, May 12, 2022 5:17 AM
> > > To: Wu, Wenjun1 <wenjun1.wu@intel.com>; Zhang, Qi Z
> > > <qi.z.zhang@intel.com>
> > > Cc: dev@dpdk.org; Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei
> > > <beilei.xing@intel.com>
> > > Subject: Re: [PATCH v1] net/iavf: fix resource leak issue
> > >
> > > 11/05/2022 17:31, Zhang, Qi Z:
> > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > > 11/05/2022 13:46, Zhang, Qi Z:
> > > > > > From: Wu, Wenjun1 <wenjun1.wu@intel.com>
> > > > > > >
> > > > > > > This patch fixes resource leak issue reported by coverity.
> > > > > > >
> > > > > > > Coverity issue: 378017
> > > > > > > Fixes: b14e8a57b9fe ("net/iavf: support quanta size
> > > > > > > configuration")
> > > > > > >
> > > > > > > Signed-off-by: Wenjun Wu <wenjun1.wu@intel.com>
> > > > > >
> > > > > > Acked-by: Qi Zhang <qi.z.zhang@intel.com>
> > > > > >
> > > > > > Applied to dpdk-next-net-intel.
> > > > >
> > > > > In the title, the word "issue" is useless.
> > > > > It would be more interesting to precise where is the leak.
> > > > >
> > > >
> > > > Added below into commit log in dpdk-next-net-intel.
> > > >
> > > > kvlist should be freed before return an error due to invalid quanta size.
> > >
> > > It does not explain what is the scenario. What is the impacted feature?
> >
> > I think the scenario has been explained as
> >
> > "kvlist should be freed before return an error due to invalid quanta size"
> > the impact has been mentioned as "resource leak" I guess this is how
> > Coverity describe the issue
> 
> No, it does not say how to reproduce (the scenario).
> We have no idea where it is in the code.

set an invalid quanta size from devargs will reproduce the issue
> 
> 

RE: [PATCH v1] net/iavf: fix resource leak issue

[Zhang, Qi Z]

> -----Original Message-----
> From: Zhang, Qi Z <qi.z.zhang@intel.com>
> Sent: Thursday, May 12, 2022 3:21 PM
> To: Thomas Monjalon <thomas@monjalon.net>
> Cc: Wu, Wenjun1 <wenjun1.wu@intel.com>; dev@dpdk.org; Wu, Jingjing
> <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>
> Subject: RE: [PATCH v1] net/iavf: fix resource leak issue
> 
> 
> 
> > -----Original Message-----
> > From: Thomas Monjalon <thomas@monjalon.net>
> > Sent: Thursday, May 12, 2022 3:07 PM
> > To: Zhang, Qi Z <qi.z.zhang@intel.com>
> > Cc: Wu, Wenjun1 <wenjun1.wu@intel.com>; dev@dpdk.org; Wu, Jingjing
> > <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>
> > Subject: Re: [PATCH v1] net/iavf: fix resource leak issue
> >
> > 12/05/2022 02:20, Zhang, Qi Z:
> > >
> > > > -----Original Message-----
> > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > Sent: Thursday, May 12, 2022 5:17 AM
> > > > To: Wu, Wenjun1 <wenjun1.wu@intel.com>; Zhang, Qi Z
> > > > <qi.z.zhang@intel.com>
> > > > Cc: dev@dpdk.org; Wu, Jingjing <jingjing.wu@intel.com>; Xing,
> > > > Beilei <beilei.xing@intel.com>
> > > > Subject: Re: [PATCH v1] net/iavf: fix resource leak issue
> > > >
> > > > 11/05/2022 17:31, Zhang, Qi Z:
> > > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > > > 11/05/2022 13:46, Zhang, Qi Z:
> > > > > > > From: Wu, Wenjun1 <wenjun1.wu@intel.com>
> > > > > > > >
> > > > > > > > This patch fixes resource leak issue reported by coverity.
> > > > > > > >
> > > > > > > > Coverity issue: 378017
> > > > > > > > Fixes: b14e8a57b9fe ("net/iavf: support quanta size
> > > > > > > > configuration")
> > > > > > > >
> > > > > > > > Signed-off-by: Wenjun Wu <wenjun1.wu@intel.com>
> > > > > > >
> > > > > > > Acked-by: Qi Zhang <qi.z.zhang@intel.com>
> > > > > > >
> > > > > > > Applied to dpdk-next-net-intel.
> > > > > >
> > > > > > In the title, the word "issue" is useless.
> > > > > > It would be more interesting to precise where is the leak.
> > > > > >
> > > > >
> > > > > Added below into commit log in dpdk-next-net-intel.
> > > > >
> > > > > kvlist should be freed before return an error due to invalid quanta size.
> > > >
> > > > It does not explain what is the scenario. What is the impacted feature?
> > >
> > > I think the scenario has been explained as
> > >
> > > "kvlist should be freed before return an error due to invalid quanta size"
> > > the impact has been mentioned as "resource leak" I guess this is how
> > > Coverity describe the issue
> >
> > No, it does not say how to reproduce (the scenario).
> > We have no idea where it is in the code.
> 
> set an invalid quanta size from devargs will reproduce the issue
> >
> >
Refine the Title / commit log in dpdk-next-net-intel as below to answer Thomas's questions

net/iavf: fix memory leak issue

Set an invalid quanta size from devargs will cause memory leak and this
is reported by coverity.

The patch fix the issue by correcting the error handle.

Re: [PATCH v1] net/iavf: fix resource leak issue

[Thomas Monjalon]

17/05/2022 02:59, Zhang, Qi Z:
> From: Zhang, Qi Z <qi.z.zhang@intel.com>
> > From: Thomas Monjalon <thomas@monjalon.net>
> > > 12/05/2022 02:20, Zhang, Qi Z:
> > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > > 11/05/2022 17:31, Zhang, Qi Z:
> > > > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > > > > 11/05/2022 13:46, Zhang, Qi Z:
> > > > > > > > From: Wu, Wenjun1 <wenjun1.wu@intel.com>
> > > > > > > > >
> > > > > > > > > This patch fixes resource leak issue reported by coverity.
> > > > > > > > >
> > > > > > > > > Coverity issue: 378017
> > > > > > > > > Fixes: b14e8a57b9fe ("net/iavf: support quanta size
> > > > > > > > > configuration")
> > > > > > > > >
> > > > > > > > > Signed-off-by: Wenjun Wu <wenjun1.wu@intel.com>
> > > > > > > >
> > > > > > > > Acked-by: Qi Zhang <qi.z.zhang@intel.com>
> > > > > > > >
> > > > > > > > Applied to dpdk-next-net-intel.
> > > > > > >
> > > > > > > In the title, the word "issue" is useless.
> > > > > > > It would be more interesting to precise where is the leak.
> > > > > > >
> > > > > >
> > > > > > Added below into commit log in dpdk-next-net-intel.
> > > > > >
> > > > > > kvlist should be freed before return an error due to invalid quanta size.
> > > > >
> > > > > It does not explain what is the scenario. What is the impacted feature?
> > > >
> > > > I think the scenario has been explained as
> > > >
> > > > "kvlist should be freed before return an error due to invalid quanta size"
> > > > the impact has been mentioned as "resource leak" I guess this is how
> > > > Coverity describe the issue
> > >
> > > No, it does not say how to reproduce (the scenario).
> > > We have no idea where it is in the code.
> > 
> > set an invalid quanta size from devargs will reproduce the issue
> > >
> > >
> Refine the Title / commit log in dpdk-next-net-intel as below to answer Thomas's questions
> 
> net/iavf: fix memory leak issue

A memory leak is an issue for sure, so no need to add "issue".

> Set an invalid quanta size from devargs will cause memory leak and this

Set -> Setting

> is reported by coverity.
> 
> The patch fix the issue by correcting the error handle.

handle -> handling

Thanks

RE: [PATCH v1] net/iavf: fix resource leak issue

[Zhang, Qi Z]

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Tuesday, May 17, 2022 2:52 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: Wu, Wenjun1 <wenjun1.wu@intel.com>; dev@dpdk.org; Wu, Jingjing
> <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>
> Subject: Re: [PATCH v1] net/iavf: fix resource leak issue
> 
> 17/05/2022 02:59, Zhang, Qi Z:
> > From: Zhang, Qi Z <qi.z.zhang@intel.com>
> > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > 12/05/2022 02:20, Zhang, Qi Z:
> > > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > > > 11/05/2022 17:31, Zhang, Qi Z:
> > > > > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > > > > > 11/05/2022 13:46, Zhang, Qi Z:
> > > > > > > > > From: Wu, Wenjun1 <wenjun1.wu@intel.com>
> > > > > > > > > >
> > > > > > > > > > This patch fixes resource leak issue reported by coverity.
> > > > > > > > > >
> > > > > > > > > > Coverity issue: 378017
> > > > > > > > > > Fixes: b14e8a57b9fe ("net/iavf: support quanta size
> > > > > > > > > > configuration")
> > > > > > > > > >
> > > > > > > > > > Signed-off-by: Wenjun Wu <wenjun1.wu@intel.com>
> > > > > > > > >
> > > > > > > > > Acked-by: Qi Zhang <qi.z.zhang@intel.com>
> > > > > > > > >
> > > > > > > > > Applied to dpdk-next-net-intel.
> > > > > > > >
> > > > > > > > In the title, the word "issue" is useless.
> > > > > > > > It would be more interesting to precise where is the leak.
> > > > > > > >
> > > > > > >
> > > > > > > Added below into commit log in dpdk-next-net-intel.
> > > > > > >
> > > > > > > kvlist should be freed before return an error due to invalid quanta
> size.
> > > > > >
> > > > > > It does not explain what is the scenario. What is the impacted feature?
> > > > >
> > > > > I think the scenario has been explained as
> > > > >
> > > > > "kvlist should be freed before return an error due to invalid quanta size"
> > > > > the impact has been mentioned as "resource leak" I guess this is
> > > > > how Coverity describe the issue
> > > >
> > > > No, it does not say how to reproduce (the scenario).
> > > > We have no idea where it is in the code.
> > >
> > > set an invalid quanta size from devargs will reproduce the issue
> > > >
> > > >
> > Refine the Title / commit log in dpdk-next-net-intel as below to
> > answer Thomas's questions
> >
> > net/iavf: fix memory leak issue
> 
> A memory leak is an issue for sure, so no need to add "issue".
> 
> > Set an invalid quanta size from devargs will cause memory leak and
> > this
> 
> Set -> Setting
> 
> > is reported by coverity.
> >
> > The patch fix the issue by correcting the error handle.
> 
> handle -> handling
> 
> Thanks
> 

Ok fixed grammar in dpdk-next-net-intel as suggested.