From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wi0-f174.google.com (mail-wi0-f174.google.com [209.85.212.174]) by dpdk.org (Postfix) with ESMTP id AC867B65F for ; Tue, 17 Feb 2015 02:48:57 +0100 (CET) Received: by mail-wi0-f174.google.com with SMTP id em10so29665342wid.1 for ; Mon, 16 Feb 2015 17:48:57 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:organization :user-agent:in-reply-to:references:mime-version :content-transfer-encoding:content-type; bh=0qygkTMZ1z772RA3JI6yRFoIZqPnl0IwSBk5PbSMiXk=; b=iUsw0p1FT/j0TyXv0NytSli2Tz4hs9jqrVnkUG3W+ua2TrMtQ9MG90X0vmgdpiJagO ghoakeiUB8qnpuaCenCGpidG5ztMjYgPMuIajDw+0PeQDj5VTB3zaK0rQNDSD50qZEXV fDV9japokgCxor8XVKwfQKW9qR0D/VUQQRb5K+aTgRdhQmLBL1LPmqoDW7VGPi+rj8pc W5sqdw7gKrpk+ExVQvGGv8BNQt5E6Dam3qqnScFiXcH393VUDvQY2CjIldoace3TRsUi YVcPMvD7dxEoWTphl++yvPbl3GcRaLPfV6UZ+sqaR8awXBLNajPq4o0fQEvXiECd2rY1 LL9w== X-Gm-Message-State: ALoCoQkkdGllSCVg0Tctr+8rrQyQgej869i9o9o076BAQRnTLrFcMKxyN6QdoynlCOrhBUFkiPcI X-Received: by 10.180.73.241 with SMTP id o17mr42521091wiv.16.1424137737529; Mon, 16 Feb 2015 17:48:57 -0800 (PST) Received: from xps13.localnet (guy78-1-82-235-116-147.fbx.proxad.net. [82.235.116.147]) by mx.google.com with ESMTPSA id fs8sm18014189wib.8.2015.02.16.17.48.55 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 16 Feb 2015 17:48:56 -0800 (PST) From: Thomas Monjalon To: Tetsuya Mukawa Date: Tue, 17 Feb 2015 02:48:24 +0100 Message-ID: <83798909.brLNWNC9tj@xps13> Organization: 6WIND User-Agent: KMail/4.14.4 (Linux/3.18.4-1-ARCH; KDE/4.14.4; x86_64; ; ) In-Reply-To: <1424060073-23484-13-git-send-email-mukawa@igel.co.jp> References: <1423470639-15744-2-git-send-email-mukawa@igel.co.jp> <1424060073-23484-1-git-send-email-mukawa@igel.co.jp> <1424060073-23484-13-git-send-email-mukawa@igel.co.jp> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH v8 12/14] eal/pci: Add rte_eal_dev_attach/detach() functions X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 17 Feb 2015 01:48:57 -0000 2015-02-16 13:14, Tetsuya Mukawa: > These functions are used for attaching or detaching a port. > When rte_eal_dev_attach() is called, the function tries to realize the > device name as pci address. If this is done successfully, > rte_eal_dev_attach() will attach physical device port. If not, attaches > virtual devive port. > When rte_eal_dev_detach() is called, the function gets the device type > of this port to know whether the port is come from physical or virtual. > And then specific detaching function will be called. > > v8: > - Add missing symbol in version map. > (Thanks to Qiu, Michael and Iremonger, Bernard) > v7: > - Fix typo of warning messages. > (Thanks to Qiu, Michael) > v5: > - Change function names like below. > rte_eal_dev_find_and_invoke() to rte_eal_vdev_find_and_invoke(). > rte_eal_dev_invoke() to rte_eal_vdev_invoke(). > - Add code to handle a return value of rte_eal_devargs_remove(). > - Fix pci address format in rte_eal_dev_detach(). > v4: > - Fix comment. > - Add error checking. > - Fix indent of 'if' statement. > - Change function name. > > Signed-off-by: Tetsuya Mukawa > --- > lib/librte_eal/common/eal_common_dev.c | 276 ++++++++++++++++++++++++ > lib/librte_eal/common/eal_private.h | 11 + > lib/librte_eal/common/include/rte_dev.h | 33 +++ > lib/librte_eal/linuxapp/eal/Makefile | 1 + > lib/librte_eal/linuxapp/eal/eal_pci.c | 6 +- > lib/librte_eal/linuxapp/eal/rte_eal_version.map | 2 + > 6 files changed, 326 insertions(+), 3 deletions(-) > > diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c > index eae5656..3d169a4 100644 > --- a/lib/librte_eal/common/eal_common_dev.c > +++ b/lib/librte_eal/common/eal_common_dev.c > @@ -32,10 +32,13 @@ > * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. > */ > > +#include > +#include > #include > #include > #include > > +#include > #include > #include > #include > @@ -107,3 +110,276 @@ rte_eal_dev_init(void) > } > return 0; > } > + > +/* So far, DPDK hotplug function only supports linux */ This comment should be in the configuration. > +#ifdef ENABLE_HOTPLUG > +static void > +rte_eal_vdev_invoke(struct rte_driver *driver, > + struct rte_devargs *devargs, enum rte_eal_invoke_type type) > +{ > + if ((driver == NULL) || (devargs == NULL)) > + return; > + > + switch (type) { > + case RTE_EAL_INVOKE_TYPE_PROBE: > + driver->init(devargs->virtual.drv_name, devargs->args); > + break; > + case RTE_EAL_INVOKE_TYPE_CLOSE: > + driver->uninit(devargs->virtual.drv_name, devargs->args); > + break; > + default: > + break; > + } > +} It would be clearer to directly call init and uninit instead of using this "invoke" method. > + > +static int > +rte_eal_vdev_find_and_invoke(const char *name, int type) > +{ > + struct rte_devargs *devargs; > + struct rte_driver *driver; > + > + if (name == NULL) > + return -EINVAL; > + > + /* call the init function for each virtual device */ This comment is wrong. > + TAILQ_FOREACH(devargs, &devargs_list, next) { > + > + if (devargs->type != RTE_DEVTYPE_VIRTUAL) > + continue; > + > + if (strncmp(name, devargs->virtual.drv_name, strlen(name))) > + continue; > + > + TAILQ_FOREACH(driver, &dev_driver_list, next) { > + if (driver->type != PMD_VDEV) > + continue; > + > + /* search a driver prefix in virtual device name */ > + if (!strncmp(driver->name, devargs->virtual.drv_name, > + strlen(driver->name))) { Why not use strcmp? > + rte_eal_vdev_invoke(driver, devargs, type); > + break; > + } > + } > + > + if (driver == NULL) { > + RTE_LOG(WARNING, EAL, "no driver found for %s\n", > + devargs->virtual.drv_name); > + } > + return 0; > + } > + return 1; > +} > + > +/* attach the new physical device, then store port_id of the device */ > +static int > +rte_eal_dev_attach_pdev(struct rte_pci_addr *addr, uint8_t *port_id) > +{ > + uint8_t new_port_id; > + struct rte_eth_dev devs[RTE_MAX_ETHPORTS]; > + > + if ((addr == NULL) || (port_id == NULL)) > + goto err; > + > + /* save current port status */ > + if (rte_eth_dev_save(devs, sizeof(devs))) > + goto err; > + /* re-construct pci_device_list */ > + if (rte_eal_pci_scan()) > + goto err; > + /* invoke probe func of the driver can handle the new device */ > + if (rte_eal_pci_probe_one(addr)) > + goto err; You should get the port_id from the previous function instead of searching it. > + /* get port_id enabled by above procedures */ > + if (rte_eth_dev_get_changed_port(devs, &new_port_id)) > + goto err; > + > + *port_id = new_port_id; > + return 0; > +err: > + RTE_LOG(ERR, EAL, "Driver, cannot attach the device\n"); > + return -1; > +} [...] > +/* attach the new virtual device, then store port_id of the device */ > +static int > +rte_eal_dev_attach_vdev(const char *vdevargs, uint8_t *port_id) > +{ > + char *args; > + uint8_t new_port_id; > + struct rte_eth_dev devs[RTE_MAX_ETHPORTS]; > + > + if ((vdevargs == NULL) || (port_id == NULL)) > + goto err0; > + > + args = strdup(vdevargs); > + if (args == NULL) > + goto err0; > + > + /* save current port status */ > + if (rte_eth_dev_save(devs, sizeof(devs))) > + goto err1; > + /* add the vdevargs to devargs_list */ > + if (rte_eal_devargs_add(RTE_DEVTYPE_VIRTUAL, args)) > + goto err1; > + /* parse vdevargs, then retrieve device name */ > + get_vdev_name(args); > + /* walk around dev_driver_list to find the driver of the device, > + * then invoke probe function o the driver */ > + if (rte_eal_vdev_find_and_invoke(args, RTE_EAL_INVOKE_TYPE_PROBE)) > + goto err2; Again, you should get port_id from the attach procedure. > + /* get port_id enabled by above procedures */ > + if (rte_eth_dev_get_changed_port(devs, &new_port_id)) > + goto err2; [...] > /** > + * Uninitilization function called for each device driver once. > + */ > +typedef int (rte_dev_uninit_t)(const char *name, const char *args); Why do you need args for uninit?