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 v2 2/4] raw/ifpga: remove vdev when ifpga is closed
Date: Wed, 25 May 2022 04:09:33 +0000	[thread overview]
Message-ID: <BN9PR11MB54830E0BC94995AA9E01EAA4E3D69@BN9PR11MB5483.namprd11.prod.outlook.com> (raw)
In-Reply-To: <1652862549-13131-3-git-send-email-wei.huang@intel.com>



> -----Original Message-----
> From: Huang, Wei <wei.huang@intel.com>
> Sent: Wednesday, May 18, 2022 4:29 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 v2 2/4] raw/ifpga: remove vdev when ifpga is closed
> 
> Virtual devices created on ifpga raw device are not removed when ifpga is
> closed. To avoid such problem, ifpga virtual device remove function is
> implemented, virtual device is removed in raw device close function.

The git message  can be changed as below:

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>
> Acked-by: Tianfei Zhang <tianfei.zhang@intel.com>
> Acked-by: Rosen Xu <rosen.xu@intel.com>
> ---
>  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..eb9a9a5 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[8];
> +	int port;
>  };

can we use a common macro definition for this immediate value 8?

> 
>  struct ifpga_rawdev *
> --
> 1.8.3.1


  reply	other threads:[~2022-05-25  4:09 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 [this message]
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
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=BN9PR11MB54830E0BC94995AA9E01EAA4E3D69@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).