[PATCH 0/3] fix insert dev core dump

[PATCH 0/3] fix insert dev core dump

[Mingjin Ye]

revert 2 patches and fix insert vdev core dump.

Mingjin Ye (3):
  bus/vdev: revert fix devargs in secondary process
  bus/vdev: revert fix devargs after multi-process bus scan
  net/vdev: fix insert vdev core dump

 drivers/bus/vdev/vdev.c | 34 ++++++----------------------------
 1 file changed, 6 insertions(+), 28 deletions(-)

-- 
2.25.1

[PATCH 1/3] bus/vdev: revert fix devargs in secondary process

[Mingjin Ye]

The asan tool detected a memory leak in the vdev driver
alloc_devargs. The previous commit was that when inserting
a vdev device, the primary process alloc devargs and the
secondary process looks for devargs. This causes the
device to not be created if the secondary process does
not initialise the vdev device. And, this is not the
root cause.

Therefore the following commit was reverted accordingly.

Fixes: 6666628362c9 ("bus/vdev: fix devargs in secondary process")

After restoring this commit, the memory leak still exists.
A new patch has since fixed this issue.

Fixes: 6666628362c9 ("bus/vdev: fix devargs in secondary process")
Cc: stable@dpdk.org

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

diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c
index 14cf856237..38d05a9fe9 100644
--- a/drivers/bus/vdev/vdev.c
+++ b/drivers/bus/vdev/vdev.c
@@ -263,22 +263,6 @@ 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,
@@ -291,10 +275,7 @@ insert_vdev(const char *name, const char *args,
 	if (name == NULL)
 		return -EINVAL;
 
-	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
-		devargs = alloc_devargs(name, args);
-	else
-		devargs = vdev_devargs_lookup(name);
+	devargs = alloc_devargs(name, args);
 
 	if (!devargs)
 		return -ENOMEM;
-- 
2.25.1

[PATCH 2/3] bus/vdev: revert fix devargs after multi-process bus scan

[Mingjin Ye]

The asan tool detected a memory leak in the vdev driver alloc_devargs.
The previous commit does not insert device arguments into devargs_list
when attaching a device during a bus scan of a secondary process.
This resulted in an existing memory leak when removing a vdev device,
since rte_devargs_remove actually does nothing.

Therefore the following commit was reverted accordingly.

Fixes: f5b2eff0847d ("bus/vdev: fix devargs after multi-process bus scan")

Restoring this commit will fix the memory leak. There was an issue with
device parameters using free devargs when inserting a vdev device when
devargs_list already existed, resulting in a core dump. A new patch
will fix this issue.

Fixes: f5b2eff0847d ("bus/vdev: fix devargs after multi-process bus scan")
Cc: stable@dpdk.org

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

diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c
index 38d05a9fe9..1a6cc7d12d 100644
--- a/drivers/bus/vdev/vdev.c
+++ b/drivers/bus/vdev/vdev.c
@@ -264,9 +264,7 @@ alloc_devargs(const char *name, const char *args)
 }
 
 static int
-insert_vdev(const char *name, const char *args,
-		struct rte_vdev_device **p_dev,
-		bool init)
+insert_vdev(const char *name, const char *args, struct rte_vdev_device **p_dev)
 {
 	struct rte_vdev_device *dev;
 	struct rte_devargs *devargs;
@@ -300,8 +298,7 @@ insert_vdev(const char *name, const char *args,
 		goto fail;
 	}
 
-	if (init)
-		rte_devargs_insert(&devargs);
+	rte_devargs_insert(&devargs);
 	dev->device.devargs = devargs;
 	TAILQ_INSERT_TAIL(&vdev_device_list, dev, next);
 
@@ -323,7 +320,7 @@ rte_vdev_init(const char *name, const char *args)
 	int ret;
 
 	rte_spinlock_recursive_lock(&vdev_device_list_lock);
-	ret = insert_vdev(name, args, &dev, true);
+	ret = insert_vdev(name, args, &dev);
 	if (ret == 0) {
 		ret = vdev_probe_all_drivers(dev);
 		if (ret) {
@@ -449,7 +446,7 @@ vdev_action(const struct rte_mp_msg *mp_msg, const void *peer)
 		break;
 	case VDEV_SCAN_ONE:
 		VDEV_LOG(INFO, "receive vdev, %s", in->name);
-		ret = insert_vdev(in->name, NULL, NULL, false);
+		ret = insert_vdev(in->name, NULL, NULL);
 		if (ret == -EEXIST)
 			VDEV_LOG(DEBUG, "device already exist, %s", in->name);
 		else if (ret < 0)
-- 
2.25.1

[PATCH 3/3] net/vdev: fix insert vdev core dump

[Mingjin Ye]

Inserting a vdev device when the device arguments are already stored
in devargs_list, the rte_devargs_insert function replaces the supplied
new devargs with the found devargs and frees the new devargs. As a
result, the use of free devargs results in a core dump.

This patch fixes the issue by using valid devargs.

Fixes: f3a1188cee4a ("devargs: make device representation generic")
Cc: stable@dpdk.org

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

diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c
index 1a6cc7d12d..53fc4a6e21 100644
--- a/drivers/bus/vdev/vdev.c
+++ b/drivers/bus/vdev/vdev.c
@@ -286,7 +286,6 @@ insert_vdev(const char *name, const char *args, struct rte_vdev_device **p_dev)
 
 	dev->device.bus = &rte_vdev_bus;
 	dev->device.numa_node = SOCKET_ID_ANY;
-	dev->device.name = devargs->name;
 
 	if (find_vdev(name)) {
 		/*
@@ -300,6 +299,7 @@ insert_vdev(const char *name, const char *args, struct rte_vdev_device **p_dev)
 
 	rte_devargs_insert(&devargs);
 	dev->device.devargs = devargs;
+	dev->device.name = devargs->name;
 	TAILQ_INSERT_TAIL(&vdev_device_list, dev, next);
 
 	if (p_dev)
-- 
2.25.1

RE: [PATCH 3/3] net/vdev: fix insert vdev core dump

[Jiang, YuX]

> -----Original Message-----
> From: Mingjin Ye <mingjinx.ye@intel.com>
> Sent: Thursday, March 14, 2024 5:37 PM
> To: dev@dpdk.org
> Cc: Ye, MingjinX <mingjinx.ye@intel.com>; stable@dpdk.org
> Subject: [PATCH 3/3] net/vdev: fix insert vdev core dump
> 
> Inserting a vdev device when the device arguments are already stored in
> devargs_list, the rte_devargs_insert function replaces the supplied new
> devargs with the found devargs and frees the new devargs. As a result, the use
> of free devargs results in a core dump.
> 
> This patch fixes the issue by using valid devargs.
> 
> Fixes: f3a1188cee4a ("devargs: make device representation generic")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>
> ---
>  drivers/bus/vdev/vdev.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

Tested-by:  Yu Jiang <yux.jiang@intel.com>

Best regards,
Yu Jiang