From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pd0-f175.google.com (mail-pd0-f175.google.com [209.85.192.175]) by dpdk.org (Postfix) with ESMTP id CE6145A6A for ; Wed, 25 Feb 2015 15:56:37 +0100 (CET) Received: by pdjg10 with SMTP id g10so5429559pdj.1 for ; Wed, 25 Feb 2015 06:56:37 -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=aMlJeby9ge6oZ9PYGjg3yom5fAxeioprjdqq8G9O6s4=; b=Sf8lC82iDashw18TD+AJoWYiUspnWcGeDG95OIWjvvQpH8ko3VsGO6QKoUy1HJ6Yeq TObbBvwOdqN1UujCVZEtYpl8knJhUO6pWak7zWXrexD9PbWyWzLW3meizbfxC+Es1LaP D0geOu37fqSdlec12qJvQtqgbUMjAWp9wGyRQhPY8N92mhweE0sQO/mHSoq86eHTkX3x ilwaDrwiYfcHzkVZOfzMkr2ojcLY4nkeF2IAl7bLi9FHqydJqnMbbx8N4jD/uaMTXkVB psFWcwfXThY5Y3KxkkEQqQtH5LKj6ZoI7yaOHTVKB6tz9EQBis6vP7SBIPYGKPC7DMlG 1CXw== X-Gm-Message-State: ALoCoQkMWbwoPxY/oG55fSZhiLrVvFNj47DQbWtJn1Py+HT2Ta7e0u+Xv5prYwFt4o9svC6CqC09 X-Received: by 10.70.48.70 with SMTP id j6mr6209417pdn.160.1424876196904; Wed, 25 Feb 2015 06:56:36 -0800 (PST) Received: from [10.16.129.101] (napt.igel.co.jp. [219.106.231.132]) by mx.google.com with ESMTPSA id z6sm25842159pbt.89.2015.02.25.06.56.34 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 25 Feb 2015 06:56:35 -0800 (PST) Message-ID: <54EDE29D.1050303@igel.co.jp> Date: Wed, 25 Feb 2015 23:56:29 +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: Thomas Monjalon References: <1424060073-23484-2-git-send-email-mukawa@igel.co.jp> <2987764.cYYQsyCFG8@xps13> <2355791.9NkeVcuA9T@xps13> In-Reply-To: <2355791.9NkeVcuA9T@xps13> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH v14 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: Wed, 25 Feb 2015 14:56:38 -0000 On 2015/02/25 23:00, Thomas Monjalon wrote: > 2015-02-25 21:32, Tetsuya Mukawa: >> 2015-02-25 20:21 GMT+09:00 Thomas Monjalon : >>> 2015-02-25 13:04, Tetsuya Mukawa: >>>> --- a/lib/librte_eal/common/eal_common_dev.c >>>> +++ b/lib/librte_eal/common/eal_common_dev.c >>>> @@ -32,10 +32,13 @@ >>>> * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. >>>> */ >>>> >>>> +#include >>>> +#include >>>> #include >>>> #include >>>> #include >>>> >>>> +#include >>>> #include >>>> #include >>> No, you must not include ethdev in EAL. >>> The ethdev layer is by design on top of EAL. >>> Maxime already asked why you did it. He was implicitly asking to remove it. >>> You said that you are calling ethdev_is_detachable() but you should >>> call a function eal_is_detachable() or something like that. >>> The detachable state must be only device-related, i.e. in EAL. >>> The ethdev API is only a wrapper (with port id) in such case. >>> >> Hi Thomas, >> >> If ethdev library is on top of EAL, hotplug functions like >> rte_eal_dev_attach/detach should be implemented in ethdev library. >> Is it right? > Yes you're right. > >> If so, I will move rte_eal_dev_attach/detach to ethdev library. >> And I will change names like rte_eth_dev_attach/detach. > It seems to be the right thing to do. > >> Also, I will add "rte_dev.h" and "rte_pci.h" in rte_ethdev.h, and call >> below EAL functions from ethdev library. >> >> - For virtual device initialization and finalization >> -- rte_eth_vdev_init >> -- rte_eth_vdev_uninit() >> - For physical NIC initialization and finalization >> -- rte_eal_pci_probe_one() >> -- rte_eal_pci_close_one() >> >> I guess this will fix this design violation. >> Is this ok? > I think yes. > If needed, we could do some cleanup after RC1. > I'm just waiting for you fixing this, to avoid introducing > a layering violation. > Would you able to do it today? Hi Thomas, I appreciate for your reply. I start trying it. Thanks, Tetsuya > Thanks > >>>> --- a/lib/librte_eal/linuxapp/eal/Makefile >>>> +++ b/lib/librte_eal/linuxapp/eal/Makefile >>>> @@ -45,6 +45,7 @@ CFLAGS += -I$(RTE_SDK)/lib/librte_eal/common/include >>>> CFLAGS += -I$(RTE_SDK)/lib/librte_ring >>>> CFLAGS += -I$(RTE_SDK)/lib/librte_mempool >>>> CFLAGS += -I$(RTE_SDK)/lib/librte_malloc >>>> +CFLAGS += -I$(RTE_SDK)/lib/librte_mbuf >>> By removing ethdev dependency, you can remove this ugly mbuf dependency. >>> >>> Thanks Tetsuya >>> >