RE: [PATCH v3] raw/ifpga: remove vdev when ifpga is closed

["Huang, Wei" <wei.huang@intel.com>]

> -----Original Message-----
> From: Zhang, Qi Z <qi.z.zhang@intel.com>
> Sent: Tuesday, March 29, 2022 10:15
> To: Huang, Wei <wei.huang@intel.com>; dev@dpdk.org;
> thomas@monjalon.net; Xu, Rosen <rosen.xu@intel.com>;
> nipun.gupta@nxp.com; hemant.agrawal@nxp.com
> Cc: stable@dpdk.org; Zhang, Tianfei <tianfei.zhang@intel.com>; Yigit, Ferruh
> <ferruh.yigit@intel.com>
> Subject: RE: [PATCH v3] raw/ifpga: remove vdev when ifpga is closed
> 
> 
> > -----Original Message-----
> > From: Huang, Wei <wei.huang@intel.com>
> > Sent: Tuesday, March 29, 2022 9:56 AM
> > To: dev@dpdk.org; thomas@monjalon.net; Xu, Rosen
> <rosen.xu@intel.com>;
> > nipun.gupta@nxp.com; hemant.agrawal@nxp.com
> > Cc: stable@dpdk.org; Zhang, Tianfei <tianfei.zhang@intel.com>; Zhang,
> > Qi Z <qi.z.zhang@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>
> > Subject: RE: [PATCH v3] raw/ifpga: remove vdev when ifpga is closed
> >
> > Hi Thomas,
> >
> > Would you please check this patch is acceptable ?
> 
> Hi Wei:
> 22.07 cycle just begins, It may not necessary to push PMD merge quite early
> 😊
> Below is the roadmap for your reference
> 
> http://core.dpdk.org/roadmap/
> 
> Proposal deadline (RFC/v1 patches): 10 April 2022 API freeze (-rc1): 30 May
> 2022 PMD features freeze (-rc2): 20 June 2022 Builtin applications features
> freeze (-rc3): 27 June 2022
> Release: 13 July 2022
> 
> Regards
> Qi
> 
Hi Qi and Thomas,
This patch is not new feature, it's a fix, can it be merged now ? 
> >
> > > -----Original Message-----
> > > From: Huang, Wei <wei.huang@intel.com>
> > > Sent: Friday, March 18, 2022 16:58
> > > To: dev@dpdk.org; thomas@monjalon.net; Xu, Rosen
> > <rosen.xu@intel.com>;
> > > nipun.gupta@nxp.com; hemant.agrawal@nxp.com
> > > Cc: stable@dpdk.org; Zhang, Tianfei <tianfei.zhang@intel.com>;
> > > Zhang, Qi Z <qi.z.zhang@intel.com>; Yigit, Ferruh
> > > <ferruh.yigit@intel.com>; Huang, Wei <wei.huang@intel.com>
> > > Subject: [PATCH v3] 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.
> > >
> > > 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>
> > > ---
> > > v2: cleanup vdev_name no matter rte_vdev_uninit succeed or not
> > > ---
> > > v3: cleanup vdev_name in ifpga_cfg_remove
> > > ---
> > >  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;
> > >  };
> > >
> > >  struct ifpga_rawdev *
> > > --
> > > 1.8.3.1
> >
>