From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wi0-f175.google.com (mail-wi0-f175.google.com [209.85.212.175]) by dpdk.org (Postfix) with ESMTP id 2CB29AD95 for ; Mon, 23 Feb 2015 14:29:09 +0100 (CET) Received: by mail-wi0-f175.google.com with SMTP id r20so17011496wiv.2 for ; Mon, 23 Feb 2015 05:29:09 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=z8nT1HzMbZTF7IfCeN7JSzYit97Qwc/fQE4P2Nfh5tY=; b=Vl5iiAtGQhlC80+l9/1vEaNC2Mwl+Y0aUhO1QOzB7PQHBA4B57Raijp5pIGRvRJXf0 AELktSxUSoUlE3cn9DTvoc+MrhK19618xoUNbNRX6pQSWnwWkf3Coweis0cyOD2wbDHW 9aqx4sntVjbvImlUHArSKj2/T8eBru53IGrVxILvBQEL9IU36jT142j8IHhOoyIWr+A2 SJZPLe3agKNnsKWTubyt2cdYJ8mlQzMeE7hRA3W3H+7VnQrIM9u/Y2QbLig+ZPXZtJX4 dO1tL2NN+1027xjt69LqkTkzsxTzKHLBoNqFfuI7G+Y/U/SRsP0yxR9uik2/XAsMGtCA T0+Q== X-Gm-Message-State: ALoCoQnHAAXUSkGxsBj58pOtOMZkdF8KiMgZcxV++FK1XVHMsRN9BzvWJ2hcC5h2aHhSmHlVYrKb MIME-Version: 1.0 X-Received: by 10.180.11.205 with SMTP id s13mr20577231wib.32.1424698149047; Mon, 23 Feb 2015 05:29:09 -0800 (PST) Received: by 10.194.185.204 with HTTP; Mon, 23 Feb 2015 05:29:08 -0800 (PST) In-Reply-To: <1424668171-8695-13-git-send-email-mukawa@igel.co.jp> References: <1424414390-18509-6-git-send-email-mukawa@igel.co.jp> <1424668171-8695-1-git-send-email-mukawa@igel.co.jp> <1424668171-8695-13-git-send-email-mukawa@igel.co.jp> Date: Mon, 23 Feb 2015 14:29:08 +0100 Message-ID: From: Maxime Leroy To: Tetsuya Mukawa Content-Type: text/plain; charset=UTF-8 Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH v11 12/13] 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: Mon, 23 Feb 2015 13:29:09 -0000 Hi Tetsuya, On Mon, Feb 23, 2015 at 6:09 AM, Tetsuya Mukawa wrote: > These functions are used for attaching or detaching a port. [...] > > +static int > +rte_eal_vdev_init(const char *name, const char *args) > +{ > + struct rte_driver *driver; > + > + if (name == NULL) > + return -EINVAL; > + > + TAILQ_FOREACH(driver, &dev_driver_list, next) { > + if (driver->type != PMD_VDEV) > + continue; > + > + /* > + * search a driver prefix in virtual device name. > + * For example, if the driver is pcap PMD, driver->name > + * will be "eth_pcap", but "name" will be "eth_pcapN". > + * So use strncmp to compare. > + */ > + if (!strncmp(driver->name, name, strlen(driver->name))) { > + driver->init(name, args); > + break; Please return the value given by init: return driver->init(name, args); . > + } > + } > + > + if (driver == NULL) { > + RTE_LOG(WARNING, EAL, "no driver found for %s\n", name); --> should be : RTE_LOG(ERR . > + return -EINVAL; > + } > + return 0; > +} > + > int > rte_eal_dev_init(void) > { > @@ -79,23 +113,10 @@ rte_eal_dev_init(void) > if (devargs->type != RTE_DEVTYPE_VIRTUAL) > 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))) { > - driver->init(devargs->virtual.drv_name, > - devargs->args); > - break; > - } > - } > - > - if (driver == NULL) { > + if (rte_eal_vdev_init(devargs->virtual.drv_name, > + devargs->args)) > rte_panic("no driver found for %s\n", > devargs->virtual.drv_name); instead of that: if (rte_eal_vdev_init(devargs->virtual.drv_name, devargs->args)) { RTE_LOG(ERR, "failed to initialize %s device\n", devargs->virtual.drv_name); return -1; } > - } > } > > /* Once the vdevs are initalized, start calling all the pdev drivers */ > @@ -107,3 +128,237 @@ rte_eal_dev_init(void) > } > return 0; > } > + > +/* So far, DPDK hotplug function only supports linux */ > +#ifdef RTE_LIBRTE_EAL_HOTPLUG > +static int > +rte_eal_vdev_uninit(const char *name) > +{ > + struct rte_driver *driver; > + > + if (name == NULL) > + return -EINVAL; > + > + TAILQ_FOREACH(driver, &dev_driver_list, next) { > + if (driver->type != PMD_VDEV) > + continue; > + > + /* > + * search a driver prefix in virtual device name. > + * For example, if the driver is pcap PMD, driver->name > + * will be "eth_pcap", but "name" will be "eth_pcapN". > + * So use strncmp to compare. > + */ > + if (!strncmp(driver->name, name, strlen(driver->name))) { > + driver->uninit(name); Please return the value given by uninit: return driver->uninit(name, args); > + break; > + } > + } > + > + if (driver == NULL) { > + RTE_LOG(WARNING, EAL, "no driver found for %s\n", name); > + return 1; As it's an error, the function should return a negative value ( i.e. -EINVAL). Please set the log level to ERR. > + } > + return 0; > +} > + [...] > +} > + > +/* 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 *name = NULL, *args = NULL; > + uint8_t new_port_id; > + struct rte_eth_dev devs[RTE_MAX_ETHPORTS]; > + int ret = -1; > + > + if ((vdevargs == NULL) || (port_id == NULL)) > + goto end; > + > + /* parse vdevargs, then retrieve device name and args */ > + if (rte_eal_parse_devargs_str(vdevargs, &name, &args)) > + goto end; > + > + /* save current port status */ > + if (rte_eth_dev_save(devs, sizeof(devs))) > + goto end; > + /* walk around dev_driver_list to find the driver of the device, > + * then invoke probe function o the driver. > + * TODO: > + * rte_eal_vdev_init() should return port_id, > + * And rte_eth_dev_save() and rte_eth_dev_get_changed_port() > + * should be removed. */ > + if (rte_eal_vdev_init(name, args)) > + goto end; > + /* get port_id enabled by above procedures */ > + if (rte_eth_dev_get_changed_port(devs, &new_port_id)) > + goto end; > + ret = 0; > + Please set port_id here, i.e. : *port_id = new_port_id; > +end: > + if (name) > + free(name); > + if (args) > + free(args); > + > + *port_id = new_port_id; and not here. > + if (ret < 0) > + RTE_LOG(ERR, EAL, "Driver, cannot attach the device\n"); > + return ret; > +} > + e\n"); > + return -1; > +} > + Regards, Maxime