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

Thomas Monjalon thomas.monjalon at 6wind.com
Wed Feb 25 15:00:16 CET 2015


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




More information about the dev mailing list