RE: [PATCH v1] raw/ifpga: remove virtual device unplug operation

["Zhang, Qi Z" <qi.z.zhang@intel.com>]

> -----Original Message-----
> From: Xu, Rosen <rosen.xu@intel.com>
> Sent: Monday, March 20, 2023 2:52 PM
> To: Huang, Wei <wei.huang@intel.com>; dev@dpdk.org;
> thomas@monjalon.net; david.marchand@redhat.com
> Cc: stable@dpdk.org; Zhang, Tianfei <tianfei.zhang@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>
> Subject: RE: [PATCH v1] raw/ifpga: remove virtual device unplug operation
> 
> Hi,
> 
> > -----Original Message-----
> > From: Huang, Wei <wei.huang@intel.com>
> > Sent: Friday, March 17, 2023 4:45 AM
> > To: dev@dpdk.org; thomas@monjalon.net; david.marchand@redhat.com
> > Cc: stable@dpdk.org; Xu, Rosen <rosen.xu@intel.com>; Zhang, Tianfei
> > <tianfei.zhang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Huang,
> > Wei <wei.huang@intel.com>
> > Subject: [PATCH v1] raw/ifpga: remove virtual device unplug operation
> >
> > VDEV bus has implemented cleanup() function to perform cleanup for
> > devices on the bus during eal_cleanup(), so there is no need for ifpga
> > driver to record virtual devices and unplug them.
> >
> > Signed-off-by: Wei Huang <wei.huang@intel.com>
> > ---
> >  drivers/raw/ifpga/ifpga_rawdev.c | 99
> > ++++------------------------------------
> >  drivers/raw/ifpga/ifpga_rawdev.h |  2 -
> >  2 files changed, 9 insertions(+), 92 deletions(-)
> >
> > diff --git a/drivers/raw/ifpga/ifpga_rawdev.c
> > b/drivers/raw/ifpga/ifpga_rawdev.c
> > index 1020adc..8e6e70f 100644
> > --- a/drivers/raw/ifpga/ifpga_rawdev.c
> > +++ b/drivers/raw/ifpga/ifpga_rawdev.c
> > @@ -140,8 +140,6 @@ struct ifpga_rawdev *
> >  	for (i = 0; i < IFPGA_MAX_IRQ; i++)
> >  		dev->intr_handle[i] = NULL;
> >  	dev->poll_enabled = 0;
> > -	for (i = 0; i < IFPGA_MAX_VDEV; i++)
> > -		dev->vdev_name[i] = NULL;
> >
> >  	return dev;
> >  }
> > @@ -749,17 +747,11 @@ static int set_surprise_link_check_aer(
> >  	struct ifpga_rawdev *ifpga_rdev = NULL;
> >  	struct opae_adapter *adapter;
> >  	struct opae_manager *mgr;
> > -	char *vdev_name = NULL;
> > -	int i, ret = 0;
> > +	int ret = 0;
> >
> >  	if (dev) {
> >  		ifpga_rdev = ifpga_rawdev_get(dev);
> >  		if (ifpga_rdev) {
> > -			for (i = 0; i < IFPGA_MAX_VDEV; i++) {
> > -				vdev_name = ifpga_rdev->vdev_name[i];
> > -				if (vdev_name)
> > -					rte_vdev_uninit(vdev_name);
> > -			}
> >  			ifpga_monitor_stop_func(ifpga_rdev);
> >  			ifpga_rdev->rawdev = NULL;
> >  		}
> > @@ -1778,104 +1770,31 @@ static int ifpga_rawdev_get_string_arg(const
> > char *key __rte_unused,  static int  ifpga_cfg_probe(struct
> > rte_vdev_device
> > *vdev)  {
> > -	struct rte_rawdev *rawdev = NULL;
> > -	struct ifpga_rawdev *ifpga_dev;
> >  	struct ifpga_vdev_args args;
> >  	char dev_name[RTE_RAWDEV_NAME_MAX_LEN];
> > -	const char *vdev_name = NULL;
> > -	int i, n, ret = 0;
> > -
> > -	vdev_name = rte_vdev_device_name(vdev);
> > -	if (!vdev_name)
> > -		return -EINVAL;
> > +	int ret = 0;
> >
> > -	IFPGA_RAWDEV_PMD_INFO("probe ifpga virtual device %s",
> > vdev_name);
> > +	IFPGA_RAWDEV_PMD_INFO("probe ifpga virtual device %s",
> > +		rte_vdev_device_name(vdev));
> >
> >  	ret = ifpga_vdev_parse_devargs(vdev->device.devargs, &args);
> >  	if (ret)
> >  		return ret;
> >
> >  	memset(dev_name, 0, sizeof(dev_name));
> > -	snprintf(dev_name, RTE_RAWDEV_NAME_MAX_LEN, "IFPGA:%s",
> > args.bdf);
> > -	rawdev = rte_rawdev_pmd_get_named_dev(dev_name);
> > -	if (!rawdev)
> > -		return -ENODEV;
> > -	ifpga_dev = ifpga_rawdev_get(rawdev);
> > -	if (!ifpga_dev)
> > -		return -ENODEV;
> > -
> > -	for (i = 0; i < IFPGA_MAX_VDEV; i++) {
> > -		if (ifpga_dev->vdev_name[i] == NULL) {
> > -			n = strlen(vdev_name) + 1;
> > -			ifpga_dev->vdev_name[i] = rte_malloc(NULL, n, 0);
> > -			if (ifpga_dev->vdev_name[i] == NULL)
> > -				return -ENOMEM;
> > -			strlcpy(ifpga_dev->vdev_name[i], vdev_name, n);
> > -			break;
> > -		}
> > -	}
> > -
> > -	if (i >= IFPGA_MAX_VDEV) {
> > -		IFPGA_RAWDEV_PMD_ERR("Can't create more virtual
> > device!");
> > -		return -ENOENT;
> > -	}
> > -
> >  	snprintf(dev_name, RTE_RAWDEV_NAME_MAX_LEN, "%d|%s",
> >  		args.port, args.bdf);
> > -	ret = rte_eal_hotplug_add(RTE_STR(IFPGA_BUS_NAME),
> > -			dev_name, vdev->device.devargs->args);
> > -	if (ret) {
> > -		rte_free(ifpga_dev->vdev_name[i]);
> > -		ifpga_dev->vdev_name[i] = NULL;
> > -	}
> > -
> > -	return ret;
> > +	return rte_eal_hotplug_add(RTE_STR(IFPGA_BUS_NAME),
> > dev_name,
> > +			vdev->device.devargs->args);
> >  }
> >
> >  static int
> >  ifpga_cfg_remove(struct rte_vdev_device *vdev)  {
> > -	struct rte_rawdev *rawdev = NULL;
> > -	struct ifpga_rawdev *ifpga_dev;
> > -	struct ifpga_vdev_args args;
> > -	char dev_name[RTE_RAWDEV_NAME_MAX_LEN];
> > -	const char *vdev_name = NULL;
> > -	char *tmp_vdev = NULL;
> > -	int i, ret = 0;
> > -
> > -	vdev_name = rte_vdev_device_name(vdev);
> > -	if (!vdev_name)
> > -		return -EINVAL;
> > +	IFPGA_RAWDEV_PMD_INFO("remove ifpga virtual device %s",
> > +		rte_vdev_device_name(vdev));
> >
> > -	IFPGA_RAWDEV_PMD_INFO("remove ifpga virtual device %s",
> > vdev_name);
> > -
> > -	ret = ifpga_vdev_parse_devargs(vdev->device.devargs, &args);
> > -	if (ret)
> > -		return ret;
> > -
> > -	memset(dev_name, 0, sizeof(dev_name));
> > -	snprintf(dev_name, RTE_RAWDEV_NAME_MAX_LEN, "IFPGA:%s",
> > args.bdf);
> > -	rawdev = rte_rawdev_pmd_get_named_dev(dev_name);
> > -	if (!rawdev)
> > -		return -ENODEV;
> > -	ifpga_dev = ifpga_rawdev_get(rawdev);
> > -	if (!ifpga_dev)
> > -		return -ENODEV;
> > -
> > -	snprintf(dev_name, RTE_RAWDEV_NAME_MAX_LEN, "%d|%s",
> > -		args.port, args.bdf);
> > -	ret = rte_eal_hotplug_remove(RTE_STR(IFPGA_BUS_NAME),
> > dev_name);
> > -
> > -	for (i = 0; i < IFPGA_MAX_VDEV; i++) {
> > -		tmp_vdev = ifpga_dev->vdev_name[i];
> > -		if (tmp_vdev && !strcmp(tmp_vdev, vdev_name)) {
> > -			free(tmp_vdev);
> > -			ifpga_dev->vdev_name[i] = NULL;
> > -			break;
> > -		}
> > -	}
> > -
> > -	return ret;
> > +	return 0;
> >  }
> >
> >  static struct rte_vdev_driver ifpga_cfg_driver = { diff --git
> > a/drivers/raw/ifpga/ifpga_rawdev.h b/drivers/raw/ifpga/ifpga_rawdev.h
> > index 0fb66cb..1c128c7 100644
> > --- a/drivers/raw/ifpga/ifpga_rawdev.h
> > +++ b/drivers/raw/ifpga/ifpga_rawdev.h
> > @@ -65,8 +65,6 @@ struct ifpga_rawdev {
> >  	void *intr_handle[IFPGA_MAX_IRQ];
> >  	/* enable monitor thread poll device's sensors or not */
> >  	int poll_enabled;
> > -	/* name of virtual devices created on raw device */
> > -	char *vdev_name[IFPGA_MAX_VDEV];
> >  };
> >
> >  struct ifpga_vdev_args {
> > --
> > 1.8.3.1
> 
> Acked-by: Rosen Xu <rosen.xu@intel.com>

Applied to dpdk-next-net-intel.

Thanks
Qi