From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pd0-f181.google.com (mail-pd0-f181.google.com [209.85.192.181]) by dpdk.org (Postfix) with ESMTP id 20E55ADC3 for ; Tue, 24 Feb 2015 05:49:04 +0100 (CET) Received: by pdev10 with SMTP id v10so30551152pde.10 for ; Mon, 23 Feb 2015 20:49:03 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:message-id:date:from:user-agent:mime-version:to :cc:subject:references:in-reply-to:content-type :content-transfer-encoding; bh=E4Z5yLgsMHDbdLlEwyCiy9RBXA2jatwbwgVJY2wCwaQ=; b=dbxFiXPdUuZb+5AhWPAqaAti8uLRuVk07K/c304uBVqdi00cJ2ytmcUjEIrpDJ9tzG E9kLABuqt2WcbmYzAKFXiBnSOrafBsQhn8hy5Fx4d3NvTXs4THkl4H/OEYeH3DQ/7xqZ VDvohmfksnLLUUx/F8Jij0IWsLSV+SIsPksJhofm9ANpF2Z2Q67RGbFUYWW1zy65PGWR F7+DhCh4+T+rQo3fDktQORjQIMDdGipo1MviWRTWlFunl7FJoj3lrL2WtAVaM/c5yVtZ wJt7Bmi1xdRi8iup4XLYnnkr6M/Rso1C4RiORBVCR8WHIMEk+OMy9+oTyCWo0mwehT8Y 7Mcw== X-Gm-Message-State: ALoCoQlr1+/ivSuck9o33SbN0Ntzr1Tt9oJ1Q0iJ5Cin7ueO5MuulDORl3X6CBxBIZ40XtrGJyEg X-Received: by 10.68.65.42 with SMTP id u10mr611217pbs.81.1424753343384; Mon, 23 Feb 2015 20:49:03 -0800 (PST) Received: from [10.16.129.101] (napt.igel.co.jp. [219.106.231.132]) by mx.google.com with ESMTPSA id fc6sm7284198pab.6.2015.02.23.20.49.01 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 23 Feb 2015 20:49:02 -0800 (PST) Message-ID: <54EC02B9.6070108@igel.co.jp> Date: Tue, 24 Feb 2015 13:48:57 +0900 From: Tetsuya Mukawa User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 MIME-Version: 1.0 To: Maxime Leroy 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> In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit 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: Tue, 24 Feb 2015 04:49:04 -0000 On 2015/02/23 22:29, Maxime Leroy wrote: > 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; >> +} >> + Hi Maxime, I appreciate your comments. I've change like your comments, and send new one soon. Regards, Tetsuya > Regards, > > Maxime