From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <thomas.monjalon@6wind.com>
Received: from mail-we0-f171.google.com (mail-we0-f171.google.com
[74.125.82.171]) by dpdk.org (Postfix) with ESMTP id A15A9678B
for <dev@dpdk.org>; Fri, 18 Apr 2014 14:02:18 +0200 (CEST)
Received: by mail-we0-f171.google.com with SMTP id t61so1483902wes.2
for <dev@dpdk.org>; Fri, 18 Apr 2014 05:02:19 -0700 (PDT)
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=CT/NtLKyzN7nHj5jIOATgEjnKpcuG1wb72JX/RqCdAY=;
b=K1MPPvJ513m7wpeYuGrRZKQn4pQ3Lmjrsu/TsN6oFc+Rq3TFZw6P6Q/hTgGBi2Ejbb
AQkTsn+Ll+SoDU+OwXTCVHyS0B7ypPSjfexYpfPKr3KeeM1cJs2bj55iQM48a/hWOsjD
jYR7g3fIzf6hzzpX+Kz0oUSvDtUFiB+Yy6sKEVUwKd7A9YsKRR/h0JzEWjV9OFi6fgLT
3efZ0s4qB2TwbFSMBIz27pXE5j+lv8jjTOqSlmv81wv+PzE1JLHhmvhVebth7FTSGfmt
KkLML69X3oEfhkN3KUyk0riAFVI4IMw5b1PfVPAtKnaWjg4fEtiuhYOiykue6ENXIv2n
kEDg==
X-Gm-Message-State: ALoCoQkO/cw2mTljwCETlnh8uAA8Yifd7/J/3wT8P2Ve0nf+22ifJ+y1S5g/bqZZG9A2xH0alfUO
X-Received: by 10.180.81.228 with SMTP id d4mr2156376wiy.49.1397822539455;
Fri, 18 Apr 2014 05:02:19 -0700 (PDT)
Received: from xps13.localnet (6wind.net2.nerim.net. [213.41.180.237])
by mx.google.com with ESMTPSA id b1sm43302146wjb.37.2014.04.18.05.02.18
for <multiple recipients>
(version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);
Fri, 18 Apr 2014 05:02:18 -0700 (PDT)
From: Thomas Monjalon <thomas.monjalon@6wind.com>
To: Neil Horman <nhorman@tuxdriver.com>
Date: Fri, 18 Apr 2014 14:02:17 +0200
Message-ID: <2223050.MHJTNgySuI@xps13>
Organization: 6WIND
User-Agent: KMail/4.12.4 (Linux/3.14.1-1-ARCH; KDE/4.12.4; x86_64; ; )
In-Reply-To: <1397585169-14537-8-git-send-email-nhorman@tuxdriver.com>
References: <1397585169-14537-1-git-send-email-nhorman@tuxdriver.com>
<1397585169-14537-8-git-send-email-nhorman@tuxdriver.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 7Bit
Content-Type: text/plain; charset="us-ascii"
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH 07/15] eal: Make vdev init path generic for
both virtual and physcial devices
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, 18 Apr 2014 12:02:18 -0000
I have some comments inline.
2014-04-15 14:06, Neil Horman:
> Currently, physical device pmds use a separate initalization path
> (rte_pmd_init_all) while virtual devices use a constructor registration and
> rte_eal_dev_init. Theres no reason to have them be separate. This patch
> removes the vdev specific nomenclature from the vdev init path and makes it
> more generic for use with all pmds. This is the first step in converting
> the physical device pmds to using the same constructor based registration
> path that the virtual devices use
>
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> - if (rte_eal_vdev_init() < 0)
> + if (rte_eal_dev_init() < 0)
> rte_panic("Cannot init virtual devices\n");
You should update the panic log here.
> +/** Global list of virtual device drivers. */
> +static struct rte_driver_list dev_driver_list =
> + TAILQ_HEAD_INITIALIZER(dev_driver_list);
Same comment about "virtual device".
> + /* No need to register drivers that are embeded in DPDK
> + * (pmd_pcap, pmd_ring, ...). The initialization function have
> + * the ((constructor)) attribute so they will register at
> + * startup. */
Should we keep this comment?
> +#ifndef _RTE_VDEV_H_
> +#define _RTE_VDEV_H_
Should be _RTE_DEV_H_
> +/**
> + * @file
> + *
> + * RTE Virtual Devices Interface
> + *
> + * This file manages the list of the virtual device drivers.
> + */
Not only virtual.
> +/** Double linked list of virtual device drivers. */
[...]
> + * Initialization function called for each virtual device probing.
[...]
> +/**
> + * A structure describing a virtual device driver.
> + */
[...]
> + * Register a virtual device driver.
[...]
> + * Unregister a virtual device driver.
You probably understood the idea ;)
> --- a/lib/librte_eal/linuxapp/eal/eal.c
> +++ b/lib/librte_eal/linuxapp/eal/eal.c
> - if (rte_eal_vdev_init() < 0)
> + if (rte_eal_dev_init() < 0)
> rte_panic("Cannot init virtual devices\n");
Still "virtual" typo
Except typos, it seems a good step.
I think we could abstract more things in order to have even simpler API and
simpler command line. But we'll see it in another step.
Thanks
--
Thomas