From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wi0-f182.google.com (mail-wi0-f182.google.com [209.85.212.182]) by dpdk.org (Postfix) with ESMTP id 23CD89A8D for ; Wed, 25 Feb 2015 15:00:51 +0100 (CET) Received: by mail-wi0-f182.google.com with SMTP id l15so5229424wiw.3 for ; Wed, 25 Feb 2015 06:00:51 -0800 (PST) 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=xO+HuCXxn/tKCAdNPNjQ1nOGiQEpWeJaKxVAy49tqdw=; b=K7BvNdb9kp6ZfE8v764BuQpoeKbNXj9vgmKpNc4BmmCol9UkFqkpRAoyEowssGqCGm 31KAnc9+EJKksf8B59Ze7+YvILoOLEFYhSROlfu+TZJIth6v3QKpqmL200qaG8M0NFwV LhE5gKUfXMSlC2J8NHIrp5IAOWO0vDG9VwqbxY1PyN3mETiVyIkfKeT+xqccuq9pP4DJ EGNJ4ZQREeFcLEcf6J95s1Nv5gutgeH51ytSRimdFK32ZUxM7h8JN9Cic7Na0GNUqUEk j4jeM3cCYGNNZqEgdUSBobelcsLkYcAdzEN1douL84LjHz0CO8lTJHnduAWimaGv0RMA kkHA== X-Gm-Message-State: ALoCoQnYK2kDwnBVmq/LVPoPu8qifTQojrvI6VmfPd5p4RjqdRmJbkRuBGOiSFfeFPOVLIo3mSyr X-Received: by 10.180.96.37 with SMTP id dp5mr39953847wib.64.1424872850189; Wed, 25 Feb 2015 06:00:50 -0800 (PST) Received: from xps13.localnet (136-92-190-109.dsl.ovh.fr. [109.190.92.136]) by mx.google.com with ESMTPSA id dm6sm25281366wib.22.2015.02.25.06.00.48 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 25 Feb 2015 06:00:49 -0800 (PST) From: Thomas Monjalon To: Tetsuya Mukawa Date: Wed, 25 Feb 2015 15:00:16 +0100 Message-ID: <2355791.9NkeVcuA9T@xps13> Organization: 6WIND User-Agent: KMail/4.14.4 (Linux/3.18.4-1-ARCH; KDE/4.14.4; x86_64; ; ) In-Reply-To: References: <1424060073-23484-2-git-send-email-mukawa@igel.co.jp> <2987764.cYYQsyCFG8@xps13> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" 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:00:51 -0000 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? 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 > >