Re: [dpdk-dev] [PATCH 1/2] drivers/mempool: add stack mempool handler as driver

[Olivier Matz <olivier.matz@6wind.com>]

Hi Shreyansh,

On Mon, 20 Mar 2017 15:33:09 +0530, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
> CONFIG_RTE_DRIVER_MEMPOOL_STACK option added to common_base.
> Stack mempool handler moved from lib/librte_mempool into drivers/mempool.
> 
> With this patch, the Stack mempool handler registration is optional and
> toggled via the configuration file. In case disabled (N), it would imply
> request for creating of mempool would fail.
> 
> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
> ---
>  config/common_base                                 |   5 +
>  drivers/Makefile                                   |   1 +
>  drivers/mempool/Makefile                           |  36 +++++
>  drivers/mempool/stack/Makefile                     |  55 ++++++++
>  drivers/mempool/stack/rte_mempool_stack.c          | 147 +++++++++++++++++++++
>  .../mempool/stack/rte_mempool_stack_version.map    |   4 +
>  lib/librte_mempool/Makefile                        |   1 -
>  lib/librte_mempool/rte_mempool_stack.c             | 147 ---------------------
>  8 files changed, 248 insertions(+), 148 deletions(-)
>  create mode 100644 drivers/mempool/Makefile
>  create mode 100644 drivers/mempool/stack/Makefile
>  create mode 100644 drivers/mempool/stack/rte_mempool_stack.c
>  create mode 100644 drivers/mempool/stack/rte_mempool_stack_version.map
>  delete mode 100644 lib/librte_mempool/rte_mempool_stack.c



I tried to pass the mempool autotest, and it issues a segfault.
I think the libraries are missing in rte.app.mk, so no handler is
registered.

Adding the following code in lib/librte_mempool/rte_mempool_ops.c
fixes the crash.

        ops = rte_mempool_get_ops(mp->ops_index);
+       if (ops == NULL || ops->alloc == NULL)
+               return -ENOTSUP;
        return ops->alloc(mp);

Now that drivers are not linked to the mempool library, it can
happen that there is no handler. Could you please add this patch in your
patchset?

I also checked that compilation works in shared lib mode. It looks good,
but I think there will be a behavior change if CONFIG_RTE_EAL_PMD_PATH
is empty (default). This option is probably used by distros to indicate
where dpdk plugins are installed, and when it is set, all drivers of
this directory are loaded, so in that case it won't change.

But when unset, the drivers have to be loaded manually, and with this
change, the mempool driver will have to be loaded with the -d eal option.
Could you please check what occurs in that case? At least see if it
displays a nice error or if it crashes. I suspect it will crash

Also, the MAINTAINERS file should be updated.


Thanks,
Olivier