Re: [dpdk-dev] [dpdk-dev, 2/3] eth_dev: add support for device dma mask

[Alejandro Lucero <alejandro.lucero@netronome.com>]

Hi Jan

On Thu, May 12, 2016 at 3:52 PM, Jan Viktorin <viktorin@rehivetech.com>
wrote:

> Hello Alejandro,
>
> On Thu, 12 May 2016 15:33:59 +0100
> "Alejandro.Lucero" <alejandro.lucero@netronome.com> wrote:
>
> > - New dma_mask field in rte_eth_dev_data.
> >  - If PMD sets device dma_mask, call to check hugepages within
>
> I think that one of the purposes of DPDK is to support DMA transfers
> in userspace. In other words, I can see no reason to support dma_mask
> at the ethdev level only.
>
> The limitation is a device limitation so I can not see a better place for
adding the device dma mask.


> We should consider adding this to the generic struct rte_device
> (currently rte_pci_device). Thomas?
>
> I guess it could be a non-pci device with such a limitation. I though
rte_ethdev is more generic.


> >    supported range.
>
> I think, the '-' is unnecessary at the beginning of line. As for me
> I prefer a fluent text describing the purpose. The '-' is useful for
> a real list of notes.
>
> >
> > Signed-off-by: Alejandro Lucero <alejandro.lucero@netronome.com>
> >
> > ---
> > lib/librte_ether/rte_ethdev.c | 7 +++++++
> >  lib/librte_ether/rte_ethdev.h | 1 +
> >  2 files changed, 8 insertions(+)
> >
> > diff --git a/lib/librte_ether/rte_ethdev.c
> b/lib/librte_ether/rte_ethdev.c
> > index a31018e..c0de88a 100644
> > --- a/lib/librte_ether/rte_ethdev.c
> > +++ b/lib/librte_ether/rte_ethdev.c
> > @@ -280,9 +280,16 @@ rte_eth_dev_init(struct rte_pci_driver *pci_drv,
> >
> >       /* Invoke PMD device initialization function */
> >       diag = (*eth_drv->eth_dev_init)(eth_dev);
> > +     if (diag)
> > +             goto err;
> > +
> > +     if (eth_dev->data->dma_mask)
> > +             diag =
> rte_eal_hugepage_check_address_mask(eth_dev->data->dma_mask);
> > +
>
> I don't understand what happens if the memory is out of the DMA mask. What
> can the driver
> do? Does it just fail?
>
> I think that this should be considered during a malloc instead. (Well,
> there is probably
> no suitable API for this at the moment.)
>
> hugepage memory allocation is done before device initialization. I see
easier to leave the normal hugepage code as it is now and add a later call
if a device requires it.

The only reasonable thing to do is to fail as the amount of required memory
can not be (safely) allocated.


> Regards
> Jan
>
> >       if (diag == 0)
> >               return 0;
> >
> > +err:
> >       RTE_PMD_DEBUG_TRACE("driver %s: eth_dev_init(vendor_id=0x%u
> device_id=0x%x) failed\n",
> >                       pci_drv->name,
> >                       (unsigned) pci_dev->id.vendor_id,
> > diff --git a/lib/librte_ether/rte_ethdev.h
> b/lib/librte_ether/rte_ethdev.h
> > index 2757510..34daa92 100644
> > --- a/lib/librte_ether/rte_ethdev.h
> > +++ b/lib/librte_ether/rte_ethdev.h
> > @@ -1675,6 +1675,7 @@ struct rte_eth_dev_data {
> >       enum rte_kernel_driver kdrv;    /**< Kernel driver passthrough */
> >       int numa_node;  /**< NUMA node connection */
> >       const char *drv_name;   /**< Driver name */
> > +     uint64_t dma_mask; /** device supported address space range */
> >  };
> >
> >  /** Device supports hotplug detach */
>
>
>
> --
>    Jan Viktorin                  E-mail: Viktorin@RehiveTech.com
>    System Architect              Web:    www.RehiveTech.com
>    RehiveTech
>    Brno, Czech Republic
>