2015-02-25 21:32, Tetsuya Mukawa: > 2015-02-25 20:21 GMT+09:00 Thomas Monjalon <thomas.monjalon at 6wind.com>: > > 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 > >