bus/vdev: fix devargs memory leak

Message ID 20230901072409.741847-1-mingjinx.ye@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers
Series bus/vdev: fix devargs memory leak |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/github-robot: build success github build: passed
ci/iol-compile-amd64-testing success Testing PASS
ci/iol-unit-amd64-testing success Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/intel-Functional success Functional PASS
ci/iol-sample-apps-testing success Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-compile-arm64-testing success Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS

Commit Message

Mingjin Ye Sept. 1, 2023, 7:24 a.m. UTC
  When a device is created by a secondary process, an empty devargs is
temporarily generated and bound to it. This causes the device to not
be associated with the correct devargs, and the empty devargs are not
released when the resource is freed.

This patch fixes the issue by matching the devargs when inserting a
device in secondary process.

Fixes: dda987315ca2 ("vdev: make virtual bus use its device struct")
Fixes: a16040453968 ("eal: extract vdev infra")
Cc: stable@dpdk.org

Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>
---
 drivers/bus/vdev/vdev.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)
  

Comments

Ling, WeiX Sept. 12, 2023, 9:07 a.m. UTC | #1
Unaddressed
> -----Original Message-----
> From: Mingjin Ye <mingjinx.ye@intel.com>
> Sent: Friday, September 1, 2023 3:24 PM
> To: dev@dpdk.org
> Cc: Yang, Qiming <qiming.yang@intel.com>; Zhou, YidingX
> <yidingx.zhou@intel.com>; Ye, MingjinX <mingjinx.ye@intel.com>;
> stable@dpdk.org; Burakov, Anatoly <anatoly.burakov@intel.com>
> Subject: [PATCH] bus/vdev: fix devargs memory leak
> 
> When a device is created by a secondary process, an empty devargs is
> temporarily generated and bound to it. This causes the device to not be
> associated with the correct devargs, and the empty devargs are not released
> when the resource is freed.
> 
> This patch fixes the issue by matching the devargs when inserting a device in
> secondary process.
> 
> Fixes: dda987315ca2 ("vdev: make virtual bus use its device struct")
> Fixes: a16040453968 ("eal: extract vdev infra")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>
> ---

Tested-by: Wei Ling <weix.ling@intel.com>
  
Mingjin Ye Sept. 15, 2023, 8:56 a.m. UTC | #2
Hi Anatoly,

Could you please review and provide suggestions if any.

Thanks,
Mingjin

> -----Original Message-----
> From: Ling, WeiX <weix.ling@intel.com>
> Sent: Tuesday, September 12, 2023 5:08 PM
> To: Ye, MingjinX <mingjinx.ye@intel.com>; dev@dpdk.org
> Cc: Yang, Qiming <qiming.yang@intel.com>; Zhou, YidingX
> <yidingx.zhou@intel.com>; Ye, MingjinX <mingjinx.ye@intel.com>;
> stable@dpdk.org; Burakov, Anatoly <anatoly.burakov@intel.com>
> Subject: RE: [PATCH] bus/vdev: fix devargs memory leak
> 
> > -----Original Message-----
> > From: Mingjin Ye <mingjinx.ye@intel.com>
> > Sent: Friday, September 1, 2023 3:24 PM
> > To: dev@dpdk.org
> > Cc: Yang, Qiming <qiming.yang@intel.com>; Zhou, YidingX
> > <yidingx.zhou@intel.com>; Ye, MingjinX <mingjinx.ye@intel.com>;
> > stable@dpdk.org; Burakov, Anatoly <anatoly.burakov@intel.com>
> > Subject: [PATCH] bus/vdev: fix devargs memory leak
> >
> > When a device is created by a secondary process, an empty devargs is
> > temporarily generated and bound to it. This causes the device to not
> > be associated with the correct devargs, and the empty devargs are not
> > released when the resource is freed.
> >
> > This patch fixes the issue by matching the devargs when inserting a
> > device in secondary process.
> >
> > Fixes: dda987315ca2 ("vdev: make virtual bus use its device struct")
> > Fixes: a16040453968 ("eal: extract vdev infra")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>
> > ---
> 
> Tested-by: Wei Ling <weix.ling@intel.com>
  
Mingjin Ye Nov. 7, 2023, 6:56 a.m. UTC | #3
Hi Anatoly,

can you please take a look at this patch.

Thanks,
Mingjin

