Re: [dpdk-dev] [PATCH] bus/vdev: fix device argument corrupt after bus scan

["Gaëtan Rivet" <gaetan.rivet@6wind.com>]

On Thu, Oct 25, 2018 at 11:30:36AM +0800, Qi Zhang wrote:
> It's not necessary to insert device argment to devargs_list
> during bus scan, but this happens when we try to attach a
> device on secondary process. The patch fix the issue.
> 
> Fixes: cdb068f031c6 ("bus/vdev: scan by multi-process channel")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> ---
>  drivers/bus/vdev/vdev.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c
> index 688e31c21..818a2bfc2 100644
> --- a/drivers/bus/vdev/vdev.c
> +++ b/drivers/bus/vdev/vdev.c
> @@ -202,7 +202,9 @@ alloc_devargs(const char *name, const char *args)
>  }
>  
>  static int
> -insert_vdev(const char *name, const char *args, struct rte_vdev_device **p_dev)
> +insert_vdev(const char *name, const char *args,
> +		struct rte_vdev_device **p_dev,
> +		bool init)

Why is vdev the only bus not using hotplug API itself and reimplementing
it on its own?

It should not have to insert devargs at all, not even in the primary
process. If it called rte_dev_probe(), this would normally already be
properly handled I think.

>  {
>  	struct rte_vdev_device *dev;
>  	struct rte_devargs *devargs;
> @@ -237,7 +239,8 @@ insert_vdev(const char *name, const char *args, struct rte_vdev_device **p_dev)
>  	}
>  
>  	TAILQ_INSERT_TAIL(&vdev_device_list, dev, next);
> -	rte_devargs_insert(devargs);
> +	if (init)
> +		rte_devargs_insert(devargs);
>  
>  	if (p_dev)
>  		*p_dev = dev;
> @@ -257,7 +260,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);
> +	ret = insert_vdev(name, args, &dev, true);
>  	if (ret == 0) {
>  		ret = vdev_probe_all_drivers(dev);
>  		if (ret) {
> @@ -383,7 +386,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);
> +		ret = insert_vdev(in->name, NULL, NULL, false);
>  		if (ret == -EEXIST)
>  			VDEV_LOG(DEBUG, "device already exist, %s", in->name);
>  		else if (ret < 0)
> -- 
> 2.13.6
> 

-- 
Gaëtan Rivet
6WIND