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 <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?
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 >>> >
