From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <maxime.leroy@6wind.com>
Received: from mail-we0-f182.google.com (mail-we0-f182.google.com
[74.125.82.182]) by dpdk.org (Postfix) with ESMTP id B95ADB555
for <dev@dpdk.org>; Fri, 20 Feb 2015 11:14:31 +0100 (CET)
Received: by wevm14 with SMTP id m14so4632352wev.13
for <dev@dpdk.org>; Fri, 20 Feb 2015 02:14:31 -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=zjLkp1yZrS9Qi0SlIIlWZq4Me4eAbxrB1XJgh6ec2iI=;
b=Q6MDbI3t4D0NibGDbYkgsvYf1KLwtVil5jyaRYWl5TBNJbebO1h4xqSvyH8eAohLSv
L+aUaXaw8ZjbD8qH2ChMPWVTaVq6hFJtbnXr8KnAS3sIXvVoLkxI0Lfvtk4QsGKjpfq4
GQN+XzLmyqOA7CpQX7RIWkLuvEUBstuzFxZTsmRvTYDJ98l3unDzEh5cMVeNhzJ3YG9P
g1sQweKxYEHAZzFM6VK7vjfVTDd0+nEAP2d3pWUPOiLjHS/BoCvjo3Wr9KXVZSugUvY/
ZhooaoL0Ncci26oNGfQuPUo7mGvRZdRvLqLkg8jXN6SAM/tQr1OVeqGjaR/bNYUTKixa
He3Q==
X-Gm-Message-State: ALoCoQmATtZAc6vgWFYUnxzINO9yvxlgcA/2NOqfgz6ep+bsy7DEz9zf0uZNa83VqHlYbZzgpqYj
MIME-Version: 1.0
X-Received: by 10.194.241.197 with SMTP id wk5mr407406wjc.44.1424427271546;
Fri, 20 Feb 2015 02:14:31 -0800 (PST)
Received: by 10.194.185.204 with HTTP; Fri, 20 Feb 2015 02:14:31 -0800 (PST)
In-Reply-To: <1424414390-18509-14-git-send-email-mukawa@igel.co.jp>
References: <1424060073-23484-2-git-send-email-mukawa@igel.co.jp>
<1424414390-18509-1-git-send-email-mukawa@igel.co.jp>
<1424414390-18509-14-git-send-email-mukawa@igel.co.jp>
Date: Fri, 20 Feb 2015 11:14:31 +0100
Message-ID: <CAEykdvpbx=n60XbjSBQnZJN7=t3o2CncC2hMo9ytB7V2GsL+RQ@mail.gmail.com>
From: Maxime Leroy <maxime.leroy@6wind.com>
To: Tetsuya Mukawa <mukawa@igel.co.jp>
Content-Type: text/plain; charset=UTF-8
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v10 13/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 <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Fri, 20 Feb 2015 10:14:32 -0000
Hi Tetsuya,
On Fri, Feb 20, 2015 at 7:39 AM, Tetsuya Mukawa <mukawa@igel.co.jp> wrote:
> These functions are used for attaching or detaching a port.
[..]
> +
> +static void
> +get_vdev_name(char *vdevargs)
> +{
> + char *sep;
> +
> + if (vdevargs == NULL)
> + return;
> +
> + /* set the first ',' to '\0' to split name and arguments */
> + sep = strchr(vdevargs, ',');
> + if (sep != NULL)
> + sep[0] = '\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 *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;
You don't need to add devargs to the devargs_list.
The devargs_list is only needed at the init to store the arguments
when we parse the command line. Then, at initialization,
rte_eal_dev_init creates the devices from this list .
Instead of adding the devargs in the list, you could have the following code:
static int
rte_eal_dev_attach_vdev(const char *vdevargs, uint8_t *port_id)
{
...
/* save current port status */
if (rte_eth_dev_save(devs, sizeof(devs)))
goto err;
devargs = rte_eal_parse_devargs_str(RTE_
DEVTYPE_VIRTUAL, vdevargs);
if (devargs == NULL)
goto err;
if (rte_eal_vdev_devinit(devargs) < 0)
goto err;
if (rte_eth_dev_get_changed_port(devs, &new_port_id))
goto err;
...
}
What do you think ?
> + /* 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.
> + * TODO:
> + * rte_eal_vdev_find_and_init() should return port_id,
> + * And rte_eth_dev_save() and rte_eth_dev_get_changed_port()
> + * should be removed. */
> + if (rte_eal_vdev_find_and_init(args))
> + goto err2;
> + /* get port_id enabled by above procedures */
> + if (rte_eth_dev_get_changed_port(devs, &new_port_id))
> + goto err2;
> +
> + free(args);
> + *port_id = new_port_id;
> + return 0;
> +err2:
> + rte_eal_devargs_remove(RTE_DEVTYPE_VIRTUAL, args);
> +err1:
> + free(args);
> +err0:
> + RTE_LOG(ERR, EAL, "Driver, cannot attach the device\n");
> + return -1;
> +}
> +
> +/* detach the new virtual device, then store the name of the device */
> +static int
> +rte_eal_dev_detach_vdev(uint8_t port_id, char *vdevname)
> +{
> + char name[RTE_ETH_NAME_MAX_LEN];
> +
> + if (vdevname == NULL)
> + goto err;
> +
> + /* check whether the driver supports detach feature, or not */
> + if (rte_eth_dev_is_detachable(port_id))
> + goto err;
> +
> + /* get device name by port id */
> + if (rte_eth_dev_get_name_by_port(port_id, name))
> + goto err;
> + /* walk around dev_driver_list to find the driver of the device,
> + * then invoke close function o the driver */
> + if (rte_eal_vdev_find_and_uninit(name))
> + goto err;
> + /* remove the vdevname from devargs_list */
> + if (rte_eal_devargs_remove(RTE_DEVTYPE_VIRTUAL, name))
> + goto err;
The same here, you don't need to remove devargs from the devargs_list.
Instead of removing the devargs in the list, you could have the following code::
static int
rte_eal_dev_detach_vdev(uint8_t port_id, char *vdevname)
{
...
/* check whether the driver supports detach feature, or not */
if (rte_eth_dev_is_detachable(port_id))
goto err;
/* get device name by port id */
if (rte_eth_dev_get_name_by_port(port_id, name))
goto err;
if (rte_eal_vdev_uninit(name))
goto err;
...
}
What do you think ?
Regards,
Maxime