Hi Olivier,

On 4/11/2017 1:28 AM, Olivier MATZ wrote:
Hi Hemant,

On Sun, 9 Apr 2017 13:29:46 +0530
Hemant Agrawal <hemant.agra...@nxp.com> wrote:

DPAA2 Hardware Mempool handlers allow enqueue/dequeue from NXP's
QBMAN hardware block.
CONFIG_RTE_MBUF_DEFAULT_MEMPOOL_OPS is set to 'dpaa2', if the pool
is enabled.

This memory pool currently supports packet mbuf type blocks only.

Signed-off-by: Hemant Agrawal <hemant.agra...@nxp.com>

[...]


--- a/drivers/bus/Makefile
+++ b/drivers/bus/Makefile
@@ -33,6 +33,10 @@ include $(RTE_SDK)/mk/rte.vars.mk

 core-libs := librte_eal librte_mbuf librte_mempool librte_ring librte_ether

+ifeq ($(CONFIG_RTE_LIBRTE_DPAA2_MEMPOOL),y)
+CONFIG_RTE_LIBRTE_FSLMC_BUS = $(CONFIG_RTE_LIBRTE_DPAA2_MEMPOOL)
+endif
+
 DIRS-$(CONFIG_RTE_LIBRTE_FSLMC_BUS) += fslmc
 DEPDIRS-fslmc = ${core-libs}


What's the purpose of this?
Not sure we are allowed to modify the configs in the Makefiles.

DPAA2_MEMPOOL will not work without the DPAA2 mempool hw instance detected on FSLMC_BUS. So, it is required that if you are enabling DPAA2_MEMPOOL, FSLMC_BUS is to be enabled.

Currently the config structure do not provide such dependency definitions.

This was done based on the suggestions on the initial patches from Ferruh and Jerin.




+       ret = dpbp_get_attributes(&avail_dpbp->dpbp, CMD_PRI_LOW,
+                                 avail_dpbp->token, &dpbp_attr);
+       if (ret != 0) {
+               PMD_INIT_LOG(ERR, "Resource read failure with"
+                            " err code: %d\n", ret);
+               p_ret = ret;
+               ret = dpbp_disable(&avail_dpbp->dpbp, CMD_PRI_LOW,
+                                  avail_dpbp->token);
+               return p_ret;
+       }
+
+       /* Allocate the bp_list which will be added into global_bp_list */
+       bp_list = (struct dpaa2_bp_list *)malloc(sizeof(struct dpaa2_bp_list));
+       if (!bp_list) {
+               PMD_INIT_LOG(ERR, "No heap memory available");
+               return -ENOMEM;
+       }
+

I think the cast is not needed.
Are you sure you want to use malloc() and not rte_malloc()? It would be in
hugepages.


Yes! you are right, cast is not required and rte_malloc will be better than malloc.

[...]





I still have some concerns about the fact that the mempool handler assumes that
the objects are necessarily mbufs. I guess for this reason it does not pass
mempool autotests?


Based on some of your previous suggestion, I have some thoughts to address it, but it will be longer term.

1. add some kind of capability APIs in mempool
2. Or, indicate to mempool to differentiate between mbuf pool vs non-mbuf pool.

I will initiate discussions on these topics.

We should probably move forward and let it go in 17.05, but this is something
that should be enhanced in my opinion.


Thanks for your valued suggestions and reviews. Yes! Once the basic stuff in in the 17.05, we will start working on making it more generic.


Regards,
Olivier




Reply via email to