> -----Original Message-----
> From: Ye, MingjinX <mingjinx.ye@intel.com>
> Sent: Friday, September 15, 2023 4:56 PM
> To: Burakov, Anatoly <anatoly.burakov@intel.com>; dev@dpdk.org
> Cc: Yang, Qiming <qiming.yang@intel.com>; Zhou, YidingX
> <yidingx.zhou@intel.com>; stable@dpdk.org; Ling, WeiX
> <weix.ling@intel.com>
> Subject: RE: [PATCH] bus/vdev: fix devargs memory leak
> 
> Hi Anatoly,
> 
> Could you please review and provide suggestions if any.
> 
> Thanks,
> Mingjin
> 
> > -----Original Message-----
> > From: Ling, WeiX <weix.ling@intel.com>
> > Sent: Tuesday, September 12, 2023 5:08 PM
> > To: Ye, MingjinX <mingjinx.ye@intel.com>; dev@dpdk.org
> > Cc: Yang, Qiming <qiming.yang@intel.com>; Zhou, YidingX
> > <yidingx.zhou@intel.com>; Ye, MingjinX <mingjinx.ye@intel.com>;
> > stable@dpdk.org; Burakov, Anatoly <anatoly.burakov@intel.com>
> > Subject: RE: [PATCH] bus/vdev: fix devargs memory leak
> >
> > > -----Original Message-----
> > > From: Mingjin Ye <mingjinx.ye@intel.com>
> > > Sent: Friday, September 1, 2023 3:24 PM
> > > To: dev@dpdk.org
> > > Cc: Yang, Qiming <qiming.yang@intel.com>; Zhou, YidingX
> > > <yidingx.zhou@intel.com>; Ye, MingjinX <mingjinx.ye@intel.com>;
> > > stable@dpdk.org; Burakov, Anatoly <anatoly.burakov@intel.com>
> > > Subject: [PATCH] bus/vdev: fix devargs memory leak
> > >
> > > When a device is created by a secondary process, an empty devargs is
> > > temporarily generated and bound to it. This causes the device to not
> > > be associated with the correct devargs, and the empty devargs are
> > > not released when the resource is freed.
> > >
> > > This patch fixes the issue by matching the devargs when inserting a
> > > device in secondary process.
> > >
> > > Fixes: dda987315ca2 ("vdev: make virtual bus use its device struct")
> > > Fixes: a16040453968 ("eal: extract vdev infra")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>
> > > ---
> >
> > Tested-by: Wei Ling <weix.ling@intel.com>
  
Mingjin Ye Nov. 17, 2023, 10:29 a.m. UTC | #4
Hi Burakov,

can you please take a look at this patch.

Thanks,
Mingjin

> > -----Original Message-----
> > From: Ling, WeiX <weix.ling@intel.com>
> > Sent: Tuesday, September 12, 2023 5:08 PM
> > To: Ye, MingjinX <mingjinx.ye@intel.com>; dev@dpdk.org
> > Cc: Yang, Qiming <qiming.yang@intel.com>; Zhou, YidingX
> > <yidingx.zhou@intel.com>; Ye, MingjinX <mingjinx.ye@intel.com>;
> > stable@dpdk.org; Burakov, Anatoly <anatoly.burakov@intel.com>
> > Subject: RE: [PATCH] bus/vdev: fix devargs memory leak
> >
> > > -----Original Message-----
> > > From: Mingjin Ye <mingjinx.ye@intel.com>
> > > Sent: Friday, September 1, 2023 3:24 PM
> > > To: dev@dpdk.org
> > > Cc: Yang, Qiming <qiming.yang@intel.com>; Zhou, YidingX
> > > <yidingx.zhou@intel.com>; Ye, MingjinX <mingjinx.ye@intel.com>;
> > > stable@dpdk.org; Burakov, Anatoly <anatoly.burakov@intel.com>
> > > Subject: [PATCH] bus/vdev: fix devargs memory leak
> > >
> > > When a device is created by a secondary process, an empty devargs is
> > > temporarily generated and bound to it. This causes the device to not
> > > be associated with the correct devargs, and the empty devargs are
> > > not released when the resource is freed.
> > >
> > > This patch fixes the issue by matching the devargs when inserting a
> > > device in secondary process.
> > >
> > > Fixes: dda987315ca2 ("vdev: make virtual bus use its device struct")
> > > Fixes: a16040453968 ("eal: extract vdev infra")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>
> > > ---
> >
> > Tested-by: Wei Ling <weix.ling@intel.com>
  
Mingjin Ye Dec. 5, 2023, 10:32 a.m. UTC | #5
Hi Burakov,

The patch has been sent for your review for quite some time. Can you please help to take a look and give some feedback?  Your prompt response will be appreciated a lot and very helpful for our next step moving forward.



-Mingjin

