From mboxrd@z Thu Jan  1 00:00:00 1970
From: Tetsuya Mukawa <mukawa-AlSX/UN32fvPDbFq/vQRIQ@public.gmane.org>
Subject: Re: [PATCH v14 12/13] eal/pci: Add
 rte_eal_dev_attach/detach() functions
Date: Wed, 25 Feb 2015 23:56:29 +0900
Message-ID: <54EDE29D.1050303@igel.co.jp>
References: <1424060073-23484-2-git-send-email-mukawa@igel.co.jp>
 <2987764.cYYQsyCFG8@xps13>
 <CAKU8wkeFtGvjdchjEYqd3jpvCYL-QhoWV9qLyWKKvnFK-LvS_A@mail.gmail.com>
 <2355791.9NkeVcuA9T@xps13>
Mime-Version: 1.0
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 7bit
Cc: dev-VfR2kkLFssw@public.gmane.org
To: Thomas Monjalon <thomas.monjalon-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>
Return-path: <dev-bounces-VfR2kkLFssw@public.gmane.org>
In-Reply-To: <2355791.9NkeVcuA9T@xps13>
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>

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

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