DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Zhang, Tianfei" <tianfei.zhang@intel.com>
To: "Huang, Wei" <wei.huang@intel.com>, "dev@dpdk.org" <dev@dpdk.org>,
	"thomas@monjalon.net" <thomas@monjalon.net>,
	"nipun.gupta@nxp.com" <nipun.gupta@nxp.com>,
	"hemant.agrawal@nxp.com" <hemant.agrawal@nxp.com>
Cc: "stable@dpdk.org" <stable@dpdk.org>,
	"Xu, Rosen" <rosen.xu@intel.com>,
	"Zhang, Qi Z" <qi.z.zhang@intel.com>
Subject: RE: [PATCH v4 2/5] raw/ifpga: remove vdev when ifpga is closed
Date: Mon, 6 Jun 2022 06:46:20 +0000	[thread overview]
Message-ID: <BN9PR11MB5483C3CE382242A5432316C6E3A29@BN9PR11MB5483.namprd11.prod.outlook.com> (raw)
In-Reply-To: <1653640433-5066-3-git-send-email-wei.huang@intel.com>



> -----Original Message-----
> From: Huang, Wei <wei.huang@intel.com>
> Sent: Friday, May 27, 2022 4:34 PM
> To: dev@dpdk.org; thomas@monjalon.net; nipun.gupta@nxp.com;
> hemant.agrawal@nxp.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 v4 2/5] raw/ifpga: remove vdev when ifpga is closed
> 
> Virtual devices created on ifpga raw device will not be removed when ifpga
> device has closed. To avoid resource leak problem, this patch introduces an
> ifpga virtual device remove function, virtual devices will be destroyed after the
> ifpga raw device closed.
> 
> Fixes: ef1e8ede3da5 ("raw/ifpga: add Intel FPGA bus rawdev driver")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Wei Huang <wei.huang@intel.com>
> ---
> v2: update commit log with Tianfei's comment
> ---
> v3: replace constant with PCI_PRI_STR_SIZE per Rosen's comment
> ---
>  drivers/raw/ifpga/ifpga_rawdev.c | 166 ++++++++++++++++++++++++++++++---
> ------
>  drivers/raw/ifpga/ifpga_rawdev.h |   8 ++
>  2 files changed, 138 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/raw/ifpga/ifpga_rawdev.c
> b/drivers/raw/ifpga/ifpga_rawdev.c
> index 6d4117c..fe3fc43 100644
> --- a/drivers/raw/ifpga/ifpga_rawdev.c
> +++ b/drivers/raw/ifpga/ifpga_rawdev.c
> @@ -134,6 +134,8 @@ 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;
>  }
> @@ -736,10 +738,22 @@ static int set_surprise_link_check_aer(  static int
> ifpga_rawdev_close(struct rte_rawdev *dev)  {
> +	struct ifpga_rawdev *ifpga_rdev = NULL;
>  	struct opae_adapter *adapter;
> +	char *vdev_name = NULL;
> +	int i = 0;
> 
>  	if (dev) {
> -		ifpga_monitor_stop_func(ifpga_rawdev_get(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;
> +		}
>  		adapter = ifpga_rawdev_get_priv(dev);
>  		if (adapter) {
>  			opae_adapter_destroy(adapter);
> @@ -1638,8 +1652,6 @@ static int fme_clean_fme_error(struct opae_manager
> *mgr)
>  		return -EINVAL;
>  	}
>  	dev = ifpga_rawdev_get(rawdev);
> -	if (dev)
> -		dev->rawdev = NULL;
> 
>  	adapter = ifpga_rawdev_get_priv(rawdev);
>  	if (!adapter)
> @@ -1714,73 +1726,118 @@ static int ifpga_rawdev_get_string_arg(const char
> *key __rte_unused,
> 
>  	return 0;
>  }
> +
>  static int
> -ifpga_cfg_probe(struct rte_vdev_device *dev)
> +ifpga_vdev_parse_devargs(struct rte_devargs *devargs,
> +	struct ifpga_vdev_args *args)
>  {
> -	struct rte_devargs *devargs;
> -	struct rte_kvargs *kvlist = NULL;
> -	struct rte_rawdev *rawdev = NULL;
> -	struct ifpga_rawdev *ifpga_dev;
> -	int port;
> +	struct rte_kvargs *kvlist;
>  	char *name = NULL;
> -	char dev_name[RTE_RAWDEV_NAME_MAX_LEN];
> -	int ret = -1;
> +	int port = 0;
> +	int ret = -EINVAL;
> 
> -	devargs = dev->device.devargs;
> +	if (!devargs || !args)
> +		return ret;
> 
>  	kvlist = rte_kvargs_parse(devargs->args, valid_args);
>  	if (!kvlist) {
> -		IFPGA_RAWDEV_PMD_LOG(ERR, "error when parsing param");
> -		goto end;
> +		IFPGA_RAWDEV_PMD_ERR("error when parsing devargs");
> +		return ret;
>  	}
> 
>  	if (rte_kvargs_count(kvlist, IFPGA_ARG_NAME) == 1) {
>  		if (rte_kvargs_process(kvlist, IFPGA_ARG_NAME,
> -				       &ifpga_rawdev_get_string_arg,
> -				       &name) < 0) {
> +			&ifpga_rawdev_get_string_arg, &name) < 0) {
>  			IFPGA_RAWDEV_PMD_ERR("error to parse %s",
> -				     IFPGA_ARG_NAME);
> +				IFPGA_ARG_NAME);
>  			goto end;
> +		} else {
> +			strlcpy(args->bdf, name, sizeof(args->bdf));
> +			rte_free(name);
>  		}
>  	} else {
>  		IFPGA_RAWDEV_PMD_ERR("arg %s is mandatory for ifpga bus",
> -			  IFPGA_ARG_NAME);
> +			IFPGA_ARG_NAME);
>  		goto end;
>  	}
> 
>  	if (rte_kvargs_count(kvlist, IFPGA_ARG_PORT) == 1) {
> -		if (rte_kvargs_process(kvlist,
> -			IFPGA_ARG_PORT,
> -			&rte_ifpga_get_integer32_arg,
> -			&port) < 0) {
> +		if (rte_kvargs_process(kvlist, IFPGA_ARG_PORT,
> +			&rte_ifpga_get_integer32_arg, &port) < 0) {
>  			IFPGA_RAWDEV_PMD_ERR("error to parse %s",
>  				IFPGA_ARG_PORT);
>  			goto end;
> +		} else {
> +			args->port = port;
>  		}
>  	} else {
>  		IFPGA_RAWDEV_PMD_ERR("arg %s is mandatory for ifpga bus",
> -			  IFPGA_ARG_PORT);
> +			IFPGA_ARG_PORT);
>  		goto end;
>  	}
> 
> +	ret = 0;
> +
> +end:
> +	if (kvlist)
> +		rte_kvargs_free(kvlist);
> +
> +	return ret;
> +}
> +
> +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;
> +
> +	IFPGA_RAWDEV_PMD_INFO("probe 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",
> name);
> +	snprintf(dev_name, RTE_RAWDEV_NAME_MAX_LEN, "IFPGA:%s",
> args.bdf);
>  	rawdev = rte_rawdev_pmd_get_named_dev(dev_name);
>  	if (!rawdev)
> -		goto end;
> +		return -ENODEV;
>  	ifpga_dev = ifpga_rawdev_get(rawdev);
>  	if (!ifpga_dev)
> -		goto end;
> +		return -ENODEV;
> 
> -	memset(dev_name, 0, sizeof(dev_name));
> -	snprintf(dev_name, RTE_RAWDEV_NAME_MAX_LEN, "%d|%s",
> -	port, name);
> +	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, devargs->args);
> -end:
> -	rte_kvargs_free(kvlist);
> -	free(name);
> +			dev_name, vdev->device.devargs->args);
> +	if (ret) {
> +		rte_free(ifpga_dev->vdev_name[i]);
> +		ifpga_dev->vdev_name[i] = NULL;
> +	}
> 
>  	return ret;
>  }
> @@ -1788,10 +1845,47 @@ static int ifpga_rawdev_get_string_arg(const char
> *key __rte_unused,  static int  ifpga_cfg_remove(struct rte_vdev_device *vdev)
> {
> -	IFPGA_RAWDEV_PMD_INFO("Remove ifpga_cfg %p",
> -		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;
> 
> -	return 0;
> +	vdev_name = rte_vdev_device_name(vdev);
> +	if (!vdev_name)
> +		return -EINVAL;
> +
> +	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;
>  }
> 
>  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 857b734..4c19119 100644
> --- a/drivers/raw/ifpga/ifpga_rawdev.h
> +++ b/drivers/raw/ifpga/ifpga_rawdev.h
> @@ -50,6 +50,7 @@ enum ifpga_rawdev_device_state {
> 
>  #define IFPGA_RAWDEV_MSIX_IRQ_NUM 7
>  #define IFPGA_RAWDEV_NUM 32
> +#define IFPGA_MAX_VDEV 4
>  #define IFPGA_MAX_IRQ 12
> 
>  struct ifpga_rawdev {
> @@ -64,6 +65,13 @@ 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 {
> +	char bdf[PCI_PRI_STR_SIZE];
> +	int port;
>  };
> 
>  struct ifpga_rawdev *
> --

It looks good for me, you can add:
Acked-by: Tianfei Zhang <tianfei.zhang@intel.com>

  reply	other threads:[~2022-06-06  6:46 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-17  6:28 [PATCH v1 0/4] Support OFS card Wei Huang
2022-05-17  6:28 ` [PATCH v1 1/4] raw/ifpga: remove experimental tag from ifpga APIs Wei Huang
2022-05-17  6:28 ` [PATCH v1 2/4] raw/ifpga: remove vdev when ifpga is closed Wei Huang
2022-05-17  6:28 ` [PATCH v1 3/4] raw/ifpga: unregister interrupt in ifpga close function Wei Huang
2022-05-17  6:28 ` [PATCH v1 4/4] raw/ifpga: support ofs card probe Wei Huang
2022-05-18  8:29   ` [PATCH v2 0/4] Support OFS card Wei Huang
2022-05-18  8:29     ` [PATCH v2 1/4] raw/ifpga: remove experimental tag from ifpga APIs Wei Huang
2022-05-25  3:22       ` Zhang, Tianfei
2022-05-18  8:29     ` [PATCH v2 2/4] raw/ifpga: remove vdev when ifpga is closed Wei Huang
2022-05-25  4:09       ` Zhang, Tianfei
2022-05-18  8:29     ` [PATCH v2 3/4] raw/ifpga: unregister interrupt in ifpga close function Wei Huang
2022-05-25  3:26       ` Zhang, Tianfei
2022-05-18  8:29     ` [PATCH v2 4/4] raw/ifpga: support ofs card probe Wei Huang
2022-05-26  3:32     ` [PATCH v3 0/5] Support OFS card Wei Huang
2022-05-26  3:32       ` [PATCH v3 1/5] raw/ifpga: remove experimental tag from ifpga APIs Wei Huang
2022-05-26  6:29         ` Xu, Rosen
2022-05-26  3:32       ` [PATCH v3 2/5] raw/ifpga: remove vdev when ifpga is closed Wei Huang
2022-05-26  6:34         ` Xu, Rosen
2022-05-26  3:32       ` [PATCH v3 3/5] raw/ifpga: unregister interrupt in ifpga close function Wei Huang
2022-05-26  6:41         ` Xu, Rosen
2022-05-27  2:57         ` Zhang, Tianfei
2022-05-26  3:32       ` [PATCH v3 4/5] raw/ifpga: support ofs card probe Wei Huang
2022-05-26  6:46         ` Xu, Rosen
2022-05-27  3:10         ` Zhang, Tianfei
2022-05-26  3:32       ` [PATCH v3 5/5] guides/rawdevs: add description of ofs in ifpga doc Wei Huang
2022-05-26  6:47         ` Xu, Rosen
2022-05-27  3:19         ` Zhang, Tianfei
2022-05-27  8:33       ` [PATCH v4 0/5] Support OFS card Wei Huang
2022-05-27  8:33         ` [PATCH v4 1/5] raw/ifpga: remove experimental tag from ifpga APIs Wei Huang
2022-05-27  8:33         ` [PATCH v4 2/5] raw/ifpga: remove vdev when ifpga is closed Wei Huang
2022-06-06  6:46           ` Zhang, Tianfei [this message]
2022-06-07  6:02           ` Xu, Rosen
2022-05-27  8:33         ` [PATCH v4 3/5] raw/ifpga: unregister interrupt in ifpga close function Wei Huang
2022-06-07  6:03           ` Xu, Rosen
2022-05-27  8:33         ` [PATCH v4 4/5] raw/ifpga: support ofs card probe Wei Huang
2022-06-07  6:04           ` Xu, Rosen
2022-05-27  8:33         ` [PATCH v4 5/5] guides/rawdevs: add description of ofs in ifpga doc Wei Huang
2022-05-31  0:24           ` Zhang, Tianfei
2022-06-06  6:45           ` Zhang, Tianfei
2022-06-07  9:07         ` [PATCH v5 0/5] Support OFS card Wei Huang
2022-06-07  9:07           ` [PATCH v5 1/5] raw/ifpga: remove experimental tag from ifpga APIs Wei Huang
2022-06-07  9:07           ` [PATCH v5 2/5] raw/ifpga: remove vdev when ifpga is closed Wei Huang
2022-06-07  9:07           ` [PATCH v5 3/5] raw/ifpga: unregister interrupt in ifpga close function Wei Huang
2022-06-07  9:07           ` [PATCH v5 4/5] raw/ifpga: support ofs card probe Wei Huang
2022-06-07  9:07           ` [PATCH v5 5/5] guides/rawdevs: add description of ofs in ifpga doc Wei Huang
2022-06-07 13:53           ` [PATCH v5 0/5] Support OFS card Thomas Monjalon
2022-05-25  3:18   ` [PATCH v1 4/4] raw/ifpga: support ofs card probe Zhang, Tianfei

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=BN9PR11MB5483C3CE382242A5432316C6E3A29@BN9PR11MB5483.namprd11.prod.outlook.com \
    --to=tianfei.zhang@intel.com \
    --cc=dev@dpdk.org \
    --cc=hemant.agrawal@nxp.com \
    --cc=nipun.gupta@nxp.com \
    --cc=qi.z.zhang@intel.com \
    --cc=rosen.xu@intel.com \
    --cc=stable@dpdk.org \
    --cc=thomas@monjalon.net \
    --cc=wei.huang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).