From: Christian Ehrhardt <christian.ehrhardt@canonical.com>
To: Wei Huang <wei.huang@intel.com>
Cc: stable@dpdk.org, rosen.xu@intel.com, tianfei.zhang@intel.com
Subject: Re: [PATCH 19.11 1/2] raw/ifpga: remove virtual devices on close
Date: Mon, 11 Jul 2022 08:16:35 +0200 [thread overview]
Message-ID: <CAATJJ0LdRfnGQoaiJEPRkDd3BoraZzEtYLeZ7QP5jrj-3gaLgg@mail.gmail.com> (raw)
In-Reply-To: <1657510853-9096-1-git-send-email-wei.huang@intel.com>
On Mon, Jul 11, 2022 at 5:32 AM Wei Huang <wei.huang@intel.com> wrote:
>
> [ upstream commit ae835aba40349ee9631ef6b52e68a2893febe7e0 ]
Thanks, applied
> 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>
> Reviewed-by: Rosen Xu <rosen.xu@intel.com>
> ---
> drivers/raw/ifpga/base/ifpga_api.c | 12 +++
> drivers/raw/ifpga/base/ifpga_enumerate.c | 16 +++
> drivers/raw/ifpga/base/ifpga_enumerate.h | 1 +
> drivers/raw/ifpga/ifpga_rawdev.c | 178 +++++++++++++++++++++++++------
> drivers/raw/ifpga/ifpga_rawdev.h | 8 ++
> 5 files changed, 181 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/raw/ifpga/base/ifpga_api.c b/drivers/raw/ifpga/base/ifpga_api.c
> index 6dbd715..1ff57fa 100644
> --- a/drivers/raw/ifpga/base/ifpga_api.c
> +++ b/drivers/raw/ifpga/base/ifpga_api.c
> @@ -330,8 +330,20 @@ static int ifpga_adapter_enumerate(struct opae_adapter *adapter)
> return -ENOMEM;
> }
>
> +static void ifpga_adapter_destroy(struct opae_adapter *adapter)
> +{
> + struct ifpga_fme_hw *fme;
> +
> + if (adapter && adapter->mgr && adapter->mgr->data) {
> + fme = (struct ifpga_fme_hw *)adapter->mgr->data;
> + if (fme->parent)
> + ifpga_bus_uinit(fme->parent);
> + }
> +}
> +
> struct opae_adapter_ops ifpga_adapter_ops = {
> .enumerate = ifpga_adapter_enumerate,
> + .destroy = ifpga_adapter_destroy,
> };
>
> /**
> diff --git a/drivers/raw/ifpga/base/ifpga_enumerate.c b/drivers/raw/ifpga/base/ifpga_enumerate.c
> index b8846e3..48b8af4 100644
> --- a/drivers/raw/ifpga/base/ifpga_enumerate.c
> +++ b/drivers/raw/ifpga/base/ifpga_enumerate.c
> @@ -722,3 +722,19 @@ int ifpga_bus_init(struct ifpga_hw *hw)
>
> return 0;
> }
> +
> +int ifpga_bus_uinit(struct ifpga_hw *hw)
> +{
> + int i;
> + struct ifpga_port_hw *port;
> +
> + if (hw) {
> + fme_hw_uinit(&hw->fme);
> + for (i = 0; i < MAX_FPGA_PORT_NUM; i++) {
> + port = &hw->port[i];
> + port_hw_uinit(port);
> + }
> + }
> +
> + return 0;
> +}
> diff --git a/drivers/raw/ifpga/base/ifpga_enumerate.h b/drivers/raw/ifpga/base/ifpga_enumerate.h
> index 14131e3..95ed594 100644
> --- a/drivers/raw/ifpga/base/ifpga_enumerate.h
> +++ b/drivers/raw/ifpga/base/ifpga_enumerate.h
> @@ -6,6 +6,7 @@
> #define _IFPGA_ENUMERATE_H_
>
> int ifpga_bus_init(struct ifpga_hw *hw);
> +int ifpga_bus_uinit(struct ifpga_hw *hw);
> int ifpga_bus_enumerate(struct ifpga_hw *hw);
>
> #endif /* _IFPGA_ENUMERATE_H_ */
> diff --git a/drivers/raw/ifpga/ifpga_rawdev.c b/drivers/raw/ifpga/ifpga_rawdev.c
> index cc23a27..7f925c3 100644
> --- a/drivers/raw/ifpga/ifpga_rawdev.c
> +++ b/drivers/raw/ifpga/ifpga_rawdev.c
> @@ -142,6 +142,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;
> }
> @@ -738,6 +740,29 @@ 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_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);
> + opae_adapter_data_free(adapter->data);
> + }
> + }
> +
> return dev ? 0:1;
> }
>
> @@ -1615,8 +1640,6 @@ static int fme_clean_fme_error(struct opae_manager *mgr)
> }
> dev = ifpga_rawdev_get(rawdev);
>
> - ifpga_monitor_stop_func(dev);
> -
> adapter = ifpga_rawdev_get_priv(rawdev);
> if (!adapter)
> return -ENODEV;
> @@ -1629,9 +1652,6 @@ static int fme_clean_fme_error(struct opae_manager *mgr)
> fme_interrupt_handler, mgr) < 0)
> return -EINVAL;
>
> - opae_adapter_data_free(adapter->data);
> - opae_adapter_free(adapter);
> -
> /* rte_rawdev_close is called by pmd_release */
> ret = rte_rawdev_pmd_release(rawdev);
> if (ret)
> @@ -1699,64 +1719,117 @@ 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;
> - 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:
> + 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, "%d|%s",
> - port, 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, devargs->args);
> -end:
> - if (kvlist)
> - rte_kvargs_free(kvlist);
> - if (name)
> - 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;
> }
> @@ -1764,10 +1837,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 7be4133..8929bd7 100644
> --- a/drivers/raw/ifpga/ifpga_rawdev.h
> +++ b/drivers/raw/ifpga/ifpga_rawdev.h
> @@ -48,6 +48,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 {
> @@ -62,6 +63,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 *
> --
> 1.8.3.1
>
--
Christian Ehrhardt
Senior Staff Engineer, Ubuntu Server
Canonical Ltd
prev parent reply other threads:[~2022-07-11 6:17 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-11 3:40 Wei Huang
2022-07-11 3:40 ` [PATCH 19.11 2/2] raw/ifpga: unregister interrupt " Wei Huang
2022-07-11 6:16 ` Christian Ehrhardt
2022-07-11 6:16 ` Christian Ehrhardt [this message]
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=CAATJJ0LdRfnGQoaiJEPRkDd3BoraZzEtYLeZ7QP5jrj-3gaLgg@mail.gmail.com \
--to=christian.ehrhardt@canonical.com \
--cc=rosen.xu@intel.com \
--cc=stable@dpdk.org \
--cc=tianfei.zhang@intel.com \
--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).