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

["Ye, MingjinX" <mingjinx.ye@intel.com>]

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Thursday, June 20, 2024 4:15 AM
> To: Ye, MingjinX <mingjinx.ye@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org; stephen@networkplumber.org;
> Richardson, Bruce <bruce.richardson@intel.com>; Marchand, David
> <david.marchand@redhat.com>
> Subject: Re: [PATCH 2/3] bus/vdev: revert fix devargs after multi-process bus
> scan
> 
> 14/03/2024 10:36, 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
> 
> What is the previous commit?
commit: f5b2eff0847d49a66301f0046502c6232cd5da3f


> Where is devargs_list in this function?
In the rte_devargs_insert function.
> 
> > 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>
> 
> I'm not comfortable with reverting a so old commit.
> Your previous attempt in this bus driver was not successful.
> Please prove the memory leak cannot be simply fixed.

dpdk/drivers/bus/vdev/vdev.c:471
ret = insert_vdev(in->name, NULL, NULL, false);

Since the last argument is always fale, this results in the objects of alloc_devargs in insert_vdev not being added to the devargs_list via rte_devargs_insert.

rte_vdev_uninit->rte_devargs_remove will release the devargs objects constructed through the program startup parameters. 
however the devargs bound to the device (the devargs objects constructed in insert_vdev) are not released. causing a memory leak.

Therefore, after undoing the patch. insert_vdev->rte_devargs_insert can update the devargs object without causing a memory leak.
>