> -----Original Message-----
> From: Ye, MingjinX <mingjinx.ye@intel.com>
> Sent: 2023年11月17日 18:29
> To: Ye, MingjinX <mingjinx.ye@intel.com>; Burakov, Anatoly
> <anatoly.burakov@intel.com>; dev@dpdk.org
> Cc: Yang, Qiming <qiming.yang@intel.com>; Zhou, YidingX
> <yidingx.zhou@intel.com>; stable@dpdk.org; Ling, WeiX
> <weix.ling@intel.com>
> Subject: RE: [PATCH] bus/vdev: fix devargs memory leak
> 
> Hi Burakov,
> 
> can you please take a look at this patch.
> 
> Thanks,
> Mingjin
> 
> > > -----Original Message-----
> > > From: Ling, WeiX <weix.ling@intel.com>
> > > Sent: Tuesday, September 12, 2023 5:08 PM
> > > To: Ye, MingjinX <mingjinx.ye@intel.com>; dev@dpdk.org
> > > Cc: Yang, Qiming <qiming.yang@intel.com>; Zhou, YidingX
> > > <yidingx.zhou@intel.com>; Ye, MingjinX <mingjinx.ye@intel.com>;
> > > stable@dpdk.org; Burakov, Anatoly <anatoly.burakov@intel.com>
> > > Subject: RE: [PATCH] bus/vdev: fix devargs memory leak
> > >
> > > > -----Original Message-----
> > > > From: Mingjin Ye <mingjinx.ye@intel.com>
> > > > Sent: Friday, September 1, 2023 3:24 PM
> > > > To: dev@dpdk.org
> > > > Cc: Yang, Qiming <qiming.yang@intel.com>; Zhou, YidingX
> > > > <yidingx.zhou@intel.com>; Ye, MingjinX <mingjinx.ye@intel.com>;
> > > > stable@dpdk.org; Burakov, Anatoly <anatoly.burakov@intel.com>
> > > > Subject: [PATCH] bus/vdev: fix devargs memory leak
> > > >
> > > > When a device is created by a secondary process, an empty devargs
> > > > is temporarily generated and bound to it. This causes the device
> > > > to not be associated with the correct devargs, and the empty
> > > > devargs are not released when the resource is freed.
> > > >
> > > > This patch fixes the issue by matching the devargs when inserting
> > > > a device in secondary process.
> > > >
> > > > Fixes: dda987315ca2 ("vdev: make virtual bus use its device
> > > > struct")
> > > > Fixes: a16040453968 ("eal: extract vdev infra")
> > > > Cc: stable@dpdk.org
> > > >
> > > > Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>
> > > > ---
> > >
> > > Tested-by: Wei Ling <weix.ling@intel.com>
  
Anatoly Burakov Feb. 2, 2024, 2:52 p.m. UTC | #6
On 9/1/2023 9:24 AM, Mingjin Ye wrote:
> When a device is created by a secondary process, an empty devargs is
> temporarily generated and bound to it. This causes the device to not
> be associated with the correct devargs, and the empty devargs are not
> released when the resource is freed.
> 
> This patch fixes the issue by matching the devargs when inserting a
> device in secondary process.
> 
> Fixes: dda987315ca2 ("vdev: make virtual bus use its device struct")
> Fixes: a16040453968 ("eal: extract vdev infra")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>
> ---
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
  
Thomas Monjalon March 6, 2024, 2:01 p.m. UTC | #7
01/09/2023 09:24, Mingjin Ye:
> When a device is created by a secondary process, an empty devargs is
> temporarily generated and bound to it. This causes the device to not
> be associated with the correct devargs, and the empty devargs are not
> released when the resource is freed.
> 
> This patch fixes the issue by matching the devargs when inserting a
> device in secondary process.
> 
> Fixes: dda987315ca2 ("vdev: make virtual bus use its device struct")
> Fixes: a16040453968 ("eal: extract vdev infra")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>
> ---
> +static struct rte_devargs *
> +vdev_devargs_lookup(const char *name)
> +{
> +	struct rte_devargs *devargs;
> +	char dev_name[32];
> +
> +	RTE_EAL_DEVARGS_FOREACH("vdev", devargs) {
> +		devargs->bus->parse(devargs->name, &dev_name);
> +		if (strcmp(dev_name, name) == 0) {
> +			VDEV_LOG(INFO, "**Devargs matched %s", dev_name);

The beginning of the log does not need such attention characters.
Removing while merging.

With Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

Applied, thanks.
  

Patch

diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c
index 7974b27295..fe39a98a9c 100644
--- a/drivers/bus/vdev/vdev.c
+++ b/drivers/bus/vdev/vdev.c
@@ -259,6 +259,22 @@  alloc_devargs(const char *name, const char *args)
 	return devargs;
 }
 
+static struct rte_devargs *
+vdev_devargs_lookup(const char *name)
+{
+	struct rte_devargs *devargs;
+	char dev_name[32];
+
+	RTE_EAL_DEVARGS_FOREACH("vdev", devargs) {
+		devargs->bus->parse(devargs->name, &dev_name);
+		if (strcmp(dev_name, name) == 0) {
+			VDEV_LOG(INFO, "**Devargs matched %s", dev_name);
+			return devargs;
+		}
+	}
+	return NULL;
+}
+
 static int
 insert_vdev(const char *name, const char *args,
 		struct rte_vdev_device **p_dev,
@@ -271,7 +287,11 @@  insert_vdev(const char *name, const char *args,
 	if (name == NULL)
 		return -EINVAL;
 
-	devargs = alloc_devargs(name, args);
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
+		devargs = alloc_devargs(name, args);
+	else
+		devargs = vdev_devargs_lookup(name);
+
 	if (!devargs)
 		return -ENOMEM;