From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 68C83A0032 for ; Mon, 11 Jul 2022 08:17:03 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 60F0D4021E; Mon, 11 Jul 2022 08:17:03 +0200 (CEST) Received: from smtp-relay-internal-1.canonical.com (smtp-relay-internal-1.canonical.com [185.125.188.123]) by mails.dpdk.org (Postfix) with ESMTP id 1924E4021E for ; Mon, 11 Jul 2022 08:17:02 +0200 (CEST) Received: from mail-lj1-f200.google.com (mail-lj1-f200.google.com [209.85.208.200]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by smtp-relay-internal-1.canonical.com (Postfix) with ESMTPS id DE4563F1E0 for ; Mon, 11 Jul 2022 06:17:01 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=canonical.com; s=20210705; t=1657520221; bh=GnC5W1N83dxHjL3JSbTi/QjRqzKrNbypndiaCw4IuZc=; h=MIME-Version:References:In-Reply-To:From:Date:Message-ID:Subject: To:Cc:Content-Type; b=sCtI7sLI8MjHAwgcdBMG8RHc67NUqcLqS1cLfQ2YyNglAi9l4lPdx2XcV8mAVaT7y xgYhIEzULHyegqXlElHpzTA79vlodis6FEn9W08InumWTmc2Efpj+XG5G+0jUYeY4m 2uZVCKLsUViZUj/45dSXgfH76yCPKm5mgjh88yOIclFukCLeHV8R9gt1AN0T8FsI5+ 1vY3j4pOPRLDiK0rjrFoLkRziCdCY8TxRBb/L+OEsD3a6A1pPd9f5RBMtymC3G4/lq x5hoHb/Nyf83RYkUAymc3Wa8Ir3L7znrWGMvwsAGcG4sUDkL2qkBDGIvp6EyL1m4Li OvKy4cowUbniQ== Received: by mail-lj1-f200.google.com with SMTP id g3-20020a2e9cc3000000b00253cc2b5ab5so641289ljj.19 for ; Sun, 10 Jul 2022 23:17:01 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=GnC5W1N83dxHjL3JSbTi/QjRqzKrNbypndiaCw4IuZc=; b=EIhaHu0g0NcXQcDeDdbQo6DHI3uYQpqkITOzv+iZTPfQebTDknnix/s6TLfgC70EQ5 CCHHZqsY3LXqS4PD5Br+UMUbfi1m6w7YjrspiNXhK1rN1egMJToPkjzBPzanTrx0A24w W+QgmXaOBrLwq4hjsWpzAgr+djxMx0sTvxh3HTVCi3qO8LY3NUxHO+kdTcue72NnlvIJ uTfYD1PGkN5hbaoqSb8q4IT8zaccOT7jS/dps4Fj5mu0t8puzecopH3TDX0VVlr/CbEp 3AdRrA7d5Q0fJcDmDBvm4jc3DEnJGhGObNOzCs+USITumoVJLl8m/qluu6Goww8W2ASe BCFg== X-Gm-Message-State: AJIora+h3LL25o+a94tB9r+8j+/Gwmx4Gk3gZCb4va/dvJ35Y9Hs0MCP HXWLrXNg0/Sp/PEEf5woufNE/Uxk96fTTco6vRSpy+XXxlpvXPF/qiAHnz9E0uCyTuIgQaF5eYu SSeuIzl2LPAlcrD0uf4VmMofRtPXQTn6R4pZRik8s X-Received: by 2002:a05:6512:220c:b0:483:d9ad:b56c with SMTP id h12-20020a056512220c00b00483d9adb56cmr10442409lfu.150.1657520221261; Sun, 10 Jul 2022 23:17:01 -0700 (PDT) X-Google-Smtp-Source: AGRyM1v0BPjJ1ykSz/zeDriaNSLm9wv/HcKJT0LLVQS/Cl/9HCu/45P3PG3F5E8Aqij1UMvmLE0CmYCJz10pGCdwM5E= X-Received: by 2002:a05:6512:220c:b0:483:d9ad:b56c with SMTP id h12-20020a056512220c00b00483d9adb56cmr10442394lfu.150.1657520220967; Sun, 10 Jul 2022 23:17:00 -0700 (PDT) MIME-Version: 1.0 References: <1657510853-9096-1-git-send-email-wei.huang@intel.com> In-Reply-To: <1657510853-9096-1-git-send-email-wei.huang@intel.com> From: Christian Ehrhardt Date: Mon, 11 Jul 2022 08:16:35 +0200 Message-ID: Subject: Re: [PATCH 19.11 1/2] raw/ifpga: remove virtual devices on close To: Wei Huang Cc: stable@dpdk.org, rosen.xu@intel.com, tianfei.zhang@intel.com Content-Type: text/plain; charset="UTF-8" X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org On Mon, Jul 11, 2022 at 5:32 AM Wei Huang 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 > Acked-by: Tianfei Zhang > Reviewed-by: Rosen Xu > --- > 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