From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 ; Fri, 18 Apr 2014 14:02:18 +0200 (CEST) Received: by mail-we0-f171.google.com with SMTP id t61so1483902wes.2 for ; 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 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 18 Apr 2014 05:02:18 -0700 (PDT) From: Thomas Monjalon To: Neil Horman 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-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 > - 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