[dpdk-dev] [PATCH v14 12/13] eal/pci: Add rte_eal_dev_attach/detach() functions

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

If so, I will move rte_eal_dev_attach/detach to ethdev library.
And I will change names like rte_eth_dev_attach/detach.
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?

Thanks,
Tetsuya

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