From mboxrd@z Thu Jan  1 00:00:00 1970
From: Thomas Monjalon <thomas.monjalon-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>
Subject: Re: [PATCH v14 12/13] eal/pci: Add
	rte_eal_dev_attach/detach() functions
Date: Wed, 25 Feb 2015 15:00:16 +0100
Message-ID: <2355791.9NkeVcuA9T@xps13>
References: <1424060073-23484-2-git-send-email-mukawa@igel.co.jp>
 <2987764.cYYQsyCFG8@xps13>
 <CAKU8wkeFtGvjdchjEYqd3jpvCYL-QhoWV9qLyWKKvnFK-LvS_A@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7Bit
Cc: dev-VfR2kkLFssw@public.gmane.org
To: Tetsuya Mukawa <mukawa-AlSX/UN32fvPDbFq/vQRIQ@public.gmane.org>
Return-path: <dev-bounces-VfR2kkLFssw@public.gmane.org>
In-Reply-To: <CAKU8wkeFtGvjdchjEYqd3jpvCYL-QhoWV9qLyWKKvnFK-LvS_A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request-VfR2kkLFssw@public.gmane.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev-VfR2kkLFssw@public.gmane.org>
List-Help: <mailto:dev-request-VfR2kkLFssw@public.gmane.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request-VfR2kkLFssw@public.gmane.org?subject=subscribe>
Errors-To: dev-bounces-VfR2kkLFssw@public.gmane.org
Sender: "dev" <dev-bounces-VfR2kkLFssw@public.gmane.org>

2015-02-25 21:32, Tetsuya Mukawa:
> 2015-02-25 20:21 GMT+09:00 Thomas Monjalon <thomas.monjalon-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>:
> > 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 <stdio.h>
> >> +#include <limits.h>
> >>  #include <string.h>
> >>  #include <inttypes.h>
> >>  #include <sys/queue.h>
> >>
> >> +#include <rte_ethdev.h>
> >>  #include <rte_dev.h>
> >>  #include <rte_devargs.h>
> >
> > 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
